From 98e837167eda4d026e3ce7fa2a9b4bedbaf837b7 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Wed, 13 Dec 2017 16:06:01 -0800 Subject: [PATCH] Changes maps to merge vs. overwrite when processing configs. Fixes #3716 --- agent/config/builder.go | 3 ++- agent/config/merge.go | 14 ++++++++++---- agent/config/merge_test.go | 8 ++++++-- agent/config/runtime_test.go | 10 +++++----- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index b058d7f22cb9..6915141214a0 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -40,7 +40,8 @@ import ( // // The config sources are merged sequentially and later values // overwrite previously set values. Slice values are merged by -// concatenating the two slices. +// concatenating the two slices. Map values are merged by over- +// laying the later maps on top of earlier ones. // // Then call Validate() to perform the semantic validation to ensure // that the configuration is ready to be used. diff --git a/agent/config/merge.go b/agent/config/merge.go index 68bb22e6d4c8..f3e6d3e1e081 100644 --- a/agent/config/merge.go +++ b/agent/config/merge.go @@ -11,7 +11,7 @@ import ( // * only values of type struct, slice, map and pointer to simple types are allowed. Other types panic. // * when merging two structs the result is the recursive merge of all fields according to the rules below // * when merging two slices the result is the second slice appended to the first -// * when merging two maps the result is the second map if it is not empty, otherwise the first +// * when merging two maps the result is the second map overlaid on the first // * when merging two pointer values the result is the second value if it is not nil, otherwise the first func Merge(files ...Config) Config { var a Config @@ -28,10 +28,16 @@ func merge(a, b interface{}) interface{} { func mergeValue(a, b reflect.Value) reflect.Value { switch a.Kind() { case reflect.Map: - if b.Len() > 0 { - return b + r := reflect.MakeMap(a.Type()) + for _, k := range a.MapKeys() { + v := a.MapIndex(k) + r.SetMapIndex(k, v) } - return a + for _, k := range b.MapKeys() { + v := b.MapIndex(k) + r.SetMapIndex(k, v) + } + return r case reflect.Ptr: if !b.IsNil() { diff --git a/agent/config/merge_test.go b/agent/config/merge_test.go index 94a1a22a34d6..f8ae77662e69 100644 --- a/agent/config/merge_test.go +++ b/agent/config/merge_test.go @@ -26,6 +26,7 @@ func TestMerge(t *testing.T) { {StartJoinAddrsLAN: []string{"b"}}, {NodeMeta: map[string]string{"a": "b"}}, {NodeMeta: map[string]string{"c": "d"}}, + {NodeMeta: map[string]string{"c": "e"}}, {Ports: Ports{DNS: pInt(1)}}, {Ports: Ports{DNS: pInt(2), HTTP: pInt(3)}}, }, @@ -34,8 +35,11 @@ func TestMerge(t *testing.T) { RaftProtocol: pInt(2), ServerMode: pBool(true), StartJoinAddrsLAN: []string{"a", "b"}, - NodeMeta: map[string]string{"c": "d"}, - Ports: Ports{DNS: pInt(2), HTTP: pInt(3)}, + NodeMeta: map[string]string{ + "a": "b", + "c": "e", + }, + Ports: Ports{DNS: pInt(2), HTTP: pInt(3)}, }, }, } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 79deb645ba59..f572f9864615 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1115,7 +1115,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { "bootstrap_expect": 0, "datacenter":"b", "start_join": ["c", "d"], - "node_meta": {"c":"d"} + "node_meta": {"a":"c"} }`, }, hcl: []string{ @@ -1131,7 +1131,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { bootstrap_expect = 0 datacenter = "b" start_join = ["c", "d"] - node_meta = { "c" = "d" } + node_meta = { "a" = "c" } `, }, patch: func(rt *RuntimeConfig) { @@ -1139,7 +1139,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.BootstrapExpect = 0 rt.Datacenter = "b" rt.StartJoinAddrsLAN = []string{"a", "b", "c", "d"} - rt.NodeMeta = map[string]string{"c": "d"} + rt.NodeMeta = map[string]string{"a": "c"} rt.DataDir = dataDir }, }, @@ -1181,7 +1181,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { `-datacenter=b`, `-data-dir=` + dataDir, `-join`, `c`, `-join=d`, - `-node-meta=c:d`, + `-node-meta=a:c`, `-recursor`, `1.2.3.6`, `-recursor=5.6.7.10`, `-serf-lan-bind=3.3.3.3`, `-serf-wan-bind=4.4.4.4`, @@ -1194,7 +1194,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.SerfAdvertiseAddrWAN = tcpAddr("2.2.2.2:8302") rt.Datacenter = "b" rt.DNSRecursors = []string{"1.2.3.6", "5.6.7.10", "1.2.3.5", "5.6.7.9"} - rt.NodeMeta = map[string]string{"c": "d"} + rt.NodeMeta = map[string]string{"a": "c"} rt.SerfBindAddrLAN = tcpAddr("3.3.3.3:8301") rt.SerfBindAddrWAN = tcpAddr("4.4.4.4:8302") rt.StartJoinAddrsLAN = []string{"c", "d", "a", "b"}