fix(gcp): scope workload identity federation to one building block definition - #291
fix(gcp): scope workload identity federation to one building block definition#291JohannesRudolph wants to merge 3 commits into
Conversation
The GCP backplanes were the only ones whose federation subject stopped at `buildingblockdefinition`, so their pools admitted every building block definition in the workspace. The uuid was missing because feeding it back in closes a dependency cycle: the definition carries the backplane's credentials. Write down both halves — the subject convention every other provider already follows, and the two steps that keep the uuid off the credential path (subjects in their own variable, pool provider resource name assembled rather than read back). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Scorecard Check
📊 meshstack-hub Module Scorecard
📋 Per-Module Category SummaryScore per category per building block.
Core Structure — ✅ all passingBasic module file structure and documentation — applies to 2 modules
Core Structure — Summary
Integration — ✅ all passingmeshstack_integration.tf conventions — applies to 2 modules
Integration — Summary
Azure Backplane — not applicableAzure UAMI-based automation principal conventions — applies to 0 modules No applicable modules. GCP Backplane — ✅ all passingGCP workload-identity-federation automation principal conventions — applies to 2 modules
GCP Backplane — Summary
STACKIT Backplane — not applicableSTACKIT WIF-based automation principal conventions — applies to 0 modules No applicable modules. Testing — ✅ all passingEnd-to-end test coverage — applies to 2 modules
Testing — Summary
📈 Overall SummaryOverall Average Score: 100%Score Distribution
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
The pool provider matched `google.subject.startsWith()` against a subject that stopped at `buildingblockdefinition`, so any building block definition in the owning workspace could federate into the backplane's service account and inherit `roles/storage.admin` on the project. Pin the definition's uuid in the subject and match it exactly, as every other provider's backplane already does. Two changes keep the uuid from closing a dependency cycle back through `credentials_json`, which the definition carries as a FILE input: - The subjects move out of `workload_identity_federation` into their own variable. OpenTofu tracks module input dependencies per variable, so a subjects field would drag the pool, the workloadIdentityUser binding and the IAM propagation wait onto the credential path with it. - The audience is assembled from the project number and the pool identifier instead of reading the pool provider's `name` back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same defect and same fix as the storage-bucket backplane: the pool provider matched `google.subject.startsWith()` against a subject without the building block definition uuid, so any definition in the owning workspace could federate into the backplane's service account and inherit `roles/billing.costsManager` on the billing account. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2c32c0b to
bd6982a
Compare
| @@ -1,3 +1,13 @@ | |||
| locals { | |||
| # Deliberately assembled from the provider's own identifiers rather than read off | |||
There was a problem hiding this comment.
d: a short "workaround: construct pool identifier to break dependency cycle, see ./agents"
is sufficient
| backplanes repeatedly (an e2e test) must derive a unique identifier per run. The soft-deleted pools | ||
| count against the project's pool limit while they linger. | ||
|
|
||
| #### Subject matching is exact |
There was a problem hiding this comment.
d: this is now a general pattern for all GCP backplanes. don't repeat this blurb in every README.md when its already in the shared instructions. Same applies to the other backplane/README.md touched in this PR
|
Not pursuing this. Pinning the building block definition uuid into the pool provider's The trade-off is accepted deliberately. Note that Recorded in |
1 participant
The defect
Both GCP backplanes built their workload identity pool provider's condition as
and both integrations passed a subject that stopped short of the building block definition uuid:
The building block runner names its per-run Kubernetes service account
workspace.<workspace>.buildingblockdefinition.<bbd-uuid>, so the token'ssubis that prefix plusthe uuid. A
startsWithagainst a subject without the uuid therefore admits every building blockdefinition owned by that workspace.
Concretely: a workspace member who can create a second building block definition in the same
workspace — any definition, running any Terraform — gets a runner token the pool accepts. It can then
impersonate the backplane's service account and use it, inheriting
roles/storage.adminon theproject (storage-bucket) or
roles/billing.costsManagerandroles/billing.vieweron the billingaccount (budget-alert). No compromise needed; ordinary workspace-level rights are enough.
modules/azure/*,modules/stackit/*andmodules/aws/*all pin the uuid already. GCP was theoutlier.
Why the uuid was missing
Both integrations feed the backplane's credentials into the definition:
so the definition already depends on the backplane. Passing the definition's uuid back in as a
subject closes a cycle. Appending it naively yields, verbatim from
tofu validate:How the cycle is broken
Same principle as
modules/aws/: nothing on the credential path may depend on a resource thatconsumes the subjects. GCP needed two steps.
1. The subjects moved out of
workload_identity_federationinto their ownvariable "workload_identity_subjects". OpenTofu tracks module input dependencies per variable,not per attribute — the two
var.workload_identity_federationnodes in the cycle above are exactlythis. A
subjectsfield inside that object taints every resource that reads any other field of it:the pool (via
workload_identity_pool_identifier), theroles/iam.workloadIdentityUserbinding (viathe pool), and
credentials_json(viasubject_token_file_pathand viatime_sleep.wait_for_iam).Splitting the variable confines the dependency to the pool provider, the only resource that needs it.
This is the one interface change: callers of
backplane/passworkload_identity_subjectsalongsideworkload_identity_federationinstead of nesting it. Both in-repo callers are updated.2.
credentials_jsonassembles the pool provider's resource name instead of reading it back.Identifiers now constructed as strings
Reviewers should check these against the live resources — they are the part that can silently drift.
credentials_json//iam.googleapis.com/${google_iam_workload_identity_pool_provider.meshstack.name}//iam.googleapis.com/projects/${data.google_project.<x>.number}/locations/global/workloadIdentityPools/${var.workload_identity_federation.workload_identity_pool_identifier}/providers/${var.workload_identity_federation.workload_identity_pool_identifier}That is the only one. The format is the provider's documented resource name for a pool provider
(
projects/{project_number}/locations/global/workloadIdentityPools/{pool_id}/providers/{provider_id}),and both modules already set the pool id and the provider id to the same input, so the string is
fully determined by inputs plus the project number.
storage-bucketgains adata "google_project"for that number;
budget-alertalready had one.Everything else still reads real attributes — in particular the service account email, the pool name
in the IAM binding, and the
time_sleep.What I chose not to narrow
The
roles/iam.workloadIdentityUserbinding staysprincipalSet://iam.googleapis.com/<pool>/*rather than becomingprincipal://iam.googleapis.com/<pool>/subject/<sub>.A subject-scoped member would put the subjects back on the credential path:
credentials_jsondepends_ontime_sleep.wait_for_iam, which must wait for this binding — GCP IAM propagation here isthe documented two-to-seven-minute
iam.serviceAccounts.getAccessToken403, and dropping the bindingfrom that wait to buy the narrower member would trade a real failure mode for a theoretical gate.
It also buys nothing here: each backplane creates its own pool, that pool holds one provider, and
that provider now accepts exactly one subject — so the principal set has exactly one member. Both
backplane READMEs and the reference doc say this explicitly so nobody "fixes" it later.
Verification
tofu init -backend=false && tofu validatein bothbackplane/directories../backplane(copied to a scratch path withthe module source rewritten). This is what catches the cycle — the backplane alone validates fine
either way. Also confirmed
tofu graph -type=planbuilds cycle-free for both.node tools/scorecard/scorecard.mjs --module=gcp/storage-bucket --module=gcp/budget-alert— both100%, no regression.
pre-commit run --all-filespasses.gcp/storage-bucketandgcp/budget-alertwere run against this branchand are green. They are the real proof that the assembled audience string is correct: a wrong
audience fails the STS token exchange, and a wrong subject fails the attribute condition.
🤖 Generated with Claude Code