diff --git a/merge/multiple_appliers_test.go b/merge/multiple_appliers_test.go index 7fad7b15..48f54cdb 100644 --- a/merge/multiple_appliers_test.go +++ b/merge/multiple_appliers_test.go @@ -1061,14 +1061,14 @@ func TestMultipleAppliersNestedType(t *testing.T) { }, }, Object: ` - mapOfMapsRecursive: - a: - c: - d: - e: - f: - g: - `, + mapOfMapsRecursive: + a: {} + c: + d: + e: + f: + g: + `, APIVersion: "v1", Managed: fieldpath.ManagedFields{ "apply-one": fieldpath.NewVersionedSet( @@ -1187,13 +1187,13 @@ func TestMultipleAppliersDeducedType(t *testing.T) { }, }, Object: ` - a: - c: - d: - e: - f: - g: - `, + a: {} + c: + d: + e: + f: + g: + `, APIVersion: "v1", Managed: fieldpath.ManagedFields{ "apply-two": fieldpath.NewVersionedSet( diff --git a/merge/nested_test.go b/merge/nested_test.go index ffdb07b8..2da4d4c9 100644 --- a/merge/nested_test.go +++ b/merge/nested_test.go @@ -554,8 +554,8 @@ func TestUpdateNestedType(t *testing.T) { }, }, Object: ` - struct: - `, + struct: {} + `, APIVersion: "v1", Managed: fieldpath.ManagedFields{ "default": fieldpath.NewVersionedSet( diff --git a/typed/remove.go b/typed/remove.go index 78ba6f50..0db1734f 100644 --- a/typed/remove.go +++ b/typed/remove.go @@ -75,6 +75,7 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) { } var newItems []interface{} + hadMatches := false iter := l.RangeUsing(w.allocator) defer w.allocator.Free(iter) for iter.Next() { @@ -98,12 +99,26 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) { continue } if isPrefixMatch { + // Removing nested items within this list item and preserve if it becomes empty + hadMatches = true + wasMap := item.IsMap() + wasList := item.IsList() item = removeItemsWithSchema(item, w.toRemove.WithPrefix(pe), w.schema, t.ElementType, w.shouldExtract) + // If item returned null but we're removing items within the structure(not the item itself), + // preserve the empty container structure + if item.IsNull() && !w.shouldExtract { + if wasMap { + item = value.NewValueInterface(map[string]interface{}{}) + } else if wasList { + item = value.NewValueInterface([]interface{}{}) + } + } } newItems = append(newItems, item.Unstructured()) } } - if len(newItems) > 0 { + // Preserve empty lists (non-nil) instead of converting to null when items were matched and removed + if len(newItems) > 0 || (hadMatches && !w.shouldExtract) { w.out = newItems } return nil @@ -141,6 +156,7 @@ func (w *removingWalker) doMap(t *schema.Map) ValidationErrors { } newMap := map[string]interface{}{} + hadMatches := false m.Iterate(func(k string, val value.Value) bool { pe := fieldpath.PathElement{FieldName: &k} path, _ := fieldpath.MakePath(pe) @@ -158,7 +174,19 @@ func (w *removingWalker) doMap(t *schema.Map) ValidationErrors { return true } if subset := w.toRemove.WithPrefix(pe); !subset.Empty() { + hadMatches = true + wasMap := val.IsMap() + wasList := val.IsList() val = removeItemsWithSchema(val, subset, w.schema, fieldType, w.shouldExtract) + // If val returned null but we're removing items within the structure (not the field itself), + // preserve the empty container structure + if val.IsNull() && !w.shouldExtract { + if wasMap { + val = value.NewValueInterface(map[string]interface{}{}) + } else if wasList { + val = value.NewValueInterface([]interface{}{}) + } + } } else { // don't save values not on the path when we shouldExtract. if w.shouldExtract { @@ -168,7 +196,8 @@ func (w *removingWalker) doMap(t *schema.Map) ValidationErrors { newMap[k] = val.Unstructured() return true }) - if len(newMap) > 0 { + // Preserve empty maps (non-nil) instead of converting to null when items were matched and removed + if len(newMap) > 0 || (hadMatches && !w.shouldExtract) { w.out = newMap } return nil diff --git a/typed/remove_test.go b/typed/remove_test.go index df59c163..76850652 100644 --- a/typed/remove_test.go +++ b/typed/remove_test.go @@ -281,7 +281,7 @@ var removeCases = []removeTestCase{{ quadruplets: []removeQuadruplet{{ `{"setBool":[false]}`, _NS(_P("setBool", _V(false))), - `{"setBool":null}`, + `{"setBool":[]}`, `{"setBool":[false]}`, }, { `{"setBool":[false]}`, @@ -671,7 +671,7 @@ var removeCases = []removeTestCase{{ _NS( _P("mapOfMapsRecursive", "a"), ), - `{"mapOfMapsRecursive"}`, + `{"mapOfMapsRecursive":{}}`, `{"mapOfMapsRecursive": {"a":null}}`, }, { // second-level map @@ -679,7 +679,7 @@ var removeCases = []removeTestCase{{ _NS( _P("mapOfMapsRecursive", "a", "b"), ), - `{"mapOfMapsRecursive":{"a":null}}`, + `{"mapOfMapsRecursive":{"a":{}}}`, `{"mapOfMapsRecursive": {"a":{"b":null}}}`, }, { // third-level map @@ -687,7 +687,7 @@ var removeCases = []removeTestCase{{ _NS( _P("mapOfMapsRecursive", "a", "b", "c"), ), - `{"mapOfMapsRecursive":{"a":{"b":null}}}`, + `{"mapOfMapsRecursive":{"a":{"b":{}}}}`, `{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`, }, { // empty list