Merge pull request kubernetes-sigs#1582 from 2uasimojo/fake-delete-ResourceVersion

⚠️ Fakeclient: Reject Delete with mismatched ResourceVersion

Author: k8s-ci-robot

pkg/client/fake/client.go

@@ -419,6 +419,28 @@ 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
+		}
+		actualRV := oldAccessor.GetResourceVersion()
+		expectRV := *delOptions.Preconditions.ResourceVersion
+		if actualRV != expectRV {
+			msg := fmt.Sprintf(
+				"the ResourceVersion in the precondition (%s) does not match the ResourceVersion in record (%s). "+
+					"The object might have been modified",
+				expectRV, actualRV)
+			return apierrors.NewConflict(gvr.GroupResource(), name, errors.New(msg))
+		}
+	}
+
 	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 matching 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",