From e1563572c4cf1f11426535f2dc49dfc5ff98527e Mon Sep 17 00:00:00 2001
From: Liam Cervante <liam.cervante@hashicorp.com>
Date: Fri, 13 Oct 2023 10:24:15 +0200
Subject: [PATCH] Fix crash on check blocks in modules with no other changes
 (#34067)

---
 .../terraform/context_apply_checks_test.go    | 52 +++++++++++++++++++
 internal/terraform/node_check.go              |  3 ++
 internal/terraform/transform_destroy_edge.go  |  7 +++
 3 files changed, 62 insertions(+)

diff --git a/internal/terraform/context_apply_checks_test.go b/internal/terraform/context_apply_checks_test.go
index 4a90dff266..5a2d2fbd07 100644
--- a/internal/terraform/context_apply_checks_test.go
+++ b/internal/terraform/context_apply_checks_test.go
@@ -744,6 +744,58 @@ check "error" {
 	}
 }
 
+func TestContextChecks_DoesNotPanicOnModuleExpansion(t *testing.T) {
+	// This is a bit of a special test, we're adding it to verify that
+	// https://github.com/hashicorp/terraform/issues/34062 is fixed.
+	//
+	// Essentially we make a check block in a child module that depends on a
+	// resource that has no changes. We don't care about the actual behaviour
+	// of the check block. We just don't want the apply operation to crash.
+
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "panic_at_the_disco" {
+   source = "./panic"
+}
+`,
+		"panic/main.tf": `
+resource "test_object" "object" {
+    test_string = "Hello, world!"
+}
+
+check "check_should_not_panic" {
+    assert {
+         condition     = test_object.object.test_string == "Hello, world!"
+         error_message = "condition violated"
+    }
+}
+`,
+	})
+
+	p := simpleMockProvider()
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(m, states.BuildState(func(state *states.SyncState) {
+		state.SetResourceInstanceCurrent(
+			mustResourceInstanceAddr("module.panic_at_the_disco.test_object.object"),
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"test_string":"Hello, world!"}`),
+				Status:    states.ObjectReady,
+			},
+			mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+		)
+	}), DefaultPlanOpts)
+	assertNoErrors(t, diags)
+
+	_, diags = ctx.Apply(plan, m)
+	assertNoErrors(t, diags)
+}
+
 func validateCheckDiagnostics(t *testing.T, stage string, expectedWarning, expectedError string, actual tfdiags.Diagnostics) bool {
 	if expectedError != "" {
 		if !actual.HasErrors() {
diff --git a/internal/terraform/node_check.go b/internal/terraform/node_check.go
index 0463d77a64..6e0c4ed640 100644
--- a/internal/terraform/node_check.go
+++ b/internal/terraform/node_check.go
@@ -58,6 +58,7 @@ var (
 	_ GraphNodeModulePath        = (*nodeExpandCheck)(nil)
 	_ GraphNodeDynamicExpandable = (*nodeExpandCheck)(nil)
 	_ GraphNodeReferencer        = (*nodeExpandCheck)(nil)
+	_ graphNodeExpandsInstances  = (*nodeExpandCheck)(nil)
 )
 
 // nodeExpandCheck creates child nodes that actually execute the assertions for
@@ -75,6 +76,8 @@ type nodeExpandCheck struct {
 	makeInstance func(addrs.AbsCheck, *configs.Check) dag.Vertex
 }
 
+func (n *nodeExpandCheck) expandsInstances() {}
+
 func (n *nodeExpandCheck) ModulePath() addrs.Module {
 	return n.addr.Module
 }
diff --git a/internal/terraform/transform_destroy_edge.go b/internal/terraform/transform_destroy_edge.go
index 77fffe831c..1ed23fa3f9 100644
--- a/internal/terraform/transform_destroy_edge.go
+++ b/internal/terraform/transform_destroy_edge.go
@@ -307,6 +307,13 @@ func (t *pruneUnusedNodesTransformer) Transform(g *Graph) error {
 			func() {
 				n := nodes[i]
 				switch n := n.(type) {
+
+				case *nodeExpandCheck:
+					// We always execute check blocks, and they never have
+					// anything referencing them. We have to explicitly list
+					// them here otherwise they'll be deleted.
+					return
+
 				case graphNodeTemporaryValue:
 					// root module outputs indicate they are not temporary by
 					// returning false here.