terraform: when promoting non-CBD to CBD, mark the config as such

This brings the change for the  new graph. See #10455

terraform/node_resource_destroy.go

@@ -30,6 +30,20 @@ func (n *NodeDestroyResource) CreateBeforeDestroy() bool {
 	return n.Config.Lifecycle.CreateBeforeDestroy
 }
 
+// GraphNodeDestroyerCBD
+func (n *NodeDestroyResource) ModifyCreateBeforeDestroy(v bool) error {
+	// If we have no config, do nothing since it won't affect the
+	// create step anyways.
+	if n.Config == nil {
+		return nil
+	}
+
+	// Set CBD to true
+	n.Config.Lifecycle.CreateBeforeDestroy = true
+
+	return nil
+}
+
 // GraphNodeReferenceable, overriding NodeAbstractResource
 func (n *NodeDestroyResource) ReferenceableName() []string {
 	result := n.NodeAbstractResource.ReferenceableName()

terraform/transform_destroy_cbd.go

@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"fmt"
 	"log"
 
 	"github.com/hashicorp/terraform/config/module"
@@ -15,6 +16,11 @@ type GraphNodeDestroyerCBD interface {
 	// CreateBeforeDestroy returns true if this node represents a node
 	// that is doing a CBD.
 	CreateBeforeDestroy() bool
+
+	// ModifyCreateBeforeDestroy is called when the CBD state of a node
+	// is changed dynamically. This can return an error if this isn't
+	// allowed.
+	ModifyCreateBeforeDestroy(bool) error
 }
 
 // CBDEdgeTransformer modifies the edges of CBD nodes that went through
@@ -48,9 +54,25 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error {
 		}
 
 		if !dn.CreateBeforeDestroy() {
+			// If there are no CBD ancestors (dependent nodes), then we
+			// do nothing here.
+			if !t.hasCBDAncestor(g, v) {
 				continue
 			}
 
+			// If this isn't naturally a CBD node, this means that an ancestor is
+			// and we need to auto-upgrade this node to CBD. We do this because
+			// a CBD node depending on non-CBD will result in cycles. To avoid this,
+			// we always attempt to upgrade it.
+			if err := dn.ModifyCreateBeforeDestroy(true); err != nil {
+				return fmt.Errorf(
+					"%s: must have create before destroy enabled because "+
+						"a dependent resource has CBD enabled. However, when "+
+						"attempting to automatically do this, an error occurred: %s",
+					dag.VertexName(v), err)
+			}
+		}
+
 		// Find the destroy edge. There should only be one.
 		for _, e := range g.EdgesTo(v) {
 			// Not a destroy edge, ignore it
@@ -189,3 +211,26 @@ func (t *CBDEdgeTransformer) depMap(
 
 	return depMap, nil
 }
+
+// hasCBDAncestor returns true if any ancestor (node that depends on this)
+// has CBD set.
+func (t *CBDEdgeTransformer) hasCBDAncestor(g *Graph, v dag.Vertex) bool {
+	s, _ := g.Ancestors(v)
+	if s == nil {
+		return true
+	}
+
+	for _, v := range s.List() {
+		dn, ok := v.(GraphNodeDestroyerCBD)
+		if !ok {
+			continue
+		}
+
+		if dn.CreateBeforeDestroy() {
+			// some ancestor is CreateBeforeDestroy, so we need to follow suit
+			return true
+		}
+	}
+
+	return false
+}

terraform/transform_destroy_cbd_test.go

@@ -36,6 +36,38 @@ func TestCBDEdgeTransformer(t *testing.T) {
 	}
 }
 
+func TestCBDEdgeTransformer_depNonCBD(t *testing.T) {
+	g := Graph{Path: RootModulePath}
+	g.Add(&graphNodeCreatorTest{AddrString: "test.A"})
+	g.Add(&graphNodeCreatorTest{AddrString: "test.B"})
+	g.Add(&graphNodeDestroyerTest{AddrString: "test.A"})
+	g.Add(&graphNodeDestroyerTest{AddrString: "test.B", CBD: true})
+
+	module := testModule(t, "transform-destroy-edge-basic")
+
+	{
+		tf := &DestroyEdgeTransformer{
+			Module: module,
+		}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		tf := &CBDEdgeTransformer{Module: module}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformCBDEdgeDepNonCBDStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
 const testTransformCBDEdgeBasicStr = `
 test.A
 test.A (destroy)
@@ -43,3 +75,14 @@ test.A (destroy)
   test.B
 test.B
 `
+
+const testTransformCBDEdgeDepNonCBDStr = `
+test.A
+test.A (destroy) (modified)
+  test.A
+  test.B
+  test.B (destroy)
+test.B
+test.B (destroy)
+  test.B
+`

terraform/transform_destroy_edge_test.go

@@ -113,10 +113,25 @@ func (n *graphNodeCreatorTest) CreateAddr() *ResourceAddress {
 type graphNodeDestroyerTest struct {
 	AddrString string
 	CBD        bool
+	Modified   bool
+}
+
+func (n *graphNodeDestroyerTest) Name() string {
+	result := n.DestroyAddr().String() + " (destroy)"
+	if n.Modified {
+		result += " (modified)"
+	}
+
+	return result
 }
 
-func (n *graphNodeDestroyerTest) Name() string              { return n.DestroyAddr().String() + " (destroy)" }
 func (n *graphNodeDestroyerTest) CreateBeforeDestroy() bool { return n.CBD }
+
+func (n *graphNodeDestroyerTest) ModifyCreateBeforeDestroy(v bool) error {
+	n.Modified = true
+	return nil
+}
+
 func (n *graphNodeDestroyerTest) DestroyAddr() *ResourceAddress {
 	addr, err := ParseResourceAddress(n.AddrString)
 	if err != nil {