NoOp nodes should not have destroy edges

NoOp changes should not participate in a destroy sequence, but because
they are included as normal update nodes the usual connections were
still being made.

internal/terraform/graph_builder_apply.go

@@ -142,7 +142,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		&ForcedCBDTransformer{},
 
 		// Destruction ordering
-		&DestroyEdgeTransformer{},
+		&DestroyEdgeTransformer{
+			Changes: b.Changes,
+		},
 		&CBDEdgeTransformer{
 			Config: b.Config,
 			State:  b.State,

internal/terraform/transform_destroy_edge.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/dag"
+	"github.com/hashicorp/terraform/internal/plans"
 )
 
 // GraphNodeDestroyer must be implemented by nodes that destroy resources.
@@ -37,7 +38,15 @@ type GraphNodeCreator interface {
 // dependent resources will block parent resources from deleting. Concrete
 // example: VPC with subnets, the VPC can't be deleted while there are
 // still subnets.
-type DestroyEdgeTransformer struct{}
+type DestroyEdgeTransformer struct {
+	// FIXME: GraphNodeCreators are not always applying changes, and should not
+	// participate in the destroy graph if there are no operations which could
+	// interract with destroy nodes. We need Changes for now to detect the
+	// action type, but perhaps this should be indicated somehow by the
+	// DiffTransformer which was intended to be the only transformer operating
+	// from the change set.
+	Changes *plans.Changes
+}
 
 // tryInterProviderDestroyEdge checks if we're inserting a destroy edge
 // across a provider boundary, and only adds the edge if it results in no cycles.
@@ -144,8 +153,20 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error {
 			resAddr := addr.ContainingResource().Config().String()
 			destroyersByResource[resAddr] = append(destroyersByResource[resAddr], n)
 		case GraphNodeCreator:
-			addr := n.CreateAddr().ContainingResource().Config().String()
-			creators[addr] = append(creators[addr], n)
+			addr := n.CreateAddr()
+			cfgAddr := addr.ContainingResource().Config().String()
+
+			if t.Changes == nil {
+				// unit tests may not have changes
+				creators[cfgAddr] = append(creators[cfgAddr], n)
+				break
+			}
+
+			// NoOp changes should not participate in the destroy dependencies.
+			rc := t.Changes.ResourceInstance(*addr)
+			if rc != nil && rc.Action != plans.NoOp {
+				creators[cfgAddr] = append(creators[cfgAddr], n)
+			}
 		}
 	}
 

internal/terraform/transform_destroy_edge_test.go

@@ -440,6 +440,75 @@ func TestPruneUnusedNodesTransformer_rootModuleOutputValues(t *testing.T) {
 	}
 }
 
+// NoOp changes should not be participating in the destroy sequence
+func TestDestroyEdgeTransformer_noOp(t *testing.T) {
+	g := Graph{Path: addrs.RootModuleInstance}
+	g.Add(testDestroyNode("test_object.A"))
+	g.Add(testUpdateNode("test_object.B"))
+	g.Add(testDestroyNode("test_object.C"))
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("test_object.A").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"A"}`),
+		},
+		mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+	)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("test_object.B").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status:       states.ObjectReady,
+			AttrsJSON:    []byte(`{"id":"B","test_string":"x"}`),
+			Dependencies: []addrs.ConfigResource{mustConfigResourceAddr("test_object.A")},
+		},
+		mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+	)
+	root.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("test_object.C").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"C","test_string":"x"}`),
+			Dependencies: []addrs.ConfigResource{mustConfigResourceAddr("test_object.A"),
+				mustConfigResourceAddr("test_object.B")},
+		},
+		mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+	)
+
+	if err := (&AttachStateTransformer{State: state}).Transform(&g); err != nil {
+		t.Fatal(err)
+	}
+
+	tf := &DestroyEdgeTransformer{
+		// We only need a minimal object to indicate GraphNodeCreator change is
+		// a NoOp here.
+		Changes: &plans.Changes{
+			Resources: []*plans.ResourceInstanceChangeSrc{
+				{
+					Addr:      mustResourceInstanceAddr("test_object.B"),
+					ChangeSrc: plans.ChangeSrc{Action: plans.NoOp},
+				},
+			},
+		},
+	}
+	if err := tf.Transform(&g); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	expected := strings.TrimSpace(`
+test_object.A (destroy)
+  test_object.C (destroy)
+test_object.B
+test_object.C (destroy)`)
+
+	actual := strings.TrimSpace(g.String())
+	if actual != expected {
+		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
+	}
+}
+
 func testDestroyNode(addrString string) GraphNodeDestroyer {
 	instAddr := mustResourceInstanceAddr(addrString)
 	inst := NewNodeAbstractResourceInstance(instAddr)