fix: do not panic in merge.Merge if map value is nil

Checking for `zeroValue` is not enough when accessing `map[string]any`.

Closes #8005

Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
This commit is contained in:
Dmitriy Matrenichev 2023-12-04 12:08:19 +03:00
parent d8a435f0e4
commit 6329222bdc
No known key found for this signature in database
GPG Key ID: D3363CF894E68892
2 changed files with 59 additions and 13 deletions

View File

@ -27,12 +27,12 @@ import (
// - if it is a struct, merge is performed for each field of the struct.
// - if the type implements 'merger' interface, Merge function is called to handle the merge process.
// - merger interface should be implemented on the pointer to the type.
func Merge(left, right interface{}) error {
func Merge(left, right any) error {
return merge(reflect.ValueOf(left), reflect.ValueOf(right), false)
}
type merger interface {
Merge(other interface{}) error
Merge(other any) error
}
var (
@ -105,7 +105,7 @@ func merge(vl, vr reflect.Value, replace bool) error {
}
for _, k := range vr.MapKeys() {
if vl.MapIndex(k) != zeroValue {
if !isNilOrZero(vl.MapIndex(k)) {
valueType := tl.Elem()
var v, rightV reflect.Value
@ -193,3 +193,19 @@ func merge(vl, vr reflect.Value, replace bool) error {
return nil
}
// isNilOrZero returns true if the [reflect.Value] is zero [reflect.Value] or something that is nil.
// We need it because if map contains a key with `nil` value, simply comparing that result to the `zeroValue`
// is not enough.
func isNilOrZero(idx reflect.Value) bool {
if idx == zeroValue {
return true
}
switch idx.Kind() { //nolint:exhaustive
case reflect.Interface, reflect.Pointer, reflect.Slice, reflect.Map, reflect.Chan, reflect.Func:
return idx.IsNil()
default:
return false
}
}

View File

@ -33,7 +33,7 @@ type Struct struct {
type CustomSlice []string
func (s *CustomSlice) Merge(other interface{}) error {
func (s *CustomSlice) Merge(other any) error {
otherSlice, ok := other.(CustomSlice)
if !ok {
return fmt.Errorf("other is not CustomSlice: %v", other)
@ -45,13 +45,13 @@ func (s *CustomSlice) Merge(other interface{}) error {
return nil
}
type Unstructured map[string]interface{}
type Unstructured map[string]any
func TestMerge(t *testing.T) {
for _, tt := range []struct {
name string
left, right interface{}
expected interface{}
left, right any
expected any
}{
{
name: "zero",
@ -249,8 +249,8 @@ func TestMerge(t *testing.T) {
name: "unstructured",
left: &Unstructured{
"a": "aa",
"map": map[string]interface{}{
"slice": []interface{}{
"map": map[string]any{
"slice": []any{
"s1",
},
"some": "value",
@ -258,8 +258,8 @@ func TestMerge(t *testing.T) {
},
right: &Unstructured{
"b": "bb",
"map": map[string]interface{}{
"slice": []interface{}{
"map": map[string]any{
"slice": []any{
"s2",
},
"other": "thing",
@ -268,8 +268,8 @@ func TestMerge(t *testing.T) {
expected: &Unstructured{
"a": "aa",
"b": "bb",
"map": map[string]interface{}{
"slice": []interface{}{
"map": map[string]any{
"slice": []any{
"s1",
"s2",
},
@ -278,6 +278,36 @@ func TestMerge(t *testing.T) {
},
},
},
{
name: "unstructed with nil value",
left: Unstructured{
"a": nil,
"b": []any{
"c",
"d",
},
},
right: Unstructured{
"a": Unstructured{
"b": []any{
"c",
"d",
},
},
},
expected: Unstructured{
"a": Unstructured{
"b": []any{
"c",
"d",
},
},
"b": []any{
"c",
"d",
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
err := merge.Merge(tt.left, tt.right)