fix: force ownership when merging secrets

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -279,10 +279,9 @@ func patchSecret(ctx context.Context, c client.Client, scheme *runtime.Scheme, s
 	if !unversioned && len(gvks) == 1 {
 		secret.SetGroupVersionKind(gvks[0])
 	}
-	// we might get into a conflict here if we are not the manager of that particular field
-	// we do not resolve the conflict and return an error instead
-	// see: https://kubernetes.io/docs/reference/using-api/server-side-apply/#conflicts
-	err = c.Patch(ctx, secret, client.Apply, client.FieldOwner("external-secrets"))
+	// 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)
 	if err != nil {
 		return fmt.Errorf(errPolicyMergePatch, secret.Name, err)
 	}

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -341,8 +341,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 	}
 
-	// controller should not force override but
-	// return an error on conflict
+	// controller should force ownership
 	mergeWithConflict := func(tc *testCase) {
 		const secretVal = "someValue"
 		// this should confict
@@ -362,23 +361,14 @@ var _ = Describe("ExternalSecret controller", func() {
 		}, client.FieldOwner(FakeManager))).To(Succeed())
 		fakeProvider.WithGetSecret([]byte(secretVal), nil)
 
-		tc.checkCondition = func(es *esv1alpha1.ExternalSecret) bool {
-			cond := GetExternalSecretCondition(es.Status, esv1alpha1.ExternalSecretReady)
-			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1alpha1.ConditionReasonSecretSyncedError {
-				return false
-			}
-			return true
-		}
-
 		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
 			// check that value stays the same
-			Expect(string(secret.Data[existingKey])).To(Equal(existingVal))
-			Expect(string(secret.Data[targetProp])).ToNot(Equal(secretVal))
+			Expect(string(secret.Data[existingKey])).To(Equal(secretVal))
 
 			// check owner/managedFields
 			Expect(hasOwnerRef(secret.ObjectMeta, "ExternalSecret", ExternalSecretName)).To(BeFalse())
-			Expect(secret.ObjectMeta.ManagedFields).To(HaveLen(1))
-			Expect(hasFieldOwnership(secret.ObjectMeta, FakeManager, "{\"f:data\":{\".\":{},\"f:targetProperty\":{}},\"f:type\":{}}")).To(BeTrue())
+			Expect(secret.ObjectMeta.ManagedFields).To(HaveLen(2))
+			Expect(hasFieldOwnership(secret.ObjectMeta, "external-secrets", "{\"f:data\":{\"f:targetProperty\":{}},\"f:immutable\":{},\"f:metadata\":{\"f:annotations\":{\"f:reconcile.external-secrets.io/data-hash\":{}}}}")).To(BeTrue())
 		}
 	}