Merge pull request #1655 from hashicorp/f-build-graph-during-plan

core: validate on verbose graph to detect some cycles earlier

command/graph.go

@@ -17,11 +17,15 @@ type GraphCommand struct {
 
 func (c *GraphCommand) Run(args []string) int {
 	var moduleDepth int
+	var verbose bool
+	var drawCycles bool
 
 	args = c.Meta.process(args, false)
 
 	cmdFlags := flag.NewFlagSet("graph", flag.ContinueOnError)
 	cmdFlags.IntVar(&moduleDepth, "module-depth", 0, "module-depth")
+	cmdFlags.BoolVar(&verbose, "verbose", false, "verbose")
+	cmdFlags.BoolVar(&drawCycles, "draw-cycles", false, "draw-cycles")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1
@@ -52,25 +56,38 @@ func (c *GraphCommand) Run(args []string) int {
 		return 1
 	}
 
-	g, err := ctx.Graph()
+	// Skip validation during graph generation - we want to see the graph even if
+	// it is invalid for some reason.
+	g, err := ctx.Graph(&terraform.ContextGraphOpts{
+		Verbose:  verbose,
+		Validate: false,
+	})
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error creating graph: %s", err))
 		return 1
 	}
 
-	c.Ui.Output(terraform.GraphDot(g, nil))
+	graphStr, err := terraform.GraphDot(g, &terraform.GraphDotOpts{
+		DrawCycles: drawCycles,
+		MaxDepth:   moduleDepth,
+		Verbose:    verbose,
+	})
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf("Error converting graph: %s", err))
+		return 1
+	}
+
+	c.Ui.Output(graphStr)
 
 	return 0
 }
 
 func (c *GraphCommand) Help() string {
 	helpText := `
-Usage: terraform graph [options] PATH
+Usage: terraform graph [options] [DIR]
 
-  Outputs the visual graph of Terraform resources. If the path given is
+  Outputs the visual dependency graph of Terraform resources according to
-  the path to a configuration, the dependency graph of the resources are
+  configuration files in DIR (or the current directory if omitted).
-  shown. If the path is a plan file, then the dependency graph of the
-  plan itself is shown.
 
   The graph is outputted in DOT format. The typical program that can
   read this format is GraphViz, but many web services are also available
@@ -78,9 +95,14 @@ Usage: terraform graph [options] PATH
 
 Options:
 
+  -draw-cycles         Highlight any cycles in the graph with colored edges.
+                       This helps when diagnosing cycle errors.
+
   -module-depth=n      The maximum depth to expand modules. By default this is
                        zero, which will not expand modules at all.
 
+  -verbose             Generate a verbose, "worst-case" graph, with all nodes
+                       for potential operations in place.
 `
 	return strings.TrimSpace(helpText)
 }

dag/dag.go

@@ -2,6 +2,7 @@ package dag
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 	"sync"
 
