diff --git a/internal/cmd/agent/deployer/desiredset/diff.go b/internal/cmd/agent/deployer/desiredset/diff.go index 5a4b8701f0..ed7f798fd7 100644 --- a/internal/cmd/agent/deployer/desiredset/diff.go +++ b/internal/cmd/agent/deployer/desiredset/diff.go @@ -452,17 +452,16 @@ func normalizeNullPatch( return false, nil } -// removeNullPatchFields recursively removes null values from patch data structures. -// It handles both maps (objects) and slices (arrays), preserving non-null values. +// removeNullPatchFields removes null map/object fields from patch objects. // // Map behavior: // - Skips entries with nil values // - Skips entries that become empty maps after cleaning // // Slice behavior: -// - Skips nil elements -// - Skips elements that become empty maps after cleaning -// - Preserves non-map elements (strings, numbers, etc.) +// - Preserves arrays as-is. JSON merge patches replace arrays atomically, so +// changing nulls inside an array changes the replacement value and can hide +// real drift in keyed lists such as container specs. func removeNullPatchFields(value any) any { switch v := value.(type) { case map[string]any: @@ -486,24 +485,7 @@ func removeNullPatchFields(value any) any { return result case []any: - result := make([]any, 0, len(v)) - for i := range v { - // Skip null elements in arrays - if v[i] == nil { - continue - } - - // Recursively clean nested structures - cleaned := removeNullPatchFields(v[i]) - - // Skip maps that became empty after cleaning - if cleanedMap, ok := cleaned.(map[string]any); ok && len(cleanedMap) == 0 { - continue - } - - result = append(result, cleaned) - } - return result + return v default: // Leaf values (strings, numbers, bools) pass through unchanged diff --git a/internal/cmd/agent/deployer/desiredset/diff_null_test.go b/internal/cmd/agent/deployer/desiredset/diff_null_test.go index 7355aaa1f9..4019c08b2c 100644 --- a/internal/cmd/agent/deployer/desiredset/diff_null_test.go +++ b/internal/cmd/agent/deployer/desiredset/diff_null_test.go @@ -39,9 +39,14 @@ func Test_Diff_NullPatch(t *testing.T) { expectPatch: `{"spec":{"template":{"spec":{"containers":[{"name":"c1","image":"nginx"}]}}}}`, }, { - name: "removes_null_elements_from_arrays", + name: "preserves_null_elements_inside_arrays", patch: `{"spec":{"tolerations":[{"key":"a","operator":null},null,{"key":"b"}]}}`, - expectPatch: `{"spec":{"tolerations":[{"key":"a"},{"key":"b"}]}}`, + expectPatch: `{"spec":{"tolerations":[{"key":"a","operator":null},null,{"key":"b"}]}}`, + }, + { + name: "preserves_null_fields_inside_arrays", + patch: `{"spec":{"template":{"spec":{"containers":[{"name":"nginx","image":"nginx:stable-alpine","imagePullPolicy":null}]}}}}`, + expectPatch: `{"spec":{"template":{"spec":{"containers":[{"name":"nginx","image":"nginx:stable-alpine","imagePullPolicy":null}]}}}}`, }, { name: "removes_multiple_nulls_across_tree", @@ -91,6 +96,7 @@ func Test_Diff_NullPatch(t *testing.T) { // Test_Diff_RemoveNullPatchFields validates the recursive null removal logic // with a complex nested structure containing maps, arrays, and null values. +// Arrays are preserved because JSON merge patches replace arrays atomically. func Test_Diff_RemoveNullPatchFields(t *testing.T) { input := map[string]any{ "spec": map[string]any{ @@ -113,7 +119,8 @@ func Test_Diff_RemoveNullPatchFields(t *testing.T) { expected := map[string]any{ "spec": map[string]any{ "list": []any{ - map[string]any{"name": "a"}, + map[string]any{"name": "a", "value": nil}, + nil, "text", }, },