From 9d2e83d56d053465bf79dc92052070995fd4742b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 20 Jul 2014 17:17:03 -0700 Subject: [PATCH] config: Merge works properly --- config/config.go | 78 ++++++++++++++++++ config/loader_test.go | 1 + config/merge.go | 182 +++++++++++++++++++++++++++--------------- config/merge_test.go | 149 ++++++++++++++++++++++++++++++++++ config/raw_config.go | 19 +++++ 5 files changed, 363 insertions(+), 66 deletions(-) create mode 100644 config/merge_test.go diff --git a/config/config.go b/config/config.go index 442035492..bb7ab76bf 100644 --- a/config/config.go +++ b/config/config.go @@ -251,6 +251,84 @@ func (c *Config) allVariables() map[string][]InterpolatedVariable { return result } +func (o *Output) mergerName() string { + return o.Name +} + +func (o *Output) mergerMerge(m merger) merger { + o2 := m.(*Output) + + result := *o + result.Name = o2.Name + result.RawConfig = result.RawConfig.merge(o2.RawConfig) + + return &result +} + +func (c *ProviderConfig) mergerName() string { + return c.Name +} + +func (c *ProviderConfig) mergerMerge(m merger) merger { + c2 := m.(*ProviderConfig) + + result := *c + result.Name = c2.Name + result.RawConfig = result.RawConfig.merge(c2.RawConfig) + + return &result +} + +func (r *Resource) mergerName() string { + return fmt.Sprintf("%s.%s", r.Type, r.Name) +} + +func (r *Resource) mergerMerge(m merger) merger { + r2 := m.(*Resource) + + result := *r + result.Name = r2.Name + result.Type = r2.Type + result.RawConfig = result.RawConfig.merge(r2.RawConfig) + + if r2.Count > 0 { + result.Count = r2.Count + } + + if len(r2.Provisioners) > 0 { + result.Provisioners = r2.Provisioners + } + + return &result +} + +// Merge merges two variables to create a new third variable. +func (v *Variable) Merge(v2 *Variable) *Variable { + // Shallow copy the variable + result := *v + + // The names should be the same, but the second name always wins. + result.Name = v2.Name + + if v2.defaultSet { + result.Default = v2.Default + result.defaultSet = true + } + if v2.Description != "" { + result.Description = v2.Description + } + + return &result +} + +func (v *Variable) mergerName() string { + return v.Name +} + +func (v *Variable) mergerMerge(m merger) merger { + return v.Merge(m.(*Variable)) +} + // Required tests whether a variable is required or not. func (v *Variable) Required() bool { return !v.defaultSet diff --git a/config/loader_test.go b/config/loader_test.go index 0f29f77c4..21e870e43 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -505,6 +505,7 @@ foo const importProvidersStr = ` aws + bar foo ` diff --git a/config/merge.go b/config/merge.go index 628f34056..e3f604635 100644 --- a/config/merge.go +++ b/config/merge.go @@ -1,9 +1,5 @@ package config -import ( - "fmt" -) - // Merge merges two configurations into a single configuration. // // Merge allows for the two configurations to have duplicate resources, @@ -24,81 +20,135 @@ func Merge(c1, c2 *Config) (*Config, error) { c.unknownKeys = append(c.unknownKeys, k) } - // Merge variables: Variable merging is quite simple. Set fields in - // later set variables override those earlier. - if len(c1.Variables) > 0 || len(c2.Variables) > 0 { - c.Variables = make([]*Variable, 0, len(c1.Variables)+len(c2.Variables)) - varMap := make(map[string]*Variable) - for _, v := range c1.Variables { - varMap[v.Name] = v - } - for _, v2 := range c2.Variables { - v1, ok := varMap[v2.Name] - if ok { - if v2.Default == "" { - v2.Default = v1.Default - } - if v2.Description == "" { - v2.Description = v1.Description - } - } + // NOTE: Everything below is pretty gross. Due to the lack of generics + // in Go, there is some hoop-jumping involved to make this merging a + // little more test-friendly and less repetitive. Ironically, making it + // less repetitive involves being a little repetitive, but I prefer to + // be repetitive with things that are less error prone than things that + // are more error prone (more logic). Type conversions to an interface + // are pretty low-error. - varMap[v2.Name] = v2 - } - for _, v := range varMap { - c.Variables = append(c.Variables, v) + var m1, m2, mresult []merger + + // Outputs + m1 = make([]merger, 0, len(c1.Outputs)) + m2 = make([]merger, 0, len(c2.Outputs)) + for _, v := range c1.Outputs { + m1 = append(m1, v) + } + for _, v := range c2.Outputs { + m2 = append(m2, v) + } + mresult = mergeSlice(m1, m2) + if len(mresult) > 0 { + c.Outputs = make([]*Output, len(mresult)) + for i, v := range mresult { + c.Outputs[i] = v.(*Output) } } - // Merge outputs: If they collide, just take the latest one for now. In - // the future, we might provide smarter merge functionality. - if len(c1.Outputs) > 0 || len(c2.Outputs) > 0 { - c.Outputs = make([]*Output, 0, len(c1.Outputs)+len(c2.Outputs)) - m := make(map[string]*Output) - for _, v := range c1.Outputs { - m[v.Name] = v - } - for _, v := range c2.Outputs { - m[v.Name] = v - } - for _, v := range m { - c.Outputs = append(c.Outputs, v) + // Provider Configs + m1 = make([]merger, 0, len(c1.ProviderConfigs)) + m2 = make([]merger, 0, len(c2.ProviderConfigs)) + for _, v := range c1.ProviderConfigs { + m1 = append(m1, v) + } + for _, v := range c2.ProviderConfigs { + m2 = append(m2, v) + } + mresult = mergeSlice(m1, m2) + if len(mresult) > 0 { + c.ProviderConfigs = make([]*ProviderConfig, len(mresult)) + for i, v := range mresult { + c.ProviderConfigs[i] = v.(*ProviderConfig) } } - // Merge provider configs: If they collide, we just take the latest one - // for now. In the future, we might provide smarter merge functionality. - if len(c1.ProviderConfigs) > 0 || len(c2.ProviderConfigs) > 0 { - m := make(map[string]*ProviderConfig) - for _, v := range c1.ProviderConfigs { - m[v.Name] = v - } - for _, v := range c2.ProviderConfigs { - m[v.Name] = v - } - - c.ProviderConfigs = make([]*ProviderConfig, 0, len(m)) - for _, v := range m { - c.ProviderConfigs = append(c.ProviderConfigs, v) + // Resources + m1 = make([]merger, 0, len(c1.Resources)) + m2 = make([]merger, 0, len(c2.Resources)) + for _, v := range c1.Resources { + m1 = append(m1, v) + } + for _, v := range c2.Resources { + m2 = append(m2, v) + } + mresult = mergeSlice(m1, m2) + if len(mresult) > 0 { + c.Resources = make([]*Resource, len(mresult)) + for i, v := range mresult { + c.Resources[i] = v.(*Resource) } } - // Merge resources: If they collide, we just take the latest one - // for now. In the future, we might provide smarter merge functionality. - resources := make(map[string]*Resource) - for _, r := range c1.Resources { - id := fmt.Sprintf("%s[%s]", r.Type, r.Name) - resources[id] = r + // Variables + m1 = make([]merger, 0, len(c1.Variables)) + m2 = make([]merger, 0, len(c2.Variables)) + for _, v := range c1.Variables { + m1 = append(m1, v) } - for _, r := range c2.Resources { - id := fmt.Sprintf("%s[%s]", r.Type, r.Name) - resources[id] = r + for _, v := range c2.Variables { + m2 = append(m2, v) } - - c.Resources = make([]*Resource, 0, len(resources)) - for _, r := range resources { - c.Resources = append(c.Resources, r) + mresult = mergeSlice(m1, m2) + if len(mresult) > 0 { + c.Variables = make([]*Variable, len(mresult)) + for i, v := range mresult { + c.Variables[i] = v.(*Variable) + } } return c, nil } + +// merger is an interface that must be implemented by types that are +// merge-able. This simplifies the implementation of Merge for the various +// components of a Config. +type merger interface { + mergerName() string + mergerMerge(merger) merger +} + +// mergeSlice merges a slice of mergers. +func mergeSlice(m1, m2 []merger) []merger { + r := make([]merger, len(m1), len(m1)+len(m2)) + copy(r, m1) + + m := map[string]struct{}{} + for _, v2 := range m2 { + // If we already saw it, just append it because its a + // duplicate and invalid... + name := v2.mergerName() + if _, ok := m[name]; ok { + r = append(r, v2) + continue + } + m[name] = struct{}{} + + // Find an original to override + var original merger + originalIndex := -1 + for i, v := range m1 { + if v.mergerName() == name { + originalIndex = i + original = v + break + } + } + + var v merger + if original == nil { + v = v2 + } else { + v = original.mergerMerge(v2) + } + + if originalIndex == -1 { + r = append(r, v) + } else { + r[originalIndex] = v + } + } + + return r +} diff --git a/config/merge_test.go b/config/merge_test.go new file mode 100644 index 000000000..fb52677b5 --- /dev/null +++ b/config/merge_test.go @@ -0,0 +1,149 @@ +package config + +import ( + "reflect" + "testing" +) + +func TestMerge(t *testing.T) { + cases := []struct { + c1, c2, result *Config + err bool + }{ + // Normal good case. + { + &Config{ + Outputs: []*Output{ + &Output{Name: "foo"}, + }, + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Name: "foo"}, + }, + Resources: []*Resource{ + &Resource{Name: "foo"}, + }, + Variables: []*Variable{ + &Variable{Name: "foo"}, + }, + + unknownKeys: []string{"foo"}, + }, + + &Config{ + Outputs: []*Output{ + &Output{Name: "bar"}, + }, + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Name: "bar"}, + }, + Resources: []*Resource{ + &Resource{Name: "bar"}, + }, + Variables: []*Variable{ + &Variable{Name: "bar"}, + }, + + unknownKeys: []string{"bar"}, + }, + + &Config{ + Outputs: []*Output{ + &Output{Name: "foo"}, + &Output{Name: "bar"}, + }, + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Name: "foo"}, + &ProviderConfig{Name: "bar"}, + }, + Resources: []*Resource{ + &Resource{Name: "foo"}, + &Resource{Name: "bar"}, + }, + Variables: []*Variable{ + &Variable{Name: "foo"}, + &Variable{Name: "bar"}, + }, + + unknownKeys: []string{"foo", "bar"}, + }, + + false, + }, + + // Test that when merging duplicates, it merges into the + // first, but keeps the duplicates so that errors still + // happen. + { + &Config{ + Outputs: []*Output{ + &Output{Name: "foo"}, + }, + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Name: "foo"}, + }, + Resources: []*Resource{ + &Resource{Name: "foo"}, + }, + Variables: []*Variable{ + &Variable{Name: "foo", Default: "foo"}, + &Variable{Name: "foo"}, + }, + + unknownKeys: []string{"foo"}, + }, + + &Config{ + Outputs: []*Output{ + &Output{Name: "bar"}, + }, + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Name: "bar"}, + }, + Resources: []*Resource{ + &Resource{Name: "bar"}, + }, + Variables: []*Variable{ + &Variable{Name: "foo", Default: "bar", defaultSet: true}, + &Variable{Name: "bar"}, + }, + + unknownKeys: []string{"bar"}, + }, + + &Config{ + Outputs: []*Output{ + &Output{Name: "foo"}, + &Output{Name: "bar"}, + }, + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Name: "foo"}, + &ProviderConfig{Name: "bar"}, + }, + Resources: []*Resource{ + &Resource{Name: "foo"}, + &Resource{Name: "bar"}, + }, + Variables: []*Variable{ + &Variable{Name: "foo", Default: "bar", defaultSet: true}, + &Variable{Name: "foo"}, + &Variable{Name: "bar"}, + }, + + unknownKeys: []string{"foo", "bar"}, + }, + + false, + }, + } + + for i, tc := range cases { + actual, err := Merge(tc.c1, tc.c2) + if (err != nil) != tc.err { + t.Fatalf("%d: error fail", i) + } + + if !reflect.DeepEqual(actual, tc.result) { + t.Fatalf("%d: bad:\n\n%#v", i, actual) + } + } +} diff --git a/config/raw_config.go b/config/raw_config.go index 12a43c54b..f16921fd1 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -92,6 +92,25 @@ func (r *RawConfig) init() error { return nil } +func (r *RawConfig) merge(r2 *RawConfig) *RawConfig { + rawRaw, err := copystructure.Copy(r.Raw) + if err != nil { + panic(err) + } + + raw := rawRaw.(map[string]interface{}) + for k, v := range r2.Raw { + raw[k] = v + } + + result, err := NewRawConfig(raw) + if err != nil { + panic(err) + } + + return result +} + // UnknownKeys returns the keys of the configuration that are unknown // because they had interpolated variables that must be computed. func (r *RawConfig) UnknownKeys() []string {