diff --git a/.github/workflows/rollback.yml b/.github/workflows/rollback.yml index a0dadda19..6aa8cb31c 100644 --- a/.github/workflows/rollback.yml +++ b/.github/workflows/rollback.yml @@ -16,10 +16,35 @@ name: Rollback Deployment # a deployment environment, so no OIDC token can be minted by a job outside the # environment's protection rules. # +# Every rollback job now binds the SAME environment name its corresponding +# deploy job uses (`dev`/`staging`/`prod` for Lambda/GCP/Azure, +# `aws-fargate-` for Fargate) instead of a compound +# `--rollback` name. This was NOT symmetric across clouds: +# - AWS already trusted all six compound `aws-{lambda,fargate}-- +# rollback` subjects (terraform/environments/aws/ci-cd-permissions/ +# role.tf, added in #1648) -- rollback-aws-lambda and +# rollback-aws-fargate could already authenticate before this change. +# - Azure never trusted ANY compound rollback subject -- its federated +# credentials (sp.tf) only cover plain dev/staging/prod -- so +# rollback-azure failed with AADSTS70021 on every attempt. This is the +# one rollback job this PR actually fixes authentication for. +# - GCP's trust condition (github_oidc.tf) checks repository+ref, never +# `sub`, so the environment name was never load-bearing there either +# way. +# Rebinding every job to match its deploy's environment name removes the +# now-dead AWS compound subjects (deleted from role.tf in this PR) in favour +# of reusing environments that already exist and are already trusted, +# rather than adding six more. Tradeoff: rollback-aws-lambda, rollback-gcp +# and rollback-azure now share their approval gate with the corresponding +# deploy environment instead of having a distinct rollback-only one; +# rollback-aws-fargate already shared it with deploy-aws-fargate even before +# this change; see #139. +# # The binding is necessary but not sufficient: GitHub auto-creates a referenced -# environment on first use with NO protection rules. The `--rollback` -# environments must therefore be configured with required reviewers in repo -# settings for the approval gate to actually stop anything. +# environment on first use with NO protection rules. `dev`/`staging`/`prod` +# and `aws-fargate-` must therefore be configured with required +# reviewers in repo settings for the approval gate to actually stop anything +# (#141). permissions: contents: read @@ -192,8 +217,9 @@ jobs: permissions: id-token: write contents: read - environment: - name: aws-lambda-${{ inputs.environment }}-rollback + # Plain dev/staging/prod, not aws-lambda--rollback: see the + # workflow-level comment above. + environment: ${{ inputs.environment }} steps: - name: Checkout code @@ -296,8 +322,11 @@ jobs: permissions: id-token: write contents: read - environment: - name: aws-fargate-${{ inputs.environment }}-rollback + # aws-fargate-, matching deploy-aws-fargate.yml's own binding + # (already trusted, role.tf) -- not plain dev/staging/prod (that's + # Lambda's environment) and not aws-fargate--rollback (dead, no + # longer trusted; see the workflow-level comment above). + environment: aws-fargate-${{ inputs.environment }} steps: - name: Checkout code @@ -376,8 +405,9 @@ jobs: permissions: id-token: write contents: read - environment: - name: gcp-${{ inputs.environment }}-rollback + # Plain dev/staging/prod, not gcp--rollback: see the workflow-level + # comment above. + environment: ${{ inputs.environment }} steps: - name: Checkout code @@ -458,8 +488,11 @@ jobs: permissions: id-token: write contents: read - environment: - name: azure-${{ inputs.environment }}-rollback + # Plain dev/staging/prod, not azure--rollback: see the + # workflow-level comment above. Also the fix for #139: Azure's federated + # credentials cover dev/staging/prod (sp.tf), never the compound rollback + # names, so azure/login failed with AADSTS70021 on every prior rollback. + environment: ${{ inputs.environment }} steps: - name: Checkout code diff --git a/terraform/environments/aws/ci-cd-permissions/README.md b/terraform/environments/aws/ci-cd-permissions/README.md index a5f7ed0c5..d8d7dad54 100644 --- a/terraform/environments/aws/ci-cd-permissions/README.md +++ b/terraform/environments/aws/ci-cd-permissions/README.md @@ -205,12 +205,16 @@ not assumed from an earlier issue's snapshot: | `aws-fargate-staging` | Yes | none | none | | `aws-fargate-prod` | **No** | n/a | n/a | | `aws-db-dev` / `aws-db-staging` / `aws-db-prod` | **No** (all three) | n/a | n/a | -| `aws-lambda-{dev,staging,prod}-rollback` | **No** (all three) | n/a | n/a | -| `aws-fargate-{dev,staging,prod}-rollback` | **No** (all three) | n/a | n/a | -Only 3 of the 15 environments this allowlist names exist yet, and none of the 3, including `dev` +`rollback.yml`'s rollback-aws-lambda and rollback-aws-fargate jobs bind to `dev`/`staging`/`prod` +and `aws-fargate-` respectively -- the same environments their corresponding deploy jobs use, +listed above -- rather than a separate `--rollback` family (#139). This allowlist +previously carried six `aws-{lambda,fargate}--rollback` subjects added for #1648; they were +removed once nothing presented them any more, rather than left as unused trust surface. + +Only 3 of the 9 environments this allowlist names exist yet, and none of the 3, including `dev` which multiple deploy jobs already use in production, has a branch policy or protection rules of any -kind. Every environment above needs (2) configured (and the 12 that don't exist yet also need to be +kind. Every environment above needs (2) configured (and the 6 that don't exist yet also need to be created) before this allowlist actually delivers "only `main` deploys" rather than "only these environments deploy, from any branch". This module has no GitHub provider configured (no `provider "github"` or `github_repository_*` resource anywhere under `terraform/` or `iac/`), so diff --git a/terraform/environments/aws/ci-cd-permissions/role.tf b/terraform/environments/aws/ci-cd-permissions/role.tf index 9dec5045d..6d4cc3b67 100644 --- a/terraform/environments/aws/ci-cd-permissions/role.tf +++ b/terraform/environments/aws/ci-cd-permissions/role.tf @@ -127,21 +127,14 @@ resource "aws_iam_role" "cudly_deploy" { "repo:${var.github_repo}:environment:aws-db-staging", "repo:${var.github_repo}:environment:aws-db-prod", - # rollback.yml's rollback-aws-lambda / rollback-aws-fargate jobs - # bind to `aws-lambda-${{ inputs.environment }}-rollback` / - # `aws-fargate-${{ inputs.environment }}-rollback`; inputs.environment - # is a workflow_dispatch choice input constrained to - # dev/staging/prod. Listing the subject here only fixes control - # (1) above (this issue, #1648); it does nothing for controls - # (2) or (3). Live check: none of these six exist yet -- first - # dispatch auto-creates them bare, with neither a branch policy - # nor reviewers, unless #1660's provisioning work lands first. - "repo:${var.github_repo}:environment:aws-lambda-dev-rollback", - "repo:${var.github_repo}:environment:aws-lambda-staging-rollback", - "repo:${var.github_repo}:environment:aws-lambda-prod-rollback", - "repo:${var.github_repo}:environment:aws-fargate-dev-rollback", - "repo:${var.github_repo}:environment:aws-fargate-staging-rollback", - "repo:${var.github_repo}:environment:aws-fargate-prod-rollback", + # rollback.yml's rollback-aws-lambda binds to plain + # dev/staging/prod (covered above) and rollback-aws-fargate + # binds to aws-fargate- (covered above), matching each + # job's own deploy workflow, rather than a compound + # `--rollback` name. See #139. This list previously + # had six `aws-{lambda,fargate}--rollback` entries added + # for #1648; nothing presents those subjects any more, so they + # were removed rather than left as unused trust surface. ] } } diff --git a/terraform/environments/azure/ci-cd-permissions/variables.tf b/terraform/environments/azure/ci-cd-permissions/variables.tf index bacf6e63d..a11cd6f31 100644 --- a/terraform/environments/azure/ci-cd-permissions/variables.tf +++ b/terraform/environments/azure/ci-cd-permissions/variables.tf @@ -25,13 +25,13 @@ variable "github_environments" { This list is NOT a complete enumeration of the environment subjects this repo presents to Azure. It covers the destroy jobs bound by cleanup-staging.yml - (`staging`) and destroy-fargate-dev.yml (`dev`), plus deploy-azure.yml's - build-and-deploy and test-deployment jobs, which bind plain - `dev`/`staging`/`prod` (see #140). + (`staging`) and destroy-fargate-dev.yml (`dev`), deploy-azure.yml's + build-and-deploy and test-deployment jobs (see #140), and rollback.yml's + rollback-azure job (see #139) — all of which bind plain + `dev`/`staging`/`prod`. - Knowingly NOT covered, tracked in #1648 — these Azure jobs bind to compound - environment names and therefore still fail with AADSTS70021: - - rollback.yml -> azure-{dev,staging,prod}-rollback + Knowingly NOT covered, tracked in #1648 — this Azure job binds a compound + environment name and therefore still fails with AADSTS70021: - database-migration.yml -> azure-db-{dev,staging,prod} They are excluded here rather than fixed because each needs its environment