fix(appset): restore ignoreApplicationDifferences after normalization

Project: argoproj/argo-cd | Language: Go | Concepts: error-handling, testing-strategy, refactoring | Difficulty: mid
Link: https://github.com/argoproj/argo-cd/pull/29070

The problem

When using ignoreApplicationDifferences rules in ApplicationSet, removing a field (like /spec/source/kustomize/images) could leave its parent struct as an empty-but-present object (e.g., kustomize: {} instead of kustomize: null). This caused the ApplicationSet controller to see a difference between the live and generated specs—even though the difference was supposed to be ignored—leading to unnecessary patches that could overwrite changes made by tools like argocd-image-updater.

Why it's non-trivial

A naive fix might just remove the ignored field, but that doesn't account for how Go structs behave: removing a leaf field can leave its parent as a non-nil, empty struct. This subtle difference ({} vs null) can cause a DeepEqual check to fail, resulting in spurious patches. Fixing this requires careful normalization after ignore rules are applied, not just before.

The approach

1. Re-normalize specs after applying ignore rules

After applying ignore rules, the code explicitly re-normalizes both the live and generated specs to collapse empty-but-present structs to nil, restoring the intended behavior and preventing spurious diffs.

+	// Removing an ignored field can leave its parent as an empty-but-present struct (e.g.
+	// kustomize.images gone but kustomize: {} remains) that the caller's pre-normalization pass had
+	// no reason to nil out, since the field was populated at the time. Re-normalize both sides now so
+	// such leftovers collapse to nil the same way they would if the field had never been set, and
+	// don't show up as a spurious diff.
+	found.Spec = *argo.NormalizeApplicationSpec(&found.Spec)
+	generatedApp.Spec = *argo.NormalizeApplicationSpec(&generatedApp.Spec)

This ensures that after ignore rules are applied, both specs are normalized so that empty structs are set to nil, preventing false differences.

2. Fix normalization for multi-source Applications

The normalization function was updated to ensure it operates on the actual pointers in the Sources slice, not on copies:

-		for _, source := range spec.Sources {
-			NormalizeSource(&source)
+		for i := range spec.Sources {
+			NormalizeSource(&spec.Sources[i])

This change ensures that normalization mutates the real objects in the slice, not just copies.

3. Add regression tests for both single-source and multi-source scenarios

Two new tests were added to verify that ignored fields do not result in spurious patches, both for single-source and multi-source Applications:

func TestCreateOrUpdateDoesNotRevertIgnoredKustomizeImages(t *testing.T) { ... }
func TestCreateOrUpdateDoesNotRevertIgnoredKustomizeImagesMultiSource(t *testing.T) { ... }

These tests simulate the real-world scenario where an external tool writes to a field that is supposed to be ignored, and ensure that the controller does not overwrite it.

From the review thread

No notable review discussion on this PR.

Takeaways

• Always normalize data after mutating or removing fields, especially when working with structs that can be empty but non-nil.
• Centralize invariant enforcement (like normalization) in shared utilities to avoid missing edge cases in multiple call sites.
• Write regression tests for subtle bugs involving ignored fields and normalization, especially when external tools may interact with your API in unexpected ways.