diff --git a/pkg/crd/markers/register.go b/pkg/crd/markers/register.go index 0e7c42694..7bd7d29d4 100644 --- a/pkg/crd/markers/register.go +++ b/pkg/crd/markers/register.go @@ -42,6 +42,15 @@ func (d *definitionWithHelp) Register(reg *markers.Registry) error { return nil } +func (d *definitionWithHelp) clone() *definitionWithHelp { + newDef := *d.Definition + // copy both parts so we don't change the definition + return &definitionWithHelp{ + Definition: &newDef, + Help: d.Help, + } +} + func must(def *markers.Definition, err error) *definitionWithHelp { return &definitionWithHelp{ Definition: markers.Must(def, err), @@ -60,7 +69,7 @@ type hasHelp interface { func mustMakeAllWithPrefix(prefix string, target markers.TargetType, objs ...interface{}) []*definitionWithHelp { defs := make([]*definitionWithHelp, len(objs)) for i, obj := range objs { - name := prefix + ":" + reflect.TypeOf(obj).Name() + name := prefix + reflect.TypeOf(obj).Name() def, err := markers.MakeDefinition(name, target, obj) if err != nil { panic(err) diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go index 48c89e6d0..911888689 100644 --- a/pkg/crd/markers/validation.go +++ b/pkg/crd/markers/validation.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "math" + "strings" apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -27,7 +28,10 @@ import ( ) const ( - SchemalessName = "kubebuilder:validation:Schemaless" + validationPrefix = "kubebuilder:validation:" + + SchemalessName = "kubebuilder:validation:Schemaless" + ValidationItemsPrefix = validationPrefix + "items:" ) // ValidationMarkers lists all available markers that affect CRD schema generation, @@ -35,7 +39,9 @@ const ( // All markers start with `+kubebuilder:validation:`, and continue with their type name. // A copy is produced of all markers that describes types as well, for making types // reusable and writing complex validations on slice items. -var ValidationMarkers = mustMakeAllWithPrefix("kubebuilder:validation", markers.DescribesField, +// At last a copy of all markers with the prefix `+kubebuilder:validation:items:` is +// produced for marking slice fields and types. +var ValidationMarkers = mustMakeAllWithPrefix(validationPrefix, markers.DescribesField, // numeric markers @@ -110,14 +116,20 @@ func init() { AllDefinitions = append(AllDefinitions, ValidationMarkers...) for _, def := range ValidationMarkers { - newDef := *def.Definition - // copy both parts so we don't change the definition - typDef := definitionWithHelp{ - Definition: &newDef, - Help: def.Help, - } + typDef := def.clone() typDef.Target = markers.DescribesType - AllDefinitions = append(AllDefinitions, &typDef) + AllDefinitions = append(AllDefinitions, typDef) + + itemsName := ValidationItemsPrefix + strings.TrimPrefix(def.Name, validationPrefix) + + itemsFieldDef := def.clone() + itemsFieldDef.Name = itemsName + AllDefinitions = append(AllDefinitions, itemsFieldDef) + + itemsTypDef := def.clone() + itemsTypDef.Name = itemsName + itemsTypDef.Target = markers.DescribesType + AllDefinitions = append(AllDefinitions, itemsTypDef) } AllDefinitions = append(AllDefinitions, FieldOnlyMarkers...) diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index cc2adc758..a6e3cd601 100644 --- a/pkg/crd/schema.go +++ b/pkg/crd/schema.go @@ -128,16 +128,23 @@ func infoToSchema(ctx *schemaContext) *apiext.JSONSchemaProps { // 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) - for _, markerValues := range markerSet { + for markerName, markerValues := range markerSet { for _, markerValue := range markerValues { if schemaMarker, isSchemaMarker := markerValue.(SchemaMarker); isSchemaMarker { - markers = append(markers, schemaMarker) + if strings.HasPrefix(markerName, crdmarkers.ValidationItemsPrefix) { + itemsMarkers = append(itemsMarkers, schemaMarker) + itemsMarkerNames[schemaMarker] = markerName + } else { + markers = append(markers, schemaMarker) + } } } } - sort.Slice(markers, func(i, j int) bool { + cmpPriority := func(markers []SchemaMarker, i, j int) bool { var iPriority, jPriority crdmarkers.ApplyPriority switch m := markers[i].(type) { @@ -159,13 +166,27 @@ func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *api } return iPriority < jPriority - }) + } + sort.Slice(markers, func(i, j int) bool { return cmpPriority(markers, i, j) }) + sort.Slice(itemsMarkers, func(i, j int) bool { return cmpPriority(itemsMarkers, i, j) }) for _, schemaMarker := range markers { if err := 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) + ctx.pkg.AddError(loader.ErrFromNode(err, node)) + } else { + itemsSchema := props.Items.Schema + if err := schemaMarker.ApplyToSchema(itemsSchema); err != nil { + ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node)) + } + } + } } // typeToSchema creates a schema for the given AST type. diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index d36beeab2..43aed8a86 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -197,7 +197,7 @@ type CronJobSpec struct { // This tests that an IntOrString can also have a pattern attached // to it. - // This can be useful if you want to limit the string to a perecentage or integer. + // This can be useful if you want to limit the string to a percentage or integer. // The XIntOrString marker is a requirement for having a pattern on this type. // +kubebuilder:validation:XIntOrString // +kubebuilder:validation:Pattern="^((100|[0-9]{1,2})%|[0-9]+)$" @@ -252,6 +252,26 @@ type CronJobSpec struct { // Checks that arrays work when the type contains a composite literal ArrayUsingCompositeLiteral [len(struct{ X [3]int }{}.X)]string `json:"arrayUsingCompositeLiteral,omitempty"` + + // This tests string slice item validation. + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:items:MinLength=1 + // +kubebuilder:validation:items:MaxLength=255 + // +kubebuilder:validation:items:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + // +listType=set + Hosts []string `json:"hosts,omitempty"` + + HostsAlias Hosts `json:"hostsAlias,omitempty"` + + // This tests string alias slice item validation. + LongerStringArray []LongerString `json:"longerStringArray,omitempty"` + + // This tests that a slice of IntOrString can also have a pattern attached to it. + // This can be useful if you want to limit the string to a percentage or integer. + // The XIntOrString marker is a requirement for having a pattern on this type. + // +kubebuilder:validation:items:XIntOrString + // +kubebuilder:validation:items:Pattern="^((100|[0-9]{1,2})%|[0-9]+)$" + IntOrStringArrayWithAPattern []*intstr.IntOrString `json:"intOrStringArrayWithAPattern,omitempty"` } type ContainsNestedMap struct { @@ -360,6 +380,14 @@ type LongerString string // TotallyABool is a bool that serializes as a string. type TotallyABool bool +// This tests string slice item validation. +// +kubebuilder:validation:MinItems=1 +// +kubebuilder:validation:items:MinLength=1 +// +kubebuilder:validation:items:MaxLength=255 +// +kubebuilder:validation:items:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ +// +listType=set +type Hosts []string + func (t TotallyABool) MarshalJSON() ([]byte, error) { if t { return []byte(`"true"`), nil diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index 83d11978b..15567a387 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -196,12 +196,44 @@ spec: description: This tests that exported fields are not skipped in the schema generation type: string + hosts: + description: This tests string slice item validation. + items: + maxLength: 255 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + minItems: 1 + type: array + x-kubernetes-list-type: set + hostsAlias: + description: This tests string slice item validation. + items: + maxLength: 255 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + minItems: 1 + type: array + x-kubernetes-list-type: set int32WithValidations: format: int32 maximum: 2 minimum: -2 multipleOf: 2 type: integer + intOrStringArrayWithAPattern: + description: |- + This tests that a slice of IntOrString can also have a pattern attached to it. + This can be useful if you want to limit the string to a percentage or integer. + The XIntOrString marker is a requirement for having a pattern on this type. + items: + anyOf: + - type: integer + - type: string + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + type: array intOrStringWithAPattern: anyOf: - type: integer @@ -209,7 +241,7 @@ spec: description: |- This tests that an IntOrString can also have a pattern attached to it. - This can be useful if you want to limit the string to a perecentage or integer. + This can be useful if you want to limit the string to a percentage or integer. The XIntOrString marker is a requirement for having a pattern on this type. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true @@ -6609,6 +6641,14 @@ spec: - bar - foo type: object + longerStringArray: + description: This tests string alias slice item validation. + items: + description: This tests that markers that are allowed on both fields + and types are applied to types + minLength: 4 + type: string + type: array mapOfArraysOfFloats: additionalProperties: items: