Allow cleanup of states that depend on prior runs outputs

internal/command/test_cleanup.go

@@ -25,11 +25,11 @@ func (c *TestCleanupCommand) Help() string {
 	helpText := `
 Usage: terraform [global options] test cleanup [options]
 
-  Cleans up left-over resources created during Terraform test runs.
+  Cleans up left-over resources in states that were created during Terraform test runs.
 
   By default, this command ignores the skip_cleanup attributes in the manifest
-  file. Use the -repair flag to override this behavior. Additionally, the
-  -target flag allows specifying which state files to clean up.
+  file. Use the -repair flag to override this behavior, which will ensure that
+  resources that were intentionally left-over are exempt from cleanup.
 
 Options:
 

internal/command/test_test.go

@@ -829,8 +829,14 @@ main.tftest.hcl/test_three, and they need to be cleaned up manually:
 			}
 		}
 		provider.Provider.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse {
+			var diags tfdiags.Diagnostics
+			// Simulate an error during apply, unless it is a destroy operation
+			if !req.PlannedState.IsNull() {
+				diags = diags.Append(fmt.Errorf("apply error"))
+			}
 			return providers.ApplyResourceChangeResponse{
-				NewState: req.PlannedState,
+				NewState:    req.PlannedState,
+				Diagnostics: diags,
 			}
 		}
 		view, done := testView(t)
@@ -852,12 +858,13 @@ main.tftest.hcl... pass
 
 Success!`
 		if diff := cmp.Diff(expectedCleanup, output.Stdout()); diff != "" {
-			t.Fatalf("unexpected cleanup output: expected %s\n, got %s\n, diff: %s", expectedCleanup, output.Stdout(), diff)
+			t.Errorf("unexpected cleanup output: expected %s\n, got %s\n, diff: %s", expectedCleanup, output.Stdout(), diff)
 		}
 
 		expectedStates := map[string][]string{}
+		actualStates := actualStates()
 
-		if diff := cmp.Diff(expectedStates, actualStates()); diff != "" {
+		if diff := cmp.Diff(expectedStates, actualStates); diff != "" {
 			t.Fatalf("unexpected states after cleanup: %s", diff)
 		}
 	})

internal/command/testdata/test/cleanup/main.tf

@@ -2,6 +2,11 @@ variable "id" {
   type = string
 }
 
