Skip to content

Commit

Permalink
pkg/crd: support validating internal list items on list types
Browse files Browse the repository at this point in the history
For kubernetes-sigs#342

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
  • Loading branch information
AlexanderYastrebov committed Mar 28, 2024
1 parent 3f5bd8e commit 4d35a95
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 16 deletions.
11 changes: 10 additions & 1 deletion pkg/crd/markers/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand Down
30 changes: 21 additions & 9 deletions pkg/crd/markers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,28 @@ import (
"encoding/json"
"fmt"
"math"
"strings"

apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"

"sigs.k8s.io/controller-tools/pkg/markers"
)

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,
// except for the few that don't make sense as type-level markers (see FieldOnlyMarkers).
// 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

Expand Down Expand Up @@ -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...)
Expand Down
29 changes: 25 additions & 4 deletions pkg/crd/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand Down
36 changes: 35 additions & 1 deletion pkg/crd/testdata/cronjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]+)$"
Expand Down Expand Up @@ -252,6 +252,32 @@ 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 slice validation.
// +kubebuilder:validation:MinItems=2
// +kubebuilder:validation:MaxItems=2
StringPair []string `json:"stringPair"`

// This tests string alias slice item validation.
// +kubebuilder:validation:MinItems=3
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 {
Expand Down Expand Up @@ -360,6 +386,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
Expand Down
51 changes: 50 additions & 1 deletion pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -196,20 +196,52 @@ 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
- type: string
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
Expand Down Expand Up @@ -6609,6 +6641,15 @@ 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
minItems: 3
type: array
mapOfArraysOfFloats:
additionalProperties:
items:
Expand Down Expand Up @@ -6742,6 +6783,13 @@ spec:
time for any reason. Missed jobs executions will be counted as failed ones.
format: int64
type: integer
stringPair:
description: This tests string slice validation.
items:
type: string
maxItems: 2
minItems: 2
type: array
stringSliceData:
additionalProperties:
items:
Expand Down Expand Up @@ -6839,6 +6887,7 @@ spec:
- nestedassociativeList
- patternObject
- schedule
- stringPair
- structWithSeveralFields
- twoOfAKindPart0
- twoOfAKindPart1
Expand Down

0 comments on commit 4d35a95

Please sign in to comment.