diff --git a/dynamic/apply/apply.go b/dynamic/apply/apply.go index 819cd6e..f6cac22 100644 --- a/dynamic/apply/apply.go +++ b/dynamic/apply/apply.go @@ -162,45 +162,73 @@ func merge(fieldPath string, observedAsDest, lastApplied, desired interface{}) ( switch observedDestVal := observedAsDest.(type) { case map[string]interface{}: + + var lastAppliedVal, desiredVal map[string]interface{} + var ok bool // In this case, observed is a map. // Make sure the others are maps too. // Nil desired &/ nil last applied are OK. - lastAppliedVal, ok := lastApplied.(map[string]interface{}) - if !ok && lastAppliedVal != nil { - return nil, - errors.Errorf( - "%s: Expecting last applied as map[string]interface{}, got %T", - fieldPath, lastApplied, - ) + + // NOTE: There are chances of having lastApplied field as nil + // if observedAsDest fields are autogenerated. If lastApplied is + // nil nothing we can do by type asserting. + if lastApplied != nil { + lastAppliedVal, ok = lastApplied.(map[string]interface{}) + if !ok { + return nil, + errors.Errorf( + "%s: Expecting last applied as map[string]interface{}, got %T", + fieldPath, lastApplied, + ) + } } - desiredVal, ok := desired.(map[string]interface{}) - if !ok && desiredVal != nil { - return nil, - errors.Errorf( - "%s: Expecting desired as map[string]interface{}, got %T", - fieldPath, desired, - ) + + // NOTE: There are chances of having desired field as nil if a caller + // doesn't need to update the fields in that path no need for any update + if desired != nil { + desiredVal, ok = desired.(map[string]interface{}) + if !ok { + return nil, + errors.Errorf( + "%s: Expecting desired as map[string]interface{}, got %T", + fieldPath, desired, + ) + } } return mergeMap(fieldPath, observedDestVal, lastAppliedVal, desiredVal) case []interface{}: + var lastAppliedVal, desiredVal []interface{} + var ok bool + // In this case observed is an array. // Make sure desired & last applied are arrays too. // Nil desired &/ last applied are OK. - lastAppliedVal, ok := lastApplied.([]interface{}) - if !ok && lastAppliedVal != nil { - return nil, - errors.Errorf( - "%s: Expecting last applied as []interface{}, got %T", - fieldPath, lastApplied, - ) + + // NOTE: There are chances of having lastApplied field as nil + // if observedAsDest fields are autogenerated. If lastApplied is + // nil nothing we can do by type asserting. + if lastApplied != nil { + lastAppliedVal, ok = lastApplied.([]interface{}) + if !ok { + return nil, + errors.Errorf( + "%s: Expecting last applied as []interface{}, got %T", + fieldPath, lastApplied, + ) + } } - desiredVal, ok := desired.([]interface{}) - if !ok && desiredVal != nil { - return nil, - fmt.Errorf( - "%s: Expecting desired as []interface{}, got %T", - fieldPath, desired, - ) + + // NOTE: There are chances of having desired field as nil if a caller + // doesn't need to update the fields in that path no need for any update + if desired != nil { + desiredVal, ok = desired.([]interface{}) + if !ok { + return nil, + fmt.Errorf( + "%s: Expecting desired as []interface{}, got %T", + fieldPath, desired, + ) + } } return mergeArray(fieldPath, observedDestVal, lastAppliedVal, desiredVal) default: diff --git a/dynamic/apply/apply_test.go b/dynamic/apply/apply_test.go index 4d986ad..74e058f 100644 --- a/dynamic/apply/apply_test.go +++ b/dynamic/apply/apply_test.go @@ -2009,6 +2009,322 @@ func TestMerge(t *testing.T) { } } +func TestMergeTwo(t *testing.T) { + tests := map[string]struct { + observed map[string]interface{} + lastApplied map[string]interface{} + desired map[string]interface{} + want map[string]interface{} + isErrorExpected bool + }{ + "Merge config map with mismatch type": { + observed: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]interface{}{ + "node.properties": "data1", + }, + }, + lastApplied: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]string{ + "node.properties": "data2", + }, + }, + desired: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]string{ + "node.properties": "data2", + }, + }, + isErrorExpected: true, + want: nil, + }, + "Merge config map with mismatch type and without lastapplied": { + observed: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]interface{}{ + "node.properties": "data1", + }, + }, + lastApplied: nil, + desired: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]string{ + "node.properties": "data2", + }, + }, + isErrorExpected: true, + want: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]string{ + "node.properties": "data2", + }, + }, + }, + "Merge configmap with correct type": { + observed: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + "labels": map[string]interface{}{ + "key1": "value1", + }, + }, + "data": map[string]interface{}{ + "node.properties": "data1", + }, + }, + lastApplied: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + "labels": map[string]interface{}{ + "key1": "value1", + }, + }, + "data": map[string]interface{}{ + "node.properties": "data2", + }, + }, + desired: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]interface{}{ + "node.properties": "data2", + }, + }, + want: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "node-cm1", + "namespace": "metac", + }, + "data": map[string]interface{}{ + "node.properties": "data2", + }, + }, + }, + "Merge array of nested interfaces with last applied": { + observed: map[string]interface{}{ + "key1": "old-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "old-value", + }, + map[string]interface{}{ + "key2": []interface{}{ + "old-value1", + "old-value2", + }, + }, + }, + }, + lastApplied: map[string]interface{}{ + "key1": "old-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "new-value", + }, + map[string]interface{}{ + "key2": []interface{}{ + "new-value1", + "new-value2", + }, + }, + }, + }, + desired: map[string]interface{}{ + "key1": "old-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "new-value", + }, + map[string]interface{}{ + "key2": []interface{}{ + "new-value1", + "new-value2", + }, + }, + }, + }, + want: map[string]interface{}{ + "key1": "old-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "new-value", + }, + map[string]interface{}{ + "key2": []interface{}{ + "new-value1", + "new-value2", + }, + }, + }, + }, + }, + "Merge array of interfaces without last applied and desired contains array of strings": { + observed: map[string]interface{}{ + "key1": "old-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "old-value1", + }, + map[string]interface{}{ + "key1": "old-value2", + "key2": []interface{}{ + "old-value1", + "old-value2", + }, + }, + }, + }, + lastApplied: nil, + desired: map[string]interface{}{ + "key1": "new-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "new-value", + }, + map[string]interface{}{ + "key2": []string{ + "new-value1", + }, + }, + }, + }, + want: map[string]interface{}{ + "key1": "new-value", + "key2": []interface{}{ + map[string]interface{}{ + "key1": "new-value", + }, + map[string]interface{}{ + "key2": []string{ + "new-value1", + }, + }, + }, + }, + }, + "Merge array list with last applied": { + observed: map[string]interface{}{ + "list1": []interface{}{ + "old-value1", + "old-value2", + }, + "list2": []interface{}{ + int64(1), + int64(2), + }, + }, + lastApplied: map[string]interface{}{ + "list1": []interface{}{ + "old-value3", + }, + }, + desired: map[string]interface{}{ + "list1": []interface{}{ + "old-value1", + "old-value3", + }, + }, + want: map[string]interface{}{ + "list1": []interface{}{ + "old-value1", + "old-value3", + }, + "list2": []interface{}{ + int64(1), + int64(2), + }, + }, + }, + "Merge array list with mismatch of type and last applied": { + observed: map[string]interface{}{ + "list1": []interface{}{ + "old-value1", + "old-value2", + }, + }, + lastApplied: map[string]interface{}{ + "list1": []string{ + "old-value3", + }, + }, + desired: map[string]interface{}{ + "list1": []string{ + "old-value1", + "old-value3", + }, + }, + want: nil, + isErrorExpected: true, + }, + } + for name, tc := range tests { + name := name + tc := tc + got, err := Merge(tc.observed, tc.lastApplied, tc.desired) + if tc.isErrorExpected && err == nil { + t.Errorf( + "%s test case expected error to occur but error didn't occur", + name) + } + if !tc.isErrorExpected && err != nil { + t.Errorf( + "%s test case expected error not to occur but got error %v", + name, + err) + } + if !tc.isErrorExpected { + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("%q:\nGot: %v\nWant: %v\nDiff: %s", + name, got, tc.want, diff.ObjectReflectDiff(got, tc.want), + ) + } + } + } +} + func TestLastAppliedAnnotation(t *testing.T) { // Round-trip some JSON through Set/Get methods. inJSON := `{