@@ -17,17 +18,21 @@ type AcyclicGraph struct {
 // WalkFunc is the callback used for walking the graph.
 type WalkFunc func(Vertex) error
 
+// DepthWalkFunc is a walk function that also receives the current depth of the
+// walk as an argument
+type DepthWalkFunc func(Vertex, int) error
+
 // Returns a Set that includes every Vertex yielded by walking down from the
 // provided starting Vertex v.
 func (g *AcyclicGraph) Ancestors(v Vertex) (*Set, error) {
 	s := new(Set)
-	start := asVertexList(g.DownEdges(v))
+	start := AsVertexList(g.DownEdges(v))
-	memoFunc := func(v Vertex) error {
+	memoFunc := func(v Vertex, d int) error {
 		s.Add(v)
 		return nil
 	}
 
-	if err := g.depthFirstWalk(start, memoFunc); err != nil {
+	if err := g.DepthFirstWalk(start, memoFunc); err != nil {
 		return nil, err
 	}
 
@@ -38,13 +43,13 @@ func (g *AcyclicGraph) Ancestors(v Vertex) (*Set, error) {
 // provided starting Vertex v.
 func (g *AcyclicGraph) Descendents(v Vertex) (*Set, error) {
 	s := new(Set)
-	start := asVertexList(g.UpEdges(v))
+	start := AsVertexList(g.UpEdges(v))
-	memoFunc := func(v Vertex) error {
+	memoFunc := func(v Vertex, d int) error {
 		s.Add(v)
 		return nil
 	}
 
-	if err := g.reverseDepthFirstWalk(start, memoFunc); err != nil {
+	if err := g.ReverseDepthFirstWalk(start, memoFunc); err != nil {
 		return nil, err
 	}
 
@@ -92,14 +97,13 @@ func (g *AcyclicGraph) TransitiveReduction() {
 	// v such that the edge (u,v) exists (v is a direct descendant of u).
 	//
 	// For each v-prime reachable from v, remove the edge (u, v-prime).
-
 	for _, u := range g.Vertices() {
 		uTargets := g.DownEdges(u)
-		vs := asVertexList(g.DownEdges(u))
+		vs := AsVertexList(g.DownEdges(u))
 
-		g.depthFirstWalk(vs, func(v Vertex) error {
+		g.DepthFirstWalk(vs, func(v Vertex, d int) error {
 			shared := uTargets.Intersection(g.DownEdges(v))
-			for _, vPrime := range asVertexList(shared) {
+			for _, vPrime := range AsVertexList(shared) {
 				g.RemoveEdge(BasicEdge(u, vPrime))
 			}
 
@@ -117,12 +121,7 @@ func (g *AcyclicGraph) Validate() error {
 
 	// Look for cycles of more than 1 component
 	var err error
-	var cycles [][]Vertex
+	cycles := g.Cycles()
-	for _, cycle := range StronglyConnected(&g.Graph) {
-		if len(cycle) > 1 {
-			cycles = append(cycles, cycle)
-		}
-	}
 	if len(cycles) > 0 {
 		for _, cycle := range cycles {
 			cycleStr := make([]string, len(cycle))
@@ -146,6 +145,16 @@ func (g *AcyclicGraph) Validate() error {
 	return err
 }
 
+func (g *AcyclicGraph) Cycles() [][]Vertex {
+	var cycles [][]Vertex
+	for _, cycle := range StronglyConnected(&g.Graph) {
+		if len(cycle) > 1 {
+			cycles = append(cycles, cycle)
+		}
+	}
+	return cycles
+}
+
 // Walk walks the graph, calling your callback as each node is visited.
 // This will walk nodes in parallel if it can. Because the walk is done
 // in parallel, the error returned will be a multierror.
@@ -175,7 +184,7 @@ func (g *AcyclicGraph) Walk(cb WalkFunc) error {
 	for _, v := range vertices {
 		// Build our list of dependencies and the list of channels to
 		// wait on until we start executing for this vertex.
-		deps := asVertexList(g.DownEdges(v))
+		deps := AsVertexList(g.DownEdges(v))
 		depChs := make([]<-chan struct{}, len(deps))
 		for i, dep := range deps {
 			depChs[i] = vertMap[dep]
@@ -229,7 +238,7 @@ func (g *AcyclicGraph) Walk(cb WalkFunc) error {
 }
 
 // simple convenience helper for converting a dag.Set to a []Vertex
-func asVertexList(s *Set) []Vertex {
+func AsVertexList(s *Set) []Vertex {
 	rawList := s.List()
 	vertexList := make([]Vertex, len(rawList))
 	for i, raw := range rawList {
@@ -238,13 +247,23 @@ func asVertexList(s *Set) []Vertex {
 	return vertexList
 }
 
+type vertexAtDepth struct {
+	Vertex Vertex
+	Depth  int
+}
+
 // depthFirstWalk does a depth-first walk of the graph starting from
 // the vertices in start. This is not exported now but it would make sense
 // to export this publicly at some point.
-func (g *AcyclicGraph) depthFirstWalk(start []Vertex, cb WalkFunc) error {
+func (g *AcyclicGraph) DepthFirstWalk(start []Vertex, f DepthWalkFunc) error {
 	seen := make(map[Vertex]struct{})
-	frontier := make([]Vertex, len(start))
+	frontier := make([]*vertexAtDepth, len(start))
-	copy(frontier, start)
+	for i, v := range start {
+		frontier[i] = &vertexAtDepth{
+			Vertex: v,
+			Depth:  0,
+		}
+	}
 	for len(frontier) > 0 {
 		// Pop the current vertex
 		n := len(frontier)
@@ -252,20 +271,24 @@ func (g *AcyclicGraph) depthFirstWalk(start []Vertex, cb WalkFunc) error {
 		frontier = frontier[:n-1]
 
 		// Check if we've seen this already and return...
-		if _, ok := seen[current]; ok {
+		if _, ok := seen[current.Vertex]; ok {
 			continue
 		}
-		seen[current] = struct{}{}
+		seen[current.Vertex] = struct{}{}
 
 		// Visit the current node
-		if err := cb(current); err != nil {
+		if err := f(current.Vertex, current.Depth); err != nil {
 			return err
 		}
 
-		// Visit targets of this in reverse order.
+		// Visit targets of this in a consistent order.
-		targets := g.DownEdges(current).List()
+		targets := AsVertexList(g.DownEdges(current.Vertex))
-		for i := len(targets) - 1; i >= 0; i-- {
+		sort.Sort(byVertexName(targets))
-			frontier = append(frontier, targets[i].(Vertex))
+		for _, t := range targets {
+			frontier = append(frontier, &vertexAtDepth{
+				Vertex: t,
+				Depth:  current.Depth + 1,
+			})
 		}
 	}
 
@@ -274,10 +297,15 @@ func (g *AcyclicGraph) depthFirstWalk(start []Vertex, cb WalkFunc) error {
 
 // reverseDepthFirstWalk does a depth-first walk _up_ the graph starting from
 // the vertices in start.
-func (g *AcyclicGraph) reverseDepthFirstWalk(start []Vertex, cb WalkFunc) error {
+func (g *AcyclicGraph) ReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) error {
 	seen := make(map[Vertex]struct{})
-	frontier := make([]Vertex, len(start))
+	frontier := make([]*vertexAtDepth, len(start))
-	copy(frontier, start)
+	for i, v := range start {
+		frontier[i] = &vertexAtDepth{
+			Vertex: v,
+			Depth:  0,
+		}
+	}
 	for len(frontier) > 0 {
 		// Pop the current vertex
 		n := len(frontier)
@@ -285,22 +313,36 @@ func (g *AcyclicGraph) reverseDepthFirstWalk(start []Vertex, cb WalkFunc) error
 		frontier = frontier[:n-1]
 
 		// Check if we've seen this already and return...
-		if _, ok := seen[current]; ok {
+		if _, ok := seen[current.Vertex]; ok {
 			continue
 		}
-		seen[current] = struct{}{}
+		seen[current.Vertex] = struct{}{}
 
 		// Visit the current node
-		if err := cb(current); err != nil {
+		if err := f(current.Vertex, current.Depth); err != nil {
 			return err
 		}
 
-		// Visit targets of this in reverse order.
+		// Visit targets of this in a consistent order.
-		targets := g.UpEdges(current).List()
+		targets := AsVertexList(g.UpEdges(current.Vertex))
-		for i := len(targets) - 1; i >= 0; i-- {
+		sort.Sort(byVertexName(targets))
-			frontier = append(frontier, targets[i].(Vertex))
+		for _, t := range targets {
+			frontier = append(frontier, &vertexAtDepth{
+				Vertex: t,
+				Depth:  current.Depth + 1,
+			})
 		}
 	}
 
 	return nil
 }
+
+// byVertexName implements sort.Interface so a list of Vertices can be sorted
+// consistently by their VertexName
+type byVertexName []Vertex
+
+func (b byVertexName) Len() int      { return len(b) }
+func (b byVertexName) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
+func (b byVertexName) Less(i, j int) bool {
+	return VertexName(b[i]) < VertexName(b[j])
+}

dot/graph.go

@@ -0,0 +1,224 @@
+// The dot package contains utilities for working with DOT graphs.
+package dot
+
+import (
+	"bytes"
+	"fmt"
+	"sort"
+	"strings"
+)
+
+// Graph is a representation of a drawable DOT graph.
+type Graph struct {
+	// Whether this is a "digraph" or just a "graph"
+	Directed bool
+
+	// Used for K/V settings in the DOT
+	Attrs map[string]string
+
+	Nodes     []*Node
+	Edges     []*Edge
+	Subgraphs []*Subgraph
+
+	nodesByName map[string]*Node
+}
+
+// Subgraph is a Graph that lives inside a Parent graph, and contains some
+// additional parameters to control how it is drawn.
+type Subgraph struct {
+	Graph
+	Name    string
+	Parent  *Graph
+	Cluster bool
+}
+
+// An Edge in a DOT graph, as expressed by recording the Name of the Node at
+// each end.
+type Edge struct {
+	// Name of source node.
+	Source string
+
+	// Name of dest node.
+	Dest string
+
+	// List of K/V attributes for this edge.
+	Attrs map[string]string
+}
+
+// A Node in a DOT graph.
+type Node struct {
+	Name  string
+	Attrs map[string]string
+}
+
+// Creates a properly initialized DOT Graph.
+func NewGraph(attrs map[string]string) *Graph {
+	return &Graph{
+		Attrs:       attrs,
+		nodesByName: make(map[string]*Node),
+	}
+}
+
+func NewEdge(src, dst string, attrs map[string]string) *Edge {
+	return &Edge{
+		Source: src,
+		Dest:   dst,
+		Attrs:  attrs,
+	}
+}
+
+func NewNode(n string, attrs map[string]string) *Node {
+	return &Node{
+		Name:  n,
+		Attrs: attrs,
+	}
+}
+
+// Initializes a Subgraph with the provided name, attaches is to this Graph,
+// and returns it.
+func (g *Graph) AddSubgraph(name string) *Subgraph {
+	subgraph := &Subgraph{
+		Graph:  *NewGraph(map[string]string{}),
+		Parent: g,
+		Name:   name,
+	}
+	g.Subgraphs = append(g.Subgraphs, subgraph)
+	return subgraph
+}
+
+func (g *Graph) AddAttr(k, v string) {
+	g.Attrs[k] = v
+}
+
+func (g *Graph) AddNode(n *Node) {
+	g.Nodes = append(g.Nodes, n)
+	g.nodesByName[n.Name] = n
+}
+
+func (g *Graph) AddEdge(e *Edge) {
+	g.Edges = append(g.Edges, e)
+}
+
+// Adds an edge between two Nodes.
+//
+// Note this does not do any verification of the existence of these nodes,
+// which means that any strings you provide that are not existing nodes will
+// result in extra auto-defined nodes in your resulting DOT.
+func (g *Graph) AddEdgeBetween(src, dst string, attrs map[string]string) error {
+	g.AddEdge(NewEdge(src, dst, attrs))
+
+	return nil
+}
+
+// Look up a node by name
+func (g *Graph) GetNode(name string) (*Node, error) {
+	node, ok := g.nodesByName[name]
+	if !ok {
+		return nil, fmt.Errorf("Could not find node: %s", name)
+	}
+	return node, nil
+}
+
+// Returns the DOT representation of this Graph.
+func (g *Graph) String() string {
+	w := newGraphWriter()
+
+	g.drawHeader(w)
+	w.Indent()
+	g.drawBody(w)
+	w.Unindent()
+	g.drawFooter(w)
+
+	return w.String()
+}
+
+func (g *Graph) drawHeader(w *graphWriter) {
+	if g.Directed {
+		w.Printf("digraph {\n")
+	} else {
+		w.Printf("graph {\n")
+	}
+}
+
+func (g *Graph) drawBody(w *graphWriter) {
+	for _, as := range attrStrings(g.Attrs) {
+		w.Printf("%s\n", as)
+	}
+
+	nodeStrings := make([]string, 0, len(g.Nodes))
+	for _, n := range g.Nodes {
+		nodeStrings = append(nodeStrings, n.String())
+	}
+	sort.Strings(nodeStrings)
+	for _, ns := range nodeStrings {
+		w.Printf(ns)
+	}
+
+	edgeStrings := make([]string, 0, len(g.Edges))
+	for _, e := range g.Edges {
+		edgeStrings = append(edgeStrings, e.String())
+	}
+	sort.Strings(edgeStrings)
+	for _, es := range edgeStrings {
+		w.Printf(es)
+	}
+
+	for _, s := range g.Subgraphs {
+		s.drawHeader(w)
+		w.Indent()
+		s.drawBody(w)
+		w.Unindent()
+		s.drawFooter(w)
+	}
+}
+
+func (g *Graph) drawFooter(w *graphWriter) {
+	w.Printf("}\n")
+}
+
+// Returns the DOT representation of this Edge.
+func (e *Edge) String() string {
+	var buf bytes.Buffer
+	buf.WriteString(
+		fmt.Sprintf(
+			"%q -> %q", e.Source, e.Dest))
+	writeAttrs(&buf, e.Attrs)
+	buf.WriteString("\n")
+
+	return buf.String()
+}
+
+func (s *Subgraph) drawHeader(w *graphWriter) {
+	name := s.Name
+	if s.Cluster {
+		name = fmt.Sprintf("cluster_%s", name)
+	}
+	w.Printf("subgraph %q {\n", name)
+}
+
+// Returns the DOT representation of this Node.
+func (n *Node) String() string {
+	var buf bytes.Buffer
+	buf.WriteString(fmt.Sprintf("%q", n.Name))
+	writeAttrs(&buf, n.Attrs)
+	buf.WriteString("\n")
+
+	return buf.String()
+}
+
+func writeAttrs(buf *bytes.Buffer, attrs map[string]string) {
+	if len(attrs) > 0 {
+		buf.WriteString(" [")
+		buf.WriteString(strings.Join(attrStrings(attrs), ", "))
+		buf.WriteString("]")
+	}
+}
+
+func attrStrings(attrs map[string]string) []string {
+	strings := make([]string, 0, len(attrs))
+	for k, v := range attrs {
+		strings = append(strings, fmt.Sprintf("%s = %q", k, v))
+	}
+	sort.Strings(strings)
+	return strings
+}

dot/graph_writer.go

@@ -0,0 +1,47 @@
+package dot
+
+import (
+	"bytes"
+	"fmt"
+)
+
+// graphWriter wraps a bytes.Buffer and tracks indent level levels.
+type graphWriter struct {
+	bytes.Buffer
+	indent    int
+	indentStr string
+}
+
+// Returns an initialized graphWriter at indent level 0.
+func newGraphWriter() *graphWriter {
+	w := &graphWriter{
+		indent: 0,
+	}
+	w.init()
+	return w
+}
+
+// Prints to the buffer at the current indent level.
+func (w *graphWriter) Printf(s string, args ...interface{}) {
+	w.WriteString(w.indentStr + fmt.Sprintf(s, args...))
+}
+
+// Increase the indent level.
+func (w *graphWriter) Indent() {
+	w.indent++
+	w.init()
+}
+
+// Decrease the indent level.
+func (w *graphWriter) Unindent() {
+	w.indent--
+	w.init()
+}
+
+func (w *graphWriter) init() {
+	indentBuf := new(bytes.Buffer)
+	for i := 0; i < w.indent; i++ {
+		indentBuf.WriteString("\t")
+	}
+	w.indentStr = indentBuf.String()
+}

terraform/context.go

@@ -116,14 +116,19 @@ func NewContext(opts *ContextOpts) *Context {
 	}
 }
 
+type ContextGraphOpts struct {
+	Validate bool
+	Verbose  bool
+}
+
 // Graph returns the graph for this config.
-func (c *Context) Graph() (*Graph, error) {
+func (c *Context) Graph(g *ContextGraphOpts) (*Graph, error) {
-	return c.GraphBuilder().Build(RootModulePath)
+	return c.graphBuilder(g).Build(RootModulePath)
 }
 
 // GraphBuilder returns the GraphBuilder that will be used to create
 // the graphs for this context.
-func (c *Context) GraphBuilder() GraphBuilder {
+func (c *Context) graphBuilder(g *ContextGraphOpts) GraphBuilder {
 	// TODO test
 	providers := make([]string, 0, len(c.providers))
 	for k, _ := range c.providers {
@@ -143,6 +148,8 @@ func (c *Context) GraphBuilder() GraphBuilder {
 		State:        c.state,
 		Targets:      c.targets,
 		Destroy:      c.destroy,
+		Validate:     g.Validate,
+		Verbose:      g.Verbose,
 	}
 }
 
@@ -226,8 +233,14 @@ func (c *Context) Input(mode InputMode) error {
 	}
 
 	if mode&InputModeProvider != 0 {
+		// Build the graph
+		graph, err := c.Graph(&ContextGraphOpts{Validate: true})
+		if err != nil {
+			return err
+		}
+
 		// Do the walk
-		if _, err := c.walk(walkInput); err != nil {
+		if _, err := c.walk(graph, walkInput); err != nil {
 			return err
 		}
 	}
@@ -247,8 +260,14 @@ func (c *Context) Apply() (*State, error) {
 	// Copy our own state
 	c.state = c.state.DeepCopy()
 
+	// Build the graph
+	graph, err := c.Graph(&ContextGraphOpts{Validate: true})
+	if err != nil {
+		return nil, err
+	}
+
 	// Do the walk
-	_, err := c.walk(walkApply)
+	_, err = c.walk(graph, walkApply)
 
 	// Clean out any unused things
 	c.state.prune()
@@ -300,8 +319,14 @@ func (c *Context) Plan() (*Plan, error) {
 	c.diff.init()
 	c.diffLock.Unlock()
 
+	// Build the graph
+	graph, err := c.Graph(&ContextGraphOpts{Validate: true})
+	if err != nil {
+		return nil, err
+	}
+
 	// Do the walk
-	if _, err := c.walk(operation); err != nil {
+	if _, err := c.walk(graph, operation); err != nil {
 		return nil, err
 	}
 	p.Diff = c.diff
@@ -322,8 +347,14 @@ func (c *Context) Refresh() (*State, error) {
 	// Copy our own state
 	c.state = c.state.DeepCopy()
 
+	// Build the graph
+	graph, err := c.Graph(&ContextGraphOpts{Validate: true})
+	if err != nil {
+		return nil, err
+	}
+
 	// Do the walk
-	if _, err := c.walk(walkRefresh); err != nil {
+	if _, err := c.walk(graph, walkRefresh); err != nil {
 		return nil, err
 	}
 
@@ -375,8 +406,18 @@ func (c *Context) Validate() ([]string, []error) {
 		}
 	}
 
+	// Build a Verbose version of the graph so we can catch any potential cycles
+	// in the validate stage
+	graph, err := c.Graph(&ContextGraphOpts{
+		Validate: true,
+		Verbose:  true,
+	})
+	if err != nil {
+		return nil, []error{err}
+	}
+
 	// Walk
-	walker, err := c.walk(walkValidate)
+	walker, err := c.walk(graph, walkValidate)
 	if err != nil {
 		return nil, multierror.Append(errs, err).Errors
 	}
@@ -429,13 +470,8 @@ func (c *Context) releaseRun(ch chan<- struct{}) {
 	c.sh.Reset()
 }
 
-func (c *Context) walk(operation walkOperation) (*ContextGraphWalker, error) {
+func (c *Context) walk(
-	// Build the graph
+	graph *Graph, operation walkOperation) (*ContextGraphWalker, error) {
-	graph, err := c.GraphBuilder().Build(RootModulePath)
-	if err != nil {
-		return nil, err
-	}
-
 	// Walk the graph
 	log.Printf("[INFO] Starting graph walk: %s", operation.String())
 	walker := &ContextGraphWalker{Context: c, Operation: operation}

terraform/context_test.go

@@ -2365,6 +2365,25 @@ func TestContext2Validate_countVariableNoDefault(t *testing.T) {
 	}
 }
 
+func TestContext2Validate_cycle(t *testing.T) {
+	p := testProvider("aws")
+	m := testModule(t, "validate-cycle")
+	c := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	w, e := c.Validate()
+	if len(w) > 0 {
+		t.Fatalf("expected no warns, got: %#v", w)
+	}
+	if len(e) != 1 {
+		t.Fatalf("expected 1 err, got: %s", e)
+	}
+}
+
 func TestContext2Validate_moduleBadOutput(t *testing.T) {
 	p := testProvider("aws")
 	m := testModule(t, "validate-bad-module-output")

terraform/graph_builder.go

@@ -16,9 +16,11 @@ type GraphBuilder interface {
 }
 
 // BasicGraphBuilder is a GraphBuilder that builds a graph out of a
-// series of transforms and validates the graph is a valid structure.
+// series of transforms and (optionally) validates the graph is a valid
+// structure.
 type BasicGraphBuilder struct {
-	Steps []GraphTransformer
+	Steps    []GraphTransformer
+	Validate bool
 }
 
 func (b *BasicGraphBuilder) Build(path []string) (*Graph, error) {
@@ -34,9 +36,11 @@ func (b *BasicGraphBuilder) Build(path []string) (*Graph, error) {
 	}
 
 	// Validate the graph structure
-	if err := g.Validate(); err != nil {
+	if b.Validate {
-		log.Printf("[ERROR] Graph validation failed. Graph:\n\n%s", g.String())
+		if err := g.Validate(); err != nil {
-		return nil, err
+			log.Printf("[ERROR] Graph validation failed. Graph:\n\n%s", g.String())
+			return nil, err
+		}
 	}
 
 	return g, nil
@@ -72,12 +76,23 @@ type BuiltinGraphBuilder struct {
 	// Destroy is set to true when we're in a `terraform destroy` or a
 	// `terraform plan -destroy`
 	Destroy bool
+
+	// Determines whether the GraphBuilder should perform graph validation before
+	// returning the Graph. Generally you want this to be done, except when you'd
+	// like to inspect a problematic graph.
+	Validate bool
+
+	// Verbose is set to true when the graph should be built "worst case",
+	// skipping any prune steps. This is used for early cycle detection during
+	// Validate and for manual inspection via `terraform graph -verbose`.
+	Verbose bool
 }
 
 // Build builds the graph according to the steps returned by Steps.
 func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) {
 	basic := &BasicGraphBuilder{
-		Steps: b.Steps(),
+		Steps:    b.Steps(),
+		Validate: b.Validate,
 	}
 
 	return basic.Build(path)
@@ -86,7 +101,7 @@ func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) {
 // Steps returns the ordered list of GraphTransformers that must be executed
 // to build a complete graph.
 func (b *BuiltinGraphBuilder) Steps() []GraphTransformer {
-	return []GraphTransformer{
+	steps := []GraphTransformer{
 		// Create all our resources from the configuration and state
 		&ConfigTransformer{Module: b.Root},
 		&OrphanTransformer{
@@ -126,7 +141,10 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer {
 		// Create the destruction nodes
 		&DestroyTransformer{},
 		&CreateBeforeDestroyTransformer{},
-		&PruneDestroyTransformer{Diff: b.Diff, State: b.State},
+		b.conditional(&conditionalOpts{
+			If:   func() bool { return !b.Verbose },
+			Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State},
+		}),
 
 		// Make sure we create one root
 		&RootTransformer{},
@@ -135,4 +153,25 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer {
 		// more sane if possible (it usually is possible).
 		&TransitiveReductionTransformer{},
 	}
+
+	// Remove nils
+	for i, s := range steps {
+		if s == nil {
+			steps = append(steps[:i], steps[i+1:]...)
+		}
+	}
+
+	return steps
+}
+
+type conditionalOpts struct {
+	If   func() bool
+	Then GraphTransformer
+}
+
+func (b *BuiltinGraphBuilder) conditional(o *conditionalOpts) GraphTransformer {
+	if o.If != nil && o.Then != nil && o.If() {
+		return o.Then
+	}
+	return nil
 }

terraform/graph_builder_test.go

@@ -41,6 +41,7 @@ func TestBasicGraphBuilder_validate(t *testing.T) {
 			&testBasicGraphBuilderTransform{1},
 			&testBasicGraphBuilderTransform{2},
 		},
+		Validate: true,
 	}
 
 	_, err := b.Build(RootModulePath)
@@ -49,6 +50,21 @@ func TestBasicGraphBuilder_validate(t *testing.T) {
 	}
 }
 
+func TestBasicGraphBuilder_validateOff(t *testing.T) {
+	b := &BasicGraphBuilder{
+		Steps: []GraphTransformer{
+			&testBasicGraphBuilderTransform{1},
+			&testBasicGraphBuilderTransform{2},
+		},
+		Validate: false,
+	}
+
+	_, err := b.Build(RootModulePath)
+	if err != nil {
+		t.Fatalf("expected no error, got: %s", err)
+	}
+}
+
 func TestBuiltinGraphBuilder_impl(t *testing.T) {
 	var _ GraphBuilder = new(BuiltinGraphBuilder)
 }
@@ -58,7 +74,8 @@ func TestBuiltinGraphBuilder_impl(t *testing.T) {
 // specific ordering of steps should be added in other tests.
 func TestBuiltinGraphBuilder(t *testing.T) {
 	b := &BuiltinGraphBuilder{
-		Root: testModule(t, "graph-builder-basic"),
+		Root:     testModule(t, "graph-builder-basic"),
+		Validate: true,
 	}
 
 	g, err := b.Build(RootModulePath)
@@ -73,11 +90,31 @@ func TestBuiltinGraphBuilder(t *testing.T) {
 	}
 }
 
+func TestBuiltinGraphBuilder_Verbose(t *testing.T) {
+	b := &BuiltinGraphBuilder{
+		Root:     testModule(t, "graph-builder-basic"),
+		Validate: true,
+		Verbose:  true,
+	}
+
+	g, err := b.Build(RootModulePath)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testBuiltinGraphBuilderVerboseStr)
+	if actual != expected {
+		t.Fatalf("bad: %s", actual)
+	}
+}
+
 // This tests a cycle we got when a CBD resource depends on a non-CBD
 // resource. This cycle shouldn't happen in the general case anymore.
 func TestBuiltinGraphBuilder_cbdDepNonCbd(t *testing.T) {
 	b := &BuiltinGraphBuilder{
-		Root: testModule(t, "graph-builder-cbd-non-cbd"),
+		Root:     testModule(t, "graph-builder-cbd-non-cbd"),
+		Validate: true,
 	}
 
 	_, err := b.Build(RootModulePath)
@@ -86,6 +123,19 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd(t *testing.T) {
 	}
 }
 
+func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) {
+	b := &BuiltinGraphBuilder{
+		Root:     testModule(t, "graph-builder-cbd-non-cbd"),
+		Validate: true,
+		Verbose:  true,
+	}
+
+	_, err := b.Build(RootModulePath)
+	if err == nil {
+		t.Fatalf("expected err, got none")
+	}
+}
+
 /*
 TODO: This exposes a really bad bug we need to fix after we merge
 the f-ast-branch. This bug still exists in master.
@@ -130,6 +180,23 @@ aws_instance.web
 provider.aws
 `
 
+const testBuiltinGraphBuilderVerboseStr = `
+aws_instance.db
+  aws_instance.db (destroy tainted)
+  aws_instance.db (destroy)
+aws_instance.db (destroy tainted)
+  aws_instance.web (destroy tainted)
+aws_instance.db (destroy)
+  aws_instance.web (destroy)
+aws_instance.web
+  aws_instance.db
+aws_instance.web (destroy tainted)
+  provider.aws
+aws_instance.web (destroy)
+  provider.aws
+provider.aws
+`
+
 const testBuiltinGraphBuilderModuleStr = `
 aws_instance.web
   aws_instance.web (destroy)

terraform/graph_config_node.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/dag"
+	"github.com/hashicorp/terraform/dot"
 )
 
 // graphNodeConfig is an interface that all graph nodes for the
@@ -223,14 +224,16 @@ func (n *GraphNodeConfigProvider) ProviderConfig() *config.RawConfig {
 }
 
 // GraphNodeDotter impl.
-func (n *GraphNodeConfigProvider) Dot(name string) string {
+func (n *GraphNodeConfigProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node {
-	return fmt.Sprintf(
+	return dot.NewNode(name, map[string]string{
-		"\"%s\" [\n"+
+		"label": n.Name(),
-			"\tlabel=\"%s\"\n"+
+		"shape": "diamond",
-			"\tshape=diamond\n"+
+	})
-			"];",
+}
-		name,
+
-		n.Name())
+// GraphNodeDotterOrigin impl.
+func (n *GraphNodeConfigProvider) DotOrigin() bool {
+	return true
 }
 
 // GraphNodeConfigResource represents a resource within the config graph.
@@ -322,18 +325,14 @@ func (n *GraphNodeConfigResource) Name() string {
 }
 
 // GraphNodeDotter impl.
-func (n *GraphNodeConfigResource) Dot(name string) string {
+func (n *GraphNodeConfigResource) DotNode(name string, opts *GraphDotOpts) *dot.Node {
-	if n.DestroyMode != DestroyNone {
+	if n.DestroyMode != DestroyNone && !opts.Verbose {
-		return ""
+		return nil
 	}
-
+	return dot.NewNode(name, map[string]string{
-	return fmt.Sprintf(
+		"label": n.Name(),
-		"\"%s\" [\n"+
+		"shape": "box",
-			"\tlabel=\"%s\"\n"+
+	})
-			"\tshape=box\n"+
-			"];",
-		name,
-		n.Name())
 }
 
 // GraphNodeDynamicExpandable impl.
@@ -639,14 +638,11 @@ func (n *graphNodeModuleExpanded) ConfigType() GraphNodeConfigType {
 }
 
 // GraphNodeDotter impl.
-func (n *graphNodeModuleExpanded) Dot(name string) string {
+func (n *graphNodeModuleExpanded) DotNode(name string, opts *GraphDotOpts) *dot.Node {
-	return fmt.Sprintf(
+	return dot.NewNode(name, map[string]string{
-		"\"%s\" [\n"+
+		"label": dag.VertexName(n.Original),
-			"\tlabel=\"%s\"\n"+
+		"shape": "component",
-			"\tshape=component\n"+
+	})
-			"];",
-		name,
-		dag.VertexName(n.Original))
 }
 
 // GraphNodeEvalable impl.

terraform/graph_dot.go

@@ -1,12 +1,10 @@
 package terraform
 
 import (
-	"bufio"
-	"bytes"
 	"fmt"
-	"strings"
 
 	"github.com/hashicorp/terraform/dag"
+	"github.com/hashicorp/terraform/dot"
 )
 
 // GraphNodeDotter can be implemented by a node to cause it to be included
@@ -14,58 +12,174 @@ import (
 // return a representation of this node.
 type GraphNodeDotter interface {
 	// Dot is called to return the dot formatting for the node.
-	// The parameter must be the title of the node.
+	// The first parameter is the title of the node.
-	Dot(string) string
+	// The second parameter includes user-specified options that affect the dot
+	// graph. See GraphDotOpts below for details.
+	DotNode(string, *GraphDotOpts) *dot.Node
+}
+
+type GraphNodeDotOrigin interface {
+	DotOrigin() bool
 }
 
 // GraphDotOpts are the options for generating a dot formatted Graph.
-type GraphDotOpts struct{}
+type GraphDotOpts struct {
+	// Allows some nodes to decide to only show themselves when the user has
+	// requested the "verbose" graph.
+	Verbose bool
+
+	// Highlight Cycles
+	DrawCycles bool
+
+	// How many levels to expand modules as we draw
+	MaxDepth int
+}
 
 // GraphDot returns the dot formatting of a visual representation of
 // the given Terraform graph.
-func GraphDot(g *Graph, opts *GraphDotOpts) string {
+func GraphDot(g *Graph, opts *GraphDotOpts) (string, error) {
-	buf := new(bytes.Buffer)
+	dg := dot.NewGraph(map[string]string{
+		"compound": "true",
+		"newrank":  "true",
+	})
+	dg.Directed = true
 
-	// Start the graph
+	err := graphDotSubgraph(dg, "root", g, opts, 0)
-	buf.WriteString("digraph {\n")
+	if err != nil {
-	buf.WriteString("\tcompound = true;\n")
+		return "", err
-
-	// Go through all the vertices and draw it
-	vertices := g.Vertices()
-	dotVertices := make(map[dag.Vertex]struct{}, len(vertices))
-	for _, v := range vertices {
-		if dn, ok := v.(GraphNodeDotter); !ok {
-			continue
-		} else if dn.Dot("fake") == "" {
-			continue
-		}
-
-		dotVertices[v] = struct{}{}
 	}
 
-	for v, _ := range dotVertices {
+	return dg.String(), nil
-		dn := v.(GraphNodeDotter)
+}
-		scanner := bufio.NewScanner(strings.NewReader(
+
-			dn.Dot(dag.VertexName(v))))
+func graphDotSubgraph(
-		for scanner.Scan() {
+	dg *dot.Graph, modName string, g *Graph, opts *GraphDotOpts, modDepth int) error {
-			buf.WriteString("\t" + scanner.Text() + "\n")
+	// Respect user-specified module depth
+	if opts.MaxDepth >= 0 && modDepth > opts.MaxDepth {
+		return nil
+	}
+
+	// Begin module subgraph
+	var sg *dot.Subgraph
+	if modDepth == 0 {
+		sg = dg.AddSubgraph(modName)
+	} else {
+		sg = dg.AddSubgraph(modName)
+		sg.Cluster = true
+		sg.AddAttr("label", modName)
+	}
+
+	origins, err := graphDotFindOrigins(g)
+	if err != nil {
+		return err
+	}
+
+	drawableVertices := make(map[dag.Vertex]struct{})
+	toDraw := make([]dag.Vertex, 0, len(g.Vertices()))
+	subgraphVertices := make(map[dag.Vertex]*Graph)
+
+	walk := func(v dag.Vertex, depth int) error {
+		// We only care about nodes that yield non-empty Dot strings.
+		if dn, ok := v.(GraphNodeDotter); !ok {
+			return nil
+		} else if dn.DotNode("fake", opts) == nil {
+			return nil
 		}
 
-		// Draw all the edges
+		drawableVertices[v] = struct{}{}
-		for _, t := range g.DownEdges(v).List() {
+		toDraw = append(toDraw, v)
+
+		if sn, ok := v.(GraphNodeSubgraph); ok {
+			subgraphVertices[v] = sn.Subgraph()
+		}
+		return nil
+	}
+
+	if err := g.ReverseDepthFirstWalk(origins, walk); err != nil {
+		return err
+	}
+
+	for _, v := range toDraw {
+		dn := v.(GraphNodeDotter)
+		nodeName := graphDotNodeName(modName, v)
+		sg.AddNode(dn.DotNode(nodeName, opts))
+
+		// Draw all the edges from this vertex to other nodes
+		targets := dag.AsVertexList(g.DownEdges(v))
+		for _, t := range targets {
 			target := t.(dag.Vertex)
-			if _, ok := dotVertices[target]; !ok {
+			// Only want edges where both sides are drawable.
+			if _, ok := drawableVertices[target]; !ok {
 				continue
 			}
 
-			buf.WriteString(fmt.Sprintf(
+			if err := sg.AddEdgeBetween(
-				"\t\"%s\" -> \"%s\";\n",
+				graphDotNodeName(modName, v),
-				dag.VertexName(v),
+				graphDotNodeName(modName, target),
-				dag.VertexName(target)))
+				map[string]string{}); err != nil {
+				return err
+			}
 		}
 	}
 
-	// End the graph
+	// Recurse into any subgraphs
-	buf.WriteString("}\n")
+	for _, v := range toDraw {
-	return buf.String()
+		subgraph, ok := subgraphVertices[v]
+		if !ok {
+			continue
+		}
+
+		err := graphDotSubgraph(dg, dag.VertexName(v), subgraph, opts, modDepth+1)
+		if err != nil {
+			return err
+		}
+	}
+
+	if opts.DrawCycles {
+		colors := []string{"red", "green", "blue"}
+		for ci, cycle := range g.Cycles() {
+			for i, c := range cycle {
+				// Catch the last wrapping edge of the cycle
+				if i+1 >= len(cycle) {
+					i = -1
+				}
+				edgeAttrs := map[string]string{
+					"color":    colors[ci%len(colors)],
+					"penwidth": "2.0",
+				}
+
+				if err := sg.AddEdgeBetween(
+					graphDotNodeName(modName, c),
+					graphDotNodeName(modName, cycle[i+1]),
+					edgeAttrs); err != nil {
+					return err
+				}
+
+			}
+		}
+	}
+
+	return nil
+}
+
+func graphDotNodeName(modName, v dag.Vertex) string {
+	return fmt.Sprintf("[%s] %s", modName, dag.VertexName(v))
+}
+
+func graphDotFindOrigins(g *Graph) ([]dag.Vertex, error) {
+	var origin []dag.Vertex
+
+	for _, v := range g.Vertices() {
+		if dr, ok := v.(GraphNodeDotOrigin); ok {
+			if dr.DotOrigin() {
+				origin = append(origin, v)
+			}
+		}
+	}
+
+	if len(origin) == 0 {
+		return nil, fmt.Errorf("No DOT origin nodes found.\nGraph: %s", g)
+	}
+
+	return origin, nil
 }

terraform/graph_dot_test.go

@@ -0,0 +1,287 @@
+package terraform
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/dot"
+)
+
+func TestGraphDot(t *testing.T) {
+	cases := map[string]struct {
+		Graph  testGraphFunc
+		Opts   GraphDotOpts
+		Expect string
+		Error  string
+	}{
+		"empty": {
+			Graph: func() *Graph { return &Graph{} },
+			Error: "No DOT origin nodes found",
+		},
+		"three-level": {
+			Graph: func() *Graph {
+				var g Graph
+				root := &testDrawableOrigin{"root"}
+				g.Add(root)
+
+				levelOne := []string{"foo", "bar"}
+				for _, s := range levelOne {
+					g.Add(&testDrawable{
+						VertexName:      s,
+						DependentOnMock: []string{"root"},
+					})
+				}
+
+				levelTwo := []string{"baz", "qux"}
+				for i, s := range levelTwo {
+					g.Add(&testDrawable{
+						VertexName:      s,
+						DependentOnMock: levelOne[i : i+1],
+					})
+				}
+
+				g.ConnectDependents()
+				return &g
+			},
+			Expect: `
+digraph {
+	compound = "true"
+	newrank = "true"
+	subgraph "root" {
+		"[root] bar"
+		"[root] baz"
+		"[root] foo"
+		"[root] qux"
+		"[root] root"
+		"[root] bar" -> "[root] root"
+		"[root] baz" -> "[root] foo"
+		"[root] foo" -> "[root] root"
+		"[root] qux" -> "[root] bar"
+	}
+}
+			`,
+		},
+		"cycle": {
+			Opts: GraphDotOpts{
+				DrawCycles: true,
+			},
+			Graph: func() *Graph {
+				var g Graph
+				root := &testDrawableOrigin{"root"}
+				g.Add(root)
+
+				g.Add(&testDrawable{
+					VertexName:      "A",
+					DependentOnMock: []string{"root", "C"},
+				})
+
+				g.Add(&testDrawable{
+					VertexName:      "B",
+					DependentOnMock: []string{"A"},
+				})
+
+				g.Add(&testDrawable{
+					VertexName:      "C",
+					DependentOnMock: []string{"B"},
+				})
+
+				g.ConnectDependents()
+				return &g
+			},
+			Expect: `
+digraph {
+	compound = "true"
+	newrank = "true"
+	subgraph "root" {
+		"[root] A"
+		"[root] B"
+		"[root] C"
+		"[root] root"
+		"[root] A" -> "[root] B" [color = "red", penwidth = "2.0"]
+		"[root] A" -> "[root] C"
+		"[root] A" -> "[root] root"
+		"[root] B" -> "[root] A"
+		"[root] B" -> "[root] C" [color = "red", penwidth = "2.0"]
+		"[root] C" -> "[root] A" [color = "red", penwidth = "2.0"]
+		"[root] C" -> "[root] B"
+	}
+}
+			`,
+		},
+		"subgraphs, no depth restriction": {
+			Opts: GraphDotOpts{
+				MaxDepth: -1,
+			},
+			Graph: func() *Graph {
+				var g Graph
+				root := &testDrawableOrigin{"root"}
+				g.Add(root)
+
+				var sub Graph
+				sub.Add(&testDrawableOrigin{"sub_root"})
+
+				var subsub Graph
+				subsub.Add(&testDrawableOrigin{"subsub_root"})
+				sub.Add(&testDrawableSubgraph{
+					VertexName:      "subsub",
+					SubgraphMock:    &subsub,
+					DependentOnMock: []string{"sub_root"},
+				})
+				g.Add(&testDrawableSubgraph{
+					VertexName:      "sub",
+					SubgraphMock:    &sub,
+					DependentOnMock: []string{"root"},
+				})
+
+				g.ConnectDependents()
+				sub.ConnectDependents()
+				return &g
+			},
+			Expect: `
+digraph {
+	compound = "true"
+	newrank = "true"
+	subgraph "root" {
+		"[root] root"
+		"[root] sub"
+		"[root] sub" -> "[root] root"
+	}
+	subgraph "cluster_sub" {
+		label = "sub"
+		"[sub] sub_root"
+		"[sub] subsub"
+		"[sub] subsub" -> "[sub] sub_root"
+	}
+	subgraph "cluster_subsub" {
+		label = "subsub"
+		"[subsub] subsub_root"
+	}
+}
+			`,
+		},
+		"subgraphs, with depth restriction": {
+			Opts: GraphDotOpts{
+				MaxDepth: 1,
+			},
+			Graph: func() *Graph {
+				var g Graph
+				root := &testDrawableOrigin{"root"}
+				g.Add(root)
+
+				var sub Graph
+				sub.Add(&testDrawableOrigin{"sub_root"})
+
+				var subsub Graph
+				subsub.Add(&testDrawableOrigin{"subsub_root"})
+				sub.Add(&testDrawableSubgraph{
+					VertexName:      "subsub",
+					SubgraphMock:    &subsub,
+					DependentOnMock: []string{"sub_root"},
+				})
+				g.Add(&testDrawableSubgraph{
+					VertexName:      "sub",
+					SubgraphMock:    &sub,
+					DependentOnMock: []string{"root"},
+				})
+
+				g.ConnectDependents()
+				sub.ConnectDependents()
+				return &g
+			},
+			Expect: `
+digraph {
+	compound = "true"
+	newrank = "true"
+	subgraph "root" {
+		"[root] root"
+		"[root] sub"
+		"[root] sub" -> "[root] root"
+	}
+	subgraph "cluster_sub" {
+		label = "sub"
+		"[sub] sub_root"
+		"[sub] subsub"
+		"[sub] subsub" -> "[sub] sub_root"
+	}
+}
+			`,
+		},
+	}
+
+	for tn, tc := range cases {
+		actual, err := GraphDot(tc.Graph(), &tc.Opts)
+		if (err == nil) && tc.Error != "" {
+			t.Fatalf("%s: expected err: %s, got none", tn, tc.Error)
+		}
+		if (err != nil) && (tc.Error == "") {
+			t.Fatalf("%s: unexpected err: %s", tn, err)
+		}
+		if (err != nil) && (tc.Error != "") {
+			if !strings.Contains(err.Error(), tc.Error) {
+				t.Fatalf("%s: expected err: %s\nto contain: %s", tn, err, tc.Error)
+			}
+			continue
+		}
+
+		expected := strings.TrimSpace(tc.Expect) + "\n"
+		if actual != expected {
+			t.Fatalf("%s:\n\nexpected:\n%s\n\ngot:\n%s", tn, expected, actual)
+		}
+	}
+}
+
+type testGraphFunc func() *Graph
+
+type testDrawable struct {
+	VertexName      string
+	DependentOnMock []string
+}
+
+func (node *testDrawable) Name() string {
+	return node.VertexName
+}
+func (node *testDrawable) DotNode(n string, opts *GraphDotOpts) *dot.Node {
+	return dot.NewNode(n, map[string]string{})
+}
+func (node *testDrawable) DependableName() []string {
+	return []string{node.VertexName}
+}
+func (node *testDrawable) DependentOn() []string {
+	return node.DependentOnMock
+}
+
+type testDrawableOrigin struct {
+	VertexName string
+}
+
+func (node *testDrawableOrigin) Name() string {
+	return node.VertexName
+}
+func (node *testDrawableOrigin) DotNode(n string, opts *GraphDotOpts) *dot.Node {
+	return dot.NewNode(n, map[string]string{})
+}
+func (node *testDrawableOrigin) DotOrigin() bool {
+	return true
+}
+func (node *testDrawableOrigin) DependableName() []string {
+	return []string{node.VertexName}
+}
+
+type testDrawableSubgraph struct {
+	VertexName      string
+	SubgraphMock    *Graph
+	DependentOnMock []string
+}
+
+func (node *testDrawableSubgraph) Name() string {
+	return node.VertexName
+}
+func (node *testDrawableSubgraph) Subgraph() *Graph {
+	return node.SubgraphMock
+}
+func (node *testDrawableSubgraph) DotNode(n string, opts *GraphDotOpts) *dot.Node {
+	return dot.NewNode(n, map[string]string{})
+}
+func (node *testDrawableSubgraph) DependentOn() []string {
+	return node.DependentOnMock
+}

terraform/test-fixtures/validate-cycle/main.tf

@@ -0,0 +1,19 @@
+provider "aws" { }
+
+/*
+ * When a CBD resource depends on a non-CBD resource,
+ * a cycle is formed that only shows up when Destroy
+ * nodes are included in the graph.
+ */
+resource "aws_security_group" "firewall" {
+}
+
+resource "aws_instance" "web" {
+    security_groups = [
+        "foo",
+        "${aws_security_group.firewall.foo}"
+    ]
+    lifecycle {
+      create_before_destroy = true
+    }
+}

terraform/transform_provider.go

@@ -6,6 +6,7 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/dag"
+	"github.com/hashicorp/terraform/dot"
 )
 
 // GraphNodeProvider is an interface that nodes that can be a provider
@@ -176,6 +177,19 @@ func (n *graphNodeDisabledProvider) Name() string {
 	return fmt.Sprintf("%s (disabled)", dag.VertexName(n.GraphNodeProvider))
 }
 
+// GraphNodeDotter impl.
+func (n *graphNodeDisabledProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node {
+	return dot.NewNode(name, map[string]string{
+		"label": n.Name(),
+		"shape": "diamond",
+	})
+}
+
+// GraphNodeDotterOrigin impl.
+func (n *graphNodeDisabledProvider) DotOrigin() bool {
+	return true
+}
+
 type graphNodeMissingProvider struct {
 	ProviderNameValue string
 }
@@ -198,14 +212,16 @@ func (n *graphNodeMissingProvider) ProviderConfig() *config.RawConfig {
 }
 
 // GraphNodeDotter impl.
-func (n *graphNodeMissingProvider) Dot(name string) string {
+func (n *graphNodeMissingProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node {
-	return fmt.Sprintf(
+	return dot.NewNode(name, map[string]string{
-		"\"%s\" [\n"+
+		"label": n.Name(),
-			"\tlabel=\"%s\"\n"+
+		"shape": "diamond",
-			"\tshape=diamond\n"+
+	})
-			"];",
+}
-		name,
+
-		n.Name())
+// GraphNodeDotterOrigin impl.
+func (n *graphNodeMissingProvider) DotOrigin() bool {
+	return true
 }
 
 func providerVertexMap(g *Graph) map[string]dag.Vertex {

terraform/transform_root.go

@@ -1,10 +1,6 @@
 package terraform
 
-import (
+import "github.com/hashicorp/terraform/dag"
-	"fmt"
-
-	"github.com/hashicorp/terraform/dag"
-)
 
 // RootTransformer is a GraphTransformer that adds a root to the graph.
 type RootTransformer struct{}
@@ -38,7 +34,3 @@ type graphNodeRoot struct{}
 func (n graphNodeRoot) Name() string {
 	return "root"
 }
-
-func (n graphNodeRoot) Dot(name string) string {
-	return fmt.Sprintf("\"%s\" [shape=circle];", name)
-}

website/source/docs/commands/graph.html.markdown

@@ -16,18 +16,26 @@ The output is in the DOT format, which can be used by
 
 ## Usage
 
-Usage: `terraform graph [options] PATH`
+Usage: `terraform graph [options] [DIR]`
 
-Outputs the visual graph of Terraform resources. If the path given is
+Outputs the visual dependency graph of Terraform resources according to
-the path to a configuration, the dependency graph of the resources are
+configuration files in DIR (or the current directory if omitted).
-shown. If the path is a plan file, then the dependency graph of the
+
-plan itself is shown.
+The graph is outputted in DOT format. The typical program that can
+read this format is GraphViz, but many web services are also available
+to read this format.
 
 Options:
 
+* `-draw-cycles`    - Highlight any cycles in the graph with colored edges.
+                      This helps when diagnosing cycle errors.
+
 * `-module-depth=n` - The maximum depth to expand modules. By default this is
                       zero, which will not expand modules at all.
 
+* `-verbose`        - Generate a verbose, "worst-case" graph, with all nodes
+                      for potential operations in place.
+
 ## Generating Images
 
 The output of `terraform graph` is in the DOT format, which can