Refactor coalesce functions to handle empty maps

Reordered coalesceValues and coalesceTablesFullKey to convert
explicit empty overrides into empty maps before merging. This
prevents default map entries from resurfacing, even for nested
tables. Additionally, the regression suite is expanded to cover
nested empty overrides, ensuring empty maps are preserved across
different override shapes.

Signed-off-by: Siew Kam Onn <kosiew@gmail.com>
pull/31331/head
Siew Kam Onn 1 year ago
parent 2f2a54a0aa
commit b099506632

@ -239,7 +239,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 // This allows Helm's various sources of values (value files or --set) to
// remove incompatible keys from any previous chart, file, or set values. // remove incompatible keys from any previous chart, file, or set values.
delete(v, key) 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]. // if v[key] is a table, merge nv's val table into v[key].
src, ok := val.(map[string]interface{}) src, ok := val.(map[string]interface{})
if !ok { if !ok {
@ -248,14 +264,16 @@ func coalesceValues(printf printFn, c chart.Charter, v map[string]interface{}, p
if val != nil { if val != nil {
printf("warning: skipped value for %s.%s: Not a table.", subPrefix, key) printf("warning: skipped value for %s.%s: Not a table.", subPrefix, key)
} }
} else { continue
}
// If the key is a child chart, coalesce tables with Merge set to true // If the key is a child chart, coalesce tables with Merge set to true
merge := childChartMergeTrue(c, key, merge) merge := childChartMergeTrue(c, key, merge)
// Because v has higher precedence than nv, dest values override src // Because v has higher precedence than nv, dest values override src
// values. // values.
coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge) coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge)
} continue
} }
} else { } else {
// If the key is not in v, copy it from nv. // If the key is not in v, copy it from nv.
@ -320,17 +338,36 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]interface{}, pref
// values. // values.
for key, val := range src { for key, val := range src {
fullkey := concatPrefix(prefix, key) fullkey := concatPrefix(prefix, key)
if dv, ok := dst[key]; ok && !merge && dv == nil { dv, ok := dst[key]
delete(dst, key) if !ok {
} else if !ok {
dst[key] = val 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) { if istable(dv) {
coalesceTablesFullKey(printf, dv.(map[string]interface{}), val.(map[string]interface{}), fullkey, merge) coalesceTablesFullKey(printf, dv.(map[string]interface{}), val.(map[string]interface{}), fullkey, merge)
} else { } else {
printf("warning: cannot overwrite table with non table for %s (%v)", fullkey, val) 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) printf("warning: destination for %s is a table. Ignoring non-table value (%v)", fullkey, val)
} }
} }
@ -342,3 +379,14 @@ func istable(v interface{}) bool {
_, ok := v.(map[string]interface{}) _, ok := v.(map[string]interface{})
return ok return ok
} }
func isExplicitEmptyOverride(v interface{}) bool {
switch vv := v.(type) {
case []interface{}:
return len(vv) == 0 || (len(vv) == 1 && vv[0] == "")
case []string:
return len(vv) == 0 || (len(vv) == 1 && vv[0] == "")
default:
return false
}
}

@ -267,6 +267,64 @@ func TestCoalesceValuesEmptyMapOverride(t *testing.T) {
assert.NotContains(t, result, "toDelete", "expected toDelete key to be removed when set to nil override") 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("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) { func ttpl(tpl string, v map[string]interface{}) (string, error) {
var b bytes.Buffer var b bytes.Buffer
tt := template.Must(template.New("t").Parse(tpl)) tt := template.Must(template.New("t").Parse(tpl))

@ -485,9 +485,18 @@ func (t *parser) valList() ([]interface{}, error) {
return list, err return list, err
case last == '}': case last == '}':
// If this is followed by ',', consume it. // If this is followed by ',', consume it.
if r, _, e := t.sc.ReadRune(); e == nil && r != ',' { r, _, e := t.sc.ReadRune()
if e == nil {
if r != ',' {
t.sc.UnreadRune() 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) v, e := t.reader(rs)
list = append(list, v) list = append(list, v)
return list, e return list, e

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

Loading…
Cancel
Save