Skip to content

Commit d85d9b2

Browse files
lmicciniclaude
andcommitted
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 5c4250a commit d85d9b2

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
@@ -942,4 +942,131 @@ var _ = Describe("OpenStackControlPlane Webhook", func() {
942942
})
943943
})
944944
})
945+
946+
Context("ValidateCreateServices - RabbitMQ instance requirement", func() {
947+
var instance *OpenStackControlPlane
948+
var basePath *field.Path
949+
950+
BeforeEach(func() {
951+
instance = &OpenStackControlPlane{
952+
ObjectMeta: metav1.ObjectMeta{
953+
Name: "test-controlplane",
954+
Namespace: "openstack",
955+
},
956+
Spec: OpenStackControlPlaneSpec{
957+
Rabbitmq: RabbitmqSection{
958+
Enabled: true,
959+
},
960+
},
961+
}
962+
basePath = field.NewPath("spec")
963+
})
964+
965+
It("should fail when RabbitMQ is enabled but no instances are defined (nil templates)", func() {
966+
instance.Spec.Rabbitmq.Templates = nil
967+
968+
warnings, errs := instance.ValidateCreateServices(basePath)
969+
Expect(warnings).To(BeEmpty())
970+
Expect(errs).To(HaveLen(1))
971+
Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
972+
Expect(errs[0].Field).To(Equal("spec.rabbitmq.templates"))
973+
Expect(errs[0].Detail).To(ContainSubstring("At least one RabbitMQ instance must be defined"))
974+
})
975+
976+
It("should fail when RabbitMQ is enabled but no instances are defined (empty templates)", func() {
977+
emptyTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{}
978+
instance.Spec.Rabbitmq.Templates = &emptyTemplates
979+
980+
warnings, errs := instance.ValidateCreateServices(basePath)
981+
Expect(warnings).To(BeEmpty())
982+
Expect(errs).To(HaveLen(1))
983+
Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
984+
Expect(errs[0].Field).To(Equal("spec.rabbitmq.templates"))
985+
Expect(errs[0].Detail).To(ContainSubstring("At least one RabbitMQ instance must be defined"))
986+
})
987+
988+
It("should succeed when RabbitMQ is enabled and at least one instance is defined", func() {
989+
templates := map[string]rabbitmqv1.RabbitMqSpecCore{
990+
"rabbitmq": {},
991+
}
992+
instance.Spec.Rabbitmq.Templates = &templates
993+
994+
warnings, errs := instance.ValidateCreateServices(basePath)
995+
Expect(warnings).To(BeEmpty())
996+
Expect(errs).To(BeEmpty())
997+
})
998+
999+
It("should succeed when RabbitMQ is disabled and no instances are defined", func() {
1000+
instance.Spec.Rabbitmq.Enabled = false
1001+
instance.Spec.Rabbitmq.Templates = nil
1002+
1003+
warnings, errs := instance.ValidateCreateServices(basePath)
1004+
Expect(warnings).To(BeEmpty())
1005+
Expect(errs).To(BeEmpty())
1006+
})
1007+
})
1008+
1009+
Context("ValidateUpdateServices - RabbitMQ instance requirement", func() {
1010+
var instance *OpenStackControlPlane
1011+
var oldSpec OpenStackControlPlaneSpec
1012+
var basePath *field.Path
1013+
1014+
BeforeEach(func() {
1015+
oldTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{
1016+
"rabbitmq": {},
1017+
}
1018+
oldSpec = OpenStackControlPlaneSpec{
1019+
Rabbitmq: RabbitmqSection{
1020+
Enabled: true,
1021+
Templates: &oldTemplates,
1022+
},
1023+
}
1024+
1025+
instance = &OpenStackControlPlane{
1026+
ObjectMeta: metav1.ObjectMeta{
1027+
Name: "test-controlplane",
1028+
Namespace: "openstack",
1029+
},
1030+
Spec: OpenStackControlPlaneSpec{
1031+
Rabbitmq: RabbitmqSection{
1032+
Enabled: true,
1033+
},
1034+
},
1035+
}
1036+
basePath = field.NewPath("spec")
1037+
})
1038+
1039+
It("should fail when trying to remove all RabbitMQ instances while enabled", func() {
1040+
emptyTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{}
1041+
instance.Spec.Rabbitmq.Templates = &emptyTemplates
1042+
1043+
warnings, errs := instance.ValidateUpdateServices(oldSpec, basePath)
1044+
Expect(warnings).To(BeEmpty())
1045+
Expect(errs).To(HaveLen(1))
1046+
Expect(errs[0].Type).To(Equal(field.ErrorTypeRequired))
1047+
Expect(errs[0].Field).To(Equal("spec.rabbitmq.templates"))
1048+
Expect(errs[0].Detail).To(ContainSubstring("At least one RabbitMQ instance must be defined"))
1049+
})
1050+
1051+
It("should succeed when updating with at least one RabbitMQ instance", func() {
1052+
templates := map[string]rabbitmqv1.RabbitMqSpecCore{
1053+
"rabbitmq": {},
1054+
}
1055+
instance.Spec.Rabbitmq.Templates = &templates
1056+
1057+
warnings, errs := instance.ValidateUpdateServices(oldSpec, basePath)
1058+
Expect(warnings).To(BeEmpty())
1059+
Expect(errs).To(BeEmpty())
1060+
})
1061+
1062+
It("should succeed when disabling RabbitMQ and removing all instances", func() {
1063+
instance.Spec.Rabbitmq.Enabled = false
1064+
emptyTemplates := map[string]rabbitmqv1.RabbitMqSpecCore{}
1065+
instance.Spec.Rabbitmq.Templates = &emptyTemplates
1066+
1067+
warnings, errs := instance.ValidateUpdateServices(oldSpec, basePath)
1068+
Expect(warnings).To(BeEmpty())
1069+
Expect(errs).To(BeEmpty())
1070+
})
1071+
})
9451072
})

0 commit comments

Comments
 (0)