From 9c0642c21b0d7e4ba962567fdb9c413173fcec4f Mon Sep 17 00:00:00 2001 From: Puneet Dixit Date: Thu, 21 May 2026 13:49:08 +0530 Subject: [PATCH 1/2] fix: preserve array nulls in drift patches Assisted-by: OpenAI GPT-5 Signed-off-by: Puneet Dixit --- .../cmd/agent/deployer/desiredset/diff.go | 28 ++++--------------- .../deployer/desiredset/diff_null_test.go | 13 +++++++-- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/internal/cmd/agent/deployer/desiredset/diff.go b/internal/cmd/agent/deployer/desiredset/diff.go index 5a4b8701f0..8312775695 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 recursively removes null values 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", }, }, From e92580f6c350ed1a3ca7993816ab4d79664f920d Mon Sep 17 00:00:00 2001 From: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Date: Sat, 23 May 2026 20:34:43 +0530 Subject: [PATCH 2/2] docs: clarify null patch normalization Assisted-by: OpenAI GPT-5 Signed-off-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> --- internal/cmd/agent/deployer/desiredset/diff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/agent/deployer/desiredset/diff.go b/internal/cmd/agent/deployer/desiredset/diff.go index 8312775695..ed7f798fd7 100644 --- a/internal/cmd/agent/deployer/desiredset/diff.go +++ b/internal/cmd/agent/deployer/desiredset/diff.go @@ -452,7 +452,7 @@ func normalizeNullPatch( return false, nil } -// removeNullPatchFields recursively removes null values from patch objects. +// removeNullPatchFields removes null map/object fields from patch objects. // // Map behavior: // - Skips entries with nil values