refactor(aws): drop the IAM access key fallback from the three WIF backplanes - #287
Closed
JohannesRudolph wants to merge 7 commits into
Closed
refactor(aws): drop the IAM access key fallback from the three WIF backplanes#287JohannesRudolph wants to merge 7 commits into
JohannesRudolph wants to merge 7 commits into
Conversation
The backplane offered two credential paths: workload identity federation, or an IAM user with an `aws_iam_access_key` when `workload_identity_federation` was left null. Supporting both cost a `count` on six resources, a policy name that branched on the same condition, and a `credentials` output that published the string "N/A; workload identity federation in use" on the federated path. `.agents/references/aws-backplane.md` already forbids this: "Long-lived IAM access keys for single-account building blocks — use WIF (Pattern A) instead". The new `aws_wif_no_access_key` and `aws_wif_nonnullable` scorecard checks now enforce it. Nothing was on the null branch. The integration always passes a non-null `workload_identity_federation` built from `data.meshstack_integrations`, so the IAM user, its policy attachment and its access key were dead code — and `internal-cloudfoundation`'s meshcloud-prod deployment is on the federated path. `workload_identity_federation` becomes `nullable = false`, the `count` guards go, the policy keeps its federated-path name (`Route53RecordBuildingBlockFederatedPolicy-*`) so no live policy is renamed, and the `credentials` output is deleted. `moved` blocks carry `aws_iam_role.assume_federated_role` and its policy attachment across the removed `count`. Without them a deployment already on the federated path would destroy and recreate its live IAM role on the next apply — unlike the GCP equivalent, these `[0]` addresses exist in production state. Module scores 100% on the AWS Backplane category. `tofu validate` passes on the backplane and on the integration root wired against it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… WIF only The backplane offered two credential paths: workload identity federation, or an IAM user with an `aws_iam_access_key` when `workload_identity_federation` was left null. Supporting both cost a `count` on six resources, a policy name that branched on the same condition, and a `credentials` output that published the string "N/A; workload identity federation in use" on the federated path. `.agents/references/aws-backplane.md` already forbids this: "Long-lived IAM access keys for single-account building blocks — use WIF (Pattern A) instead". The new `aws_wif_no_access_key` and `aws_wif_nonnullable` scorecard checks now enforce it. Nothing was on the null branch. The integration always passes a non-null `workload_identity_federation` built from `data.meshstack_integrations`, so the IAM user, its policy attachment and its access key were dead code — and `internal-cloudfoundation`'s meshcloud-prod deployment is on the federated path. `workload_identity_federation` becomes `nullable = false`, the `count` guards go, the policy keeps its federated-path name (`Route53AliasRecordBuildingBlockFederatedPolicy-*`) so no live policy is renamed, and the `credentials` output is deleted. `moved` blocks carry `aws_iam_role.assume_federated_role` and its policy attachment across the removed `count`. Without them a deployment already on the federated path would destroy and recreate its live IAM role on the next apply — unlike the GCP equivalent, these `[0]` addresses exist in production state. Module scores 100% on the AWS Backplane category. `tofu validate` passes on the backplane and on the integration root wired against it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same conversion as the two route53 backplanes, plus two conventions this module never had. The backplane offered two credential paths: workload identity federation, or an IAM user with an `aws_iam_access_key` when `workload_identity_federation` was left null — a `count` on four resources, a policy name branching on the same condition, and a `credentials` output. The integration always passes a non-null `workload_identity_federation`, so the key path was dead code. `.agents/references/aws-backplane.md` forbids it for a single-account building block, and `aws_wif_no_access_key` / `aws_wif_nonnullable` now enforce that. Two more scorecard checks were failing here but not on the route53 modules: - **`create_oidc_provider`** was missing, so the backplane always created its own OIDC provider. AWS allows one provider per issuer URL per account, so a second backplane in the same account failed its apply with `EntityAlreadyExists`. The variable and the `data` lookup are now present, matching the route53 modules, and the integration passes it through. - **The role ARN output** was named `workload_identity_federation_role_arn`; the convention is `workload_identity_federation_role`. Renamed, with the BBD's `AWS_ROLE_ARN` input updated. `workload_identity_federation` becomes `nullable = false`, the policy keeps its federated-path name (`S3BuildingBlockFederatedPolicy-*`), and `moved` blocks carry the role and its policy attachment across the removed `count` so a federated deployment does not recreate its live IAM role. Not changed: this integration still takes a hand-supplied `variable "workload_identity"` instead of reading `data.meshstack_integrations` the way the route53 modules and the reference's wiring example do. That is a separate input-contract change for whoever is next in the module — no scorecard check covers it. Module scores 100% on the AWS Backplane category. `tofu validate` passes on the backplane and on the integration root wired against it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Scorecard Check
Warning: module "aws/oidc-provider" not found — skipping. 📊 meshstack-hub Module Scorecard
📋 Per-Module Category SummaryScore per category per building block.
Core Structure — ✅ all passingBasic module file structure and documentation — applies to 3 modules
Core Structure — Summary
Integration — ✅ all passingmeshstack_integration.tf conventions — applies to 3 modules
Integration — Summary
AWS Backplane — ✅ all passingAWS automation principal conventions (WIF or cross-account StackSet) — applies to 3 modules
AWS Backplane — Summary
Azure Backplane — not applicableAzure UAMI-based automation principal conventions — applies to 0 modules No applicable modules. GCP Backplane — not applicableGCP workload-identity-federation automation principal conventions — applies to 0 modules No applicable modules. STACKIT Backplane — not applicableSTACKIT WIF-based automation principal conventions — applies to 0 modules No applicable modules. Testing — some checks failingEnd-to-end test coverage — applies to 3 modules
Testing — Summary
📈 Overall SummaryOverall Average Score: 95%Score Distribution
|
First e2e test for any AWS hub module. Build-from-source mode provisions an ephemeral backplane in the smoke-test AWS account, builds the BBD from this branch, orders a workspace-level building block, and asserts on its outputs — the bucket ARN in particular, since that is what proves the bucket was created by the federated backplane role rather than the name being echoed back. Modelled on `gcp/storage-bucket/e2e`, with two AWS-specific details: - `create_oidc_provider` comes from the fixture and is false. AWS permits one OIDC provider per issuer URL per account, so the meshStack issuer's provider is a long-lived fixture in the smoke-test account rather than something each backplane creates and destroys — otherwise the second AWS case in the parallel matrix fails with EntityAlreadyExists and the first to finish deletes the provider the other is still using. - The integration takes `workload_identity` as a hand-supplied object rather than reading `data.meshstack_integrations` itself (unlike the route53 modules), so the e2e module reads the data source and derives the subject namespace prefix from the replicator's own subject. `provider "aws"` pins `allowed_account_ids` to the fixture account, so a wrong local session errors instead of creating IAM roles in someone else's account. Requires the AWS fixture in the smoke-test harness, which does not exist yet — the test is discovered by CI but cannot pass until that lands and is applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AWS registers one OIDC provider per issuer URL per account, so the meshStack runner's issuer can be registered only once in an account regardless of how many building block backplanes federate through it. Today each AWS backplane creates its own and offers a `create_oidc_provider` toggle to opt out, which makes the first backplane deployed the de-facto owner of shared infrastructure: the second one fails with EntityAlreadyExists unless someone remembers the toggle, and destroying the owner breaks every other backplane in the account. This module is that provider, on its own, applied once per AWS account. It takes no inputs — issuer, audience and thumbprint come from `data.meshstack_integrations` — so the only thing a platform team supplies is the two providers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…provider Replaces the two checks that assumed a backplane owns its OIDC provider: - `aws_wif_oidc_provider` asserted the backplane *creates or reads* an `aws_iam_openid_connect_provider`. Now `aws_wif_external_oidc_provider` asserts the opposite — no provider resource, and a required non-nullable `oidc_provider_arn` input instead. - `aws_wif_create_oidc_provider` required the opt-out toggle. The toggle is gone with the resource, so a leftover `create_oidc_provider` variable is now itself a failure. Plus `aws_oidc_provider_notice`, which lints the fixed two-line description on both `oidc_provider_arn` (backplane) and `aws_oidc_provider_arn` (integration). That notice is the only signpost a first-time platform engineer gets: AWS has no plural OIDC-provider data source, so a missing provider cannot be turned into a friendly precondition, and terraform-docs renders backplane variables but not integration comments. Linting it keeps the copy-paste honest. `.agents/references/aws-backplane.md` gains 'The shared OIDC provider' as the canonical section both checks point at: why an account-level singleton cannot belong to a building block, the one-line module that owns it, why Azure and GCP genuinely differ, and the `removed`-block migration for an account whose provider still sits in a backplane's state. The three WIF modules now fail both checks — the next commit converts them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three WIF backplanes stop creating an `aws_iam_openid_connect_provider` and take `oidc_provider_arn` instead, supplied by `modules/aws/oidc-provider` once per AWS account. What each backplane loses: the provider resource, the `data` lookup that mirrored it, the `create_oidc_provider` toggle, and the `try()` local that picked between the two. What it gains: one required non-nullable variable. The trust policy is unchanged — it still scopes `:sub` to this building block definition's subjects. `s3_bucket` gains the input rather than trading a toggle for it: the previous commit added `create_oidc_provider` to bring it level with the route53 modules, and this removes it from all three. Reviewing that pair as one change is the honest reading — the toggle was never the destination. Each backplane carries a `removed` block with `destroy = false`. An account that already applied one of these owns the provider in that backplane's state; without the block, the next apply would delete it out from under every other backplane federating through the same issuer. With it, the apply just forgets it and the provider can be imported into the root that owns `modules/aws/oidc-provider`. Concretely for `internal-cloudfoundation`'s meshcloud-prod: `dns` owns the provider today and `dns-alias` was already passing `create_oidc_provider = false`, so the migration is one forget plus one import, with no downtime for either. The integrations expose it as `aws_oidc_provider_arn` — flat and provider-prefixed per the repo convention — carrying the same fixed notice as the backplane variable, which the scorecard's `aws_oidc_provider_notice` check enforces on both. `aws/s3_bucket/e2e` reads the ARN from `fixtures.aws.oidc_provider_arn` instead of passing a create-or-not flag, matching the harness that owns the provider for the whole account. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
Superseded by #293, which squashes both onto current |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #286 — review that one first; this PR's diff is only the three commits above it.
#286 added the
AWS Backplanescorecard category and reported three modules as newly flagged without fixing them. This PR fixes them, one commit per module, by dropping the credential path.agents/references/aws-backplane.mdalready forbids.aws/s3_bucketaws/route53-dns-recordaws/route53-dns-alias-recordAll six AWS backplanes are now 🟢 100% on the category.
The shared OIDC provider
Two commits on top of the conversion externalize the OIDC provider, which is the larger change here.
AWS registers one OIDC provider per issuer URL per account, so the meshStack runner's issuer has room for exactly one no matter how many backplanes federate through it. A backplane that creates its own is claiming shared infrastructure: the second backplane in the account fails with
EntityAlreadyExists, and destroying whichever owns it breaks every other one. That is unlike Azure (a federated credential is a child of its UAMI) and GCP (a pool is a named per-project resource), where every module can own its own and none can collide.So
modules/aws/oidc-providernow owns it — zero inputs, reading issuer, audience and thumbprint fromdata.meshstack_integrations— applied once per AWS account. Every WIF backplane takesoidc_provider_arnand creates nothing, losing the provider resource, the mirroringdatalookup, thecreate_oidc_providertoggle and thetry()local that picked between them.Migration is a forget plus an import, not a destroy. Each backplane ships
removed { … lifecycle { destroy = false } }, so an account that already applied one drops the provider from that state without deleting it out from under its siblings. For meshcloud-prod that is one forget (dnsowns it today) and one import;dns-aliasalready passedcreate_oidc_provider = false.The scorecard follows:
aws_wif_oidc_provider(asserted the backplane has a provider) is inverted intoaws_wif_external_oidc_provider,aws_wif_create_oidc_provideris deleted, andaws_oidc_provider_noticelints the fixed two-line description on bothoidc_provider_arnand the integration'saws_oidc_provider_arn. That notice is the only signpost a first-time platform engineer gets — AWS has no plural OIDC-provider data source, so a missing provider cannot be turned into a friendly precondition, and terraform-docs renders backplane variables but not integration comments.All six AWS backplanes are 🟢 100% on the category.
The change
Each backplane offered two credential paths: workload identity federation, or an IAM user with an
aws_iam_access_keywhenworkload_identity_federationwas left null. Nothing was ever on the null branch — every integration builds a non-nullworkload_identity_federation, andinternal-cloudfoundation's meshcloud-proddns/dns-aliasdeployments are federated. So the null branch was dead code carrying acounton four to six resources, a policy name that branched on the same condition, and acredentialsoutput that published the literal string"N/A; workload identity federation in use".workload_identity_federationbecomesnullable = false, thecountguards and thecredentialsoutput go, and each policy keeps its federated-path name (…FederatedPolicy-*) so no live IAM policy is renamed.s3_bucketneeded two things the route53 modules already had: acreate_oidc_providertoggle (AWS allows one OIDC provider per issuer URL per account, so a second backplane in the same account was failing its apply withEntityAlreadyExists), and its role ARN output renamed fromworkload_identity_federation_role_arnto the conventionalworkload_identity_federation_role, with the BBD input updated to match.movedblocks — the one thing the reference does not coverUnlike the GCP equivalent (7c825b9), the federated
[0]addresses here exist in production state. Removing thecountwithoutmovedblocks would destroy and recreate a live IAM role the moment meshsre bumps the hub ref. Each module therefore carries:.agents/references/aws-backplane.mdsays nothing about migrating off the fallback shape — that is a genuine gap this exercise surfaced. Eachbackplane/README.mdnow documents the migration; whether the reference should carry a general rule is worth a follow-up.Migration for anyone still on the key path
internal-cloudfoundation's meshcloud-devs3_bucketdeployment (pinned to an old SHA) is the only known one. It must passworkload_identity_federation; the next apply destroys its IAM user and revokes the key. That is the intended migration, same as the GCP key-path removal.Not changed
s3_bucket's integration still takes a hand-suppliedvariable "workload_identity"instead of readingdata.meshstack_integrationsthe way the route53 modules and the reference's wiring example do. That is an input-contract change for its consumers and no scorecard check covers it — left for whoever is next in the module.Verification — and what could not be verified
Green:
tofu validate— all threebackplane/modules.tofu validateon each integration root wired against its local backplane (modulesourcetemporarily repointed to./backplane), which is what actually proves thes3_bucketoutput rename and the newcreate_oidc_providerpass-through are wired correctly.terraform_docs,terraform_fmt, trailing-whitespace,validate_modules.sh) — this is exactly what CI runs.A fourth commit adds
modules/aws/s3_bucket/e2e— the first e2e test for any AWS hub module. It provisions an ephemeral backplane, builds the BBD from this branch, orders a workspace-level building block and asserts on its outputs, the bucket ARN in particular (that is what proves the bucket was created by the federated backplane role rather than the name being echoed back). It validates with its whole module chain resolved —../→ the integration → the backplane fetched from GitHub at this branch — which is what proves the renamed output and the newcreate_oidc_providerline up end to end.Still not verified against a live AWS account. The test is discovered by CI but cannot pass yet: the smoke-test harness has no AWS fixture — no account, no credentials — so there is nothing for it to run against. A fixture PR is up for review separately; until it is applied, this remains:
Not verified — no e2e test was run, and none could be:
e2e/directory. All seven sit at 🔴 33% Testing..tfor workflow file mentioning AWS. Hub-e2e (build-from-source) mode is therefore impossible for AWS today; AWS onboarding is a later wave of the coverage initiative.s3_bucketbuildingblock's existingtofu testcreates a real S3 bucket and needs AWS credentials, which this environment does not have.So: the conversion is verified as far as static analysis and the repo's own CI go, and unverified against a live AWS account. Worth applying the backplane by hand in meshcloud-dev before merging, or landing AWS smoke-test onboarding first.
🤖 Generated with Claude Code