Skip to content

Commit dadc25e

Browse files
committed
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]>
1 parent aa19171 commit dadc25e

File tree

3 files changed

+72
-0
lines changed

3 files changed

+72
-0
lines changed

apis/core/v1beta1/openstackcontrolplane_webhook.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@ func (r *OpenStackControlPlane) ValidateCreate() (admission.Warnings, error) {
132132
allErrs = append(allErrs, err)
133133
}
134134

135+
if err := r.ValidateNotificationsBusInstance(basePath); err != nil {
136+
allErrs = append(allErrs, err)
137+
}
138+
135139
if len(allErrs) != 0 {
136140
return allWarn, apierrors.NewInvalid(
137141
schema.GroupKind{Group: "core.openstack.org", Kind: "OpenStackControlPlane"},
@@ -161,6 +165,10 @@ func (r *OpenStackControlPlane) ValidateUpdate(old runtime.Object) (admission.Wa
161165
allErrs = append(allErrs, err)
162166
}
163167

168+
if err := r.ValidateNotificationsBusInstance(basePath); err != nil {
169+
allErrs = append(allErrs, err)
170+
}
171+
164172
if len(allErrs) != 0 {
165173
return nil, apierrors.NewInvalid(
166174
schema.GroupKind{Group: "core.openstack.org", Kind: "OpenStackControlPlane"},
@@ -1122,3 +1130,28 @@ func (r *OpenStackControlPlane) ValidateTopology(basePath *field.Path) *field.Er
11221130
}
11231131
return nil
11241132
}
1133+
1134+
// ValidateNotificationsBusInstance - returns an error if the notificationsBusInstance
1135+
// parameter is not valid.
1136+
// - nil or empty string must be raised as an error
1137+
// - when notificationsBusInstance does not point to an existing RabbitMQ instance
1138+
func (r *OpenStackControlPlane) ValidateNotificationsBusInstance(basePath *field.Path) *field.Error {
1139+
notificationsField := basePath.Child("notificationsBusInstance")
1140+
// no notificationsBusInstance field set, nothing to validate here
1141+
if r.Spec.NotificationsBusInstance == nil {
1142+
return nil
1143+
}
1144+
// When NotificationsBusInstance is set, fail if it is an empty string
1145+
if *r.Spec.NotificationsBusInstance == "" {
1146+
return field.Invalid(notificationsField, *r.Spec.NotificationsBusInstance, "notificationsBusInstance is not a valid string")
1147+
}
1148+
// NotificationsBusInstance is set and must be equal to an existing
1149+
// deployed rabbitmq instance, otherwise we should fail because it
1150+
// does not represent a valid string
1151+
for k := range(*r.Spec.Rabbitmq.Templates) {
1152+
if *r.Spec.NotificationsBusInstance == k {
1153+
return nil
1154+
}
1155+
}
1156+
return field.Invalid(notificationsField, *r.Spec.NotificationsBusInstance, "notificationsBusInstance must match an existing RabbitMQ instance name")
1157+
}

pkg/openstack/nova.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
7272
instance.Spec.Nova.Template.NodeSelector = &instance.Spec.NodeSelector
7373
}
7474

75+
// When no NotificationsBusInstance is referenced in the subCR (override)
76+
// try to inject the top-level one if defined
77+
if instance.Spec.Nova.Template.NotificationsBusInstance == nil {
78+
instance.Spec.Nova.Template.NotificationsBusInstance = instance.Spec.NotificationsBusInstance
79+
}
80+
7581
// When there's no Topology referenced in the Service Template, inject the
7682
// top-level one
7783
// NOTE: This does not check the Service subCRs: by default the generated

tests/functional/ctlplane/openstackoperator_controller_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2833,6 +2833,39 @@ var _ = Describe("OpenStackOperator controller", func() {
28332833

28342834
var _ = Describe("OpenStackOperator Webhook", func() {
28352835

2836+
DescribeTable("notificationsBusInstance",
2837+
func(getNotificationField func() (string, string)) {
2838+
spec := GetDefaultOpenStackControlPlaneSpec()
2839+
value, errMsg := getNotificationField()
2840+
spec["notificationsBusInstance"] = value
2841+
raw := map[string]interface{}{
2842+
"apiVersion": "core.openstack.org/v1beta1",
2843+
"kind": "OpenStackControlPlane",
2844+
"metadata": map[string]interface{}{
2845+
"name": "foo",
2846+
"namespace": namespace,
2847+
},
2848+
"spec": spec,
2849+
}
2850+
unstructuredObj := &unstructured.Unstructured{Object: raw}
2851+
_, err := controllerutil.CreateOrPatch(
2852+
th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
2853+
Expect(err).Should(HaveOccurred())
2854+
var statusError *k8s_errors.StatusError
2855+
Expect(errors.As(err, &statusError)).To(BeTrue())
2856+
Expect(statusError.ErrStatus.Details.Kind).To(Equal("OpenStackControlPlane"))
2857+
Expect(statusError.ErrStatus.Message).To(
2858+
ContainSubstring(errMsg),
2859+
)
2860+
},
2861+
Entry("notificationsBusInstance is wrong", func() (string, string) {
2862+
return "foo", "spec.notificationsBusInstance: Invalid value: \"foo\": notificationsBusInstance must match an existing RabbitMQ instance name"
2863+
}),
2864+
Entry("notificationsBusInstance is an empty string", func() (string, string) {
2865+
return "", "spec.notificationsBusInstance: Invalid value: \"\": notificationsBusInstance is not a valid string"
2866+
}),
2867+
)
2868+
28362869
It("Blocks creating multiple ctlplane CRs in the same namespace", func() {
28372870
spec := GetDefaultOpenStackControlPlaneSpec()
28382871
spec["tls"] = GetTLSPublicSpec()

0 commit comments

Comments
 (0)