core: Simplify our idea of "root node" and require it for DynamicExpand

internal/terraform/graph.go

@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"fmt"
 	"log"
 	"strings"
 
@@ -88,6 +89,28 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics {
 				return
 			}
 			if g != nil {
+				// The subgraph should always be valid, per our normal acyclic
+				// graph validation rules.
+				if err := g.Validate(); err != nil {
+					diags = diags.Append(tfdiags.Sourceless(
+						tfdiags.Error,
+						"Graph node has invalid dynamic subgraph",
+						fmt.Sprintf("The internal logic for %q generated an invalid dynamic subgraph: %s.\n\nThis is a bug in Terraform. Please report it!", dag.VertexName(v), err),
+					))
+					return
+				}
+				// If we passed validation then there is exactly one root node.
+				// That root node should always be "rootNode", the singleton
+				// root node value.
+				if n, err := g.Root(); err != nil || n != dag.Vertex(rootNode) {
+					diags = diags.Append(tfdiags.Sourceless(
+						tfdiags.Error,
+						"Graph node has invalid dynamic subgraph",
+						fmt.Sprintf("The internal logic for %q generated an invalid dynamic subgraph: the root node is %T, which is not a suitable root node type.\n\nThis is a bug in Terraform. Please report it!", dag.VertexName(v), v),
+					))
+					return
+				}
+
 				// Walk the subgraph
 				log.Printf("[TRACE] vertex %q: entering dynamic subgraph", dag.VertexName(v))
 				subDiags := g.walk(walker)

internal/terraform/node_local.go

@@ -73,6 +73,7 @@ func (n *nodeExpandLocal) DynamicExpand(ctx EvalContext) (*Graph, error) {
 		log.Printf("[TRACE] Expanding local: adding %s as %T", o.Addr.String(), o)
 		g.Add(o)
 	}
+	addRootNodeToGraph(&g)
 	return &g, nil
 }
 

internal/terraform/node_module_variable.go

@@ -50,6 +50,7 @@ func (n *nodeExpandModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error
 		}
 		g.Add(o)
 	}
+	addRootNodeToGraph(&g)
 	return &g, nil
 }
 

internal/terraform/node_output.go

@@ -96,6 +96,7 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) {
 		log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o)
 		g.Add(o)
 	}
+	addRootNodeToGraph(&g)
 
 	if checkableAddrs != nil {
 		checkState := ctx.Checks()

internal/terraform/node_resource_apply.go

@@ -49,6 +49,7 @@ func (n *nodeExpandApplyableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 			Addr:                 n.Addr.Resource.Absolute(module),
 		})
 	}
+	addRootNodeToGraph(&g)
 
 	return &g, nil
 }

internal/terraform/node_resource_import.go

@@ -176,11 +176,7 @@ func (n *graphNodeImportState) DynamicExpand(ctx EvalContext) (*Graph, error) {
 		})
 	}
 
-	// Root transform for a single root
-	t := &RootTransformer{}
-	if err := t.Transform(g); err != nil {
-		return nil, err
-	}
+	addRootNodeToGraph(g)
 
 	// Done!
 	return g, diags.Err()

internal/terraform/node_resource_plan.go

@@ -171,6 +171,8 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 		checkState.ReportCheckableObjects(n.NodeAbstractResource.Addr, instAddrs)
 	}
 
+	addRootNodeToGraph(&g)
+
 	return &g, diags.ErrWithWarnings()
 }
 

internal/terraform/transform_root.go

@@ -10,41 +10,48 @@ const rootNodeName = "root"
 type RootTransformer struct{}
 
 func (t *RootTransformer) Transform(g *Graph) error {
-	// If we already have a good root, we're done
-	if _, err := g.Root(); err == nil {
-		return nil
-	}
+	addRootNodeToGraph(g)
+	return nil
+}
 
