aherman
/
terraform
mirror of https://github.com/hashicorp/terraform
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: Avoid duplicated warnings (#36579)

Browse Source 
* fix: Deduplicate attribute diagnostics

* tfdiags: Allow HCL & rpcFriendly diags to be compared

* add changelog entry

* update comment per PR review

* address PR feedback (more precise comparisons)

* add some missing docs

* avoid comparing address

* compare subject range in rpc diags

* Remove Equals method from diagnosticBase

* correct positions in test

* update comparison logic for TestBackendConfig_Proxy
pull/36625/head
Radek Simko 1 year ago committed by GitHub
parent d845df938d
commit c3dc197465
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 286 additions and 41 deletions
Show Stats Download Patch File Download Diff File

5
.changes/v1.12/BUG FIXES-20250226-171815.yaml
Unescape View File

@ -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
internal/backend/local/backend_plan.go
Unescape View File

@ -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() {
@ -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.

4
internal/backend/remote-state/s3/backend_test.go
Unescape View File

@ -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)

97
internal/terraform/context_plan_ephemeral_test.go
Unescape View File

@ -5,6 +5,7 @@ package terraform
import (
"fmt"
"path/filepath"
"testing"
"github.com/google/go-cmp/cmp"
@ -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},
},
})
},
},
@ -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},
},
})
},
},
@ -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" {
@ -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},
},
})
},
},
@ -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},
},
})
},
},
@ -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},
},
})
},
},
@ -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},
},
},
)
},
@ -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},
},
})
},
},
@ -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},
},
})
},
},
@ -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
},
@ -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},
},
})
},
},
@ -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},
},
})
},
},
@ -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},
},
})
},
},
@ -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},
},
})
},
},

1
internal/terraform/node_resource_plan_partialexp.go
Unescape View File

@ -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,

2
internal/terraform/node_resource_validate.go
Unescape View File

