From 2cfa404236d9f209ea220f16a6217bde18c39eb2 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 25 Aug 2025 12:47:35 +0200 Subject: [PATCH] test: don't panic when resolving references that haven't been evaluated --- .changes/v1.13/BUG FIXES-20250825-125018.yaml | 5 +++ internal/command/test_test.go | 5 +++ .../expect-failures-assertions/child/main.tf | 12 ++++++ .../test/expect-failures-assertions/main.tf | 36 ++++++++++++++++ .../main.tftest.hcl | 41 +++++++++++++++++++ internal/instances/expander.go | 29 +++++++++++++ internal/terraform/evaluate.go | 22 +++++++++- 7 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 .changes/v1.13/BUG FIXES-20250825-125018.yaml create mode 100644 internal/command/testdata/test/expect-failures-assertions/child/main.tf create mode 100644 internal/command/testdata/test/expect-failures-assertions/main.tf create mode 100644 internal/command/testdata/test/expect-failures-assertions/main.tftest.hcl diff --git a/.changes/v1.13/BUG FIXES-20250825-125018.yaml b/.changes/v1.13/BUG FIXES-20250825-125018.yaml new file mode 100644 index 0000000000..1d915d7d5a --- /dev/null +++ b/.changes/v1.13/BUG FIXES-20250825-125018.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'terraform test: prevent panic when resolving incomplete references' +time: 2025-08-25T12:50:18.511449+02:00 +custom: + Issue: "37484" diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 5fea43fa92..64c97610a7 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -402,6 +402,11 @@ func TestTest_Runs(t *testing.T) { expectedOut: []string{"3 passed, 0 failed."}, code: 0, }, + "expect-failures-assertions": { + expectedOut: []string{"0 passed, 1 failed."}, + expectedErr: []string{"Test assertion failed"}, + code: 1, + }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { diff --git a/internal/command/testdata/test/expect-failures-assertions/child/main.tf b/internal/command/testdata/test/expect-failures-assertions/child/main.tf new file mode 100644 index 0000000000..e2c2f2df61 --- /dev/null +++ b/internal/command/testdata/test/expect-failures-assertions/child/main.tf @@ -0,0 +1,12 @@ + +variable "input" { + type = string +} + +resource "test_resource" "resource" { + value = var.input +} + +output "output" { + value = test_resource.resource.value +} diff --git a/internal/command/testdata/test/expect-failures-assertions/main.tf b/internal/command/testdata/test/expect-failures-assertions/main.tf new file mode 100644 index 0000000000..f471a1d237 --- /dev/null +++ b/internal/command/testdata/test/expect-failures-assertions/main.tf @@ -0,0 +1,36 @@ + +variable "input" { + type = string + + validation { + condition = var.input == "allow" + error_message = "invalid input value" + } +} + +variable "followup" { + type = string + default = "allow" + + validation { + condition = var.followup == var.input + error_message = "followup must match input" + } +} + +locals { + input = var.followup +} + +module "child" { + source = "./child" + input = var.input +} + +resource "test_resource" "resource" { + value = local.input +} + +output "output" { + value = var.input +} diff --git a/internal/command/testdata/test/expect-failures-assertions/main.tftest.hcl b/internal/command/testdata/test/expect-failures-assertions/main.tftest.hcl new file mode 100644 index 0000000000..8fc9171d0e --- /dev/null +++ b/internal/command/testdata/test/expect-failures-assertions/main.tftest.hcl @@ -0,0 +1,41 @@ + +// this test runs assertions againsts parts of the module that should not +// have executed because of the expected failure. this should be an error +// in the test, but it shouldn't panic or anything like that. + +run "fail" { + variables { + input = "deny" + } + + command = plan + + expect_failures = [ + var.input, + ] + + assert { + condition = var.followup == "deny" + error_message = "bad input" + } + + assert { + condition = local.input == "deny" + error_message = "bad local" + } + + assert { + condition = module.child.output == "deny" + error_message = "bad module output" + } + + assert { + condition = test_resource.resource.value == "deny" + error_message = "bad resource value" + } + + assert { + condition = output.output == "deny" + error_message = "bad output" + } +} diff --git a/internal/instances/expander.go b/internal/instances/expander.go index 099faf0fa0..5804c23828 100644 --- a/internal/instances/expander.go +++ b/internal/instances/expander.go @@ -181,6 +181,21 @@ func (e *Expander) ExpandAbsModuleCall(addr addrs.AbsModuleCall) (keyType addrs. return keyType, instKeys, true } +// AbsModuleCallExpanded checks if the specified module call has been visited +// and expanded previously. +func (e *Expander) AbsModuleCallExpanded(addr addrs.AbsModuleCall) bool { + e.mu.RLock() + defer e.mu.RUnlock() + + expParent, ok := e.findModule(addr.Module) + if !ok { + return false + } + + _, ok = expParent.moduleCalls[addr.Call] + return ok +} + // expandModule allows skipping unexpanded module addresses by setting skipUnregistered to true. // This is used by instances.Set, which is only concerned with the expanded // instances, and should not panic when looking up unknown addresses. @@ -450,6 +465,20 @@ func (e *Expander) ResourceInstanceKeys(addr addrs.AbsResource) (keyType addrs.I return exp.instanceKeys() } +// ResourceInstanceExpanded checks if the specified resource has been visited +// and expanded previously. +func (e *Expander) ResourceInstanceExpanded(addr addrs.AbsResource) bool { + e.mu.RLock() + defer e.mu.RUnlock() + + parentMod, known := e.findModule(addr.Module) + if !known { + return false + } + _, ok := parentMod.resources[addr.Resource] + return ok +} + // AllInstances returns a set of all of the module and resource instances known // to the expander. // diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 19ab9d4188..6a1cb5163c 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -342,8 +342,11 @@ func (d *evaluationStateData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.S return cty.DynamicVal, diags } - val := d.Evaluator.NamedValues.GetLocalValue(addr.Absolute(d.ModulePath)) - return val, diags + if target := addr.Absolute(d.ModulePath); d.Evaluator.NamedValues.HasLocalValue(target) { + return d.Evaluator.NamedValues.GetLocalValue(addr.Absolute(d.ModulePath)), diags + } + + return cty.DynamicVal, diags } func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { @@ -541,6 +544,21 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // result for _all_ of its work, rather than continuing to duplicate a bunch // of the logic we've tried to encapsulate over ther already. if d.Operation == walkPlan || d.Operation == walkApply { + if !d.Evaluator.Instances.ResourceInstanceExpanded(addr.Absolute(moduleAddr)) { + // Then we've asked for a resource that hasn't been evaluated yet. + // This means that either something has gone wrong in the graph or + // the console or test command has an errored plan and is attempting + // to load an invalid resource from it. + + unknownVal := cty.DynamicVal + + // If an ephemeral resource is deferred we need to mark the returned unknown value as ephemeral + if addr.Mode == addrs.EphemeralResourceMode { + unknownVal = unknownVal.Mark(marks.Ephemeral) + } + return unknownVal, diags + } + if _, _, hasUnknownKeys := d.Evaluator.Instances.ResourceInstanceKeys(addr.Absolute(moduleAddr)); hasUnknownKeys { // There really isn't anything interesting we can do in this situation, // because it means we have an unknown for_each/count, in which case