Skip to content

Commit

Permalink
Merge pull request #938 from liggitt/default
Browse files Browse the repository at this point in the history
✨ Add support for +default markers
  • Loading branch information
k8s-ci-robot authored May 15, 2024
2 parents ed2fbe2 + 5e5bc88 commit 85686cb
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 5 deletions.
49 changes: 49 additions & 0 deletions pkg/crd/markers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ var FieldOnlyMarkers = []*definitionWithHelp{

must(markers.MakeAnyTypeDefinition("kubebuilder:default", markers.DescribesField, Default{})).
WithHelp(Default{}.Help()),
must(markers.MakeDefinition("default", markers.DescribesField, KubernetesDefault{})).
WithHelp(KubernetesDefault{}.Help()),

must(markers.MakeAnyTypeDefinition("kubebuilder:example", markers.DescribesField, Example{})).
WithHelp(Example{}.Help()),
Expand Down Expand Up @@ -241,6 +243,20 @@ type Default struct {
Value interface{}
}

// +controllertools:marker:generateHelp:category="CRD validation"
// Default sets the default value for this field.
//
// A default value will be accepted as any value valid for the field.
// Only JSON-formatted values are accepted. `ref(...)` values are ignored.
// Formatting for common types include: boolean: `true`, string:
// `"Cluster"`, numerical: `1.24`, array: `[1,2]`, object: `{"policy":
// "delete"}`). Defaults should be defined in pruned form, and only best-effort
// validation will be performed. Full validation of a default requires
// submission of the containing CRD to an apiserver.
type KubernetesDefault struct {
Value interface{}
}

// +controllertools:marker:generateHelp:category="CRD validation"
// Example sets the example value for this field.
//
Expand Down Expand Up @@ -505,6 +521,39 @@ func (m Default) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
return nil
}

func (m Default) ApplyPriority() ApplyPriority {
// explicitly go after +default markers, so kubebuilder-specific defaults get applied last and stomp
return 10
}

func (m *KubernetesDefault) ParseMarker(_ string, _ string, restFields string) error {
if strings.HasPrefix(strings.TrimSpace(restFields), "ref(") {
// Skip +default=ref(...) values for now, since we don't have a good way to evaluate go constant values via AST.
// See https://github.com/kubernetes-sigs/controller-tools/pull/938#issuecomment-2096790018
return nil
}
return json.Unmarshal([]byte(restFields), &m.Value)
}

// Defaults are only valid CRDs created with the v1 API
func (m KubernetesDefault) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
if m.Value == nil {
// only apply to the schema if we have a non-nil default value
return nil
}
marshalledDefault, err := json.Marshal(m.Value)
if err != nil {
return err
}
schema.Default = &apiext.JSON{Raw: marshalledDefault}
return nil
}

func (m KubernetesDefault) ApplyPriority() ApplyPriority {
// explicitly go before +kubebuilder:default markers, so kubebuilder-specific defaults get applied last and stomp
return 9
}

