diff --git a/terraform/graph.go b/terraform/graph.go new file mode 100644 index 000000000..7b4a793c4 --- /dev/null +++ b/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) + } +} diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 099345463..1e5232214 100644 --- a/terraform/graph_config_node.go +++ b/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() -} diff --git a/terraform/graph_test.go b/terraform/graph_test.go new file mode 100644 index 000000000..0c96d2214 --- /dev/null +++ b/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 +` diff --git a/terraform/transform.go b/terraform/transform.go index 5751d03fa..e1f7828c6 100644 --- a/terraform/transform.go +++ b/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 } diff --git a/terraform/graph_config.go b/terraform/transform_config.go similarity index 57% rename from terraform/graph_config.go rename to terraform/transform_config.go index 716241ca7..5d87c7047 100644 --- a/terraform/graph_config.go +++ b/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. diff --git a/terraform/graph_config_test.go b/terraform/transform_config_test.go similarity index 57% rename from terraform/graph_config_test.go rename to terraform/transform_config_test.go index 3bd22dc69..fb4bf1c8d 100644 --- a/terraform/graph_config_test.go +++ b/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) } } diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index 2787960dd..296dab61b 100644 --- a/terraform/transform_orphan.go +++ b/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 diff --git a/terraform/transform_orphan_test.go b/terraform/transform_orphan_test.go index 76649e208..a8f894e05 100644 --- a/terraform/transform_orphan_test.go +++ b/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 ` +*/