fix: make terraform ConvertState fully deterministic (#23459)

All map iterations in ConvertState now use sorted helpers instead of
ranging over Go maps directly. Previously only coder_env and
coder_script were sorted (via sortedResourcesByType). This extends
the pattern to coder_agent, coder_devcontainer, coder_agent_instance,
coder_app, coder_metadata, coder_external_auth, and the main
resource output list.

Also fixes generate.sh writing version.txt to the wrong directory
(resources/ instead of testdata/), which caused the Makefile version
check to silently desync and trigger unnecessary regeneration.

Adds TestConvertStateDeterministic that calls ConvertState 10 times
per fixture and asserts byte-identical JSON output without any
post-hoc sorting.
This commit is contained in:
Mathias Fredriksson
2026-03-24 11:02:45 +00:00
committed by GitHub
parent 56e80b0a27
commit b23aed034f
5 changed files with 583 additions and 476 deletions
+1 -1
View File
@@ -1255,7 +1255,7 @@ coderd/notifications/.gen-golden: $(wildcard coderd/notifications/testdata/*/*.g
TZ=UTC go test ./coderd/notifications -run="Test.*Golden$$" -update
touch "$@"
provisioner/terraform/testdata/.gen-golden: $(wildcard provisioner/terraform/testdata/*/*.golden) $(GO_SRC_FILES) $(wildcard provisioner/terraform/*_test.go)
provisioner/terraform/testdata/.gen-golden: $(wildcard provisioner/terraform/testdata/*/*.golden) $(wildcard provisioner/terraform/testdata/*/*/*.golden) $(GO_SRC_FILES) $(wildcard provisioner/terraform/*_test.go)
TZ=UTC go test ./provisioner/terraform -run="Test.*Golden$$" -update
touch "$@"
@@ -127,3 +127,101 @@ func TestConvertStateGolden(t *testing.T) {
}
}
}
// TestConvertStateDeterministic verifies that ConvertState produces
// identical output across multiple runs. This catches non-deterministic
// map iteration in the implementation. Unlike TestConvertStateGolden,
// this test does NOT sort the output — it relies on ConvertState itself
// being deterministic.
func TestConvertStateDeterministic(t *testing.T) {
t.Parallel()
testResourceDirectories := filepath.Join("testdata", "resources")
entries, err := os.ReadDir(testResourceDirectories)
require.NoError(t, err)
for _, testDirectory := range entries {
if !testDirectory.IsDir() {
continue
}
testFiles, err := os.ReadDir(filepath.Join(testResourceDirectories, testDirectory.Name()))
require.NoError(t, err)
for _, step := range []string{"plan", "state"} {
srcIdx := slices.IndexFunc(testFiles, func(entry os.DirEntry) bool {
return strings.HasSuffix(entry.Name(), fmt.Sprintf(".tf%s.json", step))
})
dotIdx := slices.IndexFunc(testFiles, func(entry os.DirEntry) bool {
return strings.HasSuffix(entry.Name(), fmt.Sprintf(".tf%s.dot", step))
})
if srcIdx == -1 || dotIdx == -1 {
continue
}
t.Run(step+"_"+testDirectory.Name(), func(t *testing.T) {
t.Parallel()
testDirectoryPath := filepath.Join(testResourceDirectories, testDirectory.Name())
planFile := filepath.Join(testDirectoryPath, testFiles[srcIdx].Name())
dotFile := filepath.Join(testDirectoryPath, testFiles[dotIdx].Name())
ctx := testutil.Context(t, testutil.WaitMedium)
logger := slogtest.Make(t, nil)
tfStepRaw, err := os.ReadFile(planFile)
require.NoError(t, err)
var modules []*tfjson.StateModule
switch step {
case "plan":
var tfPlan tfjson.Plan
err = json.Unmarshal(tfStepRaw, &tfPlan)
require.NoError(t, err)
modules = []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}
if tfPlan.PriorState != nil {
modules = append(modules, tfPlan.PriorState.Values.RootModule)
}
case "state":
var tfState tfjson.State
err = json.Unmarshal(tfStepRaw, &tfState)
require.NoError(t, err)
modules = []*tfjson.StateModule{tfState.Values.RootModule}
default:
t.Fatalf("unknown step: %s", step)
}
dotFileRaw, err := os.ReadFile(dotFile)
require.NoError(t, err)
// Run ConvertState 10 times and verify all runs
// produce byte-identical JSON without any sorting.
// We apply deterministicAppIDs because plan files
// lack provider-assigned IDs, causing ConvertState
// to generate random UUIDs as a fallback.
//
// Note: json.Marshal sorts map keys, so this test
// cannot catch non-determinism in map-valued fields
// like Agent.Env. Those are populated from static
// testdata today, so this is not a practical gap.
const runs = 10
outputs := make([][]byte, runs)
for i := range runs {
state, err := terraform.ConvertState(ctx, modules, string(dotFileRaw), logger)
if err != nil {
// Error strings are deterministic.
outputs[i] = []byte(err.Error())
continue
}
deterministicAppIDs(state.Resources)
outputs[i], err = json.Marshal(state)
require.NoError(t, err, "run %d: marshal state", i)
}
for i := 1; i < runs; i++ {
require.Equal(t, string(outputs[0]), string(outputs[i]),
"ConvertState produced different output on run %d vs run 0", i)
}
})
}
}
}
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -138,4 +138,4 @@ if [[ $err -ne 0 ]]; then
exit 1
fi
terraform version -json | jq -r '.terraform_version' >version.txt
terraform version -json | jq -r '.terraform_version' >../version.txt
-1
View File
@@ -1 +0,0 @@
1.14.5