From 3878b8b0939bbbc9aaa734b9c07be2a674d81525 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Dec 2016 21:48:59 -0800 Subject: [PATCH 1/2] config: Merge respects Terraform blocks, provider aliases, and more Fixes #10715 `config.Merge` was not updated to support a number of new features. This updates the codepath to merge various fields, including the `terraform` block which was the issue in #10715. The `Merge` API is called when an `_override` file is present to _merge_ configurations. Normally configurations are _appended_. Only an override file triggers a _merge_. I started working on a generic library to do this automatically awhile back but never finished it. This might motivate me to do so. In the interest of getting a fix out though, we'll continue the manual approach. --- config/config.go | 10 ++ config/merge.go | 6 + config/merge_test.go | 281 +++++++++++++++++++++++++++++++++++++++++++ config/raw_config.go | 14 ++- 4 files changed, 309 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index c7e4f351d..34f80d4f3 100644 --- a/config/config.go +++ b/config/config.go @@ -915,7 +915,10 @@ func (o *Output) mergerMerge(m merger) merger { result := *o result.Name = o2.Name + result.Description = o2.Description result.RawConfig = result.RawConfig.merge(o2.RawConfig) + result.Sensitive = o2.Sensitive + result.DependsOn = o2.DependsOn return &result } @@ -943,6 +946,10 @@ func (c *ProviderConfig) mergerMerge(m merger) merger { result.Name = c2.Name result.RawConfig = result.RawConfig.merge(c2.RawConfig) + if c2.Alias != "" { + result.Alias = c2.Alias + } + return &result } @@ -978,6 +985,9 @@ func (v *Variable) Merge(v2 *Variable) *Variable { // The names should be the same, but the second name always wins. result.Name = v2.Name + if v2.DeclaredType != "" { + result.DeclaredType = v2.DeclaredType + } if v2.Default != nil { result.Default = v2.Default } diff --git a/config/merge.go b/config/merge.go index f72fdfa92..2e7686594 100644 --- a/config/merge.go +++ b/config/merge.go @@ -32,6 +32,12 @@ func Merge(c1, c2 *Config) (*Config, error) { c.Atlas = c2.Atlas } + // Merge the Terraform configuration, which is a complete overwrite. + c.Terraform = c1.Terraform + if c2.Terraform != nil { + c.Terraform = c2.Terraform + } + // 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 diff --git a/config/merge_test.go b/config/merge_test.go index 6fe55a2d5..b1d27b6db 100644 --- a/config/merge_test.go +++ b/config/merge_test.go @@ -153,6 +153,287 @@ func TestMerge(t *testing.T) { false, }, + + // Terraform block + { + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + &Config{}, + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + false, + }, + + { + &Config{}, + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + false, + }, + + // Provider alias + { + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "foo"}, + }, + }, + &Config{}, + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "foo"}, + }, + }, + false, + }, + + { + &Config{}, + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "foo"}, + }, + }, + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "foo"}, + }, + }, + false, + }, + + { + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "bar"}, + }, + }, + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "foo"}, + }, + }, + &Config{ + ProviderConfigs: []*ProviderConfig{ + &ProviderConfig{Alias: "foo"}, + }, + }, + false, + }, + + // Variable type + { + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "foo"}, + }, + }, + &Config{}, + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "foo"}, + }, + }, + false, + }, + + { + &Config{}, + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "foo"}, + }, + }, + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "foo"}, + }, + }, + false, + }, + + { + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "bar"}, + }, + }, + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "foo"}, + }, + }, + &Config{ + Variables: []*Variable{ + &Variable{DeclaredType: "foo"}, + }, + }, + false, + }, + + // Output description + { + &Config{ + Outputs: []*Output{ + &Output{Description: "foo"}, + }, + }, + &Config{}, + &Config{ + Outputs: []*Output{ + &Output{Description: "foo"}, + }, + }, + false, + }, + + { + &Config{}, + &Config{ + Outputs: []*Output{ + &Output{Description: "foo"}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{Description: "foo"}, + }, + }, + false, + }, + + { + &Config{ + Outputs: []*Output{ + &Output{Description: "bar"}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{Description: "foo"}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{Description: "foo"}, + }, + }, + false, + }, + + // Output depends_on + { + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"foo"}}, + }, + }, + &Config{}, + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"foo"}}, + }, + }, + false, + }, + + { + &Config{}, + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"foo"}}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"foo"}}, + }, + }, + false, + }, + + { + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"bar"}}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"foo"}}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{DependsOn: []string{"foo"}}, + }, + }, + false, + }, + + // Output sensitive + { + &Config{ + Outputs: []*Output{ + &Output{Sensitive: true}, + }, + }, + &Config{}, + &Config{ + Outputs: []*Output{ + &Output{Sensitive: true}, + }, + }, + false, + }, + + { + &Config{}, + &Config{ + Outputs: []*Output{ + &Output{Sensitive: true}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{Sensitive: true}, + }, + }, + false, + }, + + { + &Config{ + Outputs: []*Output{ + &Output{Sensitive: false}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{Sensitive: true}, + }, + }, + &Config{ + Outputs: []*Output{ + &Output{Sensitive: true}, + }, + }, + false, + }, } for i, tc := range cases { diff --git a/config/raw_config.go b/config/raw_config.go index 3bcc62412..f8498d85c 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -240,14 +240,24 @@ func (r *RawConfig) interpolate(fn interpolationWalkerFunc) error { } func (r *RawConfig) merge(r2 *RawConfig) *RawConfig { + if r == nil && r2 == nil { + return nil + } + + if r == nil { + r = &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 + if r2 != nil { + for k, v := range r2.Raw { + raw[k] = v + } } result, err := NewRawConfig(raw) From c2c5668a8d8e1925d445e1d2aca1d4f9e7182414 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Dec 2016 21:53:02 -0800 Subject: [PATCH 2/2] config: Append supports `terraform` --- config/append.go | 5 +++++ config/append_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/config/append.go b/config/append.go index bf13534e7..a421df4a0 100644 --- a/config/append.go +++ b/config/append.go @@ -35,6 +35,11 @@ func Append(c1, c2 *Config) (*Config, error) { c.Atlas = c2.Atlas } + c.Terraform = c1.Terraform + if c2.Terraform != nil { + c.Terraform = c2.Terraform + } + if len(c1.Modules) > 0 || len(c2.Modules) > 0 { c.Modules = make( []*Module, 0, len(c1.Modules)+len(c2.Modules)) diff --git a/config/append_test.go b/config/append_test.go index 8d6258ecd..aecb80e66 100644 --- a/config/append_test.go +++ b/config/append_test.go @@ -87,6 +87,37 @@ func TestAppend(t *testing.T) { false, }, + + // Terraform block + { + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + &Config{}, + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + false, + }, + + { + &Config{}, + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + &Config{ + Terraform: &Terraform{ + RequiredVersion: "A", + }, + }, + false, + }, } for i, tc := range cases {