-	// We intentionally add a graphNodeRoot value -- rather than a pointer to
-	// one -- so that all root nodes will coalesce together if two graphs
-	// are merged. Each distinct node value can only be in a graph once,
-	// so adding another graphNodeRoot value to the same graph later will
-	// be a no-op and all of the edges from root nodes will coalesce together
-	// under Graph.Subsume.
+// addRootNodeToGraph modifies the given graph in-place so that it has a root
+// node if it didn't already have one and so that any other node which doesn't
+// already depend on something will depend on that root node.
+//
+// After this function returns, the graph will have only one node that doesn't
+// depend on any other nodes.
+func addRootNodeToGraph(g *Graph) {
+	// We always add the root node. This is a singleton so if it's already
+	// in the graph this will do nothing and just retain the existing root node.
 	//
-	// It's important to retain this coalescing guarantee under future
-	// maintenence.
-	var root graphNodeRoot
-	g.Add(root)
+	// Note that rootNode is intentionally added by value and not by pointer
+	// so that all root nodes will be equal to one another and therefore
+	// coalesce when two valid graphs get merged together into a single graph.
+	g.Add(rootNode)
 
-	// We initially make the root node depend on every node except itself.
-	// If the caller subsequently runs transitive reduction on the graph then
-	// it's typical for some of these edges to then be removed.
+	// Everything that doesn't already depend on at least one other node will
+	// depend on the root node, except the root node itself.
 	for _, v := range g.Vertices() {
-		if v == root {
+		if v == dag.Vertex(rootNode) {
 			continue
 		}
 
 		if g.UpEdges(v).Len() == 0 {
-			g.Connect(dag.BasicEdge(root, v))
+			g.Connect(dag.BasicEdge(rootNode, v))
 		}
 	}
-
-	return nil
 }
 
 type graphNodeRoot struct{}
 
+// rootNode is the singleton value representing all root graph nodes.
+//
+// The root node for all graphs should be this value directly, and in particular
+// _not_ a pointer to this value. Using the value directly here means that
+// multiple root nodes will always coalesce together when subsuming one graph
+// into another.
+var rootNode graphNodeRoot
+
 func (n graphNodeRoot) Name() string {
 	return rootNodeName
 }

internal/terraform/transform_root_test.go

@@ -8,50 +8,78 @@ import (
 )
 
 func TestRootTransformer(t *testing.T) {
-	mod := testModule(t, "transform-root-basic")
+	t.Run("many nodes", func(t *testing.T) {
+		mod := testModule(t, "transform-root-basic")
 
-	g := Graph{Path: addrs.RootModuleInstance}
-	{
-		tf := &ConfigTransformer{Config: mod}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
+		g := Graph{Path: addrs.RootModuleInstance}
+		{
+			tf := &ConfigTransformer{Config: mod}
+			if err := tf.Transform(&g); err != nil {
+				t.Fatalf("err: %s", err)
+			}
 		}
-	}
 
-	{
-		transform := &MissingProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
+		{
+			transform := &MissingProviderTransformer{}
+			if err := transform.Transform(&g); err != nil {
+				t.Fatalf("err: %s", err)
+			}
 		}
-	}
 
-	{
-		transform := &ProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
+		{
+			transform := &ProviderTransformer{}
+			if err := transform.Transform(&g); err != nil {
+				t.Fatalf("err: %s", err)
+			}
+		}
+
+		{
+			transform := &RootTransformer{}
+			if err := transform.Transform(&g); err != nil {
+				t.Fatalf("err: %s", err)
+			}
+		}
+
+		actual := strings.TrimSpace(g.String())
+		expected := strings.TrimSpace(testTransformRootBasicStr)
+		if actual != expected {
+			t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
 		}
-	}
 
-	{
-		transform := &RootTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		root, err := g.Root()
+		if err != nil {
 			t.Fatalf("err: %s", err)
 		}
-	}
+		if _, ok := root.(graphNodeRoot); !ok {
+			t.Fatalf("bad: %#v", root)
+		}
+	})
+
+	t.Run("only one initial node", func(t *testing.T) {
+		g := Graph{Path: addrs.RootModuleInstance}
+		g.Add("foo")
+		addRootNodeToGraph(&g)
+		got := strings.TrimSpace(g.String())
+		want := strings.TrimSpace(`
+foo
+root
+  foo
+`)
+		if got != want {
+			t.Errorf("wrong final graph\ngot:\n%s\nwant:\n%s", got, want)
+		}
+	})
 
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformRootBasicStr)
-	if actual != expected {
-		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
-	}
+	t.Run("graph initially empty", func(t *testing.T) {
+		g := Graph{Path: addrs.RootModuleInstance}
+		addRootNodeToGraph(&g)
+		got := strings.TrimSpace(g.String())
+		want := `root`
+		if got != want {
+			t.Errorf("wrong final graph\ngot:\n%s\nwant:\n%s", got, want)
+		}
+	})
 
-	root, err := g.Root()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if _, ok := root.(graphNodeRoot); !ok {
-		t.Fatalf("bad: %#v", root)
-	}
 }
 
 const testTransformRootBasicStr = `