Skip to content

fix: update normalization for ignoreDifferences on server-side diff #747

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.

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

Merged
merged 10 commits into from
Jul 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 16 additions & 5 deletions pkg/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,20 @@ func GetNoopNormalizer() Normalizer {
// Diff performs a diff on two unstructured objects. If the live object happens to have a
// "kubectl.kubernetes.io/last-applied-configuration", then perform a three way diff.
func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) {
preDiffOpts := opts
o := applyOptions(opts)
// If server-side diff is enabled, we need to skip full normalization (including ignore differences)
// when pre-processing the config and live objects.
if o.serverSideDiff {
preDiffOpts = append(preDiffOpts, WithSkipFullNormalize(true))
}
if config != nil {
config = remarshal(config, o)
Normalize(config, opts...)
Normalize(config, preDiffOpts...)
}
if live != nil {
live = remarshal(live, o)
Normalize(live, opts...)
Normalize(live, preDiffOpts...)
}

if o.serverSideDiff {
Expand Down Expand Up @@ -845,9 +851,14 @@ func Normalize(un *unstructured.Unstructured, opts ...Option) {
normalizeEndpoint(un, o)
}

err := o.normalizer.Normalize(un)
if err != nil {
o.log.Error(err, fmt.Sprintf("Failed to normalize %s/%s/%s", un.GroupVersionKind(), un.GetNamespace(), un.GetName()))
// Skip the full normalization (ignoreDifferences + knownTypes) for server-side diff
// In the case an ignoreDifferences field is required, it needs to be present in the config
// before server-side diff is calculated and normalized before final comparison.
if !o.skipFullNormalize {
err := o.normalizer.Normalize(un)
if err != nil {
o.log.Error(err, fmt.Sprintf("Failed to normalize %s/%s/%s", un.GroupVersionKind(), un.GetNamespace(), un.GetName()))
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/diff/diff_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type options struct {
// If set to true then differences caused by aggregated roles in RBAC resources are ignored.
ignoreAggregatedRoles bool
normalizer Normalizer
skipFullNormalize bool
log logr.Logger
structuredMergeDiff bool
gvkParser *managedfields.GvkParser
Expand All @@ -31,6 +32,7 @@ func applyOptions(opts []Option) options {
ignoreAggregatedRoles: false,
ignoreMutationWebhook: true,
normalizer: GetNoopNormalizer(),
skipFullNormalize: false,
log: textlogger.NewLogger(textlogger.NewConfig()),
}
for _, opt := range opts {
Expand Down Expand Up @@ -82,6 +84,12 @@ func WithNormalizer(normalizer Normalizer) Option {
}
}

func WithSkipFullNormalize(skip bool) Option {
return func(o *options) {
o.skipFullNormalize = skip
}
}

func WithLogr(log logr.Logger) Option {
return func(o *options) {
o.log = log
Expand Down
195 changes: 195 additions & 0 deletions pkg/diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,11 @@ func TestServerSideDiff(t *testing.T) {
return opts
}

buildOptsWithNormalizer := func(predictedLive string, normalizer Normalizer) []Option {
opts := buildOpts(predictedLive)
return append(opts, WithNormalizer(normalizer))
}

t.Run("will ignore modifications done by mutation webhook by default", func(t *testing.T) {
// given
t.Parallel()
Expand Down Expand Up @@ -1061,6 +1066,61 @@ func TestServerSideDiff(t *testing.T) {
assert.False(t, predictedLabelExists)
assert.True(t, liveLabelExists)
})

t.Run("will respect ignoreDifferences when full normalization is not skipped", func(t *testing.T) {
// given
t.Parallel()
liveState := StrToUnstructured(testdata.ServiceLiveYAMLSSD)
desiredState := StrToUnstructured(testdata.ServiceConfigYAMLSSD)

// Normalizer that ignores sessionAffinity (auto-assigned field that's commonly ignored)
normalizer := &testIgnoreDifferencesNormalizer{
fieldsToRemove: [][]string{
{"spec", "sessionAffinity"},
},
}

opts := buildOptsWithNormalizer(testdata.ServicePredictedLiveJSONSSD, normalizer)

// when
result, err := serverSideDiff(desiredState, liveState, opts...)

// then
require.NoError(t, err)
assert.NotNil(t, result)

// Should show diff for other fields but not the ignored sessionAffinity
assert.True(t, result.Modified, "Should show diff for non-ignored fields")

// Convert results to strings for verification
predictedLiveStr := string(result.PredictedLive)
normalizedLiveStr := string(result.NormalizedLive)

// Ports should appear in diff (not ignored)
assert.Contains(t, predictedLiveStr, "port", "Port differences should be visible")

// The ignored sessionAffinity should NOT appear in final result
assert.NotContains(t, predictedLiveStr, "sessionAffinity", "sessionAffinity should be removed by normalization")
assert.NotContains(t, normalizedLiveStr, "sessionAffinity", "sessionAffinity should be removed by normalization")

// Other fields should still be visible (not ignored)
assert.Contains(t, predictedLiveStr, "selector", "Other fields should remain visible")
})
}

// testIgnoreDifferencesNormalizer implements a simple normalizer that removes specified fields
type testIgnoreDifferencesNormalizer struct {
fieldsToRemove [][]string
}

func (n *testIgnoreDifferencesNormalizer) Normalize(un *unstructured.Unstructured) error {
if un == nil {
return nil
}
for _, fieldPath := range n.fieldsToRemove {
unstructured.RemoveNestedField(un.Object, fieldPath...)
}
return nil
}

func createSecret(data map[string]string) *unstructured.Unstructured {
Expand Down Expand Up @@ -1613,3 +1673,138 @@ func StrToUnstructured(yamlStr string) *unstructured.Unstructured {
}
return &unstructured.Unstructured{Object: obj}
}

func TestDiffWithIgnoreDifferences(t *testing.T) {
t.Run("TwoWayDiff will respect ignoreDifferences for comparison but not output normalization", func(t *testing.T) {
// given
t.Parallel()

// Create a simple service with sessionAffinity that should be ignored
liveService := StrToUnstructured(`
apiVersion: v1
kind: Service
metadata:
name: my-service
spec:
selector:
app: my-app
ports:
- port: 80
sessionAffinity: None
type: ClusterIP
`)

desiredService := StrToUnstructured(`
apiVersion: v1
kind: Service
metadata:
name: my-service
spec:
selector:
app: my-app
ports:
- port: 80
sessionAffinity: ClientIP
type: ClusterIP
`)

// Normalizer that ignores sessionAffinity
normalizer := &testIgnoreDifferencesNormalizer{
fieldsToRemove: [][]string{
{"spec", "sessionAffinity"},
},
}

opts := []Option{
WithNormalizer(normalizer),
WithLogr(textlogger.NewLogger(textlogger.NewConfig())),
}

// when
result, err := Diff(desiredService, liveService, opts...)
require.NoError(t, err)

// then
assert.NotNil(t, result)

// Since sessionAffinity is ignored in input normalization, there should be no modification
assert.False(t, result.Modified, "Should not show diff for ignored fields")

predictedLiveStr := string(result.PredictedLive)
normalizedLiveStr := string(result.NormalizedLive)

// NOTE: Unlike server-side diff, TwoWayDiff/ThreeWayDiff don't normalize outputs
// So sessionAffinity WILL still appear in the output bytes, but Modified should be false
// because input normalization removed the differences during comparison
assert.Contains(t, predictedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
assert.Contains(t, normalizedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
})

t.Run("ThreeWayDiff will respect ignoreDifferences for comparison but not output normalization", func(t *testing.T) {
// given
t.Parallel()

// Create config and live with sessionAffinity differences that should be ignored
configService := StrToUnstructured(`
apiVersion: v1
kind: Service
metadata:
name: my-service
spec:
selector:
app: my-app
ports:
- port: 80
sessionAffinity: ClientIP
type: ClusterIP
`)

liveService := StrToUnstructured(`
apiVersion: v1
kind: Service
metadata:
name: my-service
annotations:
kubectl.kubernetes.io/last-applied-configuration: |
{"apiVersion":"v1","kind":"Service","metadata":{"name":"my-service"},"spec":{"selector":{"app":"my-app"},"ports":[{"port":80}],"sessionAffinity":"None","type":"ClusterIP"}}
spec:
selector:
app: my-app
ports:
- port: 80
sessionAffinity: None
type: ClusterIP
`)

// Normalizer that ignores sessionAffinity
normalizer := &testIgnoreDifferencesNormalizer{
fieldsToRemove: [][]string{
{"spec", "sessionAffinity"},
},
}

opts := []Option{
WithNormalizer(normalizer),
WithLogr(textlogger.NewLogger(textlogger.NewConfig())),
}

// when
result, err := Diff(configService, liveService, opts...)
require.NoError(t, err)

// then
assert.NotNil(t, result)

// Since sessionAffinity is ignored in input normalization, there should be no modification
assert.False(t, result.Modified, "Should not show diff for ignored fields")

predictedLiveStr := string(result.PredictedLive)
normalizedLiveStr := string(result.NormalizedLive)

// NOTE: Unlike server-side diff, TwoWayDiff/ThreeWayDiff don't normalize outputs
// So sessionAffinity WILL still appear in the output bytes, but Modified should be false
// because input normalization removed the differences during comparison
assert.Contains(t, predictedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
assert.Contains(t, normalizedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
})
}