Add validation webhook for permissions.rabbitmq.com

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
+}

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())
+	})
+})

config/crd/bases/rabbitmq.com_permissions.yaml

@@ -38,19 +38,16 @@ 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
                   read:
                     type: string
                   write:
                     type: string
-                required:
-                - configure
-                - read
-                - write
                 type: object
               rabbitmqClusterReference:
                 description: Reference to the RabbitmqCluster that both the provided

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:

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

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:

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:
 ****

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("^$"))
+	})
 })

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

system_tests/permissions_system_test.go

@@ -52,15 +52,14 @@ var _ = Describe("Permission", func() {
 
 		permission = &topology.Permission{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "testuser-permission",
+				Name:      "user-permission",
 				Namespace: namespace,
 			},
 			Spec: topology.PermissionSpec{
 				Vhost: "/",
 				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 {