Use Descendants rather than UpEdges in CBD

When looking for dependencies to fix when handling
create_before_destroy, we need to look past more than one edge, as
dependencies may appear transitively through outputs and variables. Use
Descendants rather than UpEdges.

We have the full graph to use for the CBD transformation, so there's no
longer any need to create a temporary graph, which may differ from the
original.

terraform/transform_config_flat.go

@@ -1,71 +0,0 @@
-package terraform
-
-import (
-	"github.com/hashicorp/terraform/configs"
-	"github.com/hashicorp/terraform/dag"
-)
-
-// FlatConfigTransformer is a GraphTransformer that adds the configuration
-// to the graph. The module used to configure this transformer must be
-// the root module.
-//
-// This transform adds the nodes but doesn't connect any of the references.
-// The ReferenceTransformer should be used for that.
-//
-// NOTE: In relation to ConfigTransformer: this is a newer generation config
-// transformer. It puts the _entire_ config into the graph (there is no
-// "flattening" step as before).
-type FlatConfigTransformer struct {
-	Concrete ConcreteResourceNodeFunc // What to turn resources into
-
-	Config *configs.Config
-}
-
-func (t *FlatConfigTransformer) Transform(g *Graph) error {
-	// We have nothing to do if there is no configuration.
-	if t.Config == nil {
-		return nil
-	}
-
-	return t.transform(g, t.Config)
-}
-
-func (t *FlatConfigTransformer) transform(g *Graph, config *configs.Config) error {
-	// If we have no configuration then there's nothing to do.
-	if config == nil {
-		return nil
-	}
-
-	// Transform all the children.
-	for _, c := range config.Children {
-		if err := t.transform(g, c); err != nil {
-			return err
-		}
-	}
-
-	module := config.Module
-	// For now we assume that each module call produces only one module
-	// instance with no key, since we don't yet support "count" and "for_each"
-	// on modules.
-	// FIXME: As part of supporting "count" and "for_each" on modules, rework
-	// this so that we'll "expand" the module call first and then create graph
-	// nodes for each module instance separately.
-	instPath := config.Path.UnkeyedInstanceShim()
-
-	for _, r := range module.ManagedResources {
-		addr := r.Addr().Absolute(instPath)
-		abstract := &NodeAbstractResource{
-			Addr:   addr,
-			Config: r,
-		}
-		// Grab the address for this resource
-		var node dag.Vertex = abstract
-		if f := t.Concrete; f != nil {
-			node = f(abstract)
-		}
-
-		g.Add(node)
-	}
-
-	return nil
-}

terraform/transform_config_flat_test.go

@@ -1,42 +0,0 @@
-package terraform
-
-import (
-	"strings"
-	"testing"
-
-	"github.com/hashicorp/terraform/addrs"
-)
-
-func TestFlatConfigTransformer_nilModule(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	tf := &FlatConfigTransformer{}
-	if err := tf.Transform(&g); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if len(g.Vertices()) > 0 {
-		t.Fatal("graph should be empty")
-	}
-}
-
-func TestFlatConfigTransformer(t *testing.T) {
-	g := Graph{Path: addrs.RootModuleInstance}
-	tf := &FlatConfigTransformer{
-		Config: testModule(t, "transform-flat-config-basic"),
-	}
-	if err := tf.Transform(&g); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformFlatConfigBasicStr)
-	if actual != expected {
-		t.Fatalf("bad:\n\n%s", actual)
-	}
-}
-
-const testTransformFlatConfigBasicStr = `
-aws_instance.bar
-aws_instance.foo
-module.child.aws_instance.baz
-`

terraform/transform_destroy_cbd.go

@@ -169,6 +169,7 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 			applyNode := de.Source()
 			destroyNode := de.Target()
 			g.Connect(&DestroyEdge{S: destroyNode, T: applyNode})
+			break
 		}
 
 		// If the address has an index, we strip that. Our depMap creation
@@ -201,12 +202,7 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 	// They key here is that B happens before A is destroyed. This is to
 	// facilitate the primary purpose for CBD: making sure that downstreams
 	// are properly updated to avoid downtime before the resource is destroyed.
-	//
+	depMap, err := t.depMap(g, destroyMap)
-	// We can't trust that the resource being destroyed or anything that
-	// depends on it is actually in our current graph so we make a new
-	// graph in order to determine those dependencies and add them in.
-	log.Printf("[TRACE] CBDEdgeTransformer: building graph to find dependencies...")
-	depMap, err := t.depMap(destroyMap)
 	if err != nil {
 		return err
 	}
@@ -248,26 +244,10 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 	return nil
 }
 
-func (t *CBDEdgeTransformer) depMap(destroyMap map[string][]dag.Vertex) (map[string][]dag.Vertex, error) {
+func (t *CBDEdgeTransformer) depMap(g *Graph, destroyMap map[string][]dag.Vertex) (map[string][]dag.Vertex, error) {
-	// Build the graph of our config, this ensures that all resources
+	// Build the list of destroy nodes that each resource address should depend
-	// are present in the graph.
+	// on. For example, when we find B, we map the address of B to A_d in the
-	g, diags := (&BasicGraphBuilder{
+	// "depMap" variable below.
-		Steps: []GraphTransformer{
-			&FlatConfigTransformer{Config: t.Config},
-			&AttachResourceConfigTransformer{Config: t.Config},
-			&AttachStateTransformer{State: t.State},
-			&AttachSchemaTransformer{Schemas: t.Schemas},
-			&ReferenceTransformer{},
-		},
-		Name: "CBDEdgeTransformer",
-	}).Build(nil)
-	if diags.HasErrors() {
-		return nil, diags.Err()
-	}
-
-	// Using this graph, build the list of destroy nodes that each resource
-	// address should depend on. For example, when we find B, we map the
-	// address of B to A_d in the "depMap" variable below.
 	depMap := make(map[string][]dag.Vertex)
 	for _, v := range g.Vertices() {
 		// We're looking for resources.
@@ -289,8 +269,10 @@ func (t *CBDEdgeTransformer) depMap(destroyMap map[string][]dag.Vertex) (map[str
 		}
 
 		// Get the nodes that depend on this on. In the example above:
-		// finding B in A => B.
+		// finding B in A => B. Since dependencies can span modules, walk all
-		for _, v := range g.UpEdges(v).List() {
+		// descendents of the resource.
+		des, _ := g.Descendents(v)
+		for _, v := range des.List() {
 			// We're looking for resources.
 			rn, ok := v.(GraphNodeResource)
 			if !ok {