From 5e5bc88a01cab8ebff8c9fc4d1e90bb858139186 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 7 May 2024 16:05:36 -0400 Subject: [PATCH] :sparkles: Add support for +default markers https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1929-built-in-default#summary https://github.com/kubernetes-sigs/kubebuilder/issues/3896 --- pkg/crd/markers/validation.go | 49 ++++++++++++ pkg/crd/markers/zz_generated.markerhelp.go | 16 ++++ pkg/crd/testdata/cronjob_types.go | 41 +++++++++- .../testdata.kubebuilder.io_cronjobs.yaml | 79 ++++++++++++++++++- pkg/markers/parse.go | 12 ++- 5 files changed, 192 insertions(+), 5 deletions(-) diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go index 2e50c18c9..7ed2c97b8 100644 --- a/pkg/crd/markers/validation.go +++ b/pkg/crd/markers/validation.go @@ -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()), @@ -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. // @@ -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 { diff --git a/pkg/crd/markers/zz_generated.markerhelp.go b/pkg/crd/markers/zz_generated.markerhelp.go index 73432ebd4..7fb1b2164 100644 --- a/pkg/crd/markers/zz_generated.markerhelp.go +++ b/pkg/crd/markers/zz_generated.markerhelp.go @@ -116,6 +116,22 @@ func (Format) Help() *markers.DefinitionHelp { } } +func (KubernetesDefault) Help() *markers.DefinitionHelp { + return &markers.DefinitionHelp{ + Category: "CRD validation", + DetailedHelp: markers.DetailedHelp{ + Summary: "Default sets the default value for this field.", + Details: "A default value will be accepted as any value valid for the field.\nOnly JSON-formatted values are accepted. `ref(...)` values are ignored.\nFormatting for common types include: boolean: `true`, string:\n`\"Cluster\"`, numerical: `1.24`, array: `[1,2]`, object: `{\"policy\":\n\"delete\"}`). Defaults should be defined in pruned form, and only best-effort\nvalidation will be performed. Full validation of a default requires\nsubmission of the containing CRD to an apiserver.", + }, + FieldHelp: map[string]markers.DetailedHelp{ + "Value": { + Summary: "", + Details: "", + }, + }, + } +} + func (ListMapKey) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD processing", diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index 6f608f178..50cd689b6 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -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 { @@ -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. @@ -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"` diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index e225694e5..7abf69da7 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -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: @@ -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 @@ -6898,6 +6968,7 @@ spec: - defaultedObject - defaultedSlice - defaultedString + - doubleDefaultedString - embeddedResource - explicitlyRequiredKubebuilder - explicitlyRequiredKubernetes @@ -6907,6 +6978,12 @@ spec: - int32WithValidations - intWithValidations - jobTemplate + - kubernetesDefaultedEmptyMap + - kubernetesDefaultedEmptyObject + - kubernetesDefaultedEmptySlice + - kubernetesDefaultedObject + - kubernetesDefaultedSlice + - kubernetesDefaultedString - mapOfInfo - nestedMapOfInfo - nestedStructWithSeveralFields diff --git a/pkg/markers/parse.go b/pkg/markers/parse.go index 259bff027..01e8950fb 100644 --- a/pkg/markers/parse.go +++ b/pkg/markers/parse.go @@ -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.