Merge pull request #25018 from hashicorp/jbardin/destroy-modules

Destroy nested, expanding modules

terraform/context_apply_test.go

@@ -8980,12 +8980,6 @@ Outputs:
 
 result_1 = hello
 result_3 = hello world
-
-module.child:
-  <no state>
-  Outputs:
-
-  result = hello
 `)
 	if got != want {
 		t.Fatalf("wrong final state\ngot:\n%s\nwant:\n%s", got, want)
@@ -11252,3 +11246,73 @@ output "c" {
 		t.Fatal("expected empty state, got:", state)
 	}
 }
+
+func TestContext2Apply_moduleExpandDependsOn(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "child" {
+  count = 1
+  source = "./child"
+
+  depends_on = [test_instance.a, test_instance.b]
+}
+
+resource "test_instance" "a" {
+}
+
+
+resource "test_instance" "b" {
+}
+`,
+		"child/main.tf": `
+resource "test_instance" "foo" {
+}
+
+output "myoutput" {
+  value = "literal string"
+}
+`})
+
+	p := testProvider("test")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	_, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+
+	state, diags := ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+
+	ctx = testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		Destroy: true,
+		State:   state,
+	})
+
+	_, diags = ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+
+	state, diags = ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+
+	if !state.Empty() {
+		t.Fatal("expected empty state, got:", state)
+	}
+}

terraform/graph_builder_apply.go

@@ -160,14 +160,21 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 			Schemas: b.Schemas,
 		},
 
-		// Create a destroy node for outputs to remove them from the state.
+		// Create a destroy node for root outputs to remove them from the
-		&DestroyOutputTransformer{Destroy: b.Destroy},
+		// state.  This does nothing unless invoked via the destroy command
+		// directly.  A destroy is identical to a normal apply, except for the
+		// fact that we also have configuration to evaluate. While the rest of
+		// the unused nodes can be programmatically pruned (via
+		// pruneUnusedNodesTransformer), root module outputs only have an
+		// implied dependency on remote state. This means that if they exist in
+		// the configuration, the only signal to remove them is via the destroy
+		// command itself.
+		&destroyRootOutputTransformer{Destroy: b.Destroy},
 
