Skip to content

Commit 0f703a7

Browse files
committed
filter /status override from target normalization guard
1 parent 102d9f8 commit 0f703a7

2 files changed

Lines changed: 36 additions & 43 deletions

File tree

controller/sync.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ func normalizeTargetResources(cr *comparisonResult) ([]*unstructured.Unstructure
424424
if err != nil {
425425
return nil, err
426426
}
427-
idc := diff.NewIgnoreDiffConfig(cr.diffConfig.Ignores(), cr.diffConfig.Overrides())
427+
idc := diff.NewIgnoreDiffConfig(cr.diffConfig.Ignores(), filterOverridesForTargetNormalization(cr.diffConfig.Overrides()))
428428
patchedTargets := []*unstructured.Unstructured{}
429429
for idx, live := range cr.reconciliationResult.Live {
430430
normalizedTarget := normalized.Targets[idx]
@@ -477,6 +477,29 @@ func normalizeTargetResources(cr *comparisonResult) ([]*unstructured.Unstructure
477477
return patchedTargets, nil
478478
}
479479

480+
// filterOverridesForTargetNormalization removes /status from override ignore
481+
// differences because status fields are irrelevant to target normalization.
482+
// Status is managed by controllers, not present in desired state from git,
483+
// and stripped by the API server on apply. Without this filter, the default
484+
// */* /status override causes HasIgnoreDifference to match every resource,
485+
// defeating the per-resource skip logic.
486+
func filterOverridesForTargetNormalization(overrides map[string]v1alpha1.ResourceOverride) map[string]v1alpha1.ResourceOverride {
487+
filtered := make(map[string]v1alpha1.ResourceOverride, len(overrides))
488+
for key, override := range overrides {
489+
var pointers []string
490+
for _, p := range override.IgnoreDifferences.JSONPointers {
491+
if p != "/status" {
492+
pointers = append(pointers, p)
493+
}
494+
}
495+
if len(pointers) > 0 || len(override.IgnoreDifferences.JQPathExpressions) > 0 || len(override.IgnoreDifferences.ManagedFieldsManagers) > 0 {
496+
override.IgnoreDifferences.JSONPointers = pointers
497+
filtered[key] = override
498+
}
499+
}
500+
return filtered
501+
}
502+
480503
// getMergePatch calculates and returns the patch between the original and the
481504
// modified unstructures.
482505
func getMergePatch(original, modified *unstructured.Unstructured, lookupPatchMeta *strategicpatch.PatchMetaFromStruct) ([]byte, error) {

controller/sync_test.go

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -486,14 +486,24 @@ func TestNormalizeTargetResources(t *testing.T) {
486486
})
487487
}
488488

489+
// defaultOverrides returns the resource overrides that the controller always
490+
// injects via GetResourceOverrides(). By default, status is ignored for all
491+
// resources ("*/*"), which adds a wildcard entry that HasIgnoreDifference
492+
// matches for every resource.
493+
func defaultOverrides() map[string]v1alpha1.ResourceOverride {
494+
return map[string]v1alpha1.ResourceOverride{
495+
"*/*": {IgnoreDifferences: v1alpha1.OverrideIgnoreDiff{JSONPointers: []string{"/status"}}},
496+
}
497+
}
498+
489499
func TestNormalizeTargetResourcesPDB(t *testing.T) {
490500
type fixture struct {
491501
comparisonResult *comparisonResult
492502
}
493503
setup := func(t *testing.T, ignores []v1alpha1.ResourceIgnoreDifferences) *fixture {
494504
t.Helper()
495505
dc, err := diff.NewDiffConfigBuilder().
496-
WithDiffSettings(ignores, nil, true, normalizers.IgnoreNormalizerOpts{}).
506+
WithDiffSettings(ignores, defaultOverrides(), true, normalizers.IgnoreNormalizerOpts{}).
497507
WithNoCache().
498508
Build()
499509
require.NoError(t, err)
@@ -582,7 +592,7 @@ func TestNormalizeTargetResourcesPDB(t *testing.T) {
582592
},
583593
}
584594
dc, err := diff.NewDiffConfigBuilder().
585-
WithDiffSettings(ignores, nil, true, normalizers.IgnoreNormalizerOpts{}).
595+
WithDiffSettings(ignores, defaultOverrides(), true, normalizers.IgnoreNormalizerOpts{}).
586596
WithNoCache().
587597
Build()
588598
require.NoError(t, err)
@@ -648,46 +658,6 @@ func TestNormalizeTargetResourcesPDB(t *testing.T) {
648658
})
649659
}
650660

651-
func TestNormalizeTargetResourcesCRD(t *testing.T) {
652-
// CRDs are not in the k8s scheme, so normalizeTargetResources falls back
653-
// to JSON merge patch which replaces arrays wholesale. Without matching
654-
// ignore rules the merge patch must be skipped entirely.
655-
setup := func(t *testing.T, ignores []v1alpha1.ResourceIgnoreDifferences) *comparisonResult {
656-
t.Helper()
657-
dc, err := diff.NewDiffConfigBuilder().
658-
WithDiffSettings(ignores, nil, true, normalizers.IgnoreNormalizerOpts{}).
659-
WithNoCache().
660-
Build()
661-
require.NoError(t, err)
662-
live := test.YamlToUnstructured(testdata.LiveHTTPProxy)
663-
target := test.YamlToUnstructured(testdata.TargetHTTPProxy)
664-
return &comparisonResult{
665-
reconciliationResult: sync.ReconciliationResult{
666-
Live: []*unstructured.Unstructured{live},
667-
Target: []*unstructured.Unstructured{target},
668-
},
669-
diffConfig: dc,
670-
}
671-
}
672-
t.Run("will not corrupt CRD arrays when no ignore rules match", func(t *testing.T) {
673-
cr := setup(t, []v1alpha1.ResourceIgnoreDifferences{})
674-
675-
targets, err := normalizeTargetResources(cr)
676-
677-
require.NoError(t, err)
678-
require.Len(t, targets, 1)
679-
// Target HTTPProxy has 2 descriptors; live has 1.
680-
// Without fix the JSON merge patch would replace the array with live's single entry.
681-
descriptors, ok, err := unstructured.NestedSlice(targets[0].Object, "spec", "routes")
682-
require.NoError(t, err)
683-
require.True(t, ok)
684-
route := descriptors[0].(map[string]any)
685-
global := route["rateLimitPolicy"].(map[string]any)["global"].(map[string]any)
686-
descs := global["descriptors"].([]any)
687-
assert.Len(t, descs, 2, "target should retain both descriptors, not be overwritten with live's single entry")
688-
})
689-
}
690-
691661
func TestNormalizeTargetResourcesWithList(t *testing.T) {
692662
type fixture struct {
693663
comparisonResult *comparisonResult

0 commit comments

Comments
 (0)