diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD index b641b609370..b9f40d1c1d6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD @@ -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", ], ) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 644557837c6..390f75f4426 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -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 + } } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index c7c9b6d494b..687add9d7b0 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -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") + } + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go index 763c0f9abf3..ac9519512b5 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go @@ -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 { diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/testserver/resources.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/testserver/resources.go index 9b9329d9452..cdfd133aa76 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/testserver/resources.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/testserver/resources.go @@ -246,6 +246,7 @@ func checkForWatchCachePrimed(crd *apiextensionsv1beta1.CustomResourceDefinition "gamma": "bar", "delta": "hello", "epsilon": "foobar", + "spec": map[string]interface{}{}, }, } if _, err := resourceClient.Create(instance); err != nil {