fix(iac/azure): stop replacing the reservation-purchaser role assignment on every apply

[cristim]

Closes #1802

Root cause

module "compute_container_apps" carries a module-level depends_on
(terraform/environments/azure/compute.tf), and Terraform propagates a
module-level depends_on to every data source inside the module. Both data
sources in that module were therefore deferred, which deploy run 31545175691
shows directly:

# module.compute_container_apps[0].data.azurerm_role_definition.cudly_reservation_purchaser will be read during apply
# module.compute_container_apps[0].data.azurerm_subscription.current will be read during apply

scope and role_definition_id are both ForceNew on azurerm_role_assignment,
so the same run planned all three subscription-scoped assignments for
replacement, and those are exactly the three destroys the issue reports:

# module.compute_container_apps[0].azurerm_role_assignment.cost_management_reader must be replaced
    ~ scope              = "/subscriptions/***" -> (known after apply) # forces replacement
# module.compute_container_apps[0].azurerm_role_assignment.reservations_purchaser must be replaced
    ~ role_definition_id = ".../roleDefinitions/ab9419be-..." -> (known after apply) # forces replacement
    ~ scope              = "/subscriptions/***" -> (known after apply) # forces replacement
# module.compute_container_apps[0].azurerm_role_assignment.subscription_reader must be replaced
    ~ scope              = "/subscriptions/***" -> (known after apply) # forces replacement

Fix

Resolve both inputs in the caller, where nothing defers them.

• The role-definition lookup moves from the module to
  terraform/environments/azure/compute.tf and reaches the module through a new
  reservation_role_definition_id input. The root module has no depends_on,
  so the read happens at plan.
• The RBAC scopes are built from a new subscription_id input rather than from
  data.azurerm_subscription.current.id. The deploy workflow already supplies
  that value as TF_VAR_subscription_id, and it is what configures the
  azurerm provider, so it is statically known at plan.
• The caller lowercases it once. Azure renders subscription GUIDs lowercase in
  resource IDs, so a scope that differed from the canonical form only by case
  would diff on every refresh and replace these ForceNew assignments forever,
  which is the failure this PR exists to remove.

Both new variables have exactly one setter: the module "compute_container_apps"
block in terraform/environments/azure/compute.tf.

data.azurerm_subscription.current stays in the module, now only for the
AZURE_TENANT_ID env var. It is still deferred, which is fine: an env var is
not ForceNew.

What was considered and rejected

• terraform_remote_state against the bootstrap stack. It removes the
  name-based lookup, but it gives the runtime stack a hard dependency on the
  bootstrap stack's state file and its backend credentials. That coupling is a
  worse problem than the one being fixed.
• Deleting the module-level depends_on. It is the actual root cause and it
  is a one-line change, and every one of its four targets is already an implicit
  dependency through referenced outputs. It was still rejected: module-level
  depends_on also orders the container app after resources that no referenced
  output covers (NSG associations, the jwt/session secrets, the database
  firewall rules), and dropping that ordering is a greenfield-apply behaviour
  change well outside what this issue asks for.
• Moving role-definition ownership into the runtime stack. Explicitly out of
  bounds. The bootstrap-vs-runtime split is unchanged here: the 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.

Are both attributes now known at plan time?

Yes.

• scope is "/subscriptions/${var.subscription_id}" inside the module, fed
  from lower(var.subscription_id) at the root. No data source involved.
• role_definition_id is var.reservation_role_definition_id, fed from a root
  data source whose name and scope are both var-derived, which has no
  depends_on, and whose provider config is fully known.

If either were still (known after apply) the fix would not work, so this is
the load-bearing claim in the PR.

Verification

Offline, at the pattern level, with a Terraform sandbox that reproduces the
mechanism: one external data source read from the root module and an identical
one read from inside a module carrying depends_on on a resource that changes
on every plan, each feeding a triggers_replace (the ForceNew stand-in).

First plan:

data.external.root: Read complete after 0s [id=-]
# module.m.data.external.in_module will be read during apply
# (depends on a resource or a module with changes pending)

Second apply, no intervening change:

# terraform_data.assignment_from_module must be replaced
  ~ triggers_replace = { - v = "module" } -> (known after apply) # forces replacement
Plan: 1 to add, 1 to change, 1 to destroy.

terraform_data.assignment_from_root does not appear in the second plan at all.
That is the before and after of this change, reproduced end to end.

Separately verified in the same way that a data.x[0] reference from a
count = 0 module block is never evaluated, so the compute_platform = "aks"
path is unaffected.

Repo checks, all exit 0:

terraform fmt -check -recursive terraform/ iac/                     # exit 0
terraform -chdir=terraform/environments/azure validate              # Success, exit 0
terraform -chdir=terraform/environments/azure/ci-cd-permissions validate  # Success, exit 0
terraform -chdir=iac/federation/azure-target/terraform validate     # Success, exit 0

What is not proven

The real test is two consecutive applies against the live stack with no
intervening change, confirming the second plan reports no changes for
module.compute_container_apps[0].azurerm_role_assignment.reservations_purchaser.
That has not run. This is unverified until two consecutive CI applies show it,
and validate passing is not evidence that it is fixed.

One residual: if the AZURE_SUBSCRIPTION_ID secret's GUID casing differs from
Azure's canonical lowercase, the first plan after this change would still show
these three assignments replaced once, converging afterwards. lower() is what
makes it converge instead of churning forever.

Customer-side module

iac/federation/azure-target/terraform does not share this defect, and is
not touched here. Its azurerm_role_assignment.cudly_reservations takes
scope = "/subscriptions/${local.subscription_id}" and role_definition_id
from module.cudly_reservation_role.role_definition_resource_id, a module in
the same state, so it is known from state after the first apply. No module in
that stack carries a depends_on, so its data.azurerm_subscription.current is
read at plan.

