Merge pull request #72 from rabbitmq/binding-webhook

bindings.rabbitmq.com webhook to prevent updates on binding.spec

.github/workflows/pr.yml

@@ -33,6 +33,7 @@ jobs:
         export PATH="$PATH:$GOPATH/bin"
         make install-tools
         kind create cluster --image kindest/node:"$K8S_VERSION"
+        make cert-manager
         make cluster-operator
         DOCKER_REGISTRY_SERVER=local-server OPERATOR_IMAGE=local-operator make deploy-kind
         make system-tests

Makefile

@@ -123,3 +123,9 @@ generate-manifests:
 	mkdir -p releases
 	kustomize build config/installation/ > releases/messaging-topology-operator.yaml
 
+CERT_MANAGER_VERSION ?=v1.2.0
+cert-manager:
+	kubectl apply -f https://github.com/jetstack/cert-manager/releases/download/$(CERT_MANAGER_VERSION)/cert-manager.yaml
+
+destroy-cert-manager:
+	kubectl delete -f https://github.com/jetstack/cert-manager/releases/download/$(CERT_MANAGER_VERSION)/cert-manager.yaml

README.md

@@ -11,6 +11,7 @@ Before deploying Messaging Topology Operator, you need to have:
 1. A Running k8s cluster
 2. RabbitMQ [Cluster Operator](https://github.com/rabbitmq/cluster-operator) installed in the k8s cluster
 3. A [RabbitMQ cluster](https://github.com/rabbitmq/cluster-operator/tree/main/docs/examples) deployed using the Cluster Operator
+4. (Optional) [cert-manager](https://cert-manager.io/docs/installation/kubernetes/) `1.2.0` or above, installed in the k8s cluster
 
 If you have `kubectl` configured to access your running k8s cluster, you can then run the following command to install the Messaging Topology Operator:
 
@@ -27,6 +28,10 @@ You can create RabbitMQ resources:
 5. [Vhost](./docs/examples/vhosts)
 6. [Policy](./docs/examples/policies)
 
+## Install without cert-manager
+
+If you do not have cert-manager in your k8s cluster, you need to generate certificates used by admission webhooks yourself and include them in the operator deployment, crds, and webhooks manifests.
+
 ## Contributing
 
 This project follows the typical GitHub pull request model. Before starting any work, please either comment on an [existing issue](https://github.com/rabbitmq/messaging-topology-operator/issues), or file a new one.

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

@@ -0,0 +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 (b *Binding) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		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{}
+
+// no validation logic on create
+func (b *Binding) ValidateCreate() error {
+	return nil
+}
+
+// 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
+}
+
+// no validation logic on delete
+func (b *Binding) ValidateDelete() error {
+	return nil
+}

api/v1alpha1/binding_webhook_test.go

@@ -0,0 +1,73 @@
+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{
+			Vhost:           "/test",
+			Source:          "test",
+			Destination:     "test",
+			DestinationType: "queue",
+			RabbitmqClusterReference: RabbitmqClusterReference{
+				Name:      "some-cluster",
+				Namespace: "default",
+			},
+		},
+	}
+
+	It("does not allow updates on vhost", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.Vhost = "/new-vhost"
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on source", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.Source = "updated-source"
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on destination", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.Destination = "updated-des"
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on destination type", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.DestinationType = "exchange"
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on routing key", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.RoutingKey = "not-allowed"
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on binding arguments", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.Arguments = &runtime.RawExtension{Raw: []byte(`{"new":"new-value"}`)}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+
+	It("does not allow updates on RabbitmqClusterReference", func() {
+		newBinding := oldBinding.DeepCopy()
+		newBinding.Spec.RabbitmqClusterReference = RabbitmqClusterReference{
+			Name:      "new-cluster",
+			Namespace: "default",
+		}
+		Expect(apierrors.IsForbidden(newBinding.ValidateUpdate(&oldBinding))).To(BeTrue())
+	})
+})

config/certmanager/certificate.yaml

@@ -1,16 +1,12 @@
-# The following manifests contain a self-signed issuer CR and a certificate CR.
-# More document can be found at https://docs.cert-manager.io
-# WARNING: Targets CertManager 0.11 check https://docs.cert-manager.io/en/latest/tasks/upgrading/index.html for 
-# breaking changes
-apiVersion: cert-manager.io/v1alpha2
+apiVersion: cert-manager.io/v1
 kind: Issuer
 metadata:
   name: selfsigned-issuer
   namespace: system
 spec:
   selfSigned: {}
 ---
-apiVersion: cert-manager.io/v1alpha2
+apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
   name: serving-cert  # this name should match the one appeared in kustomizeconfig.yaml

config/crd/kustomization.yaml

@@ -10,5 +10,12 @@ resources:
 - bases/rabbitmq.com_policies.yaml
 # +kubebuilder:scaffold:crdkustomizeresource
 
+patchesStrategicMerge:
+- patches/webhook_in_bindings.yaml
+# +kubebuilder:scaffold:crdkustomizewebhookpatch
+
+- patches/cainjection_in_bindings.yaml
+# +kubebuilder:scaffold:crdkustomizecainjectionpatch
+
 configurations:
 - kustomizeconfig.yaml

config/crd/patches/cainjection_in_bindings.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_bindings.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: bindings.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/default/base/kustomization.yaml

@@ -6,6 +6,40 @@ namespace: rabbitmq-system
 resources:
 - ../../crd
 - ../../manager
+- ../../webhook
+- ../../certmanager
+
+patches:
+- manager_webhook_patch.yaml
+- webhookcainjection_patch.yaml
+
+vars:
+- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR
+  objref:
+    kind: Certificate
+    group: cert-manager.io
+    version: v1
+    name: serving-cert # this name should match the one in certificate.yaml
+  fieldref:
+    fieldpath: metadata.namespace
+- name: CERTIFICATE_NAME
+  objref:
+    kind: Certificate
+    group: cert-manager.io
+    version: v1
+    name: serving-cert # this name should match the one in certificate.yaml
+- name: SERVICE_NAMESPACE # namespace of the service
+  objref:
+    kind: Service
+    version: v1
+    name: webhook-service
+  fieldref:
+    fieldpath: metadata.namespace
+- name: SERVICE_NAME
+  objref:
+    kind: Service
+    version: v1
+    name: webhook-service
 
 images:
 - name: controller

config/default/base/manager_webhook_patch.yaml

@@ -0,0 +1,23 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: operator
+  namespace: system
+spec:
+  template:
+    spec:
+      containers:
+      - name: manager
+        ports:
+        - containerPort: 9443
+          name: webhook-server
+          protocol: TCP
+        volumeMounts:
+        - mountPath: /tmp/k8s-webhook-server/serving-certs
+          name: cert
+          readOnly: true
+      volumes:
+      - name: cert
+        secret:
+          defaultMode: 420
+          secretName: webhook-server-cert

config/default/base/webhookcainjection_patch.yaml

@@ -0,0 +1,7 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: validating-webhook-configuration
+  annotations:
+    cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)

