Skip to content

Commit 5ea1855

Browse files
authored
Merge pull request #1080 from sbueringer/pr-fix-items-enum
🐛 Fix item validation for unhashable markers
2 parents 49ae6f8 + 79a0f50 commit 5ea1855

File tree

3 files changed

+34
-12
lines changed

3 files changed

+34
-12
lines changed

pkg/crd/schema.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,29 +125,38 @@ func infoToSchema(ctx *schemaContext) *apiext.JSONSchemaProps {
125125
return typeToSchema(ctx, ctx.info.RawSpec.Type)
126126
}
127127

128+
type schemaMarkerWithName struct {
129+
SchemaMarker SchemaMarker
130+
Name string
131+
}
132+
128133
// applyMarkers applies schema markers given their priority to the given schema
129134
func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *apiext.JSONSchemaProps, node ast.Node) {
130-
markers := make([]SchemaMarker, 0, len(markerSet))
131-
itemsMarkers := make([]SchemaMarker, 0, len(markerSet))
132-
itemsMarkerNames := make(map[SchemaMarker]string)
135+
markers := make([]schemaMarkerWithName, 0, len(markerSet))
136+
itemsMarkers := make([]schemaMarkerWithName, 0, len(markerSet))
133137

134138
for markerName, markerValues := range markerSet {
135139
for _, markerValue := range markerValues {
136140
if schemaMarker, isSchemaMarker := markerValue.(SchemaMarker); isSchemaMarker {
137141
if strings.HasPrefix(markerName, crdmarkers.ValidationItemsPrefix) {
138-
itemsMarkers = append(itemsMarkers, schemaMarker)
139-
itemsMarkerNames[schemaMarker] = markerName
142+
itemsMarkers = append(itemsMarkers, schemaMarkerWithName{
143+
SchemaMarker: schemaMarker,
144+
Name: markerName,
145+
})
140146
} else {
141-
markers = append(markers, schemaMarker)
147+
markers = append(markers, schemaMarkerWithName{
148+
SchemaMarker: schemaMarker,
149+
Name: markerName,
150+
})
142151
}
143152
}
144153
}
145154
}
146155

147-
cmpPriority := func(markers []SchemaMarker, i, j int) bool {
156+
cmpPriority := func(markers []schemaMarkerWithName, i, j int) bool {
148157
var iPriority, jPriority crdmarkers.ApplyPriority
149158

150-
switch m := markers[i].(type) {
159+
switch m := markers[i].SchemaMarker.(type) {
151160
case crdmarkers.ApplyPriorityMarker:
152161
iPriority = m.ApplyPriority()
153162
case applyFirstMarker:
@@ -156,7 +165,7 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
156165
iPriority = crdmarkers.ApplyPriorityDefault
157166
}
158167

159-
switch m := markers[j].(type) {
168+
switch m := markers[j].SchemaMarker.(type) {
160169
case crdmarkers.ApplyPriorityMarker:
161170
jPriority = m.ApplyPriority()
162171
case applyFirstMarker:
@@ -171,18 +180,18 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
171180
sort.Slice(itemsMarkers, func(i, j int) bool { return cmpPriority(itemsMarkers, i, j) })
172181

173182
for _, schemaMarker := range markers {
174-
if err := schemaMarker.ApplyToSchema(props); err != nil {
183+
if err := schemaMarker.SchemaMarker.ApplyToSchema(props); err != nil {
175184
ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
176185
}
177186
}
178187

179188
for _, schemaMarker := range itemsMarkers {
180189
if props.Type != "array" || props.Items == nil || props.Items.Schema == nil {
181-
err := fmt.Errorf("must apply %s to an array value, found %s", itemsMarkerNames[schemaMarker], props.Type)
190+
err := fmt.Errorf("must apply %s to an array value, found %s", schemaMarker.Name, props.Type)
182191
ctx.pkg.AddError(loader.ErrFromNode(err, node))
183192
} else {
184193
itemsSchema := props.Items.Schema
185-
if err := schemaMarker.ApplyToSchema(itemsSchema); err != nil {
194+
if err := schemaMarker.SchemaMarker.ApplyToSchema(itemsSchema); err != nil {
186195
ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
187196
}
188197
}

pkg/crd/testdata/cronjob_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,10 @@ type CronJobSpec struct {
325325
// +listType=set
326326
Hosts []string `json:"hosts,omitempty"`
327327

328+
// This tests slice item validation with enum
329+
// +kubebuilder:validation:items:Enum=0;1;3
330+
EnumSlice []int `json:"enumSlice,omitempty"`
331+
328332
HostsAlias Hosts `json:"hostsAlias,omitempty"`
329333

330334
// This tests that alias imported from a package is handled correctly. The

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,15 @@ spec:
182182
type: object
183183
x-kubernetes-embedded-resource: true
184184
x-kubernetes-preserve-unknown-fields: true
185+
enumSlice:
186+
description: This tests slice item validation with enum
187+
type: array
188+
items:
189+
type: integer
190+
enum:
191+
- 0
192+
- 1
193+
- 3
185194
explicitlyOptionalKubebuilder:
186195
description: This tests explicitly optional kubebuilder fields
187196
type: string

0 commit comments

Comments
 (0)