func (m Example) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
marshalledExample, err := json.Marshal(m.Value)
if err != nil {
Expand Down
16 changes: 16 additions & 0 deletions pkg/crd/markers/zz_generated.markerhelp.go

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

41 changes: 38 additions & 3 deletions pkg/crd/testdata/cronjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import (
// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.

const DefaultRefValue = "defaultRefValue"

// CronJobSpec defines the desired state of CronJob
// +kubebuilder:validation:XValidation:rule="has(oldSelf.forbiddenInt) || !has(self.forbiddenInt)",message="forbiddenInt is not allowed",fieldPath=".forbiddenInt",reason="FieldValueForbidden"
type CronJobSpec struct {
Expand Down Expand Up @@ -116,9 +118,9 @@ type CronJobSpec struct {
// +kubebuilder:example={a,b}
DefaultedSlice []string `json:"defaultedSlice"`

// This tests that object defaulting can be performed.
// +kubebuilder:default={{nested: {foo: "baz", bar: true}},{nested: {bar: false}}}
// +kubebuilder:example={{nested: {foo: "baz", bar: true}},{nested: {bar: false}}}
// This tests that slice and object defaulting can be performed.
// +kubebuilder:default={{nested: {foo: "baz", bar: true}},{nested: {foo: "qux", bar: false}}}
// +kubebuilder:example={{nested: {foo: "baz", bar: true}},{nested: {foo: "qux", bar: false}}}
DefaultedObject []RootObject `json:"defaultedObject"`

// This tests that empty slice defaulting can be performed.
Expand All @@ -133,6 +135,39 @@ type CronJobSpec struct {
// +kubebuilder:default={}
DefaultedEmptyObject EmpiableObject `json:"defaultedEmptyObject"`

// This tests that kubebuilder defaulting takes precedence.
// +kubebuilder:default="kubebuilder-default"
// +default="kubernetes-default"
DoubleDefaultedString string `json:"doubleDefaultedString"`

// This tests that primitive defaulting can be performed.
// +default="forty-two"
KubernetesDefaultedString string `json:"kubernetesDefaultedString"`

// This tests that slice defaulting can be performed.
// +default=["a","b"]
KubernetesDefaultedSlice []string `json:"kubernetesDefaultedSlice"`

// This tests that slice and object defaulting can be performed.
// +default=[{"nested": {"foo": "baz", "bar": true}},{"nested": {"foo": "qux", "bar": false}}]
KubernetesDefaultedObject []RootObject `json:"kubernetesDefaultedObject"`

// This tests that empty slice defaulting can be performed.
// +default=[]
KubernetesDefaultedEmptySlice []string `json:"kubernetesDefaultedEmptySlice"`

// This tests that an empty object defaulting can be performed on a map.
// +default={}
KubernetesDefaultedEmptyMap map[string]string `json:"kubernetesDefaultedEmptyMap"`

// This tests that an empty object defaulting can be performed on an object.
// +default={}
KubernetesDefaultedEmptyObject EmpiableObject `json:"kubernetesDefaultedEmptyObject"`

// This tests that use of +default=ref(...) doesn't break generation
// +default=ref(DefaultRefValue)
KubernetesDefaultedRef string `json:"kubernetesDefaultedRef,omitempty"`

// This tests that pattern validator is properly applied.
// +kubebuilder:validation:Pattern=`^$|^((https):\/\/?)[^\s()<>]+(?:\([\w\d]+\)|([^[:punct:]\s]|\/?))$`
PatternObject string `json:"patternObject"`
Expand Down
79 changes: 78 additions & 1 deletion pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,15 @@ spec:
foo: baz
- nested:
bar: false
description: This tests that object defaulting can be performed.
foo: qux
description: This tests that slice and object defaulting can be performed.
example:
- nested:
bar: true
foo: baz
- nested:
bar: false
foo: qux
items:
properties:
nested:
Expand Down Expand Up @@ -184,6 +186,74 @@ spec:
explicitlyRequiredKubernetes:
description: This tests explicitly required kubernetes fields
type: string
doubleDefaultedString:
default: kubebuilder-default
description: This tests that kubebuilder defaulting takes precedence.
type: string
kubernetesDefaultedEmptyMap:
additionalProperties:
type: string
default: {}
description: This tests that an empty object defaulting can be performed
on a map.
type: object
kubernetesDefaultedEmptyObject:
default: {}
description: This tests that an empty object defaulting can be performed
on an object.
properties:
bar:
type: string
foo:
default: forty-two
type: string
type: object
kubernetesDefaultedEmptySlice:
default: []
description: This tests that empty slice defaulting can be performed.
items:
type: string
type: array
kubernetesDefaultedObject:
default:
- nested:
bar: true
foo: baz
- nested:
bar: false
foo: qux
description: This tests that slice and object defaulting can be performed.
items:
properties:
nested:
properties:
bar:
type: boolean
foo:
type: string
required:
- bar
- foo
type: object
required:
- nested
type: object
type: array
kubernetesDefaultedSlice:
default:
- a
- b
description: This tests that slice defaulting can be performed.
items:
type: string
type: array
kubernetesDefaultedString:
default: forty-two
description: This tests that primitive defaulting can be performed.
type: string
kubernetesDefaultedRef:
description: This tests that use of +default=ref(...) doesn't break generation
type: string
embeddedResource:
type: object
x-kubernetes-embedded-resource: true
Expand Down Expand Up @@ -6898,6 +6968,7 @@ spec:
- defaultedObject
- defaultedSlice
- defaultedString
- doubleDefaultedString
- embeddedResource
- explicitlyRequiredKubebuilder
- explicitlyRequiredKubernetes
Expand All @@ -6907,6 +6978,12 @@ spec:
- int32WithValidations
- intWithValidations
- jobTemplate
- kubernetesDefaultedEmptyMap
- kubernetesDefaultedEmptyObject
- kubernetesDefaultedEmptySlice
- kubernetesDefaultedObject
- kubernetesDefaultedSlice
- kubernetesDefaultedString
- mapOfInfo
- nestedMapOfInfo
- nestedStructWithSeveralFields
Expand Down
12 changes: 11 additions & 1 deletion pkg/markers/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,13 +820,23 @@ func parserScanner(raw string, err func(*sc.Scanner, string)) *sc.Scanner {
return scanner
}

type markerParser interface {
ParseMarker(name string, anonymousName string, restFields string) error
}

// Parse uses the type information in this Definition to parse the given
// raw marker in the form `+a:b:c=arg,d=arg` into an output object of the
// type specified in the definition.
func (d *Definition) Parse(rawMarker string) (interface{}, error) {
name, anonName, fields := splitMarker(rawMarker)

out := reflect.Indirect(reflect.New(d.Output))
outPointer := reflect.New(d.Output)
out := reflect.Indirect(outPointer)

if parser, ok := outPointer.Interface().(markerParser); ok {
err := parser.ParseMarker(name, anonName, fields)
return out.Interface(), err
}

// if we're a not a struct or have no arguments, treat the full `a:b:c` as the name,
// otherwise, treat `c` as a field name, and `a:b` as the marker name.
Expand Down

0 comments on commit 85686cb

Please sign in to comment.