🐛 allow fakeclient to patch CR with no RV

\# Context

This PR should fix a bug
#2678 where
patching a resource would throw a conflict error when the new object
doesn't specify any RV.

- Because the new RV is an empty string, an error is thrown here:
  https://github.com/kubernetes-sigs/controller-runtime/blob/01de80b092778fd35abd9e2371d54ce0953d4613/pkg/client/fake/client.go#L429-L430
- The RV is nil, because the computed patch between the old and new
  accessor doesn't include resourceVersion:
  -
https://github.com/evanphx/json-patch/blob/v5.9.0/v5/merge.go#L253-L273
  - Witch is used by `client.MergeFrom()`:
    https://github.com/kubernetes-sigs/controller-runtime/blob/v0.17.2/pkg/client/patch.go#L150-L152

\# Changes

- Fix patch by setting the new accessors' RV to the old accessors' RV.
- Add test to ensure the correct behavior

Author: alexandremahdhaoui

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{

pkg/client/patch.go

@@ -112,6 +112,8 @@ func (s *mergeFromPatch) Data(obj Object) ([]byte, error) {
 
 		modified = modified.DeepCopyObject().(Object)
 		modified.SetResourceVersion(version)
+	} else if modified.GetResourceVersion() == "" {
+		modified.SetResourceVersion(original.GetResourceVersion())
 	}
 
 	originalJSON, err := json.Marshal(original)