From 97bb7cb65c46ab9ab862ecae65d6943b04f37024 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 21 Jul 2017 12:41:29 -0400 Subject: [PATCH 1/2] Don't allow interpolation failure to stop Input Allow module variables to fail interpolation during input. This is OK since they will be verified again during Plan. Because Input happens before Refresh, module variable interpolation can fail when referencing values that aren't yet in the state, but are expected after Refresh. --- terraform/eval_interpolate.go | 31 +++++++++++++++++++++++++- terraform/eval_state.go | 4 +++- terraform/graph_builder_input.go | 3 +++ terraform/graph_builder_plan.go | 8 ++++++- terraform/node_module_variable.go | 23 +++++++++++++++---- terraform/transform_module_variable.go | 2 ++ 6 files changed, 64 insertions(+), 7 deletions(-) diff --git a/terraform/eval_interpolate.go b/terraform/eval_interpolate.go index 6825ff590..df3bcb98e 100644 --- a/terraform/eval_interpolate.go +++ b/terraform/eval_interpolate.go @@ -1,6 +1,10 @@ package terraform -import "github.com/hashicorp/terraform/config" +import ( + "log" + + "github.com/hashicorp/terraform/config" +) // EvalInterpolate is an EvalNode implementation that takes a raw // configuration and interpolates it. @@ -22,3 +26,28 @@ func (n *EvalInterpolate) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } + +// EvalTryInterpolate is an EvalNode implementation that takes a raw +// configuration and interpolates it, but only logs a warning on an +// interpolation error, and stops further Eval steps. +// This is used during Input where a value may not be known before Refresh, but +// we don't want to block Input. +type EvalTryInterpolate struct { + Config *config.RawConfig + Resource *Resource + Output **ResourceConfig +} + +func (n *EvalTryInterpolate) Eval(ctx EvalContext) (interface{}, error) { + rc, err := ctx.Interpolate(n.Config, n.Resource) + if err != nil { + log.Printf("[WARN] Interpolation %q failed: %s", n.Config.Key, err) + return nil, EvalEarlyExitError{} + } + + if n.Output != nil { + *n.Output = rc + } + + return nil, nil +} diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 126a0e63a..1f67e3d86 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -1,6 +1,8 @@ package terraform -import "fmt" +import ( + "fmt" +) // EvalReadState is an EvalNode implementation that reads the // primary InstanceState for a specific resource out of the state. diff --git a/terraform/graph_builder_input.go b/terraform/graph_builder_input.go index 0df48cdb8..10fd8b1e9 100644 --- a/terraform/graph_builder_input.go +++ b/terraform/graph_builder_input.go @@ -10,6 +10,9 @@ import ( // and is based on the PlanGraphBuilder. The PlanGraphBuilder passed in will be // modified and should not be used for any other operations. func InputGraphBuilder(p *PlanGraphBuilder) GraphBuilder { + // convert this to an InputPlan + p.Input = true + // We're going to customize the concrete functions p.CustomConcrete = true diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 4b29bbb4b..9c7e4c1db 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -40,6 +40,9 @@ type PlanGraphBuilder struct { // Validate will do structural validation of the graph. Validate bool + // Input represents that this builder is for an Input operation. + Input bool + // CustomConcrete can be set to customize the node types created // for various parts of the plan. This is useful in order to customize // the plan behavior. @@ -107,7 +110,10 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { ), // Add module variables - &ModuleVariableTransformer{Module: b.Module}, + &ModuleVariableTransformer{ + Module: b.Module, + Input: b.Input, + }, // Connect so that the references are ready for targeting. We'll // have to connect again later for providers and so on. diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index 13fe8fc3a..63b84a9c5 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -15,6 +15,9 @@ type NodeApplyableModuleVariable struct { Value *config.RawConfig // Value is the value that is set Module *module.Tree // Antiquated, want to remove + + // Input is set if this graph was created for the Input operation. + Input bool } func (n *NodeApplyableModuleVariable) Name() string { @@ -92,12 +95,24 @@ func (n *NodeApplyableModuleVariable) EvalTree() EvalNode { // within the variables mapping. var config *ResourceConfig variables := make(map[string]interface{}) + + var interpolate EvalNode + + if n.Input { + interpolate = &EvalTryInterpolate{ + Config: n.Value, + Output: &config, + } + } else { + interpolate = &EvalInterpolate{ + Config: n.Value, + Output: &config, + } + } + return &EvalSequence{ Nodes: []EvalNode{ - &EvalInterpolate{ - Config: n.Value, - Output: &config, - }, + interpolate, &EvalVariableBlock{ Config: &config, diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go index 467950bdc..dbfd16871 100644 --- a/terraform/transform_module_variable.go +++ b/terraform/transform_module_variable.go @@ -17,6 +17,7 @@ type ModuleVariableTransformer struct { Module *module.Tree DisablePrune bool // True if pruning unreferenced should be disabled + Input bool // True if this is from an Input operation. } func (t *ModuleVariableTransformer) Transform(g *Graph) error { @@ -99,6 +100,7 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, m *module. Config: v, Value: value, Module: t.Module, + Input: t.Input, } if !t.DisablePrune { From 1664d4e228094fdb416c1aff633f772b75c8b0a4 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 10 Aug 2017 14:12:59 -0400 Subject: [PATCH 2/2] test with bad interpolation during Input The interpolation going into a module variable here will be valid after Refresh, but Refresh doesn't happen for the Input phase. --- terraform/context_input_test.go | 54 +++++++++++++++++++ .../input-module-data-vars/child/main.tf | 5 ++ .../input-module-data-vars/main.tf | 8 +++ 3 files changed, 67 insertions(+) create mode 100644 terraform/test-fixtures/input-module-data-vars/child/main.tf create mode 100644 terraform/test-fixtures/input-module-data-vars/main.tf diff --git a/terraform/context_input_test.go b/terraform/context_input_test.go index 928c11477..750db918a 100644 --- a/terraform/context_input_test.go +++ b/terraform/context_input_test.go @@ -719,3 +719,57 @@ func TestContext2Input_submoduleTriggersInvalidCount(t *testing.T) { t.Fatalf("err: %s", err) } } + +// In this case, a module variable can't be resolved from a data source until +// it's refreshed, but it can't be refreshed during Input. +func TestContext2Input_dataSourceRequiresRefresh(t *testing.T) { + input := new(MockUIInput) + p := testProvider("null") + m := testModule(t, "input-module-data-vars") + + p.ReadDataDiffFn = testDataDiffFn + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "data.null_data_source.bar": &ResourceState{ + Type: "null_data_source", + Primary: &InstanceState{ + ID: "-", + Attributes: map[string]string{ + "foo.#": "1", + "foo.0": "a", + // foo.1 exists in the data source, but needs to be refreshed. + }, + }, + }, + }, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + ProviderResolver: ResourceProviderResolverFixed( + map[string]ResourceProviderFactory{ + "null": testProviderFuncFixed(p), + }, + ), + State: state, + UIInput: input, + }) + + if err := ctx.Input(InputModeStd); err != nil { + t.Fatalf("err: %s", err) + } + + // ensure that plan works after Refresh + if _, err := ctx.Refresh(); err != nil { + t.Fatalf("err: %s", err) + } + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } +} diff --git a/terraform/test-fixtures/input-module-data-vars/child/main.tf b/terraform/test-fixtures/input-module-data-vars/child/main.tf new file mode 100644 index 000000000..aa5d69bd5 --- /dev/null +++ b/terraform/test-fixtures/input-module-data-vars/child/main.tf @@ -0,0 +1,5 @@ +variable "in" {} + +output "out" { + value = "${var.in}" +} diff --git a/terraform/test-fixtures/input-module-data-vars/main.tf b/terraform/test-fixtures/input-module-data-vars/main.tf new file mode 100644 index 000000000..0a327b102 --- /dev/null +++ b/terraform/test-fixtures/input-module-data-vars/main.tf @@ -0,0 +1,8 @@ +data "null_data_source" "bar" { + foo = ["a", "b"] +} + +module "child" { + source = "./child" + in = "${data.null_data_source.bar.foo[1]}" +}