From 2e187415fd582ea80bc316fb76cb30c3ad1aead4 Mon Sep 17 00:00:00 2001 From: Elijah Oyekunle Date: Fri, 28 Feb 2020 16:00:59 +0100 Subject: [PATCH] extend CRD map and set validation --- .../apiextensions/validation/validation.go | 68 ++ .../validation/validation_test.go | 710 ++++++++++++++++++ .../test/integration/listtype_test.go | 6 +- .../apiserver/apply/apply_crd_test.go | 6 +- 4 files changed, 785 insertions(+), 5 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 cf7c9ba1e0a..82e4e9d15dd 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 @@ -64,6 +64,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio requireStructuralSchema: requireStructuralSchema(requestGV, nil), requirePrunedDefaults: true, requireAtomicSetType: true, + requireMapListKeysMapSetValidation: true, } allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata")) @@ -93,6 +94,10 @@ type validationOptions struct { requirePrunedDefaults bool // requireAtomicSetType indicates that the items type for a x-kubernetes-list-type=set list must be atomic. requireAtomicSetType bool + // requireMapListKeysMapSetValidation indicates that: + // 1. For x-kubernetes-list-type=map list, key fields are not nullable, and are required or have a default + // 2. For x-kubernetes-list-type=map or x-kubernetes-list-type=set list, the whole item must not be nullable. + requireMapListKeysMapSetValidation bool } // ValidateCustomResourceDefinitionUpdate statically validates @@ -106,6 +111,7 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes requireStructuralSchema: requireStructuralSchema(requestGV, &oldObj.Spec), requirePrunedDefaults: requirePrunedDefaults(&oldObj.Spec), requireAtomicSetType: requireAtomicSetType(&oldObj.Spec), + requireMapListKeysMapSetValidation: requireMapListKeysMapSetValidation(&oldObj.Spec), } allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata")) @@ -866,6 +872,58 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch } } + if opts.requireMapListKeysMapSetValidation { + allErrs = append(allErrs, validateMapListKeysMapSet(schema, fldPath)...) + } + + return allErrs +} + +func validateMapListKeysMapSet(schema *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if schema.Items == nil || schema.Items.Schema == nil { + return nil + } + if schema.XListType == nil { + return nil + } + if *schema.XListType != "set" && *schema.XListType != "map" { + return nil + } + + // set and map list items cannot be nullable + if schema.Items.Schema.Nullable { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("items").Child("nullable"), "cannot be nullable when x-kubernetes-list-type is "+*schema.XListType)) + } + + switch *schema.XListType { + case "map": + // ensure all map keys are required or have a default + isRequired := make(map[string]bool, len(schema.Items.Schema.Required)) + for _, required := range schema.Items.Schema.Required { + isRequired[required] = true + } + + for _, k := range schema.XListMapKeys { + obj, ok := schema.Items.Schema.Properties[k] + if !ok { + // we validate that all XListMapKeys are existing properties in ValidateCustomResourceDefinitionOpenAPISchema, so skipping here is ok + continue + } + + if isRequired[k] == false && obj.Default == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("items").Child("properties").Key(k).Child("default"), "this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property")) + } + + if obj.Nullable { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("items").Child("properties").Key(k).Child("nullable"), "this property is in x-kubernetes-list-map-keys, so it cannot be nullable")) + } + } + case "set": + // no other set-specific validation + } + return allErrs } @@ -1268,6 +1326,16 @@ func hasNonAtomicSetType(schema *apiextensions.JSONSchemaProps) bool { }) } +func requireMapListKeysMapSetValidation(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { + return !hasSchemaWith(oldCRDSpec, hasInvalidMapListKeysMapSet) +} + +func hasInvalidMapListKeysMapSet(schema *apiextensions.JSONSchemaProps) bool { + return schemaHas(schema, func(schema *apiextensions.JSONSchemaProps) bool { + return len(validateMapListKeysMapSet(schema, field.NewPath(""))) > 0 + }) +} + // 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 { 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 0a6dee6c7ca..d34cd045d59 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 @@ -6004,6 +6004,520 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { invalid("spec", "preserveUnknownFields"), }, }, + { + name: "allow non-required key with no default in list of type map 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: nil, + }, + { + name: "reject non-required key with no default in list of type map 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{"foo": { + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + Default: jsonPtr("stuff"), + }, + }, + }, + }, + }}, + }, + }, + }, + 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: []validationMatch{ + required("spec", "validation", "openAPIV3Schema", "properties[bar]", "items", "properties[key]", "default"), + }, + }, + { + name: "allow nullable key in list of type map 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + Nullable: 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{"bar": { + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + Nullable: true, + }, + }, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: nil, + }, + { + name: "reject nullable key in list of type map 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{"foo": { + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + Nullable: true, + }, + }, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[bar]", "items", "properties[key]", "nullable"), + }, + }, + { + name: "allow nullable item in list of type map 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Nullable: true, + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Nullable: true, + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: nil, + }, + { + name: "reject nullable item in list of type map 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{"foo": { + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + 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("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Nullable: true, + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[bar]", "items", "nullable"), + }, + }, + { + name: "allow nullable items in list of type set 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: "string", + Nullable: 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{"bar": { + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + Nullable: true, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: nil, + }, + { + name: "reject nullable items in list of type set if not pre-exisiting", + 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: "string", + }, + }, + }}, + }, + }, + }, + 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: "string", + Nullable: true, + }, + }, + }}, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}}, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[bar]", "items", "nullable"), + }, + }, } for _, tc := range tests { @@ -6632,6 +7146,202 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, wantError: false, }, + { + name: "invalid map with non-required key and no default", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: true, + }, + { + name: "allowed map with required key and no default", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"key"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: false, + }, + { + name: "allowed map with non-required key and default", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + Default: jsonPtr("stuff"), + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + allowDefaults: true, + requireMapListKeysMapSetValidation: true, + }, + wantError: false, + }, + { + name: "invalid map with nullable key", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + Nullable: true, + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: true, + }, + { + name: "invalid map with nullable items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"key"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Nullable: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: true, + }, + { + name: "valid map with some required, some defaulted, and non-key fields", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Required: []string{"a", "c"}, + Properties: map[string]apiextensions.JSONSchemaProps{ + "key": { + Type: "string", + }, + "a": { + Type: "string", + }, + "b": { + Type: "string", + Default: jsonPtr("stuff"), + }, + "c": { + Type: "int", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: true, + }, + { + name: "invalid set with nullable items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Nullable: true, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: true, + }, + { + name: "allowed set with non-nullable items", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XListType: strPtr("set"), + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Nullable: false, + }, + }, + }, + }, + opts: validationOptions{ + requireMapListKeysMapSetValidation: true, + }, + wantError: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/listtype_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/listtype_test.go index e172eef5107..5d9c9a54a86 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/listtype_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/listtype_test.go @@ -67,6 +67,7 @@ properties: x-kubernetes-list-map-keys: ["a", "b"] items: type: object + required: ["a", "b"] properties: a: type: integer @@ -86,6 +87,7 @@ properties: x-kubernetes-list-map-keys: ["a", "b"] items: type: object + required: ["a", "b"] properties: a: type: integer @@ -106,9 +108,9 @@ kind: Foo apiVersion: tests.example.com/v1beta1 metadata: name: foo -correct-map: [{"a":1,"b":1,c:"1"},{"a":1,"b":2,c:"2"},{"a":1,c:"3"}] +correct-map: [{"a":1,"b":1,c:"1"},{"a":1,"b":2,c:"2"},{"a":1,"b":3,c:"3"}] correct-set: [{"a":1,"b":1},{"a":1,"b":2},{"a":1},{"a":1,"b":4}] -invalid-map: [{"a":1,"b":1,c:"1"},{"a":1,"b":2,c:"2"},{"a":1,c:"3"},{"a":1,"b":1,c:"4"}] +invalid-map: [{"a":1,"b":1,c:"1"},{"a":1,"b":2,c:"2"},{"a":1,"b":3,c:"3"},{"a":1,"b":1,c:"4"}] invalid-set: [{"a":1,"b":1},{"a":1,"b":2},{"a":1},{"a":1,"b":4},{"a":1,"b":1}] ` ) diff --git a/test/integration/apiserver/apply/apply_crd_test.go b/test/integration/apiserver/apply/apply_crd_test.go index 04597f1c177..cb91612e0af 100644 --- a/test/integration/apiserver/apply/apply_crd_test.go +++ b/test/integration/apiserver/apply/apply_crd_test.go @@ -191,12 +191,12 @@ func TestApplyCRDStructuralSchema(t *testing.T) { "type": "string" }, "protocol": { - "type": "string", - "nullable": true + "type": "string" } }, "required": [ - "containerPort" + "containerPort", + "protocol" ], "type": "object" }