Skip to content

Commit 38f73a7

Browse files
pjiang-devpjiang-dev
authored
fix: update normalization for ignoreDifferences on server-side diff (#747)
* WIP fix: update normalization for ignoreDifferences on server-side diff Signed-off-by: Peter Jiang <[email protected]> * fix linter Signed-off-by: Peter Jiang <[email protected]> * refactor Signed-off-by: Peter Jiang <[email protected]> * skip pre-normalization for ssd Signed-off-by: Peter Jiang <[email protected]> * skip pre-normalization for ssd Signed-off-by: Peter Jiang <[email protected]> * docs Signed-off-by: Peter Jiang <[email protected]> * modify Normalize to skip ignoreDifferences conditionally Signed-off-by: Peter Jiang <[email protected]> * ignoreDifferences for SSD and test backwards compatible Signed-off-by: Peter Jiang <[email protected]> * rename flags for clarity Signed-off-by: Peter Jiang <[email protected]> * doc change Signed-off-by: Peter Jiang <[email protected]> --------- Signed-off-by: Peter Jiang <[email protected]>
1 parent d590fe5 commit 38f73a7

File tree

3 files changed

+219
-5
lines changed

3 files changed

+219
-5
lines changed

‎pkg/diff/diff.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,20 @@ func GetNoopNormalizer() Normalizer {
7474
// Diff performs a diff on two unstructured objects. If the live object happens to have a
7575
// "kubectl.kubernetes.io/last-applied-configuration", then perform a three way diff.
7676
func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) {
77+
preDiffOpts := opts
7778
o := applyOptions(opts)
79+
// If server-side diff is enabled, we need to skip full normalization (including ignore differences)
80+
// when pre-processing the config and live objects.
81+
if o.serverSideDiff {
82+
preDiffOpts = append(preDiffOpts, WithSkipFullNormalize(true))
83+
}
7884
if config != nil {
7985
config = remarshal(config, o)
80-
Normalize(config, opts...)
86+
Normalize(config, preDiffOpts...)
8187
}
8288
if live != nil {
8389
live = remarshal(live, o)
84-
Normalize(live, opts...)
90+
Normalize(live, preDiffOpts...)
8591
}
8692

8793
if o.serverSideDiff {
@@ -845,9 +851,14 @@ func Normalize(un *unstructured.Unstructured, opts ...Option) {
845851
normalizeEndpoint(un, o)
846852
}
847853

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

‎pkg/diff/diff_options.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ type options struct {
1717
// If set to true then differences caused by aggregated roles in RBAC resources are ignored.
1818
ignoreAggregatedRoles bool
1919
normalizer Normalizer
20+
skipFullNormalize bool
2021
log logr.Logger
2122
structuredMergeDiff bool
2223
gvkParser *managedfields.GvkParser
@@ -31,6 +32,7 @@ func applyOptions(opts []Option) options {
3132
ignoreAggregatedRoles: false,
3233
ignoreMutationWebhook: true,
3334
normalizer: GetNoopNormalizer(),
35+
skipFullNormalize: false,
3436
log: textlogger.NewLogger(textlogger.NewConfig()),
3537
}
3638
for _, opt := range opts {
@@ -82,6 +84,12 @@ func WithNormalizer(normalizer Normalizer) Option {
8284
}
8385
}
8486

87+
func WithSkipFullNormalize(skip bool) Option {
88+
return func(o *options) {
89+
o.skipFullNormalize = skip
90+
}
91+
}
92+
8593
func WithLogr(log logr.Logger) Option {
8694
return func(o *options) {
8795
o.log = log

‎pkg/diff/diff_test.go

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,11 @@ func TestServerSideDiff(t *testing.T) {
904904
return opts
905905
}
906906

907+
buildOptsWithNormalizer := func(predictedLive string, normalizer Normalizer) []Option {
908+
opts := buildOpts(predictedLive)
909+
return append(opts, WithNormalizer(normalizer))
910+
}
911+
907912
t.Run("will ignore modifications done by mutation webhook by default", func(t *testing.T) {
908913
// given
909914
t.Parallel()
@@ -1061,6 +1066,61 @@ func TestServerSideDiff(t *testing.T) {
10611066
assert.False(t, predictedLabelExists)
10621067
assert.True(t, liveLabelExists)
10631068
})
1069+
1070+
t.Run("will respect ignoreDifferences when full normalization is not skipped", func(t *testing.T) {
1071+
// given
1072+
t.Parallel()
1073+
liveState := StrToUnstructured(testdata.ServiceLiveYAMLSSD)
1074+
desiredState := StrToUnstructured(testdata.ServiceConfigYAMLSSD)
1075+
1076+
// Normalizer that ignores sessionAffinity (auto-assigned field that's commonly ignored)
1077+
normalizer := &testIgnoreDifferencesNormalizer{
1078+
fieldsToRemove: [][]string{
1079+
{"spec", "sessionAffinity"},
1080+
},
1081+
}
1082+
1083+
opts := buildOptsWithNormalizer(testdata.ServicePredictedLiveJSONSSD, normalizer)
1084+
1085+
// when
1086+
result, err := serverSideDiff(desiredState, liveState, opts...)
1087+
1088+
// then
1089+
require.NoError(t, err)
1090+
assert.NotNil(t, result)
1091+
1092+
// Should show diff for other fields but not the ignored sessionAffinity
1093+
assert.True(t, result.Modified, "Should show diff for non-ignored fields")
1094+
1095+
// Convert results to strings for verification
1096+
predictedLiveStr := string(result.PredictedLive)
1097+
normalizedLiveStr := string(result.NormalizedLive)
1098+
1099+
// Ports should appear in diff (not ignored)
1100+
assert.Contains(t, predictedLiveStr, "port", "Port differences should be visible")
1101+
1102+
// The ignored sessionAffinity should NOT appear in final result
1103+
assert.NotContains(t, predictedLiveStr, "sessionAffinity", "sessionAffinity should be removed by normalization")
1104+
assert.NotContains(t, normalizedLiveStr, "sessionAffinity", "sessionAffinity should be removed by normalization")
1105+
1106+
// Other fields should still be visible (not ignored)
1107+
assert.Contains(t, predictedLiveStr, "selector", "Other fields should remain visible")
1108+
})
1109+
}
1110+
1111+
// testIgnoreDifferencesNormalizer implements a simple normalizer that removes specified fields
1112+
type testIgnoreDifferencesNormalizer struct {
1113+
fieldsToRemove [][]string
1114+
}
1115+
1116+
func (n *testIgnoreDifferencesNormalizer) Normalize(un *unstructured.Unstructured) error {
1117+
if un == nil {
1118+
return nil
1119+
}
1120+
for _, fieldPath := range n.fieldsToRemove {
1121+
unstructured.RemoveNestedField(un.Object, fieldPath...)
1122+
}
1123+
return nil
10641124
}
10651125

10661126
func createSecret(data map[string]string) *unstructured.Unstructured {
@@ -1613,3 +1673,138 @@ func StrToUnstructured(yamlStr string) *unstructured.Unstructured {
16131673
}
16141674
return &unstructured.Unstructured{Object: obj}
16151675
}
1676+
1677+
func TestDiffWithIgnoreDifferences(t *testing.T) {
1678+
t.Run("TwoWayDiff will respect ignoreDifferences for comparison but not output normalization", func(t *testing.T) {
1679+
// given
1680+
t.Parallel()
1681+
1682+
// Create a simple service with sessionAffinity that should be ignored
1683+
liveService := StrToUnstructured(`
1684+
apiVersion: v1
1685+
kind: Service
1686+
metadata:
1687+
name: my-service
1688+
spec:
1689+
selector:
1690+
app: my-app
1691+
ports:
1692+
- port: 80
1693+
sessionAffinity: None
1694+
type: ClusterIP
1695+
`)
1696+
1697+
desiredService := StrToUnstructured(`
1698+
apiVersion: v1
1699+
kind: Service
1700+
metadata:
1701+
name: my-service
1702+
spec:
1703+
selector:
1704+
app: my-app
1705+
ports:
1706+
- port: 80
1707+
sessionAffinity: ClientIP
1708+
type: ClusterIP
1709+
`)
1710+
1711+
// Normalizer that ignores sessionAffinity
1712+
normalizer := &testIgnoreDifferencesNormalizer{
1713+
fieldsToRemove: [][]string{
1714+
{"spec", "sessionAffinity"},
1715+
},
1716+
}
1717+
1718+
opts := []Option{
1719+
WithNormalizer(normalizer),
1720+
WithLogr(textlogger.NewLogger(textlogger.NewConfig())),
1721+
}
1722+
1723+
// when
1724+
result, err := Diff(desiredService, liveService, opts...)
1725+
require.NoError(t, err)
1726+
1727+
// then
1728+
assert.NotNil(t, result)
1729+
1730+
// Since sessionAffinity is ignored in input normalization, there should be no modification
1731+
assert.False(t, result.Modified, "Should not show diff for ignored fields")
1732+
1733+
predictedLiveStr := string(result.PredictedLive)
1734+
normalizedLiveStr := string(result.NormalizedLive)
1735+
1736+
// NOTE: Unlike server-side diff, TwoWayDiff/ThreeWayDiff don't normalize outputs
1737+
// So sessionAffinity WILL still appear in the output bytes, but Modified should be false
1738+
// because input normalization removed the differences during comparison
1739+
assert.Contains(t, predictedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
1740+
assert.Contains(t, normalizedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
1741+
})
1742+
1743+
t.Run("ThreeWayDiff will respect ignoreDifferences for comparison but not output normalization", func(t *testing.T) {
1744+
// given
1745+
t.Parallel()
1746+
1747+
// Create config and live with sessionAffinity differences that should be ignored
1748+
configService := StrToUnstructured(`
1749+
apiVersion: v1
1750+
kind: Service
1751+
metadata:
1752+
name: my-service
1753+
spec:
1754+
selector:
1755+
app: my-app
1756+
ports:
1757+
- port: 80
1758+
sessionAffinity: ClientIP
1759+
type: ClusterIP
1760+
`)
1761+
1762+
liveService := StrToUnstructured(`
1763+
apiVersion: v1
1764+
kind: Service
1765+
metadata:
1766+
name: my-service
1767+
annotations:
1768+
kubectl.kubernetes.io/last-applied-configuration: |
1769+
{"apiVersion":"v1","kind":"Service","metadata":{"name":"my-service"},"spec":{"selector":{"app":"my-app"},"ports":[{"port":80}],"sessionAffinity":"None","type":"ClusterIP"}}
1770+
spec:
1771+
selector:
1772+
app: my-app
1773+
ports:
1774+
- port: 80
1775+
sessionAffinity: None
1776+
type: ClusterIP
1777+
`)
1778+
1779+
// Normalizer that ignores sessionAffinity
1780+
normalizer := &testIgnoreDifferencesNormalizer{
1781+
fieldsToRemove: [][]string{
1782+
{"spec", "sessionAffinity"},
1783+
},
1784+
}
1785+
1786+
opts := []Option{
1787+
WithNormalizer(normalizer),
1788+
WithLogr(textlogger.NewLogger(textlogger.NewConfig())),
1789+
}
1790+
1791+
// when
1792+
result, err := Diff(configService, liveService, opts...)
1793+
require.NoError(t, err)
1794+
1795+
// then
1796+
assert.NotNil(t, result)
1797+
1798+
// Since sessionAffinity is ignored in input normalization, there should be no modification
1799+
assert.False(t, result.Modified, "Should not show diff for ignored fields")
1800+
1801+
predictedLiveStr := string(result.PredictedLive)
1802+
normalizedLiveStr := string(result.NormalizedLive)
1803+
1804+
// NOTE: Unlike server-side diff, TwoWayDiff/ThreeWayDiff don't normalize outputs
1805+
// So sessionAffinity WILL still appear in the output bytes, but Modified should be false
1806+
// because input normalization removed the differences during comparison
1807+
assert.Contains(t, predictedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
1808+
assert.Contains(t, normalizedLiveStr, "sessionAffinity", "sessionAffinity should still appear in output (no output normalization)")
1809+
})
1810+
}

0 commit comments

Comments
 (0)