Merge pull request #2725 from alexandremahdhaoui/fix_fakeclient-patch-with-no-rv-should-succeed

🐛 allow fakeclient to patch CR with no RV

Author: k8s-ci-robot

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"))
 	}

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{