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

✨ Add support for +default markers #938

Merged
merged 1 commit into from
May 15, 2024
Merged
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
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"`
liggitt marked this conversation as resolved.
Show resolved Hide resolved

// 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.
liggitt marked this conversation as resolved.
Show resolved Hide resolved
// +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