Skip to content

fix: Server-Side diff removed fields missing in diff #722

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
16 changes: 16 additions & 0 deletions pkg/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,28 @@ func removeWebhookMutation(predictedLive, live *unstructured.Unstructured, gvkPa
// Remove fields from predicted live that are not managed by the provided manager
nonArgoFieldsSet := predictedLiveFieldSet.Difference(managerFieldsSet)

// Compare the predicted live with the live resource
comparison, err := typedLive.Compare(typedPredictedLive)
if err != nil {
return nil, fmt.Errorf("error comparing predicted resource to live resource: %w", err)
}

if comparison.Removed != nil && !comparison.Removed.Empty() {
// exclude the removed fields not owned by this manager from the comparison
comparison.Removed = comparison.Removed.Difference(nonArgoFieldsSet)
}

// In case any of the removed fields cause schema violations, we will keep those fields
nonArgoFieldsSet = safelyRemoveFieldsSet(typedPredictedLive, nonArgoFieldsSet)
typedPredictedLive = typedPredictedLive.RemoveItems(nonArgoFieldsSet)

// Apply the predicted live state to the live state to get a diff without mutation webhook fields
typedPredictedLive, err = typedLive.Merge(typedPredictedLive)

// After applying the predicted live to live state, this would cause any removed fields to be restored.
// We need to re-remove these from predicted live.
typedPredictedLive = typedPredictedLive.RemoveItems(comparison.Removed)

if err != nil {
return nil, fmt.Errorf("error applying predicted live to live state: %w", err)
}
Expand Down
25 changes: 25 additions & 0 deletions pkg/diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,31 @@ func TestServerSideDiff(t *testing.T) {
assert.Empty(t, predictedDeploy.Annotations[AnnotationLastAppliedConfig])
assert.Empty(t, liveDeploy.Annotations[AnnotationLastAppliedConfig])
})

t.Run("will reflect deletion of labels in predicted live", func(t *testing.T) {
// given
t.Parallel()
liveState := StrToUnstructured(testdata.ServiceLiveLabelYAMLSSD)
desiredState := StrToUnstructured(testdata.ServiceConfigNoLabelYAMLSSD)
opts := buildOpts(testdata.ServicePredictedLiveNoLabelJSONSSD)

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

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

predictedSvc := YamlToSvc(t, result.PredictedLive)
liveSvc := YamlToSvc(t, result.NormalizedLive)

// Ensure that the deleted label is not present in predicted and exists in live
_, predictedLabelExists := predictedSvc.Labels["delete-me"]
_, liveLabelExists := liveSvc.Labels["delete-me"]
assert.False(t, predictedLabelExists)
assert.True(t, liveLabelExists)
})
}

func createSecret(data map[string]string) *unstructured.Unstructured {
Expand Down
9 changes: 9 additions & 0 deletions pkg/diff/testdata/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,13 @@ var (

//go:embed ssd-deploy-with-manual-apply-predicted-live.json
DeploymentApplyPredictedLiveJSONSSD string

//go:embed ssd-svc-label-live.yaml
ServiceLiveLabelYAMLSSD string

//go:embed ssd-svc-no-label-config.yaml
ServiceConfigNoLabelYAMLSSD string

//go:embed ssd-svc-no-label-predicted-live.json
ServicePredictedLiveNoLabelJSONSSD string
)
50 changes: 50 additions & 0 deletions pkg/diff/testdata/ssd-svc-label-live.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
apiVersion: v1
kind: Service
metadata:
creationTimestamp: "2025-05-16T19:01:22Z"
labels:
app.kubernetes.io/instance: httpbin
delete-me: delete-value
managedFields:
- apiVersion: v1
fieldsType: FieldsV1
fieldsV1:
f:metadata:
f:labels:
f:app.kubernetes.io/instance: {}
f:delete-me: {}
f:spec:
f:ports:
k:{"port":7777,"protocol":"TCP"}:
.: {}
f:name: {}
f:port: {}
f:protocol: {}
f:targetPort: {}
f:selector: {}
manager: argocd-controller
operation: Apply
time: "2025-05-16T19:01:22Z"
name: httpbin-svc
namespace: httpbin
resourceVersion: "159005"
uid: 61a7a0c2-d973-4333-bbd6-c06ba1c00190
spec:
clusterIP: 10.96.59.144
clusterIPs:
- 10.96.59.144
internalTrafficPolicy: Cluster
ipFamilies:
- IPv4
ipFamilyPolicy: SingleStack
ports:
- name: http-port
port: 7777
protocol: TCP
targetPort: 80
selector:
app: httpbin
sessionAffinity: None
type: ClusterIP
status:
loadBalancer: {}
15 changes: 15 additions & 0 deletions pkg/diff/testdata/ssd-svc-no-label-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: v1
kind: Service
metadata:
labels:
app.kubernetes.io/instance: httpbin
name: httpbin-svc
namespace: httpbin
spec:
ports:
- name: http-port
port: 7777
protocol: TCP
targetPort: 80
selector:
app: httpbin
69 changes: 69 additions & 0 deletions pkg/diff/testdata/ssd-svc-no-label-predicted-live.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
{
"apiVersion": "v1",
"kind": "Service",
"metadata": {
"creationTimestamp": "2025-05-16T19:01:22Z",
"labels": {
"app.kubernetes.io/instance": "httpbin"
},
"managedFields": [
{
"apiVersion": "v1",
"fieldsType": "FieldsV1",
"fieldsV1": {
"f:metadata": {
"f:labels": {
"f:app.kubernetes.io/instance": {}
}
},
"f:spec": {
"f:ports": {
"k:{\"port\":7777,\"protocol\":\"TCP\"}": {
".": {},
"f:name": {},
"f:port": {},
"f:protocol": {},
"f:targetPort": {}
}
},
"f:selector": {}
}
},
"manager": "argocd-controller",
"operation": "Apply",
"time": "2025-05-16T19:02:57Z"
}
],
"name": "httpbin-svc",
"namespace": "httpbin",
"resourceVersion": "159005",
"uid": "61a7a0c2-d973-4333-bbd6-c06ba1c00190"
},
"spec": {
"clusterIP": "10.96.59.144",
"clusterIPs": [
"10.96.59.144"
],
"internalTrafficPolicy": "Cluster",
"ipFamilies": [
"IPv4"
],
"ipFamilyPolicy": "SingleStack",
"ports": [
{
"name": "http-port",
"port": 7777,
"protocol": "TCP",
"targetPort": 80
}
],
"selector": {
"app": "httpbin"
},
"sessionAffinity": "None",
"type": "ClusterIP"
},
"status": {
"loadBalancer": {}
}
}