@ -5,6 +5,7 @@ package terraform
import (
"fmt"
"log"
"strings"
"github.com/hashicorp/hcl/v2"
@ -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,

5
internal/terraform/terraform_test.go
Unescape View File

@ -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())
if err != nil {
t.Fatal(err)
}
for path, configStr := range sources {
dir := filepath.Dir(path)

21
internal/tfdiags/compare.go
Unescape View File

@ -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:
//
@ -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
internal/tfdiags/compare_test.go
Unescape View File

@ -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 {

63
internal/tfdiags/contextual.go
Unescape View File

@ -205,6 +205,31 @@ func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body, addr string
return &ret
}
func (d *attributeDiagnostic) Equals(otherDiag ComparableDiagnostic) bool {
od, ok := otherDiag.(*attributeDiagnostic)
if !ok {
return false
}
if d.severity != od.severity {
return false
}
if d.summary != od.summary {
return false
}
if d.detail != od.detail {
return false
}
if !d.attrPath.Equals(od.attrPath) {
return false
}
// address can differ between and after expansion
// even though it represents the same attribute
// so we avoid comparing it here
return sourceRangeEquals(d.subject, od.subject)
}
func traversePathSteps(traverse []cty.PathStep, body hcl.Body) hcl.Body {
for i := 0; i < len(traverse); i++ {
step := traverse[i]
@ -386,3 +411,41 @@ func (d *wholeBodyDiagnostic) Source() Source {
Subject: d.subject,
}
}
func (d *wholeBodyDiagnostic) Equals(otherDiag ComparableDiagnostic) bool {
od, ok := otherDiag.(*wholeBodyDiagnostic)
if !ok {
return false
}
if d.severity != od.severity {
return false
}
if d.summary != od.summary {
return false
}
if d.detail != od.detail {
return false
}
// address can differ between and after expansion
// even though it represents the same attribute
// so we avoid comparing it here
return sourceRangeEquals(d.subject, od.subject)
}
func sourceRangeEquals(l, r *SourceRange) bool {
if l == nil || r == nil {
return l == r
}
if l.Filename != r.Filename {
return false
}
if l.Start.Byte != r.Start.Byte {
return false
}
if l.End.Byte != r.End.Byte {
return false
}
return true
}

4
internal/tfdiags/diagnostic.go
Unescape View File

@ -27,6 +27,10 @@ type Diagnostic interface {
ExtraInfo() interface{}
}
type ComparableDiagnostic interface {
Equals(otherDiag ComparableDiagnostic) bool
}
type Severity rune
//go:generate go tool golang.org/x/tools/cmd/stringer -type=Severity

40
internal/tfdiags/diagnostics.go
Unescape View File

@ -83,6 +83,46 @@ func (diags Diagnostics) Append(new ...interface{}) Diagnostics {
return diags
}
// ContainsDiagnostic returns true of a given diagnostic is contained
// within the Diagnostics slice.
// Comparisons are done via [ComparableDiagnostic].
func (diags Diagnostics) ContainsDiagnostic(diag ComparableDiagnostic) bool {
for _, d := range diags {
if cd, ok := d.(ComparableDiagnostic); ok && diag.Equals(cd) {
return true
}
}
return false
}
// AppendWithoutDuplicates appends a Diagnostic unless one is already contained
// according to [ContainsDiagnostic], i.e. based on [ComparableDiagnostic].
func (diags Diagnostics) AppendWithoutDuplicates(newDiags ...Diagnostic) Diagnostics {
for _, newItem := range newDiags {
if newItem == nil {
continue
}
cd, ok := newItem.(ComparableDiagnostic)
if !ok {
// append what we cannot compare
diags = diags.Append(newItem)
}
if diags.ContainsDiagnostic(cd) {
continue
}
diags = diags.Append(newItem)
}
if len(diags) == 0 {
return nil
}
return diags
}
func diagnosticsForError(err error) []Diagnostic {
if err == nil {
return nil

37
internal/tfdiags/hcl.go
Unescape View File

@ -57,6 +57,43 @@ func (d hclDiagnostic) ExtraInfo() interface{} {
return d.diag.Extra
}
func (d hclDiagnostic) Equals(otherDiag ComparableDiagnostic) bool {
od, ok := otherDiag.(hclDiagnostic)
if !ok {
return false
}
if d.diag.Severity != od.diag.Severity {
return false
}
if d.diag.Summary != od.diag.Summary {
return false
}
if d.diag.Detail != od.diag.Detail {
return false
}
if !hclRangeEquals(d.diag.Subject, od.diag.Subject) {
return false
}
return true
}
func hclRangeEquals(l, r *hcl.Range) bool {
if l == nil || r == nil {
return l == r
}
if l.Filename != r.Filename {
return false
}
if l.Start.Byte != r.Start.Byte {
return false
}
if l.End.Byte != r.End.Byte {
return false
}
return true
}
// SourceRangeFromHCL constructs a SourceRange from the corresponding range
// type within the HCL package.
func SourceRangeFromHCL(hclRange hcl.Range) SourceRange {

21
internal/tfdiags/rpc_friendly.go
Unescape View File

@ -51,6 +51,27 @@ func (d *rpcFriendlyDiag) Source() Source {
}
}
func (d *rpcFriendlyDiag) Equals(otherDiag ComparableDiagnostic) bool {
od, ok := otherDiag.(*rpcFriendlyDiag)
if !ok {
return false
}
if d.Severity_ != od.Severity_ {
return false
}
if d.Summary_ != od.Summary_ {
return false
}
if d.Detail_ != od.Detail_ {
return false
}
if !sourceRangeEquals(d.Subject_, od.Subject_) {
return false
}
return true
}
func (d rpcFriendlyDiag) FromExpr() *FromExpr {
// RPC-friendly diagnostics cannot preserve expression information because
// expressions themselves are not RPC-friendly.

Write Preview
Loading…
Cancel
Save