From 4d6085b46ac5e59c383303c9eb4d3d25ded2967e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Feb 2017 12:52:45 -0800 Subject: [PATCH] terraform: outputs should not be included if not targeted Fixes #10911 Outputs that aren't targeted shouldn't be included in the graph. This requires passing targets to the apply graph. This is unfortunate but long term should be removable since I'd like to move output changes to the diff as well. --- terraform/context.go | 1 + terraform/context_apply_test.go | 32 ++++++++++ terraform/graph_builder_apply.go | 9 +++ terraform/graph_builder_apply_test.go | 64 ++++++++++++++++--- terraform/node_output.go | 7 ++ .../child1/main.tf | 10 +++ .../child2/main.tf | 2 + .../main.tf | 10 +++ .../child1/main.tf | 12 ++++ .../child2/main.tf | 2 + .../graph-builder-apply-target-module/main.tf | 10 +++ 11 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf create mode 100644 terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf create mode 100644 terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf create mode 100644 terraform/test-fixtures/graph-builder-apply-target-module/child1/main.tf create mode 100644 terraform/test-fixtures/graph-builder-apply-target-module/child2/main.tf create mode 100644 terraform/test-fixtures/graph-builder-apply-target-module/main.tf diff --git a/terraform/context.go b/terraform/context.go index 4346282df..b9214f84c 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -215,6 +215,7 @@ func (c *Context) Graph(typ GraphType, opts *ContextGraphOpts) (*Graph, error) { State: c.state, Providers: c.components.ResourceProviders(), Provisioners: c.components.ResourceProvisioners(), + Targets: c.targets, Destroy: c.destroy, Validate: opts.Validate, }).Build(RootModulePath) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index f6347f263..cab3e986f 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -6977,6 +6977,38 @@ module.child: `) } +// GH-10911 untargeted outputs should not be in the graph, and therefore +// not execute. +func TestContext2Apply_targetedModuleUnrelatedOutputs(t *testing.T) { + m := testModule(t, "apply-targeted-module-unrelated-outputs") + 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.child2"}, + }) + + 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.child2: + aws_instance.foo: + ID = foo + `) +} + func TestContext2Apply_targetedModuleResource(t *testing.T) { m := testModule(t, "apply-targeted-module-resource") p := testProvider("aws") diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 4eeaab3e2..61242586a 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -28,6 +28,12 @@ type ApplyGraphBuilder struct { // Provisioners is the list of provisioners supported. Provisioners []string + // Targets are resources to target. This is only required to make sure + // unnecessary outputs aren't included in the apply graph. The plan + // builder successfully handles targeting resources. In the future, + // outputs should go into the diff so that this is unnecessary. + Targets []string + // DisableReduce, if true, will not reduce the graph. Great for testing. DisableReduce bool @@ -114,6 +120,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Add the node to fix the state count boundaries &CountBoundaryTransformer{}, + // Target + &TargetsTransformer{Targets: b.Targets}, + // Single root &RootTransformer{}, } diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 3e5134d84..b62833103 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -95,17 +95,15 @@ func TestApplyGraphBuilder_depCbd(t *testing.T) { Modules: []*ModuleDiff{ &ModuleDiff{ Path: []string{"root"}, - Resources: map[string]*InstanceDiff{ - "aws_instance.A": &InstanceDiff{ - Destroy: true, - Attributes: map[string]*ResourceAttrDiff{ - "name": &ResourceAttrDiff{ - Old: "", - New: "foo", - RequiresNew: true, - }, + Resources: map[string]*InstanceDiff{"aws_instance.A": &InstanceDiff{Destroy: true, + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + RequiresNew: true, }, }, + }, "aws_instance.B": &InstanceDiff{ Attributes: map[string]*ResourceAttrDiff{ @@ -440,6 +438,54 @@ func TestApplyGraphBuilder_provisionerDestroy(t *testing.T) { "null_resource.foo (destroy)") } +func TestApplyGraphBuilder_targetModule(t *testing.T) { + diff := &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + "null_resource.foo": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, + + &ModuleDiff{ + Path: []string{"root", "child2"}, + Resources: map[string]*InstanceDiff{ + "null_resource.foo": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, + }, + } + + b := &ApplyGraphBuilder{ + Module: testModule(t, "graph-builder-apply-target-module"), + Diff: diff, + Providers: []string{"null"}, + Targets: []string{"module.child2"}, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + testGraphNotContains(t, g, "module.child1.output.instance_id") +} + const testApplyGraphBuilderStr = ` aws_instance.create provider.aws diff --git a/terraform/node_output.go b/terraform/node_output.go index 41962c2cb..e28e6f02f 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -28,6 +28,13 @@ func (n *NodeApplyableOutput) Path() []string { return n.PathValue } +// RemovableIfNotTargeted +func (n *NodeApplyableOutput) 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 +} + // GraphNodeReferenceable func (n *NodeApplyableOutput) ReferenceableName() []string { name := fmt.Sprintf("output.%s", n.Config.Name) diff --git a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf new file mode 100644 index 000000000..b6de394c9 --- /dev/null +++ b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child1/main.tf @@ -0,0 +1,10 @@ +variable "instance_id" { +} + +output "instance_id" { + value = "${var.instance_id}" +} + +resource "aws_instance" "foo" { + foo = "${var.instance_id}" +} diff --git a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf new file mode 100644 index 000000000..b626e60c8 --- /dev/null +++ b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/child2/main.tf @@ -0,0 +1,2 @@ +resource "aws_instance" "foo" { +} diff --git a/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf new file mode 100644 index 000000000..236f0c495 --- /dev/null +++ b/terraform/test-fixtures/apply-targeted-module-unrelated-outputs/main.tf @@ -0,0 +1,10 @@ +resource "aws_instance" "foo" {} + +module "child1" { + source = "./child1" + instance_id = "${aws_instance.foo.id}" +} + +module "child2" { + source = "./child2" +} diff --git a/terraform/test-fixtures/graph-builder-apply-target-module/child1/main.tf b/terraform/test-fixtures/graph-builder-apply-target-module/child1/main.tf new file mode 100644 index 000000000..d703576a4 --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-target-module/child1/main.tf @@ -0,0 +1,12 @@ +variable "instance_id" { +} + +output "instance_id" { + value = "${var.instance_id}" +} + +resource "null_resource" "foo" { + triggers = { + instance_id = "${var.instance_id}" + } +} diff --git a/terraform/test-fixtures/graph-builder-apply-target-module/child2/main.tf b/terraform/test-fixtures/graph-builder-apply-target-module/child2/main.tf new file mode 100644 index 000000000..76a9c73c8 --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-target-module/child2/main.tf @@ -0,0 +1,2 @@ +resource "null_resource" "foo" { +} diff --git a/terraform/test-fixtures/graph-builder-apply-target-module/main.tf b/terraform/test-fixtures/graph-builder-apply-target-module/main.tf new file mode 100644 index 000000000..50c252ced --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-target-module/main.tf @@ -0,0 +1,10 @@ +resource "null_resource" "foo" {} + +module "child1" { + source = "./child1" + instance_id = "${null_resource.foo.id}" +} + +module "child2" { + source = "./child2" +}