diff --git a/CHANGELOG.md b/CHANGELOG.md index 87e77bbf52..871835545e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ BUG FIXES: * The `issensitive` function now returns an unknown result when its argument is unknown, since a sensitive unknown value can potentially become non-sensitive once more information is available. ([#3008](https://github.com/opentofu/opentofu/pull/3008)) * Provider references like "null.some_alias[each.key]" in .tf.json files are now correctly parsed ([#2915](https://github.com/opentofu/opentofu/issues/2915)) * Fixed crash when processing multiple deprecated marks on a complex object ([#3105](https://github.com/opentofu/opentofu/pull/3105)) +* Variables with validation no longer interfere with the destroy process ([#3131](https://github.com/opentofu/opentofu/pull/3131)) ## Previous Releases diff --git a/internal/tofu/context_apply2_test.go b/internal/tofu/context_apply2_test.go index ccb7207f3f..191ee1205d 100644 --- a/internal/tofu/context_apply2_test.go +++ b/internal/tofu/context_apply2_test.go @@ -4820,14 +4820,19 @@ variable "root_var" { error_message = "${local.err} root" } } +data "test_data_source" "res_parent" { +} +data "test_data_source" "res" { + count = length(data.test_data_source.res_parent.id) +} module "mod" { source = "./mod" - mod_var = local.value + mod_var = local.value + length(data.test_data_source.res[0].id) } `, "mod/mod.tofu": ` locals { - expected = 10 + expected = 21 err = "error" } variable "mod_var" { @@ -4883,7 +4888,24 @@ variable "other_var" { t.Run("valid", func(t *testing.T) { input := InputValuesFromCaller(map[string]cty.Value{"root_var": cty.NumberIntVal(10)}) - ctx := testContext2(t, &ContextOpts{}) + provider := testProvider("test") + + provider.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("data_source"), + "foo": cty.StringVal("ok"), + }), + } + } + + ps := map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(provider), + } + + ctx := testContext2(t, &ContextOpts{ + Providers: ps, + }) plan, diags := ctx.Plan(context.Background(), valid, nil, &PlanOpts{ SetVariables: input, diff --git a/internal/tofu/graph_builder_apply.go b/internal/tofu/graph_builder_apply.go index d3a5cd6331..68026737c8 100644 --- a/internal/tofu/graph_builder_apply.go +++ b/internal/tofu/graph_builder_apply.go @@ -194,7 +194,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // We need to remove configuration nodes that are not used at all, as // they may not be able to evaluate, especially during destroy. // These include variables, locals, and instance expanders. - &pruneUnusedNodesTransformer{}, + &pruneUnusedNodesTransformer{ + Op: b.Operation, + }, // Target &TargetingTransformer{Targets: b.Targets, Excludes: b.Excludes}, diff --git a/internal/tofu/graph_builder_plan.go b/internal/tofu/graph_builder_plan.go index 96ee3f748a..d4626a5df3 100644 --- a/internal/tofu/graph_builder_plan.go +++ b/internal/tofu/graph_builder_plan.go @@ -248,7 +248,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { }, &pruneUnusedNodesTransformer{ - skip: b.Operation != walkPlanDestroy, + Op: b.Operation, }, // Target diff --git a/internal/tofu/node_local.go b/internal/tofu/node_local.go index 29b20f5a28..b7a979e41e 100644 --- a/internal/tofu/node_local.go +++ b/internal/tofu/node_local.go @@ -39,7 +39,7 @@ var ( func (n *nodeExpandLocal) expandsInstances() {} // graphNodeTemporaryValue -func (n *nodeExpandLocal) temporaryValue() bool { +func (n *nodeExpandLocal) temporaryValue(_ walkOperation) bool { return true } @@ -103,7 +103,7 @@ var ( ) // graphNodeTemporaryValue -func (n *NodeLocal) temporaryValue() bool { +func (n *NodeLocal) temporaryValue(_ walkOperation) bool { return true } diff --git a/internal/tofu/node_module_variable.go b/internal/tofu/node_module_variable.go index 88b71eed4a..b058fc882b 100644 --- a/internal/tofu/node_module_variable.go +++ b/internal/tofu/node_module_variable.go @@ -41,7 +41,7 @@ var ( func (n *nodeExpandModuleVariable) expandsInstances() {} -func (n *nodeExpandModuleVariable) temporaryValue() bool { +func (n *nodeExpandModuleVariable) temporaryValue(_ walkOperation) bool { return true } @@ -130,7 +130,7 @@ var ( _ dag.GraphNodeDotter = (*nodeModuleVariable)(nil) ) -func (n *nodeModuleVariable) temporaryValue() bool { +func (n *nodeModuleVariable) temporaryValue(_ walkOperation) bool { return true } diff --git a/internal/tofu/node_output.go b/internal/tofu/node_output.go index 6575993b58..798f187f84 100644 --- a/internal/tofu/node_output.go +++ b/internal/tofu/node_output.go @@ -52,7 +52,7 @@ var ( func (n *nodeExpandOutput) expandsInstances() {} -func (n *nodeExpandOutput) temporaryValue() bool { +func (n *nodeExpandOutput) temporaryValue(_ walkOperation) bool { // non root outputs are temporary return !n.Module.IsRoot() } @@ -220,7 +220,7 @@ var ( _ dag.GraphNodeDotter = (*NodeApplyableOutput)(nil) ) -func (n *NodeApplyableOutput) temporaryValue() bool { +func (n *NodeApplyableOutput) temporaryValue(_ walkOperation) bool { // this must always be evaluated if it is a root module output return !n.Addr.Module.IsRoot() } @@ -499,7 +499,7 @@ func (n *NodeDestroyableOutput) ModulePath() addrs.Module { return n.Addr.Module.Module() } -func (n *NodeDestroyableOutput) temporaryValue() bool { +func (n *NodeDestroyableOutput) temporaryValue(_ walkOperation) bool { // this must always be evaluated if it is a root module output return !n.Addr.Module.IsRoot() } diff --git a/internal/tofu/node_value.go b/internal/tofu/node_value.go index 55b1fb46e2..137676edf5 100644 --- a/internal/tofu/node_value.go +++ b/internal/tofu/node_value.go @@ -11,5 +11,5 @@ package tofu // A boolean return value allows a node which may need to be saved to // conditionally do so. type graphNodeTemporaryValue interface { - temporaryValue() bool + temporaryValue(op walkOperation) bool } diff --git a/internal/tofu/node_variable_reference.go b/internal/tofu/node_variable_reference.go index 235f22442b..c2dbd47752 100644 --- a/internal/tofu/node_variable_reference.go +++ b/internal/tofu/node_variable_reference.go @@ -45,8 +45,8 @@ var ( func (n *nodeVariableReference) expandsInstances() {} // Abuse graphNodeTemporaryValue to keep the validation rule around -func (n *nodeVariableReference) temporaryValue() bool { - return len(n.Config.Validations) == 0 +func (n *nodeVariableReference) temporaryValue(op walkOperation) bool { + return len(n.Config.Validations) == 0 || op == walkDestroy || op == walkPlanDestroy } // GraphNodeDynamicExpandable diff --git a/internal/tofu/transform_destroy_edge.go b/internal/tofu/transform_destroy_edge.go index 578520715b..ec9309408f 100644 --- a/internal/tofu/transform_destroy_edge.go +++ b/internal/tofu/transform_destroy_edge.go @@ -287,11 +287,11 @@ type pruneUnusedNodesTransformer struct { // destroy. Planing normally involves all nodes, but during a destroy plan // we may need to prune things which are in the configuration but do not // exist in state to evaluate. - skip bool + Op walkOperation } func (t *pruneUnusedNodesTransformer) Transform(_ context.Context, g *Graph) error { - if t.skip { + if t.Op != walkPlanDestroy && t.Op != walkDestroy && t.Op != walkApply { return nil } @@ -314,7 +314,7 @@ func (t *pruneUnusedNodesTransformer) Transform(_ context.Context, g *Graph) err case graphNodeTemporaryValue: // root module outputs indicate they are not temporary by // returning false here. - if !n.temporaryValue() { + if !n.temporaryValue(t.Op) { log.Printf("[TRACE] pruneUnusedNodes: temporary value vertex %q kept because it's not a temporary value vertex", dag.VertexName(n)) return } @@ -342,7 +342,7 @@ func (t *pruneUnusedNodesTransformer) Transform(_ context.Context, g *Graph) err // root module, and so it's not actually important // to expand it and so this lets us do a bit more // pruning than we'd be able to do otherwise. - if tmp, ok := v.(graphNodeTemporaryValue); ok && !tmp.temporaryValue() { + if tmp, ok := v.(graphNodeTemporaryValue); ok && !tmp.temporaryValue(t.Op) { log.Printf("[TRACE] pruneUnusedNodes: expanding vertex %q kept because another expanding vertex %q with non-temporary value is one of its dependencies", dag.VertexName(n), dag.VertexName(v)) continue } diff --git a/internal/tofu/transform_destroy_edge_test.go b/internal/tofu/transform_destroy_edge_test.go index 3ae3bebdd5..29e4775ba1 100644 --- a/internal/tofu/transform_destroy_edge_test.go +++ b/internal/tofu/transform_destroy_edge_test.go @@ -411,7 +411,7 @@ func TestPruneUnusedNodesTransformer_rootModuleOutputValues(t *testing.T) { }, &ReferenceTransformer{}, &AttachDependenciesTransformer{}, - &pruneUnusedNodesTransformer{}, + &pruneUnusedNodesTransformer{Op: walkDestroy}, &CloseRootModuleTransformer{}, }, } diff --git a/internal/tofu/transform_targets.go b/internal/tofu/transform_targets.go index 744bd3f362..6ec2509dc8 100644 --- a/internal/tofu/transform_targets.go +++ b/internal/tofu/transform_targets.go @@ -204,7 +204,8 @@ func (t *TargetingTransformer) getTargetedOutputNodes(targetedNodes dag.Set, gra // root module outputs indicate that while they are an output type, // they not temporary and will return false here. - if tv.temporaryValue() { + // We use walkInvalid here as we only care about the op as a workaround for nodeVariableReference, which does not apply here + if tv.temporaryValue(walkInvalid) { continue }