Add support for RabbitMQ Users and Vhosts and implement credentials rotation #511

lmiccini · 2025-12-05T09:40:50Z

This commit:

adds RabbitMQUser and RabbitMQVhost CRDs
adds a basic client using the rabbitmq management api
modifies TransportURL to be able to reference users and vhosts
adds functional tests for RabbitMQUser and RabbitMQVhost
adds a RabbitMqConfig shared struct that can be used by other operators to configure the respective user and vhost

In addition:

reworks how Mirrored queues policy is applied to use the management api instead of a direct pod rsh
fixes an issue with the rabbitmq defaulting webhook that we broke with the sdk rebase

Operators can now perform rabbitmq credentials rotation by:

editing the openstackcontrolplane to point each service at a new user
wait the pods reconciliation
trigger the edpm deployment to apply the change to the data plane
finally delete the previous RabbitMQUser when desired

Rollback is possible by switching the service to use the previous RabbitMQUser.

Credential rotation happens without downtime since both the old and new credentials are valid at the same time.

Assisted-by: Claude (Anthropic AI)

openshift-ci · 2025-12-05T09:40:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lmiccini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lmiccini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

apis/rabbitmq/v1beta1/rabbitmquser_types.go

apis/rabbitmq/v1beta1/rabbitmqpolicy_types.go

apis/rabbitmq/v1beta1/rabbitmquser_webhook.go

apis/rabbitmq/v1beta1/rabbitmquser_types.go

stuggi · 2025-12-05T15:22:44Z

internal/controller/rabbitmq/transporturl_controller.go

