diff --git a/api/bases/watcher.openstack.org_watchers.yaml b/api/bases/watcher.openstack.org_watchers.yaml index 6ffefcda..5082253a 100644 --- a/api/bases/watcher.openstack.org_watchers.yaml +++ b/api/bases/watcher.openstack.org_watchers.yaml @@ -608,6 +608,22 @@ spec: description: MemcachedInstance is the name of the Memcached CR that all watcher service will use. type: string + messagingBus: + description: MessagingBus configuration (username, vhost, and cluster) + 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 nodeSelector: additionalProperties: type: string @@ -615,6 +631,23 @@ spec: NodeSelector to target subset of worker nodes running this component. Setting here overrides any global NodeSelector settings within the Watcher CR. type: object + 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 notificationsBusInstance: description: |- NotificationsBusInstance is the name of the RabbitMqCluster CR to select @@ -623,6 +656,7 @@ spec: If undefined, the value will be inherited from OpenStackControlPlane. An empty value "" leaves the notification drivers unconfigured and emitting no notifications at all. Avoid colocating it with RabbitMqClusterName or other message bus instances used for RPC. + Deprecated: Use NotificationsBus.Cluster instead type: string passwordSelectors: default: @@ -650,6 +684,7 @@ spec: description: |- RabbitMQ instance name Needed to request a transportURL that is created and used in Watcher + Deprecated: Use MessagingBus.Cluster instead type: string secret: default: osp-secret diff --git a/api/go.mod b/api/go.mod index c4033656..02b91c1f 100644 --- a/api/go.mod +++ b/api/go.mod @@ -5,10 +5,10 @@ go 1.24.4 toolchain go1.24.6 require ( - github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251110170511-c2d4a351a7c3 - github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251103072528-9eb684fef4ef - k8s.io/api v0.31.13 - k8s.io/apimachinery v0.31.13 + github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251217131115-0f117a938d4e + github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251122131503-b76943960b6c + k8s.io/api v0.31.14 + k8s.io/apimachinery v0.31.14 k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d sigs.k8s.io/controller-runtime v0.19.7 ) @@ -18,7 +18,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 @@ -45,10 +44,9 @@ 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/rogpeppe/go-internal v1.13.1 // indirect + github.com/rabbitmq/cluster-operator/v2 v2.16.0 // indirect github.com/spf13/pflag v1.0.7 // indirect github.com/x448/float16 v0.8.4 // indirect - go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67 // indirect @@ -63,7 +61,7 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiextensions-apiserver v0.33.2 // indirect - k8s.io/client-go v0.31.13 // indirect + k8s.io/client-go v0.31.14 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250902184714-7fc278399c7f // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect diff --git a/api/go.sum b/api/go.sum index 8c5ac0ee..f6a8aa90 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= @@ -78,10 +79,12 @@ github.com/onsi/ginkgo/v2 v2.27.2 h1:LzwLj0b89qtIy6SSASkzlNvX6WktqurSHwkk2ipF/Ns github.com/onsi/ginkgo/v2 v2.27.2/go.mod h1:ArE1D/XhNXBXCBkKOLkbsb2c81dQHCRcF5zwn/ykDRo= github.com/onsi/gomega v1.38.2 h1:eZCjf2xjZAqe+LeWvKb5weQ+NcPwX84kqJ0cZNxok2A= github.com/onsi/gomega v1.38.2/go.mod h1:W2MJcYxRGV63b418Ai34Ud0hEdTVXq9NW9+Sx6uXf3k= -github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251110170511-c2d4a351a7c3 h1:gKazSLpq0Ytn4OLzNtSKQpLswAdki8u8mXZgpJy83bE= -github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251110170511-c2d4a351a7c3/go.mod h1:Y9LqOS1wYhn7RT4jFknINdWa+ziYEIOU1jLNxkxiCsw= -github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251103072528-9eb684fef4ef h1:1j7kk+D4ZdIXm6C/IwEjuTzIuvWUytxO39E/x94JY7k= -github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251103072528-9eb684fef4ef/go.mod h1:kUT/SyuxZiOcX8ZuvpFN3PaQa2V8uQon8YwY+1RoQWM= +github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251217131115-0f117a938d4e h1:PIjcXzMMwfvBRFgFpaq/W9tqy0t2cYvcWX+kq6uNtTM= +github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251217131115-0f117a938d4e/go.mod h1:ex8ou6/3ms6ovR+CMXD6XhTlNakm1GhB6UZgagVRNW8= +github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251122131503-b76943960b6c h1:wM8qXCB5mQwSosCvtaydzuXitWVVKBHTzH0A2znQ+Jg= +github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251122131503-b76943960b6c/go.mod h1:+Me0raWPPdz8gRi9D4z1khmvUgS9vIKAVC8ckg1yJZU= +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/common_types.go b/api/v1beta1/common_types.go index 09a711a7..5d17b6e4 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -17,9 +17,9 @@ limitations under the License. package v1beta1 import ( - "github.com/openstack-k8s-operators/lib-common/modules/common/util" - + 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/util" corev1 "k8s.io/api/core/v1" ) @@ -83,10 +83,19 @@ type WatcherSpecCore struct { // Important: Run "make" to regenerate code after modifying this file WatcherCommon `json:",inline"` + // +kubebuilder:validation:Optional + // MessagingBus configuration (username, vhost, and cluster) + MessagingBus rabbitmqv1.RabbitMqConfig `json:"messagingBus,omitempty"` + + // +kubebuilder:validation:Optional + // NotificationsBus configuration (username, vhost, and cluster) for notifications + NotificationsBus *rabbitmqv1.RabbitMqConfig `json:"notificationsBus,omitempty"` + // +kubebuilder:validation:Required // +kubebuilder:default=rabbitmq // RabbitMQ instance name // Needed to request a transportURL that is created and used in Watcher + // Deprecated: Use MessagingBus.Cluster instead RabbitMqClusterName *string `json:"rabbitMqClusterName"` // +kubebuilder:validation:Optional @@ -136,6 +145,7 @@ type WatcherSpecCore struct { // If undefined, the value will be inherited from OpenStackControlPlane. // An empty value "" leaves the notification drivers unconfigured and emitting no notifications at all. // Avoid colocating it with RabbitMqClusterName or other message bus instances used for RPC. + // Deprecated: Use NotificationsBus.Cluster instead NotificationsBusInstance *string `json:"notificationsBusInstance,omitempty"` } diff --git a/api/v1beta1/watcher_webhook.go b/api/v1beta1/watcher_webhook.go index b6971327..065fb9c6 100644 --- a/api/v1beta1/watcher_webhook.go +++ b/api/v1beta1/watcher_webhook.go @@ -19,6 +19,7 @@ package v1beta1 import ( "fmt" + rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -59,12 +60,36 @@ func (r *Watcher) Default() { // Default - set defaults for this WatcherCore spec. func (spec *WatcherSpec) Default() { + spec.WatcherSpecCore.Default() spec.WatcherImages.Default(watcherDefaults) } // Default - set defaults for this WatcherSpecCore spec. func (spec *WatcherSpecCore) Default() { - // no validations . Placeholder for defaulting webhook integrated in the OpenStackControlPlane + // Apply kubebuilder default for RabbitMqClusterName if not set + if spec.RabbitMqClusterName == nil { + spec.RabbitMqClusterName = ptr.To("rabbitmq") + } + + // Default MessagingBus.Cluster from RabbitMqClusterName if not already set + if spec.MessagingBus.Cluster == "" { + rabbitmqv1.DefaultRabbitMqConfig(&spec.MessagingBus, *spec.RabbitMqClusterName) + } + + // Default NotificationsBus if NotificationsBusInstance is specified + if spec.NotificationsBusInstance != nil && *spec.NotificationsBusInstance != "" { + if spec.NotificationsBus == nil { + // Initialize NotificationsBus with MessagingBus values to inherit user/vhost + spec.NotificationsBus = &rabbitmqv1.RabbitMqConfig{ + User: spec.MessagingBus.User, + Vhost: spec.MessagingBus.Vhost, + } + } + // Always default the Cluster field from NotificationsBusInstance if it's empty + if spec.NotificationsBus.Cluster == "" { + rabbitmqv1.DefaultRabbitMqConfig(spec.NotificationsBus, *spec.NotificationsBusInstance) + } + } } var _ webhook.Validator = &Watcher{} @@ -95,7 +120,7 @@ func (spec *WatcherSpec) ValidateCreate(basePath *field.Path, namespace string) func (spec *WatcherSpecCore) ValidateCreate(basePath *field.Path, namespace string) field.ErrorList { var allErrs field.ErrorList - if *spec.DatabaseInstance == "" || spec.DatabaseInstance == nil { + if spec.DatabaseInstance == nil || *spec.DatabaseInstance == "" { allErrs = append( allErrs, field.Invalid( @@ -103,7 +128,7 @@ func (spec *WatcherSpecCore) ValidateCreate(basePath *field.Path, namespace stri ) } - if *spec.RabbitMqClusterName == "" || spec.RabbitMqClusterName == nil { + if spec.RabbitMqClusterName == nil || *spec.RabbitMqClusterName == "" { allErrs = append( allErrs, field.Invalid( @@ -148,7 +173,7 @@ func (spec *WatcherSpec) ValidateUpdate(old WatcherSpec, basePath *field.Path, n func (spec *WatcherSpecCore) ValidateUpdate(old WatcherSpecCore, basePath *field.Path, namespace string) field.ErrorList { var allErrs field.ErrorList - if *spec.DatabaseInstance == "" || spec.DatabaseInstance == nil { + if spec.DatabaseInstance == nil || *spec.DatabaseInstance == "" { allErrs = append( allErrs, field.Invalid( @@ -156,7 +181,7 @@ func (spec *WatcherSpecCore) ValidateUpdate(old WatcherSpecCore, basePath *field ) } - if *spec.RabbitMqClusterName == "" || spec.RabbitMqClusterName == nil { + if spec.RabbitMqClusterName == nil || *spec.RabbitMqClusterName == "" { allErrs = append( allErrs, field.Invalid( @@ -164,6 +189,22 @@ func (spec *WatcherSpecCore) ValidateUpdate(old WatcherSpecCore, basePath *field ) } + // Reject changes to deprecated RabbitMqClusterName field + if spec.RabbitMqClusterName != nil && old.RabbitMqClusterName != nil && + *spec.RabbitMqClusterName != *old.RabbitMqClusterName { + allErrs = append(allErrs, field.Forbidden( + basePath.Child("rabbitMqClusterName"), + "rabbitMqClusterName is deprecated and cannot be changed. Please use messagingBus.cluster instead")) + } + + // Reject changes to deprecated NotificationsBusInstance field + if spec.NotificationsBusInstance != nil && old.NotificationsBusInstance != nil && + *spec.NotificationsBusInstance != *old.NotificationsBusInstance { + allErrs = append(allErrs, field.Forbidden( + basePath.Child("notificationsBusInstance"), + "notificationsBusInstance is deprecated and cannot be changed. Please use notificationsBus.cluster instead")) + } + allErrs = append(allErrs, spec.ValidateWatcherTopology(basePath, namespace)...) return allErrs diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index a3555616..eaf1ac14 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" @@ -665,6 +666,12 @@ func (in *WatcherSpec) DeepCopy() *WatcherSpec { func (in *WatcherSpecCore) DeepCopyInto(out *WatcherSpecCore) { *out = *in in.WatcherCommon.DeepCopyInto(&out.WatcherCommon) + out.MessagingBus = in.MessagingBus + if in.NotificationsBus != nil { + in, out := &in.NotificationsBus, &out.NotificationsBus + *out = new(rabbitmqv1beta1.RabbitMqConfig) + **out = **in + } if in.RabbitMqClusterName != nil { in, out := &in.RabbitMqClusterName, &out.RabbitMqClusterName *out = new(string) diff --git a/config/crd/bases/watcher.openstack.org_watchers.yaml b/config/crd/bases/watcher.openstack.org_watchers.yaml index 6ffefcda..5082253a 100644 --- a/config/crd/bases/watcher.openstack.org_watchers.yaml +++ b/config/crd/bases/watcher.openstack.org_watchers.yaml @@ -608,6 +608,22 @@ spec: description: MemcachedInstance is the name of the Memcached CR that all watcher service will use. type: string + messagingBus: + description: MessagingBus configuration (username, vhost, and cluster) + 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 nodeSelector: additionalProperties: type: string @@ -615,6 +631,23 @@ spec: NodeSelector to target subset of worker nodes running this component. Setting here overrides any global NodeSelector settings within the Watcher CR. type: object + 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 notificationsBusInstance: description: |- NotificationsBusInstance is the name of the RabbitMqCluster CR to select @@ -623,6 +656,7 @@ spec: If undefined, the value will be inherited from OpenStackControlPlane. An empty value "" leaves the notification drivers unconfigured and emitting no notifications at all. Avoid colocating it with RabbitMqClusterName or other message bus instances used for RPC. + Deprecated: Use NotificationsBus.Cluster instead type: string passwordSelectors: default: @@ -650,6 +684,7 @@ spec: description: |- RabbitMQ instance name Needed to request a transportURL that is created and used in Watcher + Deprecated: Use MessagingBus.Cluster instead type: string secret: default: osp-secret diff --git a/go.mod b/go.mod index aa8f3b3c..46e5b210 100644 --- a/go.mod +++ b/go.mod @@ -8,17 +8,17 @@ require ( github.com/onsi/ginkgo/v2 v2.27.2 github.com/onsi/gomega v1.38.2 github.com/openshift/api v3.9.0+incompatible - github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251110170511-c2d4a351a7c3 + github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251217131115-0f117a938d4e github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20251027074845-ed8154b20ad1 - github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251103072528-9eb684fef4ef + github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251122131503-b76943960b6c github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20251103072528-9eb684fef4ef github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20251110170510-e669472c745c github.com/openstack-k8s-operators/watcher-operator/api v0.0.0-00010101000000-000000000000 go.uber.org/zap v1.27.1 gopkg.in/yaml.v3 v3.0.1 - k8s.io/api v0.31.13 - k8s.io/apimachinery v0.31.13 - k8s.io/client-go v0.31.13 + k8s.io/api v0.31.14 + k8s.io/apimachinery v0.31.14 + k8s.io/client-go v0.31.14 k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d sigs.k8s.io/controller-runtime v0.19.7 ) diff --git a/go.sum b/go.sum index 06805af9..411bc567 100644 --- a/go.sum +++ b/go.sum @@ -118,12 +118,12 @@ github.com/onsi/gomega v1.38.2 h1:eZCjf2xjZAqe+LeWvKb5weQ+NcPwX84kqJ0cZNxok2A= github.com/onsi/gomega v1.38.2/go.mod h1:W2MJcYxRGV63b418Ai34Ud0hEdTVXq9NW9+Sx6uXf3k= github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyUt0GEdoAE+r5TXy7YS21yNEo+2U= github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo= -github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251110170511-c2d4a351a7c3 h1:gKazSLpq0Ytn4OLzNtSKQpLswAdki8u8mXZgpJy83bE= -github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251110170511-c2d4a351a7c3/go.mod h1:Y9LqOS1wYhn7RT4jFknINdWa+ziYEIOU1jLNxkxiCsw= +github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251217131115-0f117a938d4e h1:PIjcXzMMwfvBRFgFpaq/W9tqy0t2cYvcWX+kq6uNtTM= +github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20251217131115-0f117a938d4e/go.mod h1:ex8ou6/3ms6ovR+CMXD6XhTlNakm1GhB6UZgagVRNW8= github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20251027074845-ed8154b20ad1 h1:QohvX44nxoV2GwvvOURGXYyDuCn4SCrnwubTKJtzehY= github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20251027074845-ed8154b20ad1/go.mod h1:FMFoO4MjEQ85JpdLtDHxYSZxvJ9KzHua+HdKhpl0KRI= -github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251103072528-9eb684fef4ef h1:1j7kk+D4ZdIXm6C/IwEjuTzIuvWUytxO39E/x94JY7k= -github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251103072528-9eb684fef4ef/go.mod h1:kUT/SyuxZiOcX8ZuvpFN3PaQa2V8uQon8YwY+1RoQWM= +github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251122131503-b76943960b6c h1:wM8qXCB5mQwSosCvtaydzuXitWVVKBHTzH0A2znQ+Jg= +github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20251122131503-b76943960b6c/go.mod h1:+Me0raWPPdz8gRi9D4z1khmvUgS9vIKAVC8ckg1yJZU= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20251021145236-2b84ec9fd9bb h1:wToXqX7AS1JV3Kna7RcJfkRart8rSGun2biKNfyY6Zg= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20251021145236-2b84ec9fd9bb/go.mod h1:yf13jWb60XV26eA7A8o86ZCXNWBLNK9dPkTSWFaTPCw= github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20250929092825-4c2402451077 h1:9tpPDBV2RLXMDgt13ec8XR2OatFriItseqg+Oyvx9GA= diff --git a/internal/controller/watcher_controller.go b/internal/controller/watcher_controller.go index 9c02e7b4..622180f7 100644 --- a/internal/controller/watcher_controller.go +++ b/internal/controller/watcher_controller.go @@ -204,7 +204,7 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // not-ready condition is managed here instead of in ensureMQ to distinguish between Error (when receiving) // an error, or Running when transportURL is empty. // - transportURL, op, err := r.ensureMQ(ctx, instance, helper, instance.Name+"-watcher-transport", *instance.Spec.RabbitMqClusterName, serviceLabels) + transportURL, op, err := r.ensureMQ(ctx, instance, helper, instance.Name+"-watcher-transport", instance.Spec.MessagingBus, serviceLabels) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( watcherv1beta1.WatcherRabbitMQTransportURLReadyCondition, @@ -233,14 +233,34 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re // create Notification RabbitMQ transportURL CR and get the actual URL from the associated secret that is created notificationURLSecret := &corev1.Secret{} + // Determine if notifications are enabled by checking both old and new API fields + notificationsEnabled := false + + // Check deprecated NotificationsBusInstance field first if instance.Spec.NotificationsBusInstance != nil && *instance.Spec.NotificationsBusInstance != "" { + notificationsEnabled = true + } + + // Check new NotificationsBus.Cluster field (takes precedence) + if instance.Spec.NotificationsBus != nil && instance.Spec.NotificationsBus.Cluster != "" { + notificationsEnabled = true + } + + if notificationsEnabled { instance.Status.Conditions.Set(condition.FalseCondition( watcherv1beta1.WatcherNotificationTransportURLReadyCondition, condition.RequestedReason, condition.SeverityInfo, watcherv1beta1.WatcherNotificationTransportURLReadyRunningMessage, )) - notificationURL, op, err := r.ensureMQ(ctx, instance, helper, instance.Name+"-watcher-notification", *instance.Spec.NotificationsBusInstance, serviceLabels) + + // Use NotificationsBus if specified, otherwise fall back to main MessagingBus config + notificationsRabbitMqConfig := instance.Spec.MessagingBus + if instance.Spec.NotificationsBus != nil { + notificationsRabbitMqConfig = *instance.Spec.NotificationsBus + } + + notificationURL, op, err := r.ensureMQ(ctx, instance, helper, instance.Name+"-watcher-notification", notificationsRabbitMqConfig, serviceLabels) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( watcherv1beta1.WatcherNotificationTransportURLReadyCondition, @@ -660,7 +680,7 @@ func (r *WatcherReconciler) ensureMQ( instance *watcherv1beta1.Watcher, h *helper.Helper, transportURLName string, - messageBusInstance string, + rabbitMqConfig rabbitmqv1.RabbitMqConfig, serviceLabels map[string]string, ) (*rabbitmqv1.TransportURL, controllerutil.OperationResult, error) { Log := r.GetLogger(ctx) @@ -675,7 +695,12 @@ func (r *WatcherReconciler) ensureMQ( } op, err := controllerutil.CreateOrUpdate(ctx, r.Client, transportURL, func() error { - transportURL.Spec.RabbitmqClusterName = messageBusInstance + transportURL.Spec.RabbitmqClusterName = rabbitMqConfig.Cluster + if rabbitMqConfig.User != "" { + transportURL.Spec.Username = rabbitMqConfig.User + } + // Always set Vhost - empty string means use default "/" vhost + transportURL.Spec.Vhost = rabbitMqConfig.Vhost err := controllerutil.SetControllerReference(instance, transportURL, r.Scheme) return err diff --git a/test/functional/watcher_controller_test.go b/test/functional/watcher_controller_test.go index 076bc1b2..4277cb77 100644 --- a/test/functional/watcher_controller_test.go +++ b/test/functional/watcher_controller_test.go @@ -710,6 +710,7 @@ var _ = Describe("Watcher controller", func() { It("should raise an error for empty databaseInstance", func() { spec := GetDefaultWatcherAPISpec() spec["databaseInstance"] = "" + spec["rabbitMqClusterName"] = "rabbitmq" raw := map[string]any{ "apiVersion": "watcher.openstack.org/v1beta1", @@ -741,6 +742,7 @@ var _ = Describe("Watcher controller", func() { spec := GetDefaultWatcherAPISpec() spec["topologyRef"] = map[string]any{"name": "foo", "namespace": "bar"} + spec["rabbitMqClusterName"] = "rabbitmq" raw := map[string]any{ "apiVersion": "watcher.openstack.org/v1beta1", @@ -771,6 +773,9 @@ var _ = Describe("Watcher controller", func() { It("should raise an error for empty RabbitMqClusterName", func() { spec := GetDefaultWatcherAPISpec() spec["rabbitMqClusterName"] = "" + spec["messagingBus"] = map[string]any{ + "cluster": "rabbitmq", + } raw := map[string]any{ "apiVersion": "watcher.openstack.org/v1beta1", diff --git a/test/functional/watcher_webhook_test.go b/test/functional/watcher_webhook_test.go index ed9b7c69..e9970325 100644 --- a/test/functional/watcher_webhook_test.go +++ b/test/functional/watcher_webhook_test.go @@ -20,9 +20,10 @@ import ( . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports - "k8s.io/utils/ptr" - + rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" watcherv1 "github.com/openstack-k8s-operators/watcher-operator/api/v1beta1" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" ) var _ = Describe("SetDefaultRouteAnnotations", func() { @@ -141,3 +142,428 @@ var _ = Describe("SetDefaultRouteAnnotations", func() { }) }) + +var _ = Describe("Watcher Webhook Messaging and Notifications", func() { + + Describe("RabbitMqClusterName defaulting to messagingBus.cluster", func() { + var spec *watcherv1.WatcherSpecCore + + BeforeEach(func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("my-rabbitmq"), + } + }) + + It("should default messagingBus.cluster from RabbitMqClusterName when messagingBus is empty", func() { + spec.Default() + + Expect(spec.MessagingBus.Cluster).To(Equal("my-rabbitmq")) + // Note: User and Vhost don't have defaults and remain empty unless explicitly set + Expect(spec.MessagingBus.User).To(Equal("")) + Expect(spec.MessagingBus.Vhost).To(Equal("")) + }) + + It("should not override messagingBus.cluster if already set", func() { + spec.MessagingBus.Cluster = "existing-cluster" + spec.Default() + + Expect(spec.MessagingBus.Cluster).To(Equal("existing-cluster")) + }) + }) + + Describe("Direct messagingBus field usage", func() { + var spec *watcherv1.WatcherSpecCore + + It("should preserve messagingBus fields when set directly", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + MessagingBus: rabbitmqv1.RabbitMqConfig{ + Cluster: "direct-cluster", + User: "custom-user", + Vhost: "/custom-vhost", + }, + } + spec.Default() + + Expect(spec.MessagingBus.Cluster).To(Equal("direct-cluster")) + Expect(spec.MessagingBus.User).To(Equal("custom-user")) + Expect(spec.MessagingBus.Vhost).To(Equal("/custom-vhost")) + }) + + It("should use messagingBus.cluster when both old and new fields are set", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("old-rabbitmq"), + MessagingBus: rabbitmqv1.RabbitMqConfig{ + Cluster: "new-cluster", + }, + } + spec.Default() + + // New field should take precedence + Expect(spec.MessagingBus.Cluster).To(Equal("new-cluster")) + }) + }) + + Describe("NotificationsBusInstance defaulting to notificationsBus.cluster", func() { + var spec *watcherv1.WatcherSpecCore + + BeforeEach(func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + NotificationsBusInstance: ptr.To("rabbitmq-notifications"), + } + }) + + It("should default notificationsBus.cluster from NotificationsBusInstance", func() { + spec.Default() + + Expect(spec.NotificationsBus).NotTo(BeNil()) + Expect(spec.NotificationsBus.Cluster).To(Equal("rabbitmq-notifications")) + }) + + It("should inherit user from messagingBus when NotificationsBusInstance is set", func() { + spec.Default() + + Expect(spec.NotificationsBus).NotTo(BeNil()) + // User is inherited from messagingBus, which is empty by default + Expect(spec.NotificationsBus.User).To(Equal("")) + }) + + It("should inherit vhost from messagingBus when NotificationsBusInstance is set", func() { + spec.Default() + + Expect(spec.NotificationsBus).NotTo(BeNil()) + // Vhost is inherited from messagingBus, which is empty by default + Expect(spec.NotificationsBus.Vhost).To(Equal("")) + }) + + It("should not create notificationsBus when NotificationsBusInstance is nil", func() { + spec.NotificationsBusInstance = nil + spec.Default() + + Expect(spec.NotificationsBus).To(BeNil()) + }) + + It("should not create notificationsBus when NotificationsBusInstance is empty string", func() { + spec.NotificationsBusInstance = ptr.To("") + spec.Default() + + Expect(spec.NotificationsBus).To(BeNil()) + }) + + It("should preserve existing notificationsBus.cluster if already set", func() { + spec.NotificationsBus = &rabbitmqv1.RabbitMqConfig{ + Cluster: "existing-notifications-cluster", + } + spec.Default() + + Expect(spec.NotificationsBus.Cluster).To(Equal("existing-notifications-cluster")) + }) + }) + + Describe("NotificationsBus inheritance from messagingBus", func() { + var spec *watcherv1.WatcherSpecCore + + It("should inherit user and vhost from messagingBus when notificationsBus is created", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + NotificationsBusInstance: ptr.To("rabbitmq-notifications"), + MessagingBus: rabbitmqv1.RabbitMqConfig{ + User: "custom-user", + Vhost: "/custom-vhost", + }, + } + spec.Default() + + Expect(spec.NotificationsBus).NotTo(BeNil()) + Expect(spec.NotificationsBus.User).To(Equal("custom-user")) + Expect(spec.NotificationsBus.Vhost).To(Equal("/custom-vhost")) + Expect(spec.NotificationsBus.Cluster).To(Equal("rabbitmq-notifications")) + }) + + It("should not override notificationsBus fields if already set", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + NotificationsBusInstance: ptr.To("rabbitmq-notifications"), + NotificationsBus: &rabbitmqv1.RabbitMqConfig{ + Cluster: "custom-notifications-cluster", + User: "custom-notifications-user", + Vhost: "/custom-notifications-vhost", + }, + } + spec.Default() + + Expect(spec.NotificationsBus.Cluster).To(Equal("custom-notifications-cluster")) + Expect(spec.NotificationsBus.User).To(Equal("custom-notifications-user")) + Expect(spec.NotificationsBus.Vhost).To(Equal("/custom-notifications-vhost")) + }) + }) + + Describe("Direct notificationsBus field usage", func() { + var spec *watcherv1.WatcherSpecCore + + It("should preserve notificationsBus fields when set directly without NotificationsBusInstance", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + NotificationsBus: &rabbitmqv1.RabbitMqConfig{ + Cluster: "direct-notifications-cluster", + User: "custom-user", + Vhost: "/custom-vhost", + }, + } + spec.Default() + + Expect(spec.NotificationsBus.Cluster).To(Equal("direct-notifications-cluster")) + Expect(spec.NotificationsBus.User).To(Equal("custom-user")) + Expect(spec.NotificationsBus.Vhost).To(Equal("/custom-vhost")) + }) + + It("should use notificationsBus.cluster when both old and new fields are set", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + NotificationsBusInstance: ptr.To("old-notifications"), + NotificationsBus: &rabbitmqv1.RabbitMqConfig{ + Cluster: "new-notifications-cluster", + }, + } + spec.Default() + + // New field should take precedence (already set, so defaulting shouldn't override) + Expect(spec.NotificationsBus.Cluster).To(Equal("new-notifications-cluster")) + }) + }) + + Describe("Complex scenarios with multiple fields", func() { + var spec *watcherv1.WatcherSpecCore + + It("should handle all deprecated and new fields together correctly", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + NotificationsBusInstance: ptr.To("rabbitmq-notifications"), + MessagingBus: rabbitmqv1.RabbitMqConfig{ + User: "messaging-user", + Vhost: "/messaging-vhost", + }, + } + spec.Default() + + // messagingBus should be defaulted from RabbitMqClusterName + Expect(spec.MessagingBus.Cluster).To(Equal("rabbitmq")) + Expect(spec.MessagingBus.User).To(Equal("messaging-user")) + Expect(spec.MessagingBus.Vhost).To(Equal("/messaging-vhost")) + + // notificationsBus should inherit user/vhost and default cluster from NotificationsBusInstance + Expect(spec.NotificationsBus).NotTo(BeNil()) + Expect(spec.NotificationsBus.Cluster).To(Equal("rabbitmq-notifications")) + Expect(spec.NotificationsBus.User).To(Equal("messaging-user")) + Expect(spec.NotificationsBus.Vhost).To(Equal("/messaging-vhost")) + }) + + It("should prioritize new fields over deprecated fields", func() { + spec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("old-rabbitmq"), + NotificationsBusInstance: ptr.To("old-notifications"), + MessagingBus: rabbitmqv1.RabbitMqConfig{ + Cluster: "new-rabbitmq", + User: "new-user", + Vhost: "/new-vhost", + }, + NotificationsBus: &rabbitmqv1.RabbitMqConfig{ + Cluster: "new-notifications", + User: "new-notifications-user", + Vhost: "/new-notifications-vhost", + }, + } + spec.Default() + + Expect(spec.MessagingBus.Cluster).To(Equal("new-rabbitmq")) + Expect(spec.MessagingBus.User).To(Equal("new-user")) + Expect(spec.MessagingBus.Vhost).To(Equal("/new-vhost")) + + Expect(spec.NotificationsBus.Cluster).To(Equal("new-notifications")) + Expect(spec.NotificationsBus.User).To(Equal("new-notifications-user")) + Expect(spec.NotificationsBus.Vhost).To(Equal("/new-notifications-vhost")) + }) + }) + +}) + +var _ = Describe("Watcher Webhook Update Validation", func() { + + Describe("Validation of deprecated field changes", func() { + var ( + oldSpec *watcherv1.WatcherSpecCore + newSpec *watcherv1.WatcherSpecCore + basePath *field.Path + ) + + BeforeEach(func() { + basePath = field.NewPath("spec") + oldSpec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + DatabaseInstance: ptr.To("openstack"), + } + newSpec = &watcherv1.WatcherSpecCore{ + RabbitMqClusterName: ptr.To("rabbitmq"), + DatabaseInstance: ptr.To("openstack"), + } + }) + + Describe("RabbitMqClusterName field changes", func() { + It("should reject changes to RabbitMqClusterName", func() { + newSpec.RabbitMqClusterName = ptr.To("new-rabbitmq") + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Type).To(Equal(field.ErrorTypeForbidden)) + Expect(errs[0].Field).To(Equal("spec.rabbitMqClusterName")) + Expect(errs[0].Detail).To(ContainSubstring("deprecated and cannot be changed")) + Expect(errs[0].Detail).To(ContainSubstring("messagingBus.cluster")) + }) + + It("should allow update when RabbitMqClusterName remains unchanged", func() { + // Both specs have the same RabbitMqClusterName + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + // Should have no errors related to RabbitMqClusterName + for _, err := range errs { + Expect(err.Field).NotTo(Equal("spec.rabbitMqClusterName")) + } + }) + + It("should have validation error when RabbitMqClusterName is nil", func() { + oldSpec.RabbitMqClusterName = nil + newSpec.RabbitMqClusterName = nil + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + // Should have validation error for empty RabbitMqClusterName + found := false + for _, err := range errs { + if err.Field == "spec.rabbitMqClusterName" { + found = true + Expect(err.Type).To(Equal(field.ErrorTypeInvalid)) + } + } + Expect(found).To(BeTrue(), "Expected validation error for nil rabbitMqClusterName") + }) + }) + + Describe("NotificationsBusInstance field changes", func() { + BeforeEach(func() { + oldSpec.NotificationsBusInstance = ptr.To("rabbitmq-notifications") + newSpec.NotificationsBusInstance = ptr.To("rabbitmq-notifications") + }) + + It("should reject changes to NotificationsBusInstance", func() { + newSpec.NotificationsBusInstance = ptr.To("new-rabbitmq-notifications") + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + Expect(errs).To(HaveLen(1)) + Expect(errs[0].Type).To(Equal(field.ErrorTypeForbidden)) + Expect(errs[0].Field).To(Equal("spec.notificationsBusInstance")) + Expect(errs[0].Detail).To(ContainSubstring("deprecated and cannot be changed")) + Expect(errs[0].Detail).To(ContainSubstring("notificationsBus.cluster")) + }) + + It("should allow update when NotificationsBusInstance remains unchanged", func() { + // Both specs have the same NotificationsBusInstance + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + // Should have no errors related to NotificationsBusInstance + for _, err := range errs { + Expect(err.Field).NotTo(Equal("spec.notificationsBusInstance")) + } + }) + + It("should allow update when NotificationsBusInstance is nil in both specs", func() { + oldSpec.NotificationsBusInstance = nil + newSpec.NotificationsBusInstance = nil + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + // Should have no errors related to NotificationsBusInstance + for _, err := range errs { + Expect(err.Field).NotTo(Equal("spec.notificationsBusInstance")) + } + }) + }) + + Describe("Multiple deprecated field changes", func() { + It("should reject changes to both deprecated fields and return multiple errors", func() { + oldSpec.NotificationsBusInstance = ptr.To("rabbitmq-notifications") + newSpec.RabbitMqClusterName = ptr.To("new-rabbitmq") + newSpec.NotificationsBusInstance = ptr.To("new-rabbitmq-notifications") + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + Expect(errs).To(HaveLen(2)) + + // Check for RabbitMqClusterName error + rabbitMqErr := false + notificationsErr := false + for _, err := range errs { + if err.Field == "spec.rabbitMqClusterName" { + rabbitMqErr = true + Expect(err.Type).To(Equal(field.ErrorTypeForbidden)) + } + if err.Field == "spec.notificationsBusInstance" { + notificationsErr = true + Expect(err.Type).To(Equal(field.ErrorTypeForbidden)) + } + } + Expect(rabbitMqErr).To(BeTrue(), "Expected error for rabbitMqClusterName") + Expect(notificationsErr).To(BeTrue(), "Expected error for notificationsBusInstance") + }) + }) + + Describe("New messagingBus and notificationsBus field changes", func() { + It("should allow changes to messagingBus fields", func() { + oldSpec.MessagingBus = rabbitmqv1.RabbitMqConfig{ + Cluster: "old-cluster", + User: "old-user", + Vhost: "/old-vhost", + } + newSpec.MessagingBus = rabbitmqv1.RabbitMqConfig{ + Cluster: "new-cluster", + User: "new-user", + Vhost: "/new-vhost", + } + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + // Should have no forbidden errors for messagingBus fields + for _, err := range errs { + if err.Type == field.ErrorTypeForbidden { + Expect(err.Field).NotTo(ContainSubstring("messagingBus")) + } + } + }) + + It("should allow changes to notificationsBus fields", func() { + oldSpec.NotificationsBus = &rabbitmqv1.RabbitMqConfig{ + Cluster: "old-notifications-cluster", + User: "old-user", + Vhost: "/old-vhost", + } + newSpec.NotificationsBus = &rabbitmqv1.RabbitMqConfig{ + Cluster: "new-notifications-cluster", + User: "new-user", + Vhost: "/new-vhost", + } + + errs := newSpec.ValidateUpdate(*oldSpec, basePath, "test-namespace") + + // Should have no forbidden errors for notificationsBus fields + for _, err := range errs { + if err.Type == field.ErrorTypeForbidden { + Expect(err.Field).NotTo(ContainSubstring("notificationsBus")) + } + } + }) + }) + }) + +})