From 21e2173e0a32078d4f3c7dbed1f2a868da5c99b2 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 6 Jul 2016 11:22:41 -0400 Subject: [PATCH] Fix nested module "unknown variable" during dest (#7496) * Fix nested module "unknown variable" during dstry During a destroy with nested modules, accessing a variable between them causes an "unknown variable accessed" during destroy. --- terraform/context_apply_test.go | 80 +++++++++++++++++++ terraform/graph_config_node_variable.go | 7 +- .../middle/bottom/bottom.tf | 1 + .../middle/middle.tf | 8 ++ .../top.tf | 4 + 5 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/bottom/bottom.tf create mode 100644 terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/middle.tf create mode 100644 terraform/test-fixtures/apply-destroy-nested-module-with-attrs/top.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index fa0817821..a5f425a40 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -4704,3 +4704,83 @@ aws_instance.foo: t.Fatalf("bad: \n%s", actual) } } + +// https://github.com/hashicorp/terraform/issues/7378 +func TestContext2Apply_destroyNestedModuleWithAttrsReferencingResource(t *testing.T) { + m := testModule(t, "apply-destroy-nested-module-with-attrs") + p := testProvider("null") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + var state *State + var err error + { + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "null": testProviderFuncFixed(p), + }, + }) + + // First plan and apply a create operation + if _, err := ctx.Plan(); err != nil { + t.Fatalf("plan err: %s", err) + } + + state, err = ctx.Apply() + if err != nil { + t.Fatalf("apply err: %s", err) + } + } + + { + ctx := testContext2(t, &ContextOpts{ + Destroy: true, + Module: m, + State: state, + Providers: map[string]ResourceProviderFactory{ + "null": testProviderFuncFixed(p), + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("destroy plan err: %s", err) + } + + var buf bytes.Buffer + if err := WritePlan(plan, &buf); err != nil { + t.Fatalf("plan write err: %s", err) + } + + planFromFile, err := ReadPlan(&buf) + if err != nil { + t.Fatalf("plan read err: %s", err) + } + + ctx, err = planFromFile.Context(&ContextOpts{ + Providers: map[string]ResourceProviderFactory{ + "null": testProviderFuncFixed(p), + }, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + state, err = ctx.Apply() + if err != nil { + t.Fatalf("destroy apply err: %s", err) + } + } + + //Test that things were destroyed + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(` + +module.middle: + + `) + if actual != expected { + t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual) + } +} diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index 0a86c4881..8565fd69a 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -130,6 +130,7 @@ func (n *GraphNodeConfigVariable) hasDestroyEdgeInPath(opts *NoopOpts, vertex da if vertex == nil { vertex = opts.Vertex } + log.Printf("[DEBUG] hasDestroyEdgeInPath: Looking for destroy edge: %s - %T", dag.VertexName(vertex), vertex) for _, v := range opts.Graph.UpEdges(vertex).List() { if len(opts.Graph.UpEdges(v).List()) > 1 { @@ -137,10 +138,12 @@ func (n *GraphNodeConfigVariable) hasDestroyEdgeInPath(opts *NoopOpts, vertex da return true } } + // Here we borrow the implementation of DestroyEdgeInclude, whose logic - // and semantics are exactly what we want here. + // and semantics are exactly what we want here. We add a check for the + // the root node, since we have to always depend on its existance. if cv, ok := vertex.(*GraphNodeConfigVariableFlat); ok { - if cv.DestroyEdgeInclude(v) { + if dag.VertexName(v) == rootNodeName || cv.DestroyEdgeInclude(v) { return true } } diff --git a/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/bottom/bottom.tf b/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/bottom/bottom.tf new file mode 100644 index 000000000..a9ce7fcc8 --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/bottom/bottom.tf @@ -0,0 +1 @@ +variable "bottom_param" {} diff --git a/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/middle.tf b/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/middle.tf new file mode 100644 index 000000000..0fde5830b --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/middle.tf @@ -0,0 +1,8 @@ +variable "param" {} + +resource "null_resource" "n" {} + +module "bottom" { + source = "./bottom" + bottom_param = "${var.param}" +} diff --git a/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/top.tf b/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/top.tf new file mode 100644 index 000000000..1b631f4d5 --- /dev/null +++ b/terraform/test-fixtures/apply-destroy-nested-module-with-attrs/top.tf @@ -0,0 +1,4 @@ +module "middle" { + source = "./middle" + param = "foo" +}