+				},
+				Spec: rabbitmqv1.RabbitMQVhostSpec{RabbitmqClusterName: instance.Spec.RabbitmqClusterName, Name: vhostName},
+			}
+			if err := controllerutil.SetControllerReference(instance, vhost, r.Scheme); err == nil {


what if err !=nil?

stuggi · 2025-12-05T15:23:06Z

internal/controller/rabbitmq/transporturl_controller.go

+				if !controllerutil.ContainsFinalizer(user, rabbitmqv1.UserFinalizer) {
+					controllerutil.AddFinalizer(user, rabbitmqv1.UserFinalizer)
+				}


same, could just call AddFinalizer, it returns true if it got added.

stuggi · 2025-12-05T15:23:27Z

internal/controller/rabbitmq/transporturl_controller.go

+			},
+			Spec: rabbitmqv1.RabbitMQUserSpec{RabbitmqClusterName: instance.Spec.RabbitmqClusterName, Username: instance.Spec.Username, VhostRef: vhostRef},
+		}
+		if err := controllerutil.SetControllerReference(instance, user, r.Scheme); err == nil {


same, what if err !=nil?

stuggi · 2025-12-05T15:23:56Z

internal/controller/rabbitmq/rabbitmqvhost_controller.go

+			if vhostName != "/" {
+				if err := apiClient.DeleteVhost(vhostName); err != nil {
+					// Log error but don't fail deletion - the vhost may already be gone
+					log.FromContext(ctx).Error(err, "Failed to delete vhost from RabbitMQ", "vhost", vhostName)


is it ok to just log?

internal/webhook/rabbitmq/v1beta1/doc.go

softwarefactory-project-zuul · 2025-12-06T13:50:55Z

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/infra-operator for 511,944d12baacad61d2dd7626038ab6eb7d8b06c15e

softwarefactory-project-zuul · 2025-12-06T16:06:53Z

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/infra-operator for 511,a371354f8f55d3179e796224af190b5f6bfa457d

softwarefactory-project-zuul · 2025-12-06T16:20:57Z

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/infra-operator for 511,b1562fc7b977ed317be5aa9413730a2c3fa10106

…otation This commit: - adds RabbitMQUser and RabbitMQVhost CRDs - adds a basic client using the rabbitmq management api - modifies TransportURL to be able to reference users and vhosts - adds functional tests for RabbitMQUser and RabbitMQVhost - adds a RabbitMqConfig shared struct that can be used by other operators to configure the respective user and vhost In addition: - reworks how Mirrored queues policy is applied to use the management api instead of a direct pod rsh Operators can now perform rabbitmq credentials rotation by: - editing the openstackcontrolplane to point each service at a new user - wait the pods reconciliation - trigger the edpm deployment to apply the change to the data plane - finally delete the previous RabbitMQUser when desired Rollback is possible by switching the service to use the previous RabbitMQUser. Credential rotation happens without downtime since both the old and new credentials are valid at the same time. Assisted-by: Claude (Anthropic AI)

stuggi · 2025-12-08T15:01:03Z

internal/controller/rabbitmq/rabbitmqpolicy_controller.go

+				// Log non-NotFound errors but continue with deletion
+				log.FromContext(ctx).Error(err, "Failed to get admin secret")
+			}
+			// Skip policy deletion if credentials are not accessible - finalizer will still be removed


do I get it right that we proceed with deletion and say its ok that the policy remains on the cluster?

my idea with this was not to block the cluster deletion in case we can't delete the poicy for whatever reason.

stuggi · 2025-12-08T15:11:08Z

internal/controller/rabbitmq/rabbitmquser_controller.go

+			// Note: Delete methods already treat 404 as success
 			if err := apiClient.DeletePermissions(vhostName, username); err != nil {
-				// Log error but don't fail deletion - the permissions may already be gone
+				// Log error but don't fail deletion - finalizer will still be removed to prevent CR from being stuck


couldn't there be a side effect that permissions remain on the cluster? in case of debugging you could not rely on the CR status maps to what is on the cluster configured?

same as previous comment, not sure if it's better to have the cluster stuck deleting if we can't delete the user?

openshift-ci bot requested review from dciabrin and viroel December 5, 2025 09:40

openshift-ci bot added the approved label Dec 5, 2025

lmiccini force-pushed the rabbitmq_vhosts branch from fefe46f to f8c165e Compare December 5, 2025 11:17

lmiccini requested review from stuggi and removed request for viroel December 5, 2025 11:30

stuggi reviewed Dec 5, 2025

View reviewed changes

lmiccini added the do-not-merge/hold label Dec 5, 2025

lmiccini force-pushed the rabbitmq_vhosts branch 3 times, most recently from 85b6224 to 944d12b Compare December 6, 2025 13:50

openshift-merge-robot added the needs-rebase label Dec 6, 2025

lmiccini force-pushed the rabbitmq_vhosts branch from 944d12b to a371354 Compare December 6, 2025 16:06

lmiccini force-pushed the rabbitmq_vhosts branch from a371354 to b1562fc Compare December 6, 2025 16:20

lmiccini force-pushed the rabbitmq_vhosts branch from b1562fc to 5093262 Compare December 6, 2025 16:29

openshift-merge-robot removed the needs-rebase label Dec 6, 2025

pr feedback 1

d2b549c

lmiccini force-pushed the rabbitmq_vhosts branch from 5093262 to d2b549c Compare December 6, 2025 16:43

stuggi reviewed Dec 8, 2025

View reviewed changes

feedback p2

508c2aa

lmiccini force-pushed the rabbitmq_vhosts branch from f18a119 to 508c2aa Compare December 9, 2025 08:30

Add support for RabbitMQ Users and Vhosts and implement credentials rotation #511

Are you sure you want to change the base?

Add support for RabbitMQ Users and Vhosts and implement credentials rotation #511

Conversation

lmiccini commented Dec 5, 2025

Uh oh!

openshift-ci bot commented Dec 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stuggi Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

stuggi Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

stuggi Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

stuggi Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

softwarefactory-project-zuul bot commented Dec 6, 2025

Uh oh!

softwarefactory-project-zuul bot commented Dec 6, 2025

Uh oh!

softwarefactory-project-zuul bot commented Dec 6, 2025

Uh oh!

stuggi Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

lmiccini Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

stuggi Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmiccini Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stuggi Dec 8, 2025 •

edited

Loading