Merge pull request #2786 from hashicorp/b-nested-module-orphans

Grandchild module orphans should be destroyed

terraform/context_apply_test.go

@@ -2097,6 +2097,105 @@ func TestContext2Apply_destroy(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_destroyNestedModule(t *testing.T) {
+	m := testModule(t, "apply-destroy-nested-module")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	s := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: []string{"root", "child", "subchild"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.bar": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "bar",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: s,
+	})
+
+	// First plan and apply a create operation
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Test that things were destroyed
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyDestroyNestedModuleStr)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
+func TestContext2Apply_destroyDeeplyNestedModule(t *testing.T) {
+	m := testModule(t, "apply-destroy-deeply-nested-module")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	s := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: []string{"root", "child", "subchild", "subsubchild"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.bar": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "bar",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: s,
+	})
+
+	// First plan and apply a create operation
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Test that things were destroyed
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(`
+module.child.subchild.subsubchild:
+  <no state>
+	`)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
 func TestContext2Apply_destroyOutputs(t *testing.T) {
 	m := testModule(t, "apply-destroy-outputs")
 	h := new(HookRecordApplyOrder)

terraform/state.go

@@ -104,24 +104,56 @@ func (s *State) ModuleByPath(path []string) *ModuleState {
 // returning their full paths. These paths can be used with ModuleByPath
 // to return the actual state.
 func (s *State) ModuleOrphans(path []string, c *config.Config) [][]string {
+	// direct keeps track of what direct children we have both in our config
+	// and in our state. childrenKeys keeps track of what isn't an orphan.
+	direct := make(map[string]struct{})
 	childrenKeys := make(map[string]struct{})
 	if c != nil {
 		for _, m := range c.Modules {
 			childrenKeys[m.Name] = struct{}{}
+			direct[m.Name] = struct{}{}
 		}
 	}
 
-	// Go over the direct children and find any that aren't in our
-	// keys.
+	// Go over the direct children and find any that aren't in our keys.
 	var orphans [][]string
 	for _, m := range s.Children(path) {
-		if _, ok := childrenKeys[m.Path[len(m.Path)-1]]; ok {
+		key := m.Path[len(m.Path)-1]
+
+		// Record that we found this key as a direct child. We use this
+		// later to find orphan nested modules.
+		direct[key] = struct{}{}
+
+		// If we have a direct child still in our config, it is not an orphan
+		if _, ok := childrenKeys[key]; ok {
 			continue
 		}
 
 		orphans = append(orphans, m.Path)
 	}
 
+	// Find the orphans that are nested...
+	for _, m := range s.Modules {
+		// We only want modules that are at least grandchildren
+		if len(m.Path) < len(path)+2 {
+			continue
+		}
+
+		// If it isn't part of our tree, continue
+		if !reflect.DeepEqual(path, m.Path[:len(path)]) {
+			continue
+		}
+
+		// If we have the direct child, then just skip it.
+		key := m.Path[len(path)]
+		if _, ok := direct[key]; ok {
+			continue
+		}
+
+		// Add this orphan
+		orphans = append(orphans, m.Path[:len(path)+1])
+	}
+
 	return orphans
 }
 

terraform/state_test.go

@@ -85,6 +85,28 @@ func TestStateModuleOrphans(t *testing.T) {
 	}
 }
 
+func TestStateModuleOrphans_nested(t *testing.T) {
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: RootModulePath,
+			},
+			&ModuleState{
+				Path: []string{RootModuleName, "foo", "bar"},
+			},
+		},
+	}
+
+	actual := state.ModuleOrphans(RootModulePath, nil)
+	expected := [][]string{
+		[]string{RootModuleName, "foo"},
+	}
+
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("bad: %#v", actual)
+	}
+}
+
 func TestStateModuleOrphans_nilConfig(t *testing.T) {
 	state := &State{
 		Modules: []*ModuleState{

terraform/terraform_test.go

@@ -483,6 +483,11 @@ const testTerraformApplyDestroyStr = `
 <no state>
 `
 
+const testTerraformApplyDestroyNestedModuleStr = `
+module.child.subchild:
+  <no state>
+`
+
 const testTerraformApplyErrorStr = `
 aws_instance.bar:
   ID = bar

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

@@ -0,0 +1,3 @@
+module "subchild" {
+  source = "./subchild"
+}

terraform/test-fixtures/apply-destroy-deeply-nested-module/child/subchild/main.tf

@@ -0,0 +1,5 @@
+/*
+module "subsubchild" {
+  source = "./subsubchild"
+}
+*/

terraform/test-fixtures/apply-destroy-deeply-nested-module/child/subchild/subsubchild/main.tf

@@ -0,0 +1 @@
+resource "aws_instance" "bar" {}

terraform/test-fixtures/apply-destroy-deeply-nested-module/main.tf

@@ -0,0 +1,3 @@
+module "child" {
+  source = "./child"
+}

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

@@ -0,0 +1,3 @@
+module "subchild" {
+    source = "./subchild"
+}

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

@@ -0,0 +1 @@
+resource "aws_instance" "bar" {}

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

@@ -0,0 +1,5 @@
+/*
+module "child" {
+    source = "./child"
+}
+*/

terraform/transform_orphan.go

@@ -100,9 +100,14 @@ func (t *OrphanTransformer) Transform(g *Graph) error {
 	moduleOrphans := t.State.ModuleOrphans(g.Path, config)
 	moduleVertexes := make([]dag.Vertex, len(moduleOrphans))
 	for i, path := range moduleOrphans {
+		var deps []string
+		if s := t.State.ModuleByPath(path); s != nil {
+			deps = s.Dependencies
+		}
+
 		moduleVertexes[i] = g.Add(&graphNodeOrphanModule{
 			Path:        path,
-			dependentOn: t.State.ModuleByPath(path).Dependencies,
+			dependentOn: deps,
 		})
 	}
 
@@ -356,3 +361,9 @@ func (n *graphNodeOrphanResourceFlat) CreateBeforeDestroy() bool {
 func (n *graphNodeOrphanResourceFlat) CreateNode() dag.Vertex {
 	return n
 }
+
+func (n *graphNodeOrphanResourceFlat) ProvidedBy() []string {
+	return modulePrefixList(
+		n.graphNodeOrphanResource.ProvidedBy(),
+		modulePrefixStr(n.PathValue))
+}