terraform: make adding config nodes a transformer

terraform/graph.go

@@ -0,0 +1,77 @@
+package terraform
+
+import (
+	"sync"
+
+	"github.com/hashicorp/terraform/dag"
+)
+
+// RootModuleName is the name given to the root module implicitly.
+const RootModuleName = "root"
+
+// Graph represents the graph that Terraform uses to represent resources
+// and their dependencies. Each graph represents only one module, but it
+// can contain further modules, which themselves have their own graph.
+type Graph struct {
+	// Graph is the actual DAG. This is embedded so you can call the DAG
+	// methods directly.
+	*dag.Graph
+
+	// Path is the path in the module tree that this Graph represents.
+	// The root is represented by a single element list containing
+	// RootModuleName
+	Path []string
+
+	// dependableMap is a lookaside table for fast lookups for connecting
+	// dependencies by their GraphNodeDependable value to avoid O(n^3)-like
+	// situations and turn them into O(1) with respect to the number of new
+	// edges.
+	dependableMap map[string]dag.Vertex
+
+	once sync.Once
+}
+
+// Add is the same as dag.Graph.Add.
+func (g *Graph) Add(v dag.Vertex) dag.Vertex {
+	g.once.Do(g.init)
+
+	// Call upwards to add it to the actual graph
+	g.Graph.Add(v)
+
+	// If this is a depend-able node, then store the lookaside info
+	if dv, ok := v.(GraphNodeDependable); ok {
+		for _, n := range dv.DependableName() {
+			g.dependableMap[n] = v
+		}
+	}
+
+	return v
+}
+
+// ConnectTo is a helper to create edges between a node and a list of
+// targets by their DependableNames.
+func (g *Graph) ConnectTo(source dag.Vertex, target []string) []string {
+	g.once.Do(g.init)
+
+	// TODO: test
+	var missing []string
+	for _, t := range target {
+		if dest := g.dependableMap[t]; dest != nil {
+			g.Connect(dag.BasicEdge(source, dest))
+		} else {
+			missing = append(missing, t)
+		}
+	}
+
+	return missing
+}
+
+func (g *Graph) init() {
+	if g.Graph == nil {
+		g.Graph = new(dag.Graph)
+	}
+
+	if g.dependableMap == nil {
+		g.dependableMap = make(map[string]dag.Vertex)
+	}
+}

terraform/graph_config_node.go

@@ -18,11 +18,6 @@ type graphNodeConfig interface {
 	// depends on. The values within the slice should map to the VarName()
 	// values that are returned by any nodes.
 	Variables() []string
-
-	// VarName returns the name that is used to identify a variable
-	// maps to this node. It should match the result of the
-	// `VarName` function.
-	VarName() string
 }
 
 // GraphNodeConfigModule represents a module within the configuration graph.
@@ -31,6 +26,9 @@ type GraphNodeConfigModule struct {
 	Tree   *module.Tree
 }
 
+func (n *GraphNodeConfigModule) DependableName() []string {
+	return []string{n.Name()}
+}
 func (n *GraphNodeConfigModule) Name() string {
 	return fmt.Sprintf("module.%s", n.Module.Name)
 }
@@ -45,10 +43,6 @@ func (n *GraphNodeConfigModule) Variables() []string {
 	return result
 }
 
-func (n *GraphNodeConfigModule) VarName() string {
-	return n.Name()
-}
-
 // GraphNodeConfigProvider represents a configured provider within the
 // configuration graph. These are only immediately in the graph when an
 // explicit `provider` configuration block is in the configuration.
@@ -70,10 +64,6 @@ func (n *GraphNodeConfigProvider) Variables() []string {
 	return result
 }
 
-func (n *GraphNodeConfigProvider) VarName() string {
-	return "never valid"
-}
-
 // GraphNodeConfigResource represents a resource within the config graph.
 type GraphNodeConfigResource struct {
 	Resource *config.Resource
@@ -102,7 +92,3 @@ func (n *GraphNodeConfigResource) Variables() []string {
 
 	return result
 }
-
-func (n *GraphNodeConfigResource) VarName() string {
-	return n.Resource.Id()
-}

terraform/graph_test.go

@@ -0,0 +1,24 @@
+package terraform
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestGraphAdd(t *testing.T) {
+	// Test Add since we override it and want to make sure we don't break it.
+	var g Graph
+	g.Add(42)
+	g.Add(84)
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testGraphAddStr)
+	if actual != expected {
+		t.Fatalf("bad: %s", actual)
+	}
+}
+
+const testGraphAddStr = `
+42
+84
+`

