Validations should only be run during non-destroy operations (#3131)

Signed-off-by: Christian Mesh <christianmesh1@gmail.com>

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
 

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,

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},

internal/tofu/graph_builder_plan.go

@@ -248,7 +248,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		},
 
 		&pruneUnusedNodesTransformer{
-			skip: b.Operation != walkPlanDestroy,
+			Op: b.Operation,
 		},
 
 		// Target

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
 }
 

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
 }
 

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()
 }

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
 }

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 {
+func (n *nodeVariableReference) temporaryValue(op walkOperation) bool {
-	return len(n.Config.Validations) == 0
+	return len(n.Config.Validations) == 0 || op == walkDestroy || op == walkPlanDestroy
 }
 
 // GraphNodeDynamicExpandable

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
 							}

internal/tofu/transform_destroy_edge_test.go

@@ -411,7 +411,7 @@ func TestPruneUnusedNodesTransformer_rootModuleOutputValues(t *testing.T) {
 			},
 			&ReferenceTransformer{},
 			&AttachDependenciesTransformer{},
-			&pruneUnusedNodesTransformer{},
+			&pruneUnusedNodesTransformer{Op: walkDestroy},
 			&CloseRootModuleTransformer{},
 		},
 	}

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
 		}