iac(tofu): bring environments/* and bootstrap/* to the same offline-validated bar as envs/* - #1115
Open
mdheller wants to merge 1 commit into
Open
Conversation
…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.
Contributor
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.
Aligns environments/* and bootstrap/* OpenTofu roots with the repo’s “offline-validated IaC” standard by ensuring consistent version/provider pinning and making .terraform.lock.hcl trackable where it was previously ignored.
Changes:
- Stop ignoring
.terraform.lock.hclin nestedenvironments/gcp-*.gitignorefiles so committed locks aren’t silently dropped. - Add
versions.tftobootstrap/{aws,azure,gcp}withrequired_versionand pinnedrequired_providers, and remove the inlineterraform {}blocks frommain.tf. - Declare previously-transitive providers explicitly in
environments/aws-eks/versions.tfto ensure ceilings exist at the root.
Reviewed changes
Copilot reviewed 9 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| infra/tofu/environments/gcp-persistent/.gitignore | Removes nested ignore of .terraform.lock.hcl and documents why lockfiles must be tracked here. |
| infra/tofu/environments/gcp-ephemeral/.gitignore | Removes nested ignore of .terraform.lock.hcl and documents why lockfiles must be tracked here. |
| infra/tofu/environments/aws-eks/versions.tf | Adds explicit root-level provider constraints for transitively used providers to avoid floor-only resolution. |
| infra/tofu/bootstrap/gcp/versions.tf | Introduces consistent required_version/provider pinning for the bootstrap GCP root. |
| infra/tofu/bootstrap/gcp/main.tf | Removes inline terraform {} in favor of versions.tf. |
| infra/tofu/bootstrap/azure/versions.tf | Introduces consistent required_version/provider pinning for the bootstrap Azure root. |
| infra/tofu/bootstrap/azure/main.tf | Removes inline terraform {} in favor of versions.tf. |
| infra/tofu/bootstrap/aws/versions.tf | Introduces consistent required_version/provider pinning for the bootstrap AWS root. |
| infra/tofu/bootstrap/aws/main.tf | Removes inline terraform {} in favor of versions.tf and adds an empty lifecycle filter {} to eliminate a validation warning. |
Files not reviewed (8)
- infra/tofu/bootstrap/aws/.terraform.lock.hcl: Generated file
- infra/tofu/bootstrap/azure/.terraform.lock.hcl: Generated file
- infra/tofu/bootstrap/gcp/.terraform.lock.hcl: Generated file
- infra/tofu/environments/aws-eks/.terraform.lock.hcl: Generated file
- infra/tofu/environments/azure-aks/.terraform.lock.hcl: Generated file
- infra/tofu/environments/gcp-ephemeral/.terraform.lock.hcl: Generated file
- infra/tofu/environments/gcp-gke/.terraform.lock.hcl: Generated file
- infra/tofu/environments/gcp-persistent/.terraform.lock.hcl: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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 #1108 (
fix/tofu-provider-pinning). Retarget tomainonce that lands.The decision this implements
So the bar is offline-validated IaC, uniformly:
required_versionpresent,required_providerspinned, lock committed,tofu init -backend=false && tofu validateclean. Liveplan/apply, credentials, and federation are explicitly out of scope, and no credential path needs to work.#1108 met that bar for the four
envs/*roots CI validates and deliberately left the other nine unlocked. This is the remaining gap: sixenvironments/*and threebootstrap/*.Parity matrix
All measurement with OpenTofu 1.8.3 — the
TOFU_VERSIONintofu-plan.yml— from a clean.terraform, no plugin cache,tofu init -backend=false && tofu validate.Before (
origin/main@55a3d086):required_versionrequired_providersenvs/gcp-landingenvs/sharedenvs/prodenvironments/aws-eksenvironments/azure-aksenvironments/gcp-ephemeralenvironments/gcp-gkeenvironments/gcp-persistentenvironments/ibm-iksbootstrap/awsbootstrap/azurebootstrap/gcpaws-eksis "partial" because three of its seven providers were undeclared — see the transitive section.After (this PR, on top of #1108):
required_versionrequired_providersenvs/gcp-landingenvs/sharedenvs/prodenvironments/aws-eksenvironments/azure-aksenvironments/gcp-ephemeralenvironments/gcp-gkeenvironments/gcp-persistentenvironments/ibm-iksbootstrap/awsbootstrap/azurebootstrap/gcpThe two remaining FAILs are inherited, not caused:
gcp-landingis #1103's parse error andibm-iksis #1103 + #1107. Neither is touched here.End state, all four lanes merged locally (#1103 + #1107 + #1108 + this, merged clean, measured, then discarded):
envs/localenvironments/gcp-ephemeralenvs/gcp-landingenvironments/gcp-gkeenvs/sharedenvironments/gcp-persistentenvs/prodenvironments/ibm-iksshared/bootstrap/awsenvironments/aws-eksbootstrap/azureenvironments/azure-aksbootstrap/gcp14 of 14 roots clean, zero warnings, and
tofu fmt -check -recursive infra/tofu/→ exit 0. That is the parity Michael asked for, and it needs all four PRs.The four asymmetries
1. Three google majors — decided: one, at 6.x
bootstrap/gcpwas on~> 5.0,environments/gcp-*on~> 6.0,envs/*unconstrained and resolving 7.42.0 until #1108 held them at 6.x. Nothing recorded whether any of that was intended. Two of the three were clearly accidents:envs/*had no constraint at all, andshared/versions.tfdocumented a~> 6.0pin it could not apply to anything.bootstrap/gcpmoves to~> 6.0. 6.x is the majority and the version #1108 settled on, so bootstrap follows rather than the estate splitting the difference.No warnings either side. The root creates two long-stable resources (
google_storage_bucket,google_storage_bucket_iam_member).2. Two azurerm majors — same call, same evidence
Not on the brief's list, but it is the identical defect:
bootstrap/azureon~> 3.0whileenvironments/azure-akswas already on~> 4.0. Moves to~> 4.0.azurerm 4.0 made provider
subscription_idmandatory; this root already sets it fromvar.subscription_id, which is why the move is clean.Surfaced by the bump, deliberately not fixed:
azurerm_storage_containersetsstorage_account_name, which 4.x deprecates and 5.x is expected to remove. Read off the real schema rather than from memory —tofu providers schema -json, azurerm 4.81.0:Swapping the attribute changes which identifier addresses an existing container and can produce a diff against state that already exists — an apply-time question this PR has no way to answer offline. The deprecation exists in 4.x whether or not this root points at it; the bump surfaces it, it does not create it. Filed, not hidden.
3.
bootstrap/*had norequired_versionIt was the only tier without one — all six
environments/*had it, and #1108 gave it to all fourenvs/*. These three were the only roots an older OpenTofu would happily parse.The
terraform {}block moves out ofmain.tfinto aversions.tfin each bootstrap root, carryingrequired_version = ">= 1.8.0". That is not cosmetic: after this, all 14 roots declare versions in the same filename, which is what makes the matrix above checkable by looking rather than by grepping.providerblocks stay inmain.tf, matchingenvs/*post-#1108.4.
environments/aws-ekstransitive constraints — the sharpest oneterraform-aws-modules/{vpc,eks}require four providers the root never declared, all with a floor and no ceiling. Straight frominit:tlswas resolving 4.3.0 against a floor of 3.0.0 — already a full major above its stated minimum, and it would have crossed into 5.x unannounced on the day 5.0 shipped. A floor without a ceiling is not a pin, and no amount of local pinning ofaws/helm/kubernetesconstrained these.All four are now declared at the root with ceilings.
tls = "~> 4.0"andnull = "~> 3.2"are not new inventions — they are the constraints already written inshared/versions.tf, finally applied somewhere they load. The lock now records the intersection:No resolved version changed — the ceiling was added above where resolution already sat.
aws-eksgoes from 3 declared / 7 resolved to 7 / 7.bootstrap/awswas the only root validating with a warningFixed with
filter {}— the provider's documented way to say "every object in the bucket", which is what the rule already meant, so this records the existing scope rather than narrowing it.bootstrap/awsnow returns a bareSuccess! The configuration is valid.A hole #1108's un-ignore does not reach
environments/gcp-ephemeral/andenvironments/gcp-persistent/each carry their own.gitignorethat also lists.terraform.lock.hcl. A nested.gitignoreoverrides its parent, so #1108 removing the line frominfra/tofu/.gitignoredoes not un-ignore these two — both locks would have been dropped silently bygit add, and the matrix above would have quietly shown ten locks instead of twelve.Both lines removed.
*.tfstatestays ignored in both: the lock pins the provider build, not the state.Locks
Same platform set as #1108 —
-platform=linux_amd64 -platform=darwin_arm64 -platform=darwin_amd64(CI plus both Mac architectures), generated with 1.8.3.Verified, not assumed: all 21 provider entries across the eight new locks carry exactly 3
h1:hashes. And they are stable — a second cleaninitfrom a deleted.terraformleaves all eight byte-identical (sha256 compared before/after):environments/aws-eksenvironments/gcp-persistentenvironments/azure-aksbootstrap/awsenvironments/gcp-ephemeralbootstrap/azureenvironments/gcp-gkebootstrap/gcpenvironments/aws-eksandenvironments/azure-aksneedtofu init -backend=falsebeforeproviders lock—providers lockdoes not install modules, and both roots take providers from module blocks. Worth knowing before anyone regenerates them.environments/ibm-iks— untouched, and the lock is here for #1107Not modified. #1103 fixes its
init, #1107 itsvalidateerrors, and both are open against exactly these paths. Nothing in this PR touchesenvironments/ibm-iks/ormodules/ibm-trusted-profile/.Its lock is the one piece of the matrix I cannot land, because it cannot be generated on this branch —
initfails until #1103's provider-source declaration exists. I generated it on a throwaway merge of all four lanes and verified it:3 providers, 9
h1:hashes (3 x 3 platforms), 2840 bytes, sha256f9ef4b7e8c8aa7b675cba1870da60d6d54eccce2672f0af9ea98bd7daaa11dbc.Recommendation: whichever of #1103/#1107 lands last adds it with
tofu providers lock -platform=linux_amd64 -platform=darwin_arm64 -platform=darwin_amd64.ibm-ikshas no nested.gitignore, so #1108's un-ignore is sufficient there.Recommendation: the IBM auth pieces
Keep
modules/ibm-trusted-profileandenvironments/ibm-iks. Inert, and labelled. Do not remove.The brief is right that they exist only to enable live connections, and under this decision that purpose is gone. But removal is still the wrong move:
environments/ibm-iksis a validating root only because that module exists. Removing it to achieve parity would remove IBM from the set being brought to parity.Profile-OIDCclaim rule rather than swapping in one that passes the validator and federates nothing, keeps the profile, and labels the non-federation in four places plus the IBM console description string. That is "inert and clearly labelled", already written. Deleting the module would throw that work away and collide with an open PR.One amendment I would suggest to #1107's owner, not a blocker: its README and comments frame the non-federation as an IBM limitation ("IBM Cloud cannot federate GitHub Actions OIDC"). Both are true, but the operative reason is now the decision — we are not running live connections against any of the four clouds. Saying "out of scope by decision, and also not possible on IBM" is the honest ordering, and it stops the note reading as a blocked item someone should go fix.
Recommendation: the drift workflow's cloud-auth jobs
.github/workflows/infra-drift-detect.ymlis #1090's lane (fix/drift-exit1-is-failure, open). Not touched here. This is a recommendation for its owner.The important point: this is not an IBM problem.
infra-drift-detect.ymlhas four jobs —gcp-landing,aws-eks,azure-aks,ibm-iks— and every one runstofu planagainst a live cloud. Under a no-live-connections posture the whole workflow has no purpose, not just its IBM step. Removing only thecr-tokenblock would be inconsistent, and worse, it would imply the other three are fine.The IBM step is independently broken regardless of posture — it posts a GitHub OIDC JWT to
grant_type=urn:ibm:params:oauth:grant-type:cr-token, a grant that expects a platform-injected compute-resource token (#1107 documents this in detail) — but fixing it is now moot.Recommend: make the whole workflow inert rather than delete the file.
schedule:trigger; leaveworkflow_dispatch:so it can be run deliberately if live access is ever wanted.tofu outputfeeds which CI variable (GCP_WIF_PROVIDER,AWS_TOFU_ROLE_ARN,AZURE_CLIENT_ID,IBM_TRUSTED_PROFILE_ID, ...). Deleting it loses reference material that is genuinely useful the day live connections come back into scope, and costs nothing to keep once the schedule is gone.Nothing is lost by doing this: per #1097's audit the workflow has 27 runs, 26
startup_failure, and has never once succeeded. It has never detected drift. Making that explicit is strictly better than leaving a scheduled job that looks like a control and is not one — the same defect class #1107 declined to create.Recommendation for
tofu-plan.yml(#1097's lane — not edited)#1107 named nine roots to add and verified eight pass locally. With this PR the count is twelve, and all twelve pass.
fmt -check -recursivealready walks all ofinfra/tofu/, butvalidatecovers 4 of 14 roots, which is precisely why a root that had never validated survived indefinitely.Add these eight to
fmt-and-validate(tofu init -backend=false && tofu validateeach) — all verified clean here:Plus
infra/tofu/environments/ibm-iksonce #1103 + #1107 land — verified clean on the combined merge above — andinfra/tofu/shared, which validates clean and costs nothing.A matrix over root directories beats twelve hand-written steps, and is the only version of this that stops a new root landing unvalidated. That is the failure mode that actually happened here; enumerating roots by hand just resets the clock.
Lane hygiene
git merge-tree --write-treeagainst all four open PRs, from this branch head:fix/tofu-fmt-validate-blackoutfix/ibm-iks-tofu-validatefix/tofu-provider-pinningfix/tofu-plan-setup-opentofu-blockedZero conflicts, zero overlapping files with any lane. The four lanes also merge cleanly with each other and with this branch — that is how the end-state matrix above was measured.
Every change is inside
infra/tofu/.GitHub Actions is at a spend cap — hundreds of jobs queued, zero running. No workflow ran for this branch, and none will until the cap clears. Everything above is a local OpenTofu 1.8.3 binary with
.terraformdeleted and no plugin cache, replicating the CI command sequence. It is not a CI result and should not be read as one.Specifically not covered, and deliberately so under the no-live-connections decision:
tofu plan/tofu applyanywhere. Schema compatibility across the two major bumps is confirmed; apply-time differences (changed defaults, newly-required fields) are not.linux_amd64. The hashes are in the lock and were fetched from the registry for that platform, but onlydarwin_arm64was actually initialised from them here.