Skip to content

Commit cc45b15

Browse files
lmicciniclaude
lmiccini
and
claude
committed
Add validation to require at least one RabbitMQ instance
This commit adds webhook validation to ensure that when RabbitMQ is enabled (spec.rabbitmq.enabled: true), at least one RabbitMQ instance must be defined in spec.rabbitmq.templates. Previously, the validation only checked the boolean flag (spec.rabbitmq.enabled) without verifying that actual RabbitMQ instances were configured. This allowed invalid configurations where services requiring RabbitMQ could pass validation but would fail at runtime when trying to create TransportURLs to non-existent clusters. Changes: - ValidateCreateServices: Reject creation when rabbitmq.enabled is true but templates is nil or empty - ValidateUpdateServices: Reject updates that remove all instances while RabbitMQ remains enabled - Allow disabling RabbitMQ and removing all instances in the same update operation - Add 7 comprehensive test cases covering CREATE and UPDATE scenarios The validation provides a clear error message: "At least one RabbitMQ instance must be defined when rabbitmq is enabled" Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 6fd66d8 commit cc45b15

File tree

2 files changed

+145
-9
lines changed

2 files changed

+145
-9
lines changed

‎api/core/v1beta1/openstackcontrolplane_webhook.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,11 @@ func (r *OpenStackControlPlane) ValidateCreateServices(basePath *field.Path) (ad
530530
}
531531

