tofu: GraphNodeDestroyer can now have references

Previously the ReferenceTransformer just had a broad rule that any node
which implements GraphNodeDestroyer has its expression references
completely ignored.

Now that we're starting to allow dynamic expressions for destroy-related
settings like "prevent_destroy", we need to be able to represent the
dependencies implied by those expressions.

However, the assumption that configuration is mostly ignored when planning
destroy is load-bearing for minimizing awkward dependency problems in the
destroy-planning graph, so this introduces a new concept of "destroy
references" which means that we can implement only a small, curated subset
of references -- for now, just the ones from prevent_destroy -- that get
considered for any node type that implements GraphNodeDestroyer.

Having GraphNodeDestroyer effectively take priority over
GraphNodeReferencer seems like the least disruptive way to retrofit this
idea surgically as a small change to the previous unilateral rule against
any references at all, because in practice all of the destroy nodes embed
NodeAbstractResourceInstance and therefore implement GraphNodeReferencer,
so it is important that we continue to ignore that type's
GraphNodeReferencer implementation whenever it's embedded in something
that is also a GraphNodeDestroyer.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>

internal/tofu/context_plan_test.go

@@ -1819,6 +1819,93 @@ func TestContext2Plan_preventDestroy_dynamicFromDataResource(t *testing.T) {
 	})
 }
 
