diff --git a/go.sum b/go.sum index bde0aebc..a31489b9 100644 --- a/go.sum +++ b/go.sum @@ -17,4 +17,3 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -sigs.k8s.io/structured-merge-diff v1.0.2 h1:WiMoyniAVAYm03w+ImfF9IE2G23GLR/SwDnQyaNZvPk= diff --git a/merge/default_keys_test.go b/merge/default_keys_test.go new file mode 100644 index 00000000..0f595f8b --- /dev/null +++ b/merge/default_keys_test.go @@ -0,0 +1,334 @@ +/* +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 merge_test + +import ( + "testing" + + "sigs.k8s.io/structured-merge-diff/v4/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v4/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v4/merge" + "sigs.k8s.io/structured-merge-diff/v4/typed" +) + +// portListParser sets the default value of key "protocol" to "TCP" +var portListParser = func() *typed.Parser { + parser, err := typed.NewParser(`types: +- name: v1 + map: + fields: + - name: containerPorts + type: + list: + elementType: + map: + fields: + - name: port + type: + scalar: numeric + - name: protocol + default: "TCP" + type: + scalar: string + - name: name + type: + scalar: string + elementRelationship: associative + keys: + - port + - protocol +`) + if err != nil { + panic(err) + } + return parser +}() + +func TestDefaultKeysFlat(t *testing.T) { + tests := map[string]TestCase{ + "apply_missing_defaulted_key_A": { + Ops: []Operation{ + Apply{ + Manager: "default", + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + `, + }, + }, + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + `, + Managed: fieldpath.ManagedFields{ + "default": fieldpath.NewVersionedSet( + _NS( + _P("containerPorts", _KBF("port", 80, "protocol", "TCP")), + _P("containerPorts", _KBF("port", 80, "protocol", "TCP"), "port"), + ), + "v1", + false, + ), + }, + }, + "apply_missing_defaulted_key_B": { + Ops: []Operation{ + Apply{ + Manager: "default", + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + - port: 80 + protocol: UDP + `, + }, + }, + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + - port: 80 + protocol: UDP + `, + Managed: fieldpath.ManagedFields{ + "default": fieldpath.NewVersionedSet( + _NS( + _P("containerPorts", _KBF("port", 80, "protocol", "TCP")), + _P("containerPorts", _KBF("port", 80, "protocol", "TCP"), "port"), + _P("containerPorts", _KBF("port", 80, "protocol", "UDP")), + _P("containerPorts", _KBF("port", 80, "protocol", "UDP"), "port"), + _P("containerPorts", _KBF("port", 80, "protocol", "UDP"), "protocol"), + ), + "v1", + false, + ), + }, + }, + "apply_missing_defaulted_key_with_conflict": { + Ops: []Operation{ + Apply{ + Manager: "apply-one", + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + protocol: TCP + name: foo + `, + }, + Apply{ + Manager: "apply-two", + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + name: bar + `, + Conflicts: merge.Conflicts{ + merge.Conflict{Manager: "apply-one", Path: _P("containerPorts", _KBF("port", 80, "protocol", "TCP"), "name")}, + }, + }, + }, + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + protocol: TCP + name: foo + `, + Managed: fieldpath.ManagedFields{ + "apply-one": fieldpath.NewVersionedSet( + _NS( + _P("containerPorts", _KBF("port", 80, "protocol", "TCP")), + _P("containerPorts", _KBF("port", 80, "protocol", "TCP"), "port"), + _P("containerPorts", _KBF("port", 80, "protocol", "TCP"), "protocol"), + _P("containerPorts", _KBF("port", 80, "protocol", "TCP"), "name"), + ), + "v1", + false, + ), + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if err := test.Test(portListParser); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestDefaultKeysFlatErrors(t *testing.T) { + tests := map[string]TestCase{ + "apply_missing_undefaulted_defaulted_key": { + Ops: []Operation{ + Apply{ + Manager: "default", + APIVersion: "v1", + Object: ` + containerPorts: + - protocol: TCP + `, + }, + }, + }, + "apply_missing_defaulted_key_ambiguous_A": { + Ops: []Operation{ + Apply{ + Manager: "default", + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + - port: 80 + `, + }, + }, + }, + "apply_missing_defaulted_key_ambiguous_B": { + Ops: []Operation{ + Apply{ + Manager: "default", + APIVersion: "v1", + Object: ` + containerPorts: + - port: 80 + - port: 80 + protocol: TCP + `, + }, + }, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if test.Test(portListParser) == nil { + t.Fatal("Should fail") + } + }) + } +} + +// bookParser sets the default value of key: +// * "chapter" to 1 +// * "section" to "A" +// * "page" to 2, +// * "line" to 3, +var bookParser = func() *typed.Parser { + parser, err := typed.NewParser(`types: +- name: v1 + map: + fields: + - name: book + type: + list: + elementType: + map: + fields: + - name: chapter + default: 1 + type: + scalar: numeric + - name: section + default: "A" + type: + scalar: string + - name: sentences + type: + list: + elementType: + map: + fields: + - name: page + default: 2 + type: + scalar: numeric + - name: line + default: 3 + type: + scalar: numeric + - name: text + type: + scalar: string + elementRelationship: associative + keys: + - page + - line + elementRelationship: associative + keys: + - chapter + - section +`) + if err != nil { + panic(err) + } + return parser +}() + +func TestDefaultKeysNested(t *testing.T) { + tests := map[string]TestCase{ + "apply_missing_every_key_nested": { + Ops: []Operation{ + Apply{ + Manager: "default", + APIVersion: "v1", + Object: ` + book: + - sentences: + - text: blah + `, + }, + }, + APIVersion: "v1", + Object: ` + book: + - sentences: + - text: blah + `, + Managed: fieldpath.ManagedFields{ + "default": fieldpath.NewVersionedSet( + _NS( + _P( + "book", _KBF("chapter", 1, "section", "A"), + ), + _P( + "book", _KBF("chapter", 1, "section", "A"), + "sentences", _KBF("page", 2, "line", 3), + ), + _P( + "book", _KBF("chapter", 1, "section", "A"), + "sentences", _KBF("page", 2, "line", 3), + "text", + ), + ), + "v1", + false, + ), + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if err := test.Test(bookParser); err != nil { + t.Fatal(err) + } + }) + } +} diff --git a/schema/elements.go b/schema/elements.go index 5a2859e4..01103b38 100644 --- a/schema/elements.go +++ b/schema/elements.go @@ -196,6 +196,8 @@ type StructField struct { Name string `yaml:"name,omitempty"` // Type is the field type. Type TypeRef `yaml:"type,omitempty"` + // Default value for the field, nil if not present. + Default interface{} `yaml:"default,omitempty"` } // List represents a type which contains a zero or more elements, all of the diff --git a/schema/equals.go b/schema/equals.go index 464d47de..4c303eec 100644 --- a/schema/equals.go +++ b/schema/equals.go @@ -16,6 +16,8 @@ limitations under the License. package schema +import "reflect" + // Equals returns true iff the two Schemas are equal. func (a *Schema) Equals(b *Schema) bool { if a == nil || b == nil { @@ -168,6 +170,9 @@ func (a *StructField) Equals(b *StructField) bool { if a.Name != b.Name { return false } + if !reflect.DeepEqual(a.Default, b.Default) { + return false + } return a.Type.Equals(&b.Type) } diff --git a/schema/equals_test.go b/schema/equals_test.go index a496ea3b..fff292c2 100644 --- a/schema/equals_test.go +++ b/schema/equals_test.go @@ -25,6 +25,12 @@ import ( fuzz "github.com/google/gofuzz" ) +func fuzzInterface(i *interface{}, c fuzz.Continue) { + m := map[string]string{} + c.Fuzz(&m) + *i = &m +} + func (Schema) Generate(rand *rand.Rand, size int) reflect.Value { s := Schema{} f := fuzz.New().RandSource(rand).MaxDepth(4) @@ -34,7 +40,7 @@ func (Schema) Generate(rand *rand.Rand, size int) reflect.Value { func (Map) Generate(rand *rand.Rand, size int) reflect.Value { m := Map{} - f := fuzz.New().RandSource(rand).MaxDepth(4) + f := fuzz.New().RandSource(rand).MaxDepth(4).Funcs(fuzzInterface) f.Fuzz(&m) return reflect.ValueOf(m) } @@ -53,6 +59,13 @@ func (Atom) Generate(rand *rand.Rand, size int) reflect.Value { return reflect.ValueOf(a) } +func (StructField) Generate(rand *rand.Rand, size int) reflect.Value { + a := StructField{} + f := fuzz.New().RandSource(rand).MaxDepth(4).Funcs(fuzzInterface) + f.Fuzz(&a) + return reflect.ValueOf(a) +} + func TestEquals(t *testing.T) { // In general this test will make sure people update things when they // add a field. @@ -133,6 +146,7 @@ func TestEquals(t *testing.T) { var y StructField y.Name = x.Name y.Type = x.Type + y.Default = x.Default return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x List) bool { diff --git a/schema/schemaschema.go b/schema/schemaschema.go index c76a99b8..bb60e2a5 100644 --- a/schema/schemaschema.go +++ b/schema/schemaschema.go @@ -125,6 +125,9 @@ var SchemaSchemaYAML = `types: - name: type type: namedType: typeRef + - name: default + type: + namedType: __untyped_atomic_ - name: list map: fields: @@ -145,4 +148,14 @@ var SchemaSchemaYAML = `types: - name: elementRelationship type: scalar: string +- name: __untyped_atomic_ + scalar: untyped + list: + elementType: + namedType: __untyped_atomic_ + elementRelationship: atomic + map: + elementType: + namedType: __untyped_atomic_ + elementRelationship: atomic ` diff --git a/typed/helpers.go b/typed/helpers.go index 9b6845e8..6b2b2cb4 100644 --- a/typed/helpers.go +++ b/typed/helpers.go @@ -180,11 +180,23 @@ func mapValue(a value.Allocator, val value.Value) (value.Map, error) { return val.AsMapUsing(a), nil } -func keyedAssociativeListItemToPathElement(a value.Allocator, list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) { +func getAssociativeKeyDefault(s *schema.Schema, list *schema.List, fieldName string) (interface{}, error) { + atom, ok := s.Resolve(list.ElementType) + if !ok { + return nil, errors.New("invalid elementType for list") + } + if atom.Map == nil { + return nil, errors.New("associative list may not have non-map types") + } + // If the field is not found, we can assume there is no default. + field, _ := atom.Map.FindField(fieldName) + return field.Default, nil +} + +func keyedAssociativeListItemToPathElement(a value.Allocator, s *schema.Schema, list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) { pe := fieldpath.PathElement{} if child.IsNull() { - // For now, the keys are required which means that null entries - // are illegal. + // null entries are illegal. return pe, errors.New("associative list with keys may not have a null element") } if !child.IsMap() { @@ -196,8 +208,12 @@ func keyedAssociativeListItemToPathElement(a value.Allocator, list *schema.List, for _, fieldName := range list.Keys { if val, ok := m.Get(fieldName); ok { keyMap = append(keyMap, value.Field{Name: fieldName, Value: val}) + } else if def, err := getAssociativeKeyDefault(s, list, fieldName); err != nil { + return pe, fmt.Errorf("couldn't find default value for %v: %v", fieldName, err) + } else if def != nil { + keyMap = append(keyMap, value.Field{Name: fieldName, Value: value.NewValueInterface(def)}) } else { - return pe, fmt.Errorf("associative list with keys has an element that omits key field %q", fieldName) + return pe, fmt.Errorf("associative list with keys has an element that omits key field %q (and doesn't have default value)", fieldName) } } keyMap.Sort() @@ -225,10 +241,10 @@ func setItemToPathElement(list *schema.List, index int, child value.Value) (fiel } } -func listItemToPathElement(a value.Allocator, list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) { +func listItemToPathElement(a value.Allocator, s *schema.Schema, list *schema.List, index int, child value.Value) (fieldpath.PathElement, error) { if list.ElementRelationship == schema.Associative { if len(list.Keys) > 0 { - return keyedAssociativeListItemToPathElement(a, list, index, child) + return keyedAssociativeListItemToPathElement(a, s, list, index, child) } // If there's no keys, then we must be a set of primitives. diff --git a/typed/merge.go b/typed/merge.go index 5112e253..7e20f408 100644 --- a/typed/merge.go +++ b/typed/merge.go @@ -183,7 +183,7 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err if rhs != nil { for i := 0; i < rhs.Length(); i++ { child := rhs.At(i) - pe, err := listItemToPathElement(w.allocator, t, i, child) + pe, err := listItemToPathElement(w.allocator, w.schema, t, i, child) if err != nil { errs = append(errs, errorf("rhs: element %v: %v", i, err.Error())...) // If we can't construct the path element, we can't @@ -204,7 +204,7 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err if lhs != nil { for i := 0; i < lhs.Length(); i++ { child := lhs.At(i) - pe, err := listItemToPathElement(w.allocator, t, i, child) + pe, err := listItemToPathElement(w.allocator, w.schema, t, i, child) if err != nil { errs = append(errs, errorf("lhs: element %v: %v", i, err.Error())...) // If we can't construct the path element, we can't diff --git a/typed/remove.go b/typed/remove.go index a2811953..a20fc16f 100644 --- a/typed/remove.go +++ b/typed/remove.go @@ -57,7 +57,7 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) { for iter.Next() { i, item := iter.Item() // Ignore error because we have already validated this list - pe, _ := listItemToPathElement(w.allocator, t, i, item) + pe, _ := listItemToPathElement(w.allocator, w.schema, t, i, item) path, _ := fieldpath.MakePath(pe) if w.toRemove.Has(path) { continue diff --git a/typed/tofieldset.go b/typed/tofieldset.go index 0662d0f0..bc88c086 100644 --- a/typed/tofieldset.go +++ b/typed/tofieldset.go @@ -96,7 +96,7 @@ func (v *toFieldSetWalker) doScalar(t *schema.Scalar) ValidationErrors { func (v *toFieldSetWalker) visitListItems(t *schema.List, list value.List) (errs ValidationErrors) { for i := 0; i < list.Length(); i++ { child := list.At(i) - pe, _ := listItemToPathElement(v.allocator, t, i, child) + pe, _ := listItemToPathElement(v.allocator, v.schema, t, i, child) v2 := v.prepareDescent(pe, t.ElementType) v2.value = child errs = append(errs, v2.toFieldSet()...) diff --git a/typed/validate.go b/typed/validate.go index a2f8f4ef..403ec8fe 100644 --- a/typed/validate.go +++ b/typed/validate.go @@ -123,7 +123,7 @@ func (v *validatingObjectWalker) visitListItems(t *schema.List, list value.List) pe.Index = &i } else { var err error - pe, err = listItemToPathElement(v.allocator, t, i, child) + pe, err = listItemToPathElement(v.allocator, v.schema, t, i, child) if err != nil { errs = append(errs, errorf("element %v: %v", i, err.Error())...) // If we can't construct the path element, we can't