From 3a6809539412d5337269a32aa5aa9576231f0b1d Mon Sep 17 00:00:00 2001 From: Luca Miccini Date: Tue, 16 Dec 2025 15:49:07 +0100 Subject: [PATCH] Rabbitmq vhost and user support Add new notificationsBus interface to hold cluster, user and vhost names for optional usage. The controller adds these values to the TransportURL create request when present. Additionally, we migrate RabbitMQ cluster name to RabbitMq config struct using DefaultRabbitMqConfig from infra-operator to automatically populate the new Cluster field from legacy RabbitMqClusterName. Example usage: spec: notificationsBus: cluster: rabbitmq user: custom-user vhost: custom-vhost Jira: https://issues.redhat.com/browse/OSPRH-23739 --- api/bases/glance.openstack.org_glances.yaml | 17 ++++ api/go.mod | 2 +- api/go.sum | 3 + api/v1beta1/glance_types.go | 7 +- api/v1beta1/glance_webhook.go | 36 +++++++- api/v1beta1/zz_generated.deepcopy.go | 6 ++ .../bases/glance.openstack.org_glances.yaml | 17 ++++ internal/controller/glance_controller.go | 16 +++- test/functional/glance_controller_test.go | 85 +++++++++++++++++++ test/functional/glance_webhook_test.go | 81 ++++++++++++++++++ 10 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 test/functional/glance_webhook_test.go diff --git a/api/bases/glance.openstack.org_glances.yaml b/api/bases/glance.openstack.org_glances.yaml index 1f08f86d9..6d399aca7 100644 --- a/api/bases/glance.openstack.org_glances.yaml +++ b/api/bases/glance.openstack.org_glances.yaml @@ -1629,6 +1629,23 @@ spec: Needed to request a transportURL that is created and used for notification purposes type: string + notificationsBus: + description: NotificationsBus configuration (username, vhost, and + cluster) for notifications + properties: + cluster: + description: Name of the cluster + minLength: 1 + type: string + user: + description: User - RabbitMQ username + type: string + vhost: + description: Vhost - RabbitMQ vhost name + type: string + required: + - cluster + type: object passwordSelectors: default: service: GlancePassword diff --git a/api/go.mod b/api/go.mod index affb9fe3a..f27ce5a81 100644 --- a/api/go.mod +++ b/api/go.mod @@ -17,7 +17,6 @@ require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.12.2 // indirect - github.com/evanphx/json-patch v5.9.11+incompatible // indirect github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.9.0 // indirect @@ -46,6 +45,7 @@ require ( github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.65.0 // indirect github.com/prometheus/procfs v0.16.1 // indirect + github.com/rabbitmq/cluster-operator/v2 v2.16.0 // indirect github.com/spf13/pflag v1.0.7 // indirect github.com/stretchr/testify v1.11.1 // indirect github.com/x448/float16 v0.8.4 // indirect diff --git a/api/go.sum b/api/go.sum index 838cc051d..af24e207b 100644 --- a/api/go.sum +++ b/api/go.sum @@ -1,3 +1,4 @@ +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= @@ -86,6 +87,8 @@ github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.2025123021 github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251230215914-6ba873b49a35/go.mod h1:kycZyoe7OZdW1HUghr2nI3N7wSJtNahXf6b/ypD14f4= github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20251230215914-6ba873b49a35 h1:8WZYfCt1VJHa5sJRX0UhpmoXud/fn8LHQhXsakdYXuQ= github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20251230215914-6ba873b49a35/go.mod h1:H0aQANk8iJPRhS2Bg9n6cYb/IHF0Cks9g7+uZG04Rhk= +github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20250929174222-a0d328fa4dec h1:saovr368HPAKHN0aRPh8h8n9s9dn3d8Frmfua0UYRlc= +github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20250929174222-a0d328fa4dec/go.mod h1:Nh2NEePLjovUQof2krTAg4JaAoLacqtPTZQXK6izNfg= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= diff --git a/api/v1beta1/glance_types.go b/api/v1beta1/glance_types.go index 8b479e78e..9f051b04e 100644 --- a/api/v1beta1/glance_types.go +++ b/api/v1beta1/glance_types.go @@ -17,9 +17,10 @@ limitations under the License. package v1beta1 import ( + rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" + topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/storage" - topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -139,6 +140,10 @@ type GlanceSpecCore struct { // Needed to request a transportURL that is created and used for notification // purposes NotificationBusInstance *string `json:"notificationBusInstance,omitempty"` + + // +kubebuilder:validation:Optional + // NotificationsBus configuration (username, vhost, and cluster) for notifications + NotificationsBus *rabbitmqv1.RabbitMqConfig `json:"notificationsBus,omitempty"` } // GlanceSpec defines the desired state of Glance diff --git a/api/v1beta1/glance_webhook.go b/api/v1beta1/glance_webhook.go index c8488e9a6..655447072 100644 --- a/api/v1beta1/glance_webhook.go +++ b/api/v1beta1/glance_webhook.go @@ -21,7 +21,10 @@ import ( "strings" "github.com/google/go-cmp/cmp" + rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" + topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/common/service" + common_webhook "github.com/openstack-k8s-operators/lib-common/modules/common/webhook" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -29,9 +32,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" - - common_webhook "github.com/openstack-k8s-operators/lib-common/modules/common/webhook" ) // GlanceDefaults - @@ -111,6 +111,14 @@ func (r *GlanceSpecCore) Default() { if r.DBPurge.Schedule == "" { r.DBPurge.Schedule = glanceDefaults.DBPurgeSchedule } + + // Default NotificationsBus if NotificationBusInstance is specified + if r.NotificationBusInstance != nil && *r.NotificationBusInstance != "" { + if r.NotificationsBus == nil { + r.NotificationsBus = &rabbitmqv1.RabbitMqConfig{} + } + rabbitmqv1.DefaultRabbitMqConfig(r.NotificationsBus, *r.NotificationBusInstance) + } // When no glanceAPI(s) are specified in the top-level CR // we build one by default, but we set replicas=0 and we // build a "CustomServiceConfig" template that should be @@ -352,6 +360,28 @@ func (r *GlanceSpec) ValidateUpdate(old GlanceSpec, basePath *field.Path, namesp func (r *GlanceSpecCore) ValidateUpdate(old GlanceSpecCore, basePath *field.Path, namespace string) field.ErrorList { var allErrs field.ErrorList + // Validate deprecated fields and their new equivalents + // Don't allow setting both old and new fields with different values + if r.NotificationBusInstance != nil && *r.NotificationBusInstance != "" && + r.NotificationsBus != nil && r.NotificationsBus.Cluster != "" && + *r.NotificationBusInstance != r.NotificationsBus.Cluster { + allErrs = append(allErrs, field.Invalid( + basePath.Child("notificationsBus").Child("cluster"), + r.NotificationsBus.Cluster, + fmt.Sprintf("notificationsBus.cluster cannot differ from deprecated notificationBusInstance (%s). "+ + "Either use the new notificationsBus.cluster field or the deprecated notificationBusInstance, but not both with different values", + *r.NotificationBusInstance))) + } + + // Reject changes to deprecated NotificationBusInstance field unless nulling it out + if r.NotificationBusInstance != nil && old.NotificationBusInstance != nil && + *r.NotificationBusInstance != *old.NotificationBusInstance && + *r.NotificationBusInstance != "" { + allErrs = append(allErrs, field.Forbidden( + basePath.Child("notificationBusInstance"), + "notificationBusInstance is deprecated and cannot be changed. Please use notificationsBus.cluster instead")) + } + // fail if a wrong topology is referenced allErrs = append(allErrs, topologyv1.ValidateTopologyRef( r.TopologyRef, *basePath.Child("topologyRef"), namespace)...) diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index f7a0c84c5..390748635 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1beta1 import ( + rabbitmqv1beta1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" topologyv1beta1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/service" @@ -431,6 +432,11 @@ func (in *GlanceSpecCore) DeepCopyInto(out *GlanceSpecCore) { *out = new(string) **out = **in } + if in.NotificationsBus != nil { + in, out := &in.NotificationsBus, &out.NotificationsBus + *out = new(rabbitmqv1beta1.RabbitMqConfig) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GlanceSpecCore. diff --git a/config/crd/bases/glance.openstack.org_glances.yaml b/config/crd/bases/glance.openstack.org_glances.yaml index 1f08f86d9..6d399aca7 100644 --- a/config/crd/bases/glance.openstack.org_glances.yaml +++ b/config/crd/bases/glance.openstack.org_glances.yaml @@ -1629,6 +1629,23 @@ spec: Needed to request a transportURL that is created and used for notification purposes type: string + notificationsBus: + description: NotificationsBus configuration (username, vhost, and + cluster) for notifications + properties: + cluster: + description: Name of the cluster + minLength: 1 + type: string + user: + description: User - RabbitMQ username + type: string + vhost: + description: Vhost - RabbitMQ vhost name + type: string + required: + - cluster + type: object passwordSelectors: default: service: GlancePassword diff --git a/internal/controller/glance_controller.go b/internal/controller/glance_controller.go index 842d79f95..5672451f5 100644 --- a/internal/controller/glance_controller.go +++ b/internal/controller/glance_controller.go @@ -583,7 +583,11 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance // create RabbitMQ transportURL CR and get the actual URL from the associated secret that is created // if instance.Spec.NotificationBusInstance != nil && *instance.Spec.NotificationBusInstance != "" { - notificationTransportURL, op, err := r.transportURLCreateOrUpdate(ctx, instance, serviceLabels) + notificationsRabbitMqConfig := rabbitmqv1.RabbitMqConfig{} + if instance.Spec.NotificationsBus != nil { + notificationsRabbitMqConfig = *instance.Spec.NotificationsBus + } + notificationTransportURL, op, err := r.transportURLCreateOrUpdate(ctx, instance, serviceLabels, notificationsRabbitMqConfig) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.NotificationBusInstanceReadyCondition, @@ -1373,6 +1377,7 @@ func (r *GlanceReconciler) transportURLCreateOrUpdate( ctx context.Context, instance *glancev1.Glance, serviceLabels map[string]string, + rabbitMqConfig rabbitmqv1.RabbitMqConfig, ) (*rabbitmqv1.TransportURL, controllerutil.OperationResult, error) { transportURL := &rabbitmqv1.TransportURL{ ObjectMeta: metav1.ObjectMeta{ @@ -1384,9 +1389,12 @@ func (r *GlanceReconciler) transportURLCreateOrUpdate( op, err := controllerutil.CreateOrUpdate(ctx, r.Client, transportURL, func() error { transportURL.Spec.RabbitmqClusterName = *instance.Spec.NotificationBusInstance - - err := controllerutil.SetControllerReference(instance, transportURL, r.Scheme) - return err + if rabbitMqConfig.User != "" { + transportURL.Spec.Username = rabbitMqConfig.User + } + // Always set Vhost - empty string means use default "/" vhost + transportURL.Spec.Vhost = rabbitMqConfig.Vhost + return controllerutil.SetControllerReference(instance, transportURL, r.Scheme) }) return transportURL, op, err diff --git a/test/functional/glance_controller_test.go b/test/functional/glance_controller_test.go index c3f115d54..d8e2fd4c3 100644 --- a/test/functional/glance_controller_test.go +++ b/test/functional/glance_controller_test.go @@ -812,6 +812,91 @@ var _ = Describe("Glance controller", func() { }) }) + When("Glance is created with RabbitMQ user and vhost", func() { + BeforeEach(func() { + DeferCleanup(k8sClient.Delete, ctx, CreateGlanceMessageBusSecret(glanceTest.Instance.Namespace, glanceTest.RabbitmqSecretName)) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, glanceTest.MemcachedInstance, memcachedSpec)) + infra.SimulateMemcachedReady(glanceTest.GlanceMemcached) + spec := GetGlanceDefaultSpec() + spec["notificationBusInstance"] = glanceTest.NotificationsBusInstance + spec["notificationsBus"] = map[string]interface{}{ + "user": "glance-user", + "vhost": "glance-vhost", + } + DeferCleanup(th.DeleteInstance, CreateGlance(glanceTest.Instance, spec)) + }) + It("sets custom RabbitMQ user and vhost in TransportURL", func() { + Eventually(func(g Gomega) { + transportURL := infra.GetTransportURL(glanceTest.GlanceTransportURL) + g.Expect(transportURL.Spec.Username).To(Equal("glance-user")) + g.Expect(transportURL.Spec.Vhost).To(Equal("glance-vhost")) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("Glance is created without custom RabbitMQ config", func() { + BeforeEach(func() { + DeferCleanup(k8sClient.Delete, ctx, CreateGlanceMessageBusSecret(glanceTest.Instance.Namespace, glanceTest.RabbitmqSecretName)) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, glanceTest.MemcachedInstance, memcachedSpec)) + infra.SimulateMemcachedReady(glanceTest.GlanceMemcached) + spec := GetGlanceDefaultSpec() + spec["notificationBusInstance"] = glanceTest.NotificationsBusInstance + DeferCleanup(th.DeleteInstance, CreateGlance(glanceTest.Instance, spec)) + }) + It("uses default RabbitMQ configuration in TransportURL", func() { + Eventually(func(g Gomega) { + transportURL := infra.GetTransportURL(glanceTest.GlanceTransportURL) + g.Expect(transportURL.Spec.Username).To(BeEmpty()) + g.Expect(transportURL.Spec.Vhost).To(BeEmpty()) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("Glance starts with notifications enabled and then disables them", func() { + BeforeEach(func() { + DeferCleanup(k8sClient.Delete, ctx, CreateGlanceMessageBusSecret(glanceTest.Instance.Namespace, glanceTest.RabbitmqSecretName)) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, glanceTest.MemcachedInstance, memcachedSpec)) + infra.SimulateMemcachedReady(glanceTest.GlanceMemcached) + spec := GetGlanceDefaultSpec() + spec["notificationBusInstance"] = glanceTest.NotificationsBusInstance + spec["notificationsBus"] = map[string]interface{}{ + "user": "glance-notifications", + "vhost": "glance-notifications-vhost", + } + DeferCleanup(th.DeleteInstance, CreateGlance(glanceTest.Instance, spec)) + infra.SimulateTransportURLReady(glanceTest.GlanceTransportURL) + }) + + It("should initially have notifications enabled", func() { + Eventually(func(g Gomega) { + glance := GetGlance(glanceTest.Instance) + g.Expect(glance.Status.NotificationBusSecret).ToNot(BeEmpty()) + }, timeout, interval).Should(Succeed()) + }) + + It("should disable notifications when notificationBusInstance and notificationsBus are removed", func() { + // Verify notifications are initially enabled + Eventually(func(g Gomega) { + glance := GetGlance(glanceTest.Instance) + g.Expect(glance.Status.NotificationBusSecret).ToNot(BeEmpty()) + }, timeout, interval).Should(Succeed()) + + // Update the Glance spec to remove notifications + Eventually(func(g Gomega) { + glance := GetGlance(glanceTest.Instance) + glance.Spec.NotificationBusInstance = nil + glance.Spec.NotificationsBus = nil + g.Expect(k8sClient.Update(ctx, glance)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Wait for notifications to be disabled + Eventually(func(g Gomega) { + glance := GetGlance(glanceTest.Instance) + g.Expect(glance.Status.NotificationBusSecret).To(BeEmpty()) + }, timeout, interval).Should(Succeed()) + }) + }) + // Run MariaDBAccount suite tests. these are pre-packaged ginkgo tests // that exercise standard account create / update patterns that should be // common to all controllers that ensure MariaDBAccount CRs. diff --git a/test/functional/glance_webhook_test.go b/test/functional/glance_webhook_test.go new file mode 100644 index 000000000..1527435c3 --- /dev/null +++ b/test/functional/glance_webhook_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2025. + +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 functional + +import ( + "errors" + + . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports + . "github.com/onsi/gomega" //revive:disable:dot-imports + + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +var _ = Describe("Glance webhook", func() { + It("rejects update to deprecated notificationBusInstance field", func() { + spec := GetDefaultGlanceSpec() + notificationBusInstance := "notifications-rabbitmq" + spec["notificationBusInstance"] = notificationBusInstance + + // Set replicas to 0 to skip backend validation since this test + // is focused on testing notificationBusInstance field validation + glanceAPIs := spec["glanceAPIs"].(map[string]any) + defaultAPI := glanceAPIs["default"].(map[string]any) + defaultAPI["replicas"] = 0 + + glanceName := types.NamespacedName{ + Namespace: namespace, + Name: "glance-webhook-test", + } + + raw := map[string]any{ + "apiVersion": "glance.openstack.org/v1beta1", + "kind": "Glance", + "metadata": map[string]any{ + "name": glanceName.Name, + "namespace": glanceName.Namespace, + }, + "spec": spec, + } + + // Create the Glance instance + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + ctx, k8sClient, unstructuredObj, func() error { return nil }) + Expect(err).ShouldNot(HaveOccurred()) + + // Try to update notificationBusInstance + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, glanceName, unstructuredObj)).Should(Succeed()) + specMap := unstructuredObj.Object["spec"].(map[string]any) + specMap["notificationBusInstance"] = "notifications-rabbitmq2" + err := k8sClient.Update(ctx, unstructuredObj) + g.Expect(err).Should(HaveOccurred()) + + var statusError *k8s_errors.StatusError + g.Expect(errors.As(err, &statusError)).To(BeTrue()) + g.Expect(statusError.ErrStatus.Details.Kind).To(Equal("Glance")) + g.Expect(statusError.ErrStatus.Message).To( + ContainSubstring("notificationBusInstance is deprecated and cannot be changed")) + g.Expect(statusError.ErrStatus.Message).To( + ContainSubstring("Please use notificationsBus.cluster instead")) + }, timeout, interval).Should(Succeed()) + }) +})