Add validation webhook for exchanges.rabbitmq.com

api/v1alpha1/exchange_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"
 )
 
 // ExchangeSpec defines the desired state of Exchange
@@ -64,6 +65,13 @@ type ExchangeList struct {
 	Items           []Exchange `json:"items"`
 }
 
+func (e *Exchange) GroupResource() schema.GroupResource {
+	return schema.GroupResource{
+		Group:    e.GroupVersionKind().Group,
+		Resource: e.GroupVersionKind().Kind,
+	}
+}
+
 func init() {
 	SchemeBuilder.Register(&Exchange{}, &ExchangeList{})
 }

api/v1alpha1/exchange_webhook.go

@@ -0,0 +1,88 @@
+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"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+)
+
+func (r *Exchange) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(r).
+		Complete()
+}
+
+// +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1alpha1-exchange,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=exchanges,versions=v1alpha1,name=vexchange.kb.io,sideEffects=none,admissionReviewVersions=v1
+
+var _ webhook.Validator = &Exchange{}
+
+// no validation on create
+func (e *Exchange) ValidateCreate() error {
+	return nil
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+// returns error type 'forbidden' for updates that the controller chooses to disallow: exchange name/vhost/rabbitmqClusterReference
+// returns error type 'invalid' for updates that will be rejected by rabbitmq server: exchange types/autoDelete/durable
+// exchange.spec.arguments can be updated
+func (e *Exchange) ValidateUpdate(old runtime.Object) error {
+	oldExchange, ok := old.(*Exchange)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected an exchange but got a %T", old))
+	}
+
+	var allErrs field.ErrorList
+	detailMsg := "updates on name, vhost, and rabbitmqClusterReference are all forbidden"
+	if e.Spec.Name != oldExchange.Spec.Name {
+		return apierrors.NewForbidden(e.GroupResource(), e.Name,
+			field.Forbidden(field.NewPath("spec", "name"), detailMsg))
+	}
+
+	if e.Spec.Vhost != oldExchange.Spec.Vhost {
+		return apierrors.NewForbidden(e.GroupResource(), e.Name,
+			field.Forbidden(field.NewPath("spec", "vhost"), detailMsg))
+	}
+
+	if e.Spec.RabbitmqClusterReference != oldExchange.Spec.RabbitmqClusterReference {
+		return apierrors.NewForbidden(e.GroupResource(), e.Name,
+			field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg))
+	}
+
+	if e.Spec.Type != oldExchange.Spec.Type {
+		allErrs = append(allErrs, field.Invalid(
+			field.NewPath("spec", "type"),
+			e.Spec.Type,
+			"exchange type cannot be updated",
+		))
+	}
+
+	if e.Spec.AutoDelete != oldExchange.Spec.AutoDelete {
+		allErrs = append(allErrs, field.Invalid(
+			field.NewPath("spec", "autoDelete"),
+			e.Spec.AutoDelete,
+			"autoDelete cannot be updated",
+		))
+	}
+
+	if e.Spec.Durable != oldExchange.Spec.Durable {
+		allErrs = append(allErrs, field.Invalid(
+			field.NewPath("spec", "durable"),
+			e.Spec.AutoDelete,
+			"durable cannot be updated",
+		))
+	}
+
+	if len(allErrs) == 0 {
+		return nil
+	}
+
+	return apierrors.NewInvalid(GroupVersion.WithKind("Exchange").GroupKind(), e.Name, allErrs)
+}
+
+// no validation on delete
+func (e *Exchange) ValidateDelete() error {
+	return nil
+}

api/v1alpha1/exchange_webhook_test.go

@@ -0,0 +1,74 @@
+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("exchange webhook", func() {
+
+	var exchange = Exchange{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "update-binding",
+		},
+		Spec: ExchangeSpec{
+			Name:       "test",
+			Vhost:      "/test",
+			Type:       "fanout",
+			Durable:    false,
+			AutoDelete: true,
+			RabbitmqClusterReference: RabbitmqClusterReference{
+				Name:      "some-cluster",
+				Namespace: "default",
+			},
+		},
+	}
+
+	It("does not allow updates on exchange name", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.Name = "new-name"
+		Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue())
+	})
+
+	It("does not allow updates on vhost", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.Vhost = "/a-new-vhost"
+		Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue())
+	})
+
+	It("does not allow updates on RabbitmqClusterReference", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.RabbitmqClusterReference = RabbitmqClusterReference{
+			Name:      "new-cluster",
+			Namespace: "default",
+		}
+		Expect(apierrors.IsForbidden(newExchange.ValidateUpdate(&exchange))).To(BeTrue())
+	})
+
+	It("does not allow updates on exchange type", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.Type = "direct"
+		Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue())
+	})
+
+	It("does not allow updates on durable", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.Durable = true
+		Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue())
+	})
+
+	It("does not allow updates on autoDelete", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.AutoDelete = false
+		Expect(apierrors.IsInvalid(newExchange.ValidateUpdate(&exchange))).To(BeTrue())
+	})
+
+	It("allows updates on arguments", func() {
+		newExchange := exchange.DeepCopy()
+		newExchange.Spec.Arguments = &runtime.RawExtension{Raw: []byte(`{"new":"new-value"}`)}
+		Expect(newExchange.ValidateUpdate(&exchange)).To(Succeed())
+	})
+})

