From a84ddfac6bea07616ae648c09e00c49c1808dc4e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 12 Aug 2026 02:27:39 +0200 Subject: [PATCH 1/2] fix(iac/azure): stop replacing the reservation-purchaser role assignment on every apply module.compute_container_apps carries a module-level depends_on, and Terraform propagates a module-level depends_on to every data source inside the module. Both data.azurerm_subscription.current and data.azurerm_role_definition.cudly_reservation_purchaser were therefore read during apply, leaving scope and role_definition_id "(known after apply)" on every plan. Both attributes are ForceNew on azurerm_role_assignment, so all three subscription-scoped assignments were destroyed and recreated on every deploy: cost_management_reader, subscription_reader, and the money-path reservations_purchaser, whose grant is what permits calculatePrice and reservationOrders/write. Resolve both inputs in the caller, where nothing defers them. The role-definition lookup moves to terraform/environments/azure/compute.tf and reaches the module as reservation_role_definition_id; the RBAC scopes are built from var.subscription_id, which the deploy workflow already supplies via TF_VAR_subscription_id and which also configures the azurerm provider. The caller lowercases it once so the scope matches the casing Azure returns in resource IDs. The bootstrap-vs-runtime split is unchanged: the role definition is still owned by the human-applied ci-cd-permissions stack, and the runtime side still only looks it up and creates the assignment. The lookup now fails at plan rather than at apply when the bootstrap has not been applied; the workflow's "Explain a missing bootstrap role" step already reads both logs. Refs #1802 --- .github/workflows/deploy-azure.yml | 12 ++-- terraform/environments/azure/compute.tf | 47 +++++++++++++ .../compute/azure/container-apps/main.tf | 68 ++++++++----------- .../compute/azure/container-apps/variables.tf | 10 +++ 4 files changed, 93 insertions(+), 44 deletions(-) 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/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..60b94b95a 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -107,6 +107,16 @@ 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 +} + +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 From b8c511b5db6bc0f8407646e0be058558b283fc7d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 12 Aug 2026 02:54:33 +0200 Subject: [PATCH 2/2] fix(iac/azure): fail at plan time when subscription_id is not a GUID subscription_id now builds the reconstructed role-definition lookup name ("CUDly Reservation Purchaser (custom) - ") as well as every subscription-scoped RBAC scope. A value that is not a GUID at all, such as a subscription display name, makes that lookup search for a role that cannot exist and surfaces as "role not found", which reads as a missing bootstrap rather than as bad input. Validate the GUID shape case-insensitively on both the container-apps module input and the root variable, then keep lower() for the canonical ARM identifier: the two do different jobs, and dropping lower() would reintroduce the case-only diff that replaces these ForceNew assignments on every refresh. The root declaration is validated as well as the module input because only a root variable validation is guaranteed to run before the role-definition data source in compute.tf; verified by observing the validation error abort the plan with the downstream read never attempted. This is 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, and the comments say so. Refs #1802 --- terraform/environments/azure/variables.tf | 15 ++++++++++++++- .../compute/azure/container-apps/variables.tf | 10 ++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) 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/variables.tf b/terraform/modules/compute/azure/container-apps/variables.tf index 60b94b95a..a28c005e7 100644 --- a/terraform/modules/compute/azure/container-apps/variables.tf +++ b/terraform/modules/compute/azure/container-apps/variables.tf @@ -110,6 +110,16 @@ variable "database_password_secret_name" { 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" {