From dfa49faa7923b73cac259c08636b7e3f23d0276e Mon Sep 17 00:00:00 2001 From: Siew Kam Onn Date: Fri, 10 Oct 2025 21:21:32 +0800 Subject: [PATCH] Fix isExplicitEmptyOverride for empty maps handling Enhanced the isExplicitEmptyOverride function to correctly identify empty maps and placeholders created by --set key={}, preventing unintentional reintroduction of chart defaults. Added regression tests to ensure proper treatment of empty map placeholders and validated that CLI values maintain empty maps after coalescing. Signed-off-by: Siew Kam Onn --- pkg/chart/common/util/coalesce.go | 27 +++++++++++- pkg/chart/common/util/coalesce_test.go | 13 ++++++ pkg/cli/values/options_test.go | 57 ++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/pkg/chart/common/util/coalesce.go b/pkg/chart/common/util/coalesce.go index e5f7664c6..c2b0ecd97 100644 --- a/pkg/chart/common/util/coalesce.go +++ b/pkg/chart/common/util/coalesce.go @@ -382,10 +382,33 @@ func istable(v interface{}) bool { func isExplicitEmptyOverride(v interface{}) bool { switch vv := v.(type) { + case map[string]interface{}: + return len(vv) == 0 + case map[interface{}]interface{}: + return len(vv) == 0 case []interface{}: - return len(vv) == 0 || (len(vv) == 1 && vv[0] == "") + switch len(vv) { + case 0: + return true + case 1: + switch item := vv[0].(type) { + case string: + return item == "" + default: + return isExplicitEmptyOverride(item) + } + default: + return false + } case []string: - return len(vv) == 0 || (len(vv) == 1 && vv[0] == "") + switch len(vv) { + case 0: + return true + case 1: + return vv[0] == "" + default: + return false + } default: return false } diff --git a/pkg/chart/common/util/coalesce_test.go b/pkg/chart/common/util/coalesce_test.go index 3d1b110bc..a56c3f37b 100644 --- a/pkg/chart/common/util/coalesce_test.go +++ b/pkg/chart/common/util/coalesce_test.go @@ -306,6 +306,19 @@ func TestCoalesceValuesEmptySliceOverrideForMap(t *testing.T) { assert.Empty(t, config, "expected config map to be empty") }) + t.Run("explicit empty map placeholder", func(t *testing.T) { + overrides := map[string]interface{}{ + "config": []interface{}{map[string]interface{}{}}, + } + + result, err := CoalesceValues(newChart(), overrides) + require.NoError(t, err) + + config, ok := result["config"].(map[string]interface{}) + require.Truef(t, ok, "expected config to remain a map, got %T", result["config"]) + assert.Empty(t, config, "expected config map to be empty") + }) + t.Run("nested empty slice", func(t *testing.T) { overrides := map[string]interface{}{ "config": map[string]interface{}{ diff --git a/pkg/cli/values/options_test.go b/pkg/cli/values/options_test.go index fe1afc5d2..eee363237 100644 --- a/pkg/cli/values/options_test.go +++ b/pkg/cli/values/options_test.go @@ -26,6 +26,11 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/chart/common/util" + chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/getter" ) @@ -248,6 +253,58 @@ func TestReadFile(t *testing.T) { } } +func TestMergeValuesExplicitEmptyOverrides(t *testing.T) { + newChart := func() *chart.Chart { + return &chart.Chart{ + Metadata: &chart.Metadata{Name: "emptymap"}, + Values: map[string]interface{}{ + "key": map[string]interface{}{"default": true}, + }, + } + } + + tests := []struct { + name string + setup func(t *testing.T) *Options + }{ + { + name: "empty map from values file", + setup: func(t *testing.T) *Options { + dir := t.TempDir() + file := filepath.Join(dir, "values.yaml") + require.NoError(t, os.WriteFile(file, []byte("key: {}\n"), 0o600)) + return &Options{ValueFiles: []string{file}} + }, + }, + { + name: "empty map from set flag", + setup: func(_ *testing.T) *Options { + return &Options{Values: []string{"key={}"}} + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + opts := tt.setup(t) + + overrides, err := opts.MergeValues(getter.Providers{}) + require.NoError(t, err) + + result, err := util.CoalesceValues(newChart(), overrides) + require.NoError(t, err) + + raw, ok := result["key"] + require.True(t, ok, "expected key to be present") + + actual, ok := raw.(map[string]interface{}) + require.True(t, ok, "expected key to be a map but got %T", raw) + assert.Empty(t, actual) + }) + } +} + // TestReadFileErrorMessages tests specific error scenarios and their messages func TestReadFileErrorMessages(t *testing.T) { tests := []struct {