Skip to content

Commit

Permalink
Added changes for network exception in management storage account dur…
Browse files Browse the repository at this point in the history
…ing deployment. Also changed subnet for resource processor private endpoint
  • Loading branch information
Ashis Kar committed Feb 14, 2025
1 parent 4a673c1 commit 7cb8dca
Show file tree
Hide file tree
Showing 17 changed files with 185 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
**BREAKING CHANGES & MIGRATIONS**:

ENHANCEMENTS:
* Deny public access to TRE management storage account, and add private endpoint for TRE core [#4353](https://github.com/microsoft/AzureTRE/issues/4353)
* Core key vault firewall should not be set to "Allow public access from all networks" ([#4250](https://github.com/microsoft/AzureTRE/issues/4250))
* Allow workspace App Service Plan SKU to be updated ([#4331](https://github.com/microsoft/AzureTRE/issues/4331))
* Add core requests endpoint and UI to enable requests to be managed TRE wide. ([[#2510](https://github.com/microsoft/AzureTRE/issues/2510)])
Expand Down
5 changes: 0 additions & 5 deletions core/terraform/data.tf
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,4 @@ data "azurerm_monitor_diagnostic_categories" "sb" {
depends_on = [
azurerm_servicebus_namespace.sb
]
}

data "azurerm_storage_account" "mgmt_storage" {
name = var.mgmt_storage_account_name
resource_group_name = var.mgmt_resource_group_name
}
3 changes: 3 additions & 0 deletions core/terraform/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ set -o nounset
# shellcheck disable=SC1091
source "../../devops/scripts/kv_add_network_exception.sh"

# shellcheck disable=SC1091
source ../../devops/scripts/mgmtstorage_add_network_exception.sh

# This is where we can migrate any Terraform before we plan and apply
# For instance deprecated Terraform resources
# shellcheck disable=SC1091
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/destroy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ set -o nounset
# shellcheck disable=SC1091
source "../../devops/scripts/kv_add_network_exception.sh"

# shellcheck disable=SC1091
source ../../devops/scripts/mgmtstorage_add_network_exception.sh

# These variables are loaded in for us
# shellcheck disable=SC2154
../../devops/scripts/terraform_wrapper.sh -g "${TF_VAR_mgmt_resource_group_name}" \
Expand Down
2 changes: 0 additions & 2 deletions core/terraform/locals.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ locals {

docker_registry_server = data.azurerm_container_registry.mgmt_acr.login_server

mgmt_storage_account_id = data.azurerm_storage_account.mgmt_storage.id

# https://learn.microsoft.com/en-us/azure/cosmos-db/how-to-configure-firewall#allow-requests-from-the-azure-portal

azure_portal_cosmos_ips_list = [
Expand Down
1 change: 1 addition & 0 deletions core/terraform/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ module "resource_processor_vmss_porter" {
acr_id = data.azurerm_container_registry.mgmt_acr.id
app_insights_connection_string = module.azure_monitor.app_insights_connection_string
resource_processor_subnet_id = module.network.resource_processor_subnet_id
blob_core_dns_zone_id = module.network.blob_core_dns_zone_id
docker_registry_server = local.docker_registry_server
resource_processor_vmss_porter_image_repository = var.resource_processor_vmss_porter_image_repository
service_bus_namespace_id = azurerm_servicebus_namespace.sb.id
Expand Down
21 changes: 21 additions & 0 deletions core/terraform/resource_processor/vmss_porter/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,27 @@ resource "azurerm_role_assignment" "vmss_kv_encryption_key_user" {
principal_id = azurerm_user_assigned_identity.vmss_msi.principal_id
}

resource "azurerm_private_endpoint" "mgmtblobpe" {
name = "pe-mgmt-blob-${var.tre_id}"
location = var.location
resource_group_name = var.mgmt_resource_group_name
subnet_id = var.resource_processor_subnet_id
tags = local.tre_core_tags
lifecycle { ignore_changes = [tags] }

private_dns_zone_group {
name = "private-dns-zone-group-blobcore"
private_dns_zone_ids = [var.blob_core_dns_zone_id]
}

private_service_connection {
name = "psc-mgmt-${var.tre_id}"
private_connection_resource_id = data.azurerm_storage_account.mgmt_storage.id
is_manual_connection = false
subresource_names = ["Blob"]
}
}

module "terraform_azurerm_environment_configuration" {
source = "git::https://github.com/microsoft/terraform-azurerm-environment-configuration.git?ref=0.2.0"
arm_environment = var.arm_environment
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/resource_processor/vmss_porter/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ variable "resource_group_name" {
variable "resource_processor_subnet_id" {
type = string
}
variable "blob_core_dns_zone_id" {
type = string
}
variable "resource_processor_vmss_porter_image_repository" {
type = string
}
Expand Down
28 changes: 1 addition & 27 deletions core/terraform/storage.tf
Original file line number Diff line number Diff line change
Expand Up @@ -35,32 +35,6 @@ resource "azurerm_storage_account" "stg" {
lifecycle { ignore_changes = [infrastructure_encryption_enabled, tags] }
}

resource "azurerm_private_endpoint" "mgmtblobpe" {
name = "pe-mgmt-blob-${var.tre_id}"
location = azurerm_resource_group.core.location
resource_group_name = azurerm_resource_group.core.name
subnet_id = module.network.shared_subnet_id
tags = local.tre_core_tags
lifecycle { ignore_changes = [tags] }

private_dns_zone_group {
name = "private-dns-zone-group-blobcore"
private_dns_zone_ids = [module.network.blob_core_dns_zone_id]
}

private_service_connection {
name = "psc-mgmt-${var.tre_id}"
private_connection_resource_id = local.mgmt_storage_account_id
is_manual_connection = false
subresource_names = ["Blob"]
}

# private endpoints in serial
depends_on = [
azurerm_private_endpoint.kvpe
]
}

resource "azurerm_private_endpoint" "blobpe" {
name = "pe-blob-${var.tre_id}"
location = azurerm_resource_group.core.location
Expand All @@ -83,7 +57,7 @@ resource "azurerm_private_endpoint" "blobpe" {

# private endpoints in serial
depends_on = [
azurerm_private_endpoint.mgmtblobpe
azurerm_private_endpoint.kvpe
]
}

Expand Down
2 changes: 1 addition & 1 deletion core/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.12.2"
__version__ = "0.12.3"
129 changes: 129 additions & 0 deletions devops/scripts/mgmtstorage_add_network_exception.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
#!/bin/bash

#
# Add an IP exception to the TRE management storage account for deployment, and remove it on script exit.
# Uses the current machine's IP or $PUBLIC_DEPLOYMENT_IP_ADDRESS if set.
#
# Note: Ensure you "source" this script, or else the EXIT trap won't fire at the right time.
#

function mgmtstorage_add_network_exception() {
local RESOURCE_GROUP=$(get_resource_group_name)
local SA_NAME=$(get_storage_account_name)
local MY_IP=$(get_my_ip)

echo -e "\nAdding deployment network exception to storage account $SA_NAME..."

# Ensure storage account exists
if ! does_storage_account_exist "$SA_NAME"; then
echo -e "Error: Storage account $SA_NAME does not exist.\n"
return 0 # Don't cause outer sourced script to fail
fi

# Add storage account network exception
az storage account network-rule add --resource-group "$RESOURCE_GROUP" --account-name "$SA_NAME" --ip-address "$MY_IP" --output none

for ATTEMPT in {1..10}; do
if is_ip_in_network_rule "$RESOURCE_GROUP" "$SA_NAME" "$MY_IP"; then
echo -e " Storage account $SA_NAME is now accessible\n"
return
fi

echo " Unable to access storage account $SA_NAME after $ATTEMPT/10. Waiting for network rules to take effect..."
sleep 5
done

echo -e "Error: Could not add deployment network exception for $SA_NAME after 10 attempts.\n"
exit 1
}

function mgmtstorage_remove_network_exception() {
local RESOURCE_GROUP=$(get_resource_group_name)
local SA_NAME=$(get_storage_account_name)
local MY_IP=$(get_my_ip)

echo -e "\nRemoving deployment network exception from storage account $SA_NAME..."

# Ensure storage account exists
if ! does_storage_account_exist "$SA_NAME"; then
echo -e "Error: Storage account $SA_NAME does not exist.\n"
return 0 # Don't cause outer sourced script to fail
fi

# Remove storage account network exception
az storage account network-rule remove --resource-group "$RESOURCE_GROUP" --account-name "$SA_NAME" --ip-address "$MY_IP" --output none

for ATTEMPT in {1..10}; do
if ! is_ip_in_network_rule "$RESOURCE_GROUP" "$SA_NAME" "$MY_IP"; then
echo -e " Deployment network exception removed successfully\n"
return
fi

echo " Unable to remove network exception for storage account $SA_NAME after $ATTEMPT/10. Waiting for network rules to take effect..."
sleep 5
done

echo -e "Error: Could not remove deployment network exception for $SA_NAME after 10 attempts.\n"
exit 1
}

function get_resource_group_name() {
if [[ -z "${MGMT_RESOURCE_GROUP_NAME:-}" ]]; then
echo -e "Error: MGMT_RESOURCE_GROUP_NAME is not set\nExiting...\n" >&2
exit 1
fi
echo "$MGMT_RESOURCE_GROUP_NAME"
}

function get_storage_account_name() {
if [[ -z "${MGMT_STORAGE_ACCOUNT_NAME:-}" ]]; then
echo -e "Error: MGMT_STORAGE_ACCOUNT_NAME is not set\nExiting...\n" >&2
exit 1
fi
echo "$MGMT_STORAGE_ACCOUNT_NAME"
}

function get_my_ip() {
if [[ -n "${PUBLIC_DEPLOYMENT_IP_ADDRESS:-}" ]]; then
echo "$PUBLIC_DEPLOYMENT_IP_ADDRESS"
else
local MY_IP
MY_IP=$(curl -s "https://ipecho.net/plain") || { echo "Error: Failed to fetch IP address" >&2; exit 1; }

if [[ -z "$MY_IP" ]]; then
echo "Error: Could not determine IP address." >&2
exit 1
fi

echo "$MY_IP"
fi
}

function does_storage_account_exist() {
[[ -n "$(az storage account show --name "$1" --query "id" --output tsv)" ]]
}

function is_ip_in_network_rule() {
local RESOURCE_GROUP="$1"
local SA_NAME="$2"
local MY_IP="$3"

# Step 1: Check if the IP is present in the network rules
local COUNT
COUNT=$(az storage account network-rule list --resource-group "$RESOURCE_GROUP" --account-name "$SA_NAME" --query "length(ipRules[?ipAddressOrRange=='$MY_IP'])" --output tsv)

if [[ "$COUNT" -gt 0 ]]; then
# Step 2: Try accessing storage to confirm access
if az storage container list --account-name "$SA_NAME" --auth-mode login --output none 2>/dev/null; then
return 0 # Success: IP is in rules AND access is confirmed
fi
fi

return 1 # Either rule not added or access is still restricted
}

# Setup the trap to remove the network exception on exit
trap mgmtstorage_remove_network_exception EXIT

# Add the network exception
mgmtstorage_add_network_exception "$@"
5 changes: 5 additions & 0 deletions devops/terraform/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ if ! az storage account show --resource-group "$TF_VAR_mgmt_resource_group_name"
--name "$TF_VAR_mgmt_storage_account_name" --location "$LOCATION" \
--allow-blob-public-access false --min-tls-version TLS1_2 \
--kind StorageV2 --sku Standard_LRS -o table \
--default-action Deny \
--bypass AzureServices \
--encryption-key-type-for-queue "$encryption_type" \
--encryption-key-type-for-table "$encryption_type" \
--require-infrastructure-encryption true
Expand All @@ -28,6 +30,9 @@ else
az storage account show --resource-group "$TF_VAR_mgmt_resource_group_name" --name "$TF_VAR_mgmt_storage_account_name" --output table
fi

# shellcheck disable=SC1091
source ../scripts/mgmtstorage_add_network_exception.sh

# Grant user blob data contributor permissions
echo -e "\n\e[34m»»» 🔑 \e[96mGranting Storage Blob Data Contributor role to the current user\e[0m..."
if [ -n "${ARM_CLIENT_ID:-}" ]; then
Expand Down
3 changes: 3 additions & 0 deletions devops/terraform/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ set -o pipefail
set -o nounset
# set -o xtrace

# shellcheck disable=SC1091
source ../scripts/mgmtstorage_add_network_exception.sh

PLAN_FILE="devops.tfplan"

terraform init -input=false -backend=true -reconfigure
Expand Down
10 changes: 10 additions & 0 deletions devops/terraform/destroy.sh
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
#!/bin/bash

set -o errexit
set -o pipefail
set -o nounset
# set -o xtrace

# shellcheck disable=SC1091
source ../scripts/mgmtstorage_add_network_exception.sh

terraform destroy -auto-approve
3 changes: 2 additions & 1 deletion devops/terraform/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ resource "azurerm_storage_account" "state_storage" {

network_rules {
default_action = "Deny"
ip_rules = [local.myip]
bypass = ["AzureServices"]
ip_rules = [local.myip] # Exception for deployment IP. This is removed in mgmtstorage_add_network_exception.sh
}

dynamic "identity" {
Expand Down
2 changes: 1 addition & 1 deletion devops/terraform/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ variable "kv_mgmt_encryption_key_name" {
}

variable "public_deployment_ip_address" {
description = "Your local IP address if https://ipecho.net/plain is blocked."
description = "Your local IP address if automatic detection via https://ipecho.net/plain is blocked (e.g., due to network restrictions)"
type = string
default = ""
}
2 changes: 1 addition & 1 deletion devops/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.5.5"
__version__ = "0.5.6"

0 comments on commit 7cb8dca

Please sign in to comment.