Merge pull request #26186 from hashicorp/jbardin/cbd-module-dep

don't connect module closers to destroy nodes

Author: jbardin

terraform/context_apply_test.go

@@ -11766,3 +11766,84 @@ output "outputs" {
 	// Destroying again from the empty state should not cause any errors either
 	destroy()
 }
+
+func TestContext2Apply_createBeforeDestroyWithModule(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+variable "v" {}
+
+module "mod" {
+    source = "./mod"
+    in = var.v
+}
+
+resource "test_resource" "a" {
+  value = var.v
+  depends_on = [module.mod]
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+`,
+		"mod/main.tf": `
+variable "in" {}
+
+resource "test_resource" "a" {
+  value = var.in
+}
+`})
+
+	p := testProvider("test")
+	p.ApplyFn = testApplyFn
+	p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
+		proposed := req.ProposedNewState.AsValueMap()
+		proposed["id"] = cty.UnknownVal(cty.String)
+		return providers.PlanResourceChangeResponse{
+			PlannedState:    cty.ObjectVal(proposed),
+			RequiresReplace: []cty.Path{cty.Path{cty.GetAttrStep{Name: "value"}}},
+		}
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		Variables: InputValues{
+			"v": &InputValue{
+				Value: cty.StringVal("A"),
+			},
+		},
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	state, diags := ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("apply errors: %s", diags.Err())
+	}
+
+	ctx = testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		Variables: InputValues{
+			"v": &InputValue{
+				Value: cty.StringVal("B"),
+			},
+		},
+		State: state,
+	})
+
+	if _, diags := ctx.Plan(); diags.HasErrors() {
+		t.Fatalf("plan errors: %s", diags.Err())
+	}
+
+	_, diags = ctx.Apply()
+	if diags.HasErrors() {
+		t.Fatalf("apply errors: %s", diags.Err())
+	}
+}

terraform/transform_module_expansion.go

@@ -43,8 +43,13 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error {
 	// handled by the RemovedModuleTransformer, and those module closers are in
 	// the graph already, and need to be connected to their parent closers.
 	for _, v := range g.Vertices() {
-		// skip closers so they don't attach to themselves
-		if _, ok := v.(*nodeCloseModule); ok {
+		switch v.(type) {
+		case GraphNodeDestroyer:
+			// Destroy nodes can only be ordered relative to other resource
+			// instances.
+			continue
+		case *nodeCloseModule:
+			// a module closer cannot connect to itself
 			continue
 		}
 
@@ -84,17 +89,17 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare
 		Config:     c.Module,
 		ModuleCall: modCall,
 	}
-	var v dag.Vertex = n
+	var expander dag.Vertex = n
 	if t.Concrete != nil {
-		v = t.Concrete(n)
+		expander = t.Concrete(n)
 	}
 
-	g.Add(v)
-	log.Printf("[TRACE] ModuleExpansionTransformer: Added %s as %T", c.Path, v)
+	g.Add(expander)
+	log.Printf("[TRACE] ModuleExpansionTransformer: Added %s as %T", c.Path, expander)
 
 	if parentNode != nil {
-		log.Printf("[TRACE] ModuleExpansionTransformer: %s must wait for expansion of %s", dag.VertexName(v), dag.VertexName(parentNode))
-		g.Connect(dag.BasicEdge(v, parentNode))
+		log.Printf("[TRACE] ModuleExpansionTransformer: %s must wait for expansion of %s", dag.VertexName(expander), dag.VertexName(parentNode))
+		g.Connect(dag.BasicEdge(expander, parentNode))
 	}
 
 	// Add the closer (which acts as the root module node) to provide a
@@ -103,12 +108,12 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare
 		Addr: c.Path,
 	}
 	g.Add(closer)
-	g.Connect(dag.BasicEdge(closer, v))
+	g.Connect(dag.BasicEdge(closer, expander))
 	t.closers[c.Path.String()] = closer
 
 	for _, childV := range g.Vertices() {
 		// don't connect a node to itself
-		if childV == v {
+		if childV == expander {
 			continue
 		}
 
@@ -126,13 +131,13 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare
 
 		if path.Equal(c.Path) {
 			log.Printf("[TRACE] ModuleExpansionTransformer: %s must wait for expansion of %s", dag.VertexName(childV), c.Path)
-			g.Connect(dag.BasicEdge(childV, v))
+			g.Connect(dag.BasicEdge(childV, expander))
 		}
 	}
 
 	// Also visit child modules, recursively.
 	for _, cc := range c.Children {
-		if err := t.transform(g, cc, v); err != nil {
+		if err := t.transform(g, cc, expander); err != nil {
 			return err
 		}
 	}