Skip to content

fix: preserve array nulls in drift patches #5179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
puneetdixit200 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: preserve array nulls in drift patches #5179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c0642c
fix: preserve array nulls in drift patches
puneetdixit200 May 21, 2026
e92580f
docs: clarify null patch normalization
puneetdixit200 May 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 5 additions & 23 deletions internal/cmd/agent/deployer/desiredset/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
13 changes: 10 additions & 3 deletions internal/cmd/agent/deployer/desiredset/diff_null_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand All @@ -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",
},
},
Expand Down