config/crd/kustomization.yaml

@@ -13,10 +13,12 @@ resources:
 patchesStrategicMerge:
 - patches/webhook_in_bindings.yaml
 - patches/webhook_in_queues.yaml
+- patches/webhook_in_exchanges.yaml
 # +kubebuilder:scaffold:crdkustomizewebhookpatch
 
 - patches/cainjection_in_bindings.yaml
 - patches/cainjection_in_queues.yaml
+- patches/webhook_in_exchanges.yaml
 # +kubebuilder:scaffold:crdkustomizecainjectionpatch
 
 configurations:

config/crd/patches/cainjection_in_exchanges.yaml

@@ -1,6 +1,6 @@
 # The following patch adds a directive for certmanager to inject CA into the CRD
 # CRD conversion requires k8s 1.13 or later.
-apiVersion: apiextensions.k8s.io/v1alpha1
+apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:

config/crd/patches/webhook_in_exchanges.yaml

@@ -1,17 +1,17 @@
 # The following patch enables conversion webhook for CRD
 # CRD conversion requires k8s 1.13 or later.
-apiVersion: apiextensions.k8s.io/v1alpha1
+apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   name: exchanges.rabbitmq.com
 spec:
   conversion:
     strategy: Webhook
-    webhookClientConfig:
-      # this is "\n" used as a placeholder, otherwise it will be rejected by the apiserver for being blank,
-      # but we're going to set it later using the cert-manager (or potentially a patch if not using cert-manager)
-      caBundle: Cg==
-      service:
-        namespace: system
-        name: webhook-service
-        path: /convert
+    webhook:
+      conversionReviewVersions: ["v1", "v1beta1"]
+      clientConfig:
+        caBundle: Cg==
+        service:
+          namespace: system
+          name: webhook-service
+          path: /convert

config/webhook/manifests.yaml

@@ -26,6 +26,26 @@ webhooks:
     resources:
     - bindings
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-rabbitmq-com-v1alpha1-exchange
+  failurePolicy: Fail
+  name: vexchange.kb.io
+  rules:
+  - apiGroups:
+    - rabbitmq.com
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - exchanges
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

main.go

@@ -126,6 +126,10 @@ func main() {
 		log.Error(err, "unable to create webhook", "webhook", "Queue")
 		os.Exit(1)
 	}
+	if err = (&topologyv1alpha1.Exchange{}).SetupWebhookWithManager(mgr); err != nil {
+		log.Error(err, "unable to create webhook", "webhook", "Exchange")
+		os.Exit(1)
+	}
 	// +kubebuilder:scaffold:builder
 
 	log.Info("starting manager")

system_tests/exchange_system_tests.go

@@ -65,18 +65,27 @@ var _ = Describe("Exchange", func() {
 		Expect(exchangeInfo.Arguments).To(HaveKeyWithValue("alternate-exchange", "system-test"))
 
 		By("updating status condition 'Ready'")
-		updatedExchange := topologyv1alpha1.Exchange{}
-		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &updatedExchange)).To(Succeed())
+		fetched := topologyv1alpha1.Exchange{}
+		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &fetched)).To(Succeed())
 
-		Expect(updatedExchange.Status.Conditions).To(HaveLen(1))
-		readyCondition := updatedExchange.Status.Conditions[0]
+		Expect(fetched.Status.Conditions).To(HaveLen(1))
+		readyCondition := fetched.Status.Conditions[0]
 		Expect(string(readyCondition.Type)).To(Equal("Ready"))
 		Expect(readyCondition.Status).To(Equal(corev1.ConditionTrue))
 		Expect(readyCondition.Reason).To(Equal("SuccessfulCreateOrUpdate"))
 		Expect(readyCondition.LastTransitionTime).NotTo(Equal(metav1.Time{}))
 
 		By("setting status.observedGeneration")
-		Expect(updatedExchange.Status.ObservedGeneration).To(Equal(updatedExchange.GetGeneration()))
+		Expect(fetched.Status.ObservedGeneration).To(Equal(fetched.GetGeneration()))
+
+		By("not allowing certain updates")
+		updatedExchange := topologyv1alpha1.Exchange{}
+		Expect(k8sClient.Get(ctx, types.NamespacedName{Name: exchange.Name, Namespace: exchange.Namespace}, &updatedExchange)).To(Succeed())
+		updatedExchange.Spec.Vhost = "/new-vhost"
+		Expect(k8sClient.Update(ctx, &updatedExchange).Error()).To(ContainSubstring("spec.vhost: Forbidden: updates on name, vhost, and rabbitmqClusterReference are all forbidden"))
+		updatedExchange.Spec.Vhost = exchange.Spec.Vhost
+		updatedExchange.Spec.Durable = false
+		Expect(k8sClient.Update(ctx, &updatedExchange).Error()).To(ContainSubstring("spec.durable: Invalid value: false: durable cannot be updated"))
 
 		By("deleting exchange")
 		Expect(k8sClient.Delete(ctx, exchange)).To(Succeed())