From 8403a465bce8306d63053f2e8afff800dcc4feb9 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 23 Jun 2016 14:29:13 +0300 Subject: [PATCH 1/4] core: Add test and fix for element with empty list The incldued test previously caused a panic, it now returns an error message explaining the issue. --- config/interpolate_funcs.go | 3 +++ config/interpolate_funcs_test.go | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 0ab4ce56b7..e912493b69 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -667,6 +667,9 @@ func interpolationFuncElement() ast.Function { ReturnType: ast.TypeString, Callback: func(args []interface{}) (interface{}, error) { list := args[0].([]ast.Variable) + if len(list) == 0 { + return nil, fmt.Errorf("element() may not be used with an empty list") + } index, err := strconv.Atoi(args[1].(string)) if err != nil || index < 0 { diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index 6a69d533ac..8bd03b1117 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -952,6 +952,7 @@ func TestInterpolateFuncElement(t *testing.T) { Vars: map[string]ast.Variable{ "var.a_list": interfaceToVariableSwallowError([]string{"foo", "baz"}), "var.a_short_list": interfaceToVariableSwallowError([]string{"foo"}), + "var.empty_list": interfaceToVariableSwallowError([]interface{}{}), }, Cases: []testFunctionCase{ { @@ -980,6 +981,13 @@ func TestInterpolateFuncElement(t *testing.T) { true, }, + // Empty list should fail + { + `${element(var.empty_list, 0)}`, + nil, + true, + }, + // Too many args { `${element(var.a_list, "0", "2")}`, From 17f47770390ef0584b2107f38ef14a2bf07948a5 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 23 Jun 2016 14:32:00 +0300 Subject: [PATCH 2/4] core: Fix Stringer on OutputState for types The implementation of Stringer on OutputState previously assumed outputs may only be strings - we now no longer cast to string, instead using the built in formatting directives. --- terraform/state.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/terraform/state.go b/terraform/state.go index 4ca0206854..80691fd820 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -635,8 +635,7 @@ type OutputState struct { } func (s *OutputState) String() string { - // This is a v0.6.x implementation only - return fmt.Sprintf("%s", s.Value.(string)) + return fmt.Sprintf("%#v", s.Value) } // Equal compares two OutputState structures for equality. nil values are From 983e4f13c613ae361b52984abde1a6397e549d09 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 23 Jun 2016 21:11:36 +0100 Subject: [PATCH 3/4] core: Add context test for empty lists as module outputs This test illustrates a failure which occurs during the Input walk, if an interpolation is used with the input of a splat operation resulting in a multi-variable. The bug was found during use of the RC2, but does not correspond to an open issue at present. --- terraform/context_input_test.go | 21 +++++++++++++++++++ .../main.tf | 9 ++++++++ .../moda/main.tf | 3 +++ .../modb/main.tf | 7 +++++++ 4 files changed, 40 insertions(+) create mode 100644 terraform/test-fixtures/input-module-computed-output-element/main.tf create mode 100644 terraform/test-fixtures/input-module-computed-output-element/moda/main.tf create mode 100644 terraform/test-fixtures/input-module-computed-output-element/modb/main.tf diff --git a/terraform/context_input_test.go b/terraform/context_input_test.go index 08e017b4cc..f953645c89 100644 --- a/terraform/context_input_test.go +++ b/terraform/context_input_test.go @@ -49,6 +49,27 @@ func TestContext2Input(t *testing.T) { } } +func TestContext2Input_moduleComputedOutputElement(t *testing.T) { + m := testModule(t, "input-module-computed-output-element") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + p.InputFn = func(i UIInput, c *ResourceConfig) (*ResourceConfig, error) { + return c, nil + } + + if err := ctx.Input(InputModeStd); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestContext2Input_badVarDefault(t *testing.T) { m := testModule(t, "input-bad-var-default") p := testProvider("aws") diff --git a/terraform/test-fixtures/input-module-computed-output-element/main.tf b/terraform/test-fixtures/input-module-computed-output-element/main.tf new file mode 100644 index 0000000000..bb96e24a3e --- /dev/null +++ b/terraform/test-fixtures/input-module-computed-output-element/main.tf @@ -0,0 +1,9 @@ +module "b" { + source = "./modb" +} + +module "a" { + source = "./moda" + + single_element = "${element(module.b.computed_list, 0)}" +} diff --git a/terraform/test-fixtures/input-module-computed-output-element/moda/main.tf b/terraform/test-fixtures/input-module-computed-output-element/moda/main.tf new file mode 100644 index 0000000000..eb09eb192e --- /dev/null +++ b/terraform/test-fixtures/input-module-computed-output-element/moda/main.tf @@ -0,0 +1,3 @@ +variable "single_element" { + type = "string" +} diff --git a/terraform/test-fixtures/input-module-computed-output-element/modb/main.tf b/terraform/test-fixtures/input-module-computed-output-element/modb/main.tf new file mode 100644 index 0000000000..ebe4a7eff7 --- /dev/null +++ b/terraform/test-fixtures/input-module-computed-output-element/modb/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "test" { + count = 3 +} + +output "computed_list" { + value = ["${aws_instance.test.*.id}"] +} From 2356afde84ba499a2d977360f5a52dfe205227e3 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 23 Jun 2016 21:14:09 +0100 Subject: [PATCH 4/4] core: Fix interpolation of unknown multi-variables This commit test "TestContext2Input_moduleComputedOutputElement" by ensuring that we treat a count of zero and non-reified resources independently rather than returning an empty list for both, which results in an interpolation failure when using the element function or indexing. --- terraform/interpolate.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/terraform/interpolate.go b/terraform/interpolate.go index f03154141c..870fac762e 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -162,7 +162,6 @@ func (i *Interpolater) valueModuleVar( } else { // Same reasons as the comment above. result[n] = unknownVariable() - } } @@ -485,11 +484,15 @@ func (i *Interpolater) computeResourceMultiVariable( err) } - // If we have no module in the state yet or count, return empty - if module == nil || len(module.Resources) == 0 || count == 0 { + // If count is zero, we return an empty list + if count == 0 { return &ast.Variable{Type: ast.TypeList, Value: []ast.Variable{}}, nil } + // If we have no module in the state yet or count, return unknown + if module == nil || len(module.Resources) == 0 { + return &unknownVariable, nil + } var values []string for j := 0; j < count; j++ { id := fmt.Sprintf("%s.%d", v.ResourceId(), j)