Merge pull request #926 from detiber/update

🐛 Fakeclient: Honor AllowUnconditionalUpdate and AllowCreateOnUpdate for resources that support it

pkg/client/fake/client.go

@@ -108,20 +108,34 @@ func (t versionedTracker) Update(gvr schema.GroupVersionResource, obj runtime.Ob
 	if err != nil {
 		return fmt.Errorf("failed to get accessor for object: %v", err)
 	}
+
 	if accessor.GetName() == "" {
 		return apierrors.NewInvalid(
 			obj.GetObjectKind().GroupVersionKind().GroupKind(),
 			accessor.GetName(),
 			field.ErrorList{field.Required(field.NewPath("metadata.name"), "name is required")})
 	}
+
 	oldObject, err := t.ObjectTracker.Get(gvr, ns, accessor.GetName())
 	if err != nil {
+		// If the resource is not found and the resource allows create on update, issue a
+		// create instead.
+		if apierrors.IsNotFound(err) && allowsCreateOnUpdate(obj) {
+			return t.Create(gvr, obj, ns)
+		}
 		return err
 	}
+
 	oldAccessor, err := meta.Accessor(oldObject)
 	if err != nil {
 		return err
 	}
+
+	// If the new object does not have the resource version set and it allows unconditional update,
+	// default it to the resource version of the existing resource
+	if accessor.GetResourceVersion() == "" && allowsUnconditionalUpdate(obj) {
+		accessor.SetResourceVersion(oldAccessor.GetResourceVersion())
+	}
 	if accessor.GetResourceVersion() != oldAccessor.GetResourceVersion() {
 		return apierrors.NewConflict(gvr.GroupResource(), accessor.GetName(), errors.New("object was modified"))
 	}
@@ -416,3 +430,102 @@ func (sw *fakeStatusWriter) Patch(ctx context.Context, obj runtime.Object, patch
 	// a way to update status field only.
 	return sw.client.Patch(ctx, obj, patch, opts...)
 }
+
+func allowsUnconditionalUpdate(obj runtime.Object) bool {
+	gvk := obj.GetObjectKind().GroupVersionKind()
+	switch gvk.Group {
+	case "apps":
+		switch gvk.Kind {
+		case "ControllerRevision", "DaemonSet", "Deployment", "ReplicaSet", "StatefulSet":
+			return true
+		}
+	case "autoscaling":
+		switch gvk.Kind {
+		case "HorizontalPodAutoscaler":
+			return true
+		}
+	case "batch":
+		switch gvk.Kind {
+		case "CronJob", "Job":
+			return true
+		}
+	case "certificates":
+		switch gvk.Kind {
+		case "Certificates":
+			return true
+		}
+	case "flowcontrol":
+		switch gvk.Kind {
+		case "FlowSchema", "PriorityLevelConfiguration":
+			return true
+		}
+	case "networking":
+		switch gvk.Kind {
+		case "Ingress", "IngressClass", "NetworkPolicy":
+			return true
+		}
+	case "policy":
+		switch gvk.Kind {
+		case "PodSecurityPolicy":
+			return true
+		}
+	case "rbac":
+		switch gvk.Kind {
+		case "ClusterRole", "ClusterRoleBinding", "Role", "RoleBinding":
+			return true
+		}
+	case "scheduling":
+		switch gvk.Kind {
+		case "PriorityClass":
+			return true
+		}
+	case "settings":
+		switch gvk.Kind {
+		case "PodPreset":
+			return true
+		}
+	case "storage":
+		switch gvk.Kind {
+		case "StorageClass":
+			return true
+		}
+	case "":
+		switch gvk.Kind {
+		case "ConfigMap", "Endpoint", "Event", "LimitRange", "Namespace", "Node",
+			"PersistentVolume", "PersistentVolumeClaim", "Pod", "PodTemplate",
+			"ReplicationController", "ResourceQuota", "Secret", "Service",
+			"ServiceAccount", "EndpointSlice":
+			return true
+		}
+	}
+
+	return false
+}
+
+func allowsCreateOnUpdate(obj runtime.Object) bool {
+	gvk := obj.GetObjectKind().GroupVersionKind()
+	switch gvk.Group {
+	case "coordination":
+		switch gvk.Kind {
+		case "Lease":
+			return true
+		}
+	case "node":
+		switch gvk.Kind {
+		case "RuntimeClass":
+			return true
+		}
+	case "rbac":
+		switch gvk.Kind {
+		case "ClusterRole", "ClusterRoleBinding", "Role", "RoleBinding":
+			return true
+		}
+	case "":
+		switch gvk.Kind {
+		case "Endpoint", "Event", "LimitRange", "Service":
+			return true
+		}
+	}
+
+	return false
+}

pkg/client/fake/client_test.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	appsv1 "k8s.io/api/apps/v1"
+	coordinationv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -284,6 +285,118 @@ var _ = Describe("Fake client", func() {
 			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
 		})
 
+		It("should allow updates with non-set ResourceVersion for a resource that allows unconditional updates", func() {
+			By("Updating a new configmap")
+			newcm := &corev1.ConfigMap{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "ConfigMap",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-cm",
+					Namespace: "ns2",
+				},
+				Data: map[string]string{
+					"test-key": "new-value",
+				},
+			}
+			err := cl.Update(context.Background(), newcm)
+			Expect(err).To(BeNil())
+
+			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(newcm))
+			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
+		})
+
+		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{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Binding",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-binding",
+					Namespace: "ns2",
+				},
+				Target: corev1.ObjectReference{
+					Kind:       "ConfigMap",
+					APIVersion: "v1",
+					Namespace:  cm.Namespace,
+					Name:       cm.Name,
+				},
+			}
+			Expect(cl.Create(context.Background(), binding)).To(Succeed())
+
+			By("Updating the binding with a new resource lacking resource version")
+			newBinding := &corev1.Binding{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Binding",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      binding.Name,
+					Namespace: binding.Namespace,
+				},
+				Target: corev1.ObjectReference{
+					Namespace: binding.Namespace,
+					Name:      "blue",
+				},
+			}
+			Expect(cl.Update(context.Background(), newBinding)).NotTo(Succeed())
+		})
+
+		It("should allow create on update for a resource that allows create on update", func() {
+			By("Creating a new lease with update")
+			lease := &coordinationv1.Lease{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "coordination.k8s.io/v1",
+					Kind:       "Lease",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-lease",
+					Namespace: "ns2",
+				},
+				Spec: coordinationv1.LeaseSpec{},
+			}
+			Expect(cl.Create(context.Background(), lease)).To(Succeed())
+
+			By("Getting the lease")
+			namespacedName := types.NamespacedName{
+				Name:      lease.Name,
+				Namespace: lease.Namespace,
+			}
+			obj := &coordinationv1.Lease{}
+			Expect(cl.Get(context.Background(), namespacedName, obj)).To(Succeed())
+			Expect(obj).To(Equal(lease))
+			Expect(obj.ObjectMeta.ResourceVersion).To(Equal("1"))
+		})
+
+		It("should reject create on update for a resource that does not allow create on update", func() {
+			By("Attemping to create a new configmap with update")
+			newcm := &corev1.ConfigMap{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "ConfigMap",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "different-test-cm",
+					Namespace: "ns2",
+				},
+				Data: map[string]string{
+					"test-key": "new-value",
+				},
+			}
+			Expect(cl.Update(context.Background(), newcm)).NotTo(Succeed())
+		})
+
 		It("should reject updates with non-matching ResourceVersion", func() {
 			By("Updating a new configmap")
 			newcm := &corev1.ConfigMap{
@@ -427,6 +540,7 @@ var _ = Describe("Fake client", func() {
 			scheme := runtime.NewScheme()
 			Expect(corev1.AddToScheme(scheme)).To(Succeed())
 			Expect(appsv1.AddToScheme(scheme)).To(Succeed())
+			Expect(coordinationv1.AddToScheme(scheme)).To(Succeed())
 			cl = NewFakeClientWithScheme(scheme, dep, dep2, cm)
 			close(done)
 		})