Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Avoid duplicated warnings #36579

Merged
merged 11 commits into from
Mar 5, 2025
5 changes: 5 additions & 0 deletions .changes/v1.12/BUG FIXES-20250226-171815.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BUG FIXES
body: 'Avoid reporting duplicate attribute-associated diagnostics, such as "Available Write-only Attribute Alternative"'
time: 2025-02-26T17:18:15.521208Z
custom:
Issue: "36579"
6 changes: 4 additions & 2 deletions internal/backend/local/backend_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (b *Local) opPlan(
b.ContextOpts = new(terraform.ContextOpts)
}

// Get our context
// Set up backend and get our context
lr, configSnap, opState, ctxDiags := b.localRun(op)
diags = diags.Append(ctxDiags)
if ctxDiags.HasErrors() {
Expand Down Expand Up @@ -120,7 +120,9 @@ func (b *Local) opPlan(
// NOTE: We intentionally don't stop here on errors because we always want
// to try to present a partial plan report and, if the user chose to,
// generate a partial saved plan file for external analysis.
diags = diags.Append(planDiags)
// Plan() may produce some diagnostic warnings which were already
// produced when setting up context above, so we deduplicate them here.
diags = diags.AppendWithoutDuplicates(planDiags...)

// Even if there are errors we need to handle anything that may be
// contained within the plan, so only exit if there is no data at all.
Expand Down
4 changes: 1 addition & 3 deletions internal/backend/remote-state/s3/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1935,9 +1935,7 @@ func TestBackendConfig_Proxy(t *testing.T) {
raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config))
b := raw.(*Backend)

if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" {
t.Errorf("unexpected diagnostics difference: %s", diff)
}
tfdiags.AssertDiagnosticsMatch(t, diags, tc.expectedDiags)

client := b.awsConfig.HTTPClient
bClient, ok := client.(*awshttp.BuildableClient)
Expand Down
97 changes: 96 additions & 1 deletion internal/terraform/context_plan_ephemeral_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package terraform

import (
"fmt"
"path/filepath"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -63,6 +64,11 @@ module "child" {
Severity: hcl.DiagError,
Summary: "Ephemeral value not allowed",
Detail: "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "child", "main.tf"),
Start: hcl.Pos{Line: 3, Column: 13, Byte: 30},
End: hcl.Pos{Line: 3, Column: 31, Byte: 48},
},
})
},
},
Expand Down Expand Up @@ -109,6 +115,11 @@ resource "test_object" "test" {
Severity: hcl.DiagError,
Summary: "Invalid use of ephemeral value",
Detail: `Ephemeral values are not valid for "test_string", because it is not a write-only attribute and must be persisted to state.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 6, Column: 17, Byte: 88},
End: hcl.Pos{Line: 6, Column: 52, Byte: 123},
},
})
},
},
Expand Down Expand Up @@ -164,13 +175,17 @@ resource "test_object" "test" {
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: `The given "for_each" value is derived from an ephemeral value, which means that Terraform cannot persist it between plan/apply rounds. Use only non-ephemeral values to specify a resource's instance keys.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 4, Column: 14, Byte: 83},
End: hcl.Pos{Line: 4, Column: 55, Byte: 124},
},
})
},
},