+variable "unused" {
+  type = bool
+  default = false
+}
+
 variable "destroy_fail" {
   type    = bool
   default = false
@@ -15,3 +20,7 @@ resource "test_resource" "resource" {
 output "id" {
   value = test_resource.resource.id
 }
+
+output "unused" {
+  value = var.unused
+}

internal/command/testdata/test/cleanup/main.tftest.hcl

@@ -8,6 +8,7 @@ run "test_two" {
   skip_cleanup = true # This will leave behind the state
   variables {
     id = "test_two"
+    unused = run.test.unused
   }
 }
 

internal/moduletest/graph/node_state_cleanup.go

@@ -31,6 +31,10 @@ type NodeStateCleanup struct {
 	// If applyOverride is provided, it will be applied to the state file to reach
 	// the final state, instead of running the destroy operation.
 	applyOverride *moduletest.Run
+
+	// if true, we are in repair mode and should not clean up state that was
+	// intentionally left-over.
+	repair bool
 }
 
 func (n *NodeStateCleanup) Name() string {
@@ -42,6 +46,11 @@ func (n *NodeStateCleanup) Name() string {
 // would prevent further cleanup of other states. Instead, any diagnostics
 // will be rendered directly to ensure the cleanup process continues.
 func (n *NodeStateCleanup) Execute(evalCtx *EvalContext) tfdiags.Diagnostics {
+	n.execute(evalCtx)
+	return nil
+}
+
+func (n *NodeStateCleanup) execute(evalCtx *EvalContext) tfdiags.Diagnostics {
 	file := n.opts.File
 	state := evalCtx.GetFileState(n.stateKey)
 	log.Printf("[TRACE] TestStateManager: cleaning up state for %s", file.Name)
@@ -51,7 +60,7 @@ func (n *NodeStateCleanup) Execute(evalCtx *EvalContext) tfdiags.Diagnostics {
 
 	// If the state was loaded as a result of an intentional skip, we
 	// don't need to clean it up when in repair mode.
-	if state.Reason == ReasonSkip && evalCtx.repair {
+	if n.repair && state.Reason == ReasonSkip {
 		return nil
 	}
 
@@ -95,8 +104,7 @@ func (n *NodeStateCleanup) Execute(evalCtx *EvalContext) tfdiags.Diagnostics {
 		diags = diags.Append(evalCtx.WriteFileState(n.stateKey, state))
 		evalCtx.Renderer().DestroySummary(diags, nil, file, state.State)
 
-		// intentionally return nil to allow further cleanup
-		return nil
+		return diags
 	}
 	TransformConfigForRun(evalCtx, state.Run, file)
 

internal/moduletest/graph/node_test_run.go

@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/terraform/internal/moduletest"
 	"github.com/hashicorp/terraform/internal/terraform"
 	"github.com/hashicorp/terraform/internal/tfdiags"
+	"github.com/zclconf/go-cty/cty"
 )
 
 var (
@@ -48,10 +49,15 @@ func (n *NodeTestRun) References() []*addrs.Reference {
 // Execute executes the test run block and update the status of the run block
 // based on the result of the execution.
 func (n *NodeTestRun) Execute(evalCtx *EvalContext) tfdiags.Diagnostics {
-	log.Printf("[TRACE] TestFileRunner: executing run block %s/%s", n.File().Name, n.run.Name)
+	if n.opts.CommandMode == moduletest.CleanupMode {
+		n.cleanupModeExecute(evalCtx)
+		return nil
+	}
+
 	startTime := time.Now().UTC()
 	var diags tfdiags.Diagnostics
 	file, run := n.File(), n.run
+	log.Printf("[TRACE] TestFileRunner: executing run block %s/%s", file.Name, run.Name)
 
 	// At the end of the function, we'll update the status of the file based on
 	// the status of the run block, and render the run summary.
@@ -109,6 +115,24 @@ func (n *NodeTestRun) Execute(evalCtx *EvalContext) tfdiags.Diagnostics {
 	return diags
 }
 
+func (n *NodeTestRun) cleanupModeExecute(ctx *EvalContext) {
+	// In cleanup mode, the test run block is not executed.
+	// Instead, the state file associated with the run is revisited to extract
+	// its output values. These output values are then set in the context so
+	// that they can be accessed by subsequent cleanup nodes. This is necessary
+	// because cleaning up the state file for a run may depend on the output
+	// values of previous runs.
+	state := ctx.GetFileState(n.run.Config.StateKey).State
+	if state == nil {
+		return
+	}
+	outputVals := make(map[string]cty.Value, len(state.RootOutputValues))
+	for name, out := range state.RootOutputValues {
+		outputVals[name] = out.Value
+	}
+	ctx.SetOutput(n.run, cty.ObjectVal(outputVals))
+}
+
 func (n *NodeTestRun) execute(ctx *EvalContext, waiter *operationWaiter) {
 	file, run := n.File(), n.run
 	ctx.Renderer().Run(run, file, moduletest.Starting, 0)

internal/moduletest/graph/test_graph_builder.go

@@ -61,17 +61,6 @@ func (b *TestGraphBuilder) Steps(opts *graphOptions) []terraform.GraphTransforme
 		&TestStateCleanupTransformer{opts},
 		terraform.DynamicTransformer(validateRunConfigs),
 		&TestProvidersTransformer{},
-		terraform.DynamicTransformer(func(g *terraform.Graph) error {
-			// If we're in cleanup mode, we can remove the test runs in the graph,
-			// and prevent unnecessary no-op execution.
-			// This will ensure that we only have nodes that are needed for cleanup in the graph.
-			if b.CommandMode == moduletest.CleanupMode {
-				for node := range dag.SelectSeq(g.VerticesSeq(), runFilter) {
-					g.Remove(node)
-				}
-			}
-			return nil
-		}),
 		&CloseTestGraphTransformer{},
 		&terraform.TransitiveReductionTransformer{},
 	}

internal/moduletest/graph/transform_state_cleanup.go

@@ -32,7 +32,7 @@ func (t *TestStateCleanupTransformer) Transform(g *terraform.Graph) error {
 		// Create a cleanup node for each state key
 		key := node.run.Config.StateKey
 		if _, exists := cleanupMap[key]; !exists {
-			cleanupMap[key] = &NodeStateCleanup{stateKey: key, opts: t.opts}
+			cleanupMap[key] = &NodeStateCleanup{stateKey: key, opts: t.opts, repair: t.opts.EvalContext.repair}
 			g.Add(cleanupMap[key])
 		}