From 5f1e6ad020dec9ddaf1789d3adea03ec3a5bb331 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 12 Dec 2016 20:54:46 -0800 Subject: [PATCH] terraform: TargetsTransformer should preserve module variables Fixes #10680 This moves TargetsTransformer to run after the transforms that add module variables is run. This makes targeting work across modules (test added). This is a bug that only exists in the new graph, but was caught by a shadow error in #10680. Tests were added to protect against regressions. --- terraform/context_apply_test.go | 43 +++++++++++++++++++ terraform/context_plan_test.go | 41 ++++++++++++++++++ terraform/graph_builder_plan.go | 12 +++--- terraform/node_module_variable.go | 7 +++ .../plan-targeted-cross-module/A/main.tf | 5 +++ .../plan-targeted-cross-module/B/main.tf | 5 +++ .../plan-targeted-cross-module/main.tf | 8 ++++ 7 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 terraform/test-fixtures/plan-targeted-cross-module/A/main.tf create mode 100644 terraform/test-fixtures/plan-targeted-cross-module/B/main.tf create mode 100644 terraform/test-fixtures/plan-targeted-cross-module/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index fee347b63..77b0579fb 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2708,6 +2708,49 @@ func TestContext2Apply_moduleBool(t *testing.T) { } } +// Tests that a module can be targeted and everything is properly created. +// This adds to the plan test to also just verify that apply works. +func TestContext2Apply_moduleTarget(t *testing.T) { + m := testModule(t, "plan-targeted-cross-module") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Targets: []string{"module.B"}, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + checkStateString(t, state, ` + +module.A: + aws_instance.foo: + ID = foo + foo = bar + type = aws_instance + + Outputs: + + value = foo +module.B: + aws_instance.bar: + ID = foo + foo = foo + type = aws_instance + `) +} + func TestContext2Apply_multiProvider(t *testing.T) { m := testModule(t, "apply-multi-provider") p := testProvider("aws") diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 9dd6e14f4..4902def11 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2341,6 +2341,47 @@ STATE: } } +// Test that targeting a module properly plans any inputs that depend +// on another module. +func TestContext2Plan_targetedCrossModule(t *testing.T) { + m := testModule(t, "plan-targeted-cross-module") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Targets: []string{"module.B"}, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(` +DIFF: + +module.A: + CREATE: aws_instance.foo + foo: "" => "bar" + type: "" => "aws_instance" +module.B: + CREATE: aws_instance.bar + foo: "" => "" + type: "" => "aws_instance" + +STATE: + + + `) + if actual != expected { + t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) + } +} + func TestContext2Plan_targetedOrphan(t *testing.T) { m := testModule(t, "plan-targeted-orphan") p := testProvider("aws") diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 7dd5708d3..640245fc6 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -89,6 +89,12 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // Attach the state &AttachStateTransformer{State: b.State}, + // Add root variables + &RootVariableTransformer{Module: b.Module}, + + // Add module variables + &ModuleVariableTransformer{Module: b.Module}, + // Connect so that the references are ready for targeting. We'll // have to connect again later for providers and so on. &ReferenceTransformer{}, @@ -103,12 +109,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { &ParentProviderTransformer{}, &AttachProviderConfigTransformer{Module: b.Module}, - // Add root variables - &RootVariableTransformer{Module: b.Module}, - - // Add module variables - &ModuleVariableTransformer{Module: b.Module}, - // Connect references again to connect the providers, module variables, // etc. This is idempotent. &ReferenceTransformer{}, diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index 9b7ddbf2f..13fe8fc3a 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -37,6 +37,13 @@ func (n *NodeApplyableModuleVariable) Path() []string { return rootModulePath } +// RemovableIfNotTargeted +func (n *NodeApplyableModuleVariable) RemoveIfNotTargeted() bool { + // We need to add this so that this node will be removed if + // it isn't targeted or a dependency of a target. + return true +} + // GraphNodeReferenceGlobal func (n *NodeApplyableModuleVariable) ReferenceGlobal() bool { // We have to create fully qualified references because we cross diff --git a/terraform/test-fixtures/plan-targeted-cross-module/A/main.tf b/terraform/test-fixtures/plan-targeted-cross-module/A/main.tf new file mode 100644 index 000000000..f825ecf8a --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-cross-module/A/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "foo" { + foo = "bar" +} + +output "value" { value = "${aws_instance.foo.id}" } diff --git a/terraform/test-fixtures/plan-targeted-cross-module/B/main.tf b/terraform/test-fixtures/plan-targeted-cross-module/B/main.tf new file mode 100644 index 000000000..c3aeb7b76 --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-cross-module/B/main.tf @@ -0,0 +1,5 @@ +variable "input" {} + +resource "aws_instance" "bar" { + foo = "${var.input}" +} diff --git a/terraform/test-fixtures/plan-targeted-cross-module/main.tf b/terraform/test-fixtures/plan-targeted-cross-module/main.tf new file mode 100644 index 000000000..e6a83b2a0 --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-cross-module/main.tf @@ -0,0 +1,8 @@ +module "A" { + source = "./A" +} + +module "B" { + source = "./B" + input = "${module.A.value}" +}