From 39995d388e5c0ffdaa7d8196b6901c0d67f0854e Mon Sep 17 00:00:00 2001 From: BeganovR <446110@niuitmo.ru> Date: Tue, 25 Aug 2026 14:17:45 +0300 Subject: [PATCH] fix(action): stop resurrected nulled values on the first reuse-values upgrade When --reuse-values is set together with --set key=null, the null correctly drops the key from the persisted config, but chart.Values is set to the fully resolved old release values, so the final render-time coalesce falls back to that stale override and silently brings the key back for that one upgrade. A second, identical upgrade then works, because by then the release's stored config has already lost the key. Prune any key that newVals explicitly nulls from the old values before they're assigned to chart.Values, so an explicit null takes effect on the first upgrade. Signed-off-by: BeganovR <446110@niuitmo.ru> --- pkg/action/upgrade.go | 25 +++++++++++++++++++++++++ pkg/action/upgrade_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7f66ceefb..331b93aaa 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -621,6 +621,14 @@ func (u *Upgrade) reuseValues(chart *chartv2.Chart, current *release.Release, ne return nil, fmt.Errorf("failed to rebuild old values: %w", err) } + // A `--set key=null` cancels key's value from current.Config below, + // but that alone only stops CoalesceTables from copying it over. + // oldVals becomes chart.Values, which the final render-time coalesce + // falls back to for any key newVals doesn't have, so without pruning + // the same key here it would resurface from the release being + // reused instead of the chart's own default. + pruneNilOverrides(oldVals, newVals) + newVals = util.CoalesceTables(newVals, current.Config) chart.Values = oldVals @@ -644,6 +652,23 @@ func (u *Upgrade) reuseValues(chart *chartv2.Chart, current *release.Release, ne return newVals, nil } +// pruneNilOverrides deletes from dst every key that is explicitly nil in +// newVals, recursing into nested tables so an explicit `--set key=null` +// removes the key from dst at whatever depth it appears. +func pruneNilOverrides(dst, newVals map[string]any) { + for key, val := range newVals { + if val == nil { + delete(dst, key) + continue + } + if sub, ok := val.(map[string]any); ok { + if dsub, ok := dst[key].(map[string]any); ok { + pruneNilOverrides(dsub, sub) + } + } + } +} + func validateManifest(c kube.Interface, manifest []byte, openAPIValidation bool) error { _, err := c.Build(bytes.NewReader(manifest), openAPIValidation) return err diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 53419b6a8..cca59cab3 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/resource" + chartcommon "helm.sh/helm/v4/pkg/chart/common" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" @@ -332,6 +333,35 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { } is.Equal(expectedValues, updatedRes.Config) }) + + t.Run("nulling a previously set value with reuse-values takes effect on the first upgrade", func(t *testing.T) { + is := assert.New(t) + req := require.New(t) + upAction := upgradeAction(t) + + imageTemplate := &chartcommon.File{ + Name: "templates/image", + ModTime: time.Now(), + Data: []byte("image: {{ .Values.image | default \"chart-default\" }}"), + } + ch := buildChartWithTemplates([]*chartcommon.File{imageTemplate}) + + rel := releaseStub() + rel.Name = "nuketown" + rel.Info.Status = common.StatusDeployed + rel.Chart = ch + rel.Config = map[string]any{"image": "old-image"} + req.NoError(upAction.cfg.Releases.Create(rel)) + + upAction.ReuseValues = true + resi, err := upAction.Run(rel.Name, ch, map[string]any{"image": nil}) + req.NoError(err) + res, err := releaserToV1Release(resi) + req.NoError(err) + + is.NotContains(res.Config, "image", "explicit null should remove the key from the persisted config") + is.Contains(res.Manifest, "image: chart-default", "nulling a reused value should fall back to the chart default on the first upgrade, not the second") + }) } func TestUpgradeRelease_ResetThenReuseValues(t *testing.T) {