diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 968e6eabe27..7047c847c29 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -18569,7 +18569,7 @@ "type": "array" }, "x-kubernetes-list-type": { - "description": "x-kubernetes-list-type annotates an array to further describe its topology. This extension must only be used on lists and may have 3 possible values:\n\n1) `atomic`: the list is treated as a single entity, like a scalar.\n Atomic lists will be entirely replaced when updated. This extension\n may be used on any type of list (struct, scalar, ...).\n2) `set`:\n Sets are lists that must not have multiple items with the same value. Each\n value must be a scalar (or another atomic type).\n3) `map`:\n These lists are like maps in that their elements have a non-index key\n used to identify them. Order is preserved upon merge. The map tag\n must only be used on a list with elements of type object.\nDefaults to atomic for arrays.", + "description": "x-kubernetes-list-type annotates an array to further describe its topology. This extension must only be used on lists and may have 3 possible values:\n\n1) `atomic`: the list is treated as a single entity, like a scalar.\n Atomic lists will be entirely replaced when updated. This extension\n may be used on any type of list (struct, scalar, ...).\n2) `set`:\n Sets are lists that must not have multiple items with the same value. Each\n value must be a scalar, an object with x-kubernetes-map-type `atomic` or an\n array with x-kubernetes-list-type `atomic`.\n3) `map`:\n These lists are like maps in that their elements have a non-index key\n used to identify them. Order is preserved upon merge. The map tag\n must only be used on a list with elements of type object.\nDefaults to atomic for arrays.", "type": "string" }, "x-kubernetes-map-type": { @@ -19212,7 +19212,7 @@ "type": "array" }, "x-kubernetes-list-type": { - "description": "x-kubernetes-list-type annotates an array to further describe its topology. This extension must only be used on lists and may have 3 possible values:\n\n1) `atomic`: the list is treated as a single entity, like a scalar.\n Atomic lists will be entirely replaced when updated. This extension\n may be used on any type of list (struct, scalar, ...).\n2) `set`:\n Sets are lists that must not have multiple items with the same value. Each\n value must be a scalar (or another atomic type).\n3) `map`:\n These lists are like maps in that their elements have a non-index key\n used to identify them. Order is preserved upon merge. The map tag\n must only be used on a list with elements of type object.\nDefaults to atomic for arrays.", + "description": "x-kubernetes-list-type annotates an array to further describe its topology. This extension must only be used on lists and may have 3 possible values:\n\n1) `atomic`: the list is treated as a single entity, like a scalar.\n Atomic lists will be entirely replaced when updated. This extension\n may be used on any type of list (struct, scalar, ...).\n2) `set`:\n Sets are lists that must not have multiple items with the same value. Each\n value must be a scalar, an object with x-kubernetes-map-type `atomic` or an\n array with x-kubernetes-list-type `atomic`.\n3) `map`:\n These lists are like maps in that their elements have a non-index key\n used to identify them. Order is preserved upon merge. The map tag\n must only be used on a list with elements of type object.\nDefaults to atomic for arrays.", "type": "string" }, "x-kubernetes-map-type": { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go index 90046cfd955..c0ac63e575d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go @@ -103,7 +103,8 @@ type JSONSchemaProps struct { // may be used on any type of list (struct, scalar, ...). // 2) `set`: // Sets are lists that must not have multiple items with the same value. Each - // value must be a scalar (or another atomic type). + // value must be a scalar, an object with x-kubernetes-map-type `atomic` or an + // array with x-kubernetes-list-type `atomic`. // 3) `map`: // These lists are like maps in that their elements have a non-index key // used to identify them. Order is preserved upon merge. The map tag diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto index a74d702f558..52bd0f7f131 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/generated.proto @@ -476,7 +476,8 @@ message JSONSchemaProps { // may be used on any type of list (struct, scalar, ...). // 2) `set`: // Sets are lists that must not have multiple items with the same value. Each - // value must be a scalar (or another atomic type). + // value must be a scalar, an object with x-kubernetes-map-type `atomic` or an + // array with x-kubernetes-list-type `atomic`. // 3) `map`: // These lists are like maps in that their elements have a non-index key // used to identify them. Order is preserved upon merge. The map tag diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go index d2c555a58d3..628c60389bc 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go @@ -109,7 +109,8 @@ type JSONSchemaProps struct { // may be used on any type of list (struct, scalar, ...). // 2) `set`: // Sets are lists that must not have multiple items with the same value. Each - // value must be a scalar (or another atomic type). + // value must be a scalar, an object with x-kubernetes-map-type `atomic` or an + // array with x-kubernetes-list-type `atomic`. // 3) `map`: // These lists are like maps in that their elements have a non-index key // used to identify them. Order is preserved upon merge. The map tag diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto index d97f0f7853a..597daf4df99 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto @@ -528,7 +528,8 @@ message JSONSchemaProps { // may be used on any type of list (struct, scalar, ...). // 2) `set`: // Sets are lists that must not have multiple items with the same value. Each - // value must be a scalar (or another atomic type). + // value must be a scalar, an object with x-kubernetes-map-type `atomic` or an + // array with x-kubernetes-list-type `atomic`. // 3) `map`: // These lists are like maps in that their elements have a non-index key // used to identify them. Order is preserved upon merge. The map tag diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go index 25997b38326..5bbf403f414 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go @@ -109,7 +109,8 @@ type JSONSchemaProps struct { // may be used on any type of list (struct, scalar, ...). // 2) `set`: // Sets are lists that must not have multiple items with the same value. Each - // value must be a scalar (or another atomic type). + // value must be a scalar, an object with x-kubernetes-map-type `atomic` or an + // array with x-kubernetes-list-type `atomic`. // 3) `map`: // These lists are like maps in that their elements have a non-index key // used to identify them. Order is preserved upon merge. The map tag 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 688f7b494a0..f570dd82a4b 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 @@ -65,6 +65,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio requireValidPropertyType: requireValidPropertyType(requestGV, nil), requireStructuralSchema: requireStructuralSchema(requestGV, nil), requirePrunedDefaults: true, + requireAtomicSetType: true, } allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata")) @@ -92,6 +93,8 @@ type validationOptions struct { requireStructuralSchema bool // requirePrunedDefaults indicates that defaults must be pruned requirePrunedDefaults bool + // requireAtomicSetType indicates that the items type for a x-kubernetes-list-type=set list must be atomic. + requireAtomicSetType bool } // ValidateCustomResourceDefinitionUpdate statically validates @@ -104,6 +107,7 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes requireValidPropertyType: requireValidPropertyType(requestGV, &oldObj.Spec), requireStructuralSchema: requireStructuralSchema(requestGV, &oldObj.Spec), requirePrunedDefaults: requirePrunedDefaults(&oldObj.Spec), + requireAtomicSetType: requireAtomicSetType(&oldObj.Spec), } allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata")) @@ -660,7 +664,7 @@ func validateCustomResourceDefinitionValidation(customResourceValidation *apiext requireValidPropertyType: opts.requireValidPropertyType, } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts)...) if opts.requireStructuralSchema { if ss, err := structuralschema.NewStructural(schema); err != nil { @@ -691,7 +695,7 @@ func validateCustomResourceDefinitionValidation(customResourceValidation *apiext var metaFields = sets.NewString("metadata", "kind", "apiVersion") // ValidateCustomResourceDefinitionOpenAPISchema statically validates -func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool) field.ErrorList { +func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions) field.ErrorList { allErrs := field.ErrorList{} if schema == nil { @@ -726,7 +730,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch // we have to forbid defaults inside additionalProperties because pruning without actual value is ambiguous subSsv = ssv.withForbiddenDefaults("inside additionalProperties applying to object metadata") } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false, opts)...) } if len(schema.Properties) != 0 { @@ -740,48 +744,48 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property)) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), subSsv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("properties").Key(property), subSsv, false, opts)...) } } - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts)...) if len(schema.AllOf) != 0 { for i, jsonSchema := range schema.AllOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("allOf").Index(i), ssv, false, opts)...) } } if len(schema.OneOf) != 0 { for i, jsonSchema := range schema.OneOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts)...) } } if len(schema.AnyOf) != 0 { for i, jsonSchema := range schema.AnyOf { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts)...) } } if len(schema.Definitions) != 0 { for definition, jsonSchema := range schema.Definitions { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts)...) } } if schema.Items != nil { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv, false, opts)...) if len(schema.Items.JSONSchemas) != 0 { for i, jsonSchema := range schema.Items.JSONSchemas { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv, false, opts)...) } } } if schema.Dependencies != nil { for dependency, jsonSchemaPropsOrStringArray := range schema.Dependencies { - allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false)...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts)...) } } @@ -791,9 +795,9 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch if schema.XMapType != nil && schema.Type != "object" { if len(schema.Type) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be object if x-kubernetes-map-type is set")) + allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be object if x-kubernetes-map-type is specified")) } else { - allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), schema.Type, "must be object if x-kubernetes-map-type is set")) + allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), schema.Type, "must be object if x-kubernetes-map-type is specified")) } } @@ -803,9 +807,21 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch if schema.XListType != nil && schema.Type != "array" { if len(schema.Type) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be array if x-kubernetes-list-type is set")) + allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be array if x-kubernetes-list-type is specified")) } else { - allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), schema.Type, "must be array if x-kubernetes-list-type is set")) + allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), schema.Type, "must be array if x-kubernetes-list-type is specified")) + } + } else if opts.requireAtomicSetType && schema.XListType != nil && *schema.XListType == "set" && schema.Items != nil && schema.Items.Schema != nil { // by structural schema items are present + is := schema.Items.Schema + switch is.Type { + case "array": + if is.XListType != nil && *is.XListType != "atomic" { // atomic is the implicit default behaviour if unset, hence != atomic is wrong + allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("x-kubernetes-list-type"), is.XListType, "must be atomic as item of a list with x-kubernetes-list-type=set")) + } + case "object": + if is.XMapType == nil || *is.XMapType != "atomic" { // granular is the implicit default behaviour if unset, hence nil and != atomic are wrong + allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("x-kubernetes-map-type"), is.XListType, "must be atomic as item of a list with x-kubernetes-list-type=set")) + } } } @@ -1055,81 +1071,91 @@ func allowDefaults(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.Cust } func specHasDefaults(spec *apiextensions.CustomResourceDefinitionSpec) bool { - if spec.Validation != nil && schemaHasDefaults(spec.Validation.OpenAPIV3Schema) { + return hasSchemaWith(spec, schemaHasDefaults) +} + +func schemaHasDefaults(s *apiextensions.JSONSchemaProps) bool { + return schemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return s.Default != nil + }) +} + +func hasSchemaWith(spec *apiextensions.CustomResourceDefinitionSpec, pred func(s *apiextensions.JSONSchemaProps) bool) bool { + if spec.Validation != nil && spec.Validation.OpenAPIV3Schema != nil && pred(spec.Validation.OpenAPIV3Schema) { return true } for _, v := range spec.Versions { - if v.Schema != nil && schemaHasDefaults(v.Schema.OpenAPIV3Schema) { + if v.Schema != nil && v.Schema.OpenAPIV3Schema != nil && pred(v.Schema.OpenAPIV3Schema) { return true } } return false } -func schemaHasDefaults(s *apiextensions.JSONSchemaProps) bool { +func schemaHas(s *apiextensions.JSONSchemaProps, pred func(s *apiextensions.JSONSchemaProps) bool) bool { if s == nil { return false } - if s.Default != nil { + if pred(s) { return true } if s.Items != nil { - if s.Items != nil && schemaHasDefaults(s.Items.Schema) { + if s.Items != nil && schemaHas(s.Items.Schema, pred) { return true } for _, s := range s.Items.JSONSchemas { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } } for _, s := range s.AllOf { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } for _, s := range s.AnyOf { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } for _, s := range s.OneOf { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } - if schemaHasDefaults(s.Not) { + if schemaHas(s.Not, pred) { return true } for _, s := range s.Properties { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } if s.AdditionalProperties != nil { - if schemaHasDefaults(s.AdditionalProperties.Schema) { + if schemaHas(s.AdditionalProperties.Schema, pred) { return true } } for _, s := range s.PatternProperties { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } if s.AdditionalItems != nil { - if schemaHasDefaults(s.AdditionalItems.Schema) { + if schemaHas(s.AdditionalItems.Schema, pred) { return true } } for _, s := range s.Definitions { - if schemaHasDefaults(&s) { + if schemaHas(&s, pred) { return true } } for _, d := range s.Dependencies { - if schemaHasDefaults(d.Schema) { + if schemaHas(d.Schema, pred) { return true } } @@ -1150,74 +1176,9 @@ func specHasKubernetesExtensions(spec *apiextensions.CustomResourceDefinitionSpe } func schemaHasKubernetesExtensions(s *apiextensions.JSONSchemaProps) bool { - if s == nil { - return false - } - - if s.XEmbeddedResource || s.XPreserveUnknownFields != nil || s.XIntOrString || len(s.XListMapKeys) > 0 || s.XListType != nil { - return true - } - - if s.Items != nil { - if s.Items != nil && schemaHasKubernetesExtensions(s.Items.Schema) { - return true - } - for _, s := range s.Items.JSONSchemas { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - } - for _, s := range s.AllOf { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - for _, s := range s.AnyOf { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - for _, s := range s.OneOf { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - if schemaHasKubernetesExtensions(s.Not) { - return true - } - for _, s := range s.Properties { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - if s.AdditionalProperties != nil { - if schemaHasKubernetesExtensions(s.AdditionalProperties.Schema) { - return true - } - } - for _, s := range s.PatternProperties { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - if s.AdditionalItems != nil { - if schemaHasKubernetesExtensions(s.AdditionalItems.Schema) { - return true - } - } - for _, s := range s.Definitions { - if schemaHasKubernetesExtensions(&s) { - return true - } - } - for _, d := range s.Dependencies { - if schemaHasKubernetesExtensions(d.Schema) { - return true - } - } - - return false + return schemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return s.XEmbeddedResource || s.XPreserveUnknownFields != nil || s.XIntOrString || len(s.XListMapKeys) > 0 || s.XListType != nil + }) } // requireStructuralSchema returns true if schemas specified must be structural @@ -1290,6 +1251,29 @@ func schemaHasUnprunedDefaults(schema *apiextensions.JSONSchemaProps) (bool, err return !reflect.DeepEqual(ss, pruned), nil } +// requireAtomicSetType returns true if the old CRD spec as at least one x-kubernetes-list-type=set with non-atomic items type. +func requireAtomicSetType(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { + return !hasSchemaWith(oldCRDSpec, hasNonAtomicSetType) +} + +// hasNonAtomicSetType recurses over the schema and returns whether any list of type "set" as non-atomic item types. +func hasNonAtomicSetType(schema *apiextensions.JSONSchemaProps) bool { + return schemaHas(schema, func(schema *apiextensions.JSONSchemaProps) bool { + if schema.XListType != nil && *schema.XListType == "set" && schema.Items != nil && schema.Items.Schema != nil { // we don't support schema.Items.JSONSchemas + is := schema.Items.Schema + switch is.Type { + case "array": + return is.XListType != nil && *is.XListType != "atomic" // atomic is the implicit default behaviour if unset, hence != atomic is wrong + case "object": + return is.XMapType == nil || *is.XMapType != "atomic" // granular is the implicit default behaviour if unset, hence nil and != atomic are wrong + default: + return false // scalar types are always atomic + } + } + return false + }) +} + // 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 { @@ -1377,72 +1361,7 @@ func specHasInvalidTypes(spec *apiextensions.CustomResourceDefinitionSpec) bool // SchemaHasInvalidTypes returns true if it contains invalid offending openapi-v3 specification. func SchemaHasInvalidTypes(s *apiextensions.JSONSchemaProps) bool { - if s == nil { - return false - } - - if len(s.Type) > 0 && !openapiV3Types.Has(s.Type) { - return true - } - - if s.Items != nil { - if s.Items != nil && SchemaHasInvalidTypes(s.Items.Schema) { - return true - } - for _, s := range s.Items.JSONSchemas { - if SchemaHasInvalidTypes(&s) { - return true - } - } - } - for _, s := range s.AllOf { - if SchemaHasInvalidTypes(&s) { - return true - } - } - for _, s := range s.AnyOf { - if SchemaHasInvalidTypes(&s) { - return true - } - } - for _, s := range s.OneOf { - if SchemaHasInvalidTypes(&s) { - return true - } - } - if SchemaHasInvalidTypes(s.Not) { - return true - } - for _, s := range s.Properties { - if SchemaHasInvalidTypes(&s) { - return true - } - } - if s.AdditionalProperties != nil { - if SchemaHasInvalidTypes(s.AdditionalProperties.Schema) { - return true - } - } - for _, s := range s.PatternProperties { - if SchemaHasInvalidTypes(&s) { - return true - } - } - if s.AdditionalItems != nil { - if SchemaHasInvalidTypes(s.AdditionalItems.Schema) { - return true - } - } - for _, s := range s.Definitions { - if SchemaHasInvalidTypes(&s) { - return true - } - } - for _, d := range s.Dependencies { - if SchemaHasInvalidTypes(d.Schema) { - return true - } - } - - return false + return schemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return len(s.Type) > 0 && !openapiV3Types.Has(s.Type) + }) } 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 3359213bfc2..9186e0b8378 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 @@ -4297,6 +4297,140 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, requestGV: apiextensionsv1.SchemeGroupVersion, }, + { + name: "non-atomic items in lists of type set allowed if pre-existing", + 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: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", // non-atomic + Properties: map[string]apiextensions.JSONSchemaProps{}, + }, + }, + }}, + }, + }, + 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{"bar": { + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", // non-atomic + Properties: map[string]apiextensions.JSONSchemaProps{}, + }, + }, + }}, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + requestGV: apiextensionsv1.SchemeGroupVersion, + }, + { + name: "reject non-atomic items in lists of type set if not pre-existing", + 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{}, + }, + }, + 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{"bar": { + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", // non-atomic + Properties: map[string]apiextensions.JSONSchemaProps{}, + }, + }, + }}, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + 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", old: &apiextensions.CustomResourceDefinition{ @@ -6300,6 +6434,173 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, wantError: false, }, + { + name: "allowed list-type atomic", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("atomic"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + wantError: false, + }, + { + name: "allowed list-type atomic with non-atomic items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("atomic"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{}, + }, + }, + }, + }, + wantError: false, + }, + { + name: "allowed list-type set with scalar items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + wantError: false, + }, + { + name: "allowed list-type set with atomic map items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XMapType: strPtr("atomic"), + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + }, + }, + }, + }, + wantError: false, + }, + { + name: "invalid list-type set with non-atomic map items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XMapType: strPtr("granular"), + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + }, + }, + }, + }, + opts: validationOptions{requireAtomicSetType: true}, + wantError: true, + }, + { + name: "invalid list-type set with unspecified map-type for map items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "foo": {Type: "string"}, + }, + }, + }, + }, + }, + opts: validationOptions{requireAtomicSetType: true}, + wantError: true, + }, + { + name: "allowed list-type set with atomic list items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("atomic"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + }, + }, + wantError: false, + }, + { + name: "allowed list-type set with unspecified list-type in list items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + }, + }, + wantError: false, + }, + { + name: "invalid list-type set with with non-atomic list items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + }, + }, + opts: validationOptions{requireAtomicSetType: true}, + wantError: true, + }, { name: "invalid type with map type extension (granular)", input: apiextensions.CustomResourceValidation{