⚠️ Fakeclient: Reject Update with outdated ResourceVersion

Author: alvaroaleman

pkg/client/fake/client.go

@@ -19,10 +19,12 @@ package fake
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -79,28 +81,42 @@ func NewFakeClientWithScheme(clientScheme *runtime.Scheme, initObjs ...runtime.O
 }
 
 func (t versionedTracker) Create(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error {
-	if accessor, err := meta.Accessor(obj); err == nil {
-		if accessor.GetResourceVersion() == "" {
-			accessor.SetResourceVersion("1")
-		}
-	} else {
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
 		return err
 	}
+	if accessor.GetResourceVersion() != "" {
+		return kerrors.NewBadRequest("resourceVersion can not be set for Create requests")
+	}
+	accessor.SetResourceVersion("1")
 	return t.ObjectTracker.Create(gvr, obj, ns)
 }
 
 func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error {
-	if accessor, err := meta.Accessor(obj); err == nil {
-		version := 0
-		if rv := accessor.GetResourceVersion(); rv != "" {
-			version, err = strconv.Atoi(rv)
-		}
-		if err == nil {
-			accessor.SetResourceVersion(strconv.Itoa(version + 1))
-		}
-	} else {
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return fmt.Errorf("failed to get accessor for object: %v", err)
+	}
+	oldObject, err := t.ObjectTracker.Get(gvr, ns, accessor.GetName())
+	if err != nil {
+		return err
+	}
+	oldAccessor, err := meta.Accessor(oldObject)
+	if err != nil {
 		return err
 	}
+	if accessor.GetResourceVersion() != oldAccessor.GetResourceVersion() {
+		return kerrors.NewConflict(gvr.GroupResource(), accessor.GetName(), errors.New("object was modified"))
+	}
+	if oldAccessor.GetResourceVersion() == "" {
+		oldAccessor.SetResourceVersion("0")
+	}
+	intResourceVersion, err := strconv.Atoi(oldAccessor.GetResourceVersion())
+	if err != nil {
+		return fmt.Errorf("can not convert resourceVersion %q to int: %v", oldAccessor.GetResourceVersion(), err)
+	}
+	intResourceVersion++
+	accessor.SetResourceVersion(strconv.Itoa(intResourceVersion))
 	return t.ObjectTracker.Update(gvr, obj, ns)
 }
 

pkg/client/fake/client_test.go

@@ -27,6 +27,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -172,6 +173,19 @@ var _ = Describe("Fake client", func() {
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
 		})
 
+		It("should error on create with set resourceVersion", func() {
+			By("Creating a new configmap")
+			newcm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "new-test-cm",
+					Namespace:       "ns2",
+					ResourceVersion: "1",
+				},
+			}
+			err := cl.Create(context.Background(), newcm)
+			Expect(kerrors.IsBadRequest(err)).To(BeTrue())
+		})
+
 		It("should be able to Create with GenerateName", func() {
 			By("Creating a new configmap")
 			newcm := &corev1.ConfigMap{
@@ -211,7 +225,7 @@ var _ = Describe("Fake client", func() {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:            "test-cm",
 					Namespace:       "ns2",
-					ResourceVersion: "1",
+					ResourceVersion: "",
 				},
 				Data: map[string]string{
 					"test-key": "new-value",
@@ -229,7 +243,34 @@ var _ = Describe("Fake client", func() {
 			err = cl.Get(context.Background(), namespacedName, obj)
 			Expect(err).To(BeNil())
 			Expect(obj).To(Equal(newcm))
-			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("2"))
+			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
+		})
+
+		It("should reject updates with non-matching ResourceVersion", func() {
+			By("Updating a new configmap")
+			newcm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "test-cm",
+					Namespace:       "ns2",
+					ResourceVersion: "1",
+				},
+				Data: map[string]string{
+					"test-key": "new-value",
+				},
+			}
+			err := cl.Update(context.Background(), newcm)
+			Expect(kerrors.IsConflict(err)).To(BeTrue())
+
+			By("Getting the configmap")
+			namespacedName := types.NamespacedName{
+				Name:      "test-cm",
+				Namespace: "ns2",
+			}
+			obj := &corev1.ConfigMap{}
+			err = cl.Get(context.Background(), namespacedName, obj)
+			Expect(err).To(BeNil())
+			Expect(obj).To(Equal(cm))
+			Expect(obj.ObjectMeta.ResourceVersion).To(Equal(""))
 		})
 
 		It("should be able to Delete", func() {