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 31240cc211d..fa98c93c870 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 @@ -63,6 +63,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio requireOpenAPISchema: requireOpenAPISchema(requestGV, nil), requireValidPropertyType: requireValidPropertyType(requestGV, nil), requireStructuralSchema: requireStructuralSchema(requestGV, nil), + requirePrunedDefaults: true, } allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata")) @@ -88,6 +89,8 @@ type validationOptions struct { requireValidPropertyType bool // requireStructuralSchema indicates that any schemas present must be structural requireStructuralSchema bool + // requirePrunedDefaults indicates that defaults must be pruned + requirePrunedDefaults bool } // ValidateCustomResourceDefinitionUpdate statically validates @@ -99,6 +102,7 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec), requireValidPropertyType: requireValidPropertyType(requestGV, &oldObj.Spec), requireStructuralSchema: requireStructuralSchema(requestGV, &oldObj.Spec), + requirePrunedDefaults: requirePrunedDefaults(&oldObj.Spec), } allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata")) @@ -665,7 +669,7 @@ func validateCustomResourceDefinitionValidation(customResourceValidation *apiext } } else if validationErrors := structuralschema.ValidateStructural(fldPath.Child("openAPIV3Schema"), ss); len(validationErrors) > 0 { allErrs = append(allErrs, validationErrors...) - } else if validationErrors, err := structuraldefaulting.ValidateDefaults(fldPath.Child("openAPIV3Schema"), ss, true); err != nil { + } else if validationErrors, err := structuraldefaulting.ValidateDefaults(fldPath.Child("openAPIV3Schema"), ss, true, opts.requirePrunedDefaults); err != nil { // this should never happen allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), "", err.Error())) } else { @@ -1186,6 +1190,41 @@ func schemaIsNonStructural(schema *apiextensions.JSONSchemaProps) bool { return len(structuralschema.ValidateStructural(nil, ss)) > 0 } +// requirePrunedDefaults returns false if there are any unpruned default in oldCRDSpec, and true otherwise. +func requirePrunedDefaults(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { + if oldCRDSpec.Validation != nil { + if has, err := schemaHasUnprunedDefaults(oldCRDSpec.Validation.OpenAPIV3Schema); err == nil && has { + return false + } + } + for _, v := range oldCRDSpec.Versions { + if v.Schema == nil { + continue + } + if has, err := schemaHasUnprunedDefaults(v.Schema.OpenAPIV3Schema); err == nil && has { + return false + } + } + return true +} +func schemaHasUnprunedDefaults(schema *apiextensions.JSONSchemaProps) (bool, error) { + if schema == nil || !schemaHasDefaults(schema) { + return false, nil + } + ss, err := structuralschema.NewStructural(schema) + if err != nil { + return false, err + } + if errs := structuralschema.ValidateStructural(nil, ss); len(errs) > 0 { + return false, errs.ToAggregate() + } + pruned := ss.DeepCopy() + if err := structuraldefaulting.PruneDefaults(pruned); err != nil { + return false, err + } + return !reflect.DeepEqual(ss, pruned), nil +} + // 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 == v1beta1.SchemeGroupVersion { 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 cdeb690164d..44c045c5817 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 @@ -5993,6 +5993,294 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{}, }, + { + name: "ratcheting validation of defaults with disabled feature gate via v1, non-structural, no defaults before", + 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": {}, + }, + }, + }, + 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{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), + }, + }, + { + name: "ratcheting validation of defaults with disabled feature gate via v1, unpruned => unpruned", + 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: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + Default: jsonPtr(map[string]interface{}{ + "unknown": "unknown", + }), + }, + }, + }, + }, + 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: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + Default: jsonPtr(map[string]interface{}{ + "unknown": "unknown", + }), + }, + "b": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + Default: jsonPtr(map[string]interface{}{ + "unknown": "unknown", + }), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + requestGV: apiextensionsv1.SchemeGroupVersion, + errors: []validationMatch{}, + }, + { + name: "ratcheting validation of defaults with disabled feature gate via v1, pruned => unpruned", + 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: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + Default: jsonPtr(map[string]interface{}{ + "foo": "foo", + }), + }, + }, + }, + }, + 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: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + Default: jsonPtr(map[string]interface{}{ + "foo": "foo", + }), + }, + "b": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + Default: jsonPtr(map[string]interface{}{ + "unknown": "unknown", + }), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + requestGV: apiextensionsv1.SchemeGroupVersion, + errors: []validationMatch{ + invalid("spec", "validation", "openAPIV3Schema", "properties[b]", "default"), + }, + }, { name: "add default with enabled feature gate, structural schema, without pruning", old: &apiextensions.CustomResourceDefinition{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go index d65b47c98fb..254724dd824 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go @@ -33,7 +33,7 @@ import ( ) // ValidateDefaults checks that default values validate and are properly pruned. -func ValidateDefaults(pth *field.Path, s *structuralschema.Structural, isResourceRoot bool) (field.ErrorList, error) { +func ValidateDefaults(pth *field.Path, s *structuralschema.Structural, isResourceRoot, requirePrunedDefaults bool) (field.ErrorList, error) { f := NewRootObjectFunc().WithTypeMeta(metav1.TypeMeta{APIVersion: "validation/v1", Kind: "Validation"}) if isResourceRoot { @@ -47,13 +47,13 @@ func ValidateDefaults(pth *field.Path, s *structuralschema.Structural, isResourc } } - return validate(pth, s, s, f, false) + return validate(pth, s, s, f, false, requirePrunedDefaults) } // validate is the recursive step func for the validation. insideMeta is true if s specifies // TypeMeta or ObjectMeta. The SurroundingObjectFunc f is used to validate defaults of // TypeMeta or ObjectMeta fields. -func validate(pth *field.Path, s *structuralschema.Structural, rootSchema *structuralschema.Structural, f SurroundingObjectFunc, insideMeta bool) (field.ErrorList, error) { +func validate(pth *field.Path, s *structuralschema.Structural, rootSchema *structuralschema.Structural, f SurroundingObjectFunc, insideMeta, requirePrunedDefaults bool) (field.ErrorList, error) { if s == nil { return nil, nil } @@ -88,10 +88,12 @@ func validate(pth *field.Path, s *structuralschema.Structural, rootSchema *struc } } else { // check whether default is pruned - pruned := runtime.DeepCopyJSONValue(s.Default.Object) - pruning.Prune(pruned, s, s.XEmbeddedResource) - if !reflect.DeepEqual(pruned, s.Default.Object) { - allErrs = append(allErrs, field.Invalid(pth.Child("default"), s.Default.Object, "must not have unknown fields")) + if requirePrunedDefaults { + pruned := runtime.DeepCopyJSONValue(s.Default.Object) + pruning.Prune(pruned, s, s.XEmbeddedResource) + if !reflect.DeepEqual(pruned, s.Default.Object) { + allErrs = append(allErrs, field.Invalid(pth.Child("default"), s.Default.Object, "must not have unknown fields")) + } } // check ObjectMeta/TypeMeta and everything else @@ -108,7 +110,7 @@ func validate(pth *field.Path, s *structuralschema.Structural, rootSchema *struc // do not follow additionalProperties because defaults are forbidden there if s.Items != nil { - errs, err := validate(pth.Child("items"), s.Items, rootSchema, f.Index(), insideMeta) + errs, err := validate(pth.Child("items"), s.Items, rootSchema, f.Index(), insideMeta, requirePrunedDefaults) if err != nil { return nil, err } @@ -120,7 +122,7 @@ func validate(pth *field.Path, s *structuralschema.Structural, rootSchema *struc if s.XEmbeddedResource && (k == "metadata" || k == "apiVersion" || k == "kind") { subInsideMeta = true } - errs, err := validate(pth.Child("properties").Key(k), &subSchema, rootSchema, f.Child(k), subInsideMeta) + errs, err := validate(pth.Child("properties").Key(k), &subSchema, rootSchema, f.Child(k), subInsideMeta, requirePrunedDefaults) if err != nil { return nil, err }