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 1986930d6b1..be134821a4c 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 @@ -21,9 +21,12 @@ import ( "reflect" "strings" - structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "github.com/go-openapi/strfmt" + govalidate "github.com/go-openapi/validate" + apiequality "k8s.io/apimachinery/pkg/api/equality" genericvalidation "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" @@ -32,6 +35,8 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning" apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" ) @@ -103,9 +108,9 @@ func ValidateUpdateCustomResourceDefinitionStatus(obj, oldObj *apiextensions.Cus } // ValidateCustomResourceDefinitionVersion statically validates. -func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResourceDefinitionVersion, fldPath *field.Path, mustBeStructural, statusEnabled bool) field.ErrorList { +func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResourceDefinitionVersion, fldPath *field.Path, mustBeStructural, statusEnabled, allowDefaults bool) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(version.Schema, mustBeStructural, statusEnabled, fldPath.Child("schema"))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(version.Schema, mustBeStructural, statusEnabled, allowDefaults, fldPath.Child("schema"))...) allErrs = append(allErrs, ValidateCustomResourceDefinitionSubresources(version.Subresources, fldPath.Child("subresources"))...) for i := range version.AdditionalPrinterColumns { allErrs = append(allErrs, ValidateCustomResourceColumnDefinition(&version.AdditionalPrinterColumns[i], fldPath.Child("additionalPrinterColumns").Index(i))...) @@ -115,10 +120,11 @@ func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResour // ValidateCustomResourceDefinitionSpec statically validates func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList { - return validateCustomResourceDefinitionSpec(spec, true, fldPath) + allowDefaults := utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) + return validateCustomResourceDefinitionSpec(spec, true, allowDefaults, fldPath) } -func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList { +func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion, allowDefaults bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(spec.Group) == 0 { @@ -144,6 +150,12 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi } } } + if allowDefaults && specHasDefaults(spec) { + mustBeStructural = true + if spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields == true { + allErrs = append(allErrs, field.Invalid(fldPath.Child("preserveUnknownFields"), true, "must be false in order to use defaults in the schema")) + } + } storageFlagCount := 0 versionsMap := map[string]bool{} @@ -161,7 +173,7 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi allErrs = append(allErrs, field.Invalid(fldPath.Child("versions").Index(i).Child("name"), spec.Versions[i].Name, strings.Join(errs, ","))) } subresources := getSubresourcesForVersion(spec, version.Name) - allErrs = append(allErrs, ValidateCustomResourceDefinitionVersion(&version, fldPath.Child("versions").Index(i), mustBeStructural, hasStatusEnabled(subresources))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionVersion(&version, fldPath.Child("versions").Index(i), mustBeStructural, hasStatusEnabled(subresources), allowDefaults)...) } // The top-level and per-version fields are mutual exclusive @@ -216,7 +228,7 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi } allErrs = append(allErrs, ValidateCustomResourceDefinitionNames(&spec.Names, fldPath.Child("names"))...) - allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(spec.Validation, mustBeStructural, hasAnyStatusEnabled(spec), fldPath.Child("validation"))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(spec.Validation, mustBeStructural, hasAnyStatusEnabled(spec), allowDefaults, fldPath.Child("validation"))...) allErrs = append(allErrs, ValidateCustomResourceDefinitionSubresources(spec.Subresources, fldPath.Child("subresources"))...) for i := range spec.AdditionalPrinterColumns { @@ -343,7 +355,11 @@ func validateCustomResourceConversion(conversion *apiextensions.CustomResourceCo // ValidateCustomResourceDefinitionSpecUpdate statically validates func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, established bool, fldPath *field.Path) field.ErrorList { requireRecognizedVersion := oldSpec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldSpec.Conversion.ConversionReviewVersions) - allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, fldPath) + + // find out whether any schema had default before. Then we keep allowing it. + allowDefaults := utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) || specHasDefaults(oldSpec) + + allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, allowDefaults, fldPath) if established { // these effect the storage and cannot be changed therefore @@ -546,7 +562,7 @@ type specStandardValidator interface { } // ValidateCustomResourceDefinitionValidation statically validates -func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiextensions.CustomResourceValidation, mustBeStructural, statusSubresourceEnabled bool, fldPath *field.Path) field.ErrorList { +func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiextensions.CustomResourceValidation, mustBeStructural, statusSubresourceEnabled, allowDefaults bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if customResourceValidation == nil { @@ -586,7 +602,9 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext allErrs = append(allErrs, field.Forbidden(fldPath.Child("openAPIV3Schema.nullable"), fmt.Sprintf(`nullable cannot be true at the root`))) } - openAPIV3Schema := &specStandardValidatorV3{} + openAPIV3Schema := &specStandardValidatorV3{ + allowDefaults: allowDefaults, + } allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema)...) if mustBeStructural { @@ -706,7 +724,9 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch return allErrs } -type specStandardValidatorV3 struct{} +type specStandardValidatorV3 struct { + allowDefaults bool +} // validate validates against OpenAPI Schema v3. func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList { @@ -721,7 +741,24 @@ func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps // if schema.Default != nil { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), "default is not supported")) + if v.allowDefaults { + if s, err := structuralschema.NewStructural(schema); err == nil { + // ignore errors here locally. They will show up for the root of the schema. + pruned := runtime.DeepCopyJSONValue(*schema.Default) + pruning.Prune(pruned, s) + if !reflect.DeepEqual(pruned, *schema.Default) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, "must not have unspecified fields")) + } + + // validate the default value. Only validating and pruned defaults are allowed. + validator := govalidate.NewSchemaValidator(s.ToGoOpenAPI(), nil, "", strfmt.Default) + if err := apiservervalidation.ValidateCustomResource(pruned, validator); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), schema.Default, fmt.Sprintf("must validate: %v", err))) + } + } + } else { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), "must not be set")) + } } if schema.ID != "" { @@ -830,3 +867,86 @@ func allowedAtRootSchema(field string) bool { } return false } + +func specHasDefaults(spec *apiextensions.CustomResourceDefinitionSpec) bool { + if spec.Validation != nil && schemaHasDefaults(spec.Validation.OpenAPIV3Schema) { + return true + } + for _, v := range spec.Versions { + if v.Schema != nil && schemaHasDefaults(v.Schema.OpenAPIV3Schema) { + return true + } + } + return false +} + +func schemaHasDefaults(s *apiextensions.JSONSchemaProps) bool { + if s == nil { + return false + } + + if s.Default != nil { + return true + } + + if s.Items != nil { + if s.Items != nil && schemaHasDefaults(s.Items.Schema) { + return true + } + for _, s := range s.Items.JSONSchemas { + if schemaHasDefaults(&s) { + return true + } + } + } + for _, s := range s.AllOf { + if schemaHasDefaults(&s) { + return true + } + } + for _, s := range s.AnyOf { + if schemaHasDefaults(&s) { + return true + } + } + for _, s := range s.OneOf { + if schemaHasDefaults(&s) { + return true + } + } + if schemaHasDefaults(s.Not) { + return true + } + for _, s := range s.Properties { + if schemaHasDefaults(&s) { + return true + } + } + if s.AdditionalProperties != nil { + if schemaHasDefaults(s.AdditionalProperties.Schema) { + return true + } + } + for _, s := range s.PatternProperties { + if schemaHasDefaults(&s) { + return true + } + } + if s.AdditionalItems != nil { + if schemaHasDefaults(s.AdditionalItems.Schema) { + return true + } + } + for _, s := range s.Definitions { + if schemaHasDefaults(&s) { + return true + } + } + for _, d := range s.Dependencies { + if schemaHasDefaults(d.Schema) { + return true + } + } + + return false +} 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 14bc3be9f96..475f6110950 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 @@ -17,11 +17,23 @@ limitations under the License. package validation import ( + "math/rand" + "strings" "testing" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer" + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/pkg/features" + "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/serializer" + "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/component-base/featuregate" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" ) @@ -64,9 +76,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, } tests := []struct { - name string - resource *apiextensions.CustomResourceDefinition - errors []validationMatch + name string + resource *apiextensions.CustomResourceDefinition + errors []validationMatch + enabledFeatures []featuregate.Feature }{ { name: "webhookconfig: invalid port 0", @@ -1239,10 +1252,325 @@ func TestValidateCustomResourceDefinition(t *testing.T) { invalid("spec", "versions[3]", "subresources", "scale", "labelSelectorPath"), }, }, + { + name: "defaults with disabled feature gate", + 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{ + "a": {Default: jsonPtr(42.0)}, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disabled feature-gate + }, + }, + { + name: "defaults with enabled feature gate, unstructural schema", + 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{ + "a": {Default: jsonPtr(42.0)}, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + required("spec", "validation", "openAPIV3Schema", "properties[a]", "type"), + required("spec", "validation", "openAPIV3Schema", "type"), + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, + { + name: "defaults with enabled feature gate, structural schema", + 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(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, + { + name: "defaults in value validation with enabled feature gate, structural schema", + 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", + Not: &apiextensions.JSONSchemaProps{ + Default: jsonPtr(42.0), + }, + AnyOf: []apiextensions.JSONSchemaProps{ + { + Default: jsonPtr(42.0), + }, + }, + AllOf: []apiextensions.JSONSchemaProps{ + { + Default: jsonPtr(42.0), + }, + }, + OneOf: []apiextensions.JSONSchemaProps{ + { + Default: jsonPtr(42.0), + }, + }, + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "not", "default"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "allOf[0]", "default"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "anyOf[0]", "default"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "oneOf[0]", "default"), + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, + { + name: "invalid defaults with enabled feature gate, structural schema", + 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: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": { + Type: "string", + }, + }, + Default: jsonPtr(map[string]interface{}{ + "foo": "abc", + "bar": int64(42.0), + }), + }, + "b": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": { + Type: "string", + }, + }, + Default: jsonPtr(map[string]interface{}{ + "foo": "abc", + }), + }, + "c": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": { + Type: "string", + }, + }, + Default: jsonPtr(map[string]interface{}{ + "foo": int64(42), + }), + }, + "d": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "good": { + Type: "string", + Pattern: "a", + }, + "bad": { + Type: "string", + Pattern: "+", + }, + }, + Default: jsonPtr(map[string]interface{}{ + "good": "a", + "bad": "a", + }), + }, + "e": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "preserveUnknownFields": { + Type: "object", + Default: jsonPtr(map[string]interface{}{ + "foo": "abc", + // this is under x-kubernetes-preserve-unknown-fields + "bar": int64(42.0), + }), + }, + "nestedProperties": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": { + Type: "string", + }, + }, + Default: jsonPtr(map[string]interface{}{ + "foo": "abc", + "bar": int64(42.0), + }), + }, + }, + XPreserveUnknownFields: pointer.BoolPtr(true), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), + invalid("spec", "validation", "openAPIV3Schema", "properties[c]", "default"), + invalid("spec", "validation", "openAPIV3Schema", "properties[d]", "default"), + // we also expected unpruned and valid defaults under x-kubernetes-preserve-unknown-fields. We could be more + // strict here, but want to encourage proper specifications by forbidding other defaults. + invalid("spec", "validation", "openAPIV3Schema", "properties[e]", "properties[preserveUnknownFields]", "default"), + invalid("spec", "validation", "openAPIV3Schema", "properties[e]", "properties[nestedProperties]", "default"), + }, + + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, + { + name: "defaults with enabled feature gate, structural schema, without pruning", + 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"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "preserveUnknownFields"), + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + for _, gate := range tc.enabledFeatures { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, true)() + } // duplicate defaulting behaviour 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"} @@ -1276,10 +1604,11 @@ func TestValidateCustomResourceDefinition(t *testing.T) { func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { tests := []struct { - name string - old *apiextensions.CustomResourceDefinition - resource *apiextensions.CustomResourceDefinition - errors []validationMatch + name string + old *apiextensions.CustomResourceDefinition + resource *apiextensions.CustomResourceDefinition + errors []validationMatch + enabledFeatures []featuregate.Feature }{ { name: "webhookconfig: should pass on invalid ConversionReviewVersion with old invalid versions", @@ -2180,32 +2509,206 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, errors: []validationMatch{}, }, + { + name: "setting defaults with enabled feature gate", + 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"}, + }, + }, + errors: []validationMatch{}, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, + { + name: "ratcheting validation of defaults with disabled feature gate", + 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", + Default: jsonPtr(42.0), + }, + }, + }, + }, + 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), + }, + "b": { + Type: "number", + Default: jsonPtr(43.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{}, + }, } for _, tc := range tests { - errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old) - seenErrs := make([]bool, len(errs)) + t.Run(tc.name, func(t *testing.T) { + for _, gate := range tc.enabledFeatures { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, true)() + } - for _, expectedError := range tc.errors { - found := false - for i, err := range errs { - if expectedError.matches(err) && !seenErrs[i] { - found = true - seenErrs[i] = true - break + errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old) + seenErrs := make([]bool, len(errs)) + + for _, expectedError := range tc.errors { + found := false + for i, err := range errs { + if expectedError.matches(err) && !seenErrs[i] { + found = true + seenErrs[i] = true + break + } + } + + if !found { + t.Errorf("expected %v at %v, got %v", expectedError.errorType, expectedError.path.String(), errs) } } - if !found { - t.Errorf("%s: expected %v at %v, got %v", tc.name, expectedError.errorType, expectedError.path.String(), errs) + for i, seen := range seenErrs { + if !seen { + t.Errorf("unexpected error: %v", errs[i]) + } } - } - - for i, seen := range seenErrs { - if !seen { - t.Errorf("%s: unexpected error: %v", tc.name, errs[i]) - } - } + }) } } @@ -2356,7 +2859,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ValidateCustomResourceDefinitionValidation(&tt.input, tt.mustBeStructural, tt.statusEnabled, field.NewPath("spec", "validation")) + got := ValidateCustomResourceDefinitionValidation(&tt.input, tt.mustBeStructural, tt.statusEnabled, false, 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 { @@ -2366,6 +2869,40 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { } } +func TestSchemaHasDefaults(t *testing.T) { + scheme := runtime.NewScheme() + codecs := serializer.NewCodecFactory(scheme) + if err := apiextensions.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + seed := rand.Int63() + t.Logf("seed: %d", seed) + fuzzerFuncs := fuzzer.MergeFuzzerFuncs(apiextensionsfuzzer.Funcs) + f := fuzzer.FuzzerFor(fuzzerFuncs, rand.NewSource(seed), codecs) + + for i := 0; i < 10000; i++ { + // fuzz internal types + schema := &apiextensions.JSONSchemaProps{} + f.Fuzz(schema) + + v1beta1Schema := &apiextensionsv1beta1.JSONSchemaProps{} + if err := apiextensionsv1beta1.Convert_apiextensions_JSONSchemaProps_To_v1beta1_JSONSchemaProps(schema, v1beta1Schema, nil); err != nil { + t.Fatal(err) + } + + bs, err := json.Marshal(v1beta1Schema) + if err != nil { + t.Fatal(err) + } + + expected := strings.Contains(strings.Replace(string(bs), `"default":null`, `"deleted":null`, -1), `"default":`) + if got := schemaHasDefaults(schema); got != expected { + t.Errorf("expected %v, got %v for: %s", expected, got, string(bs)) + } + } +} + var example = apiextensions.JSON(`"This is an example"`) var validValidationSchema = &apiextensions.JSONSchemaProps{ @@ -2442,3 +2979,8 @@ func float64Ptr(f float64) *float64 { func int64Ptr(f int64) *int64 { return &f } + +func jsonPtr(x interface{}) *apiextensions.JSON { + ret := apiextensions.JSON(x) + return &ret +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go index eb03eb6b8ca..c5432cb263d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go @@ -21,7 +21,7 @@ import ( ) // Prune removes object fields in obj which are not specified in s. -func Prune(obj map[string]interface{}, s *structuralschema.Structural) { +func Prune(obj interface{}, s *structuralschema.Structural) { prune(obj, s) }