Merge pull request #5022 from hashicorp/phinze/fix-provider-double-init-for-nested-module-orphans

core: fix bug detecting deeply nested module orphans

terraform/context_plan_test.go

@@ -368,6 +368,91 @@ func TestContext2Plan_moduleOrphans(t *testing.T) {
 	}
 }
 
+// https://github.com/hashicorp/terraform/issues/3114
+func TestContext2Plan_moduleOrphansWithProvisioner(t *testing.T) {
+	m := testModule(t, "plan-modules-remove-provisioners")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.DiffFn = testDiffFn
+	s := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.top": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "top",
+						},
+					},
+				},
+			},
+			&ModuleState{
+				Path: []string{"root", "parent", "childone"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "baz",
+						},
+					},
+				},
+			},
+			&ModuleState{
+				Path: []string{"root", "parent", "childtwo"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "baz",
+						},
+					},
+				},
+			},
+		},
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+		State: s,
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(`
+DIFF:
+
+module.parent.childone:
+  DESTROY: aws_instance.foo
+module.parent.childtwo:
+  DESTROY: aws_instance.foo
+
+STATE:
+
+aws_instance.top:
+  ID = top
+
+module.parent.childone:
+  aws_instance.foo:
+    ID = baz
+module.parent.childtwo:
+  aws_instance.foo:
+    ID = baz
+	`)
+	if actual != expected {
+		t.Fatalf("bad:\n%s", actual)
+	}
+}
+
 func TestContext2Plan_moduleProviderInherit(t *testing.T) {
 	var l sync.Mutex
 	var calls []string

terraform/state.go

@@ -151,8 +151,23 @@ func (s *State) ModuleOrphans(path []string, c *config.Config) [][]string {
 			continue
 		}
 
+		orphanPath := m.Path[:len(path)+1]
+
+		// Don't double-add if we've already added this orphan (which can happen if
+		// there are multiple nested sub-modules that get orphaned together).
+		alreadyAdded := false
+		for _, o := range orphans {
+			if reflect.DeepEqual(o, orphanPath) {
+				alreadyAdded = true
+				break
+			}
+		}
+		if alreadyAdded {
+			continue
+		}
+
 		// Add this orphan
-		orphans = append(orphans, m.Path[:len(path)+1])
+		orphans = append(orphans, orphanPath)
 	}
 
 	return orphans

terraform/state_test.go

@@ -150,6 +150,31 @@ func TestStateModuleOrphans_nilConfig(t *testing.T) {
 	}
 }
 
+func TestStateModuleOrphans_deepNestedNilConfig(t *testing.T) {
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: RootModulePath,
+			},
+			&ModuleState{
+				Path: []string{RootModuleName, "parent", "childfoo"},
+			},
+			&ModuleState{
+				Path: []string{RootModuleName, "parent", "childbar"},
+			},
+		},
+	}
+
+	actual := state.ModuleOrphans(RootModulePath, nil)
+	expected := [][]string{
+		[]string{RootModuleName, "parent"},
+	}
+
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("bad: %#v", actual)
+	}
+}
+
 func TestStateEqual(t *testing.T) {
 	cases := []struct {
 		Result   bool

terraform/test-fixtures/plan-modules-remove-provisioners/main.tf

@@ -0,0 +1,5 @@
+resource "aws_instance" "top" {}
+
+# module "test" {
+#   source = "./parent"
+# }

terraform/test-fixtures/plan-modules-remove-provisioners/parent/child/main.tf

@@ -0,0 +1,2 @@
+resource "aws_instance" "foo" {
+}

terraform/test-fixtures/plan-modules-remove-provisioners/parent/main.tf

@@ -0,0 +1,7 @@
+module "childone" {
+  source = "./child"
+}
+
+module "childtwo" {
+  source = "./child"
+}