From 13d8671db8e583efb678da118a3bfade9773b983 Mon Sep 17 00:00:00 2001 From: Ilia Gogotchuri Date: Thu, 4 Sep 2025 20:19:20 +0400 Subject: [PATCH] Rename the interface `graphNodeExpandsInstances` (#3237) Signed-off-by: Ilia Gogotchuri --- internal/tofu/instance_expanders.go | 19 +++++++++++++++---- internal/tofu/node_local.go | 12 ++++++------ internal/tofu/node_module_expand.go | 7 +++++-- internal/tofu/node_module_variable.go | 14 +++++++------- internal/tofu/node_output.go | 14 +++++++------- internal/tofu/node_resource_apply.go | 16 ++++++++++------ internal/tofu/node_resource_plan.go | 22 +++++++++++----------- internal/tofu/node_variable_reference.go | 14 +++++++------- internal/tofu/transform_destroy_edge.go | 4 ++-- 9 files changed, 70 insertions(+), 52 deletions(-) diff --git a/internal/tofu/instance_expanders.go b/internal/tofu/instance_expanders.go index e695aefc24..106b23e95f 100644 --- a/internal/tofu/instance_expanders.go +++ b/internal/tofu/instance_expanders.go @@ -5,8 +5,19 @@ package tofu -// graphNodeExpandsInstances is implemented by nodes that causes instances to -// be registered in the instances.Expander. -type graphNodeExpandsInstances interface { - expandsInstances() +// graphNodeRetainedByPruneUnusedNodesTransformer is a marker interface. +// A Node implementing this interface means it should not be pruned during pruneUnusedNodesTransformer. +// This interface is primarily used by nodes that expand instances but not restricted to them. +// (Two node types implementing this interface without implementing GraphNodeDynamicExpandable are +// nodeExpandModule and nodeExpandApplyableResource) +// +// Keep in mind that due to how this is used by pruneUnusedNodesTransformer, the node must have `UpEdges` +// (Graph vertices coming into it), one of which must be either of the following: +// GraphNodeProvider, GraphNodeResourceInstance, GraphNodeDynamicExpandable or graphNodeRetainedByPruneUnusedNodesTransformer itself +// to be retained during pruning. +// nodeExpandCheck is a good example of a node supposed to be implementing this interface, but due to the aforementioned +// limitation in the current implementation is not. +// More details in this issue https://github.com/opentofu/opentofu/issues/2808 +type graphNodeRetainedByPruneUnusedNodesTransformer interface { + retainDuringUnusedPruning() } diff --git a/internal/tofu/node_local.go b/internal/tofu/node_local.go index b7a979e41e..e37a6a6d87 100644 --- a/internal/tofu/node_local.go +++ b/internal/tofu/node_local.go @@ -29,14 +29,14 @@ type nodeExpandLocal struct { } var ( - _ GraphNodeReferenceable = (*nodeExpandLocal)(nil) - _ GraphNodeReferencer = (*nodeExpandLocal)(nil) - _ GraphNodeDynamicExpandable = (*nodeExpandLocal)(nil) - _ graphNodeTemporaryValue = (*nodeExpandLocal)(nil) - _ graphNodeExpandsInstances = (*nodeExpandLocal)(nil) + _ GraphNodeReferenceable = (*nodeExpandLocal)(nil) + _ GraphNodeReferencer = (*nodeExpandLocal)(nil) + _ GraphNodeDynamicExpandable = (*nodeExpandLocal)(nil) + _ graphNodeTemporaryValue = (*nodeExpandLocal)(nil) + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeExpandLocal)(nil) ) -func (n *nodeExpandLocal) expandsInstances() {} +func (n *nodeExpandLocal) retainDuringUnusedPruning() {} // graphNodeTemporaryValue func (n *nodeExpandLocal) temporaryValue(_ walkOperation) bool { diff --git a/internal/tofu/node_module_expand.go b/internal/tofu/node_module_expand.go index c3f8a57135..de33538dc2 100644 --- a/internal/tofu/node_module_expand.go +++ b/internal/tofu/node_module_expand.go @@ -21,6 +21,8 @@ type ConcreteModuleNodeFunc func(n *nodeExpandModule) dag.Vertex // nodeExpandModule represents a module call in the configuration that // might expand into multiple module instances depending on how it is // configured. +// +// "Expand" in the name refers to instances.Expander usage. This node doesn't generate a dynamic subgraph. type nodeExpandModule struct { Addr addrs.Module Config *configs.Module @@ -31,10 +33,11 @@ var ( _ GraphNodeExecutable = (*nodeExpandModule)(nil) _ GraphNodeReferencer = (*nodeExpandModule)(nil) _ GraphNodeReferenceOutside = (*nodeExpandModule)(nil) - _ graphNodeExpandsInstances = (*nodeExpandModule)(nil) + // nodeExpandModule needs to be retained during unused nodes pruning to register the module in instances.Expander + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeExpandModule)(nil) ) -func (n *nodeExpandModule) expandsInstances() {} +func (n *nodeExpandModule) retainDuringUnusedPruning() {} func (n *nodeExpandModule) Name() string { return n.Addr.String() + " (expand)" diff --git a/internal/tofu/node_module_variable.go b/internal/tofu/node_module_variable.go index b058fc882b..37ed93c251 100644 --- a/internal/tofu/node_module_variable.go +++ b/internal/tofu/node_module_variable.go @@ -31,15 +31,15 @@ type nodeExpandModuleVariable struct { } var ( - _ GraphNodeDynamicExpandable = (*nodeExpandModuleVariable)(nil) - _ GraphNodeReferenceOutside = (*nodeExpandModuleVariable)(nil) - _ GraphNodeReferenceable = (*nodeExpandModuleVariable)(nil) - _ GraphNodeReferencer = (*nodeExpandModuleVariable)(nil) - _ graphNodeTemporaryValue = (*nodeExpandModuleVariable)(nil) - _ graphNodeExpandsInstances = (*nodeExpandModuleVariable)(nil) + _ GraphNodeDynamicExpandable = (*nodeExpandModuleVariable)(nil) + _ GraphNodeReferenceOutside = (*nodeExpandModuleVariable)(nil) + _ GraphNodeReferenceable = (*nodeExpandModuleVariable)(nil) + _ GraphNodeReferencer = (*nodeExpandModuleVariable)(nil) + _ graphNodeTemporaryValue = (*nodeExpandModuleVariable)(nil) + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeExpandModuleVariable)(nil) ) -func (n *nodeExpandModuleVariable) expandsInstances() {} +func (n *nodeExpandModuleVariable) retainDuringUnusedPruning() {} func (n *nodeExpandModuleVariable) temporaryValue(_ walkOperation) bool { return true diff --git a/internal/tofu/node_output.go b/internal/tofu/node_output.go index 798f187f84..86267dd173 100644 --- a/internal/tofu/node_output.go +++ b/internal/tofu/node_output.go @@ -42,15 +42,15 @@ type nodeExpandOutput struct { } var ( - _ GraphNodeReferenceable = (*nodeExpandOutput)(nil) - _ GraphNodeReferencer = (*nodeExpandOutput)(nil) - _ GraphNodeReferenceOutside = (*nodeExpandOutput)(nil) - _ GraphNodeDynamicExpandable = (*nodeExpandOutput)(nil) - _ graphNodeTemporaryValue = (*nodeExpandOutput)(nil) - _ graphNodeExpandsInstances = (*nodeExpandOutput)(nil) + _ GraphNodeReferenceable = (*nodeExpandOutput)(nil) + _ GraphNodeReferencer = (*nodeExpandOutput)(nil) + _ GraphNodeReferenceOutside = (*nodeExpandOutput)(nil) + _ GraphNodeDynamicExpandable = (*nodeExpandOutput)(nil) + _ graphNodeTemporaryValue = (*nodeExpandOutput)(nil) + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeExpandOutput)(nil) ) -func (n *nodeExpandOutput) expandsInstances() {} +func (n *nodeExpandOutput) retainDuringUnusedPruning() {} func (n *nodeExpandOutput) temporaryValue(_ walkOperation) bool { // non root outputs are temporary diff --git a/internal/tofu/node_resource_apply.go b/internal/tofu/node_resource_apply.go index 30f2532546..53f0f55e97 100644 --- a/internal/tofu/node_resource_apply.go +++ b/internal/tofu/node_resource_apply.go @@ -13,9 +13,11 @@ import ( ) // nodeExpandApplyableResource handles the first layer of resource -// expansion during apply. Even though the resource instances themselves are -// already expanded from the plan, we still need to expand the -// NodeApplyableResource nodes into their respective modules. +// expansion during apply. Even though the resource instances themselves are already expanded from the plan, +// we still need to register resource nodes in instances.Expander with their absolute addresses +// based on the expanded module instances. +// +// "Expand" in the name refers to instances.Expander usage. This node doesn't generate a dynamic subgraph. type nodeExpandApplyableResource struct { *NodeAbstractResource } @@ -26,11 +28,13 @@ var ( _ GraphNodeReferencer = (*nodeExpandApplyableResource)(nil) _ GraphNodeConfigResource = (*nodeExpandApplyableResource)(nil) _ GraphNodeAttachResourceConfig = (*nodeExpandApplyableResource)(nil) - _ graphNodeExpandsInstances = (*nodeExpandApplyableResource)(nil) - _ GraphNodeTargetable = (*nodeExpandApplyableResource)(nil) + // nodeExpandApplyableResource needs to be retained during unused nodes pruning + // to register the resource for expanded module instances in `instances.Expander` + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeExpandApplyableResource)(nil) + _ GraphNodeTargetable = (*nodeExpandApplyableResource)(nil) ) -func (n *nodeExpandApplyableResource) expandsInstances() { +func (n *nodeExpandApplyableResource) retainDuringUnusedPruning() { } func (n *nodeExpandApplyableResource) References() []*addrs.Reference { diff --git a/internal/tofu/node_resource_plan.go b/internal/tofu/node_resource_plan.go index 3416743bda..44f3fbc68c 100644 --- a/internal/tofu/node_resource_plan.go +++ b/internal/tofu/node_resource_plan.go @@ -59,23 +59,23 @@ type nodeExpandPlannableResource struct { } var ( - _ GraphNodeDestroyerCBD = (*nodeExpandPlannableResource)(nil) - _ GraphNodeDynamicExpandable = (*nodeExpandPlannableResource)(nil) - _ GraphNodeReferenceable = (*nodeExpandPlannableResource)(nil) - _ GraphNodeReferencer = (*nodeExpandPlannableResource)(nil) - _ GraphNodeConfigResource = (*nodeExpandPlannableResource)(nil) - _ GraphNodeAttachResourceConfig = (*nodeExpandPlannableResource)(nil) - _ GraphNodeAttachDependencies = (*nodeExpandPlannableResource)(nil) - _ GraphNodeTargetable = (*nodeExpandPlannableResource)(nil) - _ graphNodeExpandsInstances = (*nodeExpandPlannableResource)(nil) - _ closableResource = (*nodeExpandPlannableResource)(nil) + _ GraphNodeDestroyerCBD = (*nodeExpandPlannableResource)(nil) + _ GraphNodeDynamicExpandable = (*nodeExpandPlannableResource)(nil) + _ GraphNodeReferenceable = (*nodeExpandPlannableResource)(nil) + _ GraphNodeReferencer = (*nodeExpandPlannableResource)(nil) + _ GraphNodeConfigResource = (*nodeExpandPlannableResource)(nil) + _ GraphNodeAttachResourceConfig = (*nodeExpandPlannableResource)(nil) + _ GraphNodeAttachDependencies = (*nodeExpandPlannableResource)(nil) + _ GraphNodeTargetable = (*nodeExpandPlannableResource)(nil) + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeExpandPlannableResource)(nil) + _ closableResource = (*nodeExpandPlannableResource)(nil) ) func (n *nodeExpandPlannableResource) Name() string { return n.NodeAbstractResource.Name() + " (expand)" } -func (n *nodeExpandPlannableResource) expandsInstances() { +func (n *nodeExpandPlannableResource) retainDuringUnusedPruning() { } // GraphNodeAttachDependencies diff --git a/internal/tofu/node_variable_reference.go b/internal/tofu/node_variable_reference.go index c2dbd47752..f824880f22 100644 --- a/internal/tofu/node_variable_reference.go +++ b/internal/tofu/node_variable_reference.go @@ -34,15 +34,15 @@ type nodeVariableReference struct { } var ( - _ GraphNodeDynamicExpandable = (*nodeVariableReference)(nil) - _ GraphNodeReferenceable = (*nodeVariableReference)(nil) - _ GraphNodeReferencer = (*nodeVariableReference)(nil) - _ graphNodeExpandsInstances = (*nodeVariableReference)(nil) - _ graphNodeTemporaryValue = (*nodeVariableReference)(nil) + _ GraphNodeDynamicExpandable = (*nodeVariableReference)(nil) + _ GraphNodeReferenceable = (*nodeVariableReference)(nil) + _ GraphNodeReferencer = (*nodeVariableReference)(nil) + _ graphNodeRetainedByPruneUnusedNodesTransformer = (*nodeVariableReference)(nil) + _ graphNodeTemporaryValue = (*nodeVariableReference)(nil) ) -// graphNodeExpandsInstances -func (n *nodeVariableReference) expandsInstances() {} +// graphNodeRetainedByPruneUnusedNodesTransformer +func (n *nodeVariableReference) retainDuringUnusedPruning() {} // Abuse graphNodeTemporaryValue to keep the validation rule around func (n *nodeVariableReference) temporaryValue(op walkOperation) bool { diff --git a/internal/tofu/transform_destroy_edge.go b/internal/tofu/transform_destroy_edge.go index ec9309408f..d8a960a87d 100644 --- a/internal/tofu/transform_destroy_edge.go +++ b/internal/tofu/transform_destroy_edge.go @@ -330,12 +330,12 @@ func (t *pruneUnusedNodesTransformer) Transform(_ context.Context, g *Graph) err } } - case graphNodeExpandsInstances: + case graphNodeRetainedByPruneUnusedNodesTransformer: // Any nodes that expand instances are kept when their // instances may need to be evaluated. for _, v := range g.UpEdges(n) { switch v.(type) { - case graphNodeExpandsInstances, GraphNodeDynamicExpandable: + case graphNodeRetainedByPruneUnusedNodesTransformer, GraphNodeDynamicExpandable: // Root module output values or checks (which the following // condition matches) are exempt because we know // there is only ever exactly one instance of the