Skip to content

Commit

Permalink
🐛 Fix int or string pattern application (#611)
Browse files Browse the repository at this point in the history
* Fix invalid pattern condition

* Add XIntOrString validation marker

This allows users to set the schema.XIntOrString property during the
marker validation phase. Which in turn allows other markers to rely on
the fact that the type is an IntOrString. Eg. This allows you to set a
pattern on an IntOrStr type

* Add IntOrString with pattern to integration test
  • Loading branch information
JoelSpeed committed Sep 16, 2021
1 parent 8fcb57b commit 9440165
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 2 deletions.
21 changes: 20 additions & 1 deletion pkg/crd/markers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ var ValidationMarkers = mustMakeAllWithPrefix("kubebuilder:validation", markers.
Type(""),
XPreserveUnknownFields{},
XEmbeddedResource{},
XIntOrString{},
)

// FieldOnlyMarkers list field-specific validation markers (i.e. those markers that don't make
Expand Down Expand Up @@ -232,6 +233,14 @@ type XPreserveUnknownFields struct{}
// field, yet it is possible. This can be combined with PreserveUnknownFields.
type XEmbeddedResource struct{}

// +controllertools:marker:generateHelp:category="CRD validation"
// IntOrString marks a fields as an IntOrString.
//
// This is required when applying patterns or other validations to an IntOrString
// field. Knwon information about the type is applied during the collapse phase
// and as such is not normally available during marker application.
type XIntOrString struct{}

// +controllertools:marker:generateHelp:category="CRD validation"
// Schemaless marks a field as being a schemaless object.
//
Expand Down Expand Up @@ -301,7 +310,7 @@ 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.
if schema.Type != "string" || schema.XIntOrString {
if schema.Type != "string" && !schema.XIntOrString {
return fmt.Errorf("must apply pattern to a `string` or `IntOrString`")
}
schema.Pattern = string(m)
Expand Down Expand Up @@ -409,3 +418,13 @@ func (m XEmbeddedResource) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
schema.XEmbeddedResource = true
return nil
}

// NB(JoelSpeed): we use this property in other markers here,
// which means the "XIntOrString" marker *must* be applied first.

func (m XIntOrString) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
schema.XIntOrString = true
return nil
}

func (m XIntOrString) ApplyFirst() {}
13 changes: 12 additions & 1 deletion pkg/crd/markers/zz_generated.markerhelp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions pkg/crd/testdata/cronjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Expand Down Expand Up @@ -160,6 +161,14 @@ type CronJobSpec struct {
// This tests that the schemaless marker works
// +kubebuilder:validation:Schemaless
Schemaless []byte `json:"schemaless,omitempty"`

// 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.
// 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]+)$"
IntOrStringWithAPattern *intstr.IntOrString `json:"intOrStringWithAPattern,omitempty"`
}

// +kubebuilder:validation:Type=object
Expand Down
10 changes: 10 additions & 0 deletions pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ spec:
a pointer to distinguish between explicit zero and not specified.
format: int32
type: integer
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. 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
jobTemplate:
description: Specifies the job that will be created when executing
a CronJob.
Expand Down

0 comments on commit 9440165

Please sign in to comment.