terraform: destroy transform must happen globally

terraform/context_test.go

@@ -4053,6 +4053,88 @@ func TestContext2Apply_module(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_moduleDestroyOrder(t *testing.T) {
+	m := testModule(t, "apply-module-destroy-order")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+
+	// Create a custom apply function to track the order they were destroyed
+	var order []string
+	var orderLock sync.Mutex
+	p.ApplyFn = func(
+		info *InstanceInfo,
+		is *InstanceState,
+		id *InstanceDiff) (*InstanceState, error) {
+		orderLock.Lock()
+		defer orderLock.Unlock()
+
+		order = append(order, is.ID)
+		return nil, nil
+	}
+
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.b": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "b",
+						},
+					},
+				},
+			},
+
+			&ModuleState{
+				Path: []string{"root", "child"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.a": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "a",
+						},
+					},
+				},
+				Outputs: map[string]string{
+					"a_output": "a",
+				},
+			},
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State:   state,
+		Destroy: true,
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	expected := []string{"b", "a"}
+	if !reflect.DeepEqual(order, expected) {
+		t.Fatalf("bad: %#v", order)
+	}
+
+	{
+		actual := strings.TrimSpace(state.String())
+		expected := strings.TrimSpace(testTerraformApplyModuleDestroyOrderStr)
+		if actual != expected {
+			t.Fatalf("bad: \n%s", actual)
+		}
+	}
+}
+
 func TestContext2Apply_moduleVarResourceCount(t *testing.T) {
 	m := testModule(t, "apply-module-var-resource-count")
 	p := testProvider("aws")

terraform/graph_builder.go

@@ -91,7 +91,7 @@ type BuiltinGraphBuilder struct {
 // Build builds the graph according to the steps returned by Steps.
 func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) {
 	basic := &BasicGraphBuilder{
-		Steps:    b.Steps(),
+		Steps:    b.Steps(path),
 		Validate: b.Validate,
 	}
 
@@ -100,7 +100,7 @@ func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) {
 
 // Steps returns the ordered list of GraphTransformers that must be executed
 // to build a complete graph.
-func (b *BuiltinGraphBuilder) Steps() []GraphTransformer {
+func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer {
 	steps := []GraphTransformer{
 		// Create all our resources from the configuration and state
 		&ConfigTransformer{Module: b.Root},
@@ -144,20 +144,31 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer {
 		// their dependencies.
 		&TargetsTransformer{Targets: b.Targets, Destroy: b.Destroy},
 
-		// Create the destruction nodes
-		&DestroyTransformer{},
-		&CreateBeforeDestroyTransformer{},
-		b.conditional(&conditionalOpts{
-			If:   func() bool { return !b.Verbose },
-			Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State},
-		}),
-
-		// Make sure we create one root
+		// Make sure we have a single root
 		&RootTransformer{},
+	}
 
-		// Perform the transitive reduction to make our graph a bit
-		// more sane if possible (it usually is possible).
-		&TransitiveReductionTransformer{},
+	// If we're on the root path, then we do a bunch of other stuff.
+	// We don't do the following for modules.
+	if len(path) <= 1 {
+		steps = append(steps,
+			// Create the destruction nodes
+			&DestroyTransformer{},
+			&CreateBeforeDestroyTransformer{},
+			b.conditional(&conditionalOpts{
+				If:   func() bool { return !b.Verbose },
+				Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State},
+			}),
+
+			// Make sure we have a single root after the above changes.
+			// This is the 2nd root transformer. In practice this shouldn't
+			// actually matter as the RootTransformer is idempotent.
+			&RootTransformer{},
+
+			// Perform the transitive reduction to make our graph a bit
+			// more sane if possible (it usually is possible).
+			&TransitiveReductionTransformer{},
+		)
 	}
 
 	// Remove nils

terraform/graph_config_node_resource.go

@@ -268,10 +268,38 @@ func (n *GraphNodeConfigResourceFlat) ProvidedBy() []string {
 }
 
 // GraphNodeDestroyable impl.
-func (n *GraphNodeConfigResourceFlat) DestroyNode(GraphNodeDestroyMode) GraphNodeDestroy {
-	// Disable destroy mode for flattened resources since we already
-	// did this within the original subgraph.
-	return nil
+func (n *GraphNodeConfigResourceFlat) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy {
+	// Get our parent destroy node. If we don't have any, just return
+	raw := n.GraphNodeConfigResource.DestroyNode(mode)
+	if raw == nil {
+		return nil
+	}
+
+	node, ok := raw.(*graphNodeResourceDestroy)
+	if !ok {
+		panic(fmt.Sprintf("unknown destroy node: %s %T", dag.VertexName(raw), raw))
+	}
+
+	// Otherwise, wrap it so that it gets the proper module treatment.
+	return &graphNodeResourceDestroyFlat{
+		graphNodeResourceDestroy: node,
+		PathValue:                n.PathValue,
+	}
+}
+
+type graphNodeResourceDestroyFlat struct {
+	*graphNodeResourceDestroy
+
+	PathValue []string
+}
+
+func (n *graphNodeResourceDestroyFlat) Name() string {
+	return fmt.Sprintf(
+		"%s.%s", modulePrefixStr(n.PathValue), n.graphNodeResourceDestroy.Name())
+}
+
+func (n *graphNodeResourceDestroyFlat) Path() []string {
+	return n.PathValue
 }
 
 // graphNodeResourceDestroy represents the logical destruction of a

terraform/terraform_test.go

@@ -362,6 +362,12 @@ module.child:
   leader = 1
 `
 
+const testTerraformApplyModuleDestroyOrderStr = `
+<no state>
+module.child:
+  <no state>
+`
+
 const testTerraformApplyMultiProviderStr = `
 aws_instance.bar:
   ID = foo

terraform/test-fixtures/apply-module-destroy-order/child/main.tf

@@ -0,0 +1,5 @@
+resource "aws_instance" "a" {}
+
+output "a_output" {
+    value = "${aws_instance.a.id}"
+}

terraform/test-fixtures/apply-module-destroy-order/main.tf

@@ -0,0 +1,7 @@
+module "child" {
+  source = "./child"
+}
+
+resource "aws_instance" "b" {
+    blah = "${module.child.a_output}"
+}

terraform/transform_destroy.go

@@ -102,6 +102,14 @@ func (t *DestroyTransformer) transform(
 		// Inherit all the edges from the old node
 		downEdges := g.DownEdges(v).List()
 		for _, edgeRaw := range downEdges {
+			// Don't inherit proxies. These are currently variables and
+			// outputs and don't affect destroys. In the future we should
+			// probably make this more obvious somehow (another interface?).
+			// Right now I'm not sure how.
+			if _, ok := edgeRaw.(GraphNodeProxy); ok {
+				continue
+			}
+
 			g.Connect(dag.BasicEdge(n, edgeRaw.(dag.Vertex)))
 		}
 
@@ -204,15 +212,6 @@ type PruneDestroyTransformer struct {
 }
 
 func (t *PruneDestroyTransformer) Transform(g *Graph) error {
-	var modDiff *ModuleDiff
-	var modState *ModuleState
-	if t.Diff != nil {
-		modDiff = t.Diff.ModuleByPath(g.Path)
-	}
-	if t.State != nil {
-		modState = t.State.ModuleByPath(g.Path)
-	}
-
 	for _, v := range g.Vertices() {
 		// If it is not a destroyer, we don't care
 		dn, ok := v.(GraphNodeDestroyPrunable)
@@ -220,6 +219,20 @@ func (t *PruneDestroyTransformer) Transform(g *Graph) error {
 			continue
 		}
 
+		path := g.Path
+		if pn, ok := v.(GraphNodeSubPath); ok {
+			path = pn.Path()
+		}
+
+		var modDiff *ModuleDiff
+		var modState *ModuleState
+		if t.Diff != nil {
+			modDiff = t.Diff.ModuleByPath(path)
+		}
+		if t.State != nil {
+			modState = t.State.ModuleByPath(path)
+		}
+
 		// Remove it if we should
 		if !dn.DestroyInclude(modDiff, modState) {
 			g.Remove(v)