Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 pkg/crd: support validating internal list items on list types #897

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions pkg/crd/markers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,15 @@ func isIntegral(value float64) bool {
return value == math.Trunc(value) && !math.IsNaN(value) && !math.IsInf(value, 0)
}

func thisOrItemsSchema(schema *apiext.JSONSchemaProps) *apiext.JSONSchemaProps {
if schema.Type == "array" &&
schema.Items != nil &&
schema.Items.Schema != nil {
return schema.Items.Schema
}
return schema
}

// +controllertools:marker:generateHelp:category="CRD validation"
// XValidation marks a field as requiring a value for which a given
// expression evaluates to true.
Expand Down Expand Up @@ -359,17 +368,19 @@ func (m MultipleOf) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
}

func (m MaxLength) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
schema = thisOrItemsSchema(schema)
if schema.Type != "string" {
return fmt.Errorf("must apply maxlength to a string")
return fmt.Errorf("must apply maxlength to a string or a string array")
}
val := int64(m)
schema.MaxLength = &val
return nil
}

func (m MinLength) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
schema = thisOrItemsSchema(schema)
if schema.Type != "string" {
return fmt.Errorf("must apply minlength to a string")
return fmt.Errorf("must apply minlength to a string or a string array")
}
val := int64(m)
schema.MinLength = &val
Expand All @@ -380,8 +391,9 @@ func (m Pattern) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
// Allow string types or IntOrStrings. An IntOrString will still
// apply the pattern validation when a string is detected, the pattern
// will not apply to ints though.
schema = thisOrItemsSchema(schema)
if schema.Type != "string" && !schema.XIntOrString {
return fmt.Errorf("must apply pattern to a `string` or `IntOrString`")
return fmt.Errorf("must apply pattern to a string or IntOrString or an array of them")
}
schema.Pattern = string(m)
return nil
Expand Down Expand Up @@ -510,6 +522,7 @@ func (m XEmbeddedResource) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
// which means the "XIntOrString" marker *must* be applied first.

func (m XIntOrString) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
schema = thisOrItemsSchema(schema)
schema.XIntOrString = true
return nil
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/crd/testdata/cronjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,24 @@ 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:MinLength=1
// +kubebuilder:validation:MaxLength=255
// +kubebuilder:validation:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
Hosts []string `json:"hosts,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have to find a way to differentiate if the validation is for the list as a whole or for the individual items in the list. This change just changes things to assume that its for the items. I don't think that is correct and its a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that is correct and its a breaking change

I don't think its a breaking change as previously these markers were rejected for slice types.
I also do not see a solid reason to introduce a new set of "items" markers as they will be mutually exclusive with those I change in this PR.


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 perecentage 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]+)$"
IntOrStringArrayWithAPattern []*intstr.IntOrString `json:"intOrStringArrayWithAPattern,omitempty"`
}

type ContainsNestedMap struct {
Expand Down Expand Up @@ -360,6 +378,12 @@ type LongerString string
// TotallyABool is a bool that serializes as a string.
type TotallyABool bool

// This tests string array item validation.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=255
// +kubebuilder:validation:Pattern=^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type Hosts []string

func (t TotallyABool) MarshalJSON() ([]byte, error) {
if t {
return []byte(`"true"`), nil
Expand Down
36 changes: 36 additions & 0 deletions pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,40 @@ 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
type: array
hostsAlias:
description: This tests string array 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
type: array
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 perecentage 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
Expand Down Expand Up @@ -6609,6 +6637,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:
Expand Down