Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRE core should use private endpoint to access TRE management storage account #4353 #4360

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,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)

BUG FIXES:

Expand Down
2 changes: 1 addition & 1 deletion core/terraform/data.tf
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ data "azurerm_monitor_diagnostic_categories" "sb" {
depends_on = [
azurerm_servicebus_namespace.sb
]
}
}
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
3 changes: 3 additions & 0 deletions core/terraform/migrate.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 ../../devops/scripts/mgmtstorage_enable_public_access.sh

get_resource_id() {
local json_data="$1"
local resource_addr="$2"
Expand Down
3 changes: 3 additions & 0 deletions core/terraform/outputs.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#!/bin/bash
set -e

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

if [ ! -f ../tre_output.json ] || [ ! -s ../tre_output.json ]; then
# Connect to the remote backend of Terraform
export TF_LOG=""
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
3 changes: 3 additions & 0 deletions core/terraform/update_tags.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 ../../devops/scripts/mgmtstorage_enable_public_access.sh

script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

core_rg_rid=$(terraform show -json | jq -r '.values.root_module.resources[] | select(.address=="azurerm_resource_group.core") | .values.id')
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.3"
__version__ = "0.12.4"
130 changes: 130 additions & 0 deletions devops/scripts/mgmtstorage_enable_public_access.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
#!/bin/bash

#
# Add an exception to the TRE management storage account by making it public for deployment, and remove it on script exit.
#
# Note: Ensure you "source" this script, or else the EXIT trap won't fire at the right time.
#

function mgmtstorage_enable_public_access() {
local RESOURCE_GROUP
RESOURCE_GROUP=$(get_resource_group_name)

local SA_NAME
SA_NAME=$(get_storage_account_name)

# Check that the storage account exists before making changes
if ! does_storage_account_exist "$SA_NAME"; then
echo -e "Error: Storage account $SA_NAME does not exist.\n" >&2
exit 1
fi

# Pre-check: if public access is already enabled, no need to update.
if is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Storage account $SA_NAME is already publicly accessible\n"
return
fi

echo -e "\nEnabling public access on storage account $SA_NAME"

# Enable public network access with explicit default action allow
az storage account update --resource-group "$RESOURCE_GROUP" --name "$SA_NAME" --public-network-access Enabled --default-action Allow --output none

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

echo " Unable to confirm public access on storage account $SA_NAME after $ATTEMPT/10. Waiting for update to take effect..."
sleep 10
done

echo -e "Error: Could not enable public access for $SA_NAME after 10 attempts.\n"
exit 1
}

function mgmtstorage_disable_public_access() {
local RESOURCE_GROUP
RESOURCE_GROUP=$(get_resource_group_name)

local SA_NAME
SA_NAME=$(get_storage_account_name)

# Check that the storage account exists before making changes
if ! does_storage_account_exist "$SA_NAME"; then
echo -e "Error: Storage account $SA_NAME does not exist.\n" >&2
exit 1
fi

# Pre-check: if public access is already disabled, no need to update.
if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Storage account $SA_NAME is already not publicly accessible\n"
return
fi

echo -e "\nDisabling public access on storage account $SA_NAME"

# Disable public network access with explicit default action deny
az storage account update --resource-group "$RESOURCE_GROUP" --name "$SA_NAME" --public-network-access Disabled --default-action Deny --output none

for ATTEMPT in {1..10}; do
if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
echo -e " Public access has been disabled successfully\n"
return
fi

echo " Unable to confirm public access is disabled on storage account $SA_NAME after $ATTEMPT/10. Waiting for update to take effect..."
sleep 10
done

echo -e "Error: Could not disable public access for $SA_NAME after 10 attempts.\n"
exit 1
}

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

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

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

function is_public_access_enabled() {
local RESOURCE_GROUP="$1"
local SA_NAME="$2"

# Try listing containers
local containers
if ! containers=$(az storage container list --account-name "$SA_NAME" --auth-mode login --query "[].name" --output tsv); then
return 1
fi

# For each container found, check blob listing
for container in $containers; do
if ! az storage blob list --container-name "$container" --account-name "$SA_NAME" --auth-mode login --output none; then
return 1
fi
done

# If container list succeeded (even if empty) and blob list (if any) succeeded, public access is enabled
return 0
}

# Setup the trap to disable public access on exit
trap mgmtstorage_disable_public_access EXIT

# Enable public access for deployment
mgmtstorage_enable_public_access "$@"
3 changes: 3 additions & 0 deletions devops/scripts/terraform_wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ if [[ -z ${tf_logfile+x} ]]; then
echo -e "No logfile provided, using ${tf_logfile}\n"
fi

# shellcheck disable=SC1091
source "$(dirname "$0")/mgmtstorage_enable_public_access.sh"

terraform init -input=false -backend=true -reconfigure \
-backend-config="resource_group_name=${mgmt_resource_group_name}" \
-backend-config="storage_account_name=${mgmt_storage_account_name}" \
Expand Down
3 changes: 3 additions & 0 deletions devops/scripts/upgrade.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ else
KEY="${TRE_ID?}_${PARENT_DIR}"
fi

# shellcheck disable=SC1091
source "$(dirname "$0")/mgmtstorage_enable_public_access.sh"

# Run terraform init with upgrade and reconfigure options
terraform -chdir="$DIR/terraform" init -upgrade -reconfigure -input=false -backend=true \
-backend-config="resource_group_name=${TF_VAR_mgmt_resource_group_name}" \
Expand Down
3 changes: 3 additions & 0 deletions devops/terraform/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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_enable_public_access.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_enable_public_access.sh

PLAN_FILE="devops.tfplan"

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

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

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

terraform destroy -auto-approve
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"
Loading