From 6574e9daf478d0ff7d3c584b4a668a8f092ddaae Mon Sep 17 00:00:00 2001
From: Alexandre Mahdhaoui <alexandre.mahdhaoui@gmail.com>
Date: Tue, 26 Mar 2024 15:13:01 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20allow=20fakeclient=20to=20patch?=
 =?UTF-8?q?=20CR=20with=20no=20RV?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Alexandre Mahdhaoui <alexandre.mahdhaoui@gmail.com>
---
 pkg/client/fake/client.go      | 13 +++++++++++--
 pkg/client/fake/client_test.go | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go
index b90a6ebb8d..38b849fa35 100644
--- a/pkg/client/fake/client.go
+++ b/pkg/client/fake/client.go
@@ -423,9 +423,18 @@ func (t versionedTracker) update(gvr schema.GroupVersionResource, obj runtime.Ob
 
 	// If the new object does not have the resource version set and it allows unconditional update,
 	// default it to the resource version of the existing resource
-	if accessor.GetResourceVersion() == "" && allowsUnconditionalUpdate(gvk) {
-		accessor.SetResourceVersion(oldAccessor.GetResourceVersion())
+	if accessor.GetResourceVersion() == "" {
+		switch {
+		case allowsUnconditionalUpdate(gvk):
+			accessor.SetResourceVersion(oldAccessor.GetResourceVersion())
+		case bytes.
+			Contains(debug.Stack(), []byte("sigs.k8s.io/controller-runtime/pkg/client/fake.(*fakeClient).patch")):
+			// We apply patches using a client-go reaction that ends up calling the trackers Update. As we can't change
+			// that reaction, we use the callstack to figure out if this originated from the "fakeClient.patch" func.
+			accessor.SetResourceVersion(oldAccessor.GetResourceVersion())
+		}
 	}
+
 	if accessor.GetResourceVersion() != oldAccessor.GetResourceVersion() {
 		return apierrors.NewConflict(gvr.GroupResource(), accessor.GetName(), errors.New("object was modified"))
 	}
diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go
index e5487de21a..b76cc61a5d 100644
--- a/pkg/client/fake/client_test.go
+++ b/pkg/client/fake/client_test.go
@@ -550,6 +550,38 @@ var _ = Describe("Fake client", func() {
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1000"))
 		})
 
+		It("should allow patch with non-set ResourceVersion for a resource that doesn't allow unconditional updates", func() {
+			schemeBuilder := &scheme.Builder{GroupVersion: schema.GroupVersion{Group: "test", Version: "v1"}}
+			schemeBuilder.Register(&WithPointerMeta{}, &WithPointerMetaList{})
+
+			scheme := runtime.NewScheme()
+			Expect(schemeBuilder.AddToScheme(scheme)).NotTo(HaveOccurred())
+
+			cl := NewClientBuilder().WithScheme(scheme).Build()
+			original := &WithPointerMeta{
+				ObjectMeta: &metav1.ObjectMeta{
+					Name:      "obj",
+					Namespace: "ns2",
+				}}
+
+			err := cl.Create(context.Background(), original)
+			Expect(err).ToNot(HaveOccurred())
+
+			newObj := &WithPointerMeta{
+				ObjectMeta: &metav1.ObjectMeta{
+					Name:      original.Name,
+					Namespace: original.Namespace,
+					Annotations: map[string]string{
+						"foo": "bar",
+					},
+				}}
+			Expect(cl.Patch(context.Background(), newObj, client.MergeFrom(original))).To(Succeed())
+
+			patched := &WithPointerMeta{}
+			Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(original), patched)).To(Succeed())
+			Expect(patched.Annotations).To(Equal(map[string]string{"foo": "bar"}))
+		})
+
 		It("should reject updates with non-set ResourceVersion for a resource that doesn't allow unconditional updates", func() {
 			By("Creating a new binding")
 			binding := &corev1.Binding{