fix(terraform): converge when a random_* value reaches a module, and evaluate every submodule on each pass - #84
Open
PushTheLimit wants to merge 2 commits into
Conversation
…ping at the first change evaluateSubmodules OR-ed the result as `changed = changed || e.evaluateSubmodule(ctx, sm)`, so once one submodule reported a change, every submodule after it in that pass was skipped. A submodule whose inputs never settle, such as one fed a random_* preset (regenerated with uuid.New() on each pass), keeps `changed` true on every pass, so the later submodules are never evaluated at all and come back with no blocks. coder/preview then reports each of them as "Module not loaded". Evaluate the submodule first and OR afterwards, so every submodule is evaluated on every pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…converges createPresetValues filled random_password, random_string, random_id, random_bytes, random_uuid and random_integer with a fresh uuid.New() or rand.Int64() on every call, and it runs on every evaluation pass. Any expression outside a resource that reads one of those values (a local, a module argument, an output) therefore changes on every pass, the context never matches the previous one, and evaluateSteps always runs all maxContextIterations passes. Derive each placeholder from the block ID and attribute name instead, so it is stable across passes within one parse and the fixed-point loop exits as soon as everything else has settled. Distinct blocks and attributes still get distinct values. On a 60-resource Coder template that passes a random_password into a module, this takes a parameter render from 3.2s to 0.29s.
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
2 participants
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.
Fixes coder/coder#30056.
Two evaluator fixes for a template that passes a
random_*value into a module. Today that makes coder/preview skip every module declared after it, so their parameters disappear from the create form, and it makes every render run all 32 evaluation passes.Evaluate every submodule on each pass.
evaluateSubmodulesdidchanged = changed || e.evaluateSubmodule(ctx, sm), so once one submodule reported a change,||short-circuited past every later submodule for that pass. A submodule whose inputs never settle therefore starved the rest of them in every pass. The call now runs before the OR.Stable
random_*placeholders.createPresetValuesreturned a newuuid.New()/rand.Int64()value on every call, so any local, module argument or output that read one changed on every pass and the fixed-point loop never exited early. Each placeholder is nowuuid.NewSHA1of the block ID and attribute name, which is the same on every pass within a parse and different for each block and attribute.Either fix alone makes the reproduction in #30056 render correctly. Both are needed for the general case: the first keeps later modules from being starved by any input that doesn't settle, and the second lets the random case converge instead of running every pass.
Testing
TestSubmoduleWithUnstableInputDoesNotStarveLaterSubmodules: a later module loads even though an earlier one is fed a random value.TestRandomPresetLetsEvaluationConverge: evaluation of a local readingrandom_password.resultfinishes in fewer thanmaxContextIterationssteps. It fails without the preset change.pkg/iac/terraform/...andpkg/iac/scanners/terraform/...pass.c0dfdeecbd89:params=[]plus a "Module not loaded" diagnostic before,params=[region]with no diagnostics after.Without the two changes (the new tests only):
With them:
The #30056 reproduction rendered through
preview.Preview(coder/previewc0dfdeecbd89), before:After:
We have run both changes in production for a week. On our largest template a render went from 3.2s to 0.28s, and the 11 modules it reported as "Module not loaded" load again.
The coder/preview and coder/coder pin bumps can follow the same path as #74.
AI disclosure
Written with AI assistance (Claude Code). I reviewed the change, ran the tests and the reproduction above myself, and have run it in production.