Skip to content

Commit a39b281

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
1 parent c0ba616 commit a39b281

3 files changed

Lines changed: 60 additions & 13 deletions

File tree

pkg/crd/schema.go

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -306,26 +306,32 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
306306
ctx.requestSchema(pkgPath, typeNameInfo.Name())
307307
link := TypeRefLink(pkgPath, typeNameInfo.Name())
308308

309+
// For type aliases to slice/map types, include the underlying type information
310+
// so that markers like +listType can be applied at the field level.
311+
// This mirrors the behavior for basic type aliases (see above).
312+
props := &apiextensionsv1.JSONSchemaProps{
313+
Ref: &link,
314+
}
315+
309316
// In cases where we have a named type, apply the type and format from the named schema
310317
// to allow markers that need this information to apply correctly.
311-
var typ, fmt string
312318
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 {
319+
switch namedInfo.Underlying().(type) {
320+
case *types.Slice:
321+
props.Type = "array"
322+
case *types.Map:
323+
props.Type = "object"
324+
case *types.Basic:
325+
// We don't want/need to do this for structs, maps, or arrays.
326+
// These are already handled in infoToSchema if they have custom marshalling.
316327
namedTypeInfo := ctx.schemaRequester.LookupType(ctx.pkg, namedInfo.Obj().Name())
317-
318328
namedSchema := infoToSchema(ctx.ForInfo(namedTypeInfo))
319-
typ = namedSchema.Type
320-
fmt = namedSchema.Format
329+
props.Type = namedSchema.Type
330+
props.Format = namedSchema.Format
321331
}
322332
}
323333

324-
return &apiextensionsv1.JSONSchemaProps{
325-
Type: typ,
326-
Format: fmt,
327-
Ref: &link,
328-
}
334+
return props
329335
}
330336

331337
// namedSchema creates a schema (ref) for an explicitly external type reference.
@@ -340,10 +346,19 @@ func namedToSchema(ctx *schemaContext, named *ast.SelectorExpr) *apiextensionsv1
340346
nonVendorPath := loader.NonVendorPath(typeNameInfo.Pkg().Path())
341347
ctx.requestSchema(nonVendorPath, typeNameInfo.Name())
342348
link := TypeRefLink(nonVendorPath, typeNameInfo.Name())
343-
return &apiextensionsv1.JSONSchemaProps{
349+
// For type aliases to slice/map types, include the underlying type information
350+
// so that markers like +listType can be applied at the field level.
351+
props := &apiextensionsv1.JSONSchemaProps{
344352
Ref: &link,
345353
}
354+
switch typeInfoRaw.Underlying().(type) {
355+
case *types.Slice:
356+
props.Type = "array"
357+
case *types.Map:
358+
props.Type = "object"
359+
}
346360
// NB(directxman12): we special-case things like resource.Quantity during the "collapse" phase.
361+
return props
347362
}
348363

349364
// arrayToSchema creates a schema for the items of the given array, dealing appropriately

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)