diff --git a/.github/workflows/deploy-azure.yml b/.github/workflows/deploy-azure.yml index 387e4b7c3..8a7c18df5 100644 --- a/.github/workflows/deploy-azure.yml +++ b/.github/workflows/deploy-azure.yml @@ -246,12 +246,12 @@ jobs: - name: Explain a missing bootstrap role if: failure() run: | - # Both logs are checked. Today the role lookup is deferred to apply -- - # the plan reports "will be read during apply (config refers to values - # not yet known)" -- because of the module-level depends_on at - # terraform/environments/azure/compute.tf:75. That is contingent, not - # structural: if that stops forcing deferral the read moves to plan, and - # the diagnostic should not have to move with it. + # Both logs are checked. Today the role lookup runs at plan: it lives in + # the root module (terraform/environments/azure/compute.tf), outside the + # module-level depends_on that used to defer it to apply and churn the + # assignment on every deploy (#1802). Keeping the apply log in the loop + # costs nothing and means the diagnostic does not have to move if the + # read ever shifts back. # # Each file is checked separately so that "this log does not exist" is an # explicit, expected case at the point of reading -- either log is absent diff --git a/terraform/environments/azure/compute.tf b/terraform/environments/azure/compute.tf index 25d20b17d..7f2ecd011 100644 --- a/terraform/environments/azure/compute.tf +++ b/terraform/environments/azure/compute.tf @@ -2,6 +2,48 @@ # Compute Module: Container Apps (Serverless) # ============================================== +locals { + # Azure renders subscription GUIDs lowercase in resource IDs, and the role + # display name the bootstrap created carries that same lowercase GUID. + # Normalising once here keeps the lookup name and every RBAC scope matching + # whatever casing the TF_VAR_subscription_id secret happens to use; a scope + # differing from the API's canonical form only by case would diff on every + # refresh and replace the (ForceNew) role assignments forever. + host_subscription_id = lower(var.subscription_id) +} + +# Bootstrap-created custom reservation-purchaser role definition, looked up by +# name. The definition is owned by the human-applied bootstrap stack +# (terraform/environments/azure/ci-cd-permissions/reservation_role.tf) because +# creating it needs Microsoft.Authorization/roleDefinitions/write, which the +# deploy service principal deliberately lacks; see the BOOTSTRAP-vs-RUNTIME +# SPLIT comment in ../../modules/compute/azure/container-apps/main.tf. +# +# The lookup lives here, not inside that module, because the module carries a +# depends_on (below) and Terraform propagates a module-level depends_on to every +# data source in the module, deferring the read to apply. That made the +# resulting role_definition_id "(known after apply)" on every plan and forced a +# destroy/recreate of the reservation-purchaser assignment on every deploy +# (#1802). Nothing defers this read, so the ID is known at plan time. +# +# A lifecycle postcondition deliberately is NOT used here. The azurerm provider +# fails the data read itself when the role is absent ("loading Role Definition +# List: could not find role ..."), so Terraform aborts before any postcondition +# evaluates -- the check would never fire for the failure it was meant to explain. +# The remediation is surfaced from the workflow's failure path instead; see the +# "Explain a missing bootstrap role" step in .github/workflows/deploy-azure.yml. +data "azurerm_role_definition" "cudly_reservation_purchaser" { + count = var.compute_platform == "container-apps" ? 1 : 0 + + # Reconstruction of local.role_definition_name from + # ../../modules/iam/azure/cudly-reservation-role. That module's + # role_definition_name output cannot be referenced here: it is instantiated in + # the ci-cd-permissions stack, which has separate state, and this repo does not + # use terraform_remote_state. Changing the format string requires changing both. + name = "CUDly Reservation Purchaser (custom) - ${local.host_subscription_id}" + scope = "/subscriptions/${local.host_subscription_id}" +} + module "compute_container_apps" { source = "../../modules/compute/azure/container-apps" count = var.compute_platform == "container-apps" ? 1 : 0 @@ -11,6 +53,11 @@ module "compute_container_apps" { resource_group_name = azurerm_resource_group.main.name location = var.location + # RBAC inputs. Both are resolved here rather than inside the module so they + # stay known at plan time despite the module-level depends_on below (#1802). + subscription_id = local.host_subscription_id + reservation_role_definition_id = data.azurerm_role_definition.cudly_reservation_purchaser[0].id + # Container image (from build module or var.image_uri) image_uri = var.enable_docker_build ? module.build[0].image_uri : var.image_uri cpu = var.container_cpu diff --git a/terraform/environments/azure/variables.tf b/terraform/environments/azure/variables.tf index dc26f8570..f8574bd81 100644 --- a/terraform/environments/azure/variables.tf +++ b/terraform/environments/azure/variables.tf @@ -3,8 +3,21 @@ # ============================================== variable "subscription_id" { - description = "Azure subscription ID" + description = "Azure subscription ID (GUID), supplied as TF_VAR_subscription_id by .github/workflows/deploy-azure.yml. Besides configuring the azurerm provider it now builds the reservation-purchaser role-definition lookup name and every subscription-scoped RBAC scope; see compute.tf." type = string + + # Validated here as well as on the container-apps module input, because only a + # root variable validation is guaranteed to run before the role-definition + # data source in compute.tf. A non-GUID value (a subscription display name, + # say) would otherwise reach that lookup and fail as "role not found", which + # reads as a missing bootstrap rather than as bad input. + # + # Input validation, not a security control: a GUID-shaped string is accepted + # whether or not that subscription exists, is reachable, or is the right one. + validation { + condition = can(regex("^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$", var.subscription_id)) + error_message = "The subscription_id must be an Azure subscription GUID in 8-4-4-4-12 hexadecimal form, upper or lower case." + } } variable "project_name" { diff --git a/terraform/modules/compute/azure/container-apps/main.tf b/terraform/modules/compute/azure/container-apps/main.tf index 04cfcc69b..fc85087d8 100644 --- a/terraform/modules/compute/azure/container-apps/main.tf +++ b/terraform/modules/compute/azure/container-apps/main.tf @@ -127,7 +127,7 @@ resource "azurerm_container_app" "main" { ADMIN_PASSWORD_SECRET = var.admin_password_secret_name SECRET_PROVIDER = "azure" AZURE_CLIENT_ID = azurerm_user_assigned_identity.container_app.client_id - AZURE_SUBSCRIPTION_ID = data.azurerm_subscription.current.subscription_id + AZURE_SUBSCRIPTION_ID = var.subscription_id AZURE_TENANT_ID = data.azurerm_subscription.current.tenant_id AZURE_KEY_VAULT_URL = var.key_vault_uri AZURE_REGION = var.location @@ -239,12 +239,25 @@ resource "azurerm_container_app" "main" { # Runtime RBAC: CUDly application cloud API access # ============================================== +# Read only for tenant_id (an env var, not an RBAC scope). This module carries a +# module-level depends_on at terraform/environments/azure/compute.tf, which +# Terraform propagates to every data source inside it, so this read is deferred +# to apply and its attributes are "(known after apply)" on every plan. That is +# tolerable for an env var; it is not tolerable for anything ForceNew, which is +# why the RBAC scopes below use var.subscription_id instead (#1802). data "azurerm_subscription" "current" {} +locals { + # Subscription RBAC scope, built from the caller-supplied ID so it is known at + # plan time. scope is ForceNew on azurerm_role_assignment, so it must match the + # casing Azure returns in resource IDs; the caller normalises for that. + subscription_resource_id = "/subscriptions/${var.subscription_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" { - scope = data.azurerm_subscription.current.id + scope = local.subscription_resource_id role_definition_name = "Cost Management Reader" principal_id = azurerm_user_assigned_identity.container_app.principal_id } @@ -254,7 +267,7 @@ resource "azurerm_role_assignment" "cost_management_reader" { # account is ingested as a "Self" account. Without this, every per-service SDK # list call against the host subscription returns 403. resource "azurerm_role_assignment" "subscription_reader" { - scope = data.azurerm_subscription.current.id + scope = local.subscription_resource_id role_definition_name = "Reader" principal_id = azurerm_user_assigned_identity.container_app.principal_id } @@ -269,49 +282,28 @@ resource "azurerm_role_assignment" "subscription_reader" { # (terraform/environments/azure/ci-cd-permissions/reservation_role.tf), NOT # here. Creating an azurerm_role_definition needs # Microsoft.Authorization/roleDefinitions/write, which the runtime CI deploy -# service principal intentionally lacks. This runtime module only looks the +# service principal intentionally lacks. The runtime side only looks the # definition up and creates the *assignment* (roleAssignments/write, which the # deploy SP does have). # -# The lookup name MUST stay in lockstep with -# terraform/modules/iam/azure/cudly-reservation-role (its role_definition_name -# output / local.role_definition_name). If the bootstrap has not been -# (re-)applied, this data source fails loudly with "Role Definition ... was not -# found" -- the signal to re-run the ci-cd-permissions bootstrap, NOT to grant -# roleDefinitions/write to the deploy SP. -locals { - # Reconstruction of local.role_definition_name from - # terraform/modules/iam/azure/cudly-reservation-role. That module's - # role_definition_name output cannot be referenced here: it is instantiated in - # the ci-cd-permissions stack, which has separate state, and this repo does not - # use terraform_remote_state. Keeping the format string in one place per module - # is the most the split allows; changing it requires changing both. - cudly_reservation_purchaser_role_name = "CUDly Reservation Purchaser (custom) - ${data.azurerm_subscription.current.subscription_id}" -} - -# A lifecycle postcondition deliberately is NOT used here. The azurerm provider -# fails the data read itself when the role is absent ("loading Role Definition -# List: could not find role ..."), so Terraform aborts before any postcondition -# evaluates -- the check would never fire for the failure it was meant to explain. -# The remediation is surfaced from the workflow's failure path instead; see the -# "Explain a missing bootstrap role" step in .github/workflows/deploy-azure.yml. -data "azurerm_role_definition" "cudly_reservation_purchaser" { - name = local.cudly_reservation_purchaser_role_name - scope = data.azurerm_subscription.current.id -} - +# The lookup itself lives in the caller (terraform/environments/azure/compute.tf) +# rather than in this module: a module-level depends_on defers every data source +# inside the module to apply time, which made role_definition_id and scope +# "(known after apply)" on every plan and destroyed/recreated this money-path +# grant on every deploy (#1802). The caller has no such depends_on, so its read +# happens at plan and this assignment is stable across applies. +# +# If the bootstrap has not been (re-)applied, the caller's data read fails loudly +# with "Role Definition ... was not found" -- the signal to re-run the +# ci-cd-permissions bootstrap, NOT to grant roleDefinitions/write to the deploy SP. resource "azurerm_role_assignment" "reservations_purchaser" { - scope = data.azurerm_subscription.current.id - # .id is the full scoped ARM resource ID of the role definition + scope = local.subscription_resource_id + # Full scoped ARM resource ID of the role definition # (/subscriptions//providers/Microsoft.Authorization/roleDefinitions/), # the same form the bootstrap exposes via role_definition_resource_id and what # azurerm_role_assignment.role_definition_id expects. - role_definition_id = data.azurerm_role_definition.cudly_reservation_purchaser.id + role_definition_id = var.reservation_role_definition_id principal_id = azurerm_user_assigned_identity.container_app.principal_id - - # RBAC propagation: the bootstrap-created role definition can take time to be - # readable at assignment time; the data dependency below also gates this. - depends_on = [data.azurerm_role_definition.cudly_reservation_purchaser] } # Key Vault Crypto User: allows the container app's managed identity diff --git a/terraform/modules/compute/azure/container-apps/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index 354f669bd..a28c005e7 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -107,6 +107,26 @@ variable "database_password_secret_name" { type = string } +variable "subscription_id" { + description = "Host subscription GUID. Supplied by the caller rather than read from data.azurerm_subscription so every RBAC scope in this module stays known at plan time; see the RBAC section of main.tf." + type = string + + # Input validation, not a security control: a GUID-shaped string is accepted + # whether or not that subscription exists, is reachable, or is the right one. + # What it buys is a clear plan-time message for a value that is not a GUID at + # all (a subscription display name, say), which would otherwise surface as an + # opaque Azure rejection of a malformed RBAC scope far from the cause. + validation { + condition = can(regex("^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$", var.subscription_id)) + error_message = "The subscription_id must be an Azure subscription GUID in 8-4-4-4-12 hexadecimal form, upper or lower case." + } +} + +variable "reservation_role_definition_id" { + description = "Full ARM resource ID of the bootstrap-created custom reservation-purchaser role definition (terraform/modules/iam/azure/cudly-reservation-role, output role_definition_resource_id). Looked up by the caller so it is known at plan time." + type = string +} + variable "key_vault_uri" { description = "Key Vault URI (data-plane, e.g. https://.vault.azure.net/)" type = string