⚠️ Fakeclient: Reject Delete with mismatched ResourceVersion

`Delete` takes optional varargs comprising `DeleteOption`s. Previously
fakeclient was validating that these could be run through
`ApplyOptions`, but otherwise ignoring them.

With this commit, fakeclient will observe a `DeleteOption` specifying a
`Precondition.ResourceVersion`, rejecting the deletion if the value does
not match what's on the "server", bringing fakeclient's behavior more in
line with a real client.

Note that, while technically a breaking change, any test that was
relying on the previous behavior was already broken.

xref #832 / 25ef372 which did something similar for `Create` and
`Update`. Major difference being that those methods do `ResourceVersion`
matching by default, whereas `Delete` only does it if opted in via the
`Precondition`.

Author: 2uasimojo

pkg/client/fake/client.go

@@ -419,6 +419,22 @@ func (c *fakeClient) Delete(ctx context.Context, obj client.Object, opts ...clie
 	delOptions := client.DeleteOptions{}
 	delOptions.ApplyOptions(opts)
 
+	// Check the ResourceVersion if that Precondition was specified.
+	if delOptions.Preconditions != nil && delOptions.Preconditions.ResourceVersion != nil {
+		name := accessor.GetName()
+		dbObj, err := c.tracker.Get(gvr, accessor.GetNamespace(), name)
+		if err != nil {
+			return err
+		}
+		oldAccessor, err := meta.Accessor(dbObj)
+		if err != nil {
+			return err
+		}
+		if oldAccessor.GetResourceVersion() != *delOptions.Preconditions.ResourceVersion {
+			return apierrors.NewConflict(gvr.GroupResource(), name, errors.New("object was modified"))
+		}
+	}
+
 	return c.deleteObject(gvr, accessor)
 }
 

pkg/client/fake/client_test.go

@@ -568,7 +568,33 @@ var _ = Describe("Fake client", func() {
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal(trackerAddResourceVersion))
 		})
 
-		It("should be able to Delete", func() {
+		It("should reject Delete with a mismatched ResourceVersion", func() {
+			bogusRV := "bogus"
+			By("Deleting with a mismatched ResourceVersion Precondition")
+			err := cl.Delete(context.Background(), dep, client.Preconditions{ResourceVersion: &bogusRV})
+			Expect(apierrors.IsConflict(err)).To(BeTrue())
+
+			list := &appsv1.DeploymentList{}
+			err = cl.List(context.Background(), list, client.InNamespace("ns1"))
+			Expect(err).To(BeNil())
+			Expect(list.Items).To(HaveLen(2))
+			Expect(list.Items).To(ConsistOf(*dep, *dep2))
+		})
+
+		It("should successfully Delete with a matching ResourceVersion", func() {
+			goodRV := trackerAddResourceVersion
+			By("Deleting with a mismatched ResourceVersion Precondition")
+			err := cl.Delete(context.Background(), dep, client.Preconditions{ResourceVersion: &goodRV})
+			Expect(err).To(BeNil())
+
+			list := &appsv1.DeploymentList{}
+			err = cl.List(context.Background(), list, client.InNamespace("ns1"))
+			Expect(err).To(BeNil())
+			Expect(list.Items).To(HaveLen(1))
+			Expect(list.Items).To(ConsistOf(*dep2))
+		})
+
+		It("should be able to Delete with no ResourceVersion Precondition", func() {
 			By("Deleting a deployment")
 			err := cl.Delete(context.Background(), dep)
 			Expect(err).To(BeNil())
@@ -581,6 +607,21 @@ var _ = Describe("Fake client", func() {
 			Expect(list.Items).To(ConsistOf(*dep2))
 		})
 
+		It("should be able to Delete with no opts even if object's ResourceVersion doesn't match server", func() {
+			By("Deleting a deployment")
+			depCopy := dep.DeepCopy()
+			depCopy.ResourceVersion = "bogus"
+			err := cl.Delete(context.Background(), depCopy)
+			Expect(err).To(BeNil())
+
+			By("Listing all deployments in the namespace")
+			list := &appsv1.DeploymentList{}
+			err = cl.List(context.Background(), list, client.InNamespace("ns1"))
+			Expect(err).To(BeNil())
+			Expect(list.Items).To(HaveLen(1))
+			Expect(list.Items).To(ConsistOf(*dep2))
+		})
+
 		It("should handle finalizers on Update", func() {
 			namespacedName := types.NamespacedName{
 				Name:      "test-cm",