"resource expansion - count": {
module: map[string]string{

"main.tf": `
ephemeral "ephem_resource" "data" {}
resource "test_object" "test" {
Expand All @@ -184,6 +199,11 @@ resource "test_object" "test" {
Severity: hcl.DiagError,
Summary: "Invalid count argument",
Detail: `The given "count" is derived from an ephemeral value, which means that Terraform cannot persist it between plan/apply rounds. Use only non-ephemeral values to specify the number of resource instances.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 4, Column: 11, Byte: 80},
End: hcl.Pos{Line: 4, Column: 53, Byte: 122},
},
})
},
},
Expand All @@ -210,6 +230,11 @@ module "child" {
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: `The given "for_each" value is derived from an ephemeral value, which means that Terraform cannot persist it between plan/apply rounds. Use only non-ephemeral values to specify a resource's instance keys.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 5, Column: 16, Byte: 71},
End: hcl.Pos{Line: 5, Column: 57, Byte: 112},
},
})
},
},
Expand All @@ -234,6 +259,11 @@ module "child" {
Severity: hcl.DiagError,
Summary: "Invalid count argument",
Detail: `The given "count" is derived from an ephemeral value, which means that Terraform cannot persist it between plan/apply rounds. Use only non-ephemeral values to specify the number of resource instances.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 4, Column: 13, Byte: 67},
End: hcl.Pos{Line: 4, Column: 55, Byte: 109},
},
})
},
},
Expand Down Expand Up @@ -261,6 +291,11 @@ resource "test_object" "test" {
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: `The given "for_each" value is derived from an ephemeral value, which means that Terraform cannot persist it between plan/apply rounds. Use only non-ephemeral values to specify a resource's instance keys.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 11, Column: 16, Byte: 207},
End: hcl.Pos{Line: 11, Column: 57, Byte: 248},
},
},
)
},
Expand Down Expand Up @@ -288,6 +323,11 @@ module "child" {
Severity: hcl.DiagError,
Summary: "Ephemeral value not allowed",
Detail: "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "child", "main.tf"),
Start: hcl.Pos{Line: 6, Column: 13, Byte: 132},
End: hcl.Pos{Line: 6, Column: 64, Byte: 183},
},
})
},
},
Expand Down Expand Up @@ -322,6 +362,11 @@ module "child" {
Severity: hcl.DiagError,
Summary: "Ephemeral value not allowed",
Detail: "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "child", "main.tf"),
Start: hcl.Pos{Line: 14, Column: 13, Byte: 245},
End: hcl.Pos{Line: 14, Column: 78, Byte: 310},
},
})
},
},
Expand Down Expand Up @@ -378,17 +423,32 @@ check "check_using_ephemeral_value" {
Severity: hcl.DiagWarning,
Summary: "Check block assertion failed",
Detail: "Fine to persist",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 6, Column: 17, Byte: 104},
End: hcl.Pos{Line: 6, Column: 60, Byte: 147},
},
})
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.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 10, Column: 17, Byte: 217},
End: hcl.Pos{Line: 10, Column: 60, Byte: 260},
},
})
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.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 11, Column: 21, Byte: 281},
End: hcl.Pos{Line: 11, Column: 83, Byte: 343},
},
})
return diags
},
Expand Down Expand Up @@ -451,14 +511,29 @@ module "child" {
Severity: hcl.DiagError,
Summary: "Ephemeral value not allowed",
Detail: "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "child", "main.tf"),
Start: hcl.Pos{Line: 15, Column: 13, Byte: 376},
End: hcl.Pos{Line: 15, Column: 33, Byte: 396},
},
}, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Ephemeral value not allowed",
Detail: "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "child", "main.tf"),
Start: hcl.Pos{Line: 18, Column: 13, Byte: 435},
End: hcl.Pos{Line: 18, Column: 31, Byte: 453},
},
}, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Ephemeral value not allowed",
Detail: "This output value is not declared as returning an ephemeral value, so it cannot be set to a result derived from an ephemeral value.",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "child", "main.tf"),
Start: hcl.Pos{Line: 21, Column: 13, Byte: 491},
End: hcl.Pos{Line: 21, Column: 30, Byte: 508},
},
})
},
},
Expand All @@ -483,6 +558,11 @@ ephemeral "ephem_resource" "data" {
Severity: hcl.DiagError,
Summary: "Resource precondition failed",
Detail: "value should not be 2",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 8, Column: 19, Byte: 116},
End: hcl.Pos{Line: 8, Column: 40, Byte: 137},
},
})
},
},
Expand All @@ -507,6 +587,11 @@ ephemeral "ephem_resource" "data" {
Severity: hcl.DiagError,
Summary: "Resource postcondition failed",
Detail: `value should be "pass"`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 8, Column: 19, Byte: 117},
End: hcl.Pos{Line: 8, Column: 39, Byte: 137},
},
})
},
},
Expand Down Expand Up @@ -541,12 +626,22 @@ output "out" {
Detail: fmt.Sprintf(`The error message included a sensitive value, so it will not be displayed.

This was checked by the validation rule at %s.`, m.Module.Variables["ephem"].Validations[0].DeclRange.String()),
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 2, Column: 1, Byte: 1},
End: hcl.Pos{Line: 2, Column: 17, Byte: 17},
},
}).Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Error message refers to ephemeral values",
Detail: `The error expression used to explain this condition refers to ephemeral values. Terraform will not display the resulting message.

You can correct this by removing references to ephemeral values, or by carefully using the ephemeralasnull() function if the expression will not reveal the ephemeral data.`,
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 8, Column: 21, Byte: 142},
End: hcl.Pos{Line: 8, Column: 76, Byte: 197},
},
})
},
},
Expand Down
1 change: 1 addition & 0 deletions internal/terraform/node_resource_plan_partialexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ func (n *nodePlannablePartialExpandedResource) managedResourceExecute(ctx EvalCo
}

unmarkedConfigVal, _ := configVal.UnmarkDeep()
log.Printf("[TRACE] Validating partially expanded config for %q", n.addr)
validateResp := provider.ValidateResourceConfig(
providers.ValidateResourceConfigRequest{
TypeName: n.addr.Resource().Type,
Expand Down
2 changes: 2 additions & 0 deletions internal/terraform/node_resource_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package terraform

import (
"fmt"
"log"
"strings"

"github.com/hashicorp/hcl/v2"
Expand Down Expand Up @@ -390,6 +391,7 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag

// Use unmarked value for validate request
unmarkedConfigVal, _ := configVal.UnmarkDeep()
log.Printf("[TRACE] Validating config for %q", n.Addr)
req := providers.ValidateResourceConfigRequest{
TypeName: n.Config.Type,
Config: unmarkedConfigVal,
Expand Down
5 changes: 4 additions & 1 deletion internal/terraform/terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config
func testModuleInline(t testing.TB, sources map[string]string) *configs.Config {
t.Helper()

cfgPath := t.TempDir()
cfgPath, err := filepath.EvalSymlinks(t.TempDir())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

off-topic -- wondering what necessitated this? Asking bc of the EvalSymlinks on Windows problems, so trying to figure out where the legitimate uses are.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's somewhat annoying but the easiest way of making tests pass for me.

Specifically, some logic inside of Plan() and Validate() evaluates symlinks and so if there are symlinks in the way, we end up with different paths than those that come from t.TempDir().

In my case t.TempDir() points somewhere to /var/tmp/... whereas /var is a symlink to /private/var. I believe this is default on macOS.

if err != nil {
t.Fatal(err)
}

for path, configStr := range sources {
dir := filepath.Dir(path)
Expand Down
21 changes: 8 additions & 13 deletions internal/tfdiags/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import "github.com/google/go-cmp/cmp"
// DiagnosticComparer returns a cmp.Option that can be used with
// the package github.com/google/go-cmp/cmp.
//
// The comparer checks these match between the diagnostics:
// 1) Severity
// 2) Description
// 3) Attribute cty.Path, if present
// The comparer relies on the underlying Diagnostic implementing
// [ComparableDiagnostic].
//
// Example usage:
//
Expand All @@ -20,18 +18,15 @@ var DiagnosticComparer cmp.Option = cmp.Comparer(diagnosticComparerSimple)
// diagnosticComparerSimple returns false when a difference is identified between
// the two Diagnostic arguments.
func diagnosticComparerSimple(l, r Diagnostic) bool {
if l.Severity() != r.Severity() {
return false
}
if l.Description() != r.Description() {
ld, ok := l.(ComparableDiagnostic)
if !ok {
return false
}

// Do the diagnostics originate from the same attribute name, if any?
lp := GetAttribute(l)
rp := GetAttribute(r)
if len(lp) != len(rp) {
rd, ok := r.(ComparableDiagnostic)
if !ok {
return false
}
return lp.Equals(rp)

return ld.Equals(rd)
}
21 changes: 0 additions & 21 deletions internal/tfdiags/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,27 +81,6 @@ func TestDiagnosticComparer(t *testing.T) {
}(),
expectDiff: true,
},
// Scenarios where diagnostics will be considered equavalent, even if they aren't fully the same
"reports that diagnostics match even if sources (Subject) are different; ignored in simple comparison": {
diag1: hclDiagnostic{&baseError},
diag2: func() Diagnostic {
d := baseError
d.Subject = &hcl.Range{
Filename: "foobar.tf",
Start: hcl.Pos{
Line: 0,
Column: 0,
Byte: 0,
},
End: hcl.Pos{
Line: 1,
Column: 1,
Byte: 1,
},
}
return hclDiagnostic{&d}
}(),
},
"reports that diagnostics match even if sources (Context) are different; ignored in simple comparison": {
diag1: hclDiagnostic{&baseError},
diag2: func() Diagnostic {
Expand Down
Loading