532532
if r.Spec.Rabbitmq.Enabled {
533-
if r.Spec.Rabbitmq.Templates != nil {
533+
if r.Spec.Rabbitmq.Templates == nil || len(*r.Spec.Rabbitmq.Templates) == 0 {
534+
err := field.Required(basePath.Child("rabbitmq").Child("templates"),
535+
"At least one RabbitMQ instance must be defined when rabbitmq is enabled")
536+
errors = append(errors, err)
537+
} else {
534538
err := common_webhook.ValidateDNS1123Label(
535539
basePath.Child("rabbitmq").Child("templates"),
536540
maps.Keys(*r.Spec.Rabbitmq.Templates),
@@ -745,19 +749,24 @@ func (r *OpenStackControlPlane) ValidateUpdateServices(old OpenStackControlPlane
745749
if old.Rabbitmq.Templates == nil {
746750
old.Rabbitmq.Templates = &map[string]rabbitmqv1.RabbitMqSpecCore{}
747751
}
748-
if r.Spec.Rabbitmq.Templates != nil {
752+
if r.Spec.Rabbitmq.Templates == nil || len(*r.Spec.Rabbitmq.Templates) == 0 {
753+
err := field.Required(basePath.Child("rabbitmq").Child("templates"),
754+
"At least one RabbitMQ instance must be defined when rabbitmq is enabled")
755+
errors = append(errors, err)
756+
} else {
749757
err := common_webhook.ValidateDNS1123Label(
750758
basePath.Child("rabbitmq").Child("templates"),
751759
maps.Keys(*r.Spec.Rabbitmq.Templates),
752760
memcachedv1.CrMaxLengthCorrection) // omit issue with statefulset pod label "controller-revision-hash": "<statefulset_name>-<hash>"
753761
errors = append(errors, err...)
754-
}
755-
oldRabbitmqs := *old.Rabbitmq.Templates
756-
for rabbitmqName, rabbitmqSpec := range *r.Spec.Rabbitmq.Templates {
757-
if oldRabbitmq, ok := oldRabbitmqs[rabbitmqName]; ok {
758-
warn, errs := rabbitmqSpec.ValidateUpdate(oldRabbitmq, basePath.Child("rabbitmq").Child("template").Key(rabbitmqName), r.Namespace)
759-
warnings = append(warnings, warn...)
760-
errors = append(errors, errs...)
762+
763+
oldRabbitmqs := *old.Rabbitmq.Templates
764+
for rabbitmqName, rabbitmqSpec := range *r.Spec.Rabbitmq.Templates {
765+
if oldRabbitmq, ok := oldRabbitmqs[rabbitmqName]; ok {
766+
warn, errs := rabbitmqSpec.ValidateUpdate(oldRabbitmq, basePath.Child("rabbitmq").Child("template").Key(rabbitmqName), r.Namespace)
767+
warnings = append(warnings, warn...)
768+
errors = append(errors, errs...)
769+
}
761770
}
762771
}
763772
}

‎api/core/v1beta1/openstackcontrolplane_webhook_test.go‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,4 +1009,131 @@ var _ = Describe("OpenStackControlPlane Webhook", func() {
10091009
Expect(template.Secret).To(Equal(""))
10101010
})
10111011
})
1012+
1013+
Context("ValidateCreateServices - RabbitMQ instance requirement", func() {
1014+
var instance *OpenStackControlPlane
1015+
var basePath *field.Path
1016+
1017+
BeforeEach(func() {
1018+
instance = &OpenStackControlPlane{
1019+
ObjectMeta: metav1.ObjectMeta{
1020+
Name: "test-controlplane",
1021+
Namespace: "openstack",
1022+
},
1023+
Spec: OpenStackControlPlaneSpec{
1024+
Rabbitmq: RabbitmqSection{
1025+
Enabled: true,
1026+
},
1027+
},
1028+
}
1029+
basePath = field.NewPath("spec")
1030+
})
1031+
1032+
It("should fail when RabbitMQ is enabled but no instances are defined (nil templates)", func() {
1033+
instance.Spec.Rabbitmq.Templates = nil
1034+
1035+
warnings, errs := instance.ValidateCreateServices(basePath)
1036+
Expect(warnings).To(BeEmpty())
1037+
Expect(errs).To(HaveLen(1))
1038+
Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
1039+
Expect(errs[0].Field).To(Equal("spec.rabbitmq.templates"))
1040+
Expect(errs[0].Detail).To(ContainSubstring("At least one RabbitMQ instance must be defined"))
1041+
})
1042+
1043+
It("should fail when RabbitMQ is enabled but no instances are defined (empty templates)", func() {
1044+
emptyTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{}
1045+
instance.Spec.Rabbitmq.Templates = &emptyTemplates
1046+
1047+
warnings, errs := instance.ValidateCreateServices(basePath)
1048+
Expect(warnings).To(BeEmpty())
1049+
Expect(errs).To(HaveLen(1))
1050+
Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
1051+
Expect(errs[0].Field).To(Equal("spec.rabbitmq.templates"))
1052+
Expect(errs[0].Detail).To(ContainSubstring("At least one RabbitMQ instance must be defined"))
1053+
})
1054+
1055+
It("should succeed when RabbitMQ is enabled and at least one instance is defined", func() {
1056+
templates := map[string]rabbitmqv1.RabbitMqSpecCore{
1057+
"rabbitmq": {},
1058+
}
1059+
instance.Spec.Rabbitmq.Templates = &templates
1060+
1061+
warnings, errs := instance.ValidateCreateServices(basePath)
1062+
Expect(warnings).To(BeEmpty())
1063+
Expect(errs).To(BeEmpty())
1064+
})
1065+
1066+
It("should succeed when RabbitMQ is disabled and no instances are defined", func() {
1067+
instance.Spec.Rabbitmq.Enabled = false
1068+
instance.Spec.Rabbitmq.Templates = nil
1069+
1070+
warnings, errs := instance.ValidateCreateServices(basePath)
1071+
Expect(warnings).To(BeEmpty())
1072+
Expect(errs).To(BeEmpty())
1073+
})
1074+
})
1075+
1076+
Context("ValidateUpdateServices - RabbitMQ instance requirement", func() {
1077+
var instance *OpenStackControlPlane
1078+
var oldSpec OpenStackControlPlaneSpec
1079+
var basePath *field.Path
1080+
1081+
BeforeEach(func() {
1082+
oldTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{
1083+
"rabbitmq": {},
1084+
}
1085+
oldSpec = OpenStackControlPlaneSpec{
1086+
Rabbitmq: RabbitmqSection{
1087+
Enabled: true,
1088+
Templates: &oldTemplates,
1089+
},
1090+
}
1091+
1092+
instance = &OpenStackControlPlane{
1093+
ObjectMeta: metav1.ObjectMeta{
1094+
Name: "test-controlplane",
1095+
Namespace: "openstack",
1096+
},
1097+
Spec: OpenStackControlPlaneSpec{
1098+
Rabbitmq: RabbitmqSection{
1099+
Enabled: true,
1100+
},
1101+
},
1102+
}
1103+
basePath = field.NewPath("spec")
1104+
})
1105+
1106+
It("should fail when trying to remove all RabbitMQ instances while enabled", func() {
1107+
emptyTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{}
1108+
instance.Spec.Rabbitmq.Templates = &emptyTemplates
1109+
1110+
warnings, errs := instance.ValidateUpdateServices(oldSpec, basePath)
1111+
Expect(warnings).To(BeEmpty())
1112+
Expect(errs).To(HaveLen(1))
1113+
Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
1114+
Expect(errs[0].Field).To(Equal("spec.rabbitmq.templates"))
1115+
Expect(errs[0].Detail).To(ContainSubstring("At least one RabbitMQ instance must be defined"))
1116+
})
1117+
1118+
It("should succeed when updating with at least one RabbitMQ instance", func() {
1119+
templates := map[string]rabbitmqv1.RabbitMqSpecCore{
1120+
"rabbitmq": {},
1121+
}
1122+
instance.Spec.Rabbitmq.Templates = &templates
1123+
1124+
warnings, errs := instance.ValidateUpdateServices(oldSpec, basePath)
1125+
Expect(warnings).To(BeEmpty())
1126+
Expect(errs).To(BeEmpty())
1127+
})
1128+
1129+
It("should succeed when disabling RabbitMQ and removing all instances", func() {
1130+
instance.Spec.Rabbitmq.Enabled = false
1131+
emptyTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{}
1132+
instance.Spec.Rabbitmq.Templates = &emptyTemplates
1133+
1134+
warnings, errs := instance.ValidateUpdateServices(oldSpec, basePath)
1135+
Expect(warnings).To(BeEmpty())
1136+
Expect(errs).To(BeEmpty())
1137+
})
1138+
})
10121139
})

0 commit comments

Comments
 (0)