From 91590f1ca014392b45d63a85f4fa01e8a9aeee92 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Thu, 19 Nov 2020 23:14:09 -0500 Subject: [PATCH 1/8] vendor: update vcontext --- go.mod | 2 +- go.sum | 8 ++++---- .../github.com/{ajeddeloh => coreos}/go-json/README | 0 .../{ajeddeloh => coreos}/go-json/decode.go | 0 .../{ajeddeloh => coreos}/go-json/encode.go | 0 .../github.com/{ajeddeloh => coreos}/go-json/fold.go | 0 .../{ajeddeloh => coreos}/go-json/indent.go | 0 .../{ajeddeloh => coreos}/go-json/scanner.go | 0 .../{ajeddeloh => coreos}/go-json/stream.go | 0 .../github.com/{ajeddeloh => coreos}/go-json/tags.go | 0 vendor/github.com/coreos/vcontext/json/json.go | 2 +- vendor/github.com/coreos/vcontext/path/path.go | 12 ++++++++++++ .../github.com/coreos/vcontext/validate/validate.go | 2 +- vendor/modules.txt | 6 +++--- 14 files changed, 22 insertions(+), 10 deletions(-) rename vendor/github.com/{ajeddeloh => coreos}/go-json/README (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/decode.go (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/encode.go (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/fold.go (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/indent.go (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/scanner.go (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/stream.go (100%) rename vendor/github.com/{ajeddeloh => coreos}/go-json/tags.go (100%) diff --git a/go.mod b/go.mod index f932cbb42..30ee3ded9 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/aws/aws-sdk-go v1.30.28 github.com/coreos/go-semver v0.3.0 github.com/coreos/go-systemd/v22 v22.0.0 - github.com/coreos/vcontext v0.0.0-20190529201340-22b159166068 + github.com/coreos/vcontext v0.0.0-20201120045928-b0e13dab675c github.com/google/renameio v0.1.0 github.com/google/uuid v1.1.1 github.com/pin/tftp v2.1.0+incompatible diff --git a/go.sum b/go.sum index 5980d0ada..b533762e7 100644 --- a/go.sum +++ b/go.sum @@ -39,8 +39,6 @@ dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/ajeddeloh/go-json v0.0.0-20170920214419-6a2fe990e083 h1:uwcvnXW76Y0rHM+qs7y8iHknWUWXYFNlD6FEVhc47TU= -github.com/ajeddeloh/go-json v0.0.0-20170920214419-6a2fe990e083/go.mod h1:otnto4/Icqn88WCcM4bhIJNSgsh9VLBuspyyCfvof9c= github.com/aws/aws-sdk-go v1.30.28 h1:SaPM7dlmp7h3Lj1nJ4jdzOkTdom08+g20k7AU5heZYg= github.com/aws/aws-sdk-go v1.30.28/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -49,12 +47,14 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5P github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= +github.com/coreos/go-json v0.0.0-20170920214419-6a2fe990e083 h1:iLYct0QOZLUuTbFBf+PDiKvpG1xPicwkcgnKaGCeTgc= +github.com/coreos/go-json v0.0.0-20170920214419-6a2fe990e083/go.mod h1:FmxyHfvrCFfCsXRylD4QQRlQmvzl+DG6iTHyEEykPfU= github.com/coreos/go-semver v0.3.0 h1:wkHLiw0WNATZnSG7epLsujiMCgPAc9xhjJ4tgnAxmfM= github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-systemd/v22 v22.0.0 h1:XJIw/+VlJ+87J+doOxznsAWIdmWuViOVhkQamW5YV28= github.com/coreos/go-systemd/v22 v22.0.0/go.mod h1:xO0FLkIi5MaZafQlIrOotqXZ90ih+1atmu1JpKERPPk= -github.com/coreos/vcontext v0.0.0-20190529201340-22b159166068 h1:y2aHj7QqyAJ6YBBONTAr17YxHHiogDkYnTsJvFNhxwY= -github.com/coreos/vcontext v0.0.0-20190529201340-22b159166068/go.mod h1:E+6hug9bFSe0KZ2ZAzr8M9F5JlArJjv5D1JS7KSkPKE= +github.com/coreos/vcontext v0.0.0-20201120045928-b0e13dab675c h1:jA28WeORitsxGFVWhyWB06sAG2HbLHPQuHwDydhU2CQ= +github.com/coreos/vcontext v0.0.0-20201120045928-b0e13dab675c/go.mod h1:z4pMVvaUrxs98RROlIYdAQCKhEicjnTirOaVyDRH5h8= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= diff --git a/vendor/github.com/ajeddeloh/go-json/README b/vendor/github.com/coreos/go-json/README similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/README rename to vendor/github.com/coreos/go-json/README diff --git a/vendor/github.com/ajeddeloh/go-json/decode.go b/vendor/github.com/coreos/go-json/decode.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/decode.go rename to vendor/github.com/coreos/go-json/decode.go diff --git a/vendor/github.com/ajeddeloh/go-json/encode.go b/vendor/github.com/coreos/go-json/encode.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/encode.go rename to vendor/github.com/coreos/go-json/encode.go diff --git a/vendor/github.com/ajeddeloh/go-json/fold.go b/vendor/github.com/coreos/go-json/fold.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/fold.go rename to vendor/github.com/coreos/go-json/fold.go diff --git a/vendor/github.com/ajeddeloh/go-json/indent.go b/vendor/github.com/coreos/go-json/indent.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/indent.go rename to vendor/github.com/coreos/go-json/indent.go diff --git a/vendor/github.com/ajeddeloh/go-json/scanner.go b/vendor/github.com/coreos/go-json/scanner.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/scanner.go rename to vendor/github.com/coreos/go-json/scanner.go diff --git a/vendor/github.com/ajeddeloh/go-json/stream.go b/vendor/github.com/coreos/go-json/stream.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/stream.go rename to vendor/github.com/coreos/go-json/stream.go diff --git a/vendor/github.com/ajeddeloh/go-json/tags.go b/vendor/github.com/coreos/go-json/tags.go similarity index 100% rename from vendor/github.com/ajeddeloh/go-json/tags.go rename to vendor/github.com/coreos/go-json/tags.go diff --git a/vendor/github.com/coreos/vcontext/json/json.go b/vendor/github.com/coreos/vcontext/json/json.go index 1dd7a553d..624ad2398 100644 --- a/vendor/github.com/coreos/vcontext/json/json.go +++ b/vendor/github.com/coreos/vcontext/json/json.go @@ -17,7 +17,7 @@ package json import ( "github.com/coreos/vcontext/tree" // todo: rewrite this dep - json "github.com/ajeddeloh/go-json" + json "github.com/coreos/go-json" ) func UnmarshalToContext(raw []byte) (tree.Node, error) { diff --git a/vendor/github.com/coreos/vcontext/path/path.go b/vendor/github.com/coreos/vcontext/path/path.go index dae54e73d..3daadc784 100644 --- a/vendor/github.com/coreos/vcontext/path/path.go +++ b/vendor/github.com/coreos/vcontext/path/path.go @@ -48,6 +48,18 @@ func (c ContextPath) Append(e ...interface{}) ContextPath { } } +func (c ContextPath) Copy() ContextPath { + // make sure to preserve reflect.DeepEqual() equality + var path []interface{} + if c.Path != nil { + path = append(path, c.Path...) + } + return ContextPath{ + Path: path, + Tag: c.Tag, + } +} + // Head returns the first element in the path, panics if empty. func (c ContextPath) Head() interface{} { return c.Path[0] diff --git a/vendor/github.com/coreos/vcontext/validate/validate.go b/vendor/github.com/coreos/vcontext/validate/validate.go index be0ea927c..876fe9873 100644 --- a/vendor/github.com/coreos/vcontext/validate/validate.go +++ b/vendor/github.com/coreos/vcontext/validate/validate.go @@ -71,7 +71,7 @@ func validate(context path.ContextPath, v reflect.Value, validateFunc CustomVali } } - r.Merge(validateFunc(v, context)) + r.Merge(validateFunc(v, context.Copy())) switch v.Kind() { case reflect.Struct: diff --git a/vendor/modules.txt b/vendor/modules.txt index 8e83d21a7..607a26220 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -10,8 +10,6 @@ cloud.google.com/go/internal/version cloud.google.com/go/storage # github.com/BurntSushi/toml v0.3.1 github.com/BurntSushi/toml -# github.com/ajeddeloh/go-json v0.0.0-20170920214419-6a2fe990e083 -github.com/ajeddeloh/go-json # github.com/aws/aws-sdk-go v1.30.28 github.com/aws/aws-sdk-go/aws github.com/aws/aws-sdk-go/aws/arn @@ -57,13 +55,15 @@ github.com/aws/aws-sdk-go/service/s3/s3iface github.com/aws/aws-sdk-go/service/s3/s3manager github.com/aws/aws-sdk-go/service/sts github.com/aws/aws-sdk-go/service/sts/stsiface +# github.com/coreos/go-json v0.0.0-20170920214419-6a2fe990e083 +github.com/coreos/go-json # github.com/coreos/go-semver v0.3.0 github.com/coreos/go-semver/semver # github.com/coreos/go-systemd/v22 v22.0.0 github.com/coreos/go-systemd/v22/dbus github.com/coreos/go-systemd/v22/journal github.com/coreos/go-systemd/v22/unit -# github.com/coreos/vcontext v0.0.0-20190529201340-22b159166068 +# github.com/coreos/vcontext v0.0.0-20201120045928-b0e13dab675c github.com/coreos/vcontext/json github.com/coreos/vcontext/path github.com/coreos/vcontext/report From f1bd31f8369c13674a8b8ea804fb053959d65ba0 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sun, 22 Nov 2020 23:39:13 -0500 Subject: [PATCH 2/8] config/util: remove stray print when parsing invalid JSON --- config/util/parsingErrors.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/util/parsingErrors.go b/config/util/parsingErrors.go index e55d96849..2af85e2bc 100644 --- a/config/util/parsingErrors.go +++ b/config/util/parsingErrors.go @@ -16,7 +16,6 @@ package util import ( "encoding/json" - "fmt" "github.com/coreos/ignition/v2/config/shared/errors" @@ -42,7 +41,6 @@ func HandleParseErrors(rawConfig []byte, to interface{}) (report.Report, error) node.Marker = tree.MarkerFromIndices(t.Offset, -1) } tree.FixLineColumn(node, rawConfig) - fmt.Printf("%+v\n", node.Marker.StartP.Index) r.AddOnError(path.ContextPath{Tag: "json"}, err) r.Correlate(node) From 66d42a5f403aa17b010e13dabe15775a9ef2e400 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Thu, 19 Nov 2020 20:46:23 -0500 Subject: [PATCH 3/8] config/merge: fix comments and add a couple more --- config/merge/merge.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config/merge/merge.go b/config/merge/merge.go index 3fc68a446..f5d64e57f 100644 --- a/config/merge/merge.go +++ b/config/merge/merge.go @@ -57,7 +57,7 @@ type structInfo struct { // checking done across all fields that share that handle mergedKeys map[string]string - // map from each handle + key() value to what list it came from + // map from each handle + key() to the corresponding item keysToValues map[handleKey]reflect.Value // map from each handle + key() to the list it came from @@ -175,6 +175,7 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { resultField.Set(reflect.MakeSlice(parentField.Type(), 0, parentField.Len()+childField.Len())) parentKeys := getKeySet(parentField) + // walk parent items for i := 0; i < parentField.Len(); i++ { parentItem := parentField.Index(i) key := util.CallKey(parentItem) @@ -202,11 +203,12 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { appendToSlice(resultField, parentItem) } } + // append child items not in parent for i := 0; i < childField.Len(); i++ { childItem := childField.Index(i) key := util.CallKey(childItem) if _, alreadyMerged := parentKeys[key]; !alreadyMerged { - // We only check the parentMap for this field. If the parent had a matching entry in a differnt field + // We only check the parentMap for this field. If the parent had a matching entry in a different field // then it would be skipped as case 2 above appendToSlice(resultField, childItem) } From 4cb9be74089caee9c9764cc5955e0e18531901fe Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sat, 21 Nov 2020 01:35:28 -0500 Subject: [PATCH 4/8] config/merge: fix typo --- config/merge/merge_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/merge/merge_test.go b/config/merge/merge_test.go index 4e0a29984..a2c51a5ec 100644 --- a/config/merge/merge_test.go +++ b/config/merge/merge_test.go @@ -641,6 +641,6 @@ func TestMerge(t *testing.T) { in2v := reflect.ValueOf(test.in2) out := MergeStruct(in1v, in2v).Interface().(types.Config) - assert.Equal(t, test.out, out, "#%d bas merge", i) + assert.Equal(t, test.out, out, "#%d bad merge", i) } } From 0296c461b00d173d02eb323c259de0d3a31c963b Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sat, 21 Nov 2020 01:54:20 -0500 Subject: [PATCH 5/8] config/merge: test primitive list items in slice case 1 We didn't have any test for merging non-IgnoreDuplicates primitive list items that are present in both lists. --- config/merge/merge_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/config/merge/merge_test.go b/config/merge/merge_test.go index a2c51a5ec..8dec701c6 100644 --- a/config/merge/merge_test.go +++ b/config/merge/merge_test.go @@ -101,6 +101,17 @@ func TestMerge(t *testing.T) { }, }, }, + Passwd: types.Passwd{ + Users: []types.PasswdUser{ + { + Name: "bovik", + SSHAuthorizedKeys: []types.SSHAuthorizedKey{ + "one", + "two", + }, + }, + }, + }, }, in2: types.Config{ Storage: types.Storage{ @@ -159,6 +170,17 @@ func TestMerge(t *testing.T) { }, }, }, + Passwd: types.Passwd{ + Users: []types.PasswdUser{ + { + Name: "bovik", + SSHAuthorizedKeys: []types.SSHAuthorizedKey{ + "three", + "two", + }, + }, + }, + }, }, out: types.Config{ Storage: types.Storage{ @@ -222,6 +244,18 @@ func TestMerge(t *testing.T) { }, }, }, + Passwd: types.Passwd{ + Users: []types.PasswdUser{ + { + Name: "bovik", + SSHAuthorizedKeys: []types.SSHAuthorizedKey{ + "one", + "two", + "three", + }, + }, + }, + }, }, }, From b7d9156f04c4ff43c33ff34382a8f596b4930d0d Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Thu, 19 Nov 2020 20:49:21 -0500 Subject: [PATCH 6/8] config/merge: access field name as fieldMeta.Name --- config/merge/merge.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/config/merge/merge.go b/config/merge/merge.go index f5d64e57f..1462b70ad 100644 --- a/config/merge/merge.go +++ b/config/merge/merge.go @@ -146,7 +146,7 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { info := newStructInfo(parent, child) for i := 0; i < parent.NumField(); i++ { - fieldName := parent.Type().Field(i).Name + fieldMeta := parent.Type().Field(i) parentField := parent.Field(i) childField := child.Field(i) resultField := result.Field(i) @@ -161,12 +161,12 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { resultField.Set(childField) case kind == reflect.Struct: resultField.Set(MergeStruct(parentField, childField)) - case kind == reflect.Slice && info.ignoreField(fieldName): + case kind == reflect.Slice && info.ignoreField(fieldMeta.Name): if parentField.Len()+childField.Len() == 0 { continue } resultField.Set(reflect.AppendSlice(parentField, childField)) - case kind == reflect.Slice && !info.ignoreField(fieldName): + case kind == reflect.Slice && !info.ignoreField(fieldMeta.Name): // ooph, this is a doosey maxlen := parentField.Len() + childField.Len() if maxlen == 0 { @@ -180,13 +180,13 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { parentItem := parentField.Index(i) key := util.CallKey(parentItem) - if childItem, childList, ok := info.getChildEntryByKey(fieldName, key); ok { - if childList == fieldName { + if childItem, childList, ok := info.getChildEntryByKey(fieldMeta.Name, key); ok { + if childList == fieldMeta.Name { // case 1: in child config in same list if childItem.Kind() == reflect.Struct { // If HTTP header Value is nil, it means that we should remove the // parent header from the result. - if fieldName == "HTTPHeaders" && childItem.FieldByName("Value").IsNil() { + if fieldMeta.Name == "HTTPHeaders" && childItem.FieldByName("Value").IsNil() { continue } appendToSlice(resultField, MergeStruct(parentItem, childItem)) From cd6b90c23194ca9dd1b8e53251a3bde3e2ba7344 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sat, 21 Nov 2020 01:34:25 -0500 Subject: [PATCH 7/8] config/merge: allow obtaining a transcript of merge operations If FCCT wants to use config merging to implement desugaring, it needs to be able to map fields in the merged config back to the original FCC YAML, which means that it needs a mapping from input to output fields. Add a merge function that also returns a transcript of field mappings. --- config/merge/merge.go | 172 ++++++++++++++++++++++++++++++++++--- config/merge/merge_test.go | 110 ++++++++++++++++++++++-- 2 files changed, 262 insertions(+), 20 deletions(-) diff --git a/config/merge/merge.go b/config/merge/merge.go index 1462b70ad..0f050e695 100644 --- a/config/merge/merge.go +++ b/config/merge/merge.go @@ -15,9 +15,13 @@ package merge import ( + "fmt" "reflect" + "strings" "github.com/coreos/ignition/v2/config/util" + + "github.com/coreos/vcontext/path" ) // Rules of Config Merging: @@ -35,6 +39,50 @@ import ( // - remove entries from the parent with the same Key() that are not in the same list // - append entries that are unique to the child +const ( + TAG_PARENT = "parent" + TAG_CHILD = "child" + TAG_RESULT = "result" +) + +// The path to one output field, and its corresponding input. From.Tag will +// be TAG_PARENT or TAG_CHILD depending on the origin of the field. +type Mapping struct { + From path.ContextPath + To path.ContextPath +} + +func (m Mapping) String() string { + return fmt.Sprintf("%s:%s → %s", m.From.Tag, m.From, m.To) +} + +type Transcript struct { + Mappings []Mapping +} + +func (t Transcript) String() string { + var lines []string + for _, m := range t.Mappings { + lines = append(lines, m.String()) + } + return strings.Join(lines, "\n") +} + +// pathAppendField looks up the JSON field name for field and returns base +// with that field name appended. +func pathAppendField(base path.ContextPath, field reflect.StructField) path.ContextPath { + tagName := strings.Split(field.Tag.Get("json"), ",")[0] + if tagName != "" { + return base.Append(tagName) + } + if field.Anonymous { + // field is a struct embedded in another struct (e.g. + // FileEmbedded1). Pretend it doesn't exist. + return base + } + panic("no JSON struct tag for " + field.Name) +} + // appendToSlice is a helper that appends to a slice without returning a new one. // panics if len >= cap func appendToSlice(s, v reflect.Value) { @@ -62,6 +110,9 @@ type structInfo struct { // map from each handle + key() to the list it came from keysToLists map[handleKey]string + + // map from each handle + key() to the index within the list + keysToListIndexes map[handleKey]int } // returns if this field should not do duplicate checking/merging @@ -71,9 +122,10 @@ func (s structInfo) ignoreField(name string) bool { } // getChildEntryByKey takes the name of a field (not handle) in the parent and a key and looks that entry -// up in the child. It will look up across all slices that share the same handle. It return the value and -// name of the field in the child it was found in. The bool indicates whether it was found. -func (s structInfo) getChildEntryByKey(fieldName, key string) (reflect.Value, string, bool) { +// up in the child. It will look up across all slices that share the same handle. It returns the value, +// name of the field in the child it was found in, and the list index within that field. The bool indicates +// whether it was found. +func (s structInfo) getChildEntryByKey(fieldName, key string) (reflect.Value, string, int, bool) { handle := fieldName if tmp, ok := s.mergedKeys[fieldName]; ok { handle = tmp @@ -84,9 +136,9 @@ func (s structInfo) getChildEntryByKey(fieldName, key string) (reflect.Value, st key: key, } if v, ok := s.keysToValues[hkey]; ok { - return v, s.keysToLists[hkey], true + return v, s.keysToLists[hkey], s.keysToListIndexes[hkey], true } - return reflect.Value{}, "", false + return reflect.Value{}, "", 0, false } func newStructInfo(parent, child reflect.Value) structInfo { @@ -102,6 +154,7 @@ func newStructInfo(parent, child reflect.Value) structInfo { keysToValues := map[handleKey]reflect.Value{} keysToLists := map[handleKey]string{} + keysToListIndexes := map[handleKey]int{} for i := 0; i < child.NumField(); i++ { field := child.Field(i) if field.Kind() != reflect.Slice { @@ -126,21 +179,42 @@ func newStructInfo(parent, child reflect.Value) structInfo { } keysToValues[hkey] = v keysToLists[hkey] = fieldName + keysToListIndexes[hkey] = j } } return structInfo{ - ignoreDups: ignoreDups, - mergedKeys: mergedKeys, - keysToValues: keysToValues, - keysToLists: keysToLists, + ignoreDups: ignoreDups, + mergedKeys: mergedKeys, + keysToValues: keysToValues, + keysToLists: keysToLists, + keysToListIndexes: keysToListIndexes, } } // MergeStruct is intended for use by config/vX_Y/ packages only. They should expose their own Merge() that is properly // typed. Use that one instead. +// The signature uses reflect.Value instead of interface{} for historical +// reasons. // parent and child MUST be the same type func MergeStruct(parent, child reflect.Value) reflect.Value { + result, _ := MergeStructTranscribe(parent.Interface(), child.Interface()) + return reflect.ValueOf(result) +} + +// MergeStructTranscribe is intended for use by external translation code. +// It merges the specified configs and returns a transcript of the actions +// taken. parent and child MUST be the same type. +func MergeStructTranscribe(parent, child interface{}) (interface{}, Transcript) { + var transcript Transcript + result := mergeStruct(reflect.ValueOf(parent), path.New(TAG_PARENT), reflect.ValueOf(child), path.New(TAG_CHILD), path.New(TAG_RESULT), &transcript) + return result.Interface(), transcript +} + +// parent and child MUST be the same type +// we transcribe all leaf fields, and all intermediate structs that wholly +// originate from either parent or child +func mergeStruct(parent reflect.Value, parentPath path.ContextPath, child reflect.Value, childPath path.ContextPath, resultPath path.ContextPath, transcript *Transcript) reflect.Value { // use New() so it's settable, addr-able, etc result := reflect.New(parent.Type()).Elem() info := newStructInfo(parent, child) @@ -150,22 +224,38 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { parentField := parent.Field(i) childField := child.Field(i) resultField := result.Field(i) + parentFieldPath := pathAppendField(parentPath, fieldMeta) + childFieldPath := pathAppendField(childPath, fieldMeta) + resultFieldPath := pathAppendField(resultPath, fieldMeta) kind := parentField.Kind() switch { case util.IsPrimitive(kind): resultField.Set(childField) + transcribe(childFieldPath, resultFieldPath, resultField, fieldMeta, transcript) case kind == reflect.Ptr && childField.IsNil(): resultField.Set(parentField) + transcribe(parentFieldPath, resultFieldPath, resultField, fieldMeta, transcript) case kind == reflect.Ptr && !childField.IsNil(): resultField.Set(childField) + transcribe(childFieldPath, resultFieldPath, resultField, fieldMeta, transcript) case kind == reflect.Struct: - resultField.Set(MergeStruct(parentField, childField)) + resultField.Set(mergeStruct(parentField, parentFieldPath, childField, childFieldPath, resultFieldPath, transcript)) case kind == reflect.Slice && info.ignoreField(fieldMeta.Name): if parentField.Len()+childField.Len() == 0 { continue } - resultField.Set(reflect.AppendSlice(parentField, childField)) + resultField.Set(reflect.MakeSlice(parentField.Type(), 0, parentField.Len()+childField.Len())) + for i := 0; i < parentField.Len(); i++ { + item := parentField.Index(i) + appendToSlice(resultField, item) + transcribe(parentFieldPath.Append(i), resultFieldPath.Append(i), item, fieldMeta, transcript) + } + for i := 0; i < childField.Len(); i++ { + item := childField.Index(i) + appendToSlice(resultField, item) + transcribe(childFieldPath.Append(i), resultFieldPath.Append(parentField.Len()+i), item, fieldMeta, transcript) + } case kind == reflect.Slice && !info.ignoreField(fieldMeta.Name): // ooph, this is a doosey maxlen := parentField.Len() + childField.Len() @@ -178,20 +268,24 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { // walk parent items for i := 0; i < parentField.Len(); i++ { parentItem := parentField.Index(i) + parentItemPath := parentFieldPath.Append(i) + resultItemPath := resultFieldPath.Append(resultField.Len()) key := util.CallKey(parentItem) - if childItem, childList, ok := info.getChildEntryByKey(fieldMeta.Name, key); ok { + if childItem, childList, childListIndex, ok := info.getChildEntryByKey(fieldMeta.Name, key); ok { if childList == fieldMeta.Name { // case 1: in child config in same list + childItemPath := childFieldPath.Append(childListIndex) if childItem.Kind() == reflect.Struct { // If HTTP header Value is nil, it means that we should remove the // parent header from the result. if fieldMeta.Name == "HTTPHeaders" && childItem.FieldByName("Value").IsNil() { continue } - appendToSlice(resultField, MergeStruct(parentItem, childItem)) + appendToSlice(resultField, mergeStruct(parentItem, parentItemPath, childItem, childItemPath, resultItemPath, transcript)) } else if util.IsPrimitive(childItem.Kind()) { appendToSlice(resultField, childItem) + transcribe(childItemPath, resultItemPath, childItem, fieldMeta, transcript) } else { panic("List of pointers or slices or something else weird") } @@ -201,16 +295,20 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { } else { // case 3: not in child config, append it appendToSlice(resultField, parentItem) + transcribe(parentItemPath, resultItemPath, parentItem, fieldMeta, transcript) } } // append child items not in parent for i := 0; i < childField.Len(); i++ { childItem := childField.Index(i) + childItemPath := childFieldPath.Append(i) + resultItemPath := resultFieldPath.Append(resultField.Len()) key := util.CallKey(childItem) if _, alreadyMerged := parentKeys[key]; !alreadyMerged { // We only check the parentMap for this field. If the parent had a matching entry in a different field // then it would be skipped as case 2 above appendToSlice(resultField, childItem) + transcribe(childItemPath, resultItemPath, childItem, fieldMeta, transcript) } } default: @@ -221,6 +319,54 @@ func MergeStruct(parent, child reflect.Value) reflect.Value { return result } +// transcribe is called by mergeStruct when the latter decides to merge a +// subtree wholesale from either the parent or child, and thus loses +// interest in that subtree. transcribe descends the rest of that subtree, +// transcribing all of its populated leaves. It returns true if we +// transcribed anything. +func transcribe(fromPath path.ContextPath, toPath path.ContextPath, value reflect.Value, fieldMeta reflect.StructField, transcript *Transcript) bool { + add := func(from, to path.ContextPath) { + transcript.Mappings = append(transcript.Mappings, Mapping{ + From: from.Copy(), + To: to.Copy(), + }) + } + + kind := value.Kind() + switch { + case util.IsPrimitive(kind): + if value.Interface() == reflect.Zero(value.Type()).Interface() { + return false + } + add(fromPath, toPath) + case kind == reflect.Ptr: + if value.IsNil() { + return false + } + add(fromPath, toPath) + case kind == reflect.Struct: + var transcribed bool + for i := 0; i < value.NumField(); i++ { + valueFieldMeta := value.Type().Field(i) + transcribed = transcribe(pathAppendField(fromPath, valueFieldMeta), pathAppendField(toPath, valueFieldMeta), value.Field(i), valueFieldMeta, transcript) || transcribed + } + // embedded structs and empty structs should be invisible + if transcribed && !fieldMeta.Anonymous { + add(fromPath, toPath) + } + return transcribed + case kind == reflect.Slice: + var transcribed bool + for i := 0; i < value.Len(); i++ { + transcribed = transcribe(fromPath.Append(i), toPath.Append(i), value.Index(i), fieldMeta, transcript) || transcribed + } + return transcribed + default: + panic("unreachable code reached") + } + return true +} + // getKeySet takes a value of a slice and returns the set of all the Key() values in that slice func getKeySet(list reflect.Value) map[string]struct{} { m := map[string]struct{}{} diff --git a/config/merge/merge_test.go b/config/merge/merge_test.go index 8dec701c6..288ce08fa 100644 --- a/config/merge/merge_test.go +++ b/config/merge/merge_test.go @@ -15,12 +15,12 @@ package merge import ( - "reflect" "testing" "github.com/coreos/ignition/v2/config/util" "github.com/coreos/ignition/v2/config/v3_3_experimental/types" + "github.com/coreos/vcontext/path" "github.com/stretchr/testify/assert" ) @@ -36,9 +36,10 @@ func toPointer(val string) *string { func TestMerge(t *testing.T) { type test struct { - in1 types.Config - in2 types.Config - out types.Config + in1 types.Config + in2 types.Config + out types.Config + transcript Transcript } tests := []test{ @@ -55,6 +56,9 @@ func TestMerge(t *testing.T) { out: types.Config{ Ignition: types.Ignition{Version: "haha this isn't validated"}, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_CHILD, "ignition", "version"), path.New(TAG_RESULT, "ignition", "version")}, + }}, }, { in1: types.Config{ @@ -257,6 +261,36 @@ func TestMerge(t *testing.T) { }, }, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_CHILD, "passwd", "users", 0, "name"), path.New(TAG_RESULT, "passwd", "users", 0, "name")}, + {path.New(TAG_PARENT, "passwd", "users", 0, "sshAuthorizedKeys", 0), path.New(TAG_RESULT, "passwd", "users", 0, "sshAuthorizedKeys", 0)}, + {path.New(TAG_CHILD, "passwd", "users", 0, "sshAuthorizedKeys", 1), path.New(TAG_RESULT, "passwd", "users", 0, "sshAuthorizedKeys", 1)}, + {path.New(TAG_CHILD, "passwd", "users", 0, "sshAuthorizedKeys", 0), path.New(TAG_RESULT, "passwd", "users", 0, "sshAuthorizedKeys", 2)}, + {path.New(TAG_CHILD, "storage", "directories", 0, "path"), path.New(TAG_RESULT, "storage", "directories", 0, "path")}, + {path.New(TAG_CHILD, "storage", "directories", 0), path.New(TAG_RESULT, "storage", "directories", 0)}, + {path.New(TAG_CHILD, "storage", "disks", 0, "device"), path.New(TAG_RESULT, "storage", "disks", 0, "device")}, + {path.New(TAG_CHILD, "storage", "disks", 0, "partitions", 0, "label"), path.New(TAG_RESULT, "storage", "disks", 0, "partitions", 0, "label")}, + {path.New(TAG_CHILD, "storage", "disks", 0, "partitions", 0, "number"), path.New(TAG_RESULT, "storage", "disks", 0, "partitions", 0, "number")}, + {path.New(TAG_PARENT, "storage", "disks", 0, "partitions", 0, "startMiB"), path.New(TAG_RESULT, "storage", "disks", 0, "partitions", 0, "startMiB")}, + {path.New(TAG_CHILD, "storage", "disks", 0, "partitions", 1, "label"), path.New(TAG_RESULT, "storage", "disks", 0, "partitions", 1, "label")}, + {path.New(TAG_CHILD, "storage", "disks", 0, "partitions", 1, "number"), path.New(TAG_RESULT, "storage", "disks", 0, "partitions", 1, "number")}, + {path.New(TAG_CHILD, "storage", "disks", 0, "partitions", 1), path.New(TAG_RESULT, "storage", "disks", 0, "partitions", 1)}, + {path.New(TAG_CHILD, "storage", "disks", 0, "wipeTable"), path.New(TAG_RESULT, "storage", "disks", 0, "wipeTable")}, + {path.New(TAG_CHILD, "storage", "disks", 1, "device"), path.New(TAG_RESULT, "storage", "disks", 1, "device")}, + {path.New(TAG_PARENT, "storage", "disks", 1, "wipeTable"), path.New(TAG_RESULT, "storage", "disks", 1, "wipeTable")}, + {path.New(TAG_CHILD, "storage", "disks", 2, "device"), path.New(TAG_RESULT, "storage", "disks", 2, "device")}, + {path.New(TAG_CHILD, "storage", "disks", 2, "wipeTable"), path.New(TAG_RESULT, "storage", "disks", 2, "wipeTable")}, + {path.New(TAG_CHILD, "storage", "disks", 2), path.New(TAG_RESULT, "storage", "disks", 2)}, + {path.New(TAG_CHILD, "storage", "files", 0, "path"), path.New(TAG_RESULT, "storage", "files", 0, "path")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "source"), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "source")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0), path.New(TAG_RESULT, "storage", "files", 0, "append", 0)}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "source"), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "source")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0), path.New(TAG_RESULT, "storage", "files", 0, "append", 1)}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 1, "source"), path.New(TAG_RESULT, "storage", "files", 0, "append", 2, "source")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 1), path.New(TAG_RESULT, "storage", "files", 0, "append", 2)}, + {path.New(TAG_CHILD, "storage", "links", 0, "path"), path.New(TAG_RESULT, "storage", "links", 0, "path")}, + {path.New(TAG_CHILD, "storage", "links", 0), path.New(TAG_RESULT, "storage", "links", 0)}, + }}, }, // merge config reference that contains HTTP headers @@ -335,6 +369,17 @@ func TestMerge(t *testing.T) { }, }, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_PARENT, "ignition", "config", "merge", 0, "httpHeaders", 0, "name"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 0, "name")}, + {path.New(TAG_PARENT, "ignition", "config", "merge", 0, "httpHeaders", 0, "value"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 0, "value")}, + {path.New(TAG_PARENT, "ignition", "config", "merge", 0, "httpHeaders", 0), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 0)}, + {path.New(TAG_CHILD, "ignition", "config", "merge", 0, "httpHeaders", 2, "name"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 1, "name")}, + {path.New(TAG_CHILD, "ignition", "config", "merge", 0, "httpHeaders", 2, "value"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 1, "value")}, + {path.New(TAG_CHILD, "ignition", "config", "merge", 0, "httpHeaders", 1, "name"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 2, "name")}, + {path.New(TAG_CHILD, "ignition", "config", "merge", 0, "httpHeaders", 1, "value"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 2, "value")}, + {path.New(TAG_CHILD, "ignition", "config", "merge", 0, "httpHeaders", 1), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "httpHeaders", 2)}, + {path.New(TAG_CHILD, "ignition", "config", "merge", 0, "source"), path.New(TAG_RESULT, "ignition", "config", "merge", 0, "source")}, + }}, }, // replace config reference that contains HTTP headers @@ -407,6 +452,17 @@ func TestMerge(t *testing.T) { }, }, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_PARENT, "ignition", "config", "replace", "httpHeaders", 0, "name"), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 0, "name")}, + {path.New(TAG_PARENT, "ignition", "config", "replace", "httpHeaders", 0, "value"), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 0, "value")}, + {path.New(TAG_PARENT, "ignition", "config", "replace", "httpHeaders", 0), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 0)}, + {path.New(TAG_CHILD, "ignition", "config", "replace", "httpHeaders", 2, "name"), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 1, "name")}, + {path.New(TAG_CHILD, "ignition", "config", "replace", "httpHeaders", 2, "value"), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 1, "value")}, + {path.New(TAG_CHILD, "ignition", "config", "replace", "httpHeaders", 1, "name"), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 2, "name")}, + {path.New(TAG_CHILD, "ignition", "config", "replace", "httpHeaders", 1, "value"), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 2, "value")}, + {path.New(TAG_CHILD, "ignition", "config", "replace", "httpHeaders", 1), path.New(TAG_RESULT, "ignition", "config", "replace", "httpHeaders", 2)}, + {path.New(TAG_CHILD, "ignition", "config", "replace", "source"), path.New(TAG_RESULT, "ignition", "config", "replace", "source")}, + }}, }, // CA reference that contains HTTP headers @@ -491,6 +547,17 @@ func TestMerge(t *testing.T) { }, }, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_PARENT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 0, "name"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 0, "name")}, + {path.New(TAG_PARENT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 0, "value"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 0, "value")}, + {path.New(TAG_PARENT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 0), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 0)}, + {path.New(TAG_CHILD, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 2, "name"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 1, "name")}, + {path.New(TAG_CHILD, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 2, "value"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 1, "value")}, + {path.New(TAG_CHILD, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 1, "name"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 2, "name")}, + {path.New(TAG_CHILD, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 1, "value"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 2, "value")}, + {path.New(TAG_CHILD, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 1), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "httpHeaders", 2)}, + {path.New(TAG_CHILD, "ignition", "security", "tls", "certificateAuthorities", 0, "source"), path.New(TAG_RESULT, "ignition", "security", "tls", "certificateAuthorities", 0, "source")}, + }}, }, // file contents that contain HTTP headers @@ -575,6 +642,17 @@ func TestMerge(t *testing.T) { }, }, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_PARENT, "storage", "files", 0, "contents", "httpHeaders", 0, "name"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 0, "name")}, + {path.New(TAG_PARENT, "storage", "files", 0, "contents", "httpHeaders", 0, "value"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 0, "value")}, + {path.New(TAG_PARENT, "storage", "files", 0, "contents", "httpHeaders", 0), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 0)}, + {path.New(TAG_CHILD, "storage", "files", 0, "contents", "httpHeaders", 2, "name"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 1, "name")}, + {path.New(TAG_CHILD, "storage", "files", 0, "contents", "httpHeaders", 2, "value"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 1, "value")}, + {path.New(TAG_CHILD, "storage", "files", 0, "contents", "httpHeaders", 1, "name"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 2, "name")}, + {path.New(TAG_CHILD, "storage", "files", 0, "contents", "httpHeaders", 1, "value"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 2, "value")}, + {path.New(TAG_CHILD, "storage", "files", 0, "contents", "httpHeaders", 1), path.New(TAG_RESULT, "storage", "files", 0, "contents", "httpHeaders", 2)}, + {path.New(TAG_CHILD, "storage", "files", 0, "contents", "source"), path.New(TAG_RESULT, "storage", "files", 0, "contents", "source")}, + }}, }, // file contents that contain HTTP headers @@ -667,14 +745,32 @@ func TestMerge(t *testing.T) { }, }, }, + transcript: Transcript{[]Mapping{ + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "httpHeaders", 0, "name"), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "httpHeaders", 0, "name")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "httpHeaders", 0, "value"), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "httpHeaders", 0, "value")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "httpHeaders", 0), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "httpHeaders", 0)}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "httpHeaders", 1, "name"), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "httpHeaders", 1, "name")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "httpHeaders", 1, "value"), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "httpHeaders", 1, "value")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "httpHeaders", 1), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "httpHeaders", 1)}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0, "source"), path.New(TAG_RESULT, "storage", "files", 0, "append", 0, "source")}, + {path.New(TAG_PARENT, "storage", "files", 0, "append", 0), path.New(TAG_RESULT, "storage", "files", 0, "append", 0)}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "httpHeaders", 0, "name"), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "httpHeaders", 0, "name")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "httpHeaders", 0, "value"), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "httpHeaders", 0, "value")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "httpHeaders", 0), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "httpHeaders", 0)}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "httpHeaders", 1, "name"), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "httpHeaders", 1, "name")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "httpHeaders", 1, "value"), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "httpHeaders", 1, "value")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "httpHeaders", 1), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "httpHeaders", 1)}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0, "source"), path.New(TAG_RESULT, "storage", "files", 0, "append", 1, "source")}, + {path.New(TAG_CHILD, "storage", "files", 0, "append", 0), path.New(TAG_RESULT, "storage", "files", 0, "append", 1)}, + }}, }, } for i, test := range tests { - in1v := reflect.ValueOf(test.in1) - in2v := reflect.ValueOf(test.in2) - out := MergeStruct(in1v, in2v).Interface().(types.Config) + outi, transcript := MergeStructTranscribe(test.in1, test.in2) + out := outi.(types.Config) assert.Equal(t, test.out, out, "#%d bad merge", i) + assert.Equal(t, test.transcript, transcript, "#%d bad transcript", i) } } From da0f58494df2e5c22e4793464b2f07e1eb78da49 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Sun, 22 Nov 2020 14:37:42 -0500 Subject: [PATCH 8/8] config: deprecate MergeStruct() Move all in-tree callers to MergeStructTranscribe(), which doesn't require using the reflection API, and deprecate MergeStruct(). We can't remove it because that would require a major version bump. --- config/merge/merge.go | 15 +++++++-------- config/v3_0/config.go | 10 ++-------- config/v3_1/config.go | 10 ++-------- config/v3_2/config.go | 10 ++-------- config/v3_3_experimental/config.go | 10 ++-------- 5 files changed, 15 insertions(+), 40 deletions(-) diff --git a/config/merge/merge.go b/config/merge/merge.go index 0f050e695..62956219e 100644 --- a/config/merge/merge.go +++ b/config/merge/merge.go @@ -192,19 +192,18 @@ func newStructInfo(parent, child reflect.Value) structInfo { } } -// MergeStruct is intended for use by config/vX_Y/ packages only. They should expose their own Merge() that is properly -// typed. Use that one instead. -// The signature uses reflect.Value instead of interface{} for historical -// reasons. -// parent and child MUST be the same type +// Deprecated: Use MergeStructTranscribe() instead. func MergeStruct(parent, child reflect.Value) reflect.Value { result, _ := MergeStructTranscribe(parent.Interface(), child.Interface()) return reflect.ValueOf(result) } -// MergeStructTranscribe is intended for use by external translation code. -// It merges the specified configs and returns a transcript of the actions -// taken. parent and child MUST be the same type. +// MergeStructTranscribe is intended for use by config/vX_Y/ packages and +// by generic external translation code. Most users should use the properly +// typed wrappers provided by the config/vX_Y/ packages. +// +// MergeStructTranscribe merges the specified configs and returns a +// transcript of the actions taken. parent and child MUST be the same type. func MergeStructTranscribe(parent, child interface{}) (interface{}, Transcript) { var transcript Transcript result := mergeStruct(reflect.ValueOf(parent), path.New(TAG_PARENT), reflect.ValueOf(child), path.New(TAG_CHILD), path.New(TAG_RESULT), &transcript) diff --git a/config/v3_0/config.go b/config/v3_0/config.go index 1fd637d59..38b50bbb3 100644 --- a/config/v3_0/config.go +++ b/config/v3_0/config.go @@ -15,8 +15,6 @@ package v3_0 import ( - "reflect" - "github.com/coreos/ignition/v2/config/merge" "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/util" @@ -28,12 +26,8 @@ import ( ) func Merge(parent, child types.Config) types.Config { - vParent := reflect.ValueOf(parent) - vChild := reflect.ValueOf(child) - - vRes := merge.MergeStruct(vParent, vChild) - res := vRes.Interface().(types.Config) - return res + res, _ := merge.MergeStructTranscribe(parent, child) + return res.(types.Config) } // Parse parses the raw config into a types.Config struct and generates a report of any diff --git a/config/v3_1/config.go b/config/v3_1/config.go index 9a395e091..0d0ec6793 100644 --- a/config/v3_1/config.go +++ b/config/v3_1/config.go @@ -15,8 +15,6 @@ package v3_1 import ( - "reflect" - "github.com/coreos/ignition/v2/config/merge" "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/util" @@ -28,12 +26,8 @@ import ( ) func Merge(parent, child types.Config) types.Config { - vParent := reflect.ValueOf(parent) - vChild := reflect.ValueOf(child) - - vRes := merge.MergeStruct(vParent, vChild) - res := vRes.Interface().(types.Config) - return res + res, _ := merge.MergeStructTranscribe(parent, child) + return res.(types.Config) } // Parse parses the raw config into a types.Config struct and generates a report of any diff --git a/config/v3_2/config.go b/config/v3_2/config.go index e32e92381..8d0abbcce 100644 --- a/config/v3_2/config.go +++ b/config/v3_2/config.go @@ -15,8 +15,6 @@ package v3_2 import ( - "reflect" - "github.com/coreos/ignition/v2/config/merge" "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/util" @@ -28,12 +26,8 @@ import ( ) func Merge(parent, child types.Config) types.Config { - vParent := reflect.ValueOf(parent) - vChild := reflect.ValueOf(child) - - vRes := merge.MergeStruct(vParent, vChild) - res := vRes.Interface().(types.Config) - return res + res, _ := merge.MergeStructTranscribe(parent, child) + return res.(types.Config) } // Parse parses the raw config into a types.Config struct and generates a report of any diff --git a/config/v3_3_experimental/config.go b/config/v3_3_experimental/config.go index bbaef132f..969d3c569 100644 --- a/config/v3_3_experimental/config.go +++ b/config/v3_3_experimental/config.go @@ -15,8 +15,6 @@ package v3_3_experimental import ( - "reflect" - "github.com/coreos/ignition/v2/config/merge" "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/util" @@ -28,12 +26,8 @@ import ( ) func Merge(parent, child types.Config) types.Config { - vParent := reflect.ValueOf(parent) - vChild := reflect.ValueOf(child) - - vRes := merge.MergeStruct(vParent, vChild) - res := vRes.Interface().(types.Config) - return res + res, _ := merge.MergeStructTranscribe(parent, child) + return res.(types.Config) } // Parse parses the raw config into a types.Config struct and generates a report of any