Summary by CodeRabbit

• Bug Fixes
  • Improved Azure deployment planning by ensuring subscription and reservation-purchaser permissions are resolved earlier and consistently.
  • Reduced the risk of Container Apps deployment failures caused by unavailable or delayed role information.
  • Preserved diagnostic coverage for permission lookup issues during both planning and deployment steps.

Input validation on subscription_id (added after review)

subscription_id now builds the reconstructed role-definition lookup name as
well as every subscription-scoped RBAC scope, so a value that is not a GUID at
all (a subscription display name, say) would search for a role that cannot exist
and surface as "role not found" - indistinguishable from a missing bootstrap.
Both subscription_id declarations this PR is responsible for now carry a
case-insensitive GUID validation block:

• terraform/modules/compute/azure/container-apps/variables.tf (the input this
  PR introduces)
• terraform/environments/azure/variables.tf (the root variable, whose job this
  PR materially changed)

The root one is not redundant: only a root variable validation is guaranteed to
run before the role-definition data source in compute.tf. Verified by planning
a reduced copy of the same shape with subscription_id="My Production Subscription" and observing the validation error abort the plan with the
downstream read never attempted. The pattern was exercised in both directions,
4 accepted (including all-uppercase) and 6 rejected.

lower() stays where it was. Validation and lower() do different jobs:
validation rejects a non-GUID, lower() pins an accepted GUID to the casing
Azure returns in resource IDs. Removing lower() would reintroduce the
case-only diff that replaces these ForceNew assignments on every refresh.

This is input validation and fail-fast, not a security control. A
GUID-shaped string is accepted whether or not that subscription exists, is
reachable, or is the right one. A shape check admits every value of that shape
and does not restrict which subscription can be targeted. The code comments say
the same.

The bootstrap stack's own subscription_id
(terraform/environments/azure/ci-cd-permissions/variables.tf) was deliberately
left alone: this PR neither introduces nor changes it, and widening into an
untouched variable in a separate human-applied stack is out of scope for #1802.
Declined in writing on the review thread; a reasonable standalone follow-up.

[coderabbitai]

📝 Walkthrough

Walkthrough

The Azure environment now resolves the custom reservation-purchaser role at plan time and passes its ID and the normalized subscription ID to the Container Apps module. The module uses these values for subscription-scoped RBAC assignments and environment configuration.

Changes

Azure RBAC plan-time resolution

Layer / File(s)	Summary
Root role resolution and module inputs .github/workflows/deploy-azure.yml, terraform/environments/azure/compute.tf	The environment normalizes the subscription ID, looks up the custom reservation-purchaser role definition, and passes both identifiers to the Container Apps module. The deployment diagnostic describes plan-time lookup.
Module RBAC scope and assignment wiring terraform/modules/compute/azure/container-apps/main.tf, terraform/modules/compute/azure/container-apps/variables.tf	The module adds required inputs for the subscription ID and role definition ID. Subscription-scoped assignments and the reservation-purchaser assignment use these caller-provided values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AzureEnvironment
  participant AzureRoleDefinition
  participant ContainerAppsModule
  participant AzureRBAC
  AzureEnvironment->>AzureRoleDefinition: Look up custom role by name and subscription scope
  AzureRoleDefinition-->>AzureEnvironment: Return role definition ID
  AzureEnvironment->>ContainerAppsModule: Pass subscription ID and role definition ID
  ContainerAppsModule->>AzureRBAC: Create subscription-scoped role assignments

Loading

Possibly related issues

• ci(azure): Deploy to Azure Container Apps has failed on every main run for 3 weeks — no Azure change since 2026-07-19 is deployed #1794 — Both changes address deferred custom reservation-purchaser role lookup in the Container Apps deployment.
• LeanerCloud/CUDly#1799 — Both changes involve the custom reservation-purchaser Azure RBAC role, but this PR changes Terraform lookup and assignment wiring.

Possibly related PRs

• LeanerCloud/CUDly#1800 — Both PRs modify the Azure reservation-purchaser role integration; this PR changes plan-time role ID and subscription wiring.
• LeanerCloud/CUDly#1795 — Both PRs modify Azure deployment diagnostics and Container Apps role lookup.
• LeanerCloud/CUDly#732 — This PR consumes the custom reservation-purchaser role definition exposed by that PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes make role_definition_id and scope plan-time known while preserving bootstrap and runtime ownership as required by issue #1802.
Out of Scope Changes check	✅ Passed	All changes support issue #1802 by fixing Azure RBAC planning behavior and documenting the related diagnostic timing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the Azure IaC fix that prevents repeated replacement of the reservation-purchaser role assignment.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1802-reservation-role-assignment-churn

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@terraform/environments/azure/compute.tf`:
- Line 12: Add validation blocks to the subscription ID variable declarations,
including the bootstrap variable, requiring an Azure subscription GUID with four
hexadecimal groups followed by eight hexadecimal characters in the standard
hyphenated format, case-insensitively. Keep lower(var.subscription_id) for
canonical ARM identifiers and reuse the same validation pattern for both
variables.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8b1cca03-2730-47d6-bad1-66c84613b2ef

📥 Commits

Reviewing files that changed from the base of the PR and between 51e89a6 and a84ddfa.

📒 Files selected for processing (4)

• .github/workflows/deploy-azure.yml
• terraform/environments/azure/compute.tf
• terraform/modules/compute/azure/container-apps/main.tf
• terraform/modules/compute/azure/container-apps/variables.tf