fix: pin every moving version default - #537
Closed
gdrojas wants to merge 1 commit into
Closed
Conversation
Four defaults in this line resolved at deploy time rather than at apply time, so a pod restart could pull a different build with no change on our side: - eks traffic_manager_version was "latest" - agent_repos_scope pointed at scopes.git#main - cert_manager_version was declared but never wired to its helm_release, so installs tracked whatever charts.jetstack.io served while the README showed a number. grep found one occurrence of the variable: its own declaration. - prometheus had no version argument and no variable for one, so it could not be pinned without editing the module. Each default is now the version it currently resolved to, so behaviour is unchanged today and stops drifting tomorrow. Nothing is required and nothing is removed: every caller that already passes a value is unaffected. The scopes ref is the exception worth reading twice. main has moved past v1.15.1, so pinning the tag steps back to the last tagged release rather than holding today's branch tip. agent_traffic_manager_tag is new, with a default. Pinning the traffic manager meant passing the whole image string through extra_envs; only the tag is exposed now, and extra_envs still wins the merge so the old way keeps working. The agent module had no outputs and no tests, so its rendered values could not be asserted on. It now exports rendered_values and agent_repos, matching what nullplatform/base already does.
gdrojas
force-pushed
the
fix/pin-moving-version-defaults
branch
from
August 27, 2026 15:45
b237dd6 to
33128c1
Compare
Collaborator
Author
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.
What was moving
Four defaults resolved at deploy time rather than at apply time. A pod restart could pull a different build with no change on our side and no diff to review.
traffic_manager_version(eks)"latest""1.8.0"agent_repos_scope...scopes.git#main...scopes.git#v1.15.1cert_manager_versionhelm_release, defaultv1.21.1prometheus_versionhelm_releasehad noversion29.27.0cert_manager_versionis the misleading one: it existed with a default, andgrepreturned exactly one occurrence — its own declaration. Thehelm_releasehad noversionargument, so installs tracked whatever charts.jetstack.io served while the README showed a number.Nothing breaks
No variable loses its default and none becomes required. Any caller already passing a value is unaffected. Each new default is the version that resolved today, so behaviour does not change on merge — it just stops drifting.
Two things that do change behaviour
mainis at446e71cbandv1.15.1at8ce6e896: the branch has moved past the tag. Pinning the tag is deliberate — it is the ref named in the pinning request — but it is not the tree the branch tip pointed at.Also
agent_traffic_manager_tagis new, with a default of1.8.0. Pinning the traffic manager required passing the whole image string throughextra_envs; the registry now lives in the module and only the tag is exposed.extra_envsstill wins the merge, so the previous approach keeps working.The
agentmodule had no outputs and no tests, so its rendered values could not be asserted on. It now exportsrendered_valuesandagent_repos, matchingnullplatform/base.Verification
132 tests, 0 failures.
ekshad an assertion expecting the"latest"default; it was corrected and coverage was added for the default now being pinned.