Add notificationBusInstance webhook validation

Because we can't control user input, this patch introduces a webhook
function to validate what we expect users to set as notificationsBusInstance
top-level parameter.
It also adds the webhook associated envTest to validate the use cases
covered by the new function.

Signed-off-by: Francesco Pantano <[email protected]>

Author: fmount

apis/core/v1beta1/openstackcontrolplane_webhook.go

@@ -132,6 +132,10 @@ func (r *OpenStackControlPlane) ValidateCreate() (admission.Warnings, error) {
 		allErrs = append(allErrs, err)
 	}
 
+	if err := r.ValidateNotificationsBusInstance(basePath); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	if len(allErrs) != 0 {
 		return allWarn, apierrors.NewInvalid(
 			schema.GroupKind{Group: "core.openstack.org", Kind: "OpenStackControlPlane"},
@@ -161,6 +165,10 @@ func (r *OpenStackControlPlane) ValidateUpdate(old runtime.Object) (admission.Wa
 		allErrs = append(allErrs, err)
 	}
 
+	if err := r.ValidateNotificationsBusInstance(basePath); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	if len(allErrs) != 0 {
 		return nil, apierrors.NewInvalid(
 			schema.GroupKind{Group: "core.openstack.org", Kind: "OpenStackControlPlane"},
@@ -1138,3 +1146,28 @@ func (r *OpenStackControlPlane) ValidateTopology(basePath *field.Path) *field.Er
 	}
 	return nil
 }
+
+// ValidateNotificationsBusInstance - returns an error if the notificationsBusInstance
+// parameter is not valid.
+// - nil or empty string must be raised as an error
+// - when notificationsBusInstance does not point to an existing RabbitMQ instance
+func (r *OpenStackControlPlane) ValidateNotificationsBusInstance(basePath *field.Path) *field.Error {
+	notificationsField := basePath.Child("notificationsBusInstance")
+	// no notificationsBusInstance field set, nothing to validate here
+	if r.Spec.NotificationsBusInstance == nil {
+		return nil
+	}
+	// When NotificationsBusInstance is set, fail if it is an empty string
+	if *r.Spec.NotificationsBusInstance == "" {
+		return field.Invalid(notificationsField, *r.Spec.NotificationsBusInstance, "notificationsBusInstance is not a valid string")
+	}
+	// NotificationsBusInstance is set and must be equal to an existing
+	// deployed rabbitmq instance, otherwise we should fail because it
+	// does not represent a valid string
+	for k := range(*r.Spec.Rabbitmq.Templates) {
+		if *r.Spec.NotificationsBusInstance == k {
+			return nil
+		}
+	}
+	return field.Invalid(notificationsField, *r.Spec.NotificationsBusInstance, "notificationsBusInstance must match an existing RabbitMQ instance name")
+}

pkg/openstack/neutron.go

@@ -155,6 +155,12 @@ func ReconcileNeutron(ctx context.Context, instance *corev1beta1.OpenStackContro
 		instance.Spec.Neutron.Template.TopologyRef = instance.Spec.TopologyRef
 	}
 
+	// When no NotificationsBusInstance is referenced in the subCR (override)
+	// try to inject the top-level one if defined
+	if instance.Spec.Neutron.Template.NotificationsBusInstance == nil {
+		instance.Spec.Neutron.Template.NotificationsBusInstance = instance.Spec.NotificationsBusInstance
+	}
+
 	Log.Info("Reconciling NeutronAPI", "NeutronAPI.Namespace", instance.Namespace, "NeutronAPI.Name", "neutron")
 	op, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), neutronAPI, func() error {
 		instance.Spec.Neutron.Template.DeepCopyInto(&neutronAPI.Spec.NeutronAPISpecCore)

pkg/openstack/nova.go

@@ -72,6 +72,12 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
 		instance.Spec.Nova.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
+	// When no NotificationsBusInstance is referenced in the subCR (override)
+	// try to inject the top-level one if defined
+	if instance.Spec.Nova.Template.NotificationsBusInstance == nil {
+		instance.Spec.Nova.Template.NotificationsBusInstance = instance.Spec.NotificationsBusInstance
+	}
+
 	// When there's no Topology referenced in the Service Template, inject the
 	// top-level one
 	// NOTE: This does not check the Service subCRs: by default the generated

pkg/openstack/watcher.go

@@ -114,6 +114,12 @@ func ReconcileWatcher(ctx context.Context, instance *corev1beta1.OpenStackContro
 		instance.Spec.Watcher.Template.TopologyRef = instance.Spec.TopologyRef
 	}
 
+	// When no NotificationsBusInstance is referenced in the subCR (override)
+	// try to inject the top-level one if defined
+	if instance.Spec.Watcher.Template.NotificationsBusInstance == nil {
+		instance.Spec.Watcher.Template.NotificationsBusInstance = instance.Spec.NotificationsBusInstance
+	}
+
 	helper.GetLogger().Info("Reconciling Watcher", "Watcher.Namespace", instance.Namespace, "Watcher.Name", "watcher")
 	op, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), watcher, func() error {
 		instance.Spec.Watcher.Template.DeepCopyInto(&watcher.Spec.WatcherSpecCore)

tests/functional/ctlplane/openstackoperator_controller_test.go

@@ -2835,6 +2835,39 @@ var _ = Describe("OpenStackOperator controller", func() {
 
 var _ = Describe("OpenStackOperator Webhook", func() {
 
+	DescribeTable("notificationsBusInstance",
+		func(getNotificationField func() (string, string)) {
+			spec := GetDefaultOpenStackControlPlaneSpec()
+			value, errMsg := getNotificationField()
+			spec["notificationsBusInstance"] = value
+			raw := map[string]interface{}{
+				"apiVersion": "core.openstack.org/v1beta1",
+				"kind":       "OpenStackControlPlane",
+				"metadata": map[string]interface{}{
+					"name":      "foo",
+					"namespace": namespace,
+				},
+				"spec": spec,
+			}
+			unstructuredObj := &unstructured.Unstructured{Object: raw}
+			_, err := controllerutil.CreateOrPatch(
+				th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
+			Expect(err).Should(HaveOccurred())
+			var statusError *k8s_errors.StatusError
+			Expect(errors.As(err, &statusError)).To(BeTrue())
+			Expect(statusError.ErrStatus.Details.Kind).To(Equal("OpenStackControlPlane"))
+			Expect(statusError.ErrStatus.Message).To(
+				ContainSubstring(errMsg),
+			)
+		},
+		Entry("notificationsBusInstance is wrong", func() (string, string) {
+			return "foo", "spec.notificationsBusInstance: Invalid value: \"foo\": notificationsBusInstance must match an existing RabbitMQ instance name"
+		}),
+		Entry("notificationsBusInstance is an empty string", func() (string, string) {
+			return "", "spec.notificationsBusInstance: Invalid value: \"\": notificationsBusInstance is not a valid string"
+		}),
+	)
+
 	It("Blocks creating multiple ctlplane CRs in the same namespace", func() {
 		spec := GetDefaultOpenStackControlPlaneSpec()
 		spec["tls"] = GetTLSPublicSpec()