terraform: iterate primary files in a deterministic order - #2633
Merged
bendrucker merged 2 commits intoAug 26, 2026
Merged
Conversation
Module.PartialContent used to iterate the m.primaries map directly. Because Go randomizes map iteration order, modules with multiple primary .tf files produced a different block order on every tflint invocation, and any rule whose logic depends on block order (e.g. "last matching block wins") produced non-reproducible results. The same bug class was previously reported and fixed for override files in terraform-linters#2124 (see tflint-ruleset-terraform#205 for the original symptom report). That fix introduced overrideFilenames plus a sort.Strings so overrides are iterated in a stable lexicographical order, but it did not touch the general primaries path. Mirror the override treatment for primaries: add primaryFilenames, populate and sort it in LoadConfigDir, and iterate it in PartialContent. Add a regression test that invokes PartialContent many times against a module with several primary files and asserts the returned block order is stable and matches the sorted filename order on every call. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@wata727, I realize I didn't create an issue for this first. Apologies. Happy to do so if it helps you track incoming requests. |
Restore the original one-line comment on the sorts and drop the running commentary from the regression test. Claude-Session: https://claude.ai/code/session_01Qnx14iQmY6PBeA5RiZ3Qxy
bendrucker
approved these changes
Aug 26, 2026
Member
|
Nice one! |
Contributor
Author
|
@bendrucker , my organization prefers to go through proper Github releases for obtaining our external dependencies. Do you know when a (beta) build will be available containing this fix? Thanks for your time. |
Member
|
We don't do betas since it's 0.x, typical feature release cadence is every 1-2 months, bug fixes sooner. @wata727 is the one cutting releases, usually within a couple of days for a non-critical bug. |
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.
Summary
Module.PartialContentiterates them.primariesmap directly. Because Go randomizes map iteration order, modules with multiple primary.tffiles produce a different block order on everytflintinvocation, and any rule whose logic depends on block order (e.g. "last matching block wins") produces non-reproducible results across runs of the same, unchanged config.This mirrors the exact same bug class that was previously reported and fixed for override files — see tflint-ruleset-terraform#205 for the original symptom report (7 tflint runs on identical input, 2 failed), and #2124 for the fix that introduced
overrideFilenames+sort.Stringsso overrides are iterated in a stable lexicographical order. That fix never touched the general primaries path, which this PR closes.Repro
A real Terraform module with several
aws_*resources spread across multiple primary.tffiles and norequired_providersconstraint will non-deterministically report the "missing version constraint" warning against a different resource on eachtflintrun (GetProviderRefsin the terraform ruleset overwritesproviderRefs[providerName]on every match, so which resource "wins" the blame depends on the primary-file iteration order that this PR pins down). I hit this reliably as an intermittently-flaking Bazeltflinttest — same file content, no changes, PASS/FAIL split acrossbazel test --runs_per_test=8.Fix
Mirror the override treatment for primaries exactly:
terraform/module.go: addprimaryFilenames []stringtoModule, initialize it inNewEmptyModule.terraform/parser.go(LoadConfigDir): populateprimaryFilenamesas each primary file is loaded, thensort.Stringsit alongsideoverrideFilenames.terraform/module.go(Module.PartialContent): iteratem.primaryFilenamesand look upm.primaries[filename], matching the existing override loop.Test
New
TestPartialContent_deterministicPrimaryOrder(interraform/module_test.go) sets up a module with four primary files (d.tf,b.tf,a.tf,c.tf), callsPartialContent20 times, and asserts every call returns blocks in the same lexicographical filename order (a,b,c,d). Also assertsLoadConfigDirpopulatesprimaryFilenamesin sorted order.Without the fix, Go's per-
rangemap iteration randomization over a 4-element map makes it effectively certain that at least one of the 20 iterations returns a different order.The existing
TestRebuildfixtures were updated to also populateprimaryFilenames/overrideFilenames(they construct aModuledirectly, bypassingNewEmptyModule/LoadConfigDir).Downstream impact
Any downstream ruleset rule that relies on
GetModuleContent/PartialContentblock order for anything order-sensitive is currently exposed to this same nondeterminism — this fix benefits all rulesets, not just the resource-order-sensitiveterraform_required_providerscase that surfaced it.Test plan
go test ./terraform/...passes locally, including newTestPartialContent_deterministicPrimaryOrdergo vet ./terraform/...clean