config: merge/append for local values

It seems that this somehow got lost in the commit/rebase shuffle and
wasn't caught by the tests that _did_ make it because they were all using
just one file.

As a result of this bug, locals would fail to work correctly in any
configuration with more than one .tf file.

Along with restoring the append/merge behavior, this also reworks some of
the tests to exercise the multi-file case as better insurance against
regressions of this sort in future.

This fixes #15969.
This commit is contained in:
Martin Atkins 2017-08-31 11:47:10 -07:00
parent 6bd9e3f2ab
commit 8cd0ee80e5
9 changed files with 112 additions and 22 deletions

View File

@ -82,5 +82,11 @@ func Append(c1, c2 *Config) (*Config, error) {
c.Variables = append(c.Variables, c2.Variables...) c.Variables = append(c.Variables, c2.Variables...)
} }
if len(c1.Locals) > 0 || len(c2.Locals) > 0 {
c.Locals = make([]*Local, 0, len(c1.Locals)+len(c2.Locals))
c.Locals = append(c.Locals, c1.Locals...)
c.Locals = append(c.Locals, c2.Locals...)
}
return c, nil return c, nil
} }

View File

@ -1,8 +1,11 @@
package config package config
import ( import (
"fmt"
"reflect" "reflect"
"testing" "testing"
"github.com/davecgh/go-spew/spew"
) )
func TestAppend(t *testing.T) { func TestAppend(t *testing.T) {
@ -30,6 +33,9 @@ func TestAppend(t *testing.T) {
Variables: []*Variable{ Variables: []*Variable{
&Variable{Name: "foo"}, &Variable{Name: "foo"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
},
unknownKeys: []string{"foo"}, unknownKeys: []string{"foo"},
}, },
@ -53,6 +59,9 @@ func TestAppend(t *testing.T) {
Variables: []*Variable{ Variables: []*Variable{
&Variable{Name: "bar"}, &Variable{Name: "bar"},
}, },
Locals: []*Local{
&Local{Name: "bar"},
},
unknownKeys: []string{"bar"}, unknownKeys: []string{"bar"},
}, },
@ -81,6 +90,10 @@ func TestAppend(t *testing.T) {
&Variable{Name: "foo"}, &Variable{Name: "foo"},
&Variable{Name: "bar"}, &Variable{Name: "bar"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
&Local{Name: "bar"},
},
unknownKeys: []string{"foo", "bar"}, unknownKeys: []string{"foo", "bar"},
}, },
@ -146,13 +159,15 @@ func TestAppend(t *testing.T) {
} }
for i, tc := range cases { for i, tc := range cases {
t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) {
actual, err := Append(tc.c1, tc.c2) actual, err := Append(tc.c1, tc.c2)
if err != nil != tc.err { if err != nil != tc.err {
t.Fatalf("%d: error fail", i) t.Errorf("unexpected error: %s", err)
} }
if !reflect.DeepEqual(actual, tc.result) { if !reflect.DeepEqual(actual, tc.result) {
t.Fatalf("%d: bad:\n\n%#v", i, actual) t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(tc.result))
} }
})
} }
} }

View File

@ -653,6 +653,22 @@ func TestNameRegexp(t *testing.T) {
} }
} }
func TestConfigValidate_localValuesMultiFile(t *testing.T) {
c, err := LoadDir(filepath.Join(fixtureDir, "validate-local-multi-file"))
if err != nil {
t.Fatalf("unexpected error during load: %s", err)
}
if err := c.Validate(); err != nil {
t.Fatalf("unexpected error from validate: %s", err)
}
if len(c.Locals) != 1 {
t.Fatalf("got 0 locals; want 1")
}
if got, want := c.Locals[0].Name, "test"; got != want {
t.Errorf("wrong local name\ngot: %#v\nwant: %#v", got, want)
}
}
func TestProviderConfigName(t *testing.T) { func TestProviderConfigName(t *testing.T) {
pcs := []*ProviderConfig{ pcs := []*ProviderConfig{
&ProviderConfig{Name: "aw"}, &ProviderConfig{Name: "aw"},

View File

@ -137,6 +137,21 @@ func Merge(c1, c2 *Config) (*Config, error) {
} }
} }
// Local Values
// These are simpler than the other config elements because they are just
// flat values and so no deep merging is required.
if localsCount := len(c1.Locals) + len(c2.Locals); localsCount != 0 {
// Explicit length check above because we want c.Locals to remain
// nil if the result would be empty.
c.Locals = make([]*Local, 0, len(c1.Locals)+len(c2.Locals))
for _, v := range c1.Locals {
c.Locals = append(c.Locals, v)
}
for _, v := range c2.Locals {
c.Locals = append(c.Locals, v)
}
}
return c, nil return c, nil
} }

