diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go index 865f4dda6dff..3e0766054e1e 100644 --- a/internal/backend/remote-state/s3/backend_test.go +++ b/internal/backend/remote-state/s3/backend_test.go @@ -362,7 +362,7 @@ func TestBackendConfig_InvalidRegion(t *testing.T) { confDiags := b.Configure(configSchema) diags = diags.Append(confDiags) - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } }) @@ -528,7 +528,7 @@ func TestBackendConfig_DynamoDBEndpoint(t *testing.T) { raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config)) b := raw.(*Backend) - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } @@ -616,7 +616,8 @@ func TestBackendConfig_IAMEndpoint(t *testing.T) { pathString(cty.GetAttrPath("iam_endpoint")), pathString(cty.GetAttrPath("endpoints").GetAttr("iam")), ), - )}, + ), + }, }, "envvar": { vars: map[string]string{ @@ -660,7 +661,7 @@ func TestBackendConfig_IAMEndpoint(t *testing.T) { _, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config)) - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } }) @@ -736,7 +737,8 @@ func TestBackendConfig_S3Endpoint(t *testing.T) { pathString(cty.GetAttrPath("endpoint")), pathString(cty.GetAttrPath("endpoints").GetAttr("s3")), ), - )}, + ), + }, }, "envvar": { vars: map[string]string{ @@ -783,7 +785,7 @@ func TestBackendConfig_S3Endpoint(t *testing.T) { raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config)) b := raw.(*Backend) - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } @@ -943,7 +945,7 @@ func TestBackendConfig_EC2MetadataEndpoint(t *testing.T) { raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config)) b := raw.(*Backend) - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } @@ -1435,7 +1437,7 @@ func TestBackendConfig_PrepareConfigValidation(t *testing.T) { _, valDiags := b.PrepareConfig(populateSchema(t, b.ConfigSchema(), tc.config)) - if diff := cmp.Diff(valDiags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(valDiags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } }) @@ -2497,7 +2499,7 @@ func TestBackendConfigKmsKeyId(t *testing.T) { diags = diags.Append(confDiags) } - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Fatalf("unexpected diagnostics difference: %s", diff) } @@ -2627,7 +2629,7 @@ func TestBackendSSECustomerKey(t *testing.T) { diags = diags.Append(confDiags) } - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Fatalf("unexpected diagnostics difference: %s", diff) } @@ -3287,7 +3289,7 @@ func TestAssumeRole_PrepareConfigValidation(t *testing.T) { var diags tfdiags.Diagnostics validateNestedAttribute(assumeRoleSchema, config, path, &diags) - if diff := cmp.Diff(diags, tc.expectedDiags, tfdiags.DiagnosticComparer); diff != "" { + if diff := cmp.Diff(diags, tc.expectedDiags, diagnosticComparer); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } }) diff --git a/internal/backend/remote-state/s3/testing_test.go b/internal/backend/remote-state/s3/testing_test.go new file mode 100644 index 000000000000..13d3056ee0da --- /dev/null +++ b/internal/backend/remote-state/s3/testing_test.go @@ -0,0 +1,28 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package s3 + +import ( + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +var diagnosticComparer cmp.Option = cmp.Comparer(diagnosticComparerS3) + +// diagnosticComparerS3 is a Comparer function for use with cmp.Diff to compare two tfdiags.Diagnostic values +func diagnosticComparerS3(l, r tfdiags.Diagnostic) bool { + if l.Severity() != r.Severity() { + return false + } + if l.Description() != r.Description() { + return false + } + + lp := tfdiags.GetAttribute(l) + rp := tfdiags.GetAttribute(r) + if len(lp) != len(rp) { + return false + } + return lp.Equals(rp) +} diff --git a/internal/tfdiags/contextual.go b/internal/tfdiags/contextual.go index 7f2bd5e5df38..3eedf82bfefe 100644 --- a/internal/tfdiags/contextual.go +++ b/internal/tfdiags/contextual.go @@ -99,6 +99,9 @@ func GetAttribute(d Diagnostic) cty.Path { return nil } +var _ Diagnostic = &attributeDiagnostic{} +var _ ComparableDiagnostic = &attributeDiagnostic{} + type attributeDiagnostic struct { diagnosticBase attrPath cty.Path @@ -384,6 +387,9 @@ func WholeContainingBody(severity Severity, summary, detail string) Diagnostic { } } +var _ Diagnostic = &wholeBodyDiagnostic{} +var _ ComparableDiagnostic = &wholeBodyDiagnostic{} + type wholeBodyDiagnostic struct { diagnosticBase subject *SourceRange // populated only after ElaborateFromConfigBody diff --git a/internal/tfdiags/diagnostic_base.go b/internal/tfdiags/diagnostic_base.go index 6c0db08280b4..e942db7ad61b 100644 --- a/internal/tfdiags/diagnostic_base.go +++ b/internal/tfdiags/diagnostic_base.go @@ -15,6 +15,13 @@ type diagnosticBase struct { address string } +var _ Diagnostic = &diagnosticBase{} + +// diagnosticBase doesn't implement ComparableDiagnostic because the lack of source data +// means separate diagnostics might be falsely identified as equal. This poses a user-facing +// risk if deduplication of diagnostics removes a diagnostic that's incorrectly been identified +// as a duplicate via comparison. + func (d diagnosticBase) Severity() Severity { return d.severity } diff --git a/internal/tfdiags/hcl.go b/internal/tfdiags/hcl.go index 3531237477f8..5db8c6ad1cfb 100644 --- a/internal/tfdiags/hcl.go +++ b/internal/tfdiags/hcl.go @@ -13,6 +13,7 @@ type hclDiagnostic struct { } var _ Diagnostic = hclDiagnostic{} +var _ ComparableDiagnostic = hclDiagnostic{} func (d hclDiagnostic) Severity() Severity { switch d.diag.Severity { diff --git a/internal/tfdiags/rpc_friendly.go b/internal/tfdiags/rpc_friendly.go index 223a21aa6046..69ccee8305ce 100644 --- a/internal/tfdiags/rpc_friendly.go +++ b/internal/tfdiags/rpc_friendly.go @@ -15,6 +15,9 @@ type rpcFriendlyDiag struct { Context_ *SourceRange } +var _ Diagnostic = &rpcFriendlyDiag{} +var _ ComparableDiagnostic = &rpcFriendlyDiag{} + // rpcFriendlyDiag transforms a given diagnostic so that is more friendly to // RPC. //