fix(iac): make ibm-iks pass tofu validate — and stop it claiming OIDC federation IBM cannot do - #1107
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes infra/tofu/environments/ibm-iks so it passes tofu init -backend=false && tofu validate by correcting an IBM tag value and removing an invalid IAM trusted profile claim-rule configuration that implied unsupported GitHub Actions OIDC federation.
Changes:
- Adjust IBM user tags to satisfy IBM’s tag regex (no
/), splitting repo owner/name into separate tags. - Remove the invalid
Profile-OIDCclaim rule from the IBM trusted profile module and document the non-federated state clearly. - Update environment/module documentation to reflect the current authentication limitations and implications.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| infra/tofu/modules/ibm-trusted-profile/main.tf | Removes unsupported claim rule; updates module docs and profile description to reflect non-federated state. |
| infra/tofu/environments/ibm-iks/main.tf | Fixes invalid IBM tag (/) and clarifies trusted-profile usage and limitations. |
| infra/tofu/environments/ibm-iks/README.md | Documents validation history and clarifies that CI auth via trusted profile is not currently possible. |
Comments suppressed due to low confidence (1)
infra/tofu/environments/ibm-iks/README.md:1
- This README text is time/PR-relative ("as of the PR that added this line"), which will become stale quickly. Consider rephrasing to a timeless statement (e.g., "This root now passes
tofu validate; previously it failed due to …") so the docs remain accurate when read out of PR context.
# ibm-iks — IBM Cloud IKS substrate (VPC)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name = "github-ci-${var.profile_name_suffix}" | ||
| # Description states the actual state of the profile, not the intent. Anyone | ||
| # reading this in the IBM console should learn that it grants nothing yet. | ||
| description = "NOT FEDERATED — no claim rule. Intended consumer ${var.github_repo}; IBM has no GitHub Actions OIDC path. See infra/tofu/modules/ibm-trusted-profile/main.tf" |
|
Disposition (merge-gate review) IaC validates cleanly: on the cumulative merged tree (#1103 -> #1107/#1108/#1115) all 14/14 roots pass The non-federated labelling is correct and should stay as-is — One small alignment nit: Copilot's inline on the trusted-profile |
… it cannot do infra/tofu/environments/ibm-iks has never passed `tofu validate`. It failed at `tofu init` until #1103 declared the IBM provider source; that unblocked init and exposed two errors that had been unreachable the whole time. Error 1 — IBM user tags must match ^[A-Za-z0-9:_ .-]+$, which rejects the "/" in "repo:SocioProphet/prophet-platform". Split into "org:socioprophet" + "repo:prophet-platform". This is not a new convention: aws-eks, azure-aks and shared/labels.tf already carry a separate "org" = "socioprophet" key and never embed the owner in the repo name. local.prophet_tags is defined per-root (and is a list here, a map elsewhere), so this is scoped to ibm-iks and applies to all four resources that reference it. Error 2 — type = "Profile-OIDC" on ibm_iam_trusted_profile_claim_rule. No such type exists, and this is not a string swap. IBM IAM accepts exactly two: Profile-SAML — federated human users from a registered SAML realm Profile-CR — IBM Cloud compute only; cr_type ∈ VSI, PVS, BMS, IKS_SA, ROKS_SA, CE A GitHub-hosted runner is neither, and cannot mint the compute resource token the cr-token grant requires. IBM's IAM identity-provider API has no OIDC type either — ibm_iam_idp allows only saml, appid, ldap. So there is no value of `type` that federates GitHub Actions; the module's "IBM's equivalent of AWS IRSA / Azure WIF / GCP WIF" header was never achievable. Rather than pick the value that makes the validator happy and federate nothing, the invalid claim rule is removed and the gap is documented in place: module header, calling comment, output description, and README all now say the profile is NOT assumable by CI, with the three supported options and a recommendation. Also flagged, not fixed (out of lane): infra-drift-detect.yml feeds a GitHub OIDC token to grant_type=urn:ibm:params:oauth:grant-type:cr-token, which cannot succeed for the same reason. Verified locally with OpenTofu 1.8.3 (TOFU_VERSION in tofu-plan.yml); Actions is at a spend cap so CI could not run.
cd138aa to
da335aa
Compare
…alidated bar as envs/* #1108 pinned providers and committed locks for the four `envs/*` roots CI validates, and deliberately left the other nine unlocked. This closes that gap for the six `environments/*` and three `bootstrap/*` roots, so every cloud root in the tree is offline-validated on the same terms: `required_version` present, `required_providers` pinned, lock committed, `tofu init -backend=false && tofu validate` clean. All measurement with OpenTofu 1.8.3 — the TOFU_VERSION in tofu-plan.yml — from a clean .terraform with no plugin cache. Four asymmetries closed: * bootstrap/* declared no `required_version`. It was the only tier without one. The terraform block moves to a versions.tf in each root, so all 14 roots now declare versions in the same filename, and `>= 1.8.0` matches every peer. * Three google majors in one estate, none of it recorded. bootstrap/gcp moves ~> 5.0 -> ~> 6.0, joining environments/gcp-* and (via #1108) envs/*. One google major, on purpose. Validated at BOTH 5.45.2 and 6.50.0: clean, no warnings either side. Same call for azurerm, which was split 3.x/4.x for the same non-reason: bootstrap/azure moves ~> 3.0 -> ~> 4.0 to match environments/azure-aks. Also validated at both majors. * environments/aws-eks pulled tls, time, null and cloudinit transitively from terraform-aws-modules/{vpc,eks} under floor-only constraints — `tls >= 3.0.0` was resolving 4.3.0 and would have crossed into 5.x unannounced. A floor without a ceiling is not a pin. All four are now declared at the root with ceilings; tls and null match the constraints already written in shared/versions.tf. No resolved version changed. * bootstrap/aws was the only root validating with a warning: aws_s3_bucket_lifecycle_configuration with neither `filter` nor `prefix`, which the provider says "will be an error in a future version". `filter {}` is the documented way to say "all objects", which is what the rule already meant. Warning gone. Also fixed: environments/gcp-{ephemeral,persistent} carry their own .gitignore that ignores .terraform.lock.hcl. A nested .gitignore overrides its parent, so #1108's un-ignore does not reach them and both locks would have been dropped silently by `git add`. Those two lines are removed; state stays ignored. Locks use the same platform set as #1108 (linux_amd64, darwin_arm64, darwin_amd64) — verified 3 h1 hashes per provider across all 21 entries — and are stable: a second clean init leaves all eight byte-identical. environments/ibm-iks is deliberately untouched. It is the one root that cannot init, #1103 fixes the cause and #1107 the validate errors, and both are open against it. Its lock belongs to whichever of those lands; the generated content is in the PR body. Evidence is local only. GitHub Actions is at a spend cap with hundreds of jobs queued and none running, so no CI run backs any of this.
…alidated bar as envs/* (#1115) #1108 pinned providers and committed locks for the four `envs/*` roots CI validates, and deliberately left the other nine unlocked. This closes that gap for the six `environments/*` and three `bootstrap/*` roots, so every cloud root in the tree is offline-validated on the same terms: `required_version` present, `required_providers` pinned, lock committed, `tofu init -backend=false && tofu validate` clean. All measurement with OpenTofu 1.8.3 — the TOFU_VERSION in tofu-plan.yml — from a clean .terraform with no plugin cache. Four asymmetries closed: * bootstrap/* declared no `required_version`. It was the only tier without one. The terraform block moves to a versions.tf in each root, so all 14 roots now declare versions in the same filename, and `>= 1.8.0` matches every peer. * Three google majors in one estate, none of it recorded. bootstrap/gcp moves ~> 5.0 -> ~> 6.0, joining environments/gcp-* and (via #1108) envs/*. One google major, on purpose. Validated at BOTH 5.45.2 and 6.50.0: clean, no warnings either side. Same call for azurerm, which was split 3.x/4.x for the same non-reason: bootstrap/azure moves ~> 3.0 -> ~> 4.0 to match environments/azure-aks. Also validated at both majors. * environments/aws-eks pulled tls, time, null and cloudinit transitively from terraform-aws-modules/{vpc,eks} under floor-only constraints — `tls >= 3.0.0` was resolving 4.3.0 and would have crossed into 5.x unannounced. A floor without a ceiling is not a pin. All four are now declared at the root with ceilings; tls and null match the constraints already written in shared/versions.tf. No resolved version changed. * bootstrap/aws was the only root validating with a warning: aws_s3_bucket_lifecycle_configuration with neither `filter` nor `prefix`, which the provider says "will be an error in a future version". `filter {}` is the documented way to say "all objects", which is what the rule already meant. Warning gone. Also fixed: environments/gcp-{ephemeral,persistent} carry their own .gitignore that ignores .terraform.lock.hcl. A nested .gitignore overrides its parent, so #1108's un-ignore does not reach them and both locks would have been dropped silently by `git add`. Those two lines are removed; state stays ignored. Locks use the same platform set as #1108 (linux_amd64, darwin_arm64, darwin_amd64) — verified 3 h1 hashes per provider across all 21 entries — and are stable: a second clean init leaves all eight byte-identical. environments/ibm-iks is deliberately untouched. It is the one root that cannot init, #1103 fixes the cause and #1107 the validate errors, and both are open against it. Its lock belongs to whichever of those lands; the generated content is in the PR body. Evidence is local only. GitHub Actions is at a spend cap with hundreds of jobs queued and none running, so no CI run backs any of this.
Stacked on #1103 (
fix/tofu-fmt-validate-blackout) — that PR's provider-source declaration is what makestofu initwork here at all. Retarget tomainonce #1103 merges.infra/tofu/environments/ibm-ikshas never passedtofu validate. It failed attofu initbecausemodules/ibm-trusted-profileusedibm_*resources without a declared provider source, so OpenTofu inferred the nonexistenthashicorp/ibm. #1103 fixed that, which unblocked init and exposed two errors that had been unreachable the entire time.Before / after
Both with OpenTofu 1.8.3 (the
TOFU_VERSIONintofu-plan.yml),tofu init -backend=false && tofu validate:Before — 2 errors:
After — from a clean
.terraform:tofu fmt -check -recursive infra/tofu/also clean.Error 1 — the tag convention, and where it applies
IBM user tags must match
^[A-Za-z0-9:_ .-]+$. No/. Sorepo:SocioProphet/prophet-platformis rejected.Convention chosen: split the owner out into its own tag.
This is not a new invention — it is what the rest of the estate already does.
environments/aws-eks,environments/azure-aksandshared/labels.tfall carry a separate"org" = "socioprophet"key and never embed the owner in the repo name. ibm-iks was the only place in the tree that tried to putowner/namein a single tag. Values are lowercase because IBM lowercases user tags on write.Where else it applies: nowhere. I checked before changing it, as asked.
local.prophet_tagsis not shared — every root declares its own, and the shapes are not even compatible:prophet_tagstypeenvironments/ibm-iks"key:value"strings (IBM tag format)environments/aws-eksenvironments/azure-aksSo this change is scoped to ibm-iks. Within ibm-iks it is applied consistently by construction: all four tagged resources (
ibm_is_vpc,ibm_is_subnet,ibm_container_vpc_cluster,ibm_cr_namespace) referencelocal.prophet_tags. Only the VPC errored because onlyibm_is_vpcattaches aValidateFunctotags, but all four were carrying the malformed value.Error 2 — this is a redesign, not a fix
Profile-CRis not the answer. There is no answer. IBM Cloud cannot federate GitHub Actions OIDC at all, so the module's premise — "IBM's equivalent of AWS IRSA / Azure WIF / GCP WIF" — was never achievable.The claim rule enum is closed at two values, hard-coded in the provider (
resource_ibm_iam_trusted_profile_claim_rule.go):Profile-CRis compute-resource-only. From the provider docs:Every one of those six is a workload running inside IBM Cloud — VPC virtual server, Power VS, bare metal, IKS service account, OpenShift service account, Code Engine. The CR token is not an arbitrary JWT; it is platform-injected, read from the instance metadata service or a projected service account file (
/var/run/secrets/tokens/sa-token,/var/run/secrets/codeengine.cloud.ibm.com/compute-resource-token/token). A GitHub-hosted runner cannot mint one.Profile-SAMLis for human SSO. Same docs:realm_nameis "The realm name of the Idp this claim rule applies to. This field is required only if the type is specified as'Profile-SAML'", and the type carries anexpiration("session expiration in seconds"). It maps a federated user arriving via browser SSO from a registered SAML realm onto a profile. GitHub Actions issues OIDC JWTs, not SAML assertions.And there is no IdP route either.
resource_ibm_iam_idp.goallows exactlysaml,appid,ldap— no OIDC type, and noissuer/jwks/client_idfields anywhere in the IAM Identity surface to point athttps://token.actions.githubusercontent.com.Corroborating: IBM's own
IBM/actions-ibmcloud-cliaction accepts exactly one auth input —api_key. No OIDC, no trusted profile, noid-tokenpermission. And the upstreamterraform-ibm-modules/terraform-ibm-trusted-profilemakes no IRSA/WIF claim; its example usesProfile-CRwithcr_type = "VSI". The IRSA-equivalence claim was added locally.IBM's own docs enumerate how a trusted profile can be assumed — federated (SAML) users, compute resources, service IDs, cloud service instances (create-trusted-profile, trusted-profile-iam-token). External OIDC providers are not on the list.
What this PR does instead
Removes the invalid claim rule rather than swapping it for one that passes the validator and federates nothing — that is precisely the "control that passes and does nothing" defect class. Consequence, stated plainly rather than hidden: the trusted profile is created but is NOT assumable by CI, and its policies grant nothing to anyone. That is now documented in four places — module header, the calling comment in
ibm-iks/main.tf, the outputdescription, and the env README — plus the IBM console description string itself readsNOT FEDERATED — no claim rule.Recommended shape (needs a decision, not mine to make unilaterally)
cr_type = "CE") or an IKS workload (IKS_SA), reduce GitHub Actions to a trigger. Only genuinely keyless option. Worth noting AWS IRSA is pod identity, which IBM does support viaProfile-CR; it is Azure/GCP-style CI federation that IBM lacks — the module conflated the two.1 is the recommendation; 2 and 3 are interim.
🔴 Also broken, flagged not fixed (out of lane)
.github/workflows/infra-drift-detect.yml(~line 149) posts a GitHub OIDC token togrant_type=urn:ibm:params:oauth:grant-type:cr-token. That grant expects a compute resource token and will reject it, for exactly the reason above. The two halves of this design never agreed with each other — the module declaredProfile-OIDCwhile the workflow assumed CR. Neither has ever run. That workflow is outsideenvironments/ibm-iks/+modules/ibm-trusted-profile/, so it is untouched here and needs reworking alongside whichever option is chosen.CI coverage — this rotted because nothing watched it
tofu-plan.ymlrunsfmt -check -recursiveacross all ofinfra/tofu/, butvalidateon only four roots, all underenvs/:local,gcp-landing,shared,prod. The entireenvironments/tree and all ofbootstrap/are unvalidated. That is why a root that had never validated survived indefinitely.tofu-plan.yml— PR #1097 owns it. Recommendation for whoever lands #1097:Add these nine roots to the
fmt-and-validatejob (tofu init -backend=false && tofu validateeach):All nine are safe to add now. I validated each locally on 1.8.3: eight already pass clean, and the ninth (
ibm-iks) passes as of this PR. So this widening goes green immediately — there is no cleanup backlog gating it.Two notes for whoever implements it:
bootstrap/awsemits a non-fatal provider deprecation warning (aws_s3_bucket_lifecycle_configurationmissingrule[0].filter) — pre-existing, warns only, does not failvalidate. Worth a separate ticket before it becomes an error in a future provider.Given the tree is 13 roots and CI watches 4, a cheap follow-up guard would be a check that every directory containing a
versions.tf/backendblock appears in the validate matrix — otherwise root #14 lands unwatched the same way this one did.Test plan
tofu init -backend=false && tofu validateinenvironments/ibm-ikson 1.8.3 — clean.terraform, passestofu fmt -check -recursive infra/tofu/— cleantofu apply— not attempted; no IBM credentials, and per the above CI cannot obtain any.