From 7e95e46b214fd5284891961cc32e831d9cf10454 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 7 Jun 2026 13:47:11 -0700 Subject: [PATCH] fix(iac/azure): move reservation role definition to bootstrap; runtime keeps only the assignment (closes #1092) The runtime container-apps module instantiated the cudly-reservation-role module, which creates an azurerm_role_definition. Creating a role definition needs Microsoft.Authorization/roleDefinitions/write, a bootstrap-class permission the CI deploy service principal intentionally lacks (it has only roleDefinitions/read plus roleAssignments/{read,write,delete}). Every Azure deploy therefore 403'd at the role-definition write, exactly as the bootstrap-vs-runtime IAM split intends. Fix, honoring the split (no roleDefinitions/write granted to the deploy SP): - Bootstrap: create the host-side reservation role definition in the human-applied ci-cd-permissions module (new reservation_role.tf) and output its resource ID and display name. - Runtime: stop creating the definition in container-apps; resolve it via data.azurerm_role_definition by name + subscription scope and keep only the azurerm_role_assignment. A missing definition fails loud ("not found"), signalling the operator to re-run the bootstrap rather than widening the SP. - Shared module: factor the role display name into a local (single source of truth) and expose it via a role_definition_name output so the runtime lookup stays in lockstep. - Drop the tenant-root "/providers/Microsoft.Capacity" assignable scope (a subscription-scoped principal cannot register a scope above its subscription, a latent 403). It is now gated behind include_capacity_provider_scope (default false); the role is only ever assigned at subscription scope. REQUIRED MANUAL STEP: a privileged human must re-apply the ci-cd-permissions bootstrap once to create the role definition before the next runtime deploy. Verified offline: terraform fmt -check, terraform validate, and tflint are clean on the shared module, the bootstrap env, and the container-apps module (apply not run; staging-first). --- .../azure/ci-cd-permissions/README.md | 16 ++++++++ .../azure/ci-cd-permissions/outputs.tf | 10 +++++ .../ci-cd-permissions/reservation_role.tf | 23 ++++++++++++ .../compute/azure/container-apps/main.tf | 37 ++++++++++++++----- .../iam/azure/cudly-reservation-role/main.tf | 25 +++++++++++-- .../azure/cudly-reservation-role/outputs.tf | 5 +++ .../azure/cudly-reservation-role/variables.tf | 6 +++ 7 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 terraform/environments/azure/ci-cd-permissions/reservation_role.tf diff --git a/terraform/environments/azure/ci-cd-permissions/README.md b/terraform/environments/azure/ci-cd-permissions/README.md index 7eadad1f9..1762cd50b 100644 --- a/terraform/environments/azure/ci-cd-permissions/README.md +++ b/terraform/environments/azure/ci-cd-permissions/README.md @@ -15,6 +15,22 @@ secrets ever need to be stored. | `azuread_application_federated_identity_credential.github_pr` | Federated credential for pull-request plan checks | | `azurerm_role_definition.cudly_deploy` | Custom role with minimum required permissions | | `azurerm_role_assignment.cudly_deploy` | Assigns the custom role to the SP at subscription scope | +| `module.cudly_reservation_role` (`azurerm_role_definition`) | Host-side custom "CUDly Reservation Purchaser" role definition consumed by the runtime container-apps deploy | + +> **Bootstrap-only role definition.** The reservation-purchaser role *definition* +> lives here, not in the runtime container-apps module, because creating an +> `azurerm_role_definition` requires +> `Microsoft.Authorization/roleDefinitions/write`, a bootstrap-class permission +> the runtime CI deploy SP intentionally does not have (it only holds +> `roleDefinitions/read` + `roleAssignments/{read,write,delete}`). The runtime +> deploy looks the role up by name via `data.azurerm_role_definition` and creates +> only the assignment. +> +> **Re-apply this module once after pulling this change** (and any time the +> reservation role's name or actions change) so the role definition exists before +> the next runtime Azure deploy. If it is missing, the runtime +> `data.azurerm_role_definition` lookup fails loudly with a "not found" error: +> the signal to re-run this bootstrap, not to widen the deploy SP's permissions. ### Permissions granted diff --git a/terraform/environments/azure/ci-cd-permissions/outputs.tf b/terraform/environments/azure/ci-cd-permissions/outputs.tf index a1a8c593d..a6cc10eee 100644 --- a/terraform/environments/azure/ci-cd-permissions/outputs.tf +++ b/terraform/environments/azure/ci-cd-permissions/outputs.tf @@ -22,3 +22,13 @@ output "tenant_id" { description = "Azure AD tenant ID — use as AZURE_TENANT_ID in GitHub Actions" value = azuread_service_principal.cudly_deploy.application_tenant_id } + +output "reservation_role_definition_id" { + description = "Full ARM resource ID of the host-side custom reservation-purchaser role definition created by this bootstrap. The runtime container-apps deploy looks the role up by name and assigns it." + value = module.cudly_reservation_role.role_definition_resource_id +} + +output "reservation_role_definition_name" { + description = "Display name of the host-side custom reservation-purchaser role definition. The runtime container-apps module resolves the role by this exact name via data.azurerm_role_definition." + value = module.cudly_reservation_role.role_definition_name +} diff --git a/terraform/environments/azure/ci-cd-permissions/reservation_role.tf b/terraform/environments/azure/ci-cd-permissions/reservation_role.tf new file mode 100644 index 000000000..48728f1ca --- /dev/null +++ b/terraform/environments/azure/ci-cd-permissions/reservation_role.tf @@ -0,0 +1,23 @@ +# Host-side custom reservation-purchaser role definition (BOOTSTRAP). +# +# Creating an azurerm_role_definition requires +# Microsoft.Authorization/roleDefinitions/write, a bootstrap-class permission +# the runtime CI deploy service principal intentionally does NOT have (it only +# has roleDefinitions/read; see locals_data.tf). This module is applied once by +# a privileged human with role-admin authority, so it is the correct home for +# the role *definition*. +# +# The runtime container-apps module +# (terraform/modules/compute/azure/container-apps) no longer creates this role. +# It looks it up via data.azurerm_role_definition and creates only the +# *assignment* (which the deploy SP can do via roleAssignments/write). If this +# bootstrap has not been (re-)applied, that runtime data lookup fails loudly, +# signalling the operator to re-run this module. +# +# name_suffix matches the runtime side (the subscription ID) so both reference +# the identical role display name. +module "cudly_reservation_role" { + source = "../../../modules/iam/azure/cudly-reservation-role" + scope = "/subscriptions/${var.subscription_id}" + name_suffix = var.subscription_id +} diff --git a/terraform/modules/compute/azure/container-apps/main.tf b/terraform/modules/compute/azure/container-apps/main.tf index 57237fcbf..e97572c6e 100644 --- a/terraform/modules/compute/azure/container-apps/main.tf +++ b/terraform/modules/compute/azure/container-apps/main.tf @@ -262,20 +262,39 @@ resource "azurerm_role_assignment" "subscription_reader" { # Custom reservation-purchaser role: same role as customer-side but scoped to # the host subscription. The built-in Reservation Purchaser lacks # reservationOrders/purchase/action (same gap fixed customer-side by PR #744). -# The role definition is factored into a shared module so both sides stay in -# lockstep with the ARM template. -module "cudly_reservation_role" { - source = "../../../iam/azure/cudly-reservation-role" - scope = data.azurerm_subscription.current.id - name_suffix = data.azurerm_subscription.current.subscription_id +# +# BOOTSTRAP-vs-RUNTIME SPLIT: the role *definition* is created by the +# human-applied bootstrap module +# (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 +# 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. +data "azurerm_role_definition" "cudly_reservation_purchaser" { + name = "CUDly Reservation Purchaser (custom) - ${data.azurerm_subscription.current.subscription_id}" + scope = data.azurerm_subscription.current.id } resource "azurerm_role_assignment" "reservations_purchaser" { - scope = data.azurerm_subscription.current.id - role_definition_id = module.cudly_reservation_role.role_definition_resource_id + scope = data.azurerm_subscription.current.id + # .id is the 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 principal_id = azurerm_user_assigned_identity.container_app.principal_id - depends_on = [module.cudly_reservation_role] + # 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/iam/azure/cudly-reservation-role/main.tf b/terraform/modules/iam/azure/cudly-reservation-role/main.tf index 0e66ce6f3..ab154e6d2 100644 --- a/terraform/modules/iam/azure/cudly-reservation-role/main.tf +++ b/terraform/modules/iam/azure/cudly-reservation-role/main.tf @@ -21,8 +21,15 @@ terraform { # container-app user-assigned identity running the CUDly process itself # # Keep the actions list here in sync with arm/CUDly-CrossSubscription/template.json. +locals { + # Single source of truth for the role display name. Consumers that look the + # role up via data.azurerm_role_definition (rather than creating it) must + # reconstruct this exact string; see role_definition_name output below. + role_definition_name = "CUDly Reservation Purchaser (custom) - ${var.name_suffix}" +} + resource "azurerm_role_definition" "cudly_reservation_purchaser" { - name = "CUDly Reservation Purchaser (custom) - ${var.name_suffix}" + name = local.role_definition_name scope = var.scope description = "Custom role granting CUDly exactly the Microsoft.Capacity and Microsoft.BillingBenefits actions required by the calculatePrice -> purchase flow. Replaces the built-in Reservation Purchaser, which lacks reservationOrders/purchase/action." @@ -44,8 +51,18 @@ resource "azurerm_role_definition" "cudly_reservation_purchaser" { not_actions = [] } - assignable_scopes = [ + # assignable_scopes is intentionally limited to the subscription scope. + # + # The tenant-root provider scope "/providers/Microsoft.Capacity" was removed: + # a subscription-scoped principal cannot register an assignable scope above its + # own subscription, so including it makes the role-definition write 403 at the + # higher scope. The role is only ever assigned at subscription scope (see the + # azurerm_role_assignment blocks in the consuming modules), so the subscription + # scope is sufficient. Set include_capacity_provider_scope = true only when the + # applying principal has tenant-root authority and a tenant-level assignment is + # actually required. + assignable_scopes = compact([ var.scope, - "/providers/Microsoft.Capacity", - ] + var.include_capacity_provider_scope ? "/providers/Microsoft.Capacity" : "", + ]) } diff --git a/terraform/modules/iam/azure/cudly-reservation-role/outputs.tf b/terraform/modules/iam/azure/cudly-reservation-role/outputs.tf index 538837c0f..24fe6beb5 100644 --- a/terraform/modules/iam/azure/cudly-reservation-role/outputs.tf +++ b/terraform/modules/iam/azure/cudly-reservation-role/outputs.tf @@ -2,3 +2,8 @@ output "role_definition_resource_id" { description = "Full ARM resource ID of the custom role definition. Use as role_definition_id in azurerm_role_assignment blocks." value = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id } + +output "role_definition_name" { + description = "Display name of the custom role definition. Runtime modules that look the role up via data.azurerm_role_definition must match this exact name." + value = local.role_definition_name +} diff --git a/terraform/modules/iam/azure/cudly-reservation-role/variables.tf b/terraform/modules/iam/azure/cudly-reservation-role/variables.tf index 23976b6d5..78bd9e885 100644 --- a/terraform/modules/iam/azure/cudly-reservation-role/variables.tf +++ b/terraform/modules/iam/azure/cudly-reservation-role/variables.tf @@ -7,3 +7,9 @@ variable "name_suffix" { description = "Suffix appended to the role display name to keep customer and host role definitions distinct within the same Azure AD tenant (e.g. the subscription ID)." type = string } + +variable "include_capacity_provider_scope" { + description = "When true, adds the tenant-root \"/providers/Microsoft.Capacity\" scope to assignable_scopes. Leave false (the default) for subscription-scoped principals: registering an assignable scope above the subscription requires tenant-root authority and otherwise 403s." + type = bool + default = false +}