pr feedback 1

Author: lmiccini

apis/bases/rabbitmq.openstack.org_rabbitmqpolicies.yaml

@@ -85,8 +85,8 @@ spec:
                 description: RabbitmqClusterName - the name of the RabbitMQ cluster
                 type: string
               vhostRef:
-                description: VhostRef - reference to the RabbitMQVhost resource (defaults
-                  to "/")
+                description: VhostRef - reference to the RabbitMQVhost resource (if
+                  empty, uses default vhost "/")
                 type: string
             required:
             - definition

apis/bases/rabbitmq.openstack.org_rabbitmqusers.yaml

@@ -65,15 +65,18 @@ spec:
                 properties:
                   configure:
                     default: .*
-                    description: Configure - configure permission regex
+                    description: Configure - configure permission regex (default ".*"
+                      allows all, "" denies all)
                     type: string
                   read:
                     default: .*
-                    description: Read - read permission regex
+                    description: Read - read permission regex (default ".*" allows
+                      all, "" denies all)
                     type: string
                   write:
                     default: .*
-                    description: Write - write permission regex
+                    description: Write - write permission regex (default ".*" allows
+                      all, "" denies all)
                     type: string
                 type: object
               rabbitmqClusterName:

apis/bases/rabbitmq.openstack.org_rabbitmqvhosts.yaml

@@ -60,6 +60,7 @@ spec:
               name:
                 default: /
                 description: Name - the vhost name in RabbitMQ (defaults to "/")
+                minLength: 1
                 type: string
               rabbitmqClusterName:
                 description: RabbitmqClusterName - the name of the RabbitMQ cluster

apis/rabbitmq/v1beta1/conditions.go

