apiextensions: allow "required" at root with status subresource

This commit is contained in:
Dr. Stefan Schimanski 2018-05-08 10:41:04 +02:00
parent e6b6e5c4b4
commit 1558da15f1
5 changed files with 96 additions and 15 deletions

View File

@ -27,8 +27,11 @@ go_test(
embed = [":go_default_library"],
deps = [
"//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
"//vendor/k8s.io/apiextensions-apiserver/pkg/features:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature/testing:go_default_library",
],
)

View File

@ -106,7 +106,11 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
allErrs = append(allErrs, ValidateCustomResourceDefinitionNames(&spec.Names, fldPath.Child("names"))...)
if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceValidation) {
allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(spec.Validation, fldPath.Child("validation"))...)
statusEnabled := false
if spec.Subresources != nil && spec.Subresources.Status != nil {
statusEnabled = true
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(spec.Validation, statusEnabled, fldPath.Child("validation"))...)
} else if spec.Validation != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("validation"), "disabled by feature-gate CustomResourceValidation"))
}
@ -187,7 +191,7 @@ type specStandardValidator interface {
}
// ValidateCustomResourceDefinitionValidation statically validates
func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiextensions.CustomResourceValidation, fldPath *field.Path) field.ErrorList {
func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiextensions.CustomResourceValidation, statusSubresourceEnabled bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if customResourceValidation == nil {
@ -195,21 +199,19 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext
}
if schema := customResourceValidation.OpenAPIV3Schema; schema != nil {
// if subresources are enabled, only properties is allowed inside the root schema
if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) {
// if subresources are enabled, only "properties" and "required" is allowed inside the root schema
if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) && statusSubresourceEnabled {
v := reflect.ValueOf(schema).Elem()
fieldsPresent := 0
for i := 0; i < v.NumField(); i++ {
field := v.Field(i).Interface()
if !reflect.DeepEqual(field, reflect.Zero(reflect.TypeOf(field)).Interface()) {
fieldsPresent++
// skip zero values
if value := v.Field(i).Interface(); reflect.DeepEqual(value, reflect.Zero(reflect.TypeOf(value)).Interface()) {
continue
}
}
if fieldsPresent > 1 || (fieldsPresent == 1 && v.FieldByName("Properties").IsNil()) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), *schema, fmt.Sprintf("if subresources for custom resources are enabled, only properties can be used at the root of the schema")))
return allErrs
if name := v.Type().Field(i).Name; name != "Properties" && name != "Required" {
allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), *schema, fmt.Sprintf(`must only have "properties" or "required" at the root if the status subresource is enabled`)))
break
}
}
}

View File

@ -19,9 +19,13 @@ package validation
import (
"testing"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apiextensions-apiserver/pkg/features"
)
type validationMatch struct {
@ -517,3 +521,70 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) {
}
}
}
func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceSubresources, true)()
tests := []struct {
name string
input apiextensions.CustomResourceValidation
statusEnabled bool
wantError bool
}{
{
name: "empty",
input: apiextensions.CustomResourceValidation{},
wantError: false,
},
{
name: "empty with status",
input: apiextensions.CustomResourceValidation{},
statusEnabled: true,
wantError: false,
},
{
name: "root type without status",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
},
},
statusEnabled: false,
wantError: false,
},
{
name: "root type with status",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
},
},
statusEnabled: true,
wantError: true,
},
{
name: "properties and required with status",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"spec": {},
"status": {},
},
Required: []string{"spec", "status"},
},
},
statusEnabled: true,
wantError: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := ValidateCustomResourceDefinitionValidation(&tt.input, tt.statusEnabled, field.NewPath("spec", "validation"))
if !tt.wantError && len(got) > 0 {
t.Errorf("Expected no error, but got: %v", got)
} else if tt.wantError && len(got) == 0 {
t.Error("Expected error, but got none")
}
})
}
}

View File

@ -355,9 +355,12 @@ func TestValidationSchema(t *testing.T) {
// fields other than properties in root schema are not allowed
noxuDefinition := newNoxuValidationCRD(apiextensionsv1beta1.NamespaceScoped)
noxuDefinition.Spec.Subresources = &apiextensionsv1beta1.CustomResourceSubresources{
Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{},
}
err = testserver.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient)
if err == nil {
t.Fatalf("unexpected non-error: if subresources for custom resources are enabled, only properties can be used at the root of the schema")
t.Fatalf(`unexpected non-error, expected: must only have "properties" or "required" at the root if the status subresource is enabled`)
}
// make sure we are not restricting fields to properties even in subschemas
@ -372,6 +375,7 @@ func TestValidationSchema(t *testing.T) {
},
},
},
Required: []string{"spec"},
}
err = testserver.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient)
if err != nil {

View File

@ -245,6 +245,7 @@ func checkForWatchCachePrimed(crd *apiextensionsv1beta1.CustomResourceDefinition
"gamma": "bar",
"delta": "hello",
"epsilon": "foobar",
"spec": map[string]interface{}{},
},
}
if _, err := resourceClient.Create(instance); err != nil {