diff --git a/controller/appcontroller.go b/controller/appcontroller.go index a03770df72e0a..de24205063800 100644 --- a/controller/appcontroller.go +++ b/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 diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go index de7dc2b86e4b7..a8bcf9fede0ed 100644 --- a/controller/appcontroller_test.go +++ b/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) +} diff --git a/pkg/apis/application/v1alpha1/generated.proto b/pkg/apis/application/v1alpha1/generated.proto index 4cb2f846959cb..e04f6f3928083 100644 --- a/pkg/apis/application/v1alpha1/generated.proto +++ b/pkg/apis/application/v1alpha1/generated.proto @@ -2218,7 +2218,6 @@ message SyncStatus { optional string status = 1; // ComparedTo contains information about what has been compared - // +patchStrategy=replace optional ComparedTo comparedTo = 2; // Revision contains information about the revision the comparison has been performed to diff --git a/pkg/apis/application/v1alpha1/openapi_generated.go b/pkg/apis/application/v1alpha1/openapi_generated.go index 2b2a0fc4f6c9b..8b0ab9c602535 100644 --- a/pkg/apis/application/v1alpha1/openapi_generated.go +++ b/pkg/apis/application/v1alpha1/openapi_generated.go @@ -7684,11 +7684,6 @@ func schema_pkg_apis_application_v1alpha1_SyncStatus(ref common.ReferenceCallbac }, }, "comparedTo": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-patch-strategy": "replace", - }, - }, SchemaProps: spec.SchemaProps{ Description: "ComparedTo contains information about what has been compared", Default: map[string]interface{}{}, diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index 4bc953c076676..49be82a443bc4 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/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 diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go index 450a0fb11a4bd..b9fb26cf21875 100644 --- a/pkg/apis/application/v1alpha1/types_test.go +++ b/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)) -}