diff --git a/api/v1alpha2/permission_webhook.go b/api/v1alpha2/permission_webhook.go new file mode 100644 index 00000000..183b74c9 --- /dev/null +++ b/api/v1alpha2/permission_webhook.go @@ -0,0 +1,56 @@ +package v1alpha2 + +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 *Permission) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(p). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-rabbitmq-com-v1alpha2-permission,mutating=false,failurePolicy=fail,groups=rabbitmq.com,resources=permissions,versions=v1alpha2,name=vpermission.kb.io,sideEffects=none,admissionReviewVersions=v1 + +var _ webhook.Validator = &Permission{} + +// no validation on update +func (p *Permission) ValidateCreate() error { + return nil +} + +// do not allow updates on spec.vhost, spec.user, and spec.rabbitmqClusterReference +// updates on spec.permissions are allowed +func (p *Permission) ValidateUpdate(old runtime.Object) error { + oldPermission, ok := old.(*Permission) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a permission but got a %T", old)) + } + + detailMsg := "updates on user, vhost and rabbitmqClusterReference are all forbidden" + if p.Spec.User != oldPermission.Spec.User { + return apierrors.NewForbidden(p.GroupResource(), p.Name, + field.Forbidden(field.NewPath("spec", "user"), detailMsg)) + } + + if p.Spec.Vhost != oldPermission.Spec.Vhost { + return apierrors.NewForbidden(p.GroupResource(), p.Name, + field.Forbidden(field.NewPath("spec", "vhost"), detailMsg)) + } + + if p.Spec.RabbitmqClusterReference != oldPermission.Spec.RabbitmqClusterReference { + return apierrors.NewForbidden(p.GroupResource(), p.Name, + field.Forbidden(field.NewPath("spec", "rabbitmqClusterReference"), detailMsg)) + } + return nil +} + +// no validation on delete +func (p *Permission) ValidateDelete() error { + return nil +} diff --git a/api/v1alpha2/permission_webhook_test.go b/api/v1alpha2/permission_webhook_test.go new file mode 100644 index 00000000..9c9e59d4 --- /dev/null +++ b/api/v1alpha2/permission_webhook_test.go @@ -0,0 +1,66 @@ +package v1alpha2 + +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("permission webhook", func() { + var permission = Permission{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: PermissionSpec{ + User: "test-user", + Vhost: "/a-vhost", + Permissions: VhostPermissions{ + Configure: ".*", + Read: ".*", + Write: ".*", + }, + RabbitmqClusterReference: RabbitmqClusterReference{ + Name: "a-cluster", + }, + }, + } + + It("does not allow updates on user", func() { + newPermission := permission.DeepCopy() + newPermission.Spec.User = "new-user" + Expect(apierrors.IsForbidden(newPermission.ValidateUpdate(&permission))).To(BeTrue()) + }) + + It("does not allow updates on vhost", func() { + newPermission := permission.DeepCopy() + newPermission.Spec.Vhost = "new-vhost" + Expect(apierrors.IsForbidden(newPermission.ValidateUpdate(&permission))).To(BeTrue()) + }) + + It("does not allow updates on RabbitmqClusterReference", func() { + newPermission := permission.DeepCopy() + newPermission.Spec.RabbitmqClusterReference = RabbitmqClusterReference{ + Name: "new-cluster", + } + Expect(apierrors.IsForbidden(newPermission.ValidateUpdate(&permission))).To(BeTrue()) + }) + + It("allows updates on permission.spec.permissions.configure", func() { + newPermission := permission.DeepCopy() + newPermission.Spec.Permissions.Configure = "?" + Expect(newPermission.ValidateUpdate(&permission)).To(Succeed()) + }) + + It("allows updates on permission.spec.permissions.read", func() { + newPermission := permission.DeepCopy() + newPermission.Spec.Permissions.Read = "?" + Expect(newPermission.ValidateUpdate(&permission)).To(Succeed()) + }) + + It("allows updates on permission.spec.permissions.write", func() { + newPermission := permission.DeepCopy() + newPermission.Spec.Permissions.Write = "?" + Expect(newPermission.ValidateUpdate(&permission)).To(Succeed()) + }) +}) diff --git a/config/crd/bases/rabbitmq.com_permissions.yaml b/config/crd/bases/rabbitmq.com_permissions.yaml index 866f925a..81b169bb 100644 --- a/config/crd/bases/rabbitmq.com_permissions.yaml +++ b/config/crd/bases/rabbitmq.com_permissions.yaml @@ -38,8 +38,9 @@ spec: properties: permissions: description: 'Defines a RabbitMQ user permissions in the specified - vhost. All properties are required: configure, write, and read. - For more information, see official doc: https://www.rabbitmq.com/access-control.html#user-management' + vhost. By not setting a property (configure/write/read), it result + in an empty string which does not not match any permission. For + more information, see official doc: https://www.rabbitmq.com/access-control.html#user-management' properties: configure: type: string @@ -47,10 +48,6 @@ spec: type: string write: type: string - required: - - configure - - read - - write type: object rabbitmqClusterReference: description: Reference to the RabbitmqCluster that both the provided diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index e716e1aa..4b9594e2 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -18,7 +18,7 @@ patchesStrategicMerge: - patches/webhook_in_vhosts.yaml - patches/webhook_in_policies.yaml - patches/webhook_in_users.yaml -#- patches/webhook_in_permissions.yaml +- patches/webhook_in_permissions.yaml # +kubebuilder:scaffold:crdkustomizewebhookpatch - patches/cainjection_in_bindings.yaml @@ -27,7 +27,7 @@ patchesStrategicMerge: - patches/cainjection_in_vhosts.yaml - patches/cainjection_in_policies.yaml - patches/cainjection_in_users.yaml -#- patches/cainjection_in_permissions.yaml +- patches/cainjection_in_permissions.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch configurations: diff --git a/config/crd/patches/webhook_in_permissions.yaml b/config/crd/patches/webhook_in_permissions.yaml index a511aae6..c60911cd 100644 --- a/config/crd/patches/webhook_in_permissions.yaml +++ b/config/crd/patches/webhook_in_permissions.yaml @@ -7,11 +7,11 @@ metadata: 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 80a3574e..7bf8b410 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-v1alpha2-permission + failurePolicy: Fail + name: vpermission.kb.io + rules: + - apiGroups: + - rabbitmq.com + apiVersions: + - v1alpha2 + operations: + - CREATE + - UPDATE + resources: + - permissions + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/docs/api/rabbitmq.com.ref.asciidoc b/docs/api/rabbitmq.com.ref.asciidoc index 3bb527d4..67ccc462 100644 --- a/docs/api/rabbitmq.com.ref.asciidoc +++ b/docs/api/rabbitmq.com.ref.asciidoc @@ -643,7 +643,7 @@ VhostList contains a list of Vhost [id="{anchor_prefix}-github.aaakk.us.kg-rabbitmq-messaging-topology-operator-api-v1alpha2-vhostpermissions"] ==== VhostPermissions -Defines a RabbitMQ user permissions in the specified vhost. All properties are required: configure, write, and read. For more information, see official doc: https://www.rabbitmq.com/access-control.html#user-management +Defines a RabbitMQ user permissions in the specified vhost. By not setting a property (configure/write/read), it result in an empty string which does not not match any permission. For more information, see official doc: https://www.rabbitmq.com/access-control.html#user-management .Appears In: **** diff --git a/internal/permissions_test.go b/internal/permissions_test.go index eaedfd22..ce6f5101 100644 --- a/internal/permissions_test.go +++ b/internal/permissions_test.go @@ -19,19 +19,25 @@ var _ = Describe("GeneratePermissions", func() { Spec: topology.PermissionSpec{ User: "a-user", Vhost: "/new-vhost", - Permissions: topology.VhostPermissions{ - Configure: ".*", - Write: ".~", - Read: ".^", - }, }, } }) - It("sets 'Configure', 'Write' and 'Read' correctly", func() { + It("sets 'Configure' correctly", func() { + p.Spec.Permissions.Configure = ".*" rmqPermissions := GeneratePermissions(p) - Expect(rmqPermissions.Read).To(Equal(".^")) - Expect(rmqPermissions.Write).To(Equal(".~")) Expect(rmqPermissions.Configure).To(Equal(".*")) }) + + It("sets 'Write' correctly", func() { + p.Spec.Permissions.Write = ".~" + rmqPermissions := GeneratePermissions(p) + Expect(rmqPermissions.Write).To(Equal(".~")) + }) + + It("sets 'Read' correctly", func() { + p.Spec.Permissions.Read = "^$" + rmqPermissions := GeneratePermissions(p) + Expect(rmqPermissions.Read).To(Equal("^$")) + }) }) diff --git a/main.go b/main.go index dc471aca..93781baa 100644 --- a/main.go +++ b/main.go @@ -27,13 +27,13 @@ import ( ) const ( - vhostControllerName = "vhost-controller" - queueControllerName = "queue-controller" - exchangeControllerName = "exchange-controller" - bindingControllerName = "binding-controller" - userControllerName = "user-controller" - policyControllerName = "policy-controller" - permissionControllerName = "permission-controller" + vhostControllerName = "vhost-controller" + queueControllerName = "queue-controller" + exchangeControllerName = "exchange-controller" + bindingControllerName = "binding-controller" + userControllerName = "user-controller" + policyControllerName = "policy-controller" + permissionControllerName = "permission-controller" ) var ( @@ -122,9 +122,9 @@ func main() { os.Exit(1) } if err = (&controllers.PermissionReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("Permission"), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("Permission"), + Scheme: mgr.GetScheme(), Recorder: mgr.GetEventRecorderFor(permissionControllerName), }).SetupWithManager(mgr); err != nil { log.Error(err, "unable to create controller", "controller", permissionControllerName) @@ -155,6 +155,10 @@ func main() { log.Error(err, "unable to create webhook", "webhook", "User") os.Exit(1) } + if err = (&topology.Permission{}).SetupWebhookWithManager(mgr); err != nil { + log.Error(err, "unable to create webhook", "webhook", "Permission") + os.Exit(1) + } // +kubebuilder:scaffold:builder log.Info("starting manager") diff --git a/system_tests/permissions_system_test.go b/system_tests/permissions_system_test.go index 6df1e9fd..05ce8bd8 100644 --- a/system_tests/permissions_system_test.go +++ b/system_tests/permissions_system_test.go @@ -52,7 +52,7 @@ var _ = Describe("Permission", func() { permission = &topology.Permission{ ObjectMeta: metav1.ObjectMeta{ - Name: "testuser-permission", + Name: "user-permission", Namespace: namespace, }, Spec: topology.PermissionSpec{ @@ -60,7 +60,6 @@ var _ = Describe("Permission", func() { User: username, Permissions: topology.VhostPermissions{ Configure: ".*", - Write: ".*", Read: ".*", }, RabbitmqClusterReference: topology.RabbitmqClusterReference{ @@ -104,6 +103,32 @@ var _ = Describe("Permission", func() { By("setting status.observedGeneration") Expect(updatedPermission.Status.ObservedGeneration).To(Equal(updatedPermission.GetGeneration())) + By("not allowing updates on certain fields") + updateTest := topology.Permission{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: permission.Name, Namespace: permission.Namespace}, &updateTest)).To(Succeed()) + updateTest.Spec.Vhost = "/a-new-vhost" + Expect(k8sClient.Update(ctx, &updateTest).Error()).To(ContainSubstring("spec.vhost: Forbidden: updates on user, vhost and rabbitmqClusterReference are all forbidden")) + + By("updating policy definitions successfully") + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: permission.Name, Namespace: permission.Namespace}, permission)).To(Succeed()) + permission.Spec.Permissions.Write = ".*" + permission.Spec.Permissions.Read = "^$" + Expect(k8sClient.Update(ctx, permission, &client.UpdateOptions{})).To(Succeed()) + + Eventually(func() string { + var err error + fetchedPermissionInfo, err = rabbitClient.GetPermissionsIn(permission.Spec.Vhost, username) + Expect(err).NotTo(HaveOccurred()) + return fetchedPermissionInfo.Write + }, 20, 2).Should(Equal(".*")) + Expect(fetchedPermissionInfo).To(MatchFields(IgnoreExtras, Fields{ + "Vhost": Equal(permission.Spec.Vhost), + "User": Equal(permission.Spec.User), + "Configure": Equal(permission.Spec.Permissions.Configure), + "Read": Equal("^$"), + "Write": Equal(".*"), + })) + By("revoking permissions successfully") Expect(k8sClient.Delete(ctx, permission)).To(Succeed()) Eventually(func() int {