From be6a6dd6d3ac97f4433d17bb47420a3e3127ffbb Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 20 Oct 2026 00:56:52 +0100 Subject: [PATCH] chore(iac/azure): reconcile prod in github_environments with its gating rule The variable's description said a credential must not be minted for an environment that does not exist and is not gated, while its default has included `prod` since #358. When #364 was filed no `prod` environment existed. Since 2026-10-04 `dev`, `staging` and `prod` all exist with a deployment branch policy of exactly `main` (#141, #2), so `prod` now meets the rule. The description still read as if it did not. State the gating rule explicitly: a name may be added once its environment exists with a main-only branch policy, because the service principal already trusts the main-branch subject and such an environment's credential therefore adds no reach. Say that the default meets it, `prod` included, that no environment has required reviewers, and that widening a policy past `main` (e.g. a tag policy for release deploys) widens the credential's reach. Update the azure-db-* note, whose stated reason (environments not created yet) is no longer true, and point the README's "allow a new environment" step at the rule. Add a mock-provider terraform test pinning the default credential set to exactly dev/staging/prod and the main-branch subject, so a change to who can obtain the service principal has to touch the test and the rule. No resource changes: the default is unchanged, so no bootstrap re-apply is needed. Closes #364 Co-Authored-By: Claude Opus 5.5 --- .../azure/ci-cd-permissions/README.md | 9 ++-- .../tests/github_environments.tftest.hcl | 45 +++++++++++++++++++ .../azure/ci-cd-permissions/variables.tf | 27 +++++++++-- 3 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 terraform/environments/azure/ci-cd-permissions/tests/github_environments.tftest.hcl diff --git a/terraform/environments/azure/ci-cd-permissions/README.md b/terraform/environments/azure/ci-cd-permissions/README.md index b71b432c6..ca9e240b1 100644 --- a/terraform/environments/azure/ci-cd-permissions/README.md +++ b/terraform/environments/azure/ci-cd-permissions/README.md @@ -183,6 +183,9 @@ obtain the deploy service principal. Restrict the branch via the environment's `workflow_dispatch` runs the workflow file as it exists on the dispatched ref, so the check can be removed on that branch. -To allow a new deployment environment, add its name to `var.github_environments`. To -deploy from a different branch or repo, add a further -`azuread_application_federated_identity_credential` resource to `sp.tf`. +To allow a new deployment environment, first create it with a deployment branch +policy that admits only `main`, then add its name to `var.github_environments` +(see the gating rule in that variable's description; the default `dev`, +`staging` and `prod` all meet it). To deploy from a different branch or repo, +add a further `azuread_application_federated_identity_credential` resource to +`sp.tf`. diff --git a/terraform/environments/azure/ci-cd-permissions/tests/github_environments.tftest.hcl b/terraform/environments/azure/ci-cd-permissions/tests/github_environments.tftest.hcl new file mode 100644 index 000000000..1cf1f6a95 --- /dev/null +++ b/terraform/environments/azure/ci-cd-permissions/tests/github_environments.tftest.hcl @@ -0,0 +1,45 @@ +# Pins which GitHub environments get a federated credential on the deploy +# service principal by default (#364). Every name here must meet the gating +# rule on var.github_environments: the environment exists with a deployment +# branch policy that admits only `main`. Changing this set changes who can +# obtain the service principal and needs a bootstrap re-apply, so update the +# variable's description and this test together. + +mock_provider "azurerm" {} +mock_provider "azuread" {} + +variables { + subscription_id = "00000000-0000-0000-0000-000000000000" +} + +run "default_environments_are_the_gated_plain_names" { + command = plan + + assert { + condition = toset(keys(azuread_application_federated_identity_credential.github_environment)) == toset(["dev", "staging", "prod"]) + error_message = "default github_environments must be exactly dev, staging and prod; a new name needs an existing environment with a main-only branch policy first (see var.github_environments)" + } + + assert { + condition = try(azuread_application_federated_identity_credential.github_environment["prod"].subject, "") == "repo:LeanerCloud/cloud-commitments-platform:environment:prod" + error_message = "prod credential must trust the plain prod environment subject that deploy-azure.yml and rollback.yml present" + } + + assert { + condition = azuread_application_federated_identity_credential.github_main[0].subject == "repo:LeanerCloud/cloud-commitments-platform:ref:refs/heads/main" + error_message = "the main-branch subject is what makes a main-only environment credential add no reach; it must stay" + } +} + +run "no_environment_credentials_without_repo" { + command = plan + + variables { + github_repo = "" + } + + assert { + condition = length(azuread_application_federated_identity_credential.github_environment) == 0 + error_message = "an empty github_repo must create no environment credentials" + } +} diff --git a/terraform/environments/azure/ci-cd-permissions/variables.tf b/terraform/environments/azure/ci-cd-permissions/variables.tf index a11cd6f31..e21c5136e 100644 --- a/terraform/environments/azure/ci-cd-permissions/variables.tf +++ b/terraform/environments/azure/ci-cd-permissions/variables.tf @@ -23,6 +23,24 @@ variable "github_environments" { deployment_branch_policy — an in-workflow ref check does not bind, because workflow_dispatch runs the file as it exists on the dispatched ref. + Gating rule: add a name only once its GitHub environment exists with a + deployment branch policy that admits only `main`. This service principal + already trusts repo::ref:refs/heads/main (sp.tf, github_main), + and only a run on `main` can enter such an environment, so its credential + adds no reach. A credential for an environment that does not exist widens + access: GitHub creates a referenced environment on first use with no + protection, so any branch could bind it and obtain this service principal. + Environments are repository settings, not managed here; check with + `gh api repos//environments//deployment-branch-policies`. + + The default meets the rule, `prod` included: `dev`, `staging` and `prod` + each exist with a `main`-only branch policy (applied 2026-10-04, see #141 + and #2), and `prod` uses the same service principal as the other two, so it + is not an exception. None has required reviewers, so the branch policy + (plus main's ruleset) is the only gate. Widening a policy past `main`, for + example a `v*` tag policy for release-triggered `prod` deploys, widens this + credential's reach to those refs and needs the same review. + 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`), deploy-azure.yml's @@ -34,10 +52,11 @@ variable "github_environments" { 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 - created and gated first; minting a credential for an ungated environment that - does not yet exist would widen access without adding a control. Do not read - the absence of a name as "no job uses it". + They were excluded because their environments did not exist yet. Those + environments now exist with a `main`-only branch policy (2026-10-04), so + they meet the gating rule; adding them is left to #1648 because it mints + three new credentials and needs a bootstrap re-apply. Do not read the + absence of a name as "no job uses it". EOT type = list(string) default = ["dev", "staging", "prod"]