View File

@ -1,8 +1,11 @@
package config package config
import ( import (
"fmt"
"reflect" "reflect"
"testing" "testing"
"github.com/davecgh/go-spew/spew"
) )
func TestMerge(t *testing.T) { func TestMerge(t *testing.T) {
@ -31,6 +34,9 @@ func TestMerge(t *testing.T) {
Variables: []*Variable{ Variables: []*Variable{
&Variable{Name: "foo"}, &Variable{Name: "foo"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
},
unknownKeys: []string{"foo"}, unknownKeys: []string{"foo"},
}, },
@ -54,6 +60,9 @@ func TestMerge(t *testing.T) {
Variables: []*Variable{ Variables: []*Variable{
&Variable{Name: "bar"}, &Variable{Name: "bar"},
}, },
Locals: []*Local{
&Local{Name: "bar"},
},
unknownKeys: []string{"bar"}, unknownKeys: []string{"bar"},
}, },
@ -82,6 +91,10 @@ func TestMerge(t *testing.T) {
&Variable{Name: "foo"}, &Variable{Name: "foo"},
&Variable{Name: "bar"}, &Variable{Name: "bar"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
&Local{Name: "bar"},
},
unknownKeys: []string{"foo", "bar"}, unknownKeys: []string{"foo", "bar"},
}, },
@ -107,6 +120,9 @@ func TestMerge(t *testing.T) {
&Variable{Name: "foo", Default: "foo"}, &Variable{Name: "foo", Default: "foo"},
&Variable{Name: "foo"}, &Variable{Name: "foo"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
},
unknownKeys: []string{"foo"}, unknownKeys: []string{"foo"},
}, },
@ -125,6 +141,9 @@ func TestMerge(t *testing.T) {
&Variable{Name: "foo", Default: "bar"}, &Variable{Name: "foo", Default: "bar"},
&Variable{Name: "bar"}, &Variable{Name: "bar"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
},
unknownKeys: []string{"bar"}, unknownKeys: []string{"bar"},
}, },
@ -147,6 +166,10 @@ func TestMerge(t *testing.T) {
&Variable{Name: "foo"}, &Variable{Name: "foo"},
&Variable{Name: "bar"}, &Variable{Name: "bar"},
}, },
Locals: []*Local{
&Local{Name: "foo"},
&Local{Name: "foo"},
},
unknownKeys: []string{"foo", "bar"}, unknownKeys: []string{"foo", "bar"},
}, },
@ -462,13 +485,15 @@ func TestMerge(t *testing.T) {
} }
for i, tc := range cases { for i, tc := range cases {
t.Run(fmt.Sprintf("%02d", i), func(t *testing.T) {
actual, err := Merge(tc.c1, tc.c2) actual, err := Merge(tc.c1, tc.c2)
if err != nil != tc.err { if err != nil != tc.err {
t.Fatalf("%d: error fail", i) t.Errorf("unexpected error: %s", err)
} }
if !reflect.DeepEqual(actual, tc.result) { if !reflect.DeepEqual(actual, tc.result) {
t.Fatalf("%d: bad:\n\n%#v", i, actual) t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(actual), spew.Sdump(tc.result))
} }
})
} }
} }

View File

@ -0,0 +1,4 @@
locals {
test = "${upper("hello, world")}"
}

View File

@ -0,0 +1,8 @@
resource "test_resource" "foo" {
value = "${local.test}"
}
output "test" {
value = "${local.test}"
}

View File

@ -8,11 +8,3 @@ locals {
result_2 = "${local.result_1}" result_2 = "${local.result_1}"
result_3 = "${local.result_2} world" result_3 = "${local.result_2} world"
} }
output "result_1" {
value = "${local.result_1}"
}
output "result_3" {
value = "${local.result_3}"
}

View File

@ -0,0 +1,9 @@
# These are in a separate file to make sure config merging is working properly
output "result_1" {
value = "${local.result_1}"
}
output "result_3" {
value = "${local.result_3}"
}