Skip to content

Commit 1d22cac

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. Signed-off-by: sivchari <shibuuuu5@gmail.com>
1 parent 77554a0 commit 1d22cac

4 files changed

Lines changed: 115 additions & 21 deletions

File tree

pkg/crd/flatten.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func flattenAllOfInto(dst *apiextensionsv1.JSONSchemaProps, src apiextensionsv1.
153153
flattenAllOfInto(dstProps.Schema, *srcProps.Schema, errRec)
154154
case "XPreserveUnknownFields":
155155
dstField.Set(srcField)
156-
case "XMapType":
156+
case "XMapType", "XListType", "XListMapKeys":
157157
dstField.Set(srcField)
158158
case "XValidations":
159159
dstField.Set(reflect.AppendSlice(srcField, dstField))

pkg/crd/schema.go

Lines changed: 35 additions & 20 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
@@ -203,7 +206,7 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api
203206
}
204207

205208
for _, schemaMarker := range itemsMarkers {
206-
if props.Type != "array" || props.Items == nil || props.Items.Schema == nil {
209+
if props.Type != arrayType || props.Items == nil || props.Items.Schema == nil {
207210
err := fmt.Errorf("must apply %s to an array value, found %s", schemaMarker.Name, props.Type)
208211
ctx.pkg.AddError(loader.ErrFromNode(err, node))
209212
} else {
@@ -314,26 +317,30 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
314317
ctx.requestSchema(pkgPath, typeNameInfo.Name())
315318
link := TypeRefLink(pkgPath, typeNameInfo.Name())
316319

317-
// In cases where we have a named type, apply the type and format from the named schema
318-
// to allow markers that need this information to apply correctly.
319-
var typ, fmt string
320-
if namedInfo, isNamed := typeInfo.(*types.Named); isNamed {
320+
props := &apiextensionsv1.JSONSchemaProps{
321+
Ref: &link,
322+
}
323+
324+
// Set the type for slice and map aliases so field-level markers like +listType
325+
// and +mapType can work. Structs are skipped — they use a $ref only.
326+
// Go 1.23+ (gotypesalias=1) represents `type Foo = []T` as *types.Alias, so
327+
// unalias first to reach the real underlying type.
328+
switch types.Unalias(typeInfo.(types.Type)).Underlying().(type) {
329+
case *types.Slice:
330+
props.Type = arrayType
331+
case *types.Map:
332+
//nolint:goconst
333+
props.Type = "object"
334+
case *types.Basic:
321335
// We don't want/need to do this for structs, maps, or arrays.
322336
// These are already handled in infoToSchema if they have custom marshalling.
323-
if _, isBasic := namedInfo.Underlying().(*types.Basic); isBasic {
324-
namedTypeInfo := ctx.schemaRequester.LookupType(ctx.pkg, namedInfo.Obj().Name())
325-
326-
namedSchema := infoToSchema(ctx.ForInfo(namedTypeInfo))
327-
typ = namedSchema.Type
328-
fmt = namedSchema.Format
329-
}
337+
namedTypeInfo := ctx.schemaRequester.LookupType(ctx.pkg, typeNameInfo.Name())
338+
namedSchema := infoToSchema(ctx.ForInfo(namedTypeInfo))
339+
props.Type = namedSchema.Type
340+
props.Format = namedSchema.Format
330341
}
331342

332-
return &apiextensionsv1.JSONSchemaProps{
333-
Type: typ,
334-
Format: fmt,
335-
Ref: &link,
336-
}
343+
return props
337344
default:
338345
ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("unsupported type %T for identifier %s", typeInfo, ident.Name), ident))
339346
return &apiextensionsv1.JSONSchemaProps{}
@@ -356,10 +363,19 @@ func namedToSchema(ctx *schemaContext, named *ast.SelectorExpr) *apiextensionsv1
356363
nonVendorPath := loader.NonVendorPath(typeNameInfo.Pkg().Path())
357364
ctx.requestSchema(nonVendorPath, typeNameInfo.Name())
358365
link := TypeRefLink(nonVendorPath, typeNameInfo.Name())
359-
return &apiextensionsv1.JSONSchemaProps{
366+
// Set the type for slice and map aliases so field-level markers like +listType
367+
// and +mapType can work. Structs are skipped — they use a $ref only.
368+
props := &apiextensionsv1.JSONSchemaProps{
360369
Ref: &link,
361370
}
371+
switch typeInfoRaw.Underlying().(type) {
372+
case *types.Slice:
373+
props.Type = arrayType
374+
case *types.Map:
375+
props.Type = "object"
376+
}
362377
// NB(directxman12): we special-case things like resource.Quantity during the "collapse" phase.
378+
return props
363379
}
364380

365381
// arrayToSchema creates a schema for the items of the given array, dealing appropriately
@@ -378,7 +394,7 @@ func arrayToSchema(ctx *schemaContext, array *ast.ArrayType) *apiextensionsv1.JS
378394
items := typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), array.Elt)
379395

