fix(helm): preserve empty-list overrides in valueYamlFiles when allowNullValues=false#4269
Open
ctrahey wants to merge 3 commits intopulumi:masterfrom
Open
fix(helm): preserve empty-list overrides in valueYamlFiles when allowNullValues=false#4269ctrahey wants to merge 3 commits intopulumi:masterfrom
ctrahey wants to merge 3 commits intopulumi:masterfrom
Conversation
Two tests that currently fail, covering the same root bug at
different levels:
- Test_MergeMaps/empty_list_overrides_non-empty_list_with_allowNil=false
exercises mergeMaps directly
- TestDecodeRelease/valueYamlFiles_with_empty_list_is_preserved
exercises the full decodeRelease path with a valueYamlFiles asset
Root cause: excludeNulls returns a nil slice (not []any{}) for an
empty input slice, making [] indistinguishable from a missing key.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`var out []any` is a nil slice. When the input slice is empty the loop
body never executes, so nil is returned and the caller cannot
distinguish "empty list" from "key not present". Helm then falls back
to the chart default instead of honouring `items: []` from a
valueYamlFiles override.
Change to `make([]any, 0)` so an empty input slice produces a non-nil
`[]any{}`, which survives the subsequent mergo merge as an explicit
empty-list override.
Fixes the `allowNullValues=false` (default) case where `items: []` in
a valueYamlFiles entry was silently dropped.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
PR is now waiting for a maintainer to run the acceptance tests. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
PR is now waiting for a maintainer to run the acceptance tests. |
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
1 participant
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.
Issue
Chart rendering is different through pulumi-kubernetes than helm when a values file overrides a list to be empty.
Proposed changes
excludeNullsrecurses into slice values usingvar out []any. When the input slice is empty, the loop never runs, sooutremains nil and is returned that way. That nil slice is written back into the filtered values map, and later Helm value coalescing distinguishes nil[]anyfrom a non-nil empty[]any{}: the nil form does not preserve the user’s explicit empty-list override, so the chart default is applied instead. Changing this toout := make([]any, 0)ensures an empty input slice round-trips as a non-nil empty slice and survives the merge as an explicit override.Notes on discovery and repro
I discovered this when I refactored a project from manual helm deployments to using this provider in Pulumi and started seeing issues in my workload, because the chart I was using had some default
podSecurityContext.sysctlsthat I needed to remove in my environment. My local values file set the list to the empty list:This worked great via helm. But using the pulumi-kubernetes provider did not behave the same way and was shipping the chart-defaults to my cluster instead. I reproduced this with a minimal setup that deployed the same chart with same values overrides via helm and via pulumi and observe different resultant configuration in the live cluster (let me know if you want to see the minimal repro and I can push a quick public repo or gist).
Disclosure: I used a combination of Codex (where I was doing the main project work when I discovered this, had it do the initial reproduction and initial hypothesis) and Claude Code with Opus 4.6 (where I honed in on this specific issue and worked up the minimal tests and fix). However, I'm generally a skeptic of agentic coding, so I've been careful to read through everything here (small as it is lol) and while I'm not a go programmer, this seems succinct and reasonable to me. I'm happy to followup with any additional contribution steps.