diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/BUILD index 1ee6b4c5436..058b942d7db 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/BUILD @@ -36,6 +36,7 @@ filegroup( srcs = [ ":package-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting:all-srcs", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning:all-srcs", ], diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/BUILD new file mode 100644 index 00000000000..10339075eda --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/BUILD @@ -0,0 +1,38 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["validation.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype", + importpath = "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["validation_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation.go new file mode 100644 index 00000000000..27ba7684fb0 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation.go @@ -0,0 +1,218 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package listtype + +import ( + "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apimachinery/pkg/util/validation/field" + + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" +) + +// ValidateListSetsAndMaps validates that arrays with x-kubernetes-list-type "map" and "set" fulfill the uniqueness +// invariants for the keys (maps) and whole elements (sets). +func ValidateListSetsAndMaps(fldPath *field.Path, s *schema.Structural, obj map[string]interface{}) field.ErrorList { + if s == nil || obj == nil { + return nil + } + + var errs field.ErrorList + + if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil { + for k, v := range obj { + errs = append(errs, validationListSetAndMaps(fldPath.Key(k), s.AdditionalProperties.Structural, v)...) + } + } + if s.Properties != nil { + for k, v := range obj { + if sub, ok := s.Properties[k]; ok { + errs = append(errs, validationListSetAndMaps(fldPath.Child(k), &sub, v)...) + } + } + } + + return errs +} + +func validationListSetAndMaps(fldPath *field.Path, s *schema.Structural, obj interface{}) field.ErrorList { + switch obj := obj.(type) { + case []interface{}: + return validateListSetsAndMapsArray(fldPath, s, obj) + case map[string]interface{}: + return ValidateListSetsAndMaps(fldPath, s, obj) + } + return nil +} + +func validateListSetsAndMapsArray(fldPath *field.Path, s *schema.Structural, obj []interface{}) field.ErrorList { + var errs field.ErrorList + + if s.XListType != nil { + switch *s.XListType { + case "set": + nonUnique, err := validateListSet(fldPath, obj) + if err != nil { + errs = append(errs, err) + } else { + for _, i := range nonUnique { + errs = append(errs, field.Duplicate(fldPath.Index(i), obj[i])) + } + } + case "map": + errs = append(errs, validateListMap(fldPath, s, obj)...) + } + } + + if s.Items != nil { + for i := range obj { + errs = append(errs, validationListSetAndMaps(fldPath.Index(i), s.Items, obj[i])...) + } + } + + return errs +} + +// validateListSet validated uniqueness of unstructured objects (scalar and compound) and +// returns the first non-unique appearance of items. +// +// As a special case to distinguish undefined key and null values, we allow unspecifiedKeyValue and nullObjectValue +// which are both handled like scalars with correct comparison by Golang. +func validateListSet(fldPath *field.Path, obj []interface{}) ([]int, *field.Error) { + if len(obj) <= 1 { + return nil, nil + } + + seenScalars := make(map[interface{}]int, len(obj)) + seenCompounds := make(map[string]int, len(obj)) + var nonUniqueIndices []int + for i, x := range obj { + switch x.(type) { + case map[string]interface{}, []interface{}: + bs, err := json.Marshal(x) + if err != nil { + return nil, field.Invalid(fldPath.Index(i), x, "internal error") + } + s := string(bs) + if times, seen := seenCompounds[s]; !seen { + seenCompounds[s] = 1 + } else { + seenCompounds[s]++ + if times == 1 { + nonUniqueIndices = append(nonUniqueIndices, i) + } + } + default: + if times, seen := seenScalars[x]; !seen { + seenScalars[x] = 1 + } else { + seenScalars[x]++ + if times == 1 { + nonUniqueIndices = append(nonUniqueIndices, i) + } + } + } + } + + return nonUniqueIndices, nil +} + +func validateListMap(fldPath *field.Path, s *schema.Structural, obj []interface{}) field.ErrorList { + // only allow nil and objects + for i, x := range obj { + if _, ok := x.(map[string]interface{}); x != nil && !ok { + return field.ErrorList{field.Invalid(fldPath.Index(i), x, "must be an object for an array of list-type map")} + } + } + + if len(obj) <= 1 { + return nil + } + + // optimize simple case of one key + if len(s.XListMapKeys) == 1 { + type unspecifiedKeyValue struct{} + + keyField := s.XListMapKeys[0] + keys := make([]interface{}, 0, len(obj)) + for _, x := range obj { + if x == nil { + keys = append(keys, unspecifiedKeyValue{}) // nil object means unspecified key + continue + } + + x := x.(map[string]interface{}) + + // undefined key? + key, ok := x[keyField] + if !ok { + keys = append(keys, unspecifiedKeyValue{}) + continue + } + + keys = append(keys, key) + } + + nonUnique, err := validateListSet(fldPath, keys) + if err != nil { + return field.ErrorList{err} + } + + var errs field.ErrorList + for _, i := range nonUnique { + switch keys[i] { + case unspecifiedKeyValue{}: + errs = append(errs, field.Duplicate(fldPath.Index(i), map[string]interface{}{})) + default: + errs = append(errs, field.Duplicate(fldPath.Index(i), map[string]interface{}{keyField: keys[i]})) + } + } + + return errs + } + + // multiple key fields + keys := make([]interface{}, 0, len(obj)) + for _, x := range obj { + key := map[string]interface{}{} + if x == nil { + keys = append(keys, key) + continue + } + + x := x.(map[string]interface{}) + + for _, keyField := range s.XListMapKeys { + if k, ok := x[keyField]; ok { + key[keyField] = k + } + } + + keys = append(keys, key) + } + + nonUnique, err := validateListSet(fldPath, keys) + if err != nil { + return field.ErrorList{err} + } + + var errs field.ErrorList + for _, i := range nonUnique { + errs = append(errs, field.Duplicate(fldPath.Index(i), keys[i])) + } + + return errs +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation_test.go new file mode 100644 index 00000000000..3b3dce9385d --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation_test.go @@ -0,0 +1,940 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package listtype + +import ( + "testing" + + "k8s.io/apimachinery/pkg/util/validation/field" + + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" +) + +func TestValidateListSetsAndMaps(t *testing.T) { + tests := []struct { + name string + schema *schema.Structural + obj map[string]interface{} + errors []validationMatch + }{ + {name: "nil"}, + {name: "no schema", obj: make(map[string]interface{})}, + {name: "no object", schema: &schema.Structural{}}, + {name: "list without schema", + obj: map[string]interface{}{ + "array": []interface{}{"a", "b", "a"}, + }, + }, + {name: "list without items", + obj: map[string]interface{}{ + "array": []interface{}{"a", "b", "a"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Generic: schema.Generic{ + Type: "array", + }, + }, + }, + }, + }, + + {name: "set list with one item", + obj: map[string]interface{}{ + "array": []interface{}{"a"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Generic: schema.Generic{ + Type: "array", + }, + }, + }, + }, + }, + {name: "set list with two equal items", + obj: map[string]interface{}{ + "array": []interface{}{"a", "a"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Generic: schema.Generic{ + Type: "array", + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "array[1]"), + }, + }, + {name: "set list with two different items", + obj: map[string]interface{}{ + "array": []interface{}{"a", "b"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Generic: schema.Generic{ + Type: "array", + }, + }, + }, + }, + }, + {name: "set list with multiple duplicated items", + obj: map[string]interface{}{ + "array": []interface{}{"a", "a", "b", "c", "d", "c"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Generic: schema.Generic{ + Type: "array", + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "array[1]"), + duplicate("root", "array[5]"), + }, + }, + + {name: "normal list with items", + obj: map[string]interface{}{ + "array": []interface{}{"a", "b", "a"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + }, + {name: "set list with items", + obj: map[string]interface{}{ + "array": []interface{}{"a", "b", "a"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "array[2]"), + }, + }, + {name: "set list with items under additionalProperties", + obj: map[string]interface{}{ + "array": []interface{}{"a", "b", "a"}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root[array][2]"), + }, + }, + {name: "set list with items under items", + obj: map[string]interface{}{ + "array": []interface{}{ + []interface{}{"a", "b", "a"}, + []interface{}{"b", "b", "a"}, + }, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "array[0][2]"), + duplicate("root", "array[1][1]"), + }, + }, + + {name: "nested set lists", + obj: map[string]interface{}{ + "array": []interface{}{ + "a", "b", "a", []interface{}{"b", "b", "a"}, + }, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "array[2]"), + duplicate("root", "array[3][1]"), + }, + }, + + {name: "set list with compound map items", + obj: map[string]interface{}{ + "strings": []interface{}{"a", "b", "a"}, + "integers": []interface{}{int64(1), int64(2), int64(1)}, + "booleans": []interface{}{false, true, true}, + "float64": []interface{}{float64(1.0), float64(2.0), float64(2.0)}, + "nil": []interface{}{"a", nil, nil}, + "empty maps": []interface{}{map[string]interface{}{"a": "b"}, map[string]interface{}{}, map[string]interface{}{}}, + "map values": []interface{}{map[string]interface{}{"a": "b"}, map[string]interface{}{"a": "c"}, map[string]interface{}{"a": "b"}}, + "nil values": []interface{}{map[string]interface{}{"a": nil}, map[string]interface{}{"b": "c", "a": nil}}, + "array": []interface{}{[]interface{}{}, []interface{}{"a"}, []interface{}{"b"}, []interface{}{"a"}}, + "nil array": []interface{}{[]interface{}{}, []interface{}{nil}, []interface{}{nil, nil}, []interface{}{nil}, []interface{}{"a"}}, + "multiple duplicates": []interface{}{map[string]interface{}{"a": "b"}, map[string]interface{}{"a": "c"}, map[string]interface{}{"a": "b"}, map[string]interface{}{"a": "c"}, map[string]interface{}{"a": "c"}}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "strings": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "integers": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "integer", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "booleans": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "boolean", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "float64": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "number", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "nil": { + Generic: schema.Generic{ + Type: "array", + }, Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + Nullable: true, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "empty maps": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "map values": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "nil values": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + Nullable: true, + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "nil array": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + Nullable: true, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + "multiple duplicates": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "strings[2]"), + duplicate("root", "integers[2]"), + duplicate("root", "booleans[2]"), + duplicate("root", "float64[2]"), + duplicate("root", "nil[2]"), + duplicate("root", "empty maps[2]"), + duplicate("root", "map values[2]"), + duplicate("root", "array[3]"), + duplicate("root", "nil array[3]"), + duplicate("root", "multiple duplicates[2]"), + duplicate("root", "multiple duplicates[3]"), + }, + }, + {name: "set list with compound array items", + obj: map[string]interface{}{ + "array": []interface{}{[]interface{}{}, []interface{}{"a"}, []interface{}{"a"}}, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Extensions: schema.Extensions{ + XListType: strPtr("set"), + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + errors: []validationMatch{ + duplicate("root", "array[2]"), + }, + }, + + {name: "map list with compound map items", + obj: map[string]interface{}{ + "strings": []interface{}{"a"}, + "integers": []interface{}{int64(1)}, + "booleans": []interface{}{false}, + "float64": []interface{}{float64(1.0)}, + "nil": []interface{}{nil}, + "array": []interface{}{[]interface{}{"a"}}, + "one key": []interface{}{map[string]interface{}{"a": "0", "c": "2"}, map[string]interface{}{"a": "1", "c": "1"}, map[string]interface{}{"a": "1", "c": "2"}, map[string]interface{}{}}, + "two keys": []interface{}{map[string]interface{}{"a": "1", "b": "1", "c": "1"}, map[string]interface{}{"a": "1", "b": "2", "c": "2"}, map[string]interface{}{"a": "1", "b": "2", "c": "3"}, map[string]interface{}{}}, + "undefined key": []interface{}{map[string]interface{}{"a": "1", "b": "1", "c": "1"}, map[string]interface{}{"a": "1", "c": "2"}, map[string]interface{}{"a": "1", "c": "3"}, map[string]interface{}{}}, + "compound key": []interface{}{map[string]interface{}{"a": []interface{}{}, "c": "1"}, map[string]interface{}{"a": nil, "c": "1"}, map[string]interface{}{"a": []interface{}{"a"}, "c": "1"}, map[string]interface{}{"a": []interface{}{"a", int64(42)}, "c": "2"}, map[string]interface{}{"a": []interface{}{"a", int64(42)}, "c": []interface{}{"3"}}}, + "nil key": []interface{}{map[string]interface{}{"a": []interface{}{}, "c": "1"}, map[string]interface{}{"a": nil, "c": "1"}, map[string]interface{}{"c": "1"}, map[string]interface{}{"a": nil}}, + "nil item": []interface{}{nil, map[string]interface{}{"a": "0", "c": "1"}, map[string]interface{}{"a": nil}, map[string]interface{}{"c": "1"}}, + "nil item multiple keys": []interface{}{nil, map[string]interface{}{"b": "0", "c": "1"}, map[string]interface{}{"a": nil}, map[string]interface{}{"c": "1"}}, + "multiple duplicates": []interface{}{ + map[string]interface{}{"a": []interface{}{}, "c": "1"}, + map[string]interface{}{"a": nil, "c": "1"}, + map[string]interface{}{"a": []interface{}{"a"}, "c": "1"}, + map[string]interface{}{"a": []interface{}{"a", int64(42)}, "c": "2"}, + map[string]interface{}{"a": []interface{}{"a", int64(42)}, "c": []interface{}{"3"}}, + map[string]interface{}{"a": []interface{}{"a"}, "c": "1", "d": "1"}, + map[string]interface{}{"a": []interface{}{"a"}, "c": "1", "d": "2"}, + }, + }, + schema: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "strings": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "integers": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "integer", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "booleans": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "boolean", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "float64": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "number", + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "nil": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + Nullable: true, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "array": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "one key": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "two keys": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a", "b"}, + }, + }, + "undefined key": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a", "b"}, + }, + }, + "compound key": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + Nullable: true, + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "nil key": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "a": { + Generic: schema.Generic{ + Type: "array", + Nullable: true, + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + "c": { + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "nil item": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "a": { + Generic: schema.Generic{ + Type: "array", + Nullable: true, + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + "c": { + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + "nil item multiple keys": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "a": { + Generic: schema.Generic{ + Type: "array", + Nullable: true, + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + "b": { + Generic: schema.Generic{ + Type: "string", + }, + }, + "c": { + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a", "b"}, + }, + }, + "multiple duplicates": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + Nullable: true, + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XListType: strPtr("map"), + XListMapKeys: []string{"a"}, + }, + }, + }, + }, + errors: []validationMatch{ + invalid("root", "strings[0]"), + invalid("root", "integers[0]"), + invalid("root", "booleans[0]"), + invalid("root", "float64[0]"), + invalid("root", "array[0]"), + duplicate("root", "one key[2]"), + duplicate("root", "two keys[2]"), + duplicate("root", "undefined key[2]"), + duplicate("root", "compound key[4]"), + duplicate("root", "nil key[3]"), + duplicate("root", "nil item[3]"), + duplicate("root", "nil item multiple keys[3]"), + duplicate("root", "multiple duplicates[4]"), + duplicate("root", "multiple duplicates[5]"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := ValidateListSetsAndMaps(field.NewPath("root"), tt.schema, tt.obj) + + seenErrs := make([]bool, len(errs)) + + for _, expectedError := range tt.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) + } + } + + for i, seen := range seenErrs { + if !seen { + t.Errorf("unexpected error: %v", errs[i]) + } + } + }) + } +} + +type validationMatch struct { + path *field.Path + errorType field.ErrorType +} + +func (v validationMatch) matches(err *field.Error) bool { + return err.Type == v.errorType && err.Field == v.path.String() +} + +func duplicate(path ...string) validationMatch { + return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeDuplicate} +} +func invalid(path ...string) validationMatch { + return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid} +} + +func strPtr(s string) *string { return &s } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/BUILD index ca597e1b0fa..16c05a4683b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/BUILD @@ -20,6 +20,7 @@ go_library( "//staging/src/k8s.io/api/autoscaling/v1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go index 886ca393ba6..9cf040b9de7 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go @@ -35,6 +35,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + structurallisttype "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype" schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta" ) @@ -43,14 +44,14 @@ type customResourceStrategy struct { runtime.ObjectTyper names.NameGenerator - namespaceScoped bool - validator customResourceValidator - schemas map[string]*structuralschema.Structural - status *apiextensions.CustomResourceSubresourceStatus - scale *apiextensions.CustomResourceSubresourceScale + namespaceScoped bool + validator customResourceValidator + structuralSchemas map[string]*structuralschema.Structural + status *apiextensions.CustomResourceSubresourceStatus + scale *apiextensions.CustomResourceSubresourceScale } -func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator *validate.SchemaValidator, schemas map[string]*structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy { +func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator *validate.SchemaValidator, structuralSchemas map[string]*structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy { return customResourceStrategy{ ObjectTyper: typer, NameGenerator: names.SimpleNameGenerator, @@ -63,7 +64,7 @@ func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.Gr schemaValidator: schemaValidator, statusSchemaValidator: statusSchemaValidator, }, - schemas: schemas, + structuralSchemas: structuralSchemas, } } @@ -137,7 +138,10 @@ func (a customResourceStrategy) Validate(ctx context.Context, obj runtime.Object // validate embedded resources if u, ok := obj.(*unstructured.Unstructured); ok { v := obj.GetObjectKind().GroupVersionKind().Version - errs = append(errs, schemaobjectmeta.Validate(nil, u.Object, a.schemas[v], false)...) + errs = append(errs, schemaobjectmeta.Validate(nil, u.Object, a.structuralSchemas[v], false)...) + + // validate x-kubernetes-list-type "map" and "set" invariant + errs = append(errs, structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], u.Object)...) } return errs @@ -163,10 +167,22 @@ func (a customResourceStrategy) ValidateUpdate(ctx context.Context, obj, old run var errs field.ErrorList errs = append(errs, a.validator.ValidateUpdate(ctx, obj, old, a.scale)...) + uNew, ok := obj.(*unstructured.Unstructured) + if !ok { + return errs + } + uOld, ok := old.(*unstructured.Unstructured) + if !ok { + return errs + } + // Checks the embedded objects. We don't make a difference between update and create for those. - if u, ok := obj.(*unstructured.Unstructured); ok { - v := obj.GetObjectKind().GroupVersionKind().Version - errs = append(errs, schemaobjectmeta.Validate(nil, u.Object, a.schemas[v], false)...) + v := obj.GetObjectKind().GroupVersionKind().Version + errs = append(errs, schemaobjectmeta.Validate(nil, uNew.Object, a.structuralSchemas[v], false)...) + + // ratcheting validation of x-kubernetes-list-type value map and set + if oldErrs := structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], uOld.Object); len(oldErrs) == 0 { + errs = append(errs, structurallisttype.ValidateListSetsAndMaps(nil, a.structuralSchemas[v], uNew.Object)...) } return errs diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD index 100480ae76a..a9d5a368d7e 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD @@ -16,6 +16,7 @@ go_test( "defaulting_test.go", "finalization_test.go", "limit_test.go", + "listtype_test.go", "objectmeta_test.go", "pruning_test.go", "registration_test.go", @@ -56,6 +57,7 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", + "//staging/src/k8s.io/client-go/util/retry:go_default_library", "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", 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 new file mode 100644 index 00000000000..e172eef5107 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/listtype_test.go @@ -0,0 +1,225 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "context" + "strings" + "testing" + "time" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/yaml" + + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var listTypeResourceFixture = &apiextensionsv1beta1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.tests.example.com"}, + Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ + Group: "tests.example.com", + Versions: []apiextensionsv1beta1.CustomResourceDefinitionVersion{ + { + Name: "v1beta1", + Storage: true, + Served: true, + }, + }, + Names: apiextensionsv1beta1.CustomResourceDefinitionNames{ + Plural: "foos", + Singular: "foo", + Kind: "Foo", + ListKind: "FooList", + }, + Scope: apiextensionsv1beta1.ClusterScoped, + Validation: &apiextensionsv1beta1.CustomResourceValidation{}, + }, +} + +const ( + // structural schema because x-kubernetes-list-type is only allowed for those + listTypeResourceSchema = ` +type: object +properties: + correct-map: + type: array + x-kubernetes-list-type: map + x-kubernetes-list-map-keys: ["a", "b"] + items: + type: object + properties: + a: + type: integer + b: + type: integer + correct-set: + type: array + x-kubernetes-list-type: set + items: + type: object + x-kubernetes-map-type: atomic + additionalProperties: + type: integer + invalid-map: + type: array + x-kubernetes-list-type: map + x-kubernetes-list-map-keys: ["a", "b"] + items: + type: object + properties: + a: + type: integer + b: + type: integer + invalid-set: + type: array + x-kubernetes-list-type: set + items: + type: object + x-kubernetes-map-type: atomic + additionalProperties: + type: integer +` + + listTypeResourceInstance = ` +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-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-set: [{"a":1,"b":1},{"a":1,"b":2},{"a":1},{"a":1,"b":4},{"a":1,"b":1}] +` +) + +var ( + validListTypeFields = []string{"correct-map", "correct-set"} + invalidListTypeFields = []string{"invalid-map", "invalid-set"} +) + +func TestListTypes(t *testing.T) { + tearDownFn, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t) + if err != nil { + t.Fatal(err) + } + defer tearDownFn() + + crd := listTypeResourceFixture.DeepCopy() + if err := yaml.Unmarshal([]byte(listTypeResourceSchema), &crd.Spec.Validation.OpenAPIV3Schema); err != nil { + t.Fatal(err) + } + + crd, err = fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + + t.Logf("Creating CR and expect list-type errors") + fooClient := dynamicClient.Resource(schema.GroupVersionResource{crd.Spec.Group, crd.Spec.Versions[0].Name, crd.Spec.Names.Plural}) + invalidInstance := &unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(listTypeResourceInstance), &invalidInstance.Object); err != nil { + t.Fatal(err) + } + _, createErr := fooClient.Create(invalidInstance, metav1.CreateOptions{}) + if createErr == nil { + t.Fatalf("Expected validation errors, but did not get one") + } + + t.Logf("Checking that valid fields DO NOT show in error") + for _, valid := range validListTypeFields { + if strings.Contains(createErr.Error(), valid) { + t.Errorf("unexpected error about %q: %v", valid, err) + } + } + + t.Logf("Checking that invalid fields DO show in error") + for _, invalid := range invalidListTypeFields { + if !strings.Contains(createErr.Error(), invalid) { + t.Errorf("expected %q to show up in the error, but didn't: %v", invalid, err) + } + } + + t.Logf("Creating fixed CR") + validInstance := &unstructured.Unstructured{} + if err := yaml.Unmarshal([]byte(listTypeResourceInstance), &validInstance.Object); err != nil { + t.Fatal(err) + } + for _, invalid := range invalidListTypeFields { + unstructured.RemoveNestedField(validInstance.Object, invalid) + } + validInstance, err = fooClient.Create(validInstance, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + t.Logf("Updating with invalid values and expecting errors") + modifiedInstance := validInstance.DeepCopy() + for _, valid := range validListTypeFields { + x := modifiedInstance.Object[valid] + l := x.([]interface{}) + l = append(l, l[0]) + modifiedInstance.Object[valid] = l + } + _, err = fooClient.Update(modifiedInstance, metav1.UpdateOptions{}) + if err == nil { + t.Fatalf("Expected validation errors, but did not get one") + } + for _, valid := range validListTypeFields { + if !strings.Contains(err.Error(), valid) { + t.Errorf("expected %q to show up in the error, but didn't: %v", valid, err) + } + } + + t.Logf("Remove \"b\" from the keys in the schema which renders the valid instance invalid") + err = retry.RetryOnConflict(retry.DefaultBackoff, func() error { + crd, err := apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(context.TODO(), crd.Name, metav1.GetOptions{}) + if err != nil { + return err + } + s := crd.Spec.Validation.OpenAPIV3Schema.Properties["correct-map"] + s.XListMapKeys = []string{"a"} + crd.Spec.Validation.OpenAPIV3Schema.Properties["correct-map"] = s + _, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{}) + return err + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + t.Logf("Updating again with invalid values, eventually successfully due to ratcheting logic") + err = wait.PollImmediate(time.Millisecond*100, wait.ForeverTestTimeout, func() (bool, error) { + _, err = fooClient.Update(modifiedInstance, metav1.UpdateOptions{}) + if err == nil { + return true, err + } + if errors.IsInvalid(err) { + // wait until modifiedInstance becomes valid again + return false, nil + } + return false, err + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 2a65c64d201..2e3a91a233b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1168,6 +1168,7 @@ k8s.io/apiextensions-apiserver/pkg/apiserver k8s.io/apiextensions-apiserver/pkg/apiserver/conversion k8s.io/apiextensions-apiserver/pkg/apiserver/schema k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting +k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning k8s.io/apiextensions-apiserver/pkg/apiserver/validation