From 97c5b8de9ae98a3ee3fdd867feb3cfbd6199df36 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 9 Aug 2021 13:13:20 -0400 Subject: [PATCH] Drop legacy validation logic for CRD API --- .../apiextensions/validation/validation.go | 78 +- .../validation/validation_test.go | 712 ++++-------------- .../customresourcedefinition/strategy.go | 16 +- .../customresourcedefinition/strategy_test.go | 37 +- 4 files changed, 153 insertions(+), 690 deletions(-) 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 32ae5e99436..c2ce09e627a 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 @@ -27,7 +27,6 @@ import ( structuraldefaulting "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting" apiequality "k8s.io/apimachinery/pkg/api/equality" genericvalidation "k8s.io/apimachinery/pkg/api/validation" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" @@ -47,7 +46,7 @@ var ( ) // ValidateCustomResourceDefinition statically validates -func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList { +func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinition) field.ErrorList { nameValidationFn := func(name string, prefix bool) []string { ret := genericvalidation.NameIsDNSSubdomain(name, prefix) requiredName := obj.Spec.Names.Plural + "." + obj.Spec.Group @@ -57,15 +56,13 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio return ret } - allowDefaults, rejectDefaultsReason := allowDefaults(requestGV, nil) opts := validationOptions{ - allowDefaults: allowDefaults, - disallowDefaultsReason: rejectDefaultsReason, + allowDefaults: true, requireRecognizedConversionReviewVersion: true, requireImmutableNames: false, - requireOpenAPISchema: requireOpenAPISchema(requestGV, nil), - requireValidPropertyType: requireValidPropertyType(requestGV, nil), - requireStructuralSchema: requireStructuralSchema(requestGV, nil), + requireOpenAPISchema: true, + requireValidPropertyType: true, + requireStructuralSchema: true, requirePrunedDefaults: true, requireAtomicSetType: true, requireMapListKeysMapSetValidation: true, @@ -75,8 +72,8 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio allErrs = append(allErrs, validateCustomResourceDefinitionSpec(&obj.Spec, opts, field.NewPath("spec"))...) allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...) allErrs = append(allErrs, ValidateCustomResourceDefinitionStoredVersions(obj.Status.StoredVersions, obj.Spec.Versions, field.NewPath("status").Child("storedVersions"))...) - allErrs = append(allErrs, validateAPIApproval(obj, nil, requestGV)...) - allErrs = append(allErrs, validatePreserveUnknownFields(obj, nil, requestGV)...) + allErrs = append(allErrs, validateAPIApproval(obj, nil)...) + allErrs = append(allErrs, validatePreserveUnknownFields(obj, nil)...) return allErrs } @@ -107,16 +104,14 @@ type validationOptions struct { } // ValidateCustomResourceDefinitionUpdate statically validates -func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList { - allowDefaults, rejectDefaultsReason := allowDefaults(requestGV, &oldObj.Spec) +func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition) field.ErrorList { opts := validationOptions{ - allowDefaults: allowDefaults, - disallowDefaultsReason: rejectDefaultsReason, + allowDefaults: true, requireRecognizedConversionReviewVersion: oldObj.Spec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldObj.Spec.Conversion.ConversionReviewVersions), requireImmutableNames: apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established), - requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec), - requireValidPropertyType: requireValidPropertyType(requestGV, &oldObj.Spec), - requireStructuralSchema: requireStructuralSchema(requestGV, &oldObj.Spec), + requireOpenAPISchema: requireOpenAPISchema(&oldObj.Spec), + requireValidPropertyType: requireValidPropertyType(&oldObj.Spec), + requireStructuralSchema: requireStructuralSchema(&oldObj.Spec), requirePrunedDefaults: requirePrunedDefaults(&oldObj.Spec), requireAtomicSetType: requireAtomicSetType(&oldObj.Spec), requireMapListKeysMapSetValidation: requireMapListKeysMapSetValidation(&oldObj.Spec), @@ -126,8 +121,8 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes allErrs = append(allErrs, validateCustomResourceDefinitionSpecUpdate(&obj.Spec, &oldObj.Spec, opts, field.NewPath("spec"))...) allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...) allErrs = append(allErrs, ValidateCustomResourceDefinitionStoredVersions(obj.Status.StoredVersions, obj.Spec.Versions, field.NewPath("status").Child("storedVersions"))...) - allErrs = append(allErrs, validateAPIApproval(obj, oldObj, requestGV)...) - allErrs = append(allErrs, validatePreserveUnknownFields(obj, oldObj, requestGV)...) + allErrs = append(allErrs, validateAPIApproval(obj, oldObj)...) + allErrs = append(allErrs, validatePreserveUnknownFields(obj, oldObj)...) return allErrs } @@ -1129,11 +1124,7 @@ func allowedAtRootSchema(field string) bool { } // requireOpenAPISchema returns true if the request group version requires a schema -func requireOpenAPISchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { - if requestGV == apiextensionsv1beta1.SchemeGroupVersion { - // for backwards compatibility - return false - } +func requireOpenAPISchema(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { if oldCRDSpec != nil && !allVersionsSpecifyOpenAPISchema(oldCRDSpec) { // don't tighten validation on existing persisted data return false @@ -1152,19 +1143,6 @@ func allVersionsSpecifyOpenAPISchema(spec *apiextensions.CustomResourceDefinitio return true } -// allowDefaults returns true if the defaulting feature is enabled and the request group version allows adding defaults, -// or false and a reason for the user if defaults are not allowed. -func allowDefaults(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) (bool, string) { - if oldCRDSpec != nil && specHasDefaults(oldCRDSpec) { - // don't tighten validation on existing persisted data - return true, "" - } - if requestGV == apiextensionsv1beta1.SchemeGroupVersion { - return false, "(cannot set default values in apiextensions.k8s.io/v1beta1 CRDs, must use apiextensions.k8s.io/v1)" - } - return true, "" -} - func specHasDefaults(spec *apiextensions.CustomResourceDefinitionSpec) bool { return hasSchemaWith(spec, schemaHasDefaults) } @@ -1277,11 +1255,7 @@ func schemaHasKubernetesExtensions(s *apiextensions.JSONSchemaProps) bool { } // requireStructuralSchema returns true if schemas specified must be structural -func requireStructuralSchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { - if requestGV == apiextensionsv1beta1.SchemeGroupVersion { - // for compatibility - return false - } +func requireStructuralSchema(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { if oldCRDSpec != nil && specHasNonStructuralSchema(oldCRDSpec) { // don't tighten validation on existing persisted data return false @@ -1380,11 +1354,7 @@ func hasInvalidMapListKeysMapSet(schema *apiextensions.JSONSchemaProps) bool { } // requireValidPropertyType returns true if valid openapi v3 types should be required for the given API version -func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { - if requestGV == apiextensionsv1beta1.SchemeGroupVersion { - // for compatibility - return false - } +func requireValidPropertyType(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { if oldCRDSpec != nil && specHasInvalidTypes(oldCRDSpec) { // don't tighten validation on existing persisted data return false @@ -1393,13 +1363,8 @@ func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiexte } // validateAPIApproval returns a list of errors if the API approval annotation isn't valid -func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList { +func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition) field.ErrorList { // check to see if we need confirm API approval for kube group. - - if requestGV == apiextensionsv1beta1.SchemeGroupVersion { - // no-op for compatibility with v1beta1 - return nil - } if !apihelpers.IsProtectedCommunityGroup(newCRD.Spec.Group) { // no-op for non-protected groups return nil @@ -1433,12 +1398,7 @@ func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition, } } -func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList { - if requestGV == apiextensionsv1beta1.SchemeGroupVersion { - // no-op for compatibility with v1beta1 - return nil - } - +func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefinition) field.ErrorList { if oldCRD != nil && oldCRD.Spec.PreserveUnknownFields != nil && *oldCRD.Spec.PreserveUnknownFields { // no-op for compatibility with existing data return nil 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 0f7cad95454..538e24f9099 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 @@ -24,12 +24,10 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apimachinery/pkg/api/apitesting/fuzzer" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/validation/field" @@ -60,9 +58,6 @@ func immutable(path ...string) validationMatch { func forbidden(path ...string) validationMatch { return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden} } -func forbiddenContains(contains string, path ...string) validationMatch { - return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden, contains: contains} -} func (v validationMatch) matches(err *field.Error) bool { return err.Type == v.errorType && err.Field == v.path.String() && strings.Contains(err.Error(), v.contains) @@ -79,44 +74,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, } tests := []struct { - name string - resource *apiextensions.CustomResourceDefinition - requestGV schema.GroupVersion - errors []validationMatch + name string + resource *apiextensions.CustomResourceDefinition + errors []validationMatch }{ { - name: "invalid types allowed via v1beta1", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{{ - Name: "version", - Served: true, - Storage: true, - }}, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{"foo": {Type: "bogus"}}, - }, - }, - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - }, - { - name: "invalid types disallowed via v1", + name: "invalid types disallowed", resource: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -141,7 +104,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ unsupported("spec.validation.openAPIV3Schema.properties[foo].type"), }, @@ -374,13 +336,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { URL: strPtr("https://example.com/webhook"), }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ forbidden("spec", "conversion", "webhookClientConfig"), }, @@ -414,13 +380,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Strategy: apiextensions.ConversionStrategyType("None"), ConversionReviewVersions: []string{"v1beta1"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ forbidden("spec", "conversion", "conversionReviewVersions"), }, @@ -726,14 +696,18 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("None"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: nil, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version1"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { name: "webhook conversion without preserveUnknownFields=false", @@ -767,13 +741,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, ConversionReviewVersions: []string{"v1beta1"}, }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: nil, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version1"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("spec", "conversion", "strategy"), }, @@ -896,13 +874,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("None"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("spec", "versions"), }, @@ -935,13 +917,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("None"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("spec", "versions"), invalid("status", "storedVersions"), @@ -975,13 +961,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("None"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("status", "storedVersions"), }, @@ -1009,13 +999,17 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Conversion: &apiextensions.CustomResourceConversion{ Strategy: apiextensions.ConversionStrategyType("None"), }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("status", "storedVersions"), }, @@ -1029,10 +1023,14 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Names: apiextensions.CustomResourceDefinitionNames{ Plural: "plural", }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("status", "storedVersions"), invalid("metadata", "name"), @@ -1074,7 +1072,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Kind: "value()*a", ListKind: "value()*a", }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ AcceptedNames: apiextensions.CustomResourceDefinitionNames{ @@ -1085,7 +1088,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("status", "storedVersions"), invalid("metadata", "name"), @@ -1122,7 +1124,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Kind: "matching", ListKind: "matching", }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ AcceptedNames: apiextensions.CustomResourceDefinitionNames{ @@ -1134,7 +1141,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("metadata", "name"), invalid("spec", "group"), @@ -1163,21 +1169,26 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, Validation: &apiextensions.CustomResourceValidation{ OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", Properties: map[string]apiextensions.JSONSchemaProps{ - "foo": {}, + "foo": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "bar": {Type: "object"}, + }, + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{Allows: false}, + }, }, - AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{Allows: false}, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ - forbidden("spec", "validation", "openAPIV3Schema", "additionalProperties"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[foo]", "additionalProperties"), }, }, { @@ -1200,22 +1211,27 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, Validation: &apiextensions.CustomResourceValidation{ OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ - Allows: true, - Schema: &apiextensions.JSONSchemaProps{ - Type: "string", + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": { + Type: "object", + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Allows: true, + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, }, }, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { name: "per-version fields may not all be set to identical values (top-level field should be used instead)", @@ -1253,13 +1269,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Kind: "Plural", ListKind: "PluralList", }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ // Per-version schema/subresources/columns may not all be set to identical values. // Note that the test will fail if we de-duplicate the expected errors below. @@ -1427,7 +1442,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ required("spec", "versions[1]", "schema", "openAPIV3Schema"), }, @@ -1468,14 +1482,13 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ required("spec", "versions[0]", "schema", "openAPIV3Schema"), required("spec", "versions[1]", "schema", "openAPIV3Schema"), }, }, { - name: "no schema via v1", + name: "schema is required", resource: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -1510,14 +1523,13 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ required("spec", "versions[0]", "schema", "openAPIV3Schema"), required("spec", "versions[1]", "schema", "openAPIV3Schema"), }, }, { - name: "preserveUnknownFields: true via v1", + name: "preserveUnknownFields: true", resource: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -1531,8 +1543,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{invalid("spec.preserveUnknownFields")}, + errors: []validationMatch{invalid("spec.preserveUnknownFields")}, }, { name: "labelSelectorPath outside of .spec and .status", @@ -1601,56 +1612,23 @@ func TestValidateCustomResourceDefinition(t *testing.T) { Kind: "Plural", ListKind: "PluralList", }, - PreserveUnknownFields: pointer.BoolPtr(true), + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version0"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ invalid("spec", "versions[3]", "subresources", "scale", "labelSelectorPath"), }, }, { - name: "defaults with enabled feature gate via v1beta1", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: singleVersionList, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{ - "a": { - Type: "number", - Default: jsonPtr(42.0), - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{ - forbiddenContains("cannot set default values in apiextensions.k8s.io/v1beta1", "spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disallowed via v1beta1 - }, - }, - { - name: "defaults with enabled feature gate via v1", + name: "defaults with enabled feature gate", resource: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -1681,116 +1659,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, - }, - { - name: "x-kubernetes-int-or-string without structural", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: singleVersionList, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "intorstring": { - XIntOrString: true, - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{ - required("spec", "validation", "openAPIV3Schema", "type"), - }, - }, - { - name: "x-kubernetes-preserve-unknown-fields without structural", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: singleVersionList, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "raw": { - XPreserveUnknownFields: pointer.BoolPtr(true), - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{ - required("spec", "validation", "openAPIV3Schema", "type"), - }, - }, - { - name: "x-kubernetes-embedded-resource without structural", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: singleVersionList, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Properties: map[string]apiextensions.JSONSchemaProps{ - "embedded": { - Type: "object", - XEmbeddedResource: true, - Properties: map[string]apiextensions.JSONSchemaProps{ - "foo": {Type: "string"}, - }, - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{ - required("spec", "validation", "openAPIV3Schema", "type"), - }, }, { name: "x-kubernetes-embedded-resource with pruning and empty properties", @@ -1809,7 +1677,8 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, Validation: &apiextensions.CustomResourceValidation{ OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", + Type: "object", + XPreserveUnknownFields: pointer.BoolPtr(true), Properties: map[string]apiextensions.JSONSchemaProps{ "nil": { Type: "object", @@ -1824,13 +1693,11 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ required("spec", "validation", "openAPIV3Schema", "properties[nil]", "properties"), required("spec", "validation", "openAPIV3Schema", "properties[empty]", "properties"), @@ -1889,13 +1756,11 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, }, }, - PreserveUnknownFields: pointer.BoolPtr(true), }, Status: apiextensions.CustomResourceDefinitionStatus{ StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ forbidden("spec", "validation", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "x-kubernetes-embedded-resource"), forbidden("spec", "validation", "openAPIV3Schema", "properties[embedded]", "properties[apiVersion]", "properties[foo]", "x-kubernetes-embedded-resource"), @@ -4083,7 +3948,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { if tc.resource.Spec.Conversion != nil && tc.resource.Spec.Conversion.Strategy == apiextensions.WebhookConverter && len(tc.resource.Spec.Conversion.ConversionReviewVersions) == 0 { tc.resource.Spec.Conversion.ConversionReviewVersions = []string{"v1beta1"} } - errs := ValidateCustomResourceDefinition(tc.resource, tc.requestGV) + errs := ValidateCustomResourceDefinition(tc.resource) seenErrs := make([]bool, len(errs)) for _, expectedError := range tc.errors { @@ -4112,14 +3977,13 @@ func TestValidateCustomResourceDefinition(t *testing.T) { func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { tests := []struct { - name string - old *apiextensions.CustomResourceDefinition - resource *apiextensions.CustomResourceDefinition - requestGV schema.GroupVersion - errors []validationMatch + name string + old *apiextensions.CustomResourceDefinition + resource *apiextensions.CustomResourceDefinition + errors []validationMatch }{ { - name: "invalid type updates allowed via v1beta1", + name: "invalid types updates disallowed", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -4167,64 +4031,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - }, - { - name: "invalid types updates disallowed via v1", - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - }, - }, - PreserveUnknownFields: pointer.BoolPtr(false), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{"foo": {Type: "bogus"}}, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(false), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ unsupported("spec.validation.openAPIV3Schema.properties[foo].type"), }, }, { - name: "invalid types updates allowed via v1 if old object has invalid types", + name: "invalid types updates allowed if old object has invalid types", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -4273,7 +4085,6 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, }, { name: "non-atomic items in lists of type set allowed if pre-existing", @@ -4343,7 +4154,6 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, }, { name: "reject non-atomic items in lists of type set if not pre-existing", @@ -4404,13 +4214,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ invalid("spec", "validation", "openAPIV3Schema", "properties[bar]", "items", "x-kubernetes-map-type"), }, }, { - name: "structural to non-structural updates allowed via v1beta1", + name: "structural to non-structural updates not allowed", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -4449,55 +4258,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - }, - { - name: "structural to non-structural updates not allowed via v1", - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"}, - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{"foo": {Type: "integer"}}, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"}, - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{"foo": {}}, // untyped object - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ required("spec.validation.openAPIV3Schema.properties[foo].type"), }, }, { - name: "absent schema to non-structural updates not allowed via v1", + name: "absent schema to non-structural updates not allowed", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -4531,13 +4297,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ required("spec.validation.openAPIV3Schema.properties[foo].type"), }, }, { - name: "non-structural updates allowed via v1 if old object has non-structural schema", + name: "non-structural updates allowed if old object has non-structural schema", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -4590,8 +4355,7 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { name: "webhookconfig: should pass on invalid ConversionReviewVersion with old invalid versions", @@ -5457,74 +5221,7 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { errors: []validationMatch{}, }, { - name: "switch on preserveUnknownFields without structural schema", - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "plural.group.com", - ResourceVersion: "42", - }, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: validValidationSchema, - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{ - { - Name: "version", - Served: true, - Storage: true, - }, - }, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - PreserveUnknownFields: pointer.BoolPtr(false), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "plural.group.com", - ResourceVersion: "42", - }, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: validUnstructuralValidationSchema, - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{ - { - Name: "version", - Served: true, - Storage: true, - }, - }, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{}, - }, - { - name: "switch to preserveUnknownFields: true is allowed via v1beta1", + name: "switch to preserveUnknownFields: true is forbidden", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -5551,42 +5248,10 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{invalid("spec.preserveUnknownFields")}, }, { - name: "switch to preserveUnknownFields: true is forbidden via v1", - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Validation: &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}}, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"}, - PreserveUnknownFields: pointer.BoolPtr(false), - }, - Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, - }, - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Validation: &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}}, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"}, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, - }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{invalid("spec.preserveUnknownFields")}, - }, - { - name: "keep preserveUnknownFields: true is allowed via v1", + name: "keep preserveUnknownFields: true is allowed", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -5613,11 +5278,10 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { - name: "schema not required via v1beta1", + name: "schema not required if old object is missing schema", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -5642,40 +5306,10 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { - name: "schema not required via v1 if old object is missing schema", - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"}, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, - }, - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"}, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, - }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{}, - }, - { - name: "schema not required via v1 if old object is missing schema for some versions", + name: "schema not required if old object is missing schema for some versions", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -5706,11 +5340,10 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { - name: "schema required via v1 if old object has top-level schema", + name: "schema required if old object has top-level schema", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -5736,13 +5369,12 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ required("spec.versions[0].schema.openAPIV3Schema"), }, }, { - name: "schema required via v1 if all versions of old object have schema", + name: "schema required if all versions of old object have schema", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"}, Spec: apiextensions.CustomResourceDefinitionSpec{ @@ -5773,14 +5405,13 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, }, - requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{ required("spec.versions[0].schema.openAPIV3Schema"), required("spec.versions[1].schema.openAPIV3Schema"), }, }, { - name: "setting defaults with enabled feature gate via v1beta1", + name: "setting defaults with enabled feature gate", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ Name: "plural.group.com", @@ -5858,90 +5489,7 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{forbidden("spec.validation.openAPIV3Schema.properties[a].default")}, - }, - { - name: "setting defaults with enabled feature gate via v1", - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "plural.group.com", - ResourceVersion: "42", - }, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: []apiextensions.CustomResourceDefinitionVersion{ - { - Name: "version", - Served: true, - Storage: true, - }, - }, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{ - "a": { - Type: "number", - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(false), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{ - Name: "plural.group.com", - ResourceVersion: "42", - }, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: []apiextensions.CustomResourceDefinitionVersion{ - { - Name: "version", - Served: true, - Storage: true, - }, - }, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{ - "a": { - Type: "number", - Default: jsonPtr(42.0), - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(false), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{}, + errors: []validationMatch{}, }, { name: "add default with enabled feature gate, structural schema, without pruning", @@ -6527,7 +6075,7 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old, tc.requestGV) + errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old) seenErrs := make([]bool, len(errs)) for _, expectedError := range tc.errors { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go index 389c87b9e3d..708ef2d9b11 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go @@ -27,9 +27,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" - genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" @@ -111,12 +109,7 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { // Validate validates a new CustomResourceDefinition. func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - var groupVersion schema.GroupVersion - if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { - groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} - } - - return validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition), groupVersion) + return validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition)) } // WarningsOnCreate returns warnings for the creation of the given object. @@ -139,12 +132,7 @@ func (strategy) Canonicalize(obj runtime.Object) { // ValidateUpdate is the default update validation for an end user updating status. func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - var groupVersion schema.GroupVersion - if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { - groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} - } - - return validation.ValidateCustomResourceDefinitionUpdate(obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition), groupVersion) + return validation.ValidateCustomResourceDefinitionUpdate(obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition)) } // WarningsOnUpdate returns warnings for the given update. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go index 14323d32575..ecef7af8dca 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go @@ -23,7 +23,6 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/pointer" ) @@ -43,29 +42,19 @@ func TestValidateAPIApproval(t *testing.T) { tests := []struct { name string - version string group string annotationValue string oldAnnotationValue *string validateError func(t *testing.T, errors field.ErrorList) }{ - { - name: "ignore v1beta1", - version: "v1beta1", - group: "sigs.k8s.io", - annotationValue: "invalid", - validateError: okFn, - }, { name: "ignore non-k8s group", - version: "v1", group: "other.io", annotationValue: "invalid", validateError: okFn, }, { name: "invalid annotation create", - version: "v1", group: "sigs.k8s.io", annotationValue: "invalid", validateError: func(t *testing.T, errors field.ErrorList) { @@ -80,7 +69,6 @@ func TestValidateAPIApproval(t *testing.T) { }, { name: "invalid annotation update", - version: "v1", group: "sigs.k8s.io", annotationValue: "invalid", oldAnnotationValue: strPtr("invalid"), @@ -88,7 +76,6 @@ func TestValidateAPIApproval(t *testing.T) { }, { name: "invalid annotation to missing", - version: "v1", group: "sigs.k8s.io", annotationValue: "", oldAnnotationValue: strPtr("invalid"), @@ -104,7 +91,6 @@ func TestValidateAPIApproval(t *testing.T) { }, { name: "missing to invalid annotation", - version: "v1", group: "sigs.k8s.io", annotationValue: "invalid", oldAnnotationValue: strPtr(""), @@ -120,7 +106,6 @@ func TestValidateAPIApproval(t *testing.T) { }, { name: "missing annotation", - version: "v1", group: "sigs.k8s.io", annotationValue: "", validateError: func(t *testing.T, errors field.ErrorList) { @@ -135,7 +120,6 @@ func TestValidateAPIApproval(t *testing.T) { }, { name: "missing annotation update", - version: "v1", group: "sigs.k8s.io", annotationValue: "", oldAnnotationValue: strPtr(""), @@ -143,33 +127,16 @@ func TestValidateAPIApproval(t *testing.T) { }, { name: "url", - version: "v1", group: "sigs.k8s.io", annotationValue: "https://github.com/kubernetes/kubernetes/pull/79724", validateError: okFn, }, { name: "unapproved", - version: "v1", group: "sigs.k8s.io", annotationValue: "unapproved, other reason", validateError: okFn, }, - { - name: "next version validates", - version: "v2", - group: "sigs.k8s.io", - annotationValue: "invalid", - validateError: func(t *testing.T, errors field.ErrorList) { - t.Helper() - if len(errors) == 0 { - t.Fatal("expected errors, got none") - } - if e, a := `metadata.annotations[api-approved.kubernetes.io]: Invalid value: "invalid": protected groups must have approval annotation "api-approved.kubernetes.io" with either a URL or a reason starting with "unapproved", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a { - t.Fatal(errors) - } - }, - }, } for _, test := range tests { @@ -212,9 +179,9 @@ func TestValidateAPIApproval(t *testing.T) { var actual field.ErrorList if oldCRD == nil { - actual = validation.ValidateCustomResourceDefinition(crd, schema.GroupVersion{Group: "apiextensions.k8s.io", Version: test.version}) + actual = validation.ValidateCustomResourceDefinition(crd) } else { - actual = validation.ValidateCustomResourceDefinitionUpdate(crd, oldCRD, schema.GroupVersion{Group: "apiextensions.k8s.io", Version: test.version}) + actual = validation.ValidateCustomResourceDefinitionUpdate(crd, oldCRD) } test.validateError(t, actual) })