terraform: new apply graph creates provisioners in modules

Fixes #9840

The new apply graph wasn't properly nesting provisioners. This resulted
in reading the provisioners being nil on apply in the shadow graph which
caused the crash in the above issue.

The actual cause of this is that the new graphs we're moving towards do
not have any "flattening" (they are flat to begin with): all modules are
in the root graph from the beginning of construction versus building a
number of different graphs and flattening them. The transform that adds
the provisioners wasn't modified to handle already-flat graphs and so
was only adding provisioners to the root module, not children.

The change modifies the `MissingProvisionerTransformer` (primarily) to
support already-flat graphs and add provisioners for all module levels.
Tests are there to cover this as well.

**NOTE:** This PR focuses on fixing that specific issue. I'm going to follow up
this PR with another PR that is more focused on being robust against
crashing (more nil checks, recover() for shadow graph, etc.). In the
interest of focus and keeping a PR reviewable this focuses only on the
issue itself.

terraform/context_apply_test.go

@@ -2246,6 +2246,43 @@ func TestContext2Apply_providerConfigureDisabled(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_provisionerModule(t *testing.T) {
+	m := testModule(t, "apply-provisioner-module")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyProvisionerModuleStr)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+
+	// Verify apply was invoked
+	if !pr.ApplyCalled {
+		t.Fatalf("provisioner not invoked")
+	}
+}
+
 func TestContext2Apply_Provisioner_compute(t *testing.T) {
 	m := testModule(t, "apply-provisioner-compute")
 	p := testProvider("aws")

terraform/graph_builder_apply_test.go

@@ -100,16 +100,16 @@ meta.count-boundary (count boundary fixup)
   module.child.aws_instance.create
   module.child.aws_instance.other
   module.child.provider.aws
+  module.child.provisioner.exec
   provider.aws
-  provisioner.exec
 module.child.aws_instance.create
   module.child.provider.aws
-  provisioner.exec
+  module.child.provisioner.exec
 module.child.aws_instance.other
   module.child.aws_instance.create
   module.child.provider.aws
 module.child.provider.aws
   provider.aws
+module.child.provisioner.exec
 provider.aws
-provisioner.exec
 `

terraform/terraform_test.go

@@ -522,6 +522,13 @@ aws_instance.foo:
   type = aws_instance
 `
 
+const testTerraformApplyProvisionerModuleStr = `
+<no state>
+module.child:
+  aws_instance.bar:
+    ID = foo
+`
+
 const testTerraformApplyProvisionerFailStr = `
 aws_instance.bar: (tainted)
   ID = foo

terraform/test-fixtures/apply-provisioner-module/child/main.tf

@@ -0,0 +1,5 @@
+resource "aws_instance" "bar" {
+    provisioner "shell" {
+        foo = "bar"
+    }
+}

terraform/test-fixtures/apply-provisioner-module/main.tf

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

terraform/test-fixtures/transform-provisioner-module/child/main.tf

@@ -0,0 +1,3 @@
+resource "aws_instance" "foo" {
+    provisioner "shell" {}
+}

terraform/test-fixtures/transform-provisioner-module/main.tf

@@ -0,0 +1,7 @@
+resource "aws_instance" "foo" {
+  provisioner "shell" {}
+}
+
+module "child" {
+  source = "./child"
+}

terraform/transform_provisioner.go

@@ -40,14 +40,15 @@ func (t *ProvisionerTransformer) Transform(g *Graph) error {
 	for _, v := range g.Vertices() {
 		if pv, ok := v.(GraphNodeProvisionerConsumer); ok {
 			for _, p := range pv.ProvisionedBy() {
-				if m[p] == nil {
+				key := provisionerMapKey(p, pv)
+				if m[key] == nil {
 					err = multierror.Append(err, fmt.Errorf(
 						"%s: provisioner %s couldn't be found",
 						dag.VertexName(v), p))
 					continue
 				}
 
-				g.Connect(dag.BasicEdge(v, m[p]))
+				g.Connect(dag.BasicEdge(v, m[key]))
 			}
 		}
 	}
@@ -80,8 +81,21 @@ func (t *MissingProvisionerTransformer) Transform(g *Graph) error {
 			continue
 		}
 
+		// If this node has a subpath, then we use that as a prefix
+		// into our map to check for an existing provider.
+		var path []string
+		if sp, ok := pv.(GraphNodeSubPath); ok {
+			raw := normalizeModulePath(sp.Path())
+			if len(raw) > len(rootModulePath) {
+				path = raw
+			}
+		}
+
 		for _, p := range pv.ProvisionedBy() {
-			if _, ok := m[p]; ok {
+			// Build the key for storing in the map
+			key := provisionerMapKey(p, pv)
+
+			if _, ok := m[key]; ok {
 				// This provisioner already exists as a configure node
 				continue
 			}
@@ -92,8 +106,23 @@ func (t *MissingProvisionerTransformer) Transform(g *Graph) error {
 				continue
 			}
 
+			// Build the vertex
+			var newV dag.Vertex = &graphNodeProvisioner{ProvisionerNameValue: p}
+			if len(path) > 0 {
+				// If we have a path, we do the flattening immediately. This
+				// is to support new-style graph nodes that are already
+				// flattened.
+				if fn, ok := newV.(GraphNodeFlattenable); ok {
+					var err error
+					newV, err = fn.Flatten(path)
+					if err != nil {
+						return err
+					}
+				}
+			}
+
 			// Add the missing provisioner node to the graph
-			m[p] = g.Add(&graphNodeProvisioner{ProvisionerNameValue: p})
+			m[key] = g.Add(newV)
 		}
 	}
 
@@ -131,6 +160,20 @@ func (t *CloseProvisionerTransformer) Transform(g *Graph) error {
 	return nil
 }
 
+// provisionerMapKey is a helper that gives us the key to use for the
+// maps returned by things such as provisionerVertexMap.
+func provisionerMapKey(k string, v dag.Vertex) string {
+	pathPrefix := ""
+	if sp, ok := v.(GraphNodeSubPath); ok {
+		raw := normalizeModulePath(sp.Path())
+		if len(raw) > len(rootModulePath) {
+			pathPrefix = modulePrefixStr(raw) + "."
+		}
+	}
+
+	return pathPrefix + k
+}
+
 func provisionerVertexMap(g *Graph) map[string]dag.Vertex {
 	m := make(map[string]dag.Vertex)
 	for _, v := range g.Vertices() {

terraform/transform_provisioner_test.go

@@ -39,6 +39,74 @@ func TestMissingProvisionerTransformer(t *testing.T) {
 	}
 }
 
+func TestMissingProvisionerTransformer_module(t *testing.T) {
+	mod := testModule(t, "transform-provisioner-module")
+
+	g := Graph{Path: RootModulePath}
+	{
+		concreteResource := func(a *NodeAbstractResource) dag.Vertex {
+			return a
+		}
+
+		var state State
+		state.init()
+		state.Modules = []*ModuleState{
+			&ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Primary: &InstanceState{ID: "foo"},
+					},
+				},
+			},
+
+			&ModuleState{
+				Path: []string{"root", "child"},
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Primary: &InstanceState{ID: "foo"},
+					},
+				},
+			},
+		}
+
+		tf := &StateTransformer{
+			Concrete: concreteResource,
+			State:    &state,
+		}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &AttachResourceConfigTransformer{Module: mod}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &MissingProvisionerTransformer{Provisioners: []string{"shell"}}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		transform := &ProvisionerTransformer{}
+		if err := transform.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformMissingProvisionerModuleStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
 func TestCloseProvisionerTransformer(t *testing.T) {
 	mod := testModule(t, "transform-provisioner-basic")
 
@@ -96,6 +164,15 @@ aws_instance.web
 provisioner.shell
 `
 
+const testTransformMissingProvisionerModuleStr = `
+aws_instance.foo
+  provisioner.shell
+module.child.aws_instance.foo
+  module.child.provisioner.shell
+module.child.provisioner.shell
+provisioner.shell
+`
+
 const testTransformCloseProvisionerBasicStr = `
 aws_instance.web
   provisioner.shell