Skip to content

Commit 639035d

Browse files
committed
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 <shibuuuu5@gmail.com>
1 parent c0ba616 commit 639035d

3 files changed

Lines changed: 65 additions & 15 deletions

File tree

pkg/crd/schema.go

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ import (
3838
const (
3939
// defPrefix is the prefix used to link to definitions in the OpenAPI schema.
4040
defPrefix = "#/definitions/"
41+
42+
// arrayType is the JSON schema type for arrays.
43+
arrayType = "array"
4144
)
4245

4346
// byteType is the types.Type for byte (see the types documention
@@ -201,7 +204,7 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
201204
}
202205

203206
for _, schemaMarker := range itemsMarkers {
204-
if props.Type != "array" || props.Items == nil || props.Items.Schema == nil {
207+
if props.Type != arrayType || props.Items == nil || props.Items.Schema == nil {
205208
err := fmt.Errorf("must apply %s to an array value, found %s", schemaMarker.Name, props.Type)
206209
ctx.pkg.AddError(loader.ErrFromNode(err, node))
207210
} else {
@@ -306,26 +309,32 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
306309
ctx.requestSchema(pkgPath, typeNameInfo.Name())
307310
link := TypeRefLink(pkgPath, typeNameInfo.Name())
308311

312+
// For type aliases to slice/map types, include the underlying type information
313+
// so that markers like +listType can be applied at the field level.
314+
// This mirrors the behavior for basic type aliases (see above).
315+
props := &apiextensionsv1.JSONSchemaProps{
316+
Ref: &link,
317+
}
318+
309319
// In cases where we have a named type, apply the type and format from the named schema
310320
// to allow markers that need this information to apply correctly.
311-
var typ, fmt string
312321
if namedInfo, isNamed := typeInfo.(*types.Named); isNamed {
313-
// We don't want/need to do this for structs, maps, or arrays.
314-
// These are already handled in infoToSchema if they have custom marshalling.
315-
if _, isBasic := namedInfo.Underlying().(*types.Basic); isBasic {
322+
switch namedInfo.Underlying().(type) {
323+
case *types.Slice:
324+
props.Type = arrayType
325+
case *types.Map:
326+
props.Type = "object"
327+
case *types.Basic:
328+
// We don't want/need to do this for structs, maps, or arrays.
329+
// These are already handled in infoToSchema if they have custom marshalling.
316330
namedTypeInfo := ctx.schemaRequester.LookupType(ctx.pkg, namedInfo.Obj().Name())
317-
318331
namedSchema := infoToSchema(ctx.ForInfo(namedTypeInfo))
319-
typ = namedSchema.Type
320-
fmt = namedSchema.Format
332+
props.Type = namedSchema.Type
333+
props.Format = namedSchema.Format
321334
}
322335
}
323336

324-
return &apiextensionsv1.JSONSchemaProps{
325-
Type: typ,
326-
Format: fmt,
327-
Ref: &link,
328-
}
337+
return props
329338
}
330339

331340
// namedSchema creates a schema (ref) for an explicitly external type reference.
@@ -340,10 +349,19 @@ func namedToSchema(ctx *schemaContext, named *ast.SelectorExpr) *apiextensionsv1
340349
nonVendorPath := loader.NonVendorPath(typeNameInfo.Pkg().Path())
341350
ctx.requestSchema(nonVendorPath, typeNameInfo.Name())
342351
link := TypeRefLink(nonVendorPath, typeNameInfo.Name())
343-
return &apiextensionsv1.JSONSchemaProps{
352+
// For type aliases to slice/map types, include the underlying type information
353+
// so that markers like +listType can be applied at the field level.
354+
props := &apiextensionsv1.JSONSchemaProps{
344355
Ref: &link,
345356
}
357+
switch typeInfoRaw.Underlying().(type) {
358+
case *types.Slice:
359+
props.Type = arrayType
360+
case *types.Map:
361+
props.Type = "object"
362+
}
346363
// NB(directxman12): we special-case things like resource.Quantity during the "collapse" phase.
364+
return props
347365
}
348366

349367
// arrayToSchema creates a schema for the items of the given array, dealing appropriately
@@ -362,7 +380,7 @@ func arrayToSchema(ctx *schemaContext, array *ast.ArrayType) *apiextensionsv1.JS
362380
items := typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), array.Elt)
363381

364382
return &apiextensionsv1.JSONSchemaProps{
365-
Type: "array",
383+
Type: arrayType,
366384
Items: &apiextensionsv1.JSONSchemaPropsOrArray{Schema: items},
367385
}
368386
}

pkg/crd/testdata/cronjob_types.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,12 @@ type CronJobSpec struct {
207207
// This tests that associative lists work via a nested type.
208208
NestedAssociativeList NestedAssociativeList `json:"nestedassociativeList"`
209209

210+
// This tests that +listType can be applied at the field level for type aliases (issue #988).
211+
// +listType=map
212+
// +listMapKey=name
213+
// +listMapKey=secondary
214+
FieldLevelListType ConditionsWithoutMarker `json:"fieldLevelListType"`
215+
210216
// A map that allows different actors to manage different fields
211217
// +mapType=granular
212218
MapOfInfo map[string][]byte `json:"mapOfInfo"`
@@ -544,6 +550,10 @@ type NestedAssociativeList []AssociativeType
544550
// +mapType=granular
545551
type NestedMapOfInfo map[string][]byte
546552

553+
// ConditionsWithoutMarker is a type alias to a slice without type-level markers.
554+
// This tests that +listType can be applied at the field level (issue #988).
555+
type ConditionsWithoutMarker []AssociativeType
556+
547557
// +kubebuilder:validation:MinLength=4
548558
// This tests that markers that are allowed on both fields and types are applied to types
549559
type LongerString string

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,27 @@ spec:
235235
This tests that field-level overrides are handled correctly
236236
for local type aliases.
237237
type: string
238+
fieldLevelListType:
239+
description: 'This tests that +listType can be applied at the field
240+
level for type aliases (issue #988).'
241+
items:
242+
properties:
243+
foo:
244+
type: string
245+
name:
246+
type: string
247+
secondary:
248+
type: integer
249+
required:
250+
- foo
251+
- name
252+
- secondary
253+
type: object
254+
type: array
255+
x-kubernetes-list-map-keys:
256+
- name
257+
- secondary
258+
x-kubernetes-list-type: map
238259
fieldLevelLocalDeclarationOverride:
239260
allOf:
240261
- minLength: 4
@@ -9721,6 +9742,7 @@ spec:
97219742
- explicitlyRequiredK8s
97229743
- explicitlyRequiredKubebuilder
97239744
- explicitlyRequiredKubernetes
9745+
- fieldLevelListType
97249746
- float64WithValidations
97259747
- floatWithValidations
97269748
- foo

0 commit comments

Comments
 (0)