Merge pull request #1080 from sbueringer/pr-fix-items-enum

k8s-ci-robot · web-flow · commit 5ea1855ffc82 · 2024-10-25T17:10:53.000+01:00
🐛 Fix item validation for unhashable markers
diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go
@@ -125,29 +125,38 @@ func infoToSchema(ctx *schemaContext) *apiext.JSONSchemaProps {
 	return typeToSchema(ctx, ctx.info.RawSpec.Type)
 }
 
+type schemaMarkerWithName struct {
+	SchemaMarker SchemaMarker
+	Name         string
+}
+
 // applyMarkers applies schema markers given their priority to the given schema
 func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *apiext.JSONSchemaProps, node ast.Node) {
-	markers := make([]SchemaMarker, 0, len(markerSet))
-	itemsMarkers := make([]SchemaMarker, 0, len(markerSet))
-	itemsMarkerNames := make(map[SchemaMarker]string)
+	markers := make([]schemaMarkerWithName, 0, len(markerSet))
+	itemsMarkers := make([]schemaMarkerWithName, 0, len(markerSet))
 
 	for markerName, markerValues := range markerSet {
 		for _, markerValue := range markerValues {
 			if schemaMarker, isSchemaMarker := markerValue.(SchemaMarker); isSchemaMarker {
 				if strings.HasPrefix(markerName, crdmarkers.ValidationItemsPrefix) {
-					itemsMarkers = append(itemsMarkers, schemaMarker)
-					itemsMarkerNames[schemaMarker] = markerName
+					itemsMarkers = append(itemsMarkers, schemaMarkerWithName{
+						SchemaMarker: schemaMarker,
+						Name:         markerName,
+					})
 				} else {
-					markers = append(markers, schemaMarker)
+					markers = append(markers, schemaMarkerWithName{
+						SchemaMarker: schemaMarker,
+						Name:         markerName,
+					})
 				}
 			}
 		}
 	}
 
-	cmpPriority := func(markers []SchemaMarker, i, j int) bool {
+	cmpPriority := func(markers []schemaMarkerWithName, i, j int) bool {
 		var iPriority, jPriority crdmarkers.ApplyPriority
 
-		switch m := markers[i].(type) {
+		switch m := markers[i].SchemaMarker.(type) {
 		case crdmarkers.ApplyPriorityMarker:
 			iPriority = m.ApplyPriority()
 		case applyFirstMarker:
@@ -156,7 +165,7 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
 			iPriority = crdmarkers.ApplyPriorityDefault
 		}
 
-		switch m := markers[j].(type) {
+		switch m := markers[j].SchemaMarker.(type) {
 		case crdmarkers.ApplyPriorityMarker:
 			jPriority = m.ApplyPriority()
 		case applyFirstMarker:
@@ -171,18 +180,18 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
 	sort.Slice(itemsMarkers, func(i, j int) bool { return cmpPriority(itemsMarkers, i, j) })
 
 	for _, schemaMarker := range markers {
-		if err := schemaMarker.ApplyToSchema(props); err != nil {
+		if err := schemaMarker.SchemaMarker.ApplyToSchema(props); err != nil {
 			ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
 		}
 	}
 
 	for _, schemaMarker := range itemsMarkers {
 		if props.Type != "array" || props.Items == nil || props.Items.Schema == nil {
-			err := fmt.Errorf("must apply %s to an array value, found %s", itemsMarkerNames[schemaMarker], props.Type)
+			err := fmt.Errorf("must apply %s to an array value, found %s", schemaMarker.Name, props.Type)
 			ctx.pkg.AddError(loader.ErrFromNode(err, node))
 		} else {
 			itemsSchema := props.Items.Schema
-			if err := schemaMarker.ApplyToSchema(itemsSchema); err != nil {
+			if err := schemaMarker.SchemaMarker.ApplyToSchema(itemsSchema); err != nil {
 				ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
 			}
 		}
diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go
@@ -325,6 +325,10 @@ type CronJobSpec struct {
 	// +listType=set
 	Hosts []string `json:"hosts,omitempty"`
 
+	// This tests slice item validation with enum
+	// +kubebuilder:validation:items:Enum=0;1;3
+	EnumSlice []int `json:"enumSlice,omitempty"`
+
 	HostsAlias Hosts `json:"hostsAlias,omitempty"`
 
 	// This tests that alias imported from a package is handled correctly. The
diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
@@ -182,6 +182,15 @@ spec:
                 type: object
                 x-kubernetes-embedded-resource: true
                 x-kubernetes-preserve-unknown-fields: true
+              enumSlice:
+                description: This tests slice item validation with enum
+                type: array
+                items:
+                  type: integer
+                  enum:
+                  - 0
+                  - 1
+                  - 3
               explicitlyOptionalKubebuilder:
                 description: This tests explicitly optional kubebuilder fields
                 type: string