-		// Prune unreferenced values, which may have interpolations that can't
+		// We need to remove configuration nodes that are not used at all, as
-		// be resolved.
+		// they may not be able to evaluate, especially during destroy.
-		&PruneUnusedValuesTransformer{
+		// These include variables, locals, and instance expanders.
-			Destroy: b.Destroy,
+		&pruneUnusedNodesTransformer{},
-		},
 
 		// Add the node to fix the state count boundaries
 		&CountBoundaryTransformer{

terraform/instance_expanders.go

@@ -0,0 +1,7 @@
+package terraform
+
+// graphNodeExpandsInstances is implemented by nodes that causes instances to
+// be registered in the instances.Expander.
+type graphNodeExpandsInstances interface {
+	expandsInstances()
+}

terraform/node_local.go

@@ -23,8 +23,11 @@ var (
 	_ GraphNodeReferencer        = (*nodeExpandLocal)(nil)
 	_ GraphNodeDynamicExpandable = (*nodeExpandLocal)(nil)
 	_ graphNodeTemporaryValue    = (*nodeExpandLocal)(nil)
+	_ graphNodeExpandsInstances  = (*nodeExpandLocal)(nil)
 )
 
+func (n *nodeExpandLocal) expandsInstances() {}
+
 // graphNodeTemporaryValue
 func (n *nodeExpandLocal) temporaryValue() bool {
 	return true

terraform/node_module_expand.go

@@ -1,7 +1,6 @@
 package terraform
 
 import (
-	"fmt"
 	"log"
 
 	"github.com/hashicorp/terraform/addrs"
@@ -10,12 +9,6 @@ import (
 	"github.com/hashicorp/terraform/lang"
 )
 
-// graphNodeModuleCloser is an interface implemented by nodes that finalize the
-// evaluation of modules.
-type graphNodeModuleCloser interface {
-	CloseModule() addrs.Module
-}
-
 type ConcreteModuleNodeFunc func(n *nodeExpandModule) dag.Vertex
 
 // nodeExpandModule represents a module call in the configuration that
@@ -28,21 +21,22 @@ type nodeExpandModule struct {
 }
 
 var (
-	_ RemovableIfNotTargeted = (*nodeExpandModule)(nil)
+	_ RemovableIfNotTargeted    = (*nodeExpandModule)(nil)
-	_ GraphNodeEvalable      = (*nodeExpandModule)(nil)
+	_ GraphNodeEvalable         = (*nodeExpandModule)(nil)
-	_ GraphNodeReferencer    = (*nodeExpandModule)(nil)
+	_ GraphNodeReferencer       = (*nodeExpandModule)(nil)
+	_ GraphNodeReferenceOutside = (*nodeExpandModule)(nil)
+	_ graphNodeExpandsInstances = (*nodeExpandModule)(nil)
 )
 
+func (n *nodeExpandModule) expandsInstances() {}
+
 func (n *nodeExpandModule) Name() string {
 	return n.Addr.String() + " (expand)"
 }
 
 // GraphNodeModulePath implementation
 func (n *nodeExpandModule) ModulePath() addrs.Module {
-	// This node represents the module call within a module,
+	return n.Addr
-	// so return the CallerAddr as the path as the module
-	// call may expand into multiple child instances
-	return n.Addr.Parent()
 }
 
 // GraphNodeReferencer implementation
@@ -88,6 +82,11 @@ func (n *nodeExpandModule) References() []*addrs.Reference {
 	return appendResourceDestroyReferences(refs)
 }
 
+// GraphNodeReferenceOutside
+func (n *nodeExpandModule) ReferenceOutside() (selfPath, referencePath addrs.Module) {
+	return n.Addr, n.Addr.Parent()
+}
+
 // RemovableIfNotTargeted implementation
 func (n *nodeExpandModule) RemoveIfNotTargeted() bool {
 	// We need to add this so that this node will be removed if
@@ -113,20 +112,19 @@ func (n *nodeExpandModule) EvalTree() EvalNode {
 // empty resources and modules from the state.
 type nodeCloseModule struct {
 	Addr addrs.Module
-
-	// orphaned indicates that this module has no expansion, because it no
-	// longer exists in the configuration
-	orphaned bool
 }
 
 var (
-	_ graphNodeModuleCloser  = (*nodeCloseModule)(nil)
+	_ GraphNodeReferenceable    = (*nodeCloseModule)(nil)
-	_ GraphNodeReferenceable = (*nodeCloseModule)(nil)
+	_ GraphNodeReferenceOutside = (*nodeCloseModule)(nil)
 )
 
 func (n *nodeCloseModule) ModulePath() addrs.Module {
-	mod, _ := n.Addr.Call()
+	return n.Addr
-	return mod
+}
+
+func (n *nodeCloseModule) ReferenceOutside() (selfPath, referencePath addrs.Module) {
+	return n.Addr.Parent(), n.Addr
 }
 
 func (n *nodeCloseModule) ReferenceableAddrs() []addrs.Referenceable {
@@ -143,10 +141,6 @@ func (n *nodeCloseModule) Name() string {
 	return n.Addr.String() + " (close)"
 }
 
-func (n *nodeCloseModule) CloseModule() addrs.Module {
-	return n.Addr
-}
-
 // RemovableIfNotTargeted implementation
 func (n *nodeCloseModule) RemoveIfNotTargeted() bool {
 	// We need to add this so that this node will be removed if
@@ -160,8 +154,7 @@ func (n *nodeCloseModule) EvalTree() EvalNode {
 			&EvalOpFilter{
 				Ops: []walkOperation{walkApply, walkDestroy},
 				Node: &evalCloseModule{
-					Addr:     n.Addr,
+					Addr: n.Addr,
-					orphaned: n.orphaned,
 				},
 			},
 		},
@@ -169,8 +162,7 @@ func (n *nodeCloseModule) EvalTree() EvalNode {
 }
 
 type evalCloseModule struct {
-	Addr     addrs.Module
+	Addr addrs.Module
-	orphaned bool
 }
 
 func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) {
@@ -179,13 +171,6 @@ func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) {
 	state := ctx.State().Lock()
 	defer ctx.State().Unlock()
 
-	expander := ctx.InstanceExpander()
-	var currentModuleInstances []addrs.ModuleInstance
-	// we can't expand if we're just removing
-	if !n.orphaned {
-		currentModuleInstances = expander.ExpandModule(n.Addr)
-	}
-
 	for modKey, mod := range state.Modules {
 		if !n.Addr.Equal(mod.Addr.Module()) {
 			continue
@@ -198,27 +183,8 @@ func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) {
 			}
 		}
 
-		found := false
+		// empty child modules are always removed
-		if n.orphaned {
+		if len(mod.Resources) == 0 && !mod.Addr.IsRoot() {
-			// we're removing the entire module, so all instances must go
-			found = true
-		} else {
-			// if this instance is not in the current expansion, remove it from
-			// the state
-			for _, current := range currentModuleInstances {
-				if current.Equal(mod.Addr) {
-					found = true
-					break
-				}
-			}
-		}
-
-		if !found {
-			if len(mod.Resources) > 0 {
-				// FIXME: add more info to this error
-				return nil, fmt.Errorf("module %q still contains resources in state", mod.Addr)
-			}
-
 			delete(state.Modules, modKey)
 		}
 	}

terraform/node_module_variable.go

@@ -27,8 +27,11 @@ var (
 	_ GraphNodeReferencer        = (*nodeExpandModuleVariable)(nil)
 	_ graphNodeTemporaryValue    = (*nodeExpandModuleVariable)(nil)
 	_ RemovableIfNotTargeted     = (*nodeExpandModuleVariable)(nil)
+	_ graphNodeExpandsInstances  = (*nodeExpandModuleVariable)(nil)
 )
 
+func (n *nodeExpandModuleVariable) expandsInstances() {}
+
 func (n *nodeExpandModuleVariable) temporaryValue() bool {
 	return true
 }

terraform/node_output.go

@@ -23,9 +23,12 @@ var (
 	_ GraphNodeReferenceable     = (*nodeExpandOutput)(nil)
 	_ GraphNodeReferencer        = (*nodeExpandOutput)(nil)
 	_ GraphNodeDynamicExpandable = (*nodeExpandOutput)(nil)
-	_ graphNodeTemporaryValue    = (*NodeApplyableOutput)(nil)
+	_ graphNodeTemporaryValue    = (*nodeExpandOutput)(nil)
+	_ graphNodeExpandsInstances  = (*nodeExpandOutput)(nil)
 )
 
+func (n *nodeExpandOutput) expandsInstances() {}
+
 func (n *nodeExpandOutput) temporaryValue() bool {
 	// this must always be evaluated if it is a root module output
 	return !n.Module.IsRoot()
@@ -254,7 +257,6 @@ type NodeDestroyableOutput struct {
 var (
 	_ RemovableIfNotTargeted    = (*NodeDestroyableOutput)(nil)
 	_ GraphNodeTargetDownstream = (*NodeDestroyableOutput)(nil)
-	_ GraphNodeReferencer       = (*NodeDestroyableOutput)(nil)
 	_ GraphNodeEvalable         = (*NodeDestroyableOutput)(nil)
 	_ dag.GraphNodeDotter       = (*NodeDestroyableOutput)(nil)
 )
@@ -281,11 +283,6 @@ func (n *NodeDestroyableOutput) TargetDownstream(targetedDeps, untargetedDeps da
 	return true
 }
 
-// GraphNodeReferencer
-func (n *NodeDestroyableOutput) References() []*addrs.Reference {
-	return referencesForOutput(n.Config)
-}
-
 // GraphNodeEvalable
 func (n *NodeDestroyableOutput) EvalTree() EvalNode {
 	return &EvalDeleteOutput{

terraform/node_output_orphan.go

@@ -1,53 +0,0 @@
-package terraform
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/terraform/addrs"
-)
-
-// NodeOutputOrphan represents an output that is an orphan.
-type NodeOutputOrphan struct {
-	Addr addrs.AbsOutputValue
-}
-
-var (
-	_ GraphNodeModuleInstance   = (*NodeOutputOrphan)(nil)
-	_ GraphNodeReferenceable    = (*NodeOutputOrphan)(nil)
-	_ GraphNodeReferenceOutside = (*NodeOutputOrphan)(nil)
-	_ GraphNodeEvalable         = (*NodeOutputOrphan)(nil)
-)
-
-func (n *NodeOutputOrphan) Name() string {
-	return fmt.Sprintf("%s (orphan)", n.Addr.String())
-}
-
-// GraphNodeReferenceOutside implementation
-func (n *NodeOutputOrphan) ReferenceOutside() (selfPath, referencePath addrs.Module) {
-	return referenceOutsideForOutput(n.Addr)
-}
-
-// GraphNodeReferenceable
-func (n *NodeOutputOrphan) ReferenceableAddrs() []addrs.Referenceable {
-	return referenceableAddrsForOutput(n.Addr)
-}
-
-// GraphNodeModuleInstance
-func (n *NodeOutputOrphan) Path() addrs.ModuleInstance {
-	return n.Addr.Module
-}
-
-// GraphNodeModulePath
-func (n *NodeOutputOrphan) ModulePath() addrs.Module {
-	return n.Addr.Module.Module()
-}
-
-// GraphNodeEvalable
-func (n *NodeOutputOrphan) EvalTree() EvalNode {
-	return &EvalOpFilter{
-		Ops: []walkOperation{walkRefresh, walkApply, walkDestroy},
-		Node: &EvalDeleteOutput{
-			Addr: n.Addr,
-		},
-	}
-}

terraform/node_resource_apply.go

@@ -6,7 +6,6 @@ import (
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/dag"
 	"github.com/hashicorp/terraform/lang"
-	"github.com/hashicorp/terraform/states"
 )
 
 // nodeExpandApplyableResource handles the first layer of resource
@@ -23,8 +22,11 @@ var (
 	_ GraphNodeReferencer           = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeConfigResource       = (*nodeExpandApplyableResource)(nil)
 	_ GraphNodeAttachResourceConfig = (*nodeExpandApplyableResource)(nil)
+	_ graphNodeExpandsInstances     = (*nodeExpandApplyableResource)(nil)
 )
 
+func (n *nodeExpandApplyableResource) expandsInstances() {}
+
 func (n *nodeExpandApplyableResource) References() []*addrs.Reference {
 	return (&NodeApplyableResource{NodeAbstractResource: n.NodeAbstractResource}).References()
 }
@@ -48,26 +50,6 @@ func (n *nodeExpandApplyableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 		})
 	}
 
-	// Lock the state while we inspect it to find any resources orphaned by
-	// changes in module expansion.
-	state := ctx.State().Lock()
-	defer ctx.State().Unlock()
-
-	var orphans []*states.Resource
-	for _, res := range state.Resources(n.Addr) {
-		found := false
-		for _, m := range moduleInstances {
-			if m.Equal(res.Addr.Module) {
-				found = true
-				break
-			}
-		}
-		// Address form state was not found in the current config
-		if !found {
-			orphans = append(orphans, res)
-		}
-	}
-
 	return &g, nil
 }
 

terraform/terraform_test.go

@@ -441,12 +441,6 @@ const testTerraformApplyEmptyModuleStr = `
 Outputs:
 
 end = XXXX
-
-module.child:
-<no state>
-Outputs:
-
-aws_route53_zone_id = XXXX
 `
 
 const testTerraformApplyDependsCreateBeforeStr = `
@@ -661,12 +655,6 @@ aws_instance.bar:
   provider = provider["registry.terraform.io/hashicorp/aws"]
   foo = true
   type = aws_instance
-
-module.child:
-  <no state>
-  Outputs:
-
-  leader = true
 `
 
 const testTerraformApplyModuleDestroyOrderStr = `

terraform/transform_destroy_edge.go

@@ -2,6 +2,7 @@ package terraform
 
 import (
 	"log"
+	"sort"
 
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/states"
@@ -163,46 +164,141 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error {
 		}
 	}
 
-	return t.pruneResources(g)
-}
-
-// If there are only destroy instances for a particular resource, there's no
-// reason for the resource node to prepare the state. Remove Resource nodes so
-// that they don't fail by trying to evaluate a resource that is only being
-// destroyed along with its dependencies.
-func (t *DestroyEdgeTransformer) pruneResources(g *Graph) error {
-	for _, v := range g.Vertices() {
-		n, ok := v.(*nodeExpandApplyableResource)
-		if !ok {
-			continue
-		}
-
-		// if there are only destroy dependencies, we don't need this node
-		descendents, err := g.Descendents(n)
-		if err != nil {
-			return err
-		}
-
-		nonDestroyInstanceFound := false
-		for _, v := range descendents {
-			if _, ok := v.(*NodeApplyableResourceInstance); ok {
-				nonDestroyInstanceFound = true
-				break
-			}
-		}
-
-		if nonDestroyInstanceFound {
-			continue
-		}
-
-		// connect all the through-edges, then delete the node
-		for _, d := range g.DownEdges(n) {
-			for _, u := range g.UpEdges(n) {
-				g.Connect(dag.BasicEdge(u, d))
-			}
-		}
-		log.Printf("DestroyEdgeTransformer: pruning unused resource node %s", dag.VertexName(n))
-		g.Remove(n)
-	}
 	return nil
 }
+
+// Remove any nodes that aren't needed when destroying modules.
+// Variables, outputs, locals, and expanders may not be able to evaluate
+// correctly, so we can remove these if nothing depends on them. The module
+// closers also need to disable their use of expansion if the module itself is
+// no longer present.
+type pruneUnusedNodesTransformer struct {
+}
+
+func (t *pruneUnusedNodesTransformer) Transform(g *Graph) error {
+	// We need a reverse depth first walk of modules, processing them in order
+	// from the leaf modules to the root. This allows us to remove unneeded
+	// dependencies from child modules, freeing up nodes in the parent module
+	// to also be removed.
+
+	// First collect the nodes into their respective modules based on
+	// configuration path.
+	moduleMap := make(map[string]pruneUnusedNodesMod)
+	for _, v := range g.Vertices() {
+		var path addrs.Module
+		switch v := v.(type) {
+		case GraphNodeModulePath:
+			path = v.ModulePath()
+		default:
+			continue
+		}
+		m := moduleMap[path.String()]
+		m.addr = path
+		m.nodes = append(m.nodes, v)
+
+		moduleMap[path.String()] = m
+	}
+
+	// now we need to restructure the modules so we can sort them
+	var modules []pruneUnusedNodesMod
+
+	for _, mod := range moduleMap {
+		modules = append(modules, mod)
+	}
+
+	// Sort them by path length, longest first, so that start with the deepest
+	// modules.  The order of modules at the same tree level doesn't matter, we
+	// just need to ensure that child modules are processed before parent
+	// modules.
+	sort.Slice(modules, func(i, j int) bool {
+		return len(modules[i].addr) > len(modules[j].addr)
+	})
+
+	for _, mod := range modules {
+		mod.removeUnused(g)
+	}
+
+	return nil
+}
+
+// pruneUnusedNodesMod is a container to hold the nodes that belong to a
+// particular configuration module for the pruneUnusedNodesTransformer
+type pruneUnusedNodesMod struct {
+	addr  addrs.Module
+	nodes []dag.Vertex
+}
+
+// Remove any unused locals, variables, outputs and expanders.  Since module
+// closers can also lookup expansion info to detect orphaned instances, disable
+// them if their associated expander is removed.
+func (m *pruneUnusedNodesMod) removeUnused(g *Graph) {
+	// We modify the nodes slice during processing here.
+	// Make a copy so no one is surprised by this changing in the future.
+	nodes := make([]dag.Vertex, len(m.nodes))
+	copy(nodes, m.nodes)
+
+	// since we have no defined structure within the module, just cycle through
+	// the nodes in each module until there are no more removals
+	removed := true
+	for {
+		if !removed {
+			return
+		}
+		removed = false
+
+		for i := 0; i < len(nodes); i++ {
+			// run this in a closure, so we can return early rather than
+			// dealing with complex looping and labels
+			func() {
+				n := nodes[i]
+				switch n.(type) {
+				case graphNodeTemporaryValue:
+					// temporary value, which consist of variables, locals, and
+					// outputs, must be kept if anything refers to them.
+					if n, ok := n.(GraphNodeModulePath); ok {
+						// root outputs always have an implicit dependency on
+						// remote state.
+						if n.ModulePath().IsRoot() {
+							return
+						}
+					}
+					for _, v := range g.UpEdges(n) {
+						// keep any value which is connected through a
+						// reference
+						if _, ok := v.(GraphNodeReferencer); ok {
+							return
+						}
+					}
+
+				case graphNodeExpandsInstances:
+					// Any nodes that expand instances are kept when their
+					// instances may need to be evaluated.
+					for _, v := range g.UpEdges(n) {
+						switch v.(type) {
+						case graphNodeExpandsInstances:
+							// expanders can always depend on module expansion
+							// themselves
+							return
+						case GraphNodeResourceInstance:
+							// resource instances always depend on their
+							// resource node, which is an expander
+							return
+						}
+					}
+
+				default:
+					return
+				}
+
+				log.Printf("[DEBUG] pruneUnusedNodes: %s is no longer needed, removing", dag.VertexName(n))
+				g.Remove(n)
+				removed = true
+
+				// remove the node from our iteration as well
+				last := len(nodes) - 1
+				nodes[i], nodes[last] = nodes[last], nodes[i]
+				nodes = nodes[:last]
+			}()
+		}
+	}
+}

terraform/transform_module_expansion.go

@@ -3,6 +3,7 @@ package terraform
 import (
 	"log"
 
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/dag"
 )
@@ -42,6 +43,11 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error {
 	// handled by the RemovedModuleTransformer, and those module closers are in
 	// the graph already, and need to be connected to their parent closers.
 	for _, v := range g.Vertices() {
+		// skip closers so they don't attach to themselves
+		if _, ok := v.(*nodeCloseModule); ok {
+			continue
+		}
+
 		// any node that executes within the scope of a module should be a
 		// GraphNodeModulePath
 		pather, ok := v.(GraphNodeModulePath)
@@ -49,13 +55,23 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error {
 			continue
 		}
 		if closer, ok := t.closers[pather.ModulePath().String()]; ok {
-			// The module root depends on each child resource instance, since
+			// The module closer depends on each child resource instance, since
 			// during apply the module expansion will complete before the
 			// individual instances are applied.
 			g.Connect(dag.BasicEdge(closer, v))
 		}
 	}
 
+	// Modules implicitly depend on their child modules, so connect closers to
+	// other which contain their path.
+	for _, c := range t.closers {
+		for _, d := range t.closers {
+			if len(d.Addr) > len(c.Addr) && c.Addr.Equal(d.Addr[:len(c.Addr)]) {
+				g.Connect(dag.BasicEdge(c, d))
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -91,11 +107,26 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare
 	t.closers[c.Path.String()] = closer
 
 	for _, childV := range g.Vertices() {
-		pather, ok := childV.(GraphNodeModulePath)
+		// don't connect a node to itself
-		if !ok {
+		if childV == v {
 			continue
 		}
-		if pather.ModulePath().Equal(c.Path) {
+
+		var path addrs.Module
+		switch t := childV.(type) {
+		case GraphNodeDestroyer:
+			// skip destroyers, as they can only depend on other resources.
+			continue
+
+		case GraphNodeModulePath:
+			path = t.ModulePath()
+		case GraphNodeReferenceOutside:
+			path, _ = t.ReferenceOutside()
+		default:
+			continue
+		}
+
+		if path.Equal(c.Path) {
 			log.Printf("[TRACE] ModuleExpansionTransformer: %s must wait for expansion of %s", dag.VertexName(childV), c.Path)
 			g.Connect(dag.BasicEdge(childV, v))
 		}

terraform/transform_orphan_output.go

@@ -51,7 +51,7 @@ func (t *OrphanOutputTransformer) transform(g *Graph, ms *states.Module) error {
 			continue
 		}
 
-		g.Add(&NodeOutputOrphan{
+		g.Add(&NodeDestroyableOutput{
 			Addr: addrs.OutputValue{Name: name}.Absolute(moduleAddr),
 		})
 	}

terraform/transform_output.go

@@ -53,14 +53,14 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error {
 	return nil
 }
 
-// DestroyOutputTransformer is a GraphTransformer that adds nodes to delete
+// destroyRootOutputTransformer is a GraphTransformer that adds nodes to delete
 // outputs during destroy. We need to do this to ensure that no stale outputs
 // are ever left in the state.
-type DestroyOutputTransformer struct {
+type destroyRootOutputTransformer struct {
 	Destroy bool
 }
 
-func (t *DestroyOutputTransformer) Transform(g *Graph) error {
+func (t *destroyRootOutputTransformer) Transform(g *Graph) error {
 	// Only clean root outputs on a full destroy
 	if !t.Destroy {
 		return nil

terraform/transform_reference.go

@@ -254,56 +254,6 @@ func (t AttachDependenciesTransformer) Transform(g *Graph) error {
 	return nil
 }
 
-// PruneUnusedValuesTransformer is a GraphTransformer that removes local,
-// variable, and output values which are not referenced in the graph. If these
-// values reference a resource that is no longer in the state the interpolation
-// could fail.
-type PruneUnusedValuesTransformer struct {
-	Destroy bool
-}
-
-func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
-	// Pruning a value can effect previously checked edges, so loop until there
-	// are no more changes.
-	for removed := 0; ; removed = 0 {
-		for _, v := range g.Vertices() {
-			// we're only concerned with values that don't need to be saved in state
-			switch v := v.(type) {
-			case graphNodeTemporaryValue:
-				if !v.temporaryValue() {
-					continue
-				}
-			default:
-				continue
-			}
-
-			dependants := g.UpEdges(v)
-
-			// any referencers in the dependents means we need to keep this
-			// value for evaluation
-			removable := true
-			for _, d := range dependants.List() {
-				if _, ok := d.(GraphNodeReferencer); ok {
-					removable = false
-					break
-				}
-			}
-
-			if removable {
-				log.Printf("[TRACE] PruneUnusedValuesTransformer: removing unused value %s", dag.VertexName(v))
-				g.Remove(v)
-				removed++
-			}
-		}
-
-		if removed == 0 {
-			break
-		}
-	}
-
-	return nil
-}
-
 // ReferenceMap is a structure that can be used to efficiently check
 // for references on a graph, mapping internal reference keys (as produced by
 // the mapKey method) to one or more vertices that are identified by each key.

terraform/transform_removed_modules.go

@@ -35,8 +35,7 @@ func (t *RemovedModuleTransformer) Transform(g *Graph) error {
 	// add closers to collect any module instances we're removing
 	for _, modAddr := range removed {
 		closer := &nodeCloseModule{
-			Addr:     modAddr,
+			Addr: modAddr,
-			orphaned: true,
 		}
 		g.Add(closer)
 	}