Merge pull request #9846 from hashicorp/b-provisioner-crash

terraform: new apply graph creates provisioners in modules

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