From 79a35f39688ddb1d89665c11b4e05ef0b1618fd2 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 4 Nov 2024 11:16:13 +0000 Subject: [PATCH 1/2] lang: Add Extra to avoid duplicate diagnostic in check blocks (#35944) --- internal/lang/checks.go | 11 ++++++++++- internal/moduletest/eval_context.go | 2 +- internal/terraform/eval_conditions.go | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/lang/checks.go b/internal/lang/checks.go index 7c6b39818aa3..9ad8d91e04d2 100644 --- a/internal/lang/checks.go +++ b/internal/lang/checks.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" @@ -21,7 +22,7 @@ import ( // It will either return a non-empty message string or it'll return diagnostics // with either errors or warnings that explain why the given expression isn't // acceptable. -func EvalCheckErrorMessage(expr hcl.Expression, hclCtx *hcl.EvalContext) (string, tfdiags.Diagnostics) { +func EvalCheckErrorMessage(expr hcl.Expression, hclCtx *hcl.EvalContext, ruleAddr *addrs.CheckRule) (string, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics val, hclDiags := expr.Value(hclCtx) @@ -73,6 +74,13 @@ You can correct this by removing references to sensitive values, or by carefully } if _, ephemeral := valMarks[marks.Ephemeral]; ephemeral { + var extra interface{} + if ruleAddr != nil { + extra = &addrs.CheckRuleDiagnosticExtra{ + CheckRule: *ruleAddr, + } + } + diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagWarning, Summary: "Error message refers to ephemeral values", @@ -80,6 +88,7 @@ You can correct this by removing references to sensitive values, or by carefully You can correct this by removing references to ephemeral values, or by using the ephemeralasnull() function on the references to not reveal ephemeral data.`, Subject: expr.Range().Ptr(), + Extra: extra, }) return "", diags } diff --git a/internal/moduletest/eval_context.go b/internal/moduletest/eval_context.go index 3822d2b11919..0c4583b706a8 100644 --- a/internal/moduletest/eval_context.go +++ b/internal/moduletest/eval_context.go @@ -109,7 +109,7 @@ func (ec *EvalContext) Evaluate() (Status, cty.Value, tfdiags.Diagnostics) { continue } - errorMessage, moreDiags := lang.EvalCheckErrorMessage(rule.ErrorMessage, hclCtx) + errorMessage, moreDiags := lang.EvalCheckErrorMessage(rule.ErrorMessage, hclCtx, nil) ruleDiags = ruleDiags.Append(moreDiags) runVal, hclDiags := rule.Condition.Value(hclCtx) diff --git a/internal/terraform/eval_conditions.go b/internal/terraform/eval_conditions.go index 09be10951d34..6a221339e439 100644 --- a/internal/terraform/eval_conditions.go +++ b/internal/terraform/eval_conditions.go @@ -103,7 +103,7 @@ func validateCheckRule(addr addrs.CheckRule, rule *configs.CheckRule, ctx EvalCo hclCtx, moreDiags := scope.EvalContext(refs) diags = diags.Append(moreDiags) - errorMessage, moreDiags := lang.EvalCheckErrorMessage(rule.ErrorMessage, hclCtx) + errorMessage, moreDiags := lang.EvalCheckErrorMessage(rule.ErrorMessage, hclCtx, &addr) diags = diags.Append(moreDiags) return errorMessage, hclCtx, diags From 43d616ff3fd2b12dbe454519082170694dcf6314 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 4 Nov 2024 15:38:22 +0000 Subject: [PATCH 2/2] terraform: Update test for check blocks w/ ephemeral values (#35945) --- .../terraform/context_plan_ephemeral_test.go | 57 ++++++++++++++++--- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/internal/terraform/context_plan_ephemeral_test.go b/internal/terraform/context_plan_ephemeral_test.go index 3e56926022fb..5e0af42b9659 100644 --- a/internal/terraform/context_plan_ephemeral_test.go +++ b/internal/terraform/context_plan_ephemeral_test.go @@ -7,6 +7,7 @@ import ( "fmt" "testing" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" @@ -332,15 +333,18 @@ module "child" { }, "check blocks": { - toBeImplemented: true, module: map[string]string{ "main.tf": ` ephemeral "ephem_resource" "data" {} check "check_using_ephemeral_value" { assert { - condition = ephemeral.ephem_resource.data.bool - error_message = "This should not fail" + condition = ephemeral.ephem_resource.data.bool == false + error_message = "Fine to persist" + } + assert { + condition = ephemeral.ephem_resource.data.bool == false + error_message = "Shall not be persisted ${ephemeral.ephem_resource.data.bool}" } } `, @@ -350,10 +354,49 @@ check "check_using_ephemeral_value" { expectCloseEphemeralResourceCalled: true, assertPlan: func(t *testing.T, p *plans.Plan) { - // Checks using ephemeral values should not be included in the plan - if p.Checks.ConfigResults.Len() > 0 { - t.Fatalf("Expected no checks to be included in the plan, but got %d", p.Checks.ConfigResults.Len()) + key := addrs.ConfigCheck{ + Module: addrs.RootModule, + Check: addrs.Check{ + Name: "check_using_ephemeral_value", + }, + } + result, ok := p.Checks.ConfigResults.GetOk(key) + if !ok { + t.Fatalf("expected to find check result for %q", key) + } + objKey := addrs.AbsCheck{ + Module: addrs.RootModuleInstance, + Check: addrs.Check{ + Name: "check_using_ephemeral_value", + }, } + obj, ok := result.ObjectResults.GetOk(objKey) + if !ok { + t.Fatalf("expected to find object for %q", objKey) + } + expectedMessages := []string{"Fine to persist"} + if diff := cmp.Diff(expectedMessages, obj.FailureMessages); diff != "" { + t.Fatalf("unexpected messages: %s", diff) + } + }, + expectPlanDiagnostics: func(m *configs.Config) (diags tfdiags.Diagnostics) { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Check block assertion failed", + Detail: "Fine to persist", + }) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Check block assertion failed", + Detail: "This check failed, but has an invalid error message as described in the other accompanying messages.", + }) + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Error message refers to ephemeral values", + Detail: "The error expression used to explain this condition refers to ephemeral values, so Terraform will not display the resulting message." + + "\n\nYou can correct this by removing references to ephemeral values, or by using the ephemeralasnull() function on the references to not reveal ephemeral data.", + }) + return diags }, }, @@ -510,7 +553,7 @@ ephemeral "ephem_resource" "data" { }, }, Functions: map[string]providers.FunctionDecl{ - "either": providers.FunctionDecl{ + "either": { Parameters: []providers.FunctionParam{ { Name: "a",