fix: RemoveItems should preserve empty list and map

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(

merge/nested_test.go

@@ -554,8 +554,8 @@ func TestUpdateNestedType(t *testing.T) {
 				},
 			},
 			Object: `
-				struct:
-			`,
+			struct: {}
+		`,
 			APIVersion: "v1",
 			Managed: fieldpath.ManagedFields{
 				"default": fieldpath.NewVersionedSet(

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

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,23 +671,23 @@ var removeCases = []removeTestCase{{
 		_NS(
 			_P("mapOfMapsRecursive", "a"),
 		),
-		`{"mapOfMapsRecursive"}`,
+		`{"mapOfMapsRecursive":{}}`,
 		`{"mapOfMapsRecursive": {"a":null}}`,
 	}, {
 		// second-level map
 		`{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`,
 		_NS(
 			_P("mapOfMapsRecursive", "a", "b"),
 		),
-		`{"mapOfMapsRecursive":{"a":null}}`,
+		`{"mapOfMapsRecursive":{"a":{}}}`,
 		`{"mapOfMapsRecursive": {"a":{"b":null}}}`,
 	}, {
 		// third-level map
 		`{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`,
 		_NS(
 			_P("mapOfMapsRecursive", "a", "b", "c"),
 		),
-		`{"mapOfMapsRecursive":{"a":{"b":null}}}`,
+		`{"mapOfMapsRecursive":{"a":{"b":{}}}}`,
 		`{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`,
 	}, {
 		// empty list