fix: remove last-applied-configuration before diff in ssa (#460)

* fix: remove last-apply-configurations before diff in ssa Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * fix: add tests to validate expected behaviour Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
2026-03-04 07:28:47 +01:00 · 2022-09-16 10:22:00 -04:00
parent 517c1fff4e
commit 3951079de1
5 changed files with 26 additions and 234 deletions
--- a/pkg/diff/diff.go
+++ b/pkg/diff/diff.go
@@ -29,7 +29,10 @@ import (
 	kubescheme "github.com/argoproj/gitops-engine/pkg/utils/kube/scheme"
 )

-const couldNotMarshalErrMsg = "Could not unmarshal to object of type %s: %v"
+const (
+	couldNotMarshalErrMsg       = "Could not unmarshal to object of type %s: %v"
+	AnnotationLastAppliedConfig = "kubectl.kubernetes.io/last-applied-configuration"
+)

 // Holds diffing result of two resources
 type DiffResult struct {
@@ -173,13 +176,13 @@ func structuredMergeDiff(config, live *unstructured.Unstructured, pt *typed.Pars
 	}

 	// 6) Apply default values in predicted live
-	predictedLive, err := applyDefaultValues(mergedCleanLive)
+	predictedLive, err := normalizeTypedValue(mergedCleanLive)
 	if err != nil {
 		return nil, fmt.Errorf("error applying default values in predicted live: %w", err)
 	}

 	// 7) Apply default values in live
-	taintedLive, err := applyDefaultValues(tvLive)
+	taintedLive, err := normalizeTypedValue(tvLive)
 	if err != nil {
 		return nil, fmt.Errorf("error applying default values in live: %w", err)
 	}
@@ -187,30 +190,35 @@ func structuredMergeDiff(config, live *unstructured.Unstructured, pt *typed.Pars
 	return buildDiffResult(predictedLive, taintedLive), nil
 }

-func applyDefaultValues(result *typed.TypedValue) ([]byte, error) {
-	ru := result.AsValue().Unstructured()
+// normalizeTypedValue will prepare the given tv so it can be used in diffs by:
+// - removing last-applied-configuration annotation
+// - applying default values
+func normalizeTypedValue(tv *typed.TypedValue) ([]byte, error) {
+	ru := tv.AsValue().Unstructured()
 	r, ok := ru.(map[string]interface{})
 	if !ok {
 		return nil, fmt.Errorf("error converting result typedValue: expected map got %T", ru)
 	}
-	mergedUnstructured := &unstructured.Unstructured{Object: r}
-	mergedBytes, err := json.Marshal(mergedUnstructured)
+	resultUn := &unstructured.Unstructured{Object: r}
+	unstructured.RemoveNestedField(resultUn.Object, "metadata", "annotations", AnnotationLastAppliedConfig)
+
+	resultBytes, err := json.Marshal(resultUn)
 	if err != nil {
 		return nil, fmt.Errorf("error while marshaling merged unstructured: %w", err)
 	}

-	obj, err := scheme.Scheme.New(mergedUnstructured.GroupVersionKind())
+	obj, err := scheme.Scheme.New(resultUn.GroupVersionKind())
 	if err == nil {
-		err := json.Unmarshal(mergedBytes, &obj)
+		err := json.Unmarshal(resultBytes, &obj)
 		if err != nil {
 			return nil, fmt.Errorf("error unmarshaling merged bytes into object: %w", err)
 		}
-		mergedBytes, err = patchDefaultValues(mergedBytes, obj)
+		resultBytes, err = patchDefaultValues(resultBytes, obj)
 		if err != nil {
 			return nil, fmt.Errorf("error applying defaults: %w", err)
 		}
 	}
-	return mergedBytes, nil
+	return resultBytes, nil
 }

 func buildDiffResult(predictedBytes []byte, liveBytes []byte) *DiffResult {
--- a/pkg/diff/diff_test.go
+++ b/pkg/diff/diff_test.go
@@ -770,14 +770,13 @@ func TestStructuredMergeDiff(t *testing.T) {
 		assert.NotNil(t, liveSVC.Spec.InternalTrafficPolicy)
 		assert.Equal(t, "Cluster", string(*predictedSVC.Spec.InternalTrafficPolicy))
 		assert.Equal(t, "Cluster", string(*liveSVC.Spec.InternalTrafficPolicy))
+		assert.Empty(t, predictedSVC.Annotations[AnnotationLastAppliedConfig])
+		assert.Empty(t, liveSVC.Annotations[AnnotationLastAppliedConfig])
 	})
 	t.Run("will remove entries in list", func(t *testing.T) {
 		// given
 		liveState := StrToUnstructured(testdata.ServiceLiveYAML)
 		desiredState := StrToUnstructured(testdata.ServiceConfigWith2Ports)
-		expectedLiveState := StrToUnstructured(testdata.ExpectedServiceLiveWith2PortsYAML)
-		expectedLiveBytes, err := json.Marshal(expectedLiveState)
-		require.NoError(t, err)

 		// when
 		result, err := structuredMergeDiff(desiredState, liveState, &svcParseType, manager)
@@ -786,15 +785,13 @@ func TestStructuredMergeDiff(t *testing.T) {
 		require.NoError(t, err)
 		assert.NotNil(t, result)
 		assert.True(t, result.Modified)
-		assert.Equal(t, string(expectedLiveBytes), string(result.PredictedLive))
+		svc := YamlToSvc(t, result.PredictedLive)
+		assert.Len(t, svc.Spec.Ports, 2)
 	})
 	t.Run("will remove previously added fields not present in desired state", func(t *testing.T) {
 		// given
 		liveState := StrToUnstructured(testdata.LiveServiceWithTypeYAML)
 		desiredState := StrToUnstructured(testdata.ServiceConfigYAML)
-		expectedLiveState := StrToUnstructured(testdata.ServiceLiveNoTypeYAML)
-		expectedLiveBytes, err := json.Marshal(expectedLiveState)
-		require.NoError(t, err)

 		// when
 		result, err := structuredMergeDiff(desiredState, liveState, &svcParseType, manager)
@@ -803,7 +800,8 @@ func TestStructuredMergeDiff(t *testing.T) {
 		require.NoError(t, err)
 		assert.NotNil(t, result)
 		assert.True(t, result.Modified)
-		assert.Equal(t, string(expectedLiveBytes), string(result.PredictedLive))
+		svc := YamlToSvc(t, result.PredictedLive)
+		assert.Equal(t, corev1.ServiceTypeClusterIP, svc.Spec.Type)
 	})
 	t.Run("will apply service with multiple ports", func(t *testing.T) {
 		// given
@@ -818,7 +816,7 @@ func TestStructuredMergeDiff(t *testing.T) {
 		assert.NotNil(t, result)
 		assert.True(t, result.Modified)
 		svc := YamlToSvc(t, result.PredictedLive)
-		assert.Equal(t, 5, len(svc.Spec.Ports))
+		assert.Len(t, svc.Spec.Ports, 5)
 	})
 }

--- a/pkg/diff/testdata/data.go
+++ b/pkg/diff/testdata/data.go
@@ -9,15 +9,9 @@ var (
 	//go:embed smd-service-live.yaml
 	ServiceLiveYAML string

-	//go:embed smd-service-live-no-type.yaml
-	ServiceLiveNoTypeYAML string
-
 	//go:embed smd-service-config-2-ports.yaml
 	ServiceConfigWith2Ports string

-	//go:embed smd-service-live-expected-2-ports.yaml
-	ExpectedServiceLiveWith2PortsYAML string
-
 	//go:embed smd-service-live-with-type.yaml
 	LiveServiceWithTypeYAML string

--- a/pkg/diff/testdata/smd-service-live-expected-2-ports.yaml
+++ b/pkg/diff/testdata/smd-service-live-expected-2-ports.yaml
@@ -1,100 +0,0 @@
-apiVersion: v1
-kind: Service
-metadata:
-  annotations:
-    argocd.argoproj.io/sync-options: ServerSideApply=true
-    kubectl.kubernetes.io/last-applied-configuration: >
-      {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"argocd.argoproj.io/sync-options":"ServerSideApply=true"},"name":"multiple-protocol-port-svc","namespace":"default"},"spec":{"ports":[{"name":"rtmpk","port":1986,"protocol":"UDP","targetPort":1986},{"name":"rtmp","port":1935,"targetPort":1935},{"name":"https","port":443,"targetPort":443}]}}
-  creationTimestamp: '2022-06-24T19:37:02Z'
-  labels:
-    app.kubernetes.io/instance: big-crd
-  managedFields:
-    - apiVersion: v1
-      fieldsType: FieldsV1
-      fieldsV1:
-        'f:metadata':
-          'f:annotations':
-            'f:argocd.argoproj.io/sync-options': {}
-          'f:labels':
-            'f:app.kubernetes.io/instance': {}
-        'f:spec':
-          'f:ports':
-            'k:{"port":1935,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:targetPort': {}
-            'k:{"port":1986,"protocol":"UDP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-            'k:{"port":443,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:targetPort': {}
-      manager: argocd-controller
-      operation: Apply
-      time: '2022-06-24T19:45:02Z'
-    - apiVersion: v1
-      fieldsType: FieldsV1
-      fieldsV1:
-        'f:metadata':
-          'f:annotations':
-            .: {}
-            'f:argocd.argoproj.io/sync-options': {}
-            'f:kubectl.kubernetes.io/last-applied-configuration': {}
-        'f:spec':
-          'f:internalTrafficPolicy': {}
-          'f:ports':
-            .: {}
-            'k:{"port":1935,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-            'k:{"port":1986,"protocol":"UDP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-            'k:{"port":443,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-          'f:sessionAffinity': {}
-          'f:type': {}
-      manager: kubectl-client-side-apply
-      operation: Update
-      time: '2022-06-24T19:37:02Z'
-  name: multiple-protocol-port-svc
-  namespace: default
-  resourceVersion: '1825080'
-  uid: af42e800-bd33-4412-bc77-d204d298613d
-spec:
-  clusterIP: 10.111.193.74
-  clusterIPs:
-    - 10.111.193.74
-  internalTrafficPolicy: Cluster
-  ipFamilies:
-    - IPv4
-  ipFamilyPolicy: SingleStack
-  ports:
-    - name: rtmpk
-      port: 1986
-      protocol: UDP
-      targetPort: 1986
-    - name: rtmp
-      port: 1935
-      protocol: TCP
-      targetPort: 1935
-  sessionAffinity: None
-  type: ClusterIP
-status:
-  loadBalancer: {}
--- a/pkg/diff/testdata/smd-service-live-no-type.yaml
+++ b/pkg/diff/testdata/smd-service-live-no-type.yaml
@@ -1,108 +0,0 @@
-apiVersion: v1
-kind: Service
-metadata:
-  annotations:
-    argocd.argoproj.io/sync-options: ServerSideApply=true
-    kubectl.kubernetes.io/last-applied-configuration: >
-      {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"argocd.argoproj.io/sync-options":"ServerSideApply=true"},"name":"multiple-protocol-port-svc","namespace":"default"},"spec":{"ports":[{"name":"rtmpk","port":1986,"protocol":"UDP","targetPort":1986},{"name":"rtmp","port":1935,"protocol":"TCP","targetPort":1935},{"name":"rtmpq","port":1935,"protocol":"UDP","targetPort":1935}]}}
-  creationTimestamp: '2022-06-24T19:37:02Z'
-  labels:
-    app.kubernetes.io/instance: big-crd
-  managedFields:
-    - apiVersion: v1
-      fieldsType: FieldsV1
-      fieldsV1:
-        'f:metadata':
-          'f:annotations':
-            'f:argocd.argoproj.io/sync-options': {}
-          'f:labels':
-            'f:app.kubernetes.io/instance': {}
-        'f:spec':
-          'f:ports':
-            'k:{"port":1935,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:targetPort': {}
-            'k:{"port":1986,"protocol":"UDP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-            'k:{"port":443,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:targetPort': {}
-          'f:type': {}
-      manager: argocd-controller
-      operation: Apply
-      time: '2022-06-30T16:28:09Z'
-    - apiVersion: v1
-      fieldsType: FieldsV1
-      fieldsV1:
-        'f:metadata':
-          'f:annotations':
-            .: {}
-            'f:kubectl.kubernetes.io/last-applied-configuration': {}
-        'f:spec':
-          'f:internalTrafficPolicy': {}
-          'f:ports':
-            .: {}
-            'k:{"port":1935,"protocol":"TCP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-            'k:{"port":1986,"protocol":"UDP"}':
-              .: {}
-              'f:name': {}
-              'f:port': {}
-              'f:protocol': {}
-              'f:targetPort': {}
-          'f:sessionAffinity': {}
-      manager: kubectl-client-side-apply
-      operation: Update
-      time: '2022-06-25T04:18:10Z'
-    - apiVersion: v1
-      fieldsType: FieldsV1
-      fieldsV1:
-        'f:status':
-          'f:loadBalancer':
-            'f:ingress': {}
-      manager: kube-vpnkit-forwarder
-      operation: Update
-      subresource: status
-      time: '2022-06-29T12:36:34Z'
-  name: multiple-protocol-port-svc
-  namespace: default
-  resourceVersion: '2138591'
-  uid: af42e800-bd33-4412-bc77-d204d298613d
-spec:
-  clusterIP: 10.111.193.74
-  clusterIPs:
-    - 10.111.193.74
-  externalTrafficPolicy: Cluster
-  internalTrafficPolicy: Cluster
-  ipFamilies:
-    - IPv4
-  ipFamilyPolicy: SingleStack
-  ports:
-    - name: rtmpk
-      port: 1986
-      protocol: UDP
-      targetPort: 1986
-    - name: rtmp
-      port: 1935
-      protocol: TCP
-      targetPort: 1935
-    - name: https
-      port: 443
-      protocol: TCP
-      targetPort: 443
-  sessionAffinity: None
-  type: ClusterIP
-status:
-  loadBalancer: {}