From bd07659b4e21df7c49145e2c8c98619a8c8c4153 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 10 Aug 2026 22:45:50 +0200 Subject: [PATCH 1/3] fix(ci/azure): name the missing bootstrap stack when the role lookup fails The Azure deploy has failed on every main run since 2026-07-19 (0 successes in 100 runs) because a custom role definition is absent from the subscription. The provider error, "loading Role Definition List: could not find role ...", names neither the prerequisite nor the stack that creates it, so three weeks of identical failures produced no action. The role is created by terraform/environments/azure/ci-cd-permissions and looked up by the deploy stack via data.azurerm_role_definition. That split is deliberate: creating a role definition needs roleDefinitions/write, which the deploy service principal intentionally lacks. The remediation was documented, but only in a source comment nobody reading a failed CI log would see. A Terraform precondition cannot help here. The provider fails the data read itself when the role is absent, so Terraform aborts before any postcondition evaluates; such a check would never fire for the failure it exists to explain. The remediation is emitted from the workflow's failure path instead, gated on the provider's error text. Terraform Apply now tees to a log so that step can inspect it. set -o pipefail is required: the default shell is bash -e, where a pipeline takes tee's exit status, and a failed apply would otherwise be reported as success. Verified both directions. Also hoists the reconstructed role name into a named local. The bootstrap module's role_definition_name output cannot be referenced across stack boundaries without terraform_remote_state, which this repo does not use, so the format string stays duplicated by necessity; the local at least makes it one place per module and records why. This does not fix the deploy. The bootstrap stack still has to be applied. It makes the next occurrence self-explaining rather than silent. Refs #1794 --- .github/workflows/deploy-azure.yml | 36 ++++++++++++++++++- .../compute/azure/container-apps/main.tf | 18 +++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy-azure.yml b/.github/workflows/deploy-azure.yml index 04b1c071f..8a9860a72 100644 --- a/.github/workflows/deploy-azure.yml +++ b/.github/workflows/deploy-azure.yml @@ -217,8 +217,11 @@ jobs: TF_VAR_admin_email: ${{ secrets.ADMIN_EMAIL }} TF_VAR_subscription_id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} run: | + # pipefail is required: the default shell is `bash -e`, where a pipeline + # takes tee's exit status and a failed apply would be reported as success. + set -o pipefail cd terraform/environments/azure - terraform apply -auto-approve tfplan + terraform apply -auto-approve tfplan 2>&1 | tee "${RUNNER_TEMP}/tf-apply.log" # Get app URL APP_URL=$(terraform output -raw container_app_url 2>/dev/null | grep -v '::' || echo "") @@ -228,6 +231,37 @@ jobs: APP_NAME=$(terraform output -raw container_app_name 2>/dev/null | grep -v '::' || echo "") echo "app_name=$APP_NAME" >> $GITHUB_OUTPUT + # The provider fails the data read outright when the custom role is absent, + # so a Terraform precondition can never run for this case. Issue #1794: the + # Azure deploy failed on every main run for three weeks because the raw + # provider error names neither the missing prerequisite nor the stack that + # creates it, and the explanation lived only in a source comment. + - name: Explain a missing bootstrap role + if: failure() + run: | + if ! grep -qiE 'could not find role|Role Definition .* was not found' "${RUNNER_TEMP}/tf-apply.log" 2>/dev/null; then + exit 0 + fi + cat >&2 <<'EOT' + ::error title=Missing bootstrap role::The custom reservation-purchaser role does not exist in this subscription. + + Apply the bootstrap stack that creates it: + terraform/environments/azure/ci-cd-permissions + + Do NOT grant Microsoft.Authorization/roleDefinitions/write to the deploy + service principal to work around this. The split is deliberate: this + pipeline holds roleAssignments/write only. + + If the bootstrap HAS been applied, check in this order: + 1. the deploy SP has Microsoft.Authorization/roleDefinitions/read. + Without it the lookup returns empty, which is indistinguishable + from the role being absent. + 2. the role was not renamed or deleted out of band. + 3. the name suffix still matches. The runtime module looks up + "CUDly Reservation Purchaser (custom) - "; + the bootstrap builds the name from its own var.name_suffix. + EOT + - name: Release state lock on failure if: failure() || cancelled() env: diff --git a/terraform/modules/compute/azure/container-apps/main.tf b/terraform/modules/compute/azure/container-apps/main.tf index e97572c6e..433d49349 100644 --- a/terraform/modules/compute/azure/container-apps/main.tf +++ b/terraform/modules/compute/azure/container-apps/main.tf @@ -278,8 +278,24 @@ resource "azurerm_role_assignment" "subscription_reader" { # (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 = "CUDly Reservation Purchaser (custom) - ${data.azurerm_subscription.current.subscription_id}" + name = local.cudly_reservation_purchaser_role_name scope = data.azurerm_subscription.current.id } From e11d47b6b43b7dc018c8978a6583300dc37c331b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 11 Aug 2026 00:10:27 +0200 Subject: [PATCH 2/3] fix(ci/azure): cover a plan-time role failure and grep each log separately Review finding F2: the data source is read at plan time as well as apply, so a missing role can surface in either step. The diagnostic only inspected the apply log, so a plan-time failure would have produced the same silence this change exists to end. Terraform Plan now tees to its own log. It needs its own `set -o pipefail` for the same reason the apply step does: under `bash -e` a pipeline takes tee's exit status, so a failed plan would be reported as success. The two logs are grepped separately rather than as two operands to one grep. With multiple operands where one does not exist, grep's exit status is implementation-defined: GNU returns 0 when -q matched an earlier file, BSD returns 2. Either log may legitimately be absent when an earlier step failed first, so the combined form goes silent on some hosts. Caught by testing the one-file-missing case, which is the case that actually occurs. Verified in all four states: plan-only match with apply absent fires, apply-only match with plan absent fires, both present with neither matching stays silent, both absent stays silent. Refs #1794 --- .github/workflows/deploy-azure.yml | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/.github/workflows/deploy-azure.yml b/.github/workflows/deploy-azure.yml index 8a9860a72..a59520ba1 100644 --- a/.github/workflows/deploy-azure.yml +++ b/.github/workflows/deploy-azure.yml @@ -164,11 +164,15 @@ jobs: TF_VAR_admin_email: ${{ secrets.ADMIN_EMAIL }} TF_VAR_subscription_id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} run: | + # pipefail for the same reason as the apply step below: the default + # shell is `bash -e`, where a pipeline takes tee's exit status and a + # failed plan would be reported as success. + set -o pipefail cd terraform/environments/azure terraform plan \ -var-file="github-${{ needs.prepare.outputs.environment }}.tfvars" \ -var="location=${{ env.AZURE_LOCATION }}" \ - -out=tfplan + -out=tfplan 2>&1 | tee "${RUNNER_TEMP}/tf-plan.log" - name: Wait for resource group deletion to complete @@ -239,9 +243,23 @@ jobs: - name: Explain a missing bootstrap role if: failure() run: | - if ! grep -qiE 'could not find role|Role Definition .* was not found' "${RUNNER_TEMP}/tf-apply.log" 2>/dev/null; then - exit 0 - fi + # Both logs: the data source is read at plan time as well as apply, so a + # missing role can surface in either step depending on state freshness. + # + # Each file is grepped separately rather than passing both to one grep. + # With multiple operands where one does not exist, grep's exit status is + # implementation-defined: GNU returns 0 when -q matched an earlier file, + # BSD returns 2. Either log may legitimately be absent when an earlier + # step failed first, so the combined form would go silent on some hosts. + found=0 + for log in "${RUNNER_TEMP}/tf-plan.log" "${RUNNER_TEMP}/tf-apply.log"; do + [ -f "$log" ] || continue + if grep -qiE 'could not find role|Role Definition .* was not found' "$log"; then + found=1 + break + fi + done + [ "$found" -eq 1 ] || exit 0 cat >&2 <<'EOT' ::error title=Missing bootstrap role::The custom reservation-purchaser role does not exist in this subscription. From eb0a87283289a610cd1fdd4d9c8ac8084d766b33 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 11 Aug 2026 00:29:59 +0200 Subject: [PATCH 3/3] docs(ci/azure): correct two false claims in the diagnostic's comments Review findings on the prose, both in a change whose entire purpose is that a future reader understands why the deploy failed. The grep-exit-status claim was wrong. The comment asserted that multi-operand behaviour is implementation-defined, GNU returning 0 and BSD 2. Measured on both real binaries, they are identical and match POSIX: -q exits 0 if a line is selected even when another operand errored, and the only combinations returning 2 are those with no match anywhere, where silence is the wanted behaviour. The combined form would have worked. The contradicting measurement came from ugrep: `grep` in the authoring shell is a function that execs ugrep 7.5.0, so the experiment measured neither the system grep nor what CI runs. The per-file loop is kept because it makes the expected "this log does not exist" case explicit at the point of reading, not because the alternative is broken. The plan-time claim overstated what was observed. The role lookup is deferred to apply today, reported by the plan as "will be read during apply (config refers to values not yet known)". That is contingent on the module-level depends_on at terraform/environments/azure/compute.tf:75 rather than structural, so teeing both logs is still correct; the comment now names the depends_on so the next reader can check whether it still holds. Refs #1794 --- .github/workflows/deploy-azure.yml | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/deploy-azure.yml b/.github/workflows/deploy-azure.yml index a59520ba1..f541f174f 100644 --- a/.github/workflows/deploy-azure.yml +++ b/.github/workflows/deploy-azure.yml @@ -243,14 +243,19 @@ jobs: - name: Explain a missing bootstrap role if: failure() run: | - # Both logs: the data source is read at plan time as well as apply, so a - # missing role can surface in either step depending on state freshness. + # 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. # - # Each file is grepped separately rather than passing both to one grep. - # With multiple operands where one does not exist, grep's exit status is - # implementation-defined: GNU returns 0 when -q matched an earlier file, - # BSD returns 2. Either log may legitimately be absent when an earlier - # step failed first, so the combined form would go silent on some hosts. + # 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 + # when an earlier step failed first. Passing both to one grep would also + # work (POSIX: -q exits 0 if a line is selected, and the only combinations + # returning 2 are those with no match anywhere, where silence is wanted), + # but it leaves that reasoning implicit. found=0 for log in "${RUNNER_TEMP}/tf-plan.log" "${RUNNER_TEMP}/tf-apply.log"; do [ -f "$log" ] || continue