From 8093c7e7154309000fd6fe67e3cf26c9c7d69b98 Mon Sep 17 00:00:00 2001 From: Lalit Chauhan Date: Mon, 28 Jul 2025 23:15:33 +0000 Subject: [PATCH] feat(crd): Add k8s:required and k8s:optional markers This commit introduces `k8s:required` and `k8s:optional` markers to align with the declarative validation KEP. These markers serve as a Kubernetes-style alternative to the existing `+required` and `+optional` markers for marking CRD fields. This change allows controller-gen to better understand native types in CRDs, improving the declarative validation process. Reference: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md --- pkg/crd/markers/validation.go | 5 ++++- pkg/crd/markers/zz_generated.markerhelp.go | 19 +++++++++++++++---- pkg/crd/schema.go | 4 ++-- pkg/crd/testdata/cronjob_types.go | 8 ++++++++ .../testdata.kubebuilder.io_cronjobs.yaml | 7 +++++++ 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go index cb7e73769..c91f5e6a1 100644 --- a/pkg/crd/markers/validation.go +++ b/pkg/crd/markers/validation.go @@ -103,7 +103,10 @@ var FieldOnlyMarkers = []*definitionWithHelp{ WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is required.")), must(markers.MakeDefinition("optional", markers.DescribesField, struct{}{})). WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is optional.")), - + must(markers.MakeDefinition("k8s:required", markers.DescribesField, struct{}{})). + WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is required.")), + must(markers.MakeDefinition("k8s:optional", markers.DescribesField, struct{}{})). + WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is optional.")), must(markers.MakeDefinition("nullable", markers.DescribesField, Nullable{})). WithHelp(Nullable{}.Help()), diff --git a/pkg/crd/markers/zz_generated.markerhelp.go b/pkg/crd/markers/zz_generated.markerhelp.go index 1f336df9c..bf650ab23 100644 --- a/pkg/crd/markers/zz_generated.markerhelp.go +++ b/pkg/crd/markers/zz_generated.markerhelp.go @@ -24,6 +24,17 @@ import ( "sigs.k8s.io/controller-tools/pkg/markers" ) +func (AtLeastOneOf) Help() *markers.DefinitionHelp { + return &markers.DefinitionHelp{ + Category: "CRD validation", + DetailedHelp: markers.DetailedHelp{ + Summary: "adds a validation constraint that allows at least one of the specified fields.", + Details: "This marker may be repeated to specify multiple AtLeastOneOf constraints that are mutually exclusive.", + }, + FieldHelp: map[string]markers.DetailedHelp{}, + } +} + func (AtMostOneOf) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD validation", @@ -142,7 +153,7 @@ func (KubernetesDefault) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD validation", DetailedHelp: markers.DetailedHelp{ - Summary: "Default sets the default value for this field.", + Summary: "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{ @@ -544,7 +555,7 @@ func (XEmbeddedResource) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD validation", DetailedHelp: markers.DetailedHelp{ - Summary: "EmbeddedResource marks a fields as an embedded resource with apiVersion, kind and metadata fields.", + Summary: "marks a fields as an embedded resource with apiVersion, kind and metadata fields.", Details: "An embedded resource is a value that has apiVersion, kind and metadata fields.\nThey are validated implicitly according to the semantics of the currently\nrunning apiserver. It is not necessary to add any additional schema for these\nfield, yet it is possible. This can be combined with PreserveUnknownFields.", }, FieldHelp: map[string]markers.DetailedHelp{}, @@ -555,7 +566,7 @@ func (XIntOrString) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD validation", DetailedHelp: markers.DetailedHelp{ - Summary: "IntOrString marks a fields as an IntOrString.", + Summary: "marks a fields as an IntOrString.", Details: "This is required when applying patterns or other validations to an IntOrString\nfield. Known information about the type is applied during the collapse phase\nand as such is not normally available during marker application.", }, FieldHelp: map[string]markers.DetailedHelp{}, @@ -566,7 +577,7 @@ func (XPreserveUnknownFields) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD processing", DetailedHelp: markers.DetailedHelp{ - Summary: "PreserveUnknownFields stops the apiserver from pruning fields which are not specified.", + Summary: "stops the apiserver from pruning fields which are not specified.", Details: "By default the apiserver drops unknown fields from the request payload\nduring the decoding step. This marker stops the API server from doing so.\nIt affects fields recursively, but switches back to normal pruning behaviour\nif nested properties or additionalProperties are specified in the schema.\nThis can either be true or undefined. False\nis forbidden.\n\nNB: The kubebuilder:validation:XPreserveUnknownFields variant is deprecated\nin favor of the kubebuilder:pruning:PreserveUnknownFields variant. They function\nidentically.", }, FieldHelp: map[string]markers.DetailedHelp{}, diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index 981ac433c..4a95a1ced 100644 --- a/pkg/crd/schema.go +++ b/pkg/crd/schema.go @@ -472,9 +472,9 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON } // explicitly required - kubebuilder props.Required = append(props.Required, fieldName) - case field.Markers.Get("optional") != nil: + case field.Markers.Get("optional") != nil, field.Markers.Get("k8s:optional") != nil: // explicitly optional - kubernetes - case field.Markers.Get("required") != nil: + case field.Markers.Get("required") != nil, field.Markers.Get("k8s:required") != nil: if exactlyOneOf.Has(fieldName) || atMostOneOf.Has(fieldName) || atLeastOneOf.Has(fieldName) { ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("field %s is part of OneOf constraint and cannot be marked as required", fieldName), structType)) return props diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index 3d2b5ddac..8f6caea58 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -238,6 +238,10 @@ type CronJobSpec struct { // +optional ExplicitlyOptionalKubernetes string `json:"explicitlyOptionalKubernetes"` + // This tests explicitly optional k8s fields + // +k8s:optional + ExplicitlyOptionalK8s string `json:"explicitlyOptionalK8s"` + // This tests explicitly required kubebuilder fields // +kubebuilder:validation:Required ExplicitlyRequiredKubebuilder string `json:"explicitlyRequiredKubebuilder,omitempty"` @@ -246,6 +250,10 @@ type CronJobSpec struct { // +required ExplicitlyRequiredKubernetes string `json:"explicitlyRequiredKubernetes,omitempty"` + // This tests explicitly required k8s fields + // +k8s:required + ExplicitlyRequiredK8s string `json:"explicitlyRequiredK8s,omitempty"` + // This tests that min/max properties work MinMaxProperties MinMaxObject `json:"minMaxProperties,omitempty"` diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index 477d63017..1f9007dcf 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -201,12 +201,18 @@ spec: - 3 type: integer type: array + explicitlyOptionalK8s: + description: This tests explicitly optional k8s fields + type: string explicitlyOptionalKubebuilder: description: This tests explicitly optional kubebuilder fields type: string explicitlyOptionalKubernetes: description: This tests explicitly optional kubernetes fields type: string + explicitlyRequiredK8s: + description: This tests explicitly required k8s fields + type: string explicitlyRequiredKubebuilder: description: This tests explicitly required kubebuilder fields type: string @@ -9174,6 +9180,7 @@ spec: - defaultedString - doubleDefaultedString - embeddedResource + - explicitlyRequiredK8s - explicitlyRequiredKubebuilder - explicitlyRequiredKubernetes - float64WithValidations