From 79a0f504e9e476c56cc5172fab8011b9ef2ef460 Mon Sep 17 00:00:00 2001
From: Stefan Bueringer <buringerst@vmware.com>
Date: Fri, 25 Oct 2024 17:43:44 +0200
Subject: [PATCH] Fix item validation for unhashable markers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stefan Büringer buringerst@vmware.com
---
 pkg/crd/schema.go                             | 33 ++++++++++++-------
 pkg/crd/testdata/cronjob_types.go             |  4 +++
 .../testdata.kubebuilder.io_cronjobs.yaml     |  9 +++++
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go
index d4bd91b41..219d0dbb6 100644
--- 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
index 44b1f699e..e81a28acb 100644
--- 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
index 913e18bee..cdc5190b2 100644
--- 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