+func TestContext2Plan_preventDestroy_dynamicFromManagedResourceDestroyMode(t *testing.T) {
+	// This test is verifying that a prevent_destroy argument's dependencies
+	// are respected even when we're in plans.DestroyMode, which has separate
+	// rules for how its graph gets built.
+	//
+	// With the design of the system at the time of writing this comment, the
+	// likely failure mode is that no dependency from test_instance.foo to
+	// var.prevent_destroy is created in the child module, and so the variable
+	// itself is eligible to be "pruned" from the graph as apparently irrelevant
+	// to the destroy phase. That's incorrect because, unlike most of the rest
+	// of a resource configuration, prevent_destroy is relevant during destroy
+	// planning just like in normal planning.
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+				module "child" {
+					source = "./child"
+
+					prevent_destroy = true
+				}
+			`,
+		"child/main.tf": `
+				variable "prevent_destroy" {
+					type = bool
+				}
+
+				resource "test_instance" "foo" {
+					require_new = "yes"
+
+					lifecycle {
+						prevent_destroy = var.prevent_destroy
+					}
+				}
+			`,
+	})
+	mainP := testProvider("test")
+	mainP.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
+		// Because we're simulating a "replace" situation here, the
+		// plan function actually gets called twice with the second
+		// one planning the "create" part of the replace. We want
+		// do do our fake synctest sleep only on the first plan call
+		// (where there's a prior state) because that's the one whose
+		// execution is supposed to wait until the data resource read
+		// has completed.
+		if !req.PriorState.IsNull() {
+			log.Printf("[TRACE] TestContext2Plan_preventDestroy_dynamicFromManagedResourceDestroyMode: test_instance.foo plan")
+		} else {
+			log.Printf("[TRACE] TestContext2Plan_preventDestroy_dynamicFromManagedResourceDestroyMode: test_instance.foo followup plan")
+		}
+		return testDiffFn(req)
+	}
+	state := states.NewState()
+	child := state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
+	child.SetResourceInstanceCurrent(
+		mustResourceInstanceAddr("test_instance.foo").Resource,
+		&states.ResourceInstanceObjectSrc{
+			Status:    states.ObjectReady,
+			AttrsJSON: []byte(`{"id":"i-abc123"}`),
+		},
+		mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`),
+		addrs.NoKey,
+	)
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(mainP),
+		},
+	})
+
+	_, diags := ctx.Plan(context.Background(), m, state, &PlanOpts{
+		Mode:        plans.DestroyMode,
+		SkipRefresh: true,
+	})
+	if !diags.HasErrors() {
+		t.Fatalf("unexpected success; want error about prevent_destroy being set")
+	}
+	gotErr := diags.Err().Error()
+	wantErr := `test_instance.foo has prevent_destroy set`
+	if strings.Contains(gotErr, "has a prevent_destroy argument but its value will not be known until the apply step") {
+		// This is just a best-effort hint for one failure mode we've seen before,
+		// so that if it regresses we'll have a clue about what to investigate.
+		t.Errorf("unexpected error about unknown values; is there a missing dependency edge between the resource and the input variable in the walkPlanDestroy graph?")
+	}
+	if !strings.Contains(gotErr, wantErr) {
+		t.Fatalf("missing expected error\ngot: %s\nwant error diagnostics with substring %q", gotErr, wantErr)
+	}
+}
+
 func TestContext2Plan_preventDestroy_countBad(t *testing.T) {
 	m := testModule(t, "plan-prevent-destroy-count-bad")
 	p := testProvider("aws")

internal/tofu/node_resource_abstract.go

@@ -251,6 +251,28 @@ func (n *NodeAbstractResource) References() []*addrs.Reference {
 	return result
 }
 
+// DestroyReferences is a _partial_ implementation of [GraphNodeDestroyer]
+// providing a default implementation for any embedding node type that has
+// its own implementations of all of the other methods of that interface.
+func (n *NodeAbstractResource) DestroyReferences() []*addrs.Reference {
+	// Config is always optional at destroy time, but if it's present then
+	// it might influence how we plan and apply the destroy actions.
+	var result []*addrs.Reference
+	if c := n.Config; c != nil {
+		if c.Managed != nil {
+			// The prevent_destroy setting, if present, forces planning to fail
+			// if the planned action for any instance of the resource is to
+			// destroy it, so we need to be able to evaluate the given expression
+			// for destroy nodes too.
+			if c.Managed.PreventDestroy != nil {
+				refs, _ := lang.ReferencesInExpr(addrs.ParseRef, c.Managed.PreventDestroy)
+				result = append(result, refs...)
+			}
+		}
+	}
+	return result
+}
+
 // referencesInImportAddress find all references relevant to the node in an import target address expression.
 // The only references we care about here are the references that exist in the keys of hclsyntax.IndexExpr.
 // For example, if the address is module.my_module1[expression1].aws_s3_bucket.bucket[expression2], then we would only

internal/tofu/node_resource_abstract_instance.go

@@ -168,6 +168,29 @@ func (n *NodeAbstractResourceInstance) References() []*addrs.Reference {
 	return nil
 }
 
+// DestroyReferences is a _partial_ implementation of [GraphNodeDestroyer],
+// providing a default implementation of this method for any embedder of
+// [NodeAbstractResourceInstance] that implements all of the other methods
+// of that interface.
+func (n *NodeAbstractResourceInstance) DestroyReferences() []*addrs.Reference {
+	// If we have a configuration attached then we'll delegate to our
+	// embedded abstract resource, which knows how to extract dependencies
+	// from configuration. If there is no config, then the dependencies will
+	// be connected during destroy from those stored in the state.
+	if n.Config != nil {
+		if n.Schema == nil {
+			// We'll produce a log message about this out here so that
+			// we can include the full instance address, since the equivalent
+			// message in NodeAbstractResource.References cannot see it.
+			log.Printf("[WARN] no schema is attached to %s, so destroy-time config references cannot be detected", n.Name())
+			return nil
+		}
+		return n.NodeAbstractResource.DestroyReferences()
+	}
+
+	return nil
+}
+
 func (n *NodeAbstractResourceInstance) resolveProvider(ctx context.Context, evalCtx EvalContext, hasExpansionData bool, deposedKey states.DeposedKey) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
 

internal/tofu/transform_destroy_edge.go

@@ -22,6 +22,14 @@ type GraphNodeDestroyer interface {
 	// destroyed by this node. If this returns nil, then this node
 	// is not destroying anything.
 	DestroyAddr() *addrs.AbsResourceInstance
+
+	// DestroyReferences is the equivalent of [GraphNodeReferencer.References]
+	// for destroy-related nodes, representing the fact that when destroying
+	// we only care about references in the subset of expressions that relate
+	// to destroying. Destroy is _mostly_ a state-only operation, but considers
+	// some metadata from the configuration when available to influence how
+	// destroying is planned.
+	DestroyReferences() []*addrs.Reference
 }
 
 // GraphNodeCreator must be implemented by nodes that create OR update resources.

internal/tofu/transform_reference.go

@@ -124,12 +124,6 @@ func (t *ReferenceTransformer) Transform(_ context.Context, g *Graph) error {
 
 	// Find the things that reference things and connect them
 	for _, v := range vs {
-		if _, ok := v.(GraphNodeDestroyer); ok {
-			// destroy nodes references are not connected, since they can only
-			// use their own state.
-			continue
-		}
-
 		parents := m.References(v)
 		parentsDbg := make([]string, len(parents))
 		for i, v := range parents {
@@ -300,20 +294,30 @@ type ReferenceMap map[string][]dag.Vertex
 // References returns the set of vertices that the given vertex refers to,
 // and any referenced addresses that do not have corresponding vertices.
 func (m ReferenceMap) References(v dag.Vertex) []dag.Vertex {
-	rn, ok := v.(GraphNodeReferencer)
-	if !ok {
+	var refsFunc func() []*addrs.Reference
+	if rn, ok := v.(GraphNodeDestroyer); ok {
+		// For nodes that represent destroying something we use a separate
+		// method because destroying typically uses only a subset of the
+		// references that would be relevant for other operations: destroying
+		// is _mostly_ a state-only operation that only use the configuration
+		// opportunistically for some metadata that influences how the destroy
+		// action is planned.
+		refsFunc = rn.DestroyReferences
+	} else if rn, ok := v.(GraphNodeReferencer); ok {
+		refsFunc = rn.References
+	} else {
 		return nil
 	}
 
 	var matches []dag.Vertex
 
-	if rrn, ok := rn.(GraphNodeRootReferencer); ok {
+	if rrn, ok := v.(GraphNodeRootReferencer); ok {
 		for _, ref := range rrn.RootReferences() {
 			matches = append(matches, m.addReference(addrs.RootModule, v, ref)...)
 		}
 	}
 
-	for _, ref := range rn.References() {
+	for _, ref := range refsFunc() {
 		matches = append(matches, m.addReference(vertexReferencePath(v), v, ref)...)
 	}