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) {