diff --git a/api/v1alpha1/binding_types.go b/api/v1alpha1/binding_types.go index 3d5cdbe2..29c7ba0a 100644 --- a/api/v1alpha1/binding_types.go +++ b/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{}) } diff --git a/api/v1alpha1/binding_webhook.go b/api/v1alpha1/binding_webhook.go index 2f8add76..f7aac2cc 100644 --- a/api/v1alpha1/binding_webhook.go +++ b/api/v1alpha1/binding_webhook.go @@ -1,7 +1,10 @@ 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" @@ -9,9 +12,9 @@ import ( 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() } @@ -19,23 +22,28 @@ func (r *Binding) SetupWebhookWithManager(mgr ctrl.Manager) error { 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 } diff --git a/api/v1alpha1/binding_webhook_test.go b/api/v1alpha1/binding_webhook_test.go new file mode 100644 index 00000000..d739a3c5 --- /dev/null +++ b/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()) + }) +}) diff --git a/system_tests/binding_system_tests.go b/system_tests/binding_system_tests.go index b6875f1d..9c203f8f 100644 --- a/system_tests/binding_system_tests.go +++ b/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")) }) })