Merge 3a1719e4e1 into 3120e88f9b

pkg/chart/common/util/coalesce.go

@@ -238,7 +238,23 @@ func coalesceValues(printf printFn, c chart.Charter, v map[string]interface{}, p
 				// This allows Helm's various sources of values (value files or --set) to
 				// remove incompatible keys from any previous chart, file, or set values.
 				delete(v, key)
-			} else if dest, ok := value.(map[string]interface{}); ok {
+				continue
+			}
+
+			if isExplicitEmptyOverride(value) {
+				// Interpret empty slices from values sources (for example, --set key={})
+				// as an explicit request for an empty map so chart defaults are removed.
+				dest := map[string]interface{}{}
+				v[key] = dest
+
+				if src, ok := val.(map[string]interface{}); ok {
+					merge := childChartMergeTrue(c, key, merge)
+					coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge)
+				}
+				continue
+			}
+
+			if dest, ok := value.(map[string]interface{}); ok {
 				// if v[key] is a table, merge nv's val table into v[key].
 				src, ok := val.(map[string]interface{})
 				if !ok {
@@ -247,14 +263,16 @@ func coalesceValues(printf printFn, c chart.Charter, v map[string]interface{}, p
 					if val != nil {
 						printf("warning: skipped value for %s.%s: Not a table.", subPrefix, key)
 					}
-				} else {
-					// If the key is a child chart, coalesce tables with Merge set to true
-					merge := childChartMergeTrue(c, key, merge)
-
-					// Because v has higher precedence than nv, dest values override src
-					// values.
-					coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge)
+					continue
 				}
+
+				// If the key is a child chart, coalesce tables with Merge set to true
+				merge := childChartMergeTrue(c, key, merge)
+
+				// Because v has higher precedence than nv, dest values override src
+				// values.
+				coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge)
+				continue
 			}
 		} else {
 			// If the key is not in v, copy it from nv.
@@ -302,6 +320,14 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref
 	if dst == nil {
 		return src
 	}
+	// If dst is empty and we're operating on a sub-key (prefix != ""),
+	// bail out when not merging so that child charts don't inherit defaults
+	// for an explicitly empty table in the parent. We only do this for
+	// non-top-level coalescing (prefix != "") because top-level coalescing
+	// still needs to pull chart defaults across when dst is empty.
+	if len(dst) == 0 && prefix != "" && !merge {
+		return dst
+	}
 	for key, val := range dst {
 		if val == nil {
 			src[key] = nil
@@ -311,17 +337,36 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref
 	// values.
 	for key, val := range src {
 		fullkey := concatPrefix(prefix, key)
-		if dv, ok := dst[key]; ok && !merge && dv == nil {
-			delete(dst, key)
-		} else if !ok {
+		dv, ok := dst[key]
+		if !ok {
 			dst[key] = val
-		} else if istable(val) {
+			continue
+		}
+
+		if !merge && dv == nil {
+			delete(dst, key)
+			continue
+		}
+
+		if isExplicitEmptyOverride(dv) {
+			empty := map[string]interface{}{}
+			dst[key] = empty
+			if srcMap, ok := val.(map[string]interface{}); ok {
+				coalesceTablesFullKey(printf, empty, srcMap, fullkey, merge)
+			}
+			continue
+		}
+
+		if istable(val) {
 			if istable(dv) {
 				coalesceTablesFullKey(printf, dv.(map[string]interface{}), val.(map[string]interface{}), fullkey, merge)
 			} else {
 				printf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val)
 			}
-		} else if istable(dv) && val != nil {
+			continue
+		}
+
+		if istable(dv) && val != nil {
 			printf("warning: destination for %s is a table. Ignoring non-table value (%v)", fullkey, val)
 		}
 	}
@@ -333,3 +378,37 @@ func istable(v interface{}) bool {
 	_, ok := v.(map[string]interface{})
 	return ok
 }
+
+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{}:
+		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:
+		switch len(vv) {
+		case 0:
+			return true
+		case 1:
+			return vv[0] == ""
+		default:
+			return false
+		}
+	default:
+		return false
+	}
+}

pkg/chart/common/util/coalesce_test.go

@@ -25,6 +25,7 @@ import (
 	"text/template"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"helm.sh/helm/v4/pkg/chart/common"
 	chart "helm.sh/helm/v4/pkg/chart/v2"
@@ -241,6 +242,102 @@ func TestCoalesceValues(t *testing.T) {
 	is.Equal(valsCopy, vals)
 }
 
+func TestCoalesceValuesEmptyMapOverride(t *testing.T) {
+	c := &chart.Chart{
+		Metadata: &chart.Metadata{Name: "emptymap"},
+		Values: map[string]interface{}{
+			"config":   map[string]interface{}{"foo": "bar"},
+			"toDelete": map[string]interface{}{"baz": "qux"},
+		},
+	}
+
+	overrides := map[string]interface{}{
+		"config":   map[string]interface{}{},
+		"toDelete": nil,
+	}
+
+	result, err := CoalesceValues(c, 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")
+	assert.NotContains(t, config, "foo", "expected config override to drop default key")
+
+	assert.NotContains(t, result, "toDelete", "expected toDelete key to be removed when set to nil override")
+}
+
+func TestCoalesceValuesEmptySliceOverrideForMap(t *testing.T) {
+	newChart := func() *chart.Chart {
+		return &chart.Chart{
+			Metadata: &chart.Metadata{Name: "emptymap"},
+			Values: map[string]interface{}{
+				"config": map[string]interface{}{
+					"foo":    "bar",
+					"nested": map[string]interface{}{"baz": "qux"},
+				},
+			},
+		}
+	}
+
+	t.Run("empty interface slice", func(t *testing.T) {
+		overrides := map[string]interface{}{
+			"config": []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("legacy single empty element slice", func(t *testing.T) {
+		overrides := map[string]interface{}{
+			"config": []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("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{}{
+				"nested": []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"])
+
+		nested, ok := config["nested"].(map[string]interface{})
+		require.Truef(t, ok, "expected nested override to remain a map, got %T", config["nested"])
+		assert.Empty(t, nested, "expected nested map to be empty")
+	})
+}
+
 func ttpl(tpl string, v map[string]interface{}) (string, error) {
 	var b bytes.Buffer
 	tt := template.Must(template.New("t").Parse(tpl))
@@ -401,6 +498,28 @@ func TestMergeValues(t *testing.T) {
 }
 
 func TestCoalesceTables(t *testing.T) {
+	t.Run("empty destination table overrides defaults", func(t *testing.T) {
+		t.Helper()
+
+		dst := map[string]interface{}{
+			"config": map[string]interface{}{},
+		}
+		src := map[string]interface{}{
+			"config": map[string]interface{}{
+				"enabled": true,
+				"port":    8080,
+			},
+		}
+
+		CoalesceTables(dst, src)
+
+		config, ok := dst["config"].(map[string]interface{})
+		require.Truef(t, ok, "config should remain a map, got %T", dst["config"])
+		assert.Empty(t, config, "expected empty config map")
+		assert.NotContains(t, config, "enabled", "expected default \"enabled\" key to be absent")
+		assert.NotContains(t, config, "port", "expected default \"port\" key to be absent")
+	})
+
 	dst := map[string]interface{}{
 		"name": "Ishmael",
 		"address": map[string]interface{}{

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,60 @@ 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 {
+				t.Helper()
+				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(t *testing.T) *Options {
+				t.Helper()
+				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 {

pkg/strvals/parser.go

@@ -485,8 +485,17 @@ func (t *parser) valList() ([]interface{}, error) {
 			return list, err
 		case last == '}':
 			// If this is followed by ',', consume it.
-			if r, _, e := t.sc.ReadRune(); e == nil && r != ',' {
-				t.sc.UnreadRune()
+			r, _, e := t.sc.ReadRune()
+			if e == nil {
+				if r != ',' {
+					t.sc.UnreadRune()
+				}
+			} else if e != io.EOF {
+				return list, e
+			}
+			if len(rs) == 0 && len(list) == 0 {
+				// Interpret "{}" as an explicit empty map, not an empty list.
+				return []interface{}{map[string]interface{}{}}, nil
 			}
 			v, e := t.reader(rs)
 			list = append(list, v)

pkg/strvals/parser_test.go

@@ -265,6 +265,11 @@ func TestParseSet(t *testing.T) {
 			map[string]interface{}{"name1": []string{"value1", "value2"}},
 			false,
 		},
+		{
+			"emptylist={}",
+			map[string]interface{}{"emptylist": []interface{}{map[string]interface{}{}}},
+			false,
+		},
 		{
 			"name1={value1,value2},name2={value1,value2}",
 			map[string]interface{}{