Merge pull request #7648 from hashicorp/phinze/GH-7378

dag: fix ReverseDepthFirstWalk when nodes remove themselves

dag/dag.go

@@ -332,12 +332,7 @@ func (g *AcyclicGraph) ReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) er
 		}
 		seen[current.Vertex] = struct{}{}
 
-		// Visit the current node
-		if err := f(current.Vertex, current.Depth); err != nil {
-			return err
-		}
-
-		// Visit targets of this in a consistent order.
+		// Add next set of targets in a consistent order.
 		targets := AsVertexList(g.UpEdges(current.Vertex))
 		sort.Sort(byVertexName(targets))
 		for _, t := range targets {
@@ -346,6 +341,11 @@ func (g *AcyclicGraph) ReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) er
 				Depth:  current.Depth + 1,
 			})
 		}
+
+		// Visit the current node
+		if err := f(current.Vertex, current.Depth); err != nil {
+			return err
+		}
 	}
 
 	return nil

dag/dag_test.go

@@ -260,6 +260,33 @@ func TestAcyclicGraphWalk_error(t *testing.T) {
 	t.Fatalf("bad: %#v", visits)
 }
 
+func TestAcyclicGraph_ReverseDepthFirstWalk_WithRemoval(t *testing.T) {
+	var g AcyclicGraph
+	g.Add(1)
+	g.Add(2)
+	g.Add(3)
+	g.Connect(BasicEdge(3, 2))
+	g.Connect(BasicEdge(2, 1))
+
+	var visits []Vertex
+	var lock sync.Mutex
+	err := g.ReverseDepthFirstWalk([]Vertex{1}, func(v Vertex, d int) error {
+		lock.Lock()
+		defer lock.Unlock()
+		visits = append(visits, v)
+		g.Remove(v)
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	expected := []Vertex{1, 2, 3}
+	if !reflect.DeepEqual(visits, expected) {
+		t.Fatalf("expected: %#v, got: %#v", expected, visits)
+	}
+}
+
 const testGraphTransReductionStr = `
 1
   2

dag/graph.go

@@ -202,16 +202,17 @@ func (g *Graph) StringWithNodeTypes() string {
 
 		// Alphabetize dependencies
 		deps := make([]string, 0, targets.Len())
-		targetNodes := make([]Vertex, 0, targets.Len())
+		targetNodes := make(map[string]Vertex)
 		for _, target := range targets.List() {
-			deps = append(deps, VertexName(target))
-			targetNodes = append(targetNodes, target)
+			dep := VertexName(target)
+			deps = append(deps, dep)
+			targetNodes[dep] = target
 		}
 		sort.Strings(deps)
 
 		// Write dependencies
-		for i, d := range deps {
-			buf.WriteString(fmt.Sprintf("  %s - %T\n", d, targetNodes[i]))
+		for _, d := range deps {
+			buf.WriteString(fmt.Sprintf("  %s - %T\n", d, targetNodes[d]))
 		}
 	}
 

terraform/context_apply_test.go

@@ -4873,6 +4873,8 @@ func TestContext2Apply_destroyNestedModuleWithAttrsReferencingResource(t *testin
 	expected := strings.TrimSpace(`
 <no state>
 module.middle:
+  <no state>
+module.middle.bottom:
   <no state>
 		`)
 	if actual != expected {

terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/bottom/bottom.tf

@@ -1 +1,5 @@
-variable "bottom_param" {}
+variable bottom_param {}
+
+resource "null_resource" "bottom" {
+  value = "${var.bottom_param}"
+}

terraform/test-fixtures/apply-destroy-nested-module-with-attrs/middle/middle.tf

@@ -1,8 +1,10 @@
-variable "param" {}
-
-resource "null_resource" "n" {}
+variable param {}
 
 module "bottom" {
   source       = "./bottom"
   bottom_param = "${var.param}"
 }
+
+resource "null_resource" "middle" {
+  value = "${var.param}"
+}