Switch use of storage account keys by Terraform to use Azure AD authentication (#4104)

* Switch use of storage account keys to use Entra authentication where possible
Fixes #4103

* Fix health-services bundle

* Increase core version

* Add tags to fix linting.

* Fix linting

* # tflint-ignore-file: azurerm_resource_missing_tags

* Revert linting fixes

Author: marrobi

.github/actions/devcontainer_run_command/action.yml

@@ -186,6 +186,9 @@ runs:
           -e TF_INPUT="0" \
           -e TF_IN_AUTOMATION="1" \
           -e USE_ENV_VARS_NOT_FILES="true" \
+          -e ARM_STORAGE_USE_AZUREAD="true" \
+          -e ARM_USE_AZUREAD="true" \
+          -e ARM_USE_OIDC="true" \
           -e BUNDLE_TYPE="${{ inputs.BUNDLE_TYPE }}" \
           -e WORKSPACE_SERVICE_NAME="${{ inputs.WORKSPACE_SERVICE_NAME }}" \
           -e ARM_ENVIRONMENT="${{ env.ARM_ENVIRONMENT }}" \

CHANGELOG.md

@@ -7,6 +7,8 @@ FEATURES:
 
 ENHANCEMENTS:
 * Expose APP_SERVICE_SKU build variable to allow enablement of App Gateway WAF ([#4111](https://github.com/microsoft/AzureTRE/pull/4111))
+* Update Terraform to use Azure AD authentication rather than storage account keys ([#4103](https://github.com/microsoft/AzureTRE/issues/4103))
+
 BUG FIXES:
 
 COMPONENTS:

core/terraform/migrate.sh

@@ -5,7 +5,14 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
-terraform_wrapper_path="../../devops/scripts/terraform_wrapper.sh"
+# Configure AzureRM provider to user Azure AD to connect to storage accounts
+export ARM_STORAGE_USE_AZUREAD=true
+
+# Configure AzureRM backend to user Azure AD to connect to storage accounts
+export ARM_USE_AZUREAD=true
+export ARM_USE_OIDC=true
+
+# terraform_wrapper_path="../../devops/scripts/terraform_wrapper.sh"
 
 # This variables are loaded in for us
 # shellcheck disable=SC2154
@@ -16,214 +23,42 @@ terraform init -input=false -backend=true -reconfigure \
     -backend-config="key=${TRE_ID}"
 
 echo "*** Migrating TF Resources... ***"
-# terraform show might fail if provider schema has changed. Since we don't call apply at this stage a refresh is needed
-terraform refresh
 
-# 1. Check we have a root_module in state
-# 2. Grab the Resource ID
-# 3. Delete the old resource from state
-# 4. Import the new resource type in using the existing Azure Resource ID
 
 terraform_show_json=$(terraform show -json)
 
-# azurerm_app_service_plan -> azurerm_service_plan
-core_app_service_plan_id=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_app_service_plan.core") | .values.id')
-if [ -n "${core_app_service_plan_id}" ]; then
-  echo "Migrating ${core_app_service_plan_id}"
-  terraform state rm azurerm_app_service_plan.core
-  if [[ $(az resource list --query "[?id=='${core_app_service_plan_id}'] | length(@)") == 0 ]];
-  then
-    echo "The resource doesn't exist on Azure. Skipping importing it back to state."
-  else
-    terraform import azurerm_service_plan.core "${core_app_service_plan_id}"
-  fi
-fi
-
-# azurerm_app_service -> azurerm_linux_web_app
-api_app_service_id=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_app_service.api") | .values.id')
-if [ -n "${api_app_service_id}" ]; then
-  echo "Migrating ${api_app_service_id}"
-  terraform state rm azurerm_app_service.api
-  if [[ $(az resource list --query "[?id=='${api_app_service_id}'] | length(@)") == 0 ]];
-  then
-    echo "The resource doesn't exist on Azure. Skipping importing it back to state."
-  else
-    terraform import azurerm_linux_web_app.api "${api_app_service_id}"
-  fi
-fi
-
-# app insights via -> native tf resource
-app_insights_via_arm=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.child_modules != null) .values.root_module.child_modules[] | select (.address=="module.azure_monitor") | .resources[] | select(.address=="module.azure_monitor.azurerm_resource_group_template_deployment.app_insights_core") | .values.id')
-if [ -n "${app_insights_via_arm}" ]; then
-  echo "Migrating ${app_insights_via_arm}"
-
-  PLAN_FILE="tfplan$$"
-  TS=$(date +"%s")
-  LOG_FILE="${TS}-tre-core-migrate.log"
-
-  # This variables are loaded in for us
-  # shellcheck disable=SC2154
-  "${terraform_wrapper_path}" \
-    -g "${TF_VAR_mgmt_resource_group_name}" \
-    -s "${TF_VAR_mgmt_storage_account_name}" \
-    -n "${TF_VAR_terraform_state_container_name}" \
-    -k "${TRE_ID}" \
-    -l "${LOG_FILE}" \
-    -c "terraform plan -target module.azure_monitor.azurerm_resource_group_template_deployment.app_insights_core -target module.azure_monitor.azurerm_resource_group_template_deployment.ampls_core -out ${PLAN_FILE} && \
-    terraform apply -input=false -auto-approve ${PLAN_FILE}"
-fi
-
-# support downgrading core app service plan
-core_plan=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_service_plan.core") | .values.id')
-api_diag=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_monitor_diagnostic_setting.webapp_api") | .values.id')
-if [ -n "${core_plan}" ] && [ -n "${api_diag}" ]; then
-  set +o errexit
-  terraform plan -target "azurerm_service_plan.core" -detailed-exitcode
-  plan_exit_code=$?
-  set -o errexit
-
-  if [ "${plan_exit_code}" == "2" ]; then
-    echo "Migrating ${api_diag}"
-    PLAN_FILE="tfplan$$"
-    TS=$(date +"%s")
-    LOG_FILE="${TS}-tre-core-migrate.log"
-
-    # This variables are loaded in for us
-    # shellcheck disable=SC2154
-    "${terraform_wrapper_path}" \
-      -g "${TF_VAR_mgmt_resource_group_name}" \
-      -s "${TF_VAR_mgmt_storage_account_name}" \
-      -n "${TF_VAR_terraform_state_container_name}" \
-      -k "${TRE_ID}" \
-      -l "${LOG_FILE}" \
-      -c "terraform plan -destroy -target azurerm_monitor_diagnostic_setting.webapp_api -out ${PLAN_FILE} && \
-      terraform apply -input=false -auto-approve ${PLAN_FILE}"
-  fi
-fi
-
-# remove app insights profiler storage account
-app_insights_byo_storage=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.child_modules != null) .values.root_module.child_modules[] | select (.address=="module.azure_monitor") | .resources[] | select(.address=="module.azure_monitor.azurerm_resource_group_template_deployment.app_insights_byo_storage") | .values.id')
-if [ -n "${app_insights_byo_storage}" ]; then
-  echo "Removing state of app_insights_byo_storage"
-  terraform state rm module.azure_monitor.azurerm_resource_group_template_deployment.app_insights_byo_storage
-fi
-
-# airlock inline vnet integration (instead of via swift)
-airlock_vnet_integration=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.child_modules != null) .values.root_module.child_modules[] | select (.address=="module.airlock_resources") | .resources[] | select(.address=="module.airlock_resources.azurerm_app_service_virtual_network_swift_connection.airlock_integrated_vnet") | .values.id')
-if [ -n "${airlock_vnet_integration}" ]; then
-  echo "Migrating ${airlock_vnet_integration}"
-
-  PLAN_FILE="tfplan$$"
-  TS=$(date +"%s")
-  LOG_FILE="${TS}-tre-core-migrate.log"
-
-  # This variables are loaded in for us
-  # shellcheck disable=SC2154
-  "${terraform_wrapper_path}" \
-    -g "${TF_VAR_mgmt_resource_group_name}" \
-    -s "${TF_VAR_mgmt_storage_account_name}" \
-    -n "${TF_VAR_terraform_state_container_name}" \
-    -k "${TRE_ID}" \
-    -l "${LOG_FILE}" \
-    -c "terraform plan -target module.airlock_resources.azurerm_app_service_virtual_network_swift_connection.airlock_integrated_vnet -out ${PLAN_FILE} && \
-    terraform apply -input=false -auto-approve ${PLAN_FILE}"
-fi
-
-# api inline vnet integration (instead of via swift)
-api_vnet_integration=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_app_service_virtual_network_swift_connection.api_integrated_vnet") | .values.id')
-if [ -n "${api_vnet_integration}" ]; then
-  echo "Migrating ${api_vnet_integration}"
-
-  PLAN_FILE="tfplan$$"
-  TS=$(date +"%s")
-  LOG_FILE="${TS}-tre-core-migrate.log"
+# Remove cnab-state legacy state path form state. Needs to be run before refresh, as refresh will fail.
+state_store_legacy_path=$(echo "${terraform_show_json}" \
+   | jq 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_storage_share.storage_state_path") | .values.id')
 
-  # This variables are loaded in for us
-  # shellcheck disable=SC2154
-  "${terraform_wrapper_path}" \
-    -g "${TF_VAR_mgmt_resource_group_name}" \
-    -s "${TF_VAR_mgmt_storage_account_name}" \
-    -n "${TF_VAR_terraform_state_container_name}" \
-    -k "${TRE_ID}" \
-    -l "${LOG_FILE}" \
-    -c "terraform plan -target azurerm_app_service_virtual_network_swift_connection.api_integrated_vnet -out ${PLAN_FILE} && \
-    terraform apply -input=false -auto-approve ${PLAN_FILE}"
+if [ -n "${state_store_legacy_path}" ]; then
+  echo -e "\n\e[96mRemoving legacy state path from TF state\e[0m..."
+  terraform state rm azurerm_storage_share.storage_state_path
 fi
 
-# support changing the resource processor subnet size
-rp_subnet=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.child_modules != null) .values.root_module.child_modules[] | select (.address=="module.network") | .resources[] | select(.address=="module.network.azurerm_subnet.resource_processor") | .values.id')
-if [ -n "${rp_subnet}" ]; then
-  set +o errexit
-  terraform plan -target "module.network.azurerm_subnet.resource_processor" -detailed-exitcode
-  plan_exit_code=$?
-  set -o errexit
-
-  if [ "${plan_exit_code}" == "2" ]; then
-    echo "Migrating ${rp_subnet}"
-    PLAN_FILE="tfplan$$"
-    TS=$(date +"%s")
-    LOG_FILE="${TS}-tre-core-migrate-rp-subnet.log"
-
-    # This variables are loaded in for us
-    # shellcheck disable=SC2154
-    "${terraform_wrapper_path}" \
-      -g "${TF_VAR_mgmt_resource_group_name}" \
-      -s "${TF_VAR_mgmt_storage_account_name}" \
-      -n "${TF_VAR_terraform_state_container_name}" \
-      -k "${TRE_ID}" \
-      -l "${LOG_FILE}" \
-      -c "terraform plan -destroy -target module.resource_processor_vmss_porter[0].azurerm_linux_virtual_machine_scale_set.vm_linux \
-      -target azurerm_private_endpoint.sbpe \
-      -target azurerm_private_endpoint.mongo \
-      -out ${PLAN_FILE} && \
-      terraform apply -input=false -auto-approve ${PLAN_FILE}"
-  fi
-fi
-
-# DNS Zones migration. We can't use a moved block due the the vars being used.
-nexus_dns_zone=$(echo "${terraform_show_json}" \
-  | jq -r 'select(.values.root_module.child_modules != null) .values.root_module.child_modules[] | select (.address=="module.network") | .resources[] | select(.address=="module.network.azurerm_private_dns_zone.nexus") | .values.id')
-if [ -n "${nexus_dns_zone}" ]; then
-  terraform state rm module.network.azurerm_private_dns_zone.nexus
-  terraform import azurerm_private_dns_zone.non_core[\""nexus-${TRE_ID}.${LOCATION}.cloudapp.azure.com"\"] "${nexus_dns_zone}"
-fi
+# terraform show might fail if provider schema has changed. Since we don't call apply at this stage a refresh is needed
+terraform refresh
 
-# Additional DNS Zones migration. We changed the name for the nexus dns zone hence we need to apply the change.
-NEXUS_DNS_NAME="nexus-${TRE_ID}.${LOCATION}.cloudapp.azure.com"
-nexus_dns_zone_changed=$(echo "${terraform_show_json}" \
-  |  jq -r --arg nexus_dns_name "$NEXUS_DNS_NAME" 'select(.values.root_module.resources != null) .values.root_module.resources[] | select (.address=="azurerm_private_dns_zone.non_core[\""+$nexus_dns_name+"\"]") | .values.id')
-if [ -n "${nexus_dns_zone_changed}" ]; then
-  terraform state rm azurerm_private_dns_zone.non_core[\""nexus-${TRE_ID}.${LOCATION}.cloudapp.azure.com"\"]
-  terraform import azurerm_private_dns_zone.nexus "${nexus_dns_zone_changed}"
-fi
+# 1. Check we have a root_module in state
+# 2. Grab the Resource ID
+# 3. Delete the old resource from state
+# 4. Import the new resource type in using the existing Azure Resource ID
 
-# this isn't a classic migration, but impacts how terraform handles the deployment in the next phase
-state_store_serverless=$(echo "${terraform_show_json}" \
-  | jq 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_cosmosdb_account.tre_db_account") | any(.values.capabilities[]; .name=="EnableServerless")')
-# false = resource EXITS in the state WITHOUT the serverless capability.
-# true = exists with the capability, empty value = resource doesn't exist.
-if [ "${state_store_serverless}" == "false" ]; then
-  echo "Identified CosmosDB with defined throughput."
-  TF_VAR_is_cosmos_defined_throughput="true"
-  export TF_VAR_is_cosmos_defined_throughput
-fi
+terraform_show_json=$(terraform show -json)
 
-# prep for migration of azurerm_servicebus_namespace_network_rule_set https://github.com/microsoft/AzureTRE/pull/3858
-# as described https://github.com/hashicorp/terraform-provider-azurerm/issues/23954
-state_store_servicebus_network_rule_set=$(echo "${terraform_show_json}" \
-  | jq 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_servicebus_namespace_network_rule_set.servicebus_network_rule_set") | .values.id')
-if [ -n "${state_store_servicebus_network_rule_set}" ]; then
-  echo "Removing state of azurerm_servicebus_namespace_network_rule_set"
-  terraform state rm azurerm_servicebus_namespace_network_rule_set.servicebus_network_rule_set
-fi
+# example migration
+# # azurerm_app_service_plan -> azurerm_service_plan
+# core_app_service_plan_id=$(echo "${terraform_show_json}" \
+#   | jq -r 'select(.values.root_module.resources != null) | .values.root_module.resources[] | select(.address=="azurerm_app_service_plan.core") | .values.id')
+# if [ -n "${core_app_service_plan_id}" ]; then
+#   echo "Migrating ${core_app_service_plan_id}"
+#   terraform state rm azurerm_app_service_plan.core
+#   if [[ $(az resource list --query "[?id=='${core_app_service_plan_id}'] | length(@)") == 0 ]];
+#   then
+#     echo "The resource doesn't exist on Azure. Skipping importing it back to state."
+#   else
+#     terraform import azurerm_service_plan.core "${core_app_service_plan_id}"
+#   fi
+# fi
 
 echo "*** Migration is done. ***"

core/terraform/resource_processor/vmss_porter/data.tf

@@ -45,3 +45,7 @@ data "template_cloudinit_config" "config" {
   }
 }
 
+data "azurerm_storage_account" "mgmt_storage" {
+  name                = var.mgmt_storage_account_name
+  resource_group_name = var.mgmt_resource_group_name
+}

core/terraform/resource_processor/vmss_porter/main.tf

@@ -151,6 +151,12 @@ resource "terraform_data" "vm_linux_reimage" {
   ]
 }
 
+resource "azurerm_role_assignment" "mgmt_storage_account_blob_contributor" {
+  scope                = data.azurerm_storage_account.mgmt_storage.id
+  role_definition_name = "Storage Blob Data Contributor"
+  principal_id         = azurerm_user_assigned_identity.vmss_msi.principal_id
+}
+
 resource "azurerm_role_assignment" "vmss_acr_pull" {
   scope                = var.acr_id
   role_definition_name = "AcrPull"

core/terraform/storage.tf

@@ -9,12 +9,6 @@ resource "azurerm_storage_account" "stg" {
   lifecycle { ignore_changes = [tags] }
 }
 
-resource "azurerm_storage_share" "storage_state_path" {
-  name                 = "cnab-state"
-  storage_account_name = azurerm_storage_account.stg.name
-  quota                = 50
-}
-
 resource "azurerm_private_endpoint" "blobpe" {
   name                = "pe-blob-${var.tre_id}"
   location            = azurerm_resource_group.core.location

core/version.txt

@@ -1 +1 @@
-__version__ = "0.10.10"
+__version__ = "0.10.11"

devops/scripts/increment_porter_versions.sh

@@ -0,0 +1,27 @@
+#!/bin/bash
+
+# Directory containing the porter.yaml files
+TEMPLATES_DIR="./templates"
+
+# Function to increment the version number
+increment_version() {
+  local version=$1
+  local major minor patch
+  IFS='.' read -r major minor patch <<< "$version"
+  patch=$((patch + 1))
+  echo "$major.$minor.$patch"
+}
+
+# Find all porter.yaml files in the templates directory
+find "$TEMPLATES_DIR" -name "porter.yaml" | while read -r file; do
+  # Read the current version from the file
+  current_version=$(grep -E '^version: ' "$file" | awk '{print $2}')
+
+  # Increment the version number
+  new_version=$(increment_version "$current_version")
+
+  # Update the version number in the file
+  sed -i "s/version: $current_version/version: $new_version/" "$file"
+
+  echo "Updated $file from version $current_version to $new_version"
+done

devops/scripts/load_and_validate_env.sh

@@ -83,6 +83,11 @@ else
 
     TRE_URL=$(construct_tre_url "${TRE_ID}" "${LOCATION}" "${AZURE_ENVIRONMENT}")
     export TRE_URL
+
+    # Configure AzureRM provider and backend to use Azure AD to connect to storage accounts
+    export ARM_STORAGE_USE_AZUREAD=true
+    export ARM_USE_AZUREAD=true
+    export ARM_USE_OIDC=true
 fi
 
 # if local debugging is configured, then set vars required by ~/.porter/config.yaml

devops/scripts/terraform_wrapper.sh

@@ -107,14 +107,14 @@ do
 
     # upload the log file?
     if [[ $TF_LOG == "DEBUG" ]] ; then
-      az storage blob upload --file $tf_logfile \
+      az storage blob upload --file "$tf_logfile" \
         --container-name "tflogs" \
         --account-name "$mgmt_storage_account_name" \
         --auth-mode key
     fi
 
-    LOCKED_STATE=$(cat ${tf_logfile} |  grep -c 'Error acquiring the state lock') || true;
-    TF_ERROR=$(cat ${tf_logfile} |  grep -c 'COMMAND_EXIT_CODE="1"') || true;
+    LOCKED_STATE=$(cat < "$tf_logfile" |  grep -c 'Error acquiring the state lock') || true;
+    TF_ERROR=$(cat < "$tf_logfile" |  grep -c 'COMMAND_EXIT_CODE="1"') || true;
     if [[ $LOCKED_STATE -gt 0  ]];
     then
         RUN_COMMAND=1