Skip to content

Commit 8f84a6e

Browse files
Address review feedback for spec.groups validation
- Replace MinLength=1 with RFC 4122 UUID pattern validation on AggregateGroup.UUID to reject malformed UUIDs at the CRD level. - Tighten negative validation test assertions to check specific field paths in error messages (trait.name, aggregate.name, aggregate.uuid). - Add round-trip assertion verifying aggregate metadata survives serialization after creation. - Update all test UUIDs to valid RFC 4122 format.
1 parent ca3e552 commit 8f84a6e

3 files changed

Lines changed: 22 additions & 13 deletions

File tree

api/v1/hypervisor_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ type AggregateGroup struct {
244244
// +kubebuilder:validation:MinLength=1
245245
Name string `json:"name"`
246246

247-
// +kubebuilder:validation:MinLength=1
247+
// +kubebuilder:validation:Pattern=`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$`
248248
UUID string `json:"uuid"`
249249

250250
// +kubebuilder:validation:Optional

api/v1/hypervisor_validation_test.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -506,19 +506,21 @@ var _ = Describe("Groups CEL Validation", func() {
506506
}
507507
err := k8sClient.Create(ctx, hypervisor)
508508
Expect(err).To(HaveOccurred())
509+
Expect(err.Error()).To(ContainSubstring("spec.groups[0].trait.name"))
509510
})
510511

511512
It("should reject an aggregate with empty name", func(ctx SpecContext) {
512513
hypervisor = &Hypervisor{
513514
ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
514515
Spec: HypervisorSpec{
515516
Groups: []Group{
516-
{Aggregate: &AggregateGroup{Name: "", UUID: "uuid-1"}},
517+
{Aggregate: &AggregateGroup{Name: "", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}},
517518
},
518519
},
519520
}
520521
err := k8sClient.Create(ctx, hypervisor)
521522
Expect(err).To(HaveOccurred())
523+
Expect(err.Error()).To(ContainSubstring("spec.groups[0].aggregate.name"))
522524
})
523525

524526
It("should reject an aggregate with empty UUID", func(ctx SpecContext) {
@@ -532,14 +534,15 @@ var _ = Describe("Groups CEL Validation", func() {
532534
}
533535
err := k8sClient.Create(ctx, hypervisor)
534536
Expect(err).To(HaveOccurred())
537+
Expect(err.Error()).To(ContainSubstring("spec.groups[0].aggregate.uuid"))
535538
})
536539

537540
It("should accept an aggregate without metadata", func(ctx SpecContext) {
538541
hypervisor = &Hypervisor{
539542
ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name},
540543
Spec: HypervisorSpec{
541544
Groups: []Group{
542-
{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "uuid-1"}},
545+
{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}},
543546
},
544547
},
545548
}
@@ -553,13 +556,19 @@ var _ = Describe("Groups CEL Validation", func() {
553556
Groups: []Group{
554557
{Aggregate: &AggregateGroup{
555558
Name: "fast-storage",
556-
UUID: "uuid-1",
559+
UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11",
557560
Metadata: map[string]string{"ssd": "true"},
558561
}},
559562
},
560563
},
561564
}
562565
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
566+
567+
created := &Hypervisor{}
568+
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(hypervisor), created)).To(Succeed())
569+
Expect(created.Spec.Groups).To(HaveLen(1))
570+
Expect(created.Spec.Groups[0].Aggregate).NotTo(BeNil())
571+
Expect(created.Spec.Groups[0].Aggregate.Metadata).To(HaveKeyWithValue("ssd", "true"))
563572
})
564573

565574
It("should accept an empty groups list", func(ctx SpecContext) {
@@ -578,8 +587,8 @@ var _ = Describe("Group Helper Functions", func() {
578587
groups := []Group{
579588
{Trait: &TraitGroup{Name: "HW_CPU_X86_AVX2"}},
580589
{Trait: &TraitGroup{Name: "COMPUTE_STATUS_DISABLED"}},
581-
{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "uuid-1", Metadata: map[string]string{"ssd": "true"}}},
582-
{Aggregate: &AggregateGroup{Name: "slow-storage", UUID: "uuid-2"}},
590+
{Aggregate: &AggregateGroup{Name: "fast-storage", UUID: "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11", Metadata: map[string]string{"ssd": "true"}}},
591+
{Aggregate: &AggregateGroup{Name: "slow-storage", UUID: "b1ffbc99-9c0b-4ef8-bb6d-6bb9bd380a22"}},
583592
}
584593

585594
Context("HasTrait", func() {
@@ -605,7 +614,7 @@ var _ = Describe("Group Helper Functions", func() {
605614
})
606615

607616
It("should return empty for a list with no traits", func() {
608-
aggs := []Group{{Aggregate: &AggregateGroup{Name: "a", UUID: "u"}}}
617+
aggs := []Group{{Aggregate: &AggregateGroup{Name: "a", UUID: "d0eebc99-9c0b-4ef8-bb6d-6bb9bd380a33"}}}
609618
Expect(GetTraits(aggs)).To(BeEmpty())
610619
})
611620

@@ -616,15 +625,15 @@ var _ = Describe("Group Helper Functions", func() {
616625

617626
Context("HasAggregate", func() {
618627
It("should return true for an existing aggregate UUID", func() {
619-
Expect(HasAggregate(groups, "uuid-1")).To(BeTrue())
628+
Expect(HasAggregate(groups, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11")).To(BeTrue())
620629
})
621630

622631
It("should return false for a missing aggregate UUID", func() {
623-
Expect(HasAggregate(groups, "uuid-999")).To(BeFalse())
632+
Expect(HasAggregate(groups, "c2ffbc99-9c0b-4ef8-bb6d-6bb9bd380a99")).To(BeFalse())
624633
})
625634

626635
It("should return false for an empty list", func() {
627-
Expect(HasAggregate(nil, "uuid-1")).To(BeFalse())
636+
Expect(HasAggregate(nil, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11")).To(BeFalse())
628637
})
629638
})
630639

@@ -633,10 +642,10 @@ var _ = Describe("Group Helper Functions", func() {
633642
aggs := GetAggregates(groups)
634643
Expect(aggs).To(HaveLen(2))
635644
Expect(aggs[0].Name).To(Equal("fast-storage"))
636-
Expect(aggs[0].UUID).To(Equal("uuid-1"))
645+
Expect(aggs[0].UUID).To(Equal("a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"))
637646
Expect(aggs[0].Metadata).To(HaveKeyWithValue("ssd", "true"))
638647
Expect(aggs[1].Name).To(Equal("slow-storage"))
639-
Expect(aggs[1].UUID).To(Equal("uuid-2"))
648+
Expect(aggs[1].UUID).To(Equal("b1ffbc99-9c0b-4ef8-bb6d-6bb9bd380a22"))
640649
})
641650

642651
It("should return empty for a list with no aggregates", func() {

charts/openstack-hypervisor-operator/crds/kvm.cloud.sap_hypervisors.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ spec:
175175
minLength: 1
176176
type: string
177177
uuid:
178-
minLength: 1
178+
pattern: ^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$
179179
type: string
180180
required:
181181
- name

0 commit comments

Comments
 (0)