Merge pull request #944 from liggitt/required

✨ add support for kubernetes +required annotations
kubernetes-sigs · May 7, 2024 · 1218899 · 1218899
2 parents 775e273 + 8a753ae
commit 1218899
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 12 deletions.
diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go
@@ -80,11 +80,13 @@ var ValidationMarkers = mustMakeAllWithPrefix(validationPrefix, markers.Describe
 // sense on a type, and thus aren't in ValidationMarkers).
 var FieldOnlyMarkers = []*definitionWithHelp{
 	must(markers.MakeDefinition("kubebuilder:validation:Required", markers.DescribesField, struct{}{})).
-		WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is required, if fields are optional by default.")),
+		WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is required.")),
 	must(markers.MakeDefinition("kubebuilder:validation:Optional", markers.DescribesField, struct{}{})).
-		WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is optional, if fields are required by default.")),
+		WithHelp(markers.SimpleHelp("CRD validation", "specifies that this field is optional.")),
+	must(markers.MakeDefinition("required", markers.DescribesField, struct{}{})).
+		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, if fields are required by default.")),
+		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/schema.go b/pkg/crd/schema.go
@@ -404,20 +404,28 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON
 			defaultMode = "optional"
 		}
 
-		switch defaultMode {
+		switch {
+		case field.Markers.Get("kubebuilder:validation:Optional") != nil:
+			// explicity optional - kubebuilder
+		case field.Markers.Get("kubebuilder:validation:Required") != nil:
+			// explicitly required - kubebuilder
+			props.Required = append(props.Required, fieldName)
+		case field.Markers.Get("optional") != nil:
+			// explicity optional - kubernetes
+		case field.Markers.Get("required") != nil:
+			// explicitly required - kubernetes
+			props.Required = append(props.Required, fieldName)
+
 		// if this package isn't set to optional default...
-		case "required":
-			// ...everything that's not inline, omitempty, or explicitly optional is required
-			if !inline && !omitEmpty && field.Markers.Get("kubebuilder:validation:Optional") == nil && field.Markers.Get("optional") == nil {
+		case defaultMode == "required":
+			// ...everything that's not inline / omitempty is required
+			if !inline && !omitEmpty {
 				props.Required = append(props.Required, fieldName)
 			}
 
 		// if this package isn't set to required default...
-		case "optional":
-			// ...everything that isn't explicitly required is optional
-			if field.Markers.Get("kubebuilder:validation:Required") != nil {
-				props.Required = append(props.Required, fieldName)
-			}
+		case defaultMode == "optional":
+			// implicitly optional
 		}
 
 		var propSchema *apiext.JSONSchemaProps

diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go
@@ -189,6 +189,22 @@ type CronJobSpec struct {
 	// +kubebuilder:validation:optional
 	JustNestedObject *JustNestedObject `json:"justNestedObject,omitempty"`
 
+	// This tests explicitly optional kubebuilder fields
+	// +kubebuilder:validation:Optional
+	ExplicitlyOptionalKubebuilder string `json:"explicitlyOptionalKubebuilder"`
+
+	// This tests explicitly optional kubernetes fields
+	// +optional
+	ExplicitlyOptionalKubernetes string `json:"explicitlyOptionalKubernetes"`
+
+	// This tests explicitly required kubebuilder fields
+	// +kubebuilder:validation:Required
+	ExplicitlyRequiredKubebuilder string `json:"explicitlyRequiredKubebuilder,omitempty"`
+
+	// This tests explicitly required kubernetes fields
+	// +required
+	ExplicitlyRequiredKubernetes string `json:"explicitlyRequiredKubernetes,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
@@ -172,6 +172,18 @@ spec:
                 description: This tests that primitive defaulting can be performed.
                 example: forty-two
                 type: string
+              explicitlyOptionalKubebuilder:
+                description: This tests explicitly optional kubebuilder fields
+                type: string
+              explicitlyOptionalKubernetes:
+                description: This tests explicitly optional kubernetes fields
+                type: string
+              explicitlyRequiredKubebuilder:
+                description: This tests explicitly required kubebuilder fields
+                type: string
+              explicitlyRequiredKubernetes:
+                description: This tests explicitly required kubernetes fields
+                type: string
               embeddedResource:
                 type: object
                 x-kubernetes-embedded-resource: true
@@ -6887,6 +6899,8 @@ spec:
             - defaultedSlice
             - defaultedString
             - embeddedResource
+            - explicitlyRequiredKubebuilder
+            - explicitlyRequiredKubernetes
             - float64WithValidations
             - floatWithValidations
             - foo