Optimise patching so as changes only happen with something changes

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -17,6 +17,7 @@ package externalsecret
 import (
 	"context"
 	"fmt"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -263,10 +264,13 @@ func patchSecret(ctx context.Context, c client.Client, scheme *runtime.Scheme, s
 	if err != nil {
 		return fmt.Errorf(errPolicyMergeGetSecret, secret.Name, err)
 	}
+	existing := secret.DeepCopyObject()
+
 	err = mutationFunc()
 	if err != nil {
 		return fmt.Errorf(errPolicyMergeMutate, secret.Name, err)
 	}
+
 	// GVK is missing in the Secret, see:
 	// https://github.com/kubernetes-sigs/controller-runtime/issues/526
 	// https://github.com/kubernetes-sigs/controller-runtime/issues/1517
@@ -279,6 +283,11 @@ func patchSecret(ctx context.Context, c client.Client, scheme *runtime.Scheme, s
 	if !unversioned && len(gvks) == 1 {
 		secret.SetGroupVersionKind(gvks[0])
 	}
+
+	if equality.Semantic.DeepEqual(existing, secret) {
+		return nil
+	}
+
 	// we're not able to resolve conflicts so we force ownership
 	// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#using-server-side-apply-in-a-controller
 	err = c.Patch(ctx, secret, client.Apply, client.FieldOwner("external-secrets"), client.ForceOwnership)

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -318,6 +318,28 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 	}
 
+	// should not update if no changes
+	mergeWithSecretNoChange := func(tc *testCase) {
+		const existingKey = "pre-existing-key"
+		existingVal := "someValue"
+		tc.externalSecret.Spec.Target.CreationPolicy = esv1alpha1.Merge
+
+		// create secret beforehand
+		Expect(k8sClient.Create(context.Background(), &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      ExternalSecretTargetSecretName,
+				Namespace: ExternalSecretNamespace,
+			},
+			Data: map[string][]byte{
+				existingKey: []byte(existingVal),
+			},
+		}, client.FieldOwner(FakeManager))).To(Succeed())
+
+		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
+			Expect(secret.ResourceVersion).To(Equal("295"))
+		}
+	}
+
 	// should not merge with secret if it doesn't exist
 	mergeWithSecretErr := func(tc *testCase) {
 		const secretVal = "someValue"
@@ -925,6 +947,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		Entry("should merge with existing secret using creationPolicy=Merge", mergeWithSecret),
 		Entry("should error if secret doesn't exist when using creationPolicy=Merge", mergeWithSecretErr),
 		Entry("should not resolve conflicts with creationPolicy=Merge", mergeWithConflict),
+		Entry("should not update unchanged secret using creationPolicy=Merge", mergeWithSecretNoChange),
 		Entry("should sync with template", syncWithTemplate),
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
 		Entry("should refresh secret from template", refreshWithTemplate),