fix: ArgoCD 2.10 - Loop of PATCH calls to Application objects (#19340)

controller/appcontroller.go

@@ -1758,6 +1758,22 @@ func (ctrl *ApplicationController) normalizeApplication(orig, app *appv1.Applica
 	}
 }
 
+func createMergePatch(orig, new interface{}) ([]byte, bool, error) {
+	origBytes, err := json.Marshal(orig)
+	if err != nil {
+		return nil, false, err
+	}
+	newBytes, err := json.Marshal(new)
+	if err != nil {
+		return nil, false, err
+	}
+	patch, err := jsonpatch.CreateMergePatch(origBytes, newBytes)
+	if err != nil {
+		return nil, false, err
+	}
+	return patch, string(patch) != "{}", nil
+}
+
 // persistAppStatus persists updates to application status. If no changes were made, it is a no-op
 func (ctrl *ApplicationController) persistAppStatus(orig *appv1.Application, newStatus *appv1.ApplicationStatus) (patchMs time.Duration) {
 	logCtx := log.WithFields(log.Fields{"application": orig.QualifiedName()})
@@ -1777,9 +1793,9 @@ func (ctrl *ApplicationController) persistAppStatus(orig *appv1.Application, new
 		}
 		delete(newAnnotations, appv1.AnnotationKeyRefresh)
 	}
