From 4be15fd0584d0be78951fe76d6542815dcc790cd Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Fri, 14 Feb 2020 11:45:07 +0000 Subject: [PATCH] UPSTREAM: 65357: Allow more fields at root of CRD schema if status is enabled Signed-off-by: James Munnelly --- .../apiextensions/validation/validation.go | 29 ++++++++- .../validation/validation_test.go | 61 +++++++++++++++++-- .../test/integration/subresources_test.go | 4 +- 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 1179226dd751..2177cbd7e457 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/vendor/k8s.io/kubernetes/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 +} diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index 68dc8aaede5e..794142b7072d 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/vendor/k8s.io/kubernetes/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 +} diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go index bce63a4a5aba..a8cf13b5879f 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go +++ b/vendor/k8s.io/kubernetes/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