From edf79eb745ab43e95ec4daaf935c06ac73abd87e Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Fri, 19 Mar 2021 09:55:23 +0000 Subject: [PATCH 1/3] Add validation webhook for vhosts.rabbitmq.com - updates on name and rabbitmeClusterReference not allowed - spec.tracing can be updated --- api/v1alpha1/exchange_webhook_test.go | 2 +- api/v1alpha1/vhost_types.go | 8 +++ api/v1alpha1/vhost_webhook.go | 52 +++++++++++++++++++ api/v1alpha1/vhost_webhook_test.go | 46 ++++++++++++++++ config/crd/kustomization.yaml | 2 + config/crd/patches/cainjection_in_vhosts.yaml | 4 +- config/crd/patches/webhook_in_vhosts.yaml | 18 +++---- config/webhook/manifests.yaml | 20 +++++++ main.go | 4 ++ system_tests/vhost_system_tests.go | 6 +++ 10 files changed, 149 insertions(+), 13 deletions(-) create mode 100644 api/v1alpha1/vhost_webhook.go create mode 100644 api/v1alpha1/vhost_webhook_test.go diff --git a/api/v1alpha1/exchange_webhook_test.go b/api/v1alpha1/exchange_webhook_test.go index 12ec778a..3b2d88f5 100644 --- a/api/v1alpha1/exchange_webhook_test.go +++ b/api/v1alpha1/exchange_webhook_test.go @@ -12,7 +12,7 @@ var _ = Describe("exchange webhook", func() { var exchange = Exchange{ ObjectMeta: metav1.ObjectMeta{ - Name: "update-binding", + Name: "test-exchange", }, Spec: ExchangeSpec{ Name: "test", diff --git a/api/v1alpha1/vhost_types.go b/api/v1alpha1/vhost_types.go index b2701abb..b9d79fdf 100644 --- a/api/v1alpha1/vhost_types.go +++ b/api/v1alpha1/vhost_types.go @@ -11,6 +11,7 @@ package v1alpha1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ) // VhostSpec defines the desired state of Vhost @@ -54,6 +55,13 @@ type VhostList struct { Items []Vhost `json:"items"` } +func (v *Vhost) GroupResource() schema.GroupResource { + return schema.GroupResource{ + Group: v.GroupVersionKind().Group, + Resource: v.GroupVersionKind().Kind, + } +} + func init() { SchemeBuilder.Register(&Vhost{}, &VhostList{}) } diff --git a/api/v1alpha1/vhost_webhook.go b/api/v1alpha1/vhost_webhook.go new file mode 100644 index 00000000..c0c4469a --- /dev/null +++ b/api/v1alpha1/vhost_webhook.go @@ -0,0 +1,52 @@ +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 *Vhost) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1alpha1-vhost,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=vhosts,versions=v1alpha1,name=vvhost.kb.io,sideEffects=none,admissionReviewVersions=v1 + +var _ webhook.Validator = &Vhost{} + +// no validation on create +func (v *Vhost) ValidateCreate() error { + return nil +} + +// returns error type 'forbidden' for updates on vhost name and rabbitmqClusterReference +// vhost.spec.tracing can be updated +func (v *Vhost) ValidateUpdate(old runtime.Object) error { + oldVhost, ok := old.(*Vhost) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a vhost but got a %T", old)) + } + + detailMsg := "updates on name and rabbitmqClusterReference are all forbidden" + if v.Spec.Name != oldVhost.Spec.Name { + return apierrors.NewForbidden(v.GroupResource(), v.Name, + field.Forbidden(field.NewPath("spec", "name"), detailMsg)) + } + + if v.Spec.RabbitmqClusterReference != oldVhost.Spec.RabbitmqClusterReference { + return apierrors.NewForbidden(v.GroupResource(), v.Name, + field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) + } + + return nil +} + +// no validation on delete +func (v *Vhost) ValidateDelete() error { + return nil +} diff --git a/api/v1alpha1/vhost_webhook_test.go b/api/v1alpha1/vhost_webhook_test.go new file mode 100644 index 00000000..8ee4d2cc --- /dev/null +++ b/api/v1alpha1/vhost_webhook_test.go @@ -0,0 +1,46 @@ +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" +) + +var _ = Describe("vhost webhook", func() { + + var vhost = Vhost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vhost", + }, + Spec: VhostSpec{ + Name: "test", + Tracing: false, + RabbitmqClusterReference: RabbitmqClusterReference{ + Name: "a-cluster", + Namespace: "default", + }, + }, + } + + It("does not allow updates on vhost name", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newVhost.ValidateUpdate(&vhost))).To(BeTrue()) + }) + + It("does not allow updates on RabbitmqClusterReference", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + Namespace: "default", + } + Expect(apierrors.IsForbidden(newVhost.ValidateUpdate(&vhost))).To(BeTrue()) + }) + + It("allows updates on vhost.spec.tracing", func() { + newVhost := vhost.DeepCopy() + newVhost.Spec.Tracing = true + Expect(newVhost.ValidateUpdate(&vhost)).To(Succeed()) + }) +}) diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 101e9a06..08b380bd 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -14,11 +14,13 @@ patchesStrategicMerge: - patches/webhook_in_bindings.yaml - patches/webhook_in_queues.yaml - patches/webhook_in_exchanges.yaml +- patches/webhook_in_vhosts.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch - patches/cainjection_in_bindings.yaml - patches/cainjection_in_queues.yaml - patches/webhook_in_exchanges.yaml +- patches/webhook_in_vhosts.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch configurations: diff --git a/config/crd/patches/cainjection_in_vhosts.yaml b/config/crd/patches/cainjection_in_vhosts.yaml index d7728b32..5e867a62 100644 --- a/config/crd/patches/cainjection_in_vhosts.yaml +++ b/config/crd/patches/cainjection_in_vhosts.yaml @@ -1,6 +1,4 @@ -# 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/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: diff --git a/config/crd/patches/webhook_in_vhosts.yaml b/config/crd/patches/webhook_in_vhosts.yaml index a61dd6e5..4b328a76 100644 --- a/config/crd/patches/webhook_in_vhosts.yaml +++ b/config/crd/patches/webhook_in_vhosts.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/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: vhosts.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 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 7ff5d598..29a905af 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -66,3 +66,23 @@ webhooks: resources: - queues sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-rabbitmq-com-v1alpha1-vhost + failurePolicy: Fail + name: vvhost.kb.io + rules: + - apiGroups: + - rabbitmq.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - vhosts + sideEffects: None diff --git a/main.go b/main.go index fdc0ccd4..04d72bc6 100644 --- a/main.go +++ b/main.go @@ -130,6 +130,10 @@ func main() { log.Error(err, "unable to create webhook", "webhook", "Exchange") os.Exit(1) } + if err = (&topologyv1alpha1.Vhost{}).SetupWebhookWithManager(mgr); err != nil { + log.Error(err, "unable to create webhook", "webhook", "Vhost") + os.Exit(1) + } // +kubebuilder:scaffold:builder log.Info("starting manager") diff --git a/system_tests/vhost_system_tests.go b/system_tests/vhost_system_tests.go index 6659b45c..17986858 100644 --- a/system_tests/vhost_system_tests.go +++ b/system_tests/vhost_system_tests.go @@ -62,6 +62,12 @@ var _ = Describe("vhost", func() { By("setting status.observedGeneration") Expect(updatedVhost.Status.ObservedGeneration).To(Equal(updatedVhost.GetGeneration())) + By("not allowing updates on certain fields") + updateTest := topologyv1alpha1.Vhost{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: vhost.Name, Namespace: vhost.Namespace}, &updateTest)).To(Succeed()) + updateTest.Spec.Name = "new-name" + Expect(k8sClient.Update(ctx, &updateTest).Error()).To(ContainSubstring("spec.name: Forbidden: updates on name and rabbitmqClusterReference are all forbidden")) + By("deleting a vhost") Expect(k8sClient.Delete(ctx, vhost)).To(Succeed()) var err error From b15855d06a6c58bf6850f6ebbdcbdf9e2623a050 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Fri, 19 Mar 2021 12:15:45 +0000 Subject: [PATCH 2/3] Add validation webhook for policies.rabbitmq.com --- api/v1alpha1/policy_types.go | 8 ++ api/v1alpha1/policy_webhook.go | 55 ++++++++++++++ api/v1alpha1/policy_webhook_test.go | 73 +++++++++++++++++++ config/crd/kustomization.yaml | 6 +- .../crd/patches/cainjection_in_policies.yaml | 2 +- config/crd/patches/webhook_in_policies.yaml | 18 ++--- config/webhook/manifests.yaml | 20 +++++ main.go | 4 + system_tests/policy_system_tests.go | 8 +- 9 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 api/v1alpha1/policy_webhook.go create mode 100644 api/v1alpha1/policy_webhook_test.go diff --git a/api/v1alpha1/policy_types.go b/api/v1alpha1/policy_types.go index 971d09a9..58e5cc7d 100644 --- a/api/v1alpha1/policy_types.go +++ b/api/v1alpha1/policy_types.go @@ -3,6 +3,7 @@ package v1alpha1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ) // PolicySpec defines the desired state of Policy @@ -66,6 +67,13 @@ type PolicyList struct { Items []Policy `json:"items"` } +func (p *Policy) GroupResource() schema.GroupResource { + return schema.GroupResource{ + Group: p.GroupVersionKind().Group, + Resource: p.GroupVersionKind().Kind, + } +} + func init() { SchemeBuilder.Register(&Policy{}, &PolicyList{}) } diff --git a/api/v1alpha1/policy_webhook.go b/api/v1alpha1/policy_webhook.go new file mode 100644 index 00000000..88fae908 --- /dev/null +++ b/api/v1alpha1/policy_webhook.go @@ -0,0 +1,55 @@ +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 (p *Policy) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(p). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1alpha1-policy,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=policies,versions=v1alpha1,name=vpolicy.kb.io,sideEffects=none,admissionReviewVersions=v1 + +var _ webhook.Validator = &Policy{} + +// no validation on create +func (p *Policy) ValidateCreate() error { + return nil +} + +// returns error type 'forbidden' for updates on policy name, vhost and rabbitmqClusterReference +func (p *Policy) ValidateUpdate(old runtime.Object) error { + oldPolicy, ok := old.(*Policy) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a policy but got a %T", old)) + } + + detailMsg := "updates on name, vhost and rabbitmqClusterReference are all forbidden" + if p.Spec.Name != oldPolicy.Spec.Name { + return apierrors.NewForbidden(p.GroupResource(), p.Name, + field.Forbidden(field.NewPath("spec", "name"), detailMsg)) + } + + if p.Spec.Vhost != oldPolicy.Spec.Vhost { + return apierrors.NewForbidden(p.GroupResource(), p.Name, + field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) + } + + if p.Spec.RabbitmqClusterReference != oldPolicy.Spec.RabbitmqClusterReference { + return apierrors.NewForbidden(p.GroupResource(), p.Name, + field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) + } + return nil +} + +// no validation on delete +func (p *Policy) ValidateDelete() error { + return nil +} diff --git a/api/v1alpha1/policy_webhook_test.go b/api/v1alpha1/policy_webhook_test.go new file mode 100644 index 00000000..4d544c79 --- /dev/null +++ b/api/v1alpha1/policy_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("policy webhook", func() { + var policy = Policy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: PolicySpec{ + Name: "test", + Vhost: "/test", + Pattern: "a-pattern", + ApplyTo: "all", + Priority: 0, + RabbitmqClusterReference: RabbitmqClusterReference{ + Name: "a-cluster", + Namespace: "default", + }, + }, + } + + It("does not allow updates on policy name", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Name = "new-name" + Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + }) + + It("does not allow updates on vhost", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Vhost = "new-vhost" + Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + }) + + It("does not allow updates on RabbitmqClusterReference", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + Namespace: "default", + } + Expect(apierrors.IsForbidden(newPolicy.ValidateUpdate(&policy))).To(BeTrue()) + }) + + It("allows updates on policy.spec.pattern", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Pattern = "new-pattern" + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) + + It("allows updates on policy.spec.applyTo", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.ApplyTo = "queues" + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) + + It("allows updates on policy.spec.priority", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Priority = 1000 + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) + + It("allows updates on policy.spec.definition", func() { + newPolicy := policy.DeepCopy() + newPolicy.Spec.Definition = &runtime.RawExtension{Raw: []byte(`{"key":"new-definition-value"}`)} + Expect(newPolicy.ValidateUpdate(&policy)).To(Succeed()) + }) +}) diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 08b380bd..baadb96b 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -15,12 +15,14 @@ patchesStrategicMerge: - patches/webhook_in_queues.yaml - patches/webhook_in_exchanges.yaml - patches/webhook_in_vhosts.yaml +- patches/webhook_in_policies.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch - patches/cainjection_in_bindings.yaml - patches/cainjection_in_queues.yaml -- patches/webhook_in_exchanges.yaml -- patches/webhook_in_vhosts.yaml +- patches/cainjection_in_exchanges.yaml +- patches/cainjection_in_vhosts.yaml +- patches/cainjection_in_policies.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch configurations: diff --git a/config/crd/patches/cainjection_in_policies.yaml b/config/crd/patches/cainjection_in_policies.yaml index b88ffb6f..654d9839 100644 --- a/config/crd/patches/cainjection_in_policies.yaml +++ b/config/crd/patches/cainjection_in_policies.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/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: diff --git a/config/crd/patches/webhook_in_policies.yaml b/config/crd/patches/webhook_in_policies.yaml index 35cd2a57..b5db75f1 100644 --- a/config/crd/patches/webhook_in_policies.yaml +++ b/config/crd/patches/webhook_in_policies.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/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: policies.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 diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 29a905af..f796c6c0 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -46,6 +46,26 @@ webhooks: resources: - exchanges sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-rabbitmq-com-v1alpha1-policy + failurePolicy: Fail + name: vpolicy.kb.io + rules: + - apiGroups: + - rabbitmq.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - policies + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/main.go b/main.go index 04d72bc6..57348176 100644 --- a/main.go +++ b/main.go @@ -134,6 +134,10 @@ func main() { log.Error(err, "unable to create webhook", "webhook", "Vhost") os.Exit(1) } + if err = (&topologyv1alpha1.Policy{}).SetupWebhookWithManager(mgr); err != nil { + log.Error(err, "unable to create webhook", "webhook", "Policy") + os.Exit(1) + } // +kubebuilder:scaffold:builder log.Info("starting manager") diff --git a/system_tests/policy_system_tests.go b/system_tests/policy_system_tests.go index bd343a5c..18c7f49d 100644 --- a/system_tests/policy_system_tests.go +++ b/system_tests/policy_system_tests.go @@ -78,7 +78,13 @@ var _ = Describe("Policy", func() { By("setting status.observedGeneration") Expect(updatedPolicy.Status.ObservedGeneration).To(Equal(updatedPolicy.GetGeneration())) - By("updating policy") + By("not allowing updates on certain fields") + updateTest := topologyv1alpha1.Policy{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: policy.Name, Namespace: policy.Namespace}, &updateTest)).To(Succeed()) + updateTest.Spec.Vhost = "/a-new-vhost" + Expect(k8sClient.Update(ctx, &updateTest).Error()).To(ContainSubstring("spec.vhost: Forbidden: updates on name, vhost and rabbitmqClusterReference are all forbidden")) + + By("updating policy definitions successfully") Expect(k8sClient.Get(ctx, types.NamespacedName{Name: policy.Name, Namespace: policy.Namespace}, policy)).To(Succeed()) policy.Spec.Definition = &runtime.RawExtension{ Raw: []byte(`{"ha-mode":"exactly", From db1ffb243740d9b6ccea92171cb795cc68ba3765 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Fri, 19 Mar 2021 13:24:03 +0000 Subject: [PATCH 3/3] Add validation webhook for users.rabbitmq.com --- api/v1alpha1/user_types.go | 8 ++++ api/v1alpha1/user_webhook.go | 45 ++++++++++++++++++++ api/v1alpha1/user_webhook_test.go | 38 +++++++++++++++++ config/crd/kustomization.yaml | 2 + config/crd/patches/cainjection_in_users.yaml | 2 +- config/crd/patches/webhook_in_users.yaml | 2 +- config/webhook/manifests.yaml | 20 +++++++++ main.go | 4 ++ system_tests/user_system_tests.go | 6 +++ 9 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 api/v1alpha1/user_webhook.go create mode 100644 api/v1alpha1/user_webhook_test.go diff --git a/api/v1alpha1/user_types.go b/api/v1alpha1/user_types.go index 8e772a72..e646b5fa 100644 --- a/api/v1alpha1/user_types.go +++ b/api/v1alpha1/user_types.go @@ -12,6 +12,7 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ) // UserSpec defines the desired state of User. @@ -74,6 +75,13 @@ type UserList struct { Items []User `json:"items"` } +func (u *User) GroupResource() schema.GroupResource { + return schema.GroupResource{ + Group: u.GroupVersionKind().Group, + Resource: u.GroupVersionKind().Kind, + } +} + func init() { SchemeBuilder.Register(&User{}, &UserList{}) } diff --git a/api/v1alpha1/user_webhook.go b/api/v1alpha1/user_webhook.go new file mode 100644 index 00000000..9134b167 --- /dev/null +++ b/api/v1alpha1/user_webhook.go @@ -0,0 +1,45 @@ +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 (u *User) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(u). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1alpha1-user,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=users,versions=v1alpha1,name=vuser.kb.io,sideEffects=none,admissionReviewVersions=v1 + +var _ webhook.Validator = &User{} + +// no validation on create +func (u *User) ValidateCreate() error { + return nil +} + +// returns error type 'forbidden' for updates on rabbitmqClusterReference +// user.spec.tags can be updated +func (u *User) ValidateUpdate(old runtime.Object) error { + oldUser, ok := old.(*User) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a user but got a %T", old)) + } + + if u.Spec.RabbitmqClusterReference != oldUser.Spec.RabbitmqClusterReference { + return apierrors.NewForbidden(u.GroupResource(), u.Name, + field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), "update on rabbitmqClusterReference is forbidden")) + } + return nil +} + +// no validation on delete +func (u *User) ValidateDelete() error { + return nil +} diff --git a/api/v1alpha1/user_webhook_test.go b/api/v1alpha1/user_webhook_test.go new file mode 100644 index 00000000..75bf588b --- /dev/null +++ b/api/v1alpha1/user_webhook_test.go @@ -0,0 +1,38 @@ +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" +) + +var _ = Describe("user webhook", func() { + var user = User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: UserSpec{ + Tags: []UserTag{"policymaker"}, + RabbitmqClusterReference: RabbitmqClusterReference{ + Name: "a-cluster", + Namespace: "default", + }, + }, + } + + It("does not allow updates on RabbitmqClusterReference", func() { + new := user.DeepCopy() + new.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + Namespace: "default", + } + Expect(apierrors.IsForbidden(new.ValidateUpdate(&user))).To(BeTrue()) + }) + + It("allows update on tags", func() { + new := user.DeepCopy() + new.Spec.Tags = []UserTag{"monitoring"} + Expect(new.ValidateUpdate(&user)).To(Succeed()) + }) +}) diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index baadb96b..bfbae70d 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -16,6 +16,7 @@ patchesStrategicMerge: - patches/webhook_in_exchanges.yaml - patches/webhook_in_vhosts.yaml - patches/webhook_in_policies.yaml +- patches/webhook_in_users.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch - patches/cainjection_in_bindings.yaml @@ -23,6 +24,7 @@ patchesStrategicMerge: - patches/cainjection_in_exchanges.yaml - patches/cainjection_in_vhosts.yaml - patches/cainjection_in_policies.yaml +- patches/cainjection_in_users.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch configurations: diff --git a/config/crd/patches/cainjection_in_users.yaml b/config/crd/patches/cainjection_in_users.yaml index 655823d5..bd4128db 100644 --- a/config/crd/patches/cainjection_in_users.yaml +++ b/config/crd/patches/cainjection_in_users.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/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: diff --git a/config/crd/patches/webhook_in_users.yaml b/config/crd/patches/webhook_in_users.yaml index 489f953f..d43c889b 100644 --- a/config/crd/patches/webhook_in_users.yaml +++ b/config/crd/patches/webhook_in_users.yaml @@ -1,6 +1,6 @@ # The following patch enables conversion webhook for CRD # CRD conversion requires k8s 1.13 or later. -apiVersion: apiextensions.k8s.io/v1beta1 +apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: users.rabbitmq.com diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index f796c6c0..d5d59c42 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -86,6 +86,26 @@ webhooks: resources: - queues sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-rabbitmq-com-v1alpha1-user + failurePolicy: Fail + name: vuser.kb.io + rules: + - apiGroups: + - rabbitmq.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - users + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/main.go b/main.go index 57348176..529dfd69 100644 --- a/main.go +++ b/main.go @@ -138,6 +138,10 @@ func main() { log.Error(err, "unable to create webhook", "webhook", "Policy") os.Exit(1) } + if err = (&topologyv1alpha1.User{}).SetupWebhookWithManager(mgr); err != nil { + log.Error(err, "unable to create webhook", "webhook", "User") + os.Exit(1) + } // +kubebuilder:scaffold:builder log.Info("starting manager") diff --git a/system_tests/user_system_tests.go b/system_tests/user_system_tests.go index 664cdf9b..5baea645 100644 --- a/system_tests/user_system_tests.go +++ b/system_tests/user_system_tests.go @@ -115,6 +115,12 @@ var _ = Describe("Users", func() { By("setting status.observedGeneration") Expect(updatedUser.Status.ObservedGeneration).To(Equal(updatedUser.GetGeneration())) + By("not allowing updates on certain fields") + updateTest := topologyv1alpha1.User{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: user.Name, Namespace: user.Namespace}, &updateTest)).To(Succeed()) + updateTest.Spec.RabbitmqClusterReference = topologyv1alpha1.RabbitmqClusterReference{Name: "a-new-cluster", Namespace: "default"} + Expect(k8sClient.Update(ctx, &updateTest).Error()).To(ContainSubstring("spec.rabbitmqClusterReference: Forbidden: update on rabbitmqClusterReference is forbidden")) + By("deleting user") Expect(k8sClient.Delete(ctx, user)).To(Succeed()) Eventually(func() error {