Repository: opentofu/opentofu
File: internal/tofu/node_resource_apply.go (Go)

Code before:
// NodeApplyableResource nodes into their respective modules.
type nodeExpandApplyableResource struct {
	*NodeAbstractResource
}

var (
	_ GraphNodeDestroyerCBD         = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeReferenceable        = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeReferencer           = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeConfigResource       = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeAttachResourceConfig = (*nodeExpandApplyableResource)(nil)
	_ graphNodeExpandsInstances     = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeTargetable           = (*nodeExpandApplyableResource)(nil)
)

func (n *nodeExpandApplyableResource) expandsInstances() {
}

// CreateBeforeDestroy implementation for GraphNodeDestroyerCBD
func (n *nodeExpandApplyableResource) CreateBeforeDestroy() bool {
	// If we have no config, we just assume no
	if n.Config == nil || n.Config.Managed == nil {
		return false
	}

	return n.Config.Managed.CreateBeforeDestroy
}

// ModifyCreateBeforeDestroy implementation for GraphNodeDestroyerCBD actually does nothing for this node type.
func (n *nodeExpandApplyableResource) ModifyCreateBeforeDestroy(_ bool) error {
	return nil
}

func (n *nodeExpandApplyableResource) References() []*addrs.Reference {
	refs := n.NodeAbstractResource.References()

	// The expand node needs to connect to the individual resource instances it
	// references, but cannot refer to it's own instances without causing
	// cycles. It would be preferable to entirely disallow self references
	// without the `self` identifier, but those were allowed in provisioners
	// for compatibility with legacy configuration. We also can't always just
	// filter them out for all resource node types, because the only method we
	// have for catching certain invalid configurations are the cycles that
	// result from these inter-instance references.
	return filterSelfRefs(n.Addr.Resource, refs)
}

func (n *nodeExpandApplyableResource) Name() string {
	return n.NodeAbstractResource.Name() + " (expand)"
}


Review comment (suggestion):
Typically the apply phase tries to use information from the _plan_ rather than information from the config, where possible.

In the plan model [we distinguish between the two different kinds of replace](https://github.com/opentofu/opentofu/blob/9a86d86f4d2f3d43ce6078d25ba5d3d262aec690/internal/plans/action.go#L15-L16) -- `CreateThenDelete` corresponds to what the configuration language calls "create before destroy" -- and so I wonder if we should prefer to use that to make the decision here, to make sure that we're always consistent with what was decided during the planning phase and thus hopefully the two can't become inconsistent again in future.

An interesting consequence of doing that is that the plan should already have recorded a `CreateThenDelete` action for any resource instance that was subjected to `ModifyCreateBeforeDestroy(true)` during the planning phase, which would potentially make the "forced CBD transformer" redundant during the apply phase... but I think that's pretty consistent with the fact that your `ModifyCreateBeforeDestroy` implementation below is a no-op, since by the time we're in the apply phase we should already know _for sure_ whether we're doing `CreateThenDelete` or `DeleteThenCreate`, due to the work done by `ForcedCBDTransformer` during the plan phase. :thinking: 


Code after:
// NodeApplyableResource nodes into their respective modules.
type nodeExpandApplyableResource struct {
	*NodeAbstractResource
}

var (
	_ GraphNodeReferenceable        = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeReferencer           = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeConfigResource       = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeAttachResourceConfig = (*nodeExpandApplyableResource)(nil)
	_ graphNodeExpandsInstances     = (*nodeExpandApplyableResource)(nil)
	_ GraphNodeTargetable           = (*nodeExpandApplyableResource)(nil)
)

func (n *nodeExpandApplyableResource) expandsInstances() {
}

func (n *nodeExpandApplyableResource) References() []*addrs.Reference {
	refs := n.NodeAbstractResource.References()

	// The expand node needs to connect to the individual resource instances it
	// references, but cannot refer to it's own instances without causing
	// cycles. It would be preferable to entirely disallow self references
	// without the `self` identifier, but those were allowed in provisioners
	// for compatibility with legacy configuration. We also can't always just
	// filter them out for all resource node types, because the only method we
	// have for catching certain invalid configurations are the cycles that
	// result from these inter-instance references.
	return filterSelfRefs(n.Addr.Resource, refs)
}

func (n *nodeExpandApplyableResource) Name() string {
	return n.NodeAbstractResource.Name() + " (expand)"
}

func (n *nodeExpandApplyableResource) Execute(ctx EvalContext, op walkOperation) tfdiags.Diagnostics {
	var diags tfdiags.Diagnostics
	expander := ctx.InstanceExpander()
	moduleInstances := expander.ExpandModule(n.Addr.Module)
	for _, module := range moduleInstances {
		ctx = ctx.WithPath(module)
		diags = diags.Append(n.writeResourceState(ctx, n.Addr.Resource.Absolute(module)))
	}

	return diags
}