From c29cb25f86fc3449f646a08ebdce5143829352b7 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 13 May 2026 14:12:38 -0400 Subject: [PATCH] remove old resource phase addressing At one point there was an attempt to connect the destroy nodes in the apply graph by using special addressing and manipulating the references for resources. This doesn't work for a few reasons, but in particular, destroy nodes are entirely built from their state, they do not reference anything, and they cannot be referenced. --- internal/addrs/resource_phase.go | 120 ------------------ .../terraform/node_resource_apply_instance.go | 31 +---- internal/terraform/node_resource_destroy.go | 22 +--- internal/terraform/transform_reference.go | 4 - 4 files changed, 3 insertions(+), 174 deletions(-) delete mode 100644 internal/addrs/resource_phase.go diff --git a/internal/addrs/resource_phase.go b/internal/addrs/resource_phase.go deleted file mode 100644 index bfcaecbf55..0000000000 --- a/internal/addrs/resource_phase.go +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright IBM Corp. 2014, 2026 -// SPDX-License-Identifier: BUSL-1.1 - -package addrs - -import "fmt" - -// ResourceInstancePhase is a special kind of reference used only internally -// during graph building to represent resource instances that are in a -// non-primary state. -// -// Graph nodes can declare themselves referenceable via an instance phase -// or can declare that they reference an instance phase in order to accomodate -// secondary graph nodes dealing with, for example, destroy actions. -// -// This special reference type cannot be accessed directly by end-users, and -// should never be shown in the UI. -type ResourceInstancePhase struct { - referenceable - ResourceInstance ResourceInstance - Phase ResourceInstancePhaseType -} - -var _ Referenceable = ResourceInstancePhase{} - -// Phase returns a special "phase address" for the receving instance. See the -// documentation of ResourceInstancePhase for the limited situations where this -// is intended to be used. -func (r ResourceInstance) Phase(rpt ResourceInstancePhaseType) ResourceInstancePhase { - return ResourceInstancePhase{ - ResourceInstance: r, - Phase: rpt, - } -} - -// ContainingResource returns an address for the same phase of the resource -// that this instance belongs to. -func (rp ResourceInstancePhase) ContainingResource() ResourcePhase { - return rp.ResourceInstance.Resource.Phase(rp.Phase) -} - -func (rp ResourceInstancePhase) String() string { - // We use a different separator here than usual to ensure that we'll - // never conflict with any non-phased resource instance string. This - // is intentionally something that would fail parsing with ParseRef, - // because this special address type should never be exposed in the UI. - return fmt.Sprintf("%s#%s", rp.ResourceInstance, rp.Phase) -} - -func (rp ResourceInstancePhase) UniqueKey() UniqueKey { - return rp // A ResourceInstancePhase is its own UniqueKey -} - -func (rp ResourceInstancePhase) uniqueKeySigil() {} - -// ResourceInstancePhaseType is an enumeration used with ResourceInstancePhase. -type ResourceInstancePhaseType string - -const ( - // ResourceInstancePhaseDestroy represents the "destroy" phase of a - // resource instance. - ResourceInstancePhaseDestroy ResourceInstancePhaseType = "destroy" - - // ResourceInstancePhaseDestroyCBD is similar to ResourceInstancePhaseDestroy - // but is used for resources that have "create_before_destroy" set, thus - // requiring a different dependency ordering. - ResourceInstancePhaseDestroyCBD ResourceInstancePhaseType = "destroy-cbd" -) - -func (rpt ResourceInstancePhaseType) String() string { - return string(rpt) -} - -// ResourcePhase is a special kind of reference used only internally -// during graph building to represent resources that are in a -// non-primary state. -// -// Graph nodes can declare themselves referenceable via a resource phase -// or can declare that they reference a resource phase in order to accomodate -// secondary graph nodes dealing with, for example, destroy actions. -// -// Since resources (as opposed to instances) aren't actually phased, this -// address type is used only as an approximation during initial construction -// of the resource-oriented plan graph, under the assumption that resource -// instances with ResourceInstancePhase addresses will be created in dynamic -// subgraphs during the graph walk. -// -// This special reference type cannot be accessed directly by end-users, and -// should never be shown in the UI. -type ResourcePhase struct { - referenceable - Resource Resource - Phase ResourceInstancePhaseType -} - -var _ Referenceable = ResourcePhase{} - -// Phase returns a special "phase address" for the receving instance. See the -// documentation of ResourceInstancePhase for the limited situations where this -// is intended to be used. -func (r Resource) Phase(rpt ResourceInstancePhaseType) ResourcePhase { - return ResourcePhase{ - Resource: r, - Phase: rpt, - } -} - -func (rp ResourcePhase) String() string { - // We use a different separator here than usual to ensure that we'll - // never conflict with any non-phased resource instance string. This - // is intentionally something that would fail parsing with ParseRef, - // because this special address type should never be exposed in the UI. - return fmt.Sprintf("%s#%s", rp.Resource, rp.Phase) -} - -func (rp ResourcePhase) UniqueKey() UniqueKey { - return rp // A ResourcePhase is its own UniqueKey -} - -func (rp ResourcePhase) uniqueKeySigil() {} diff --git a/internal/terraform/node_resource_apply_instance.go b/internal/terraform/node_resource_apply_instance.go index f4b618fcd7..7abf2999df 100644 --- a/internal/terraform/node_resource_apply_instance.go +++ b/internal/terraform/node_resource_apply_instance.go @@ -54,36 +54,7 @@ func (n *NodeApplyableResourceInstance) CreateAddr() *addrs.AbsResourceInstance // GraphNodeReferencer, overriding NodeAbstractResourceInstance func (n *NodeApplyableResourceInstance) References() []*addrs.Reference { - // Start with the usual resource instance implementation - ret := n.NodeAbstractResourceInstance.References() - - // Applying a resource must also depend on the destruction of any of its - // dependencies, since this may for example affect the outcome of - // evaluating an entire list of resources with "count" set (by reducing - // the count). - // - // However, we can't do this in create_before_destroy mode because that - // would create a dependency cycle. We make a compromise here of requiring - // changes to be updated across two applies in this case, since the first - // plan will use the old values. - if !n.CreateBeforeDestroy() { - for _, ref := range ret { - switch tr := ref.Subject.(type) { - case addrs.ResourceInstance: - newRef := *ref // shallow copy so we can mutate - newRef.Subject = tr.Phase(addrs.ResourceInstancePhaseDestroy) - newRef.Remaining = nil // can't access attributes of something being destroyed - ret = append(ret, &newRef) - case addrs.Resource: - newRef := *ref // shallow copy so we can mutate - newRef.Subject = tr.Phase(addrs.ResourceInstancePhaseDestroy) - newRef.Remaining = nil // can't access attributes of something being destroyed - ret = append(ret, &newRef) - } - } - } - - return ret + return n.NodeAbstractResourceInstance.References() } // GraphNodeAttachDependencies diff --git a/internal/terraform/node_resource_destroy.go b/internal/terraform/node_resource_destroy.go index 16b195be7c..17b5b62959 100644 --- a/internal/terraform/node_resource_destroy.go +++ b/internal/terraform/node_resource_destroy.go @@ -81,26 +81,8 @@ func (n *NodeDestroyResourceInstance) ModifyCreateBeforeDestroy(v bool) error { // GraphNodeReferenceable, overriding NodeAbstractResource func (n *NodeDestroyResourceInstance) ReferenceableAddrs() []addrs.Referenceable { - normalAddrs := n.NodeAbstractResourceInstance.ReferenceableAddrs() - destroyAddrs := make([]addrs.Referenceable, len(normalAddrs)) - - phaseType := addrs.ResourceInstancePhaseDestroy - if n.CreateBeforeDestroy() { - phaseType = addrs.ResourceInstancePhaseDestroyCBD - } - - for i, normalAddr := range normalAddrs { - switch ta := normalAddr.(type) { - case addrs.Resource: - destroyAddrs[i] = ta.Phase(phaseType) - case addrs.ResourceInstance: - destroyAddrs[i] = ta.Phase(phaseType) - default: - destroyAddrs[i] = normalAddr - } - } - - return destroyAddrs + // a destroy node is not referenceable + return []addrs.Referenceable{} } // GraphNodeReferencer, overriding NodeAbstractResource diff --git a/internal/terraform/transform_reference.go b/internal/terraform/transform_reference.go index 09109c9ed9..e7881a74aa 100644 --- a/internal/terraform/transform_reference.go +++ b/internal/terraform/transform_reference.go @@ -545,10 +545,6 @@ func (m ReferenceMap) referenceMapKey(path addrs.Module, addr addrs.Referenceabl return m.mapKey(path, ri.ContainingResource()) } - if rip, ok := addr.(addrs.ResourceInstancePhase); ok { - return m.mapKey(path, rip.ContainingResource()) - } - if mcio, ok := addr.(addrs.ModuleCallInstanceOutput); ok { // A module call instance output is a reference to an output of a