Merge pull request #65357 from nikhita/crd-subresources-root-schema

Automatic merge from submit-queue (batch tested with PRs 65357, 65568). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Allow more fields at root of CRD schema if status is enabled

Fixes https://github.com/kubernetes/kubernetes/issues/65293

Currently, we allow only `properties`, `required` and `description` at the root of the CRD schema when the status subresource is enabled.

We can also include some other fields, even though sometimes they might not make sense (but they don't harm).

The main idea is that when validation schema for status is extracted as `properties["status"]`, validation for status is not lost.

**Release note**:

```release-note
More fields are allowed at the root of the CRD validation schema when the status subresource is enabled.
```

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go

@@ -291,7 +291,8 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext
 	}
 
 	if schema := customResourceValidation.OpenAPIV3Schema; schema != nil {
-		// if subresources are enabled, only "properties", "required" and "description" are allowed inside the root schema
+		// if the status subresource is enabled, only certain fields are allowed inside the root schema.
+		// these fields are chosen such that, if status is extracted as properties["status"], it's validation is not lost.
 		if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) && statusSubresourceEnabled {
 			v := reflect.ValueOf(schema).Elem()
 			for i := 0; i < v.NumField(); i++ {
@@ -300,8 +301,19 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext
 					continue
 				}
 
-				if name := v.Type().Field(i).Name; name != "Properties" && name != "Required" && name != "Description" {
-					allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), *schema, fmt.Sprintf(`must only have "properties", "required" or "description" at the root if the status subresource is enabled`)))
+				fieldName := v.Type().Field(i).Name
+
+				// only "object" type is valid at root of the schema since validation schema for status is extracted as properties["status"]
+				if fieldName == "Type" {
+					if schema.Type != "object" {
+						allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema.type"), schema.Type, fmt.Sprintf(`only "object" is allowed as the type at the root of the schema if the status subresource is enabled`)))
+						break
+					}
+					continue
+				}
+
+				if !allowedAtRootSchema(fieldName) {
+					allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), *schema, fmt.Sprintf(`only %v fields are allowed at the root of the schema if the status subresource is enabled`, allowedFieldsAtRootSchema)))
 					break
 				}
 			}
@@ -521,3 +533,14 @@ func validateSimpleJSONPath(s string, fldPath *field.Path) field.ErrorList {
 
 	return allErrs
 }
+
+var allowedFieldsAtRootSchema = []string{"Description", "Type", "Format", "Title", "Maximum", "ExclusiveMaximum", "Minimum", "ExclusiveMinimum", "MaxLength", "MinLength", "Pattern", "MaxItems", "MinItems", "UniqueItems", "MultipleOf", "Required", "Items", "Properties", "ExternalDocs", "Example"}
+
+func allowedAtRootSchema(field string) bool {
+	for _, v := range allowedFieldsAtRootSchema {
+		if field == v {
+			return true
+		}
+	}
+	return false
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go

@@ -832,32 +832,71 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 			name: "root type without status",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
-					Type: "object",
+					Type: "string",
 				},
 			},
 			statusEnabled: false,
 			wantError:     false,
 		},
 		{
-			name: "root type with status",
+			name: "root type having invalid value, with status",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
-					Type: "object",
+					Type: "string",
 				},
 			},
 			statusEnabled: true,
 			wantError:     true,
 		},
 		{
-			name: "properties, required and description with status",
+			name: "non-allowed root field with status",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					AnyOf: []apiextensions.JSONSchemaProps{
+						{
+							Description: "First schema",
+						},
+						{
+							Description: "Second schema",
+						},
+					},
+				},
+			},
+			statusEnabled: true,
+			wantError:     true,
+		},
+		{
+			name: "all allowed fields at the root of the schema with status",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Description:      "This is a description",
+					Type:             "object",
+					Format:           "date-time",
+					Title:            "This is a title",
+					Maximum:          float64Ptr(10),
+					ExclusiveMaximum: true,
+					Minimum:          float64Ptr(5),
+					ExclusiveMinimum: true,
+					MaxLength:        int64Ptr(10),
+					MinLength:        int64Ptr(5),
+					Pattern:          "^[a-z]$",
+					MaxItems:         int64Ptr(10),
+					MinItems:         int64Ptr(5),
+					MultipleOf:       float64Ptr(3),
+					Required:         []string{"spec", "status"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						Schema: &apiextensions.JSONSchemaProps{
+							Description: "This is a schema nested under Items",
+						},
+					},
 					Properties: map[string]apiextensions.JSONSchemaProps{
 						"spec":   {},
 						"status": {},
 					},
-					Required:    []string{"spec", "status"},
-					Description: "This is a description",
+					ExternalDocs: &apiextensions.ExternalDocumentation{
+						Description: "This is an external documentation description",
+					},
+					Example: &example,
 				},
 			},
 			statusEnabled: true,
@@ -875,3 +914,13 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 		})
 	}
 }
+
+var example = apiextensions.JSON(`"This is an example"`)
+
+func float64Ptr(f float64) *float64 {
+	return &f
+}
+
+func int64Ptr(f int64) *int64 {
+	return &f
+}

staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go

@@ -321,7 +321,7 @@ func TestScaleSubresource(t *testing.T) {
 	}
 }
 
-func TestValidationSchema(t *testing.T) {
+func TestValidationSchemaWithStatus(t *testing.T) {
 	stopCh, config, err := testserver.StartDefaultServer()
 	if err != nil {
 		t.Fatal(err)
@@ -344,7 +344,7 @@ func TestValidationSchema(t *testing.T) {
 	}
 	_, err = testserver.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient)
 	if err == nil {
-		t.Fatalf(`unexpected non-error, expected: must only have "properties" or "required" at the root if the status subresource is enabled`)
+		t.Fatalf(`unexpected non-error, expected: must not have "additionalProperties" at the root of the schema if the status subresource is enabled`)
 	}
 
 	// make sure we are not restricting fields to properties even in subschemas