terraform/transform.go

@@ -1,11 +1,7 @@
 package terraform
 
-import (
-	"github.com/hashicorp/terraform/dag"
-)
-
 // GraphTransformer is the interface that transformers implement. This
 // interface is only for transforms that need entire graph visibility.
 type GraphTransformer interface {
-	Transform(*dag.Graph) error
+	Transform(*Graph) error
 }

terraform/transform_config.go

@@ -7,22 +7,27 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
-	"github.com/hashicorp/terraform/dag"
 )
 
-// Graph takes a module tree and builds a logical graph of all the nodes
-// in that module.
-func Graph2(mod *module.Tree) (*dag.Graph, error) {
+// ConfigTransformer is a GraphTransformer that adds the configuration
+// to the graph. It is assumed that the module tree given in Module matches
+// the Path attribute of the Graph being transformed. If this is not the case,
+// the behavior is unspecified, but unlikely to be what you want.
+type ConfigTransformer struct {
+	Module *module.Tree
+}
+
+func (t *ConfigTransformer) Transform(g *Graph) error {
 	// A module is required and also must be completely loaded.
-	if mod == nil {
-		return nil, errors.New("module must not be nil")
+	if t.Module == nil {
+		return errors.New("module must not be nil")
 	}
-	if !mod.Loaded() {
-		return nil, errors.New("module must be loaded")
+	if !t.Module.Loaded() {
+		return errors.New("module must be loaded")
 	}
 
 	// Get the configuration for this module
-	config := mod.Config()
+	config := t.Module.Config()
 
 	// Create the node list we'll use for the graph
 	nodes := make([]graphNodeConfig, 0,
@@ -39,7 +44,7 @@ func Graph2(mod *module.Tree) (*dag.Graph, error) {
 	}
 
 	// Write all the modules out
-	children := mod.Children()
+	children := t.Module.Children()
 	for _, m := range config.Modules {
 		nodes = append(nodes, &GraphNodeConfigModule{
 			Module: m,
@@ -47,46 +52,33 @@ func Graph2(mod *module.Tree) (*dag.Graph, error) {
 		})
 	}
 
-	// Build the full map of the var names to the nodes.
-	fullMap := make(map[string]dag.Vertex)
-	for _, n := range nodes {
-		fullMap[n.VarName()] = n
-	}
+	// Err is where the final error value will go if there is one
+	var err error
 
 	// Build the graph vertices
-	var g dag.Graph
 	for _, n := range nodes {
 		g.Add(n)
 	}
 
-	// Err is where the final error value will go if there is one
-	var err error
-
-	// Go through all the nodes and build up the actual graph edges. We
-	// do this by getting the variables that each node depends on and then
-	// building the dep map based on the fullMap which contains the mapping
-	// of var names to the actual node with that name.
+	// Build up the dependencies. We have to do this outside of the above
+	// loop since the nodes need to be in place for us to build the deps.
 	for _, n := range nodes {
-		for _, id := range n.Variables() {
-			if id == "" {
-				// Empty name means its a variable we don't care about
-				continue
+		vars := n.Variables()
+		targets := make([]string, 0, len(vars))
+		for _, t := range vars {
+			if t != "" {
+				targets = append(targets, t)
 			}
-
-			target, ok := fullMap[id]
-			if !ok {
-				// We can't find the target meaning the dependency
-				// is missing. Accumulate the error.
+		}
+		if missing := g.ConnectTo(n, targets); len(missing) > 0 {
+			for _, m := range missing {
 				err = multierror.Append(err, fmt.Errorf(
-					"%s: missing dependency: %s", n, id))
-				continue
+					"%s: missing dependency: %s", n.Name(), m))
 			}
-
-			g.Connect(dag.BasicEdge(n, target))
 		}
 	}
 
-	return &g, err
+	return err
 }
 
 // varNameForVar returns the VarName value for an interpolated variable.

terraform/transform_config_test.go

@@ -8,28 +8,32 @@ import (
 	"github.com/hashicorp/terraform/config/module"
 )
 
-func TestGraph_nilModule(t *testing.T) {
-	_, err := Graph2(nil)
-	if err == nil {
+func TestConfigTransformer_nilModule(t *testing.T) {
+	var g Graph
+	tf := &ConfigTransformer{}
+	if err := tf.Transform(&g); err == nil {
 		t.Fatal("should error")
 	}
 }
 
-func TestGraph_unloadedModule(t *testing.T) {
+func TestConfigTransformer_unloadedModule(t *testing.T) {
 	mod, err := module.NewTreeModule(
 		"", filepath.Join(fixtureDir, "graph-basic"))
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
-	if _, err := Graph2(mod); err == nil {
+	var g Graph
+	tf := &ConfigTransformer{Module: mod}
+	if err := tf.Transform(&g); err == nil {
 		t.Fatal("should error")
 	}
 }
 
-func TestGraph(t *testing.T) {
-	g, err := Graph2(testModule(t, "graph-basic"))
-	if err != nil {
+func TestConfigTransformer(t *testing.T) {
+	var g Graph
+	tf := &ConfigTransformer{Module: testModule(t, "graph-basic")}
+	if err := tf.Transform(&g); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -40,9 +44,10 @@ func TestGraph(t *testing.T) {
 	}
 }
 
-func TestGraph2_dependsOn(t *testing.T) {
-	g, err := Graph2(testModule(t, "graph-depends-on"))
-	if err != nil {
+func TestConfigTransformer_dependsOn(t *testing.T) {
+	var g Graph
+	tf := &ConfigTransformer{Module: testModule(t, "graph-depends-on")}
+	if err := tf.Transform(&g); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -53,9 +58,10 @@ func TestGraph2_dependsOn(t *testing.T) {
 	}
 }
 
-func TestGraph2_modules(t *testing.T) {
-	g, err := Graph2(testModule(t, "graph-modules"))
-	if err != nil {
+func TestConfigTransformer_modules(t *testing.T) {
+	var g Graph
+	tf := &ConfigTransformer{Module: testModule(t, "graph-modules")}
+	if err := tf.Transform(&g); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -66,10 +72,11 @@ func TestGraph2_modules(t *testing.T) {
 	}
 }
 
-func TestGraph2_errMissingDeps(t *testing.T) {
-	_, err := Graph2(testModule(t, "graph-missing-deps"))
-	if err == nil {
-		t.Fatal("should error")
+func TestConfigTransformer_errMissingDeps(t *testing.T) {
+	var g Graph
+	tf := &ConfigTransformer{Module: testModule(t, "graph-missing-deps")}
+	if err := tf.Transform(&g); err == nil {
+		t.Fatalf("err: %s", err)
 	}
 }
 

terraform/transform_orphan.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/terraform/config"
-	"github.com/hashicorp/terraform/dag"
 )
 
 // OrphanTransformer is a GraphTransformer that adds orphans to the
@@ -14,7 +13,7 @@ type OrphanTransformer struct {
 	Config *config.Config
 }
 
-func (t *OrphanTransformer) Transform(g *dag.Graph) error {
+func (t *OrphanTransformer) Transform(g *Graph) error {
 	// Get the orphans from our configuration. This will only get resources.
 	orphans := t.State.Orphans(t.Config)
 	if len(orphans) == 0 {
@@ -23,8 +22,9 @@ func (t *OrphanTransformer) Transform(g *dag.Graph) error {
 
 	// Go over each orphan and add it to the graph.
 	for _, k := range orphans {
-		v := g.Add(&graphNodeOrphanResource{ResourceName: k})
-		GraphConnectDeps(g, v, t.State.Resources[k].Dependencies)
+		g.ConnectTo(
+			g.Add(&graphNodeOrphanResource{ResourceName: k}),
+			t.State.Resources[k].Dependencies)
 	}
 
 	// TODO: modules

terraform/transform_orphan_test.go

@@ -1,5 +1,6 @@
 package terraform
 
+/*
 import (
 	"strings"
 	"testing"
@@ -96,3 +97,4 @@ aws_instance.db (orphan)
   aws_instance.web
 aws_instance.web
 `
+*/