From fc4ac52014c2ed80f72b1d21278ea4ecdf3aa0ec Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 19 May 2016 10:46:51 -0700 Subject: [PATCH 1/3] Module variables not being interpolated Variables weren't being interpolated during the Input phase, causing a syntax error on the interpolation string. Adding `walkInput` to the EvalTree operations prevents skipping the interpolation step. --- terraform/context_input_test.go | 23 +++++++++++++++++++ terraform/eval_context_builtin.go | 1 + terraform/eval_interpolate.go | 4 +--- terraform/graph_config_node_output.go | 2 +- terraform/interpolate.go | 1 + .../input-interpolate-var/child/main.tf | 5 ++++ .../input-interpolate-var/main.tf | 7 ++++++ .../input-interpolate-var/source/main.tf | 1 + 8 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 terraform/test-fixtures/input-interpolate-var/child/main.tf create mode 100644 terraform/test-fixtures/input-interpolate-var/main.tf create mode 100644 terraform/test-fixtures/input-interpolate-var/source/main.tf diff --git a/terraform/context_input_test.go b/terraform/context_input_test.go index dae45e0d0..08e017b4c 100644 --- a/terraform/context_input_test.go +++ b/terraform/context_input_test.go @@ -569,3 +569,26 @@ func TestContext2Input_varPartiallyComputed(t *testing.T) { t.Fatalf("err: %s", err) } } + +// Module variables weren't being interpolated during the Input walk. +// https://github.com/hashicorp/terraform/issues/5322 +func TestContext2Input_interpolateVar(t *testing.T) { + input := new(MockUIInput) + + m := testModule(t, "input-interpolate-var") + p := testProvider("null") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "template": testProviderFuncFixed(p), + }, + UIInput: input, + }) + + if err := ctx.Input(InputModeStd); err != nil { + t.Fatalf("err: %s", err) + } +} diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 4dff93a4c..d85cb77fc 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -291,6 +291,7 @@ func (ctx *BuiltinEvalContext) Interpolate( Path: ctx.Path(), Resource: r, } + vs, err := ctx.Interpolater.Values(scope, cfg.Variables) if err != nil { return nil, err diff --git a/terraform/eval_interpolate.go b/terraform/eval_interpolate.go index 063473079..6825ff590 100644 --- a/terraform/eval_interpolate.go +++ b/terraform/eval_interpolate.go @@ -1,8 +1,6 @@ package terraform -import ( - "github.com/hashicorp/terraform/config" -) +import "github.com/hashicorp/terraform/config" // EvalInterpolate is an EvalNode implementation that takes a raw // configuration and interpolates it. diff --git a/terraform/graph_config_node_output.go b/terraform/graph_config_node_output.go index 8410102d2..ac364a308 100644 --- a/terraform/graph_config_node_output.go +++ b/terraform/graph_config_node_output.go @@ -44,7 +44,7 @@ func (n *GraphNodeConfigOutput) DependentOn() []string { // GraphNodeEvalable impl. func (n *GraphNodeConfigOutput) EvalTree() EvalNode { return &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy}, + Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy, walkInput}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalWriteOutput{ diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 0a627aca6..8ed74490d 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -129,6 +129,7 @@ func (i *Interpolater) valueModuleVar( n string, v *config.ModuleVariable, result map[string]ast.Variable) error { + // If we're computing all dynamic fields, then module vars count // and we mark it as computed. if i.Operation == walkValidate { diff --git a/terraform/test-fixtures/input-interpolate-var/child/main.tf b/terraform/test-fixtures/input-interpolate-var/child/main.tf new file mode 100644 index 000000000..9a2ff39cc --- /dev/null +++ b/terraform/test-fixtures/input-interpolate-var/child/main.tf @@ -0,0 +1,5 @@ +variable "list" { } +resource "template_file" "temp" { + count = "${length(split(",", var.list))}" + template = "foo" +} diff --git a/terraform/test-fixtures/input-interpolate-var/main.tf b/terraform/test-fixtures/input-interpolate-var/main.tf new file mode 100644 index 000000000..ae6cf627f --- /dev/null +++ b/terraform/test-fixtures/input-interpolate-var/main.tf @@ -0,0 +1,7 @@ +module "source" { + source = "./source" +} +module "child" { + source = "./child" + list = "${module.source.list}" +} diff --git a/terraform/test-fixtures/input-interpolate-var/source/main.tf b/terraform/test-fixtures/input-interpolate-var/source/main.tf new file mode 100644 index 000000000..fb7b33fb3 --- /dev/null +++ b/terraform/test-fixtures/input-interpolate-var/source/main.tf @@ -0,0 +1 @@ +output "list" { value = "foo,bar,baz" } From ed042ab0675ff539bafcfb3cb783825087c292bf Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 23 May 2016 12:46:06 -0400 Subject: [PATCH 2/3] Interpolation also skipped during Validate phase Adding walkValidate to the EvalTree operations, and removing the walkValidate guard from the Interpolater.valueModuleVar allows the values to be interpolated for Validate. --- terraform/context_validate_test.go | 27 +++++++++++++++++++++++++++ terraform/graph_config_node_output.go | 3 ++- terraform/interpolate.go | 10 ---------- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index af8d7d3c4..3258c093a 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -749,3 +749,30 @@ func TestContext2Validate_varRefFilled(t *testing.T) { t.Fatalf("bad: %#v", value) } } + +// Module variables weren't being interpolated during Validate phase. +// related to https://github.com/hashicorp/terraform/issues/5322 +func TestContext2Validate_interpolateVar(t *testing.T) { + input := new(MockUIInput) + + m := testModule(t, "input-interpolate-var") + p := testProvider("null") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "template": testProviderFuncFixed(p), + }, + UIInput: input, + }) + + w, e := ctx.Validate() + if w != nil { + t.Log("warnings:", w) + } + if e != nil { + t.Fatal("err:", e) + } +} diff --git a/terraform/graph_config_node_output.go b/terraform/graph_config_node_output.go index ac364a308..0704a0cb7 100644 --- a/terraform/graph_config_node_output.go +++ b/terraform/graph_config_node_output.go @@ -44,7 +44,8 @@ func (n *GraphNodeConfigOutput) DependentOn() []string { // GraphNodeEvalable impl. func (n *GraphNodeConfigOutput) EvalTree() EvalNode { return &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply, walkDestroy, walkInput}, + Ops: []walkOperation{walkRefresh, walkPlan, walkApply, + walkDestroy, walkInput, walkValidate}, Node: &EvalSequence{ Nodes: []EvalNode{ &EvalWriteOutput{ diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 8ed74490d..2d3c57c5b 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -130,16 +130,6 @@ func (i *Interpolater) valueModuleVar( v *config.ModuleVariable, result map[string]ast.Variable) error { - // If we're computing all dynamic fields, then module vars count - // and we mark it as computed. - if i.Operation == walkValidate { - result[n] = ast.Variable{ - Value: config.UnknownVariableValue, - Type: ast.TypeString, - } - return nil - } - // Build the path to the child module we want path := make([]string, len(scope.Path), len(scope.Path)+1) copy(path, scope.Path) From b205ac2123159e0873edcf3a3057638636995f36 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 23 May 2016 15:54:14 -0500 Subject: [PATCH 3/3] core: Another validate test for computed module var refs I wanted to make sure this case was handled, and it is! So here's an extra test for us. --- terraform/context_validate_test.go | 27 +++++++++++++++++++ .../dest/main.tf | 5 ++++ .../validate-computed-module-var-ref/main.tf | 8 ++++++ .../source/main.tf | 3 +++ 4 files changed, 43 insertions(+) create mode 100644 terraform/test-fixtures/validate-computed-module-var-ref/dest/main.tf create mode 100644 terraform/test-fixtures/validate-computed-module-var-ref/main.tf create mode 100644 terraform/test-fixtures/validate-computed-module-var-ref/source/main.tf diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 3258c093a..56de23a36 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -776,3 +776,30 @@ func TestContext2Validate_interpolateVar(t *testing.T) { t.Fatal("err:", e) } } + +// When module vars reference something that is actually computed, this +// shouldn't cause validation to fail. +func TestContext2Validate_interpolateComputedModuleVarDef(t *testing.T) { + input := new(MockUIInput) + + m := testModule(t, "validate-computed-module-var-ref") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + UIInput: input, + }) + + w, e := ctx.Validate() + if w != nil { + t.Log("warnings:", w) + } + if e != nil { + t.Fatal("err:", e) + } +} diff --git a/terraform/test-fixtures/validate-computed-module-var-ref/dest/main.tf b/terraform/test-fixtures/validate-computed-module-var-ref/dest/main.tf new file mode 100644 index 000000000..44095ea75 --- /dev/null +++ b/terraform/test-fixtures/validate-computed-module-var-ref/dest/main.tf @@ -0,0 +1,5 @@ +variable "destin" { } + +resource "aws_instance" "dest" { + attr = "${var.destin}" +} diff --git a/terraform/test-fixtures/validate-computed-module-var-ref/main.tf b/terraform/test-fixtures/validate-computed-module-var-ref/main.tf new file mode 100644 index 000000000..d7c799cc8 --- /dev/null +++ b/terraform/test-fixtures/validate-computed-module-var-ref/main.tf @@ -0,0 +1,8 @@ +module "source" { + source = "./source" +} + +module "dest" { + source = "./dest" + destin = "${module.source.sourceout}" +} diff --git a/terraform/test-fixtures/validate-computed-module-var-ref/source/main.tf b/terraform/test-fixtures/validate-computed-module-var-ref/source/main.tf new file mode 100644 index 000000000..f923e8080 --- /dev/null +++ b/terraform/test-fixtures/validate-computed-module-var-ref/source/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "source" { } + +output "sourceout" { value = "${aws_instance.source.id}" }