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