terraform: Graph, config: don't build graph

config/config.go

@@ -5,8 +5,6 @@ package config
 import (
 	"fmt"
 	"strings"
-
-	"github.com/hashicorp/terraform/depgraph"
 )
 
 // Config is the configuration that comes from loading a collection
@@ -89,107 +87,6 @@ func (r *Resource) Id() string {
 	return fmt.Sprintf("%s.%s", r.Type, r.Name)
 }
 
-// Graph returns a dependency graph of the resources from this
-// Terraform configuration.
-//
-// The graph can contain both *Resource and *ProviderConfig. When consuming
-// the graph, you'll have to use type inference to determine what it is
-// and the proper behavior.
-func (c *Config) Graph() *depgraph.Graph {
-	// This tracks all the resource nouns
-	nouns := make(map[string]*depgraph.Noun)
-	for _, r := range c.Resources {
-		noun := &depgraph.Noun{
-			Name: r.Id(),
-			Meta: r,
-		}
-		nouns[noun.Name] = noun
-	}
-
-	// Build the list of nouns that we iterate over
-	nounsList := make([]*depgraph.Noun, 0, len(nouns))
-	for _, n := range nouns {
-		nounsList = append(nounsList, n)
-	}
-
-	// This tracks the provider configs that are nouns in our dep graph
-	pcNouns := make(map[string]*depgraph.Noun)
-
-	i := 0
-	for i < len(nounsList) {
-		noun := nounsList[i]
-		i += 1
-
-		// Determine depenencies based on variables. Both resources
-		// and provider configurations have dependencies in this case.
-		var vars map[string]InterpolatedVariable
-		switch n := noun.Meta.(type) {
-		case *Resource:
-			vars = n.RawConfig.Variables
-		case *ProviderConfig:
-			vars = n.RawConfig.Variables
-		}
-		for _, v := range vars {
-			// Only resource variables impose dependencies
-			rv, ok := v.(*ResourceVariable)
-			if !ok {
-				continue
-			}
-
-			// Build the dependency
-			dep := &depgraph.Dependency{
-				Name:   rv.ResourceId(),
-				Source: noun,
-				Target: nouns[rv.ResourceId()],
-			}
-
-			noun.Deps = append(noun.Deps, dep)
-		}
-
-		// If this is a Resource, then check if we have to also
-		// depend on a provider configuration.
-		if r, ok := noun.Meta.(*Resource); ok {
-			// If there is a provider config that matches this resource
-			// then we add that as a dependency.
-			if pcName := ProviderConfigName(r.Type, c.ProviderConfigs); pcName != "" {
-				pcNoun, ok := pcNouns[pcName]
-				if !ok {
-					pcNoun = &depgraph.Noun{
-						Name: fmt.Sprintf("provider.%s", pcName),
-						Meta: c.ProviderConfigs[pcName],
-					}
-					pcNouns[pcName] = pcNoun
-					nounsList = append(nounsList, pcNoun)
-				}
-
-				dep := &depgraph.Dependency{
-					Name:   pcName,
-					Source: noun,
-					Target: pcNoun,
-				}
-
-				noun.Deps = append(noun.Deps, dep)
-			}
-		}
-	}
-
-	// Create a root that just depends on everything else finishing.
-	root := &depgraph.Noun{Name: "root"}
-	for _, n := range nounsList {
-		root.Deps = append(root.Deps, &depgraph.Dependency{
-			Name:   n.Name,
-			Source: root,
-			Target: n,
-		})
-	}
-	nounsList = append(nounsList, root)
-
-	return &depgraph.Graph{
-		Name:  "resources",
-		Nouns: nounsList,
-	}
-}
-
 // Validate does some basic semantic checking of the configuration.
 func (c *Config) Validate() error {
 	// TODO(mitchellh): make sure all referenced variables exist

config/config_test.go

@@ -1,45 +1,12 @@
 package config
 
 import (
-	"path/filepath"
-	"strings"
 	"testing"
 )
 
 // This is the directory where our test fixtures are.
 const fixtureDir = "./test-fixtures"
 
-func TestConfigGraph(t *testing.T) {
-	c, err := Load(filepath.Join(fixtureDir, "resource_graph.tf"))
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	graph := c.Graph()
-	if err := graph.Validate(); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	actual := strings.TrimSpace(graph.String())
-	expected := resourceGraphValue
-
-	if actual != strings.TrimSpace(expected) {
-		t.Fatalf("bad:\n%s", actual)
-	}
-}
-
-func TestConfigGraph_cycle(t *testing.T) {
-	c, err := Load(filepath.Join(fixtureDir, "resource_graph_cycle.tf"))
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	graph := c.Graph()
-	if err := graph.Validate(); err == nil {
-		t.Fatal("graph should be invalid")
-	}
-}
-
 func TestNewResourceVariable(t *testing.T) {
 	v, err := NewResourceVariable("foo.bar.baz")
 	if err != nil {

depgraph/graph.go

@@ -8,6 +8,7 @@ package depgraph
 import (
 	"bytes"
 	"fmt"
+	"sort"
 
 	"github.com/hashicorp/terraform/digraph"
 )
@@ -113,10 +114,31 @@ func (g *Graph) CheckConstraints() error {
 func (g *Graph) String() string {
 	var buf bytes.Buffer
 
-	buf.WriteString(fmt.Sprintf("root: %s\n", g.Root.Name))
+	// Alphabetize the output based on the noun name
+	keys := make([]string, 0, len(g.Nouns))
+	mapping := make(map[string]*Noun)
 	for _, n := range g.Nouns {
+		mapping[n.Name] = n
+		keys = append(keys, n.Name)
+	}
+	sort.Strings(keys)
+
+	buf.WriteString(fmt.Sprintf("root: %s\n", g.Root.Name))
+	for _, k := range keys {
+		n := mapping[k]
 		buf.WriteString(fmt.Sprintf("%s\n", n.Name))
-		for _, dep := range n.Deps {
+
+		// Alphabetize the dependency names
+		depKeys := make([]string, 0, len(n.Deps))
+		depMapping := make(map[string]*Dependency)
+		for _, d := range n.Deps {
+			depMapping[d.Target.Name] = d
+			depKeys = append(depKeys, d.Target.Name)
+		}
+		sort.Strings(depKeys)
+
+		for _, k := range depKeys {
+			dep := depMapping[k]
 			buf.WriteString(fmt.Sprintf(
 				"  %s -> %s\n",
 				dep.Source,

terraform/graph.go

@@ -0,0 +1,175 @@
+package terraform
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/terraform/config"
+	"github.com/hashicorp/terraform/depgraph"
+)
+
+// GraphRootNode is the name of the root node in the Terraform resource
+// graph. This node is just a placemarker and has no associated functionality.
+const GraphRootNode = "root"
+
+// Graph builds a dependency graph for the given configuration and state.
+//
+// This dependency graph shows the correct order that any resources need
+// to be operated on.
+func Graph(c *config.Config, s *State) *depgraph.Graph {
+	g := new(depgraph.Graph)
+
+	// First, build the initial resource graph. This only has the resources
+	// and no dependencies.
+	graphAddConfigResources(g, c)
+
+	// Next, add the state orphans if we have any
+	if s != nil {
+		graphAddOrphans(g, c, s)
+	}
+
+	// Map the provider configurations to all of the resources
+	graphAddProviderConfigs(g, c)
+
+	// Add all the variable dependencies
+	graphAddVariableDeps(g)
+
+	// Build the root so that we have a single valid root
+	graphAddRoot(g)
+
+	return g
+}
+
+// configGraph turns a configuration structure into a dependency graph.
+func graphAddConfigResources(g *depgraph.Graph, c *config.Config) {
+	// This tracks all the resource nouns
+	nouns := make(map[string]*depgraph.Noun)
+	for _, r := range c.Resources {
+		noun := &depgraph.Noun{
+			Name: r.Id(),
+			Meta: r,
+		}
+		nouns[noun.Name] = noun
+	}
+
+	// Build the list of nouns that we iterate over
+	nounsList := make([]*depgraph.Noun, 0, len(nouns))
+	for _, n := range nouns {
+		nounsList = append(nounsList, n)
+	}
+
+	g.Name = "terraform"
+	g.Nouns = append(g.Nouns, nounsList...)
+}
+
+// graphAddOrphans adds the orphans to the graph.
+func graphAddOrphans(g *depgraph.Graph, c *config.Config, s *State) {
+	for _, k := range s.Orphans(c) {
+		rs := s.Resources[k]
+		noun := &depgraph.Noun{
+			Name: k,
+			Meta: rs,
+		}
+		g.Nouns = append(g.Nouns, noun)
+	}
+}
+
+// graphAddProviderConfigs cycles through all the resource-like nodes
+// and adds the provider configuration nouns into the tree.
+func graphAddProviderConfigs(g *depgraph.Graph, c *config.Config) {
+	nounsList := make([]*depgraph.Noun, 0, 2)
+	pcNouns := make(map[string]*depgraph.Noun)
+	for _, noun := range g.Nouns {
+		var rtype string
+		switch m := noun.Meta.(type) {
+		case *config.Resource:
+			rtype = m.Type
+		case *ResourceState:
+			rtype = m.Type
+		default:
+			continue
+		}
+
+		// Look up the provider config for this resource
+		pcName := config.ProviderConfigName(rtype, c.ProviderConfigs)
+		if pcName == "" {
+			continue
+		}
+
+		// We have one, so build the noun if it hasn't already been made
+		pcNoun, ok := pcNouns[pcName]
+		if !ok {
+			pcNoun = &depgraph.Noun{
+				Name: fmt.Sprintf("provider.%s", pcName),
+				Meta: c.ProviderConfigs[pcName],
+			}
+			pcNouns[pcName] = pcNoun
+			nounsList = append(nounsList, pcNoun)
+		}
+
+		dep := &depgraph.Dependency{
+			Name:   pcName,
+			Source: noun,
+			Target: pcNoun,
+		}
+		noun.Deps = append(noun.Deps, dep)
+	}
+
+	// Add all the provider config nouns to the graph
+	g.Nouns = append(g.Nouns, nounsList...)
+}
+
+// graphAddRoot adds a root element to the graph so that there is a single
+// root to point to all the dependencies.
+func graphAddRoot(g *depgraph.Graph) {
+	root := &depgraph.Noun{Name: GraphRootNode}
+	for _, n := range g.Nouns {
+		root.Deps = append(root.Deps, &depgraph.Dependency{
+			Name:   n.Name,
+			Source: root,
+			Target: n,
+		})
+	}
+	g.Nouns = append(g.Nouns, root)
+}
+
+// graphAddVariableDeps inspects all the nouns and adds any dependencies
+// based on variable values.
+func graphAddVariableDeps(g *depgraph.Graph) {
+	for _, n := range g.Nouns {
+		var vars map[string]config.InterpolatedVariable
+		switch m := n.Meta.(type) {
+		case *config.Resource:
+			vars = m.RawConfig.Variables
+		case *config.ProviderConfig:
+			vars = m.RawConfig.Variables
+		default:
+			continue
+		}
+
+		for _, v := range vars {
+			// Only resource variables impose dependencies
+			rv, ok := v.(*config.ResourceVariable)
+			if !ok {
+				continue
+			}
+
+			// Find the target
+			var target *depgraph.Noun
+			for _, n := range g.Nouns {
+				if n.Name == rv.ResourceId() {
+					target = n
+					break
+				}
+			}
+
+			// Build the dependency
+			dep := &depgraph.Dependency{
+				Name:   rv.ResourceId(),
+				Source: n,
+				Target: target,
+			}
+
+			n.Deps = append(n.Deps, dep)
+		}
+	}
+}

terraform/graph_test.go

@@ -0,0 +1,98 @@
+package terraform
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestTerraformGraph(t *testing.T) {
+	config := testConfig(t, "graph-basic")
+
+	g := Graph(config, nil)
+	if err := g.Validate(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTerraformGraphStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
+func TestTerraformGraph_cycle(t *testing.T) {
+	config := testConfig(t, "graph-cycle")
+
+	g := Graph(config, nil)
+	if err := g.Validate(); err == nil {
+		t.Fatal("should error")
+	}
+}
+
+func TestTerraformGraph_state(t *testing.T) {
+	config := testConfig(t, "graph-basic")
+	state := &State{
+		Resources: map[string]*ResourceState{
+			"aws_instance.old": &ResourceState{
+				ID:   "foo",
+				Type: "aws_instance",
+			},
+		},
+	}
+
+	g := Graph(config, state)
+	if err := g.Validate(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTerraformGraphStateStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
+const testTerraformGraphStr = `
+root: root
+aws_instance.web
+  aws_instance.web -> aws_security_group.firewall
+  aws_instance.web -> provider.aws
+aws_load_balancer.weblb
+  aws_load_balancer.weblb -> aws_instance.web
+  aws_load_balancer.weblb -> provider.aws
+aws_security_group.firewall
+  aws_security_group.firewall -> provider.aws
+openstack_floating_ip.random
+provider.aws
+  provider.aws -> openstack_floating_ip.random
+root
+  root -> aws_instance.web
+  root -> aws_load_balancer.weblb
+  root -> aws_security_group.firewall
+  root -> openstack_floating_ip.random
+  root -> provider.aws
+`
+
+const testTerraformGraphStateStr = `
+root: root
+aws_instance.old
+  aws_instance.old -> provider.aws
+aws_instance.web
+  aws_instance.web -> aws_security_group.firewall
+  aws_instance.web -> provider.aws
+aws_load_balancer.weblb
+  aws_load_balancer.weblb -> aws_instance.web
+  aws_load_balancer.weblb -> provider.aws
+aws_security_group.firewall
+  aws_security_group.firewall -> provider.aws
+openstack_floating_ip.random
+provider.aws
+  provider.aws -> openstack_floating_ip.random
+root
+  root -> aws_instance.old
+  root -> aws_instance.web
+  root -> aws_load_balancer.weblb
+  root -> aws_security_group.firewall
+  root -> openstack_floating_ip.random
+  root -> provider.aws
+`

terraform/semantics.go

@@ -81,7 +81,7 @@ ResourceLoop:
 
 		// Find the matching provider configuration for this resource
 		var pc *config.ProviderConfig
-		pcName := r.ProviderConfigName(c.Config.ProviderConfigs)
+		pcName := config.ProviderConfigName(r.Type, c.Config.ProviderConfigs)
 		if pcName != "" {
 			pc = c.Config.ProviderConfigs[pcName]
 		}

terraform/state.go

@@ -27,6 +27,27 @@ func (s *State) init() {
 	})
 }
 
+// Orphans returns a list of keys of resources that are in the State
+// but aren't present in the configuration itself. Hence, these keys
+// represent the state of resources that are orphans.
+func (s *State) Orphans(c *config.Config) []string {
+	keys := make(map[string]struct{})
+	for k, _ := range s.Resources {
+		keys[k] = struct{}{}
+	}
+
+	for _, r := range c.Resources {
+		delete(keys, r.Id())
+	}
+
+	result := make([]string, 0, len(keys))
+	for k, _ := range keys {
+		result = append(result, k)
+	}
+
+	return result
+}
+
 func (s *State) String() string {
 	var buf bytes.Buffer
 

terraform/terraform.go

@@ -46,42 +46,45 @@ type Config struct {
 // can be properly initialized, can be configured, etc.
 func New(c *Config) (*Terraform, error) {
 	var errs []error
+	var mapping map[*config.Resource]*terraformProvider
 
-	// Validate that all required variables have values
-	if err := smcVariables(c); err != nil {
-		errs = append(errs, err...)
-	}
-
-	// Match all the resources with a provider and initialize the providers
-	mapping, err := smcProviders(c)
-	if err != nil {
-		errs = append(errs, err...)
-	}
-
-	// Validate all the configurations, once.
-	tps := make(map[*terraformProvider]struct{})
-	for _, tp := range mapping {
-		if _, ok := tps[tp]; !ok {
-			tps[tp] = struct{}{}
-		}
-	}
-	for tp, _ := range tps {
-		var rc *ResourceConfig
-		if tp.Config != nil {
-			rc = NewResourceConfig(tp.Config.RawConfig)
+	if c.Config != nil {
+		// Validate that all required variables have values
+		if err := smcVariables(c); err != nil {
+			errs = append(errs, err...)
 		}
 
-		_, tpErrs := tp.Provider.Validate(rc)
-		if len(tpErrs) > 0 {
-			errs = append(errs, tpErrs...)
+		// Match all the resources with a provider and initialize the providers
+		mapping, err := smcProviders(c)
+		if err != nil {
+			errs = append(errs, err...)
 		}
-	}
 
-	// Build the resource graph
-	graph := c.Config.Graph()
-	if err := graph.Validate(); err != nil {
-		errs = append(errs, fmt.Errorf(
-			"Resource graph has an error: %s", err))
+		// Validate all the configurations, once.
+		tps := make(map[*terraformProvider]struct{})
+		for _, tp := range mapping {
+			if _, ok := tps[tp]; !ok {
+				tps[tp] = struct{}{}
+			}
+		}
+		for tp, _ := range tps {
+			var rc *ResourceConfig
+			if tp.Config != nil {
+				rc = NewResourceConfig(tp.Config.RawConfig)
+			}
+
+			_, tpErrs := tp.Provider.Validate(rc)
+			if len(tpErrs) > 0 {
+				errs = append(errs, tpErrs...)
+			}
+		}
+
+		// Build the resource graph
+		graph := c.Config.Graph()
+		if err := graph.Validate(); err != nil {
+			errs = append(errs, fmt.Errorf(
+				"Resource graph has an error: %s", err))
+		}
 	}
 
 	// If we accumulated any errors, then return them all
@@ -107,6 +110,20 @@ func (t *Terraform) Apply(p *Plan) (*State, error) {
 	return result, err
 }
 
+// Graph returns the dependency graph for the given configuration and
+// state file.
+//
+// The resulting graph may have more resources than the configuration, because
+// it can contain resources in the state file that need to be modified.
+func (t *Terraform) Graph(c *config.Config, s *State) (*depgraph.Graph, error) {
+	g := Graph(c, s)
+	if err := g.Validate(); err != nil {
+		return nil, err
+	}
+
+	return g, nil
+}
+
 func (t *Terraform) Plan(s *State) (*Plan, error) {
 	graph := t.config.Graph()
 	if err := graph.Validate(); err != nil {
@@ -122,8 +139,17 @@ func (t *Terraform) Plan(s *State) (*Plan, error) {
 	return result, nil
 }
 
-func (t *Terraform) Refresh(*State) (*State, error) {
-	return nil, nil
+// Refresh goes through all the resources in the state and refreshes them
+// to their latest status.
+func (t *Terraform) Refresh(c *config.Config, s *State) (*State, error) {
+	_, err := t.Graph(c, s)
+	if err != nil {
+		return s, err
+	}
+
+	result := new(State)
+	//err = graph.Walk(t.refreshWalkFn(s, result))
+	return result, err
 }
 
 func (t *Terraform) applyWalkFn(

terraform/terraform_test.go

@@ -12,198 +12,6 @@ import (
 // This is the directory where our test fixtures are.
 const fixtureDir = "./test-fixtures"
 
-func TestNew(t *testing.T) {
-	configVal := testConfig(t, "new-good")
-	tfConfig := &Config{
-		Config: configVal,
-		Providers: map[string]ResourceProviderFactory{
-			"aws": testProviderFunc("aws", []string{"aws_instance"}),
-			"do":  testProviderFunc("do", []string{"do_droplet"}),
-		},
-	}
-
-	tf, err := New(tfConfig)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if tf == nil {
-		t.Fatal("tf should not be nil")
-	}
-
-	if len(tf.mapping) != 2 {
-		t.Fatalf("bad: %#v", tf.mapping)
-	}
-	if testProviderName(t, tf, "aws_instance.foo") != "aws" {
-		t.Fatalf("bad: %#v", tf.mapping)
-	}
-	if testProviderName(t, tf, "do_droplet.bar") != "do" {
-		t.Fatalf("bad: %#v", tf.mapping)
-	}
-
-	var pc *config.ProviderConfig
-
-	pc = testProviderConfig(tf, "do_droplet.bar")
-	if pc != nil {
-		t.Fatalf("bad: %#v", pc)
-	}
-
-	pc = testProviderConfig(tf, "aws_instance.foo")
-	if pc.RawConfig.Raw["foo"].(string) != "bar" {
-		t.Fatalf("bad: %#v", pc)
-	}
-}
-
-func TestNew_graphCycle(t *testing.T) {
-	config := testConfig(t, "new-graph-cycle")
-	tfConfig := &Config{
-		Config: config,
-		Providers: map[string]ResourceProviderFactory{
-			"aws": testProviderFunc("aws", []string{"aws_instance"}),
-		},
-	}
-
-	tf, err := New(tfConfig)
-	if err == nil {
-		t.Fatal("should error")
-	}
-	if tf != nil {
-		t.Fatalf("should not return tf")
-	}
-}
-
-func TestNew_providerConfigCache(t *testing.T) {
-	configVal := testConfig(t, "new-pc-cache")
-	tfConfig := &Config{
-		Config: configVal,
-		Providers: map[string]ResourceProviderFactory{
-			"aws": testProviderFunc(
-				"aws", []string{"aws_elb", "aws_instance"}),
-			"do": testProviderFunc("do", []string{"do_droplet"}),
-		},
-	}
-
-	tf, err := New(tfConfig)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if tf == nil {
-		t.Fatal("tf should not be nil")
-	}
-
-	if testProviderName(t, tf, "aws_instance.foo") != "aws" {
-		t.Fatalf("bad: %#v", tf.mapping)
-	}
-	if testProviderName(t, tf, "aws_elb.lb") != "aws" {
-		t.Fatalf("bad: %#v", tf.mapping)
-	}
-	if testProviderName(t, tf, "do_droplet.bar") != "do" {
-		t.Fatalf("bad: %#v", tf.mapping)
-	}
-
-	if testProvider(tf, "aws_instance.foo") !=
-		testProvider(tf, "aws_instance.bar") {
-		t.Fatalf("bad equality")
-	}
-	if testProvider(tf, "aws_instance.foo") ==
-		testProvider(tf, "aws_elb.lb") {
-		t.Fatal("should not be equal")
-	}
-
-	var pc *config.ProviderConfig
-	pc = testProviderConfig(tf, "do_droplet.bar")
-	if pc != nil {
-		t.Fatalf("bad: %#v", pc)
-	}
-	pc = testProviderConfig(tf, "aws_instance.foo")
-	if pc.RawConfig.Raw["foo"].(string) != "bar" {
-		t.Fatalf("bad: %#v", pc)
-	}
-	pc = testProviderConfig(tf, "aws_elb.lb")
-	if pc.RawConfig.Raw["foo"].(string) != "baz" {
-		t.Fatalf("bad: %#v", pc)
-	}
-
-	if testProviderConfig(tf, "aws_instance.foo") !=
-		testProviderConfig(tf, "aws_instance.bar") {
-		t.Fatal("should be same")
-	}
-	if testProviderConfig(tf, "aws_instance.foo") ==
-		testProviderConfig(tf, "aws_elb.lb") {
-		t.Fatal("should be different")
-	}
-
-	// Finally, verify some internals here that we're using the
-	// IDENTICAL *terraformProvider pointer for matching types
-	if testTerraformProvider(tf, "aws_instance.foo") !=
-		testTerraformProvider(tf, "aws_instance.bar") {
-		t.Fatal("should be same")
-	}
-}
-
-func TestNew_providerValidate(t *testing.T) {
-	config := testConfig(t, "new-provider-validate")
-	tfConfig := &Config{
-		Config: config,
-		Providers: map[string]ResourceProviderFactory{
-			"aws": testProviderFunc("aws", []string{"aws_instance"}),
-		},
-	}
-
-	tf, err := New(tfConfig)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	p := testProviderMock(testProvider(tf, "aws_instance.foo"))
-	if !p.ValidateCalled {
-		t.Fatal("validate should be called")
-	}
-}
-
-func TestNew_variables(t *testing.T) {
-	config := testConfig(t, "new-variables")
-	tfConfig := &Config{
-		Config: config,
-	}
-
-	// Missing
-	tfConfig.Variables = map[string]string{
-		"bar": "baz",
-	}
-	tf, err := New(tfConfig)
-	if err == nil {
-		t.Fatal("should error")
-	}
-	if tf != nil {
-		t.Fatalf("should not return tf")
-	}
-
-	// Good
-	tfConfig.Variables = map[string]string{
-		"foo": "bar",
-	}
-	tf, err = New(tfConfig)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if tf == nil {
-		t.Fatal("tf should not be nil")
-	}
-
-	// Good
-	tfConfig.Variables = map[string]string{
-		"foo": "bar",
-		"bar": "baz",
-	}
-	tf, err = New(tfConfig)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if tf == nil {
-		t.Fatal("tf should not be nil")
-	}
-}
-
 func TestTerraformApply(t *testing.T) {
 	tf := testTerraform(t, "apply-good")
 
@@ -581,6 +389,22 @@ func testTerraform(t *testing.T, name string) *Terraform {
 	return tf
 }
 
+func testTerraform2(t *testing.T, c *Config) *Terraform {
+	if c == nil {
+		c = new(Config)
+	}
+
+	tf, err := New(c)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if tf == nil {
+		t.Fatal("tf should not be nil")
+	}
+
+	return tf
+}
+
 func testTerraformProvider(tf *Terraform, n string) *terraformProvider {
 	for r, tp := range tf.mapping {
 		if r.Id() == n {

terraform/test-fixtures/graph-basic/main.tf

@@ -0,0 +1,24 @@
+variable "foo" {
+    default = "bar";
+    description = "bar";
+}
+
+provider "aws" {
+    foo = "${openstack_floating_ip.random.value}"
+}
+
+resource "openstack_floating_ip" "random" {}
+
+resource "aws_security_group" "firewall" {}
+
+resource "aws_instance" "web" {
+    ami = "${var.foo}"
+    security_groups = [
+        "foo",
+        "${aws_security_group.firewall.foo}"
+    ]
+}
+
+resource "aws_load_balancer" "weblb" {
+    members = "${aws_instance.web.id_list}"
+}

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

@@ -0,0 +1,18 @@
+variable "foo" {
+    default = "bar";
+    description = "bar";
+}
+
+provider "aws" {
+    foo = "${aws_security_group.firewall.value}"
+}
+
+resource "aws_security_group" "firewall" {}
+
+resource "aws_instance" "web" {
+    ami = "${var.foo}"
+    security_groups = [
+        "foo",
+        "${aws_security_group.firewall.foo}"
+    ]
+}