Apply top-level configOverride keys in the Helm chart - #274
Draft
liam-lowe wants to merge 1 commit into
Draft
Conversation
liam-lowe
marked this pull request as draft
September 2, 2026 22:43
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.
s2s-proxy.mergedConfigmergedconfigOverride.clusterConnectionsinto the defaults and wrote back only that key. Every other top-level key an operator set was silently discarded.configOverride.metrics,configOverride.loggingand anything else rendered as the default, with no error and no warning.This merges the remaining top-level keys over the defaults.
deepCopyguards the source. Sprig'smergeOverwritemutates its first argument. The loop above it already writes defaults into.Values.configOverride.clusterConnectionsin place.This changes rendered output for existing installs
Any install already carrying a top-level
configOverridekey sees it take effect for the first time on upgrade. Two consequences worth stating plainly:config.yaml.config.LoadConfigdecodes withKnownFields(true). An unrecognised key is fatal. The pod will not start. A stale or speculative override that was previously inert becomes a startup failure.configOverride.metrics.prometheus.listenAddressmoves both the listen address and therpc-metricscontainer 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.metrics.prometheus.listenAddressoverride reaches the rendered config. It also asserts theclusterConnectionsmerge is unaffected. Removing the merge line fails that case and only that case.make helm-example: no drift.