380396
return &apiextensionsv1.JSONSchemaProps{
381-
Type: "array",
397+
Type: arrayType,
382398
Items: &apiextensionsv1.JSONSchemaPropsOrArray{Schema: items},
383399
}
384400
}
@@ -436,7 +452,6 @@ func mapToSchema(ctx *schemaContext, mapType *ast.MapType) *apiextensionsv1.JSON
436452
return &apiextensionsv1.JSONSchemaProps{}
437453
}
438454

439-
//nolint:goconst
440455
return &apiextensionsv1.JSONSchemaProps{
441456
Type: "object",
442457
AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{

pkg/crd/testdata/cronjob_types.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,27 @@ type CronJobSpec struct {
215215
// This tests that associative lists work via a nested type.
216216
NestedAssociativeList NestedAssociativeList `json:"nestedassociativeList"`
217217

218+
// This tests that +listType can be applied at the field level for type aliases (issue #988).
219+
// +listType=map
220+
// +listMapKey=name
221+
// +listMapKey=secondary
222+
FieldLevelListType ConditionsWithoutMarker `json:"fieldLevelListType"`
223+
224+
// This tests that a field-level +listType combined with a type-level +listType
225+
// is preserved at the top level instead of being wiped into allOf (issue #988).
226+
// +listType=map
227+
// +listMapKey=name
228+
// +listMapKey=secondary
229+
ListTypeFromTypeAndField NestedAssociativeList `json:"listTypeFromTypeAndField"`
230+
218231
// A map that allows different actors to manage different fields
219232
// +mapType=granular
220233
MapOfInfo map[string][]byte `json:"mapOfInfo"`
221234

235+
// This tests that +mapType can be applied at the field level for type aliases (issue #988).
236+
// +mapType=granular
237+
FieldLevelMapType MapWithoutMarker `json:"fieldLevelMapType"`
238+
222239
// A map that allows different actors to manage different fields via a nested type.
223240
NestedMapOfInfo NestedMapOfInfo `json:"nestedMapOfInfo"`
224241

@@ -562,6 +579,14 @@ type NestedAssociativeList []AssociativeType
562579
// +mapType=granular
563580
type NestedMapOfInfo map[string][]byte
564581

582+
// ConditionsWithoutMarker is a type alias to a slice without type-level markers.
583+
// This tests that +listType can be applied at the field level (issue #988).
584+
type ConditionsWithoutMarker []AssociativeType
585+
586+
// MapWithoutMarker is a type alias to a map without type-level markers.
587+
// This tests that +mapType can be applied at the field level (issue #988).
588+
type MapWithoutMarker map[string][]byte
589+
565590
// +kubebuilder:validation:MinLength=4
566591
// This tests that markers that are allowed on both fields and types are applied to types
567592
type LongerString string

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

Lines changed: 54 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
@@ -244,6 +265,14 @@ spec:
244265
for local type declarations.
245266
maxLength: 10
246267
type: string
268+
fieldLevelMapType:
269+
additionalProperties:
270+
format: byte
271+
type: string
272+
description: 'This tests that +mapType can be applied at the field
273+
level for type aliases (issue #988).'
274+
type: object
275+
x-kubernetes-map-type: granular
247276
float64WithValidations:
248277
maximum: 1.5
249278
minimum: -0.5
@@ -9429,6 +9458,28 @@ spec:
94299458
default: forty-two
94309459
description: This tests that primitive defaulting can be performed.
94319460
type: string
9461+
listTypeFromTypeAndField:
9462+
description: |-
9463+
This tests that a field-level +listType combined with a type-level +listType
9464+
is preserved at the top level instead of being wiped into allOf (issue #988).
9465+
items:
9466+
properties:
9467+
foo:
9468+
type: string
9469+
name:
9470+
type: string
9471+
secondary:
9472+
type: integer
9473+
required:
9474+
- foo
9475+
- name
9476+
- secondary
9477+
type: object
9478+
type: array
9479+
x-kubernetes-list-map-keys:
9480+
- name
9481+
- secondary
9482+
x-kubernetes-list-type: map
94329483
locallyBoundedInteger:
94339484
description: |-
94349485
This tests that unmarkered types are handled correctly.
@@ -9752,6 +9803,8 @@ spec:
97529803
- explicitlyRequiredK8s
97539804
- explicitlyRequiredKubebuilder
97549805
- explicitlyRequiredKubernetes
9806+
- fieldLevelListType
9807+
- fieldLevelMapType
97559808
- float64WithValidations
97569809
- floatWithValidations
97579810
- foo
@@ -9764,6 +9817,7 @@ spec:
97649817
- kubernetesDefaultedObject
97659818
- kubernetesDefaultedSlice
97669819
- kubernetesDefaultedString
9820+
- listTypeFromTypeAndField
97679821
- mapOfInfo
97689822
- nestedMapOfInfo
97699823
- nestedStructWithSeveralFields

0 commit comments

Comments
 (0)