Update validation on bindings to make it immutable

api/v1alpha1/binding_types.go

@@ -12,6 +12,7 @@ package v1alpha1
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 // BindingSpec defines the desired state of Binding
@@ -66,6 +67,13 @@ type BindingList struct {
 	Items           []Binding `json:"items"`
 }
 
+func (b *Binding) GroupResource() schema.GroupResource {
+	return schema.GroupResource{
+		Group:    b.GroupVersionKind().Group,
+		Resource: b.GroupVersionKind().Kind,
+	}
+}
+
 func init() {
 	SchemeBuilder.Register(&Binding{}, &BindingList{})
 }

api/v1alpha1/binding_webhook.go

@@ -1,41 +1,49 @@
 package v1alpha1
 
 import (
+	"fmt"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
 var logger = logf.Log.WithName("binding-webhook")
 
-func (r *Binding) SetupWebhookWithManager(mgr ctrl.Manager) error {
+func (b *Binding) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(r).
+		For(b).
 		Complete()
 }
 
 // +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1alpha1-binding,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=bindings,versions=v1alpha1,name=vbinding.kb.io,sideEffects=none,admissionReviewVersions=v1
 
 var _ webhook.Validator = &Binding{}
 
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (r *Binding) ValidateCreate() error {
-	logger.Info("validate create", "name", r.Name)
-
+// no validation logic on create
+func (b *Binding) ValidateCreate() error {
 	return nil
 }
 
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (r *Binding) ValidateUpdate(old runtime.Object) error {
-	logger.Info("validate update", "name", r.Name)
-
+// updates on bindings.rabbitmq.com is forbidden
+func (b *Binding) ValidateUpdate(old runtime.Object) error {
+	oldBinding, ok := old.(*Binding)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a binding but got a %T", old))
+	}
+
+	if oldBinding.Spec != b.Spec {
+		return apierrors.NewForbidden(
+			b.GroupResource(),
+			b.Name,
+			field.Forbidden(field.NewPath("spec"), "binding.spec is immutable"))
+	}
 	return nil
 }
 
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
-func (r *Binding) ValidateDelete() error {
-	logger.Info("validate delete", "name", r.Name)
-
+// no validation logic on delete
+func (b *Binding) ValidateDelete() error {
 	return nil
 }

api/v1alpha1/binding_webhook_test.go

@@ -0,0 +1,139 @@
+package v1alpha1
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+var _ = Describe("Binding webhook", func() {
+
+	var oldBinding = Binding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "update-binding",
+		},
+		Spec: BindingSpec{
+			Source:          "test",
+			Destination:     "test",
+			DestinationType: "queue",
+			RabbitmqClusterReference: RabbitmqClusterReference{
+				Name:      "some-cluster",
+				Namespace: "default",
+			},
+		},
+	}
+
+	It("does not allow updates on source", func() {
+		newBinding := Binding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "update-binding",
+			},
+			Spec: BindingSpec{
+				Source:          "updated-source",
+				Destination:     "test",
+				DestinationType: "queue",
+				RabbitmqClusterReference: RabbitmqClusterReference{
+					Name:      "some-cluster",
+					Namespace: "default",
+				},
+			},
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on destination", func() {
+		newBinding := Binding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "update-binding",
+			},
+			Spec: BindingSpec{
+				Source:          "test",
+				Destination:     "updated-des",
+				DestinationType: "queue",
+				RabbitmqClusterReference: RabbitmqClusterReference{
+					Name:      "some-cluster",
+					Namespace: "default",
+				},
+			},
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on destination type", func() {
+		newBinding := Binding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "update-binding",
+			},
+			Spec: BindingSpec{
+				Source:          "test",
+				Destination:     "test",
+				DestinationType: "exchange",
+				RabbitmqClusterReference: RabbitmqClusterReference{
+					Name:      "some-cluster",
+					Namespace: "default",
+				},
+			},
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on routing key", func() {
+		newBinding := Binding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "update-binding",
+			},
+			Spec: BindingSpec{
+				Source:          "test",
+				Destination:     "test",
+				DestinationType: "queue",
+				RoutingKey:      "not-allowed",
+				RabbitmqClusterReference: RabbitmqClusterReference{
+					Name:      "some-cluster",
+					Namespace: "default",
+				},
+			},
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on binding arguments", func() {
+		newBinding := Binding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "update-binding",
+			},
+			Spec: BindingSpec{
+				Source:          "test",
+				Destination:     "test",
+				DestinationType: "queue",
+				Arguments: &runtime.RawExtension{
+					Raw: []byte(`{"new":"new-value"}`),
+				},
+				RabbitmqClusterReference: RabbitmqClusterReference{
+					Name:      "some-cluster",
+					Namespace: "default",
+				},
+			},
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on RabbitmqClusterReference", func() {
+		newBinding := Binding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "update-binding",
+			},
+			Spec: BindingSpec{
+				Source:          "test",
+				Destination:     "test",
+				DestinationType: "queue",
+				RabbitmqClusterReference: RabbitmqClusterReference{
+					Name:      "new-cluster",
+					Namespace: "default",
+				},
+			},
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+})

system_tests/binding_system_tests.go

@@ -124,5 +124,11 @@ var _ = Describe("Binding", func() {
 
 		By("setting status.observedGeneration")
 		Expect(updatedBinding.Status.ObservedGeneration).To(Equal(updatedBinding.GetGeneration()))
+
+		By("not allowing updates on binding.spec")
+		updateBinding := topologyv1alpha1.Binding{}
+		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name, Namespace: binding.Namespace}, &updateBinding)).To(Succeed())
+		updatedBinding.Spec.RoutingKey = "new-key"
+		Expect(k8sClient.Update(ctx, &updatedBinding).Error()).To(ContainSubstring("spec: Forbidden: binding.spec is immutable"))
 	})
 })