Treat empty error and nil/null as equivalent.

Fixes: kro-run#289
barney-s · Feb 12, 2025 · 7b6ee51 · 7b6ee51
1 parent dc12732
commit 7b6ee51
Show file tree

Hide file tree

Showing 2 changed files with 129 additions and 10 deletions.
diff --git a/pkg/controller/instance/delta/delta.go b/pkg/controller/instance/delta/delta.go
@@ -100,11 +100,6 @@ func cleanMetadata(obj *unstructured.Unstructured) {
 //
 // Records a Difference if values don't match or are of different types.
 func walkCompare(desired, observed interface{}, path string, differences *[]Difference) {
-	// Special case: treat empty array and nil as equivalent
-	if isEmptyArrayOrNil(desired) && isEmptyArrayOrNil(observed) {
-		return
-	}
-
 	switch d := desired.(type) {
 	case map[string]interface{}:
 		e, ok := observed.(map[string]interface{})
@@ -118,13 +113,19 @@ func walkCompare(desired, observed interface{}, path string, differences *[]Diff
 		}
 		walkMap(d, e, path, differences)
 
+	case nil:
+		// Special case: treat empty array and nil as equivalent
+		if isEmptyArrayOrNil(desired) && isEmptyArrayOrNil(observed) {
+			return
+		}
+
 	case []interface{}:
+		// Special case: if desired is empty array and observed is nil, treat as equal
+		if isEmptyArrayOrNil(desired) && isEmptyArrayOrNil(observed) {
+			return
+		}
 		e, ok := observed.([]interface{})
 		if !ok {
-			// Special case: if desired is empty array and observed is nil, treat as equal
-			if len(d) == 0 && observed == nil {
-				return
-			}
 			*differences = append(*differences, Difference{
 				Path:     path,
 				Observed: observed,
@@ -168,7 +169,7 @@ func walkMap(desired, observed map[string]interface{}, path string, differences
 		}
 
 		observedVal, exists := observed[k]
-		if !exists && desiredVal != nil {
+		if !exists && !isEmptyArrayOrNil(desiredVal) {
 			*differences = append(*differences, Difference{
 				Path:     newPath,
 				Observed: nil,

diff --git a/pkg/controller/instance/delta/delta_test.go b/pkg/controller/instance/delta/delta_test.go
@@ -458,3 +458,121 @@ func TestCompare_EmptyMaps(t *testing.T) {
 		})
 	}
 }
+
+func TestCompare_EmptyArraysAndNulls(t *testing.T) {
+	tests := []struct {
+		name     string
+		desired  *unstructured.Unstructured
+		observed *unstructured.Unstructured
+		wantDiff bool
+	}{
+		{
+			name: "empty array equals null in nested structure",
+			desired: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"template": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"env": []interface{}{},
+								},
+							},
+						},
+					},
+				},
+			},
+			observed: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"template": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"env": nil,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantDiff: false,
+		},
+		{
+			name: "null equals empty array in nested structure",
+			desired: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"template": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"env": nil,
+								},
+							},
+						},
+					},
+				},
+			},
+			observed: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"template": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"env": []interface{}{},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantDiff: false,
+		},
+		{
+			name: "non-empty array differs from null",
+			desired: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"template": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"env": []interface{}{
+										map[string]interface{}{
+											"name":  "TEST",
+											"value": "value",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			observed: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{
+						"template": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"env": nil,
+								},
+							},
+						},
+					},
+				},
+			},
+			wantDiff: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			differences, err := Compare(tt.desired, tt.observed)
+			if err != nil {
+				t.Errorf("Compare() error = %v", err)
+				return
+			}
+			if (len(differences) > 0) != tt.wantDiff {
+				t.Errorf("Compare() got differences = %v, want diff = %v", differences, tt.wantDiff)
+			}
+		})
+	}
+}