From 778978b5f1b9dc2f304b81ee7f9c7a66c49e40b7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 8 Sep 2026 09:47:56 +0200 Subject: [PATCH] sec(iac/azure): pull and push ACR with Entra identities, disable the admin account The Container App pulled its image with the registry's admin username and password, which put a push-capable credential in Terraform state, in the app's secrets, and in every deploy log, while the AcrPull grant to the app's managed identity governed nothing. Switch the registry block to the user-assigned identity, move the AcrPull assignment into the container-apps module (moved block, no recreate) so the app can depend on it, log the CI/local image push in with az acr login as the caller's Entra identity, and set admin_enabled = false. Closes #1970 Refs #1623 Co-Authored-By: claude-flow Claude-Session: https://claude.ai/code/session_01Fu9uWjxtDFx5HDKeMRt1jC --- terraform/environments/azure/build.tf | 5 +-- terraform/environments/azure/compute.tf | 7 ++-- terraform/environments/azure/registry.tf | 14 ++++---- .../compute/azure/container-apps/main.tf | 33 +++++++++---------- .../compute/azure/container-apps/variables.tf | 13 ++------ 5 files changed, 30 insertions(+), 42 deletions(-) diff --git a/terraform/environments/azure/build.tf b/terraform/environments/azure/build.tf index b921b867e..ad237360f 100644 --- a/terraform/environments/azure/build.tf +++ b/terraform/environments/azure/build.tf @@ -20,8 +20,9 @@ module "build" { source_path = "${path.root}/../../.." # Root of the project (where Dockerfile is) # platform not set — auto-detected from builder host (Container Apps and AKS support arm64 and amd64) - # Registry login for ACR using admin credentials - registry_login_command = "echo '${nonsensitive(azurerm_container_registry.main.admin_password)}' | docker login ${azurerm_container_registry.main.login_server} -u ${azurerm_container_registry.main.admin_username} --password-stdin" + # Registry login with the caller's Entra identity (deploy SP in CI, az login + # locally); the principal needs AcrPush on the registry. + registry_login_command = "az acr login --name ${azurerm_container_registry.main.name}" # Build options skip_docker_build = false diff --git a/terraform/environments/azure/compute.tf b/terraform/environments/azure/compute.tf index 7f2ecd011..042024b58 100644 --- a/terraform/environments/azure/compute.tf +++ b/terraform/environments/azure/compute.tf @@ -97,10 +97,9 @@ module "compute_container_apps" { }, var.additional_env_vars ) - # ACR registry credentials for image pull - registry_server = azurerm_container_registry.main.login_server - registry_username = azurerm_container_registry.main.admin_username - registry_password = azurerm_container_registry.main.admin_password + # Image pulls authenticate with the app's managed identity (AcrPull inside the module) + registry_server = azurerm_container_registry.main.login_server + container_registry_id = azurerm_container_registry.main.id # Scheduled tasks (Logic Apps) # diff --git a/terraform/environments/azure/registry.tf b/terraform/environments/azure/registry.tf index 058f7f155..2eaa2d32d 100644 --- a/terraform/environments/azure/registry.tf +++ b/terraform/environments/azure/registry.tf @@ -7,16 +7,14 @@ resource "azurerm_container_registry" "main" { resource_group_name = azurerm_resource_group.main.name location = var.location sku = "Basic" - admin_enabled = true # Enables username/password login for docker push + admin_enabled = false tags = local.common_tags } -# Grant Container Apps managed identity permission to pull images -resource "azurerm_role_assignment" "acr_pull" { - count = var.compute_platform == "container-apps" && length(module.compute_container_apps) > 0 ? 1 : 0 - - scope = azurerm_container_registry.main.id - role_definition_name = "AcrPull" - principal_id = module.compute_container_apps[0].managed_identity_principal_id +# The Container App pulls with its user-assigned identity; the AcrPull grant +# lives inside the container-apps module so the app can depend on it. +moved { + from = azurerm_role_assignment.acr_pull[0] + to = module.compute_container_apps[0].azurerm_role_assignment.acr_pull } diff --git a/terraform/modules/compute/azure/container-apps/main.tf b/terraform/modules/compute/azure/container-apps/main.tf index fc85087d8..13fcb65e3 100644 --- a/terraform/modules/compute/azure/container-apps/main.tf +++ b/terraform/modules/compute/azure/container-apps/main.tf @@ -85,14 +85,10 @@ resource "azurerm_container_app" "main" { identity_ids = [azurerm_user_assigned_identity.container_app.id] } - # Registry authentication - dynamic "registry" { - for_each = var.registry_server != "" ? [1] : [] - content { - server = var.registry_server - username = var.registry_username - password_secret_name = "registry-password" - } + # Registry authentication: pull with the user-assigned identity (AcrPull below) + registry { + server = var.registry_server + identity = azurerm_user_assigned_identity.container_app.id } # Container configuration @@ -220,19 +216,15 @@ resource "azurerm_container_app" "main" { } } - # Registry password secret (for ACR admin auth) - dynamic "secret" { - for_each = var.registry_server != "" ? [1] : [] - content { - name = "registry-password" - value = var.registry_password - } - } - tags = merge(var.tags, { managed_by = "terraform" architecture = "x86_64" }) + + # Ordering only, not a propagation wait: the first revision's image pull + # needs AcrPull to exist. If RBAC has not propagated yet the revision fails + # to provision and the apply errors; re-running the apply recovers. + depends_on = [azurerm_role_assignment.acr_pull] } # ============================================== @@ -254,6 +246,13 @@ locals { subscription_resource_id = "/subscriptions/${var.subscription_id}" } +# AcrPull: lets the container app's identity pull the image from the registry. +resource "azurerm_role_assignment" "acr_pull" { + scope = var.container_registry_id + role_definition_name = "AcrPull" + principal_id = azurerm_user_assigned_identity.container_app.principal_id +} + # Cost Management Reader: grants access to Azure Consumption API (reservation # recommendations, reservation details) needed for CUDly RI/SP features. resource "azurerm_role_assignment" "cost_management_reader" { diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index a28c005e7..f604c27a7 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -192,20 +192,11 @@ variable "custom_domains" { variable "registry_server" { description = "Container registry server URL (e.g. myacr.azurecr.io)" type = string - default = "" -} - -variable "registry_username" { - description = "Container registry username (for admin auth)" - type = string - default = "" } -variable "registry_password" { - description = "Container registry password (for admin auth)" +variable "container_registry_id" { + description = "Container registry ARM resource ID, the scope of the AcrPull grant to the app's managed identity" type = string - default = "" - sensitive = true } variable "tags" {