@@ -23,6 +23,9 @@ import (
 const (
 	// TransportURLReadyCondition Status=True condition which indicates if TransportURL is configured and operational
 	TransportURLReadyCondition condition.Type = "TransportURLReady"
+
+	// TransportURLFinalizer - finalizer to add to RabbitMQUsers owned by TransportURL
+	TransportURLFinalizer = "transporturl.rabbitmq.openstack.org/finalizer"
 )
 
 // TransportURL Reasons used by API objects.

apis/rabbitmq/v1beta1/rabbitmq_types.go

@@ -43,6 +43,14 @@ const (
 	CrMaxLengthCorrection   = 11
 	errInvalidOverride      = "invalid spec override (%s)"
 	warnOverrideStatefulSet = "%s: is deprecated and will be removed in a future API version"
+
+	// Queue types
+	// QueueTypeMirrored - mirrored queue type
+	QueueTypeMirrored = "Mirrored"
+	// QueueTypeQuorum - quorum queue type
+	QueueTypeQuorum = "Quorum"
+	// QueueTypeNone - no special queue type
+	QueueTypeNone = "None"
 )
 
 // PodOverride defines per-pod service configurations

apis/rabbitmq/v1beta1/rabbitmqpolicy_types.go

@@ -29,7 +29,7 @@ type RabbitMQPolicySpec struct {
 	RabbitmqClusterName string `json:"rabbitmqClusterName"`
 
 	// +kubebuilder:validation:Optional
-	// VhostRef - reference to the RabbitMQVhost resource (defaults to "/")
+	// VhostRef - reference to the RabbitMQVhost resource (if empty, uses default vhost "/")
 	VhostRef string `json:"vhostRef,omitempty"`
 
 	// +kubebuilder:validation:Optional
@@ -104,15 +104,15 @@ func (instance RabbitMQPolicy) IsReady() bool {
 }
 
 const (
-	// PolicyReadyCondition indicates that the policy is ready
-	PolicyReadyCondition condition.Type = "PolicyReady"
+	// RabbitMQPolicyReadyCondition indicates that the policy is ready
+	RabbitMQPolicyReadyCondition condition.Type = "RabbitMQPolicyReady"
 
-	// PolicyReadyMessage is the message for the PolicyReady condition
-	PolicyReadyMessage = "RabbitMQ policy is ready"
+	// RabbitMQPolicyReadyMessage is the message for the RabbitMQPolicyReady condition
+	RabbitMQPolicyReadyMessage = "RabbitMQ policy is ready"
 
-	// PolicyReadyInitMessage is the message for the PolicyReady condition when not started
-	PolicyReadyInitMessage = "RabbitMQ policy not started"
+	// RabbitMQPolicyReadyInitMessage is the message for the RabbitMQPolicyReady condition when not started
+	RabbitMQPolicyReadyInitMessage = "RabbitMQ policy not started"
 
-	// PolicyReadyErrorMessage is the message format for the PolicyReady condition when an error occurs
-	PolicyReadyErrorMessage = "RabbitMQ policy error occurred %s"
+	// RabbitMQPolicyReadyErrorMessage is the message format for the RabbitMQPolicyReady condition when an error occurs
+	RabbitMQPolicyReadyErrorMessage = "RabbitMQ policy error occurred %s"
 )

apis/rabbitmq/v1beta1/rabbitmqpolicy_webhook.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2024.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+var rabbitmqpolicylog = logf.Log.WithName("rabbitmqpolicy-resource")
+
+//+kubebuilder:webhook:path=/mutate-rabbitmq-openstack-org-v1beta1-rabbitmqpolicy,mutating=true,failurePolicy=fail,sideEffects=None,groups=rabbitmq.openstack.org,resources=rabbitmqpolicies,verbs=create;update,versions=v1beta1,name=mrabbitmqpolicy.kb.io,admissionReviewVersions=v1
+
+// Default implements defaulting for RabbitMQPolicy
+func (r *RabbitMQPolicy) Default(_ client.Client) {
+	rabbitmqpolicylog.Info("default", "name", r.Name)
+
+	// Default the policy name to the CR name if not specified
+	if r.Spec.Name == "" {
+		r.Spec.Name = r.Name
+	}
+}
+
+//+kubebuilder:webhook:path=/validate-rabbitmq-openstack-org-v1beta1-rabbitmqpolicy,mutating=false,failurePolicy=fail,sideEffects=None,groups=rabbitmq.openstack.org,resources=rabbitmqpolicies,verbs=create;update,versions=v1beta1,name=vrabbitmqpolicy.kb.io,admissionReviewVersions=v1
+
+// ValidateCreate validates the RabbitMQPolicy on creation
+func (r *RabbitMQPolicy) ValidateCreate(_ client.Client) (admission.Warnings, error) {
+	rabbitmqpolicylog.Info("validate create", "name", r.Name)
+	return nil, nil
+}
+
+// ValidateUpdate validates the RabbitMQPolicy on update
+func (r *RabbitMQPolicy) ValidateUpdate(_ client.Client, old runtime.Object) (admission.Warnings, error) {
+	rabbitmqpolicylog.Info("validate update", "name", r.Name)
+
+	oldPolicy, ok := old.(*RabbitMQPolicy)
+	if !ok {
+		return nil, fmt.Errorf("expected RabbitMQPolicy but got %T", old)
+	}
+
+	// Prevent changing the policy name after creation
+	if r.Spec.Name != oldPolicy.Spec.Name {
+		return nil, apierrors.NewInvalid(
+			schema.GroupKind{Group: "rabbitmq.openstack.org", Kind: "RabbitMQPolicy"},
+			r.Name,
+			field.ErrorList{
+				field.Forbidden(
+					field.NewPath("spec", "name"),
+					"policy name cannot be changed after creation",
+				),
+			},
+		)
+	}
+
+	return nil, nil
+}
+
+// ValidateDelete validates the RabbitMQPolicy on deletion
+func (r *RabbitMQPolicy) ValidateDelete(_ client.Client) (admission.Warnings, error) {
+	return nil, nil
+}

apis/rabbitmq/v1beta1/rabbitmquser_types.go

@@ -25,17 +25,17 @@ import (
 type RabbitMQUserPermissions struct {
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=".*"
-	// Configure - configure permission regex
+	// Configure - configure permission regex (default ".*" allows all, "" denies all)
 	Configure string `json:"configure"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=".*"
-	// Write - write permission regex
+	// Write - write permission regex (default ".*" allows all, "" denies all)
 	Write string `json:"write"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=".*"
-	// Read - read permission regex
+	// Read - read permission regex (default ".*" allows all, "" denies all)
 	Read string `json:"read"`
 }
 
@@ -55,7 +55,7 @@ type RabbitMQUserSpec struct {
 
 	// +kubebuilder:validation:Optional
 	// Permissions - user permissions on the vhost
-	Permissions RabbitMQUserPermissions `json:"permissions,omitempty"`
+	Permissions RabbitMQUserPermissions `json:"permissions"`
 
 	// +kubebuilder:validation:Optional
 	// Tags - RabbitMQ user tags
@@ -120,15 +120,15 @@ const (
 	// UserFinalizer - finalizer to protect user from deletion when owned by TransportURL
 	UserFinalizer = "rabbitmquser.rabbitmq.openstack.org/finalizer"
 
-	// UserReadyCondition indicates that the user is ready
-	UserReadyCondition condition.Type = "UserReady"
+	// RabbitMQUserReadyCondition indicates that the user is ready
+	RabbitMQUserReadyCondition condition.Type = "RabbitMQUserReady"
 
-	// UserReadyMessage is the message for the UserReady condition
-	UserReadyMessage = "RabbitMQ user is ready"
+	// RabbitMQUserReadyMessage is the message for the RabbitMQUserReady condition
+	RabbitMQUserReadyMessage = "RabbitMQ user is ready"
 
-	// UserReadyInitMessage is the message for the UserReady condition when not started
-	UserReadyInitMessage = "RabbitMQ user not started"
+	// RabbitMQUserReadyInitMessage is the message for the RabbitMQUserReady condition when not started
+	RabbitMQUserReadyInitMessage = "RabbitMQ user not started"
 
-	// UserReadyErrorMessage is the message format for the UserReady condition when an error occurs
-	UserReadyErrorMessage = "RabbitMQ user error occurred %s"
+	// RabbitMQUserReadyErrorMessage is the message format for the RabbitMQUserReady condition when an error occurs
+	RabbitMQUserReadyErrorMessage = "RabbitMQ user error occurred %s"
 )

apis/rabbitmq/v1beta1/rabbitmquser_webhook.go

@@ -31,6 +31,18 @@ import (
 
 var rabbitmquserlog = logf.Log.WithName("rabbitmquser-resource")
 
+//+kubebuilder:webhook:path=/mutate-rabbitmq-openstack-org-v1beta1-rabbitmquser,mutating=true,failurePolicy=fail,sideEffects=None,groups=rabbitmq.openstack.org,resources=rabbitmqusers,verbs=create;update,versions=v1beta1,name=mrabbitmquser.kb.io,admissionReviewVersions=v1
+
+// Default implements defaulting for RabbitMQUser
+func (r *RabbitMQUser) Default(_ client.Client) {
+	rabbitmquserlog.Info("default", "name", r.Name)
+
+	// Default the username to the CR name if not specified
+	if r.Spec.Username == "" {
+		r.Spec.Username = r.Name
+	}
+}
+
 //+kubebuilder:webhook:path=/validate-rabbitmq-openstack-org-v1beta1-rabbitmquser,mutating=false,failurePolicy=fail,sideEffects=None,groups=rabbitmq.openstack.org,resources=rabbitmqusers,verbs=create;update,versions=v1beta1,name=vrabbitmquser.kb.io,admissionReviewVersions=v1
 
 // ValidateCreate validates the RabbitMQUser on creation
@@ -40,8 +52,28 @@ func (r *RabbitMQUser) ValidateCreate(k8sClient client.Client) (admission.Warnin
 }
 
 // ValidateUpdate validates the RabbitMQUser on update
-func (r *RabbitMQUser) ValidateUpdate(k8sClient client.Client, _ runtime.Object) (admission.Warnings, error) {
+func (r *RabbitMQUser) ValidateUpdate(k8sClient client.Client, old runtime.Object) (admission.Warnings, error) {
 	rabbitmquserlog.Info("validate update", "name", r.Name)
+
+	oldUser, ok := old.(*RabbitMQUser)
+	if !ok {
+		return nil, fmt.Errorf("expected RabbitMQUser but got %T", old)
+	}
+
+	// Prevent changing the username after creation
+	if r.Spec.Username != oldUser.Spec.Username {
+		return nil, apierrors.NewInvalid(
+			schema.GroupKind{Group: "rabbitmq.openstack.org", Kind: "RabbitMQUser"},
+			r.Name,
+			field.ErrorList{
+				field.Forbidden(
+					field.NewPath("spec", "username"),
+					"username cannot be changed after creation",
+				),
+			},
+		)
+	}
+
 	return nil, r.validateUniqueUsername(k8sClient)
 }
 
@@ -52,16 +84,6 @@ func (r *RabbitMQUser) ValidateDelete(client.Client) (admission.Warnings, error)
 
 // validateUniqueUsername checks that no other RabbitMQUser exists with the same username, vhost, and cluster
 func (r *RabbitMQUser) validateUniqueUsername(k8sClient client.Client) error {
-	if k8sClient == nil {
-		return nil
-	}
-
-	// Determine the username that will be used
-	username := r.Spec.Username
-	if username == "" {
-		username = r.Name
-	}
-
 	// List all RabbitMQUsers in the same namespace
 	userList := &RabbitMQUserList{}
 	if err := k8sClient.List(context.TODO(), userList, client.InNamespace(r.Namespace)); err != nil {
@@ -85,22 +107,16 @@ func (r *RabbitMQUser) validateUniqueUsername(k8sClient client.Client) error {
 			continue
 		}
 
-		// Determine the other user's username
-		otherUsername := user.Spec.Username
-		if otherUsername == "" {
-			otherUsername = user.Name
-		}
-
 		// If usernames match, reject
-		if username == otherUsername {
+		if r.Spec.Username == user.Spec.Username {
 			return apierrors.NewInvalid(
 				schema.GroupKind{Group: "rabbitmq.openstack.org", Kind: "RabbitMQUser"},
 				r.Name,
 				field.ErrorList{
 					field.Duplicate(
 						field.NewPath("spec", "username"),
 						fmt.Sprintf("username %q already exists in vhost %q on cluster %q (existing RabbitMQUser: %s)",
-							username, r.Spec.VhostRef, r.Spec.RabbitmqClusterName, user.Name),
+							r.Spec.Username, r.Spec.VhostRef, r.Spec.RabbitmqClusterName, user.Name),
 					),
 				},
 			)

apis/rabbitmq/v1beta1/rabbitmqvhost_types.go

@@ -28,6 +28,7 @@ type RabbitMQVhostSpec struct {
 	RabbitmqClusterName string `json:"rabbitmqClusterName"`
 
 	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:default="/"
 	// Name - the vhost name in RabbitMQ (defaults to "/")
 	Name string `json:"name"`
@@ -81,15 +82,15 @@ const (
 	// VhostFinalizer - finalizer to protect vhost from deletion when owned by TransportURL
 	VhostFinalizer = "rabbitmqvhost.rabbitmq.openstack.org/finalizer"
 
-	// VhostReadyCondition indicates that the vhost is ready
-	VhostReadyCondition condition.Type = "VhostReady"
+	// RabbitMQVhostReadyCondition indicates that the vhost is ready
+	RabbitMQVhostReadyCondition condition.Type = "RabbitMQVhostReady"
 
-	// VhostReadyMessage is the message for the VhostReady condition
-	VhostReadyMessage = "RabbitMQ vhost is ready"
+	// RabbitMQVhostReadyMessage is the message for the RabbitMQVhostReady condition
+	RabbitMQVhostReadyMessage = "RabbitMQ vhost is ready"
 
-	// VhostReadyInitMessage is the message for the VhostReady condition when not started
-	VhostReadyInitMessage = "RabbitMQ vhost not started"
+	// RabbitMQVhostReadyInitMessage is the message for the RabbitMQVhostReady condition when not started
+	RabbitMQVhostReadyInitMessage = "RabbitMQ vhost not started"
 
-	// VhostReadyErrorMessage is the message format for the VhostReady condition when an error occurs
-	VhostReadyErrorMessage = "RabbitMQ vhost error occurred %s"
+	// RabbitMQVhostReadyErrorMessage is the message format for the RabbitMQVhostReady condition when an error occurs
+	RabbitMQVhostReadyErrorMessage = "RabbitMQ vhost error occurred %s"
 )