-	patch, modified, err := diff.CreateTwoWayMergePatch(
+	patch, modified, err := createMergePatch(
 		&appv1.Application{ObjectMeta: metav1.ObjectMeta{Annotations: orig.GetAnnotations()}, Status: orig.Status},
-		&appv1.Application{ObjectMeta: metav1.ObjectMeta{Annotations: newAnnotations}, Status: *newStatus}, appv1.Application{})
+		&appv1.Application{ObjectMeta: metav1.ObjectMeta{Annotations: newAnnotations}, Status: *newStatus})
 	if err != nil {
 		logCtx.Errorf("Error constructing app status patch: %v", err)
 		return

controller/appcontroller_test.go

@@ -986,7 +986,7 @@ func TestNormalizeApplication(t *testing.T) {
 		normalized := false
 		fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
 			if patchAction, ok := action.(kubetesting.PatchAction); ok {
-				if string(patchAction.GetPatch()) == `{"spec":{"project":"default"},"status":{"sync":{"comparedTo":{"destination":{},"source":{"repoURL":""}}}}}` {
+				if string(patchAction.GetPatch()) == `{"spec":{"project":"default"}}` {
 					normalized = true
 				}
 			}
@@ -1911,3 +1911,65 @@ func TestAddControllerNamespace(t *testing.T) {
 		assert.Equal(t, test.FakeArgoCDNamespace, updatedApp.Status.ControllerNamespace)
 	})
 }
+
+func TestHelmValuesObjectHasReplaceStrategy(t *testing.T) {
+	app := v1alpha1.Application{
+		Status: v1alpha1.ApplicationStatus{Sync: v1alpha1.SyncStatus{ComparedTo: v1alpha1.ComparedTo{
+			Source: v1alpha1.ApplicationSource{
+				Helm: &v1alpha1.ApplicationSourceHelm{
+					ValuesObject: &runtime.RawExtension{
+						Object: &unstructured.Unstructured{Object: map[string]interface{}{"key": []string{"value"}}},
+					},
+				},
+			},
+		}}},
+	}
+
+	appModified := v1alpha1.Application{
+		Status: v1alpha1.ApplicationStatus{Sync: v1alpha1.SyncStatus{ComparedTo: v1alpha1.ComparedTo{
+			Source: v1alpha1.ApplicationSource{
+				Helm: &v1alpha1.ApplicationSourceHelm{
+					ValuesObject: &runtime.RawExtension{
+						Object: &unstructured.Unstructured{Object: map[string]interface{}{"key": []string{"value-modified1"}}},
+					},
+				},
+			},
+		}}},
+	}
+
+	patch, _, err := createMergePatch(
+		app,
+		appModified)
+	require.NoError(t, err)
+	assert.Equal(t, `{"status":{"sync":{"comparedTo":{"source":{"helm":{"valuesObject":{"key":["value-modified1"]}}}}}}}`, string(patch))
+}
+
+func TestAppStatusIsReplaced(t *testing.T) {
+	original := &v1alpha1.ApplicationStatus{Sync: v1alpha1.SyncStatus{
+		ComparedTo: v1alpha1.ComparedTo{
+			Destination: v1alpha1.ApplicationDestination{
+				Server: "https://mycluster",
+			},
+		},
+	}}
+
+	updated := &v1alpha1.ApplicationStatus{Sync: v1alpha1.SyncStatus{
+		ComparedTo: v1alpha1.ComparedTo{
+			Destination: v1alpha1.ApplicationDestination{
+				Name: "mycluster",
+			},
+		},
+	}}
+
+	patchData, ok, err := createMergePatch(original, updated)
+
+	require.NoError(t, err)
+	require.True(t, ok)
+	patchObj := map[string]interface{}{}
+	require.NoError(t, json.Unmarshal(patchData, &patchObj))
+
+	val, has, err := unstructured.NestedFieldNoCopy(patchObj, "sync", "comparedTo", "destination", "server")
+	require.NoError(t, err)
+	require.True(t, has)
+	require.Nil(t, val)
+}

pkg/apis/application/v1alpha1/types.go

@@ -1497,8 +1497,7 @@ type SyncStatus struct {
 	// Status is the sync state of the comparison
 	Status SyncStatusCode `json:"status" protobuf:"bytes,1,opt,name=status,casttype=SyncStatusCode"`
 	// ComparedTo contains information about what has been compared
-	// +patchStrategy=replace
-	ComparedTo ComparedTo `json:"comparedTo,omitempty" protobuf:"bytes,2,opt,name=comparedTo" patchStrategy:"replace"`
+	ComparedTo ComparedTo `json:"comparedTo,omitempty" protobuf:"bytes,2,opt,name=comparedTo"`
 	// Revision contains information about the revision the comparison has been performed to
 	Revision string `json:"revision,omitempty" protobuf:"bytes,3,opt,name=revision"`
 	// Revisions contains information about the revisions of multiple sources the comparison has been performed to

pkg/apis/application/v1alpha1/types_test.go

@@ -11,14 +11,10 @@ import (
 	"testing"
 	"time"
 
-	"github.com/argoproj/gitops-engine/pkg/diff"
+	argocdcommon "github.com/argoproj/argo-cd/v2/common"
 	"github.com/stretchr/testify/require"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/pointer"
 
-	argocdcommon "github.com/argoproj/argo-cd/v2/common"
-
 	"github.com/argoproj/gitops-engine/pkg/sync/common"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -3623,35 +3619,3 @@ func TestOptionalMapEquality(t *testing.T) {
 		})
 	}
 }
-
-func TestHelmValuesObjectHasReplaceStrategy(t *testing.T) {
-	app := Application{
-		Status: ApplicationStatus{Sync: SyncStatus{ComparedTo: ComparedTo{
-			Source: ApplicationSource{
-				Helm: &ApplicationSourceHelm{
-					ValuesObject: &runtime.RawExtension{
-						Object: &unstructured.Unstructured{Object: map[string]interface{}{"key": []string{"value"}}},
-					},
-				},
-			},
-		}}},
-	}
-
-	appModified := Application{
-		Status: ApplicationStatus{Sync: SyncStatus{ComparedTo: ComparedTo{
-			Source: ApplicationSource{
-				Helm: &ApplicationSourceHelm{
-					ValuesObject: &runtime.RawExtension{
-						Object: &unstructured.Unstructured{Object: map[string]interface{}{"key": []string{"value-modified1"}}},
-					},
-				},
-			},
-		}}},
-	}
-
-	patch, _, err := diff.CreateTwoWayMergePatch(
-		app,
-		appModified, Application{})
-	require.NoError(t, err)
-	assert.Equal(t, `{"status":{"sync":{"comparedTo":{"destination":{},"source":{"helm":{"valuesObject":{"key":["value-modified1"]}},"repoURL":""}}}}}`, string(patch))
-}