Merge pull request #1816 from hashicorp/b-validate-plan-graph-not-verbose-graph

core: validate graph w/ diff during plan phase

terraform/context.go

@@ -331,6 +331,12 @@ func (c *Context) Plan() (*Plan, error) {
 	}
 	p.Diff = c.diff
 
+	// Now that we have a diff, we can build the exact graph that Apply will use
+	// and catch any possible cycles during the Plan phase.
+	if _, err := c.Graph(&ContextGraphOpts{Validate: true}); err != nil {
+		return nil, err
+	}
+
 	return p, nil
 }
 
@@ -406,12 +412,11 @@ func (c *Context) Validate() ([]string, []error) {
 		}
 	}
 
-	// Build a Verbose version of the graph so we can catch any potential cycles
+	// Build the graph so we can walk it and run Validate on nodes.
-	// in the validate stage
+	// We also validate the graph generated here, but this graph doesn't
-	graph, err := c.Graph(&ContextGraphOpts{
+	// necessarily match the graph that Plan will generate, so we'll validate the
-		Validate: true,
+	// graph again later after Planning.
-		Verbose:  false,
+	graph, err := c.Graph(&ContextGraphOpts{Validate: true})
-	})
 	if err != nil {
 		return nil, []error{err}
 	}

terraform/graph_config_node_resource.go

@@ -426,10 +426,13 @@ func (n *graphNodeResourceDestroy) destroyIncludePrimary(
 	// to include this resource.
 	prefix := n.Original.Resource.Id()
 
-	// If we're present in the diff proper, then keep it.
+	// If we're present in the diff proper, then keep it. We're looking
+	// only for resources in the diff that match our resource or a count-index
+	// of our resource that are marked for destroy.
 	if d != nil {
-		for k, _ := range d.Resources {
+		for k, d := range d.Resources {
-			if strings.HasPrefix(k, prefix) {
+			match := k == prefix || strings.HasPrefix(k, prefix+".")
+			if match && d.Destroy {
 				return true
 			}
 		}

terraform/transform_destroy_test.go

@@ -166,7 +166,7 @@ func TestPruneDestroyTransformer_diff(t *testing.T) {
 	actual := strings.TrimSpace(g.String())
 	expected := strings.TrimSpace(testTransformPruneDestroyBasicDiffStr)
 	if actual != expected {
-		t.Fatalf("bad:\n\n%s", actual)
+		t.Fatalf("expected:\n\n%s\n\nbad:\n\n%s", expected, actual)
 	}
 }
 
@@ -421,9 +421,7 @@ aws_instance.foo
 
 const testTransformPruneDestroyBasicDiffStr = `
 aws_instance.bar
-  aws_instance.bar (destroy)
   aws_instance.foo
-aws_instance.bar (destroy)
 aws_instance.foo
 `