config/webhook/kustomizeconfig.yaml

@@ -1,5 +1,3 @@
-# the following config is for teaching kustomize where to look at when substituting vars.
-# It requires kustomize v2.1.0 or newer to work properly.
 nameReference:
 - kind: Service
   version: v1

config/webhook/manifests.yaml

@@ -0,0 +1,28 @@
+
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  creationTimestamp: null
+  name: validating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-rabbitmq-com-v1alpha1-binding
+  failurePolicy: Fail
+  name: vbinding.kb.io
+  rules:
+  - apiGroups:
+    - rabbitmq.com
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - bindings
+  sideEffects: None

config/webhook/service.yaml

@@ -1,4 +1,3 @@
-
 apiVersion: v1
 kind: Service
 metadata:
@@ -9,4 +8,4 @@ spec:
     - port: 443
       targetPort: 9443
   selector:
-    control-plane: controller-manager
+    app.kubernetes.io/name: messaging-topology-operator

main.go

@@ -21,6 +21,7 @@ import (
 
 	rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
 
+	rabbitmqcomv1alpha1 "github.com/rabbitmq/messaging-topology-operator/api/v1alpha1"
 	topologyv1alpha1 "github.com/rabbitmq/messaging-topology-operator/api/v1alpha1"
 	"github.com/rabbitmq/messaging-topology-operator/controllers"
 	// +kubebuilder:scaffold:imports
@@ -118,6 +119,10 @@ func main() {
 		log.Error(err, "unable to create controller", "controller", policyControllerName)
 		os.Exit(1)
 	}
+	if err = (&rabbitmqcomv1alpha1.Binding{}).SetupWebhookWithManager(mgr); err != nil {
+		log.Error(err, "unable to create webhook", "webhook", "Binding")
+		os.Exit(1)
+	}
 	// +kubebuilder:scaffold:builder
 
 	log.Info("starting manager")

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"))
 	})
 })