From 24c45fcd5dfe196f3e268d884bc84ad61b1c02ed Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Fri, 29 Jul 2016 10:53:13 -0500 Subject: [PATCH] terraform: Filter untargeted variable nodes When targeting, only Addressable untargeted nodes were being removed from the graph. Variable nodes are not directly Addressable, so they were hanging around. This caused problems with module variables that referred to Resource nodes. The Resource node would be filtered out of the graph, but the module Variable node would not, so it would try to interpolate during the graph walk and be unable to find it's referent. This would present itself as strange "cannot find variable" errors for variables that were uninvolved with the currently targeted set of resources. Here, we introduce a new interface that can be implemented by graph nodes to indicate they should be filtered out from targeting even though they are not directly addressable themselves. --- terraform/context_plan_test.go | 37 +++++++++++++++++++ terraform/graph_config_node_variable.go | 8 ++++ .../child/main.tf | 5 +++ .../main.tf | 12 ++++++ terraform/transform_targets.go | 24 ++++++++++-- 5 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 terraform/test-fixtures/plan-targeted-module-untargeted-variable/child/main.tf create mode 100644 terraform/test-fixtures/plan-targeted-module-untargeted-variable/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index a3a38cc7f5..65ba03566a 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2114,6 +2114,43 @@ module.child: } } +func TestContext2Plan_targetedModuleUntargetedVariable(t *testing.T) { + m := testModule(t, "plan-targeted-module-untargeted-variable") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Targets: []string{"aws_instance.blue", "module.blue_mod"}, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(` +DIFF: + +CREATE: aws_instance.blue + +module.blue_mod: + CREATE: aws_instance.mod + type: "" => "aws_instance" + value: "" => "" + +STATE: + + +`) + if actual != expected { + t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) + } +} + // https://github.com/hashicorp/terraform/issues/4515 func TestContext2Plan_targetedOverTen(t *testing.T) { m := testModule(t, "plan-targeted-over-ten") diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index 6098042efd..ba62eb0567 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -36,6 +36,14 @@ func (n *GraphNodeConfigVariable) DependableName() []string { return []string{n.Name()} } +// RemoveIfNotTargeted implements RemovableIfNotTargeted. +// When targeting is active, variables that are not targeted should be removed +// from the graph, because otherwise module variables trying to interpolate +// their references can fail when they're missing the referent resource node. +func (n *GraphNodeConfigVariable) RemoveIfNotTargeted() bool { + return true +} + func (n *GraphNodeConfigVariable) DependentOn() []string { // If we don't have any value set, we don't depend on anything if n.Value == nil { diff --git a/terraform/test-fixtures/plan-targeted-module-untargeted-variable/child/main.tf b/terraform/test-fixtures/plan-targeted-module-untargeted-variable/child/main.tf new file mode 100644 index 0000000000..f7b424b841 --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-module-untargeted-variable/child/main.tf @@ -0,0 +1,5 @@ +variable "id" {} + +resource "aws_instance" "mod" { + value = "${var.id}" +} diff --git a/terraform/test-fixtures/plan-targeted-module-untargeted-variable/main.tf b/terraform/test-fixtures/plan-targeted-module-untargeted-variable/main.tf new file mode 100644 index 0000000000..90e44dceba --- /dev/null +++ b/terraform/test-fixtures/plan-targeted-module-untargeted-variable/main.tf @@ -0,0 +1,12 @@ +resource "aws_instance" "blue" { } +resource "aws_instance" "green" { } + +module "blue_mod" { + source = "./child" + id = "${aws_instance.blue.id}" +} + +module "green_mod" { + source = "./child" + id = "${aws_instance.green.id}" +} diff --git a/terraform/transform_targets.go b/terraform/transform_targets.go index db577b361f..4e99baddad 100644 --- a/terraform/transform_targets.go +++ b/terraform/transform_targets.go @@ -37,11 +37,16 @@ func (t *TargetsTransformer) Transform(g *Graph) error { } for _, v := range g.Vertices() { + removable := false if _, ok := v.(GraphNodeAddressable); ok { - if !targetedNodes.Include(v) { - log.Printf("[DEBUG] Removing %q, filtered by targeting.", dag.VertexName(v)) - g.Remove(v) - } + removable = true + } + if vr, ok := v.(RemovableIfNotTargeted); ok { + removable = vr.RemoveIfNotTargeted() + } + if removable && !targetedNodes.Include(v) { + log.Printf("[DEBUG] Removing %q, filtered by targeting.", dag.VertexName(v)) + g.Remove(v) } } } @@ -110,3 +115,14 @@ func (t *TargetsTransformer) nodeIsTarget( } return false } + +// RemovableIfNotTargeted is a special interface for graph nodes that +// aren't directly addressable, but need to be removed from the graph when they +// are not targeted. (Nodes that are not directly targeted end up in the set of +// targeted nodes because something that _is_ targeted depends on them.) The +// initial use case for this interface is GraphNodeConfigVariable, which was +// having trouble interpolating for module variables in targeted scenarios that +// filtered out the resource node being referenced. +type RemovableIfNotTargeted interface { + RemoveIfNotTargeted() bool +}