Fix +listType marker not working on type alias fields

Include underlying type info (array/object) in schema for slice/map
type aliases, allowing field-level markers like +listType to work.

Fixes #988

Signed-off-by: sivchari <[email protected]>

Author: sivchari

pkg/crd/schema.go

@@ -38,6 +38,9 @@ import (
 const (
 	// defPrefix is the prefix used to link to definitions in the OpenAPI schema.
 	defPrefix = "#/definitions/"
+
+	// arrayType is the JSON schema type for arrays.
+	arrayType = "array"
 )
 
 // byteType is the types.Type for byte (see the types documention
@@ -201,7 +204,7 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
 	}
 
 	for _, schemaMarker := range itemsMarkers {
-		if props.Type != "array" || props.Items == nil || props.Items.Schema == nil {
+		if props.Type != arrayType || props.Items == nil || props.Items.Schema == nil {
 			err := fmt.Errorf("must apply %s to an array value, found %s", schemaMarker.Name, props.Type)
 			ctx.pkg.AddError(loader.ErrFromNode(err, node))
 		} else {
@@ -312,26 +315,31 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
 		ctx.requestSchema(pkgPath, typeNameInfo.Name())
 		link := TypeRefLink(pkgPath, typeNameInfo.Name())
 
+		// For type aliases to slice/map types, include the underlying type information
+		// so that markers like +listType can be applied at the field level.
+		props := &apiextensionsv1.JSONSchemaProps{
+			Ref: &link,
+		}
+
 		// In cases where we have a named type, apply the type and format from the named schema
 		// to allow markers that need this information to apply correctly.
-		var typ, fmt string
 		if namedInfo, isNamed := typeInfo.(*types.Named); isNamed {
-			// We don't want/need to do this for structs, maps, or arrays.
-			// These are already handled in infoToSchema if they have custom marshalling.
-			if _, isBasic := namedInfo.Underlying().(*types.Basic); isBasic {
+			switch namedInfo.Underlying().(type) {
+			case *types.Slice:
+				props.Type = arrayType
+			case *types.Map:
+				props.Type = "object"
+			case *types.Basic:
+				// We don't want/need to do this for structs, maps, or arrays.
+				// These are already handled in infoToSchema if they have custom marshalling.
 				namedTypeInfo := ctx.schemaRequester.LookupType(ctx.pkg, namedInfo.Obj().Name())
-
 				namedSchema := infoToSchema(ctx.ForInfo(namedTypeInfo))
-				typ = namedSchema.Type
-				fmt = namedSchema.Format
+				props.Type = namedSchema.Type
+				props.Format = namedSchema.Format
 			}
 		}
 
-		return &apiextensionsv1.JSONSchemaProps{
-			Type:   typ,
-			Format: fmt,
-			Ref:    &link,
-		}
+		return props
 	default:
 		ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("unsupported type %T for identifier %s", typeInfo, ident.Name), ident))
 		return &apiextensionsv1.JSONSchemaProps{}
@@ -354,10 +362,19 @@ func namedToSchema(ctx *schemaContext, named *ast.SelectorExpr) *apiextensionsv1
 	nonVendorPath := loader.NonVendorPath(typeNameInfo.Pkg().Path())
 	ctx.requestSchema(nonVendorPath, typeNameInfo.Name())
 	link := TypeRefLink(nonVendorPath, typeNameInfo.Name())
-	return &apiextensionsv1.JSONSchemaProps{
+	// For type aliases to slice/map types, include the underlying type information
+	// so that markers like +listType can be applied at the field level.
+	props := &apiextensionsv1.JSONSchemaProps{
 		Ref: &link,
 	}
+	switch typeInfoRaw.Underlying().(type) {
+	case *types.Slice:
+		props.Type = arrayType
+	case *types.Map:
+		props.Type = "object"
+	}
 	// NB(directxman12): we special-case things like resource.Quantity during the "collapse" phase.
+	return props
 }
 
 // arrayToSchema creates a schema for the items of the given array, dealing appropriately
@@ -376,7 +393,7 @@ func arrayToSchema(ctx *schemaContext, array *ast.ArrayType) *apiextensionsv1.JS
 	items := typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), array.Elt)
 
 	return &apiextensionsv1.JSONSchemaProps{
-		Type:  "array",
+		Type:  arrayType,
 		Items: &apiextensionsv1.JSONSchemaPropsOrArray{Schema: items},
 	}
 }

pkg/crd/testdata/cronjob_types.go

@@ -207,6 +207,12 @@ type CronJobSpec struct {
 	// This tests that associative lists work via a nested type.
 	NestedAssociativeList NestedAssociativeList `json:"nestedassociativeList"`
 
+	// This tests that +listType can be applied at the field level for type aliases (issue #988).
+	// +listType=map
+	// +listMapKey=name
+	// +listMapKey=secondary
+	FieldLevelListType ConditionsWithoutMarker `json:"fieldLevelListType"`
+
 	// A map that allows different actors to manage different fields
 	// +mapType=granular
 	MapOfInfo map[string][]byte `json:"mapOfInfo"`
@@ -544,6 +550,10 @@ type NestedAssociativeList []AssociativeType
 // +mapType=granular
 type NestedMapOfInfo map[string][]byte
 
+// ConditionsWithoutMarker is a type alias to a slice without type-level markers.
+// This tests that +listType can be applied at the field level (issue #988).
+type ConditionsWithoutMarker []AssociativeType
+
 // +kubebuilder:validation:MinLength=4
 // This tests that markers that are allowed on both fields and types are applied to types
 type LongerString string

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

@@ -235,6 +235,27 @@ spec:
                   This tests that field-level overrides are handled correctly
                   for local type aliases.
                 type: string
+              fieldLevelListType:
+                description: 'This tests that +listType can be applied at the field
+                  level for type aliases (issue #988).'
+                items:
+                  properties:
+                    foo:
+                      type: string
+                    name:
+                      type: string
+                    secondary:
+                      type: integer
+                  required:
+                  - foo
+                  - name
+                  - secondary
+                  type: object
+                type: array
+                x-kubernetes-list-map-keys:
+                - name
+                - secondary
+                x-kubernetes-list-type: map
               fieldLevelLocalDeclarationOverride:
                 allOf:
                 - minLength: 4
@@ -9721,6 +9742,7 @@ spec:
             - explicitlyRequiredK8s
             - explicitlyRequiredKubebuilder
             - explicitlyRequiredKubernetes
+            - fieldLevelListType
             - float64WithValidations
             - floatWithValidations
             - foo