From 72c8f35ee06d28e4c261b8895e225a6a42a121fe Mon Sep 17 00:00:00 2001 From: Aitor Perez <1515757+Zerpet@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:26:16 +0200 Subject: [PATCH 1/6] Conditionally omit RBAC resources for RabbitMQ >= 4.1.0 Add a helper function to verify if RBAC resources (ServiceAccount, Role, and RoleBinding) should be created based on the RabbitMQ version annotation. The RBAC resources are no longer required for RabbitMQ >= 4.1.0. - Skip adding RBAC resource builders for clusters >= 4.1.0 - Ensure any existing RBAC resources are deleted during reconciliation if the cluster is upgraded to a version that doesn't need them - Conditionally set ServiceAccountName and AutomountServiceAccountToken on the StatefulSet depending on the version - Include unit and integration tests to verify the new behavior Made-with: Cursor --- .../controller/rabbitmqcluster_controller.go | 14 +++++ .../rabbitmqcluster_controller_test.go | 38 ++++++++++++ .../resource/rabbitmq_resource_builder.go | 31 ++++++++-- .../rabbitmq_resource_builder_test.go | 60 +++++++++++++++++++ internal/resource/statefulset.go | 8 ++- internal/resource/statefulset_test.go | 22 +++++++ 6 files changed, 167 insertions(+), 6 deletions(-) diff --git a/internal/controller/rabbitmqcluster_controller.go b/internal/controller/rabbitmqcluster_controller.go index d776cb87e..4f811e711 100644 --- a/internal/controller/rabbitmqcluster_controller.go +++ b/internal/controller/rabbitmqcluster_controller.go @@ -183,6 +183,20 @@ func (r *RabbitmqClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ Scheme: r.Scheme, } + if !resource.ShouldCreateRBAC(rabbitmqCluster) { + // Ensure ServiceAccount, Role, and RoleBinding are deleted + for _, obj := range []client.Object{ + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: rabbitmqCluster.ChildResourceName("server"), Namespace: rabbitmqCluster.Namespace}}, + &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: rabbitmqCluster.ChildResourceName("peer-discovery"), Namespace: rabbitmqCluster.Namespace}}, + &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: rabbitmqCluster.ChildResourceName("server"), Namespace: rabbitmqCluster.Namespace}}, + } { + if err := r.Client.Delete(ctx, obj); client.IgnoreNotFound(err) != nil { + logger.Error(err, "Failed to delete RBAC resource") + return ctrl.Result{}, err + } + } + } + builders := resourceBuilder.ResourceBuilders() for _, builder := range builders { diff --git a/internal/controller/rabbitmqcluster_controller_test.go b/internal/controller/rabbitmqcluster_controller_test.go index a30037ace..0fbb3df41 100644 --- a/internal/controller/rabbitmqcluster_controller_test.go +++ b/internal/controller/rabbitmqcluster_controller_test.go @@ -617,6 +617,44 @@ var _ = Describe("RabbitmqClusterController", func() { }) }) + When("the RabbitMQ version is upgraded to 4.1.0 or greater", func() { + It("deletes the ServiceAccount, Role, and RoleBinding", func() { + // First, ensure the resources exist + _, err := clientSet.CoreV1().ServiceAccounts(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.RbacV1().Roles(cluster.Namespace).Get(ctx, cluster.ChildResourceName("peer-discovery"), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + _, err = clientSet.RbacV1().RoleBindings(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + // Upgrade to 4.1.5 + Expect(updateWithRetry(cluster, func(r *rabbitmqv1beta1.RabbitmqCluster) { + if r.Annotations == nil { + r.Annotations = make(map[string]string) + } + r.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.1.5" + })).To(Succeed()) + + // Verify the resources are deleted + Eventually(func() bool { + _, err := clientSet.CoreV1().ServiceAccounts(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) + return apierrors.IsNotFound(err) + }, 5).Should(BeTrue()) + + Eventually(func() bool { + _, err := clientSet.RbacV1().Roles(cluster.Namespace).Get(ctx, cluster.ChildResourceName("peer-discovery"), metav1.GetOptions{}) + return apierrors.IsNotFound(err) + }, 5).Should(BeTrue()) + + Eventually(func() bool { + _, err := clientSet.RbacV1().RoleBindings(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) + return apierrors.IsNotFound(err) + }, 5).Should(BeTrue()) + }) + }) + It("service type is updated", func() { Expect(updateWithRetry(cluster, func(r *rabbitmqv1beta1.RabbitmqCluster) { r.Spec.Service.Type = "NodePort" diff --git a/internal/resource/rabbitmq_resource_builder.go b/internal/resource/rabbitmq_resource_builder.go index d892bf39f..0ab8ef4f2 100644 --- a/internal/resource/rabbitmq_resource_builder.go +++ b/internal/resource/rabbitmq_resource_builder.go @@ -10,6 +10,7 @@ package resource import ( + "github.com/Masterminds/semver/v3" rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,6 +28,21 @@ type ResourceBuilder interface { UpdateMayRequireStsRecreate() bool } +func ShouldCreateRBAC(rmq *rabbitmqv1beta1.RabbitmqCluster) bool { + version := rmq.GetRabbitMQVersion() + if version == rabbitmqv1beta1.VersionNotAnnotated { + return true + } + + v, err := semver.NewVersion(version) + if err != nil { + return true + } + + constraint, _ := semver.NewConstraint(">= 4.1.0") + return !constraint.Check(v) +} + func (builder *RabbitmqResourceBuilder) ResourceBuilders() []ResourceBuilder { builders := []ResourceBuilder{ @@ -36,11 +52,18 @@ func (builder *RabbitmqResourceBuilder) ResourceBuilders() []ResourceBuilder { builder.DefaultUserSecret(), builder.RabbitmqPluginsConfigMap(), builder.ServerConfigMap(), - builder.ServiceAccount(), - builder.Role(), - builder.RoleBinding(), - builder.StatefulSet(), } + + if ShouldCreateRBAC(builder.Instance) { + builders = append(builders, + builder.ServiceAccount(), + builder.Role(), + builder.RoleBinding(), + ) + } + + builders = append(builders, builder.StatefulSet()) + if builder.Instance.VaultDefaultUserSecretEnabled() || builder.Instance.ExternalSecretEnabled() { // do not create default-user K8s Secret builders = slices.Delete(builders, 3, 3+1) diff --git a/internal/resource/rabbitmq_resource_builder_test.go b/internal/resource/rabbitmq_resource_builder_test.go index 49d5da354..bd24a3550 100644 --- a/internal/resource/rabbitmq_resource_builder_test.go +++ b/internal/resource/rabbitmq_resource_builder_test.go @@ -20,6 +20,51 @@ import ( ) var _ = Describe("RabbitmqResourceBuilder", func() { + Context("ShouldCreateRBAC", func() { + It("returns true if version is not annotated", func() { + rmq := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{}}, + } + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + }) + + It("returns true if version cannot be parsed", func() { + rmq := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + rabbitmqv1beta1.RabbitmqVersionAnnotation: "invalid", + }}, + } + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + }) + + It("returns true if version is less than 4.1.0", func() { + rmq := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + rabbitmqv1beta1.RabbitmqVersionAnnotation: "3.13.0", + }}, + } + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + + rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.0.0" + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + }) + + It("returns false if version is 4.1.0 or greater", func() { + rmq := &rabbitmqv1beta1.RabbitmqCluster{ + ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{ + rabbitmqv1beta1.RabbitmqVersionAnnotation: "4.1.0", + }}, + } + Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalse()) + + rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.1.5" + Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalse()) + + rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.2.0" + Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalse()) + }) + }) + Context("ResourceBuilders", func() { var ( instance *rabbitmqv1beta1.RabbitmqCluster @@ -79,5 +124,20 @@ var _ = Describe("RabbitmqResourceBuilder", func() { Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.DefaultUserSecretBuilder{}))) }) }) + + When("RabbitMQ version is 4.1.0 or greater", func() { + BeforeEach(func() { + instance.Annotations = map[string]string{ + rabbitmqv1beta1.RabbitmqVersionAnnotation: "4.1.0", + } + }) + It("returns all resource builders except for RBAC resources", func() { + resourceBuilders := builder.ResourceBuilders() + Expect(resourceBuilders).To(HaveLen(7)) + Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.ServiceAccountBuilder{}))) + Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.RoleBuilder{}))) + Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.RoleBindingBuilder{}))) + }) + }) }) }) diff --git a/internal/resource/statefulset.go b/internal/resource/statefulset.go index def1426e2..aecd63f4a 100644 --- a/internal/resource/statefulset.go +++ b/internal/resource/statefulset.go @@ -593,8 +593,6 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st }, ImagePullSecrets: builder.Instance.Spec.ImagePullSecrets, TerminationGracePeriodSeconds: builder.Instance.Spec.TerminationGracePeriodSeconds, - ServiceAccountName: builder.Instance.ChildResourceName(serviceAccountName), - AutomountServiceAccountToken: ptr.To(true), Affinity: builder.Instance.Spec.Affinity, Tolerations: builder.Instance.Spec.Tolerations, InitContainers: []corev1.Container{setupContainer(builder.Instance)}, @@ -693,6 +691,12 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st podTemplateSpec.Spec.Containers = append(podTemplateSpec.Spec.Containers, defaultUserCredentialUpdater(builder.Instance)) } + + if ShouldCreateRBAC(builder.Instance) { + podTemplateSpec.Spec.ServiceAccountName = builder.Instance.ChildResourceName(serviceAccountName) + podTemplateSpec.Spec.AutomountServiceAccountToken = ptr.To(true) + } + return podTemplateSpec } diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index e6c84c317..7610daba5 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -1415,6 +1415,28 @@ default_pass = {{ .Data.data.password }} Expect(*statefulSet.Spec.Template.Spec.AutomountServiceAccountToken).To(BeTrue()) }) + When("RabbitMQ version is 4.1.0 or greater", func() { + BeforeEach(func() { + instance.Annotations = map[string]string{ + rabbitmqv1beta1.RabbitmqVersionAnnotation: "4.1.0", + } + }) + + It("omits the service account", func() { + stsBuilder := builder.StatefulSet() + Expect(stsBuilder.Update(statefulSet)).To(Succeed()) + + Expect(statefulSet.Spec.Template.Spec.ServiceAccountName).To(BeEmpty()) + }) + + It("does not explicitly mount the service account token", func() { + stsBuilder := builder.StatefulSet() + Expect(stsBuilder.Update(statefulSet)).To(Succeed()) + + Expect(statefulSet.Spec.Template.Spec.AutomountServiceAccountToken).To(BeNil()) + }) + }) + It("creates the required SecurityContext", func() { stsBuilder := builder.StatefulSet() Expect(stsBuilder.Update(statefulSet)).To(Succeed()) From 5f70c0faedbf525cd6247fb4190decba81aa9a34 Mon Sep 17 00:00:00 2001 From: Aitor Perez <1515757+Zerpet@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:25:59 +0200 Subject: [PATCH 2/6] Refactor BeTrue/BeFalse assertions to use BeTrueBecause/BeFalseBecause Update newly added tests to use BeTrueBecause and BeFalseBecause for more descriptive error messages when assertions fail. Made-with: Cursor --- .../controller/rabbitmqcluster_controller_test.go | 6 +++--- .../resource/rabbitmq_resource_builder_test.go | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/controller/rabbitmqcluster_controller_test.go b/internal/controller/rabbitmqcluster_controller_test.go index 0fbb3df41..7c440cb77 100644 --- a/internal/controller/rabbitmqcluster_controller_test.go +++ b/internal/controller/rabbitmqcluster_controller_test.go @@ -641,17 +641,17 @@ var _ = Describe("RabbitmqClusterController", func() { Eventually(func() bool { _, err := clientSet.CoreV1().ServiceAccounts(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) return apierrors.IsNotFound(err) - }, 5).Should(BeTrue()) + }, 5).Should(BeTrueBecause("ServiceAccount should be deleted when version >= 4.1.0")) Eventually(func() bool { _, err := clientSet.RbacV1().Roles(cluster.Namespace).Get(ctx, cluster.ChildResourceName("peer-discovery"), metav1.GetOptions{}) return apierrors.IsNotFound(err) - }, 5).Should(BeTrue()) + }, 5).Should(BeTrueBecause("Role should be deleted when version >= 4.1.0")) Eventually(func() bool { _, err := clientSet.RbacV1().RoleBindings(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) return apierrors.IsNotFound(err) - }, 5).Should(BeTrue()) + }, 5).Should(BeTrueBecause("RoleBinding should be deleted when version >= 4.1.0")) }) }) diff --git a/internal/resource/rabbitmq_resource_builder_test.go b/internal/resource/rabbitmq_resource_builder_test.go index bd24a3550..eab728dad 100644 --- a/internal/resource/rabbitmq_resource_builder_test.go +++ b/internal/resource/rabbitmq_resource_builder_test.go @@ -25,7 +25,7 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rmq := &rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{}}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("fallback to old behavior when version is not annotated")) }) It("returns true if version cannot be parsed", func() { @@ -34,7 +34,7 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "invalid", }}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("fallback to old behavior when version cannot be parsed")) }) It("returns true if version is less than 4.1.0", func() { @@ -43,10 +43,10 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "3.13.0", }}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("version is less than 4.1.0")) rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.0.0" - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrue()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("version is less than 4.1.0")) }) It("returns false if version is 4.1.0 or greater", func() { @@ -55,13 +55,13 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "4.1.0", }}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalse()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalseBecause("RBAC resources are no longer required for 4.1.0 or greater")) rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.1.5" - Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalse()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalseBecause("RBAC resources are no longer required for 4.1.0 or greater")) rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.2.0" - Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalse()) + Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalseBecause("RBAC resources are no longer required for 4.1.0 or greater")) }) }) From 9cea2eb9f4788b65442ed456d2656371d0a24bda Mon Sep 17 00:00:00 2001 From: Aitor Perez <1515757+Zerpet@users.noreply.github.com> Date: Thu, 30 Apr 2026 11:47:14 +0200 Subject: [PATCH 3/6] Delete peer-discovery Role/Binding for RMQ >= 4.1 They are no longer required by the peer discovery plugin in 4.1+ Keeping the Service Account because it is still required for Vault integration. --- config/rbac/role.yaml | 13 ++++++++- .../controller/rabbitmqcluster_controller.go | 13 +++++---- .../rabbitmqcluster_controller_test.go | 15 +++++----- .../resource/rabbitmq_resource_builder.go | 28 +++++++++++++++---- .../rabbitmq_resource_builder_test.go | 22 +++++++-------- internal/resource/statefulset.go | 6 ++-- internal/resource/statefulset_test.go | 8 +++--- 7 files changed, 67 insertions(+), 38 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 9f9c99b0e..882981a6c 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -18,7 +18,6 @@ rules: - configmaps - persistentvolumeclaims - secrets - - serviceaccounts - services verbs: - create @@ -57,6 +56,17 @@ rules: - pods/exec verbs: - create +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - delete + - get + - list + - update + - watch - apiGroups: - apps resources: @@ -106,6 +116,7 @@ rules: - roles verbs: - create + - delete - get - list - update diff --git a/internal/controller/rabbitmqcluster_controller.go b/internal/controller/rabbitmqcluster_controller.go index 4f811e711..d2bfcdac6 100644 --- a/internal/controller/rabbitmqcluster_controller.go +++ b/internal/controller/rabbitmqcluster_controller.go @@ -90,10 +90,10 @@ type RabbitmqClusterReconciler struct { // +kubebuilder:rbac:groups="",resources=events,verbs=get;create;patch // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;create;update -// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;update;delete // +kubebuilder:rbac:groups="discovery.k8s.io",resources=endpointslices,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=endpoints,verbs=get;watch;list -// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;update;delete func (r *RabbitmqClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := ctrl.LoggerFrom(ctx) @@ -183,15 +183,16 @@ func (r *RabbitmqClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ Scheme: r.Scheme, } - if !resource.ShouldCreateRBAC(rabbitmqCluster) { - // Ensure ServiceAccount, Role, and RoleBinding are deleted + if !resource.ShouldCreatePeerDiscoveryRBAC(rabbitmqCluster) { + // Ensure peer-discovery Role and RoleBinding are deleted. + // The ServiceAccount is intentionally kept because other integrations + // (e.g. Vault Kubernetes auth) may rely on it. for _, obj := range []client.Object{ - &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: rabbitmqCluster.ChildResourceName("server"), Namespace: rabbitmqCluster.Namespace}}, &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: rabbitmqCluster.ChildResourceName("peer-discovery"), Namespace: rabbitmqCluster.Namespace}}, &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: rabbitmqCluster.ChildResourceName("server"), Namespace: rabbitmqCluster.Namespace}}, } { if err := r.Client.Delete(ctx, obj); client.IgnoreNotFound(err) != nil { - logger.Error(err, "Failed to delete RBAC resource") + logger.Error(err, "Failed to delete peer-discovery RBAC resource") return ctrl.Result{}, err } } diff --git a/internal/controller/rabbitmqcluster_controller_test.go b/internal/controller/rabbitmqcluster_controller_test.go index 7c440cb77..69e6a9773 100644 --- a/internal/controller/rabbitmqcluster_controller_test.go +++ b/internal/controller/rabbitmqcluster_controller_test.go @@ -618,7 +618,7 @@ var _ = Describe("RabbitmqClusterController", func() { }) When("the RabbitMQ version is upgraded to 4.1.0 or greater", func() { - It("deletes the ServiceAccount, Role, and RoleBinding", func() { + It("deletes the peer-discovery Role and RoleBinding but keeps the ServiceAccount", func() { // First, ensure the resources exist _, err := clientSet.CoreV1().ServiceAccounts(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -637,12 +637,7 @@ var _ = Describe("RabbitmqClusterController", func() { r.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.1.5" })).To(Succeed()) - // Verify the resources are deleted - Eventually(func() bool { - _, err := clientSet.CoreV1().ServiceAccounts(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) - return apierrors.IsNotFound(err) - }, 5).Should(BeTrueBecause("ServiceAccount should be deleted when version >= 4.1.0")) - + // Verify Role and RoleBinding are deleted Eventually(func() bool { _, err := clientSet.RbacV1().Roles(cluster.Namespace).Get(ctx, cluster.ChildResourceName("peer-discovery"), metav1.GetOptions{}) return apierrors.IsNotFound(err) @@ -652,6 +647,12 @@ var _ = Describe("RabbitmqClusterController", func() { _, err := clientSet.RbacV1().RoleBindings(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) return apierrors.IsNotFound(err) }, 5).Should(BeTrueBecause("RoleBinding should be deleted when version >= 4.1.0")) + + // Verify the ServiceAccount is kept (other integrations such as Vault Kubernetes auth may rely on it) + Consistently(func() bool { + _, err := clientSet.CoreV1().ServiceAccounts(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{}) + return err == nil + }, 3).Should(BeTrueBecause("ServiceAccount should be kept when version >= 4.1.0")) }) }) diff --git a/internal/resource/rabbitmq_resource_builder.go b/internal/resource/rabbitmq_resource_builder.go index 0ab8ef4f2..a18ce61f7 100644 --- a/internal/resource/rabbitmq_resource_builder.go +++ b/internal/resource/rabbitmq_resource_builder.go @@ -28,7 +28,26 @@ type ResourceBuilder interface { UpdateMayRequireStsRecreate() bool } -func ShouldCreateRBAC(rmq *rabbitmqv1beta1.RabbitmqCluster) bool { +// peerDiscoveryRBACConstraint is the semver constraint used to determine whether +// peer-discovery RBAC (Role and RoleBinding) should be created. It is created +// once at package scope to avoid repeated allocations. +var peerDiscoveryRBACConstraint = mustNewConstraint(">= 4.1.0") + +// mustNewConstraint creates a semver constraint and panics if parsing fails. +// This is only used for constant constraint strings that are always valid. +func mustNewConstraint(c string) *semver.Constraints { + constraint, err := semver.NewConstraint(c) + if err != nil { + panic(err) + } + return constraint +} + +// ShouldCreatePeerDiscoveryRBAC returns true if the peer-discovery Role and +// RoleBinding should be created for this RabbitmqCluster. The ServiceAccount is +// always created regardless of RabbitMQ version because other integrations (e.g. +// Vault Kubernetes auth) may rely on it. +func ShouldCreatePeerDiscoveryRBAC(rmq *rabbitmqv1beta1.RabbitmqCluster) bool { version := rmq.GetRabbitMQVersion() if version == rabbitmqv1beta1.VersionNotAnnotated { return true @@ -39,8 +58,7 @@ func ShouldCreateRBAC(rmq *rabbitmqv1beta1.RabbitmqCluster) bool { return true } - constraint, _ := semver.NewConstraint(">= 4.1.0") - return !constraint.Check(v) + return !peerDiscoveryRBACConstraint.Check(v) } func (builder *RabbitmqResourceBuilder) ResourceBuilders() []ResourceBuilder { @@ -52,11 +70,11 @@ func (builder *RabbitmqResourceBuilder) ResourceBuilders() []ResourceBuilder { builder.DefaultUserSecret(), builder.RabbitmqPluginsConfigMap(), builder.ServerConfigMap(), + builder.ServiceAccount(), } - if ShouldCreateRBAC(builder.Instance) { + if ShouldCreatePeerDiscoveryRBAC(builder.Instance) { builders = append(builders, - builder.ServiceAccount(), builder.Role(), builder.RoleBinding(), ) diff --git a/internal/resource/rabbitmq_resource_builder_test.go b/internal/resource/rabbitmq_resource_builder_test.go index eab728dad..6d6f5dd93 100644 --- a/internal/resource/rabbitmq_resource_builder_test.go +++ b/internal/resource/rabbitmq_resource_builder_test.go @@ -20,12 +20,12 @@ import ( ) var _ = Describe("RabbitmqResourceBuilder", func() { - Context("ShouldCreateRBAC", func() { + Context("ShouldCreatePeerDiscoveryRBAC", func() { It("returns true if version is not annotated", func() { rmq := &rabbitmqv1beta1.RabbitmqCluster{ ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{}}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("fallback to old behavior when version is not annotated")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeTrueBecause("fallback to old behavior when version is not annotated")) }) It("returns true if version cannot be parsed", func() { @@ -34,7 +34,7 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "invalid", }}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("fallback to old behavior when version cannot be parsed")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeTrueBecause("fallback to old behavior when version cannot be parsed")) }) It("returns true if version is less than 4.1.0", func() { @@ -43,10 +43,10 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "3.13.0", }}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("version is less than 4.1.0")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeTrueBecause("version is less than 4.1.0")) rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.0.0" - Expect(resource.ShouldCreateRBAC(rmq)).To(BeTrueBecause("version is less than 4.1.0")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeTrueBecause("version is less than 4.1.0")) }) It("returns false if version is 4.1.0 or greater", func() { @@ -55,13 +55,13 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "4.1.0", }}, } - Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalseBecause("RBAC resources are no longer required for 4.1.0 or greater")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeFalseBecause("peer-discovery RBAC is no longer required for 4.1.0 or greater")) rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.1.5" - Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalseBecause("RBAC resources are no longer required for 4.1.0 or greater")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeFalseBecause("peer-discovery RBAC is no longer required for 4.1.0 or greater")) rmq.Annotations[rabbitmqv1beta1.RabbitmqVersionAnnotation] = "4.2.0" - Expect(resource.ShouldCreateRBAC(rmq)).To(BeFalseBecause("RBAC resources are no longer required for 4.1.0 or greater")) + Expect(resource.ShouldCreatePeerDiscoveryRBAC(rmq)).To(BeFalseBecause("peer-discovery RBAC is no longer required for 4.1.0 or greater")) }) }) @@ -131,10 +131,10 @@ var _ = Describe("RabbitmqResourceBuilder", func() { rabbitmqv1beta1.RabbitmqVersionAnnotation: "4.1.0", } }) - It("returns all resource builders except for RBAC resources", func() { + It("returns all resource builders except for peer-discovery Role and RoleBinding", func() { resourceBuilders := builder.ResourceBuilders() - Expect(resourceBuilders).To(HaveLen(7)) - Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.ServiceAccountBuilder{}))) + Expect(resourceBuilders).To(HaveLen(8)) + Expect(resourceBuilders).To(ContainElement(BeAssignableToTypeOf(&resource.ServiceAccountBuilder{}))) Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.RoleBuilder{}))) Expect(resourceBuilders).NotTo(ContainElement(BeAssignableToTypeOf(&resource.RoleBindingBuilder{}))) }) diff --git a/internal/resource/statefulset.go b/internal/resource/statefulset.go index aecd63f4a..fe88c80be 100644 --- a/internal/resource/statefulset.go +++ b/internal/resource/statefulset.go @@ -692,10 +692,8 @@ func (builder *StatefulSetBuilder) podTemplateSpec(previousPodAnnotations map[st defaultUserCredentialUpdater(builder.Instance)) } - if ShouldCreateRBAC(builder.Instance) { - podTemplateSpec.Spec.ServiceAccountName = builder.Instance.ChildResourceName(serviceAccountName) - podTemplateSpec.Spec.AutomountServiceAccountToken = ptr.To(true) - } + podTemplateSpec.Spec.ServiceAccountName = builder.Instance.ChildResourceName(serviceAccountName) + podTemplateSpec.Spec.AutomountServiceAccountToken = ptr.To(true) return podTemplateSpec } diff --git a/internal/resource/statefulset_test.go b/internal/resource/statefulset_test.go index 7610daba5..fc2bb3597 100644 --- a/internal/resource/statefulset_test.go +++ b/internal/resource/statefulset_test.go @@ -1422,18 +1422,18 @@ default_pass = {{ .Data.data.password }} } }) - It("omits the service account", func() { + It("still uses the correct service account", func() { stsBuilder := builder.StatefulSet() Expect(stsBuilder.Update(statefulSet)).To(Succeed()) - Expect(statefulSet.Spec.Template.Spec.ServiceAccountName).To(BeEmpty()) + Expect(statefulSet.Spec.Template.Spec.ServiceAccountName).To(Equal(instance.ChildResourceName("server"))) }) - It("does not explicitly mount the service account token", func() { + It("still mounts the service account token in its pods", func() { stsBuilder := builder.StatefulSet() Expect(stsBuilder.Update(statefulSet)).To(Succeed()) - Expect(statefulSet.Spec.Template.Spec.AutomountServiceAccountToken).To(BeNil()) + Expect(*statefulSet.Spec.Template.Spec.AutomountServiceAccountToken).To(BeTrue()) }) }) From f1ec4a062a181bdb5f269ca680ee4cba61b07d4d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 12:00:25 +0000 Subject: [PATCH 4/6] Conditional peer-discovery RBAC creation based on RabbitMQ version Agent-Logs-Url: https://github.com/rabbitmq/cluster-operator/sessions/99f20b88-4e9d-460d-8e59-c385360b7b8e Co-authored-by: Zerpet <1515757+Zerpet@users.noreply.github.com> --- go.mod | 2 +- go.sum | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 46e35a45c..1d624343d 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/rabbitmq/cluster-operator/v2 go 1.26.2 require ( + github.com/Masterminds/semver/v3 v3.4.0 github.com/cloudflare/cfssl v1.6.5 github.com/eclipse/paho.mqtt.golang v1.5.1 github.com/go-logr/logr v1.4.3 @@ -25,7 +26,6 @@ require ( require ( cel.dev/expr v0.25.1 // indirect - github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.1 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect diff --git a/go.sum b/go.sum index 1001945fd..c1202ff73 100644 --- a/go.sum +++ b/go.sum @@ -50,7 +50,6 @@ github.com/fxamacker/cbor/v2 v2.9.1/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj2 github.com/gkampitakis/ciinfo v0.3.2 h1:JcuOPk8ZU7nZQjdUhctuhQofk7BGHuIy0c9Ez8BNhXs= github.com/gkampitakis/ciinfo v0.3.2/go.mod h1:1NIwaOcFChN4fa/B0hEBdAb6npDlFL8Bwx4dfRLRqAo= github.com/gkampitakis/go-diff v1.3.2 h1:Qyn0J9XJSDTgnsgHRdz9Zp24RaJeKMUHg2+PDZZdC4M= -github.com/gkampitakis/go-diff v1.3.2/go.mod h1:LLgOrpqleQe26cte8s36HTWcTmMEur6OPYerdAAS9tk= github.com/gkampitakis/go-snaps v0.5.15 h1:amyJrvM1D33cPHwVrjo9jQxX8g/7E2wYdZ+01KS3zGE= github.com/gkampitakis/go-snaps v0.5.15/go.mod h1:HNpx/9GoKisdhw9AFOBT1N7DBs9DiHo/hGheFGBZ+mc= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= From a39416e9ea86c36ec2260c01254da57830bb67ed Mon Sep 17 00:00:00 2001 From: Aitor Perez <1515757+Zerpet@users.noreply.github.com> Date: Thu, 30 Apr 2026 14:30:45 +0200 Subject: [PATCH 5/6] Bump system test interval check Because sometimes system tests flake attempting to verify that rabbit has started. From the logs looks like `kubectl` command does not return. By bumping the timeout, we should avoid hammering the CI runner with a pile of pending `kubectl` commands. --- test/system/utils_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/system/utils_test.go b/test/system/utils_test.go index 3235b3358..7d009a5ec 100644 --- a/test/system/utils_test.go +++ b/test/system/utils_test.go @@ -654,7 +654,7 @@ func waitForRabbitmqNotRunningWithOffset(cluster *rabbitmqv1beta1.RabbitmqCluste } return string(output) - }, podCreationTimeout, 1).Should(Equal("'False'")) + }, podCreationTimeout, 2).Should(Equal("'False'")) ExpectWithOffset(callStackOffset, err).NotTo(HaveOccurred()) } @@ -679,7 +679,7 @@ func waitForRabbitmqRunningWithOffset(cluster *rabbitmqv1beta1.RabbitmqCluster, } return string(output) - }, podCreationTimeout, 1).Should(Equal("'True'")) + }, podCreationTimeout, 2).Should(Equal("'True'")) ExpectWithOffset(callStackOffset, err).NotTo(HaveOccurred()) } From bf74c71700da03d1732f29fc60bf443afc3cc99b Mon Sep 17 00:00:00 2001 From: Aitor Perez <1515757+Zerpet@users.noreply.github.com> Date: Tue, 5 May 2026 16:53:29 +0100 Subject: [PATCH 6/6] Re-generate RBAC role --- config/rbac/role.yaml | 12 +----------- internal/resource/rabbitmq_resource_builder.go | 6 +++++- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 882981a6c..7bf012b18 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -18,6 +18,7 @@ rules: - configmaps - persistentvolumeclaims - secrets + - serviceaccounts - services verbs: - create @@ -56,17 +57,6 @@ rules: - pods/exec verbs: - create -- apiGroups: - - "" - resources: - - serviceaccounts - verbs: - - create - - delete - - get - - list - - update - - watch - apiGroups: - apps resources: diff --git a/internal/resource/rabbitmq_resource_builder.go b/internal/resource/rabbitmq_resource_builder.go index a18ce61f7..9504a869d 100644 --- a/internal/resource/rabbitmq_resource_builder.go +++ b/internal/resource/rabbitmq_resource_builder.go @@ -10,11 +10,12 @@ package resource import ( + "slices" + "github.com/Masterminds/semver/v3" rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/v2/api/v1beta1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "slices" ) type RabbitmqResourceBuilder struct { @@ -80,6 +81,9 @@ func (builder *RabbitmqResourceBuilder) ResourceBuilders() []ResourceBuilder { ) } + // Appending StatefulSet builder separately because the order of the builders is important + // The SA, ConfigMap, and Secret need to be created before the StatefulSet. Otherwise, Pods + // created by the StatefulSet will block on the creation of dependent resources. builders = append(builders, builder.StatefulSet()) if builder.Instance.VaultDefaultUserSecretEnabled() || builder.Instance.ExternalSecretEnabled() {