Skip to content

Apply top-level configOverride keys in the Helm chart - #274

Draft
liam-lowe wants to merge 1 commit into
mainfrom
liam-lowe/chart-toplevel-override
Draft

Apply top-level configOverride keys in the Helm chart#274
liam-lowe wants to merge 1 commit into
mainfrom
liam-lowe/chart-toplevel-override

Conversation

@liam-lowe

Copy link
Copy Markdown
Contributor

s2s-proxy.mergedConfig merged configOverride.clusterConnections into the defaults and wrote back only that key. Every other top-level key an operator set was silently discarded. configOverride.metrics, configOverride.logging and anything else rendered as the default, with no error and no warning.

This merges the remaining top-level keys over the defaults.

deepCopy guards the source. Sprig's mergeOverwrite mutates its first argument. The loop above it already writes defaults into .Values.configOverride.clusterConnections in place.

This changes rendered output for existing installs

Any install already carrying a top-level configOverride key sees it take effect for the first time on upgrade. Two consequences worth stating plainly:

  • A key the binary does not recognise now reaches config.yaml. config.LoadConfig decodes with KnownFields(true). An unrecognised key is fatal. The pod will not start. A stale or speculative override that was previously inert becomes a startup failure.
  • A real override now applies. configOverride.metrics.prometheus.listenAddress moves both the listen address and the rpc-metrics container port. A scrape target can move.

Both are the intended behaviour of configOverride. Neither was reachable before.

Testing

  • helm unittest s2s-proxy/: 5 tests pass.
  • A new configmap case asserts a top-level metrics.prometheus.listenAddress override reaches the rendered config. It also asserts the clusterConnections merge is unaffected. Removing the merge line fails that case and only that case.
  • make helm-example: no drift.

@liam-lowe
liam-lowe requested a review from a team as a code owner September 2, 2026 22:41
@liam-lowe
liam-lowe marked this pull request as draft September 2, 2026 22:43
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