Skip to content

fix: pin every moving version default - #537

Closed
gdrojas wants to merge 1 commit into
6.xfrom
fix/pin-moving-version-defaults
Closed

fix: pin every moving version default#537
gdrojas wants to merge 1 commit into
6.xfrom
fix/pin-moving-version-defaults

Conversation

@gdrojas

@gdrojas gdrojas commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

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.

Variable Before Now
traffic_manager_version (eks) "latest" "1.8.0"
agent_repos_scope ...scopes.git#main ...scopes.git#v1.15.1
cert_manager_version declared and never referenced wired to the helm_release, default v1.21.1
prometheus_version did not exist; the helm_release had no version new, default 29.27.0

cert_manager_version is the misleading one: it existed with a default, and grep returned exactly one occurrence — its own declaration. The helm_release had no version argument, 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

  • The scopes ref steps back. main is at 446e71cb and v1.15.1 at 8ce6e896: 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.
  • cert-manager and prometheus go from "whatever the chart repository serves" to a fixed number. If your install was pinned by some other means, check before applying.

Also

agent_traffic_manager_tag is new, with a default of 1.8.0. Pinning the traffic manager required passing the whole image string through extra_envs; the registry now lives in the module and only the tag is exposed. extra_envs still wins the merge, so the previous approach 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 nullplatform/base.

Verification

132 tests, 0 failures. eks had an assertion expecting the "latest" default; it was corrected and coverage was added for the default now being pinned.

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
gdrojas force-pushed the fix/pin-moving-version-defaults branch from b237dd6 to 33128c1 Compare August 27, 2026 15:45
@gdrojas

gdrojas commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #540. The non-breaking approach here pinned each moving default to its current value; #540 removes the defaults entirely so every version has to be declared, which is what was asked for.

@gdrojas gdrojas closed this Aug 27, 2026
@sebastiancorrea81
sebastiancorrea81 deleted the fix/pin-moving-version-defaults branch September 3, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant