Repository: kubernetes-sigs/controller-runtime

Issue:
empty resourceVersion during creation with fakeclient.Patch with SSA
when using fakeclient, if I explicitly create an object it will contain `resourceVersion == "1"`, but if the object is created using Patch with client.Apply option (and probably using client.Apply with ApplyConfiguration, although I haven't tested) resourceVersion remains empty. This leads to `does not have any resource version we can use` when trying to patch the resource again using OptimisticLock.

Discussion:

Solution patch:
diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go
index 45f9e00e..4109b5c5 100644
--- a/pkg/client/fake/client.go
+++ b/pkg/client/fake/client.go
@@ -1171,9 +1171,15 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client
 		return err
 	}
 
-	// SSA deletionTimestamp updates are silently ignored
-	if patch.Type() == types.ApplyPatchType && !isApplyCreate {
-		obj.SetDeletionTimestamp(oldAccessor.GetDeletionTimestamp())
+	if patch.Type() == types.ApplyPatchType {
+		if isApplyCreate {
+			// Overwrite it unconditionally, this matches the apiserver behavior
+			// which allows to set it on create, but will then ignore it.
+			obj.SetResourceVersion("1")
+		} else {
+			// SSA deletionTimestamp updates are silently ignored
+			obj.SetDeletionTimestamp(oldAccessor.GetDeletionTimestamp())
+		}
 	}
 
 	data, err := patch.Data(obj)



Test patch:
diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go
index 72c20fd5..328dfeec 100644
--- a/pkg/client/fake/client_test.go
+++ b/pkg/client/fake/client_test.go
@@ -2877,6 +2877,29 @@ var _ = Describe("Fake client", func() {
 		Expect(len(cms.Items)).To(BeEquivalentTo(1))
 	})
 
+	It("sets resourceVersion on SSA create", func(ctx SpecContext) {
+		obj := corev1applyconfigurations.
+			ConfigMap("foo", "default").
+			WithData(map[string]string{"some": "data"})
+
+		cl := NewClientBuilder().Build()
+		Expect(cl.Apply(ctx, obj, client.FieldOwner("foo"))).NotTo(HaveOccurred())
+		// Ideally we should only test for it to not be empty, realistically we will
+		// break ppl if we ever start setting a different value.
+		Expect(obj.ResourceVersion).To(BeEquivalentTo(ptr.To("1")))
+	})
+
+	It("ignores a passed resourceVersion on SSA create", func(ctx SpecContext) {
+		obj := corev1applyconfigurations.
+			ConfigMap("foo", "default").
+			WithData(map[string]string{"some": "data"}).
+			WithResourceVersion("1234")
+
+		cl := NewClientBuilder().Build()
+		Expect(cl.Apply(ctx, obj, client.FieldOwner("foo"))).NotTo(HaveOccurred())
+		Expect(obj.ResourceVersion).To(BeEquivalentTo(ptr.To("1")))
+	})
+
 	It("allows to set deletionTimestamp on an object during SSA create", func(ctx SpecContext) {
 		now := metav1.Time{Time: time.Now().Round(time.Second)}
 		obj := corev1applyconfigurations.