fix: Avoid duplicated warnings

[radeksimko]

Currently, some warning diagnostics, such as the following are being duplicated during terraform plan

Human Interface

Plan: 2 to add, 0 to change, 0 to destroy.
╷
│ Warning: Available Write-only Attribute Alternative
│
│   with aws_db_instance.test,
│   on main.tf line 30, in resource "aws_db_instance" "test":
│   30:   password            = random_password.password.result
│
│ The attribute password has a write-only alternative password_wo available. Use the write-only alternative of the attribute when possible.
│
│ (and one more similar warning elsewhere)
╵

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't guarantee to take exactly these actions if you run "terraform apply" now.

Machine Interface

{"@level":"info","@message":"Terraform 1.12.0-dev","@module":"terraform.ui","@timestamp":"2025-02-25T19:06:07.599739Z","terraform":"1.12.0-dev","type":"version","ui":"1.2"}
{"@level":"info","@message":"random_password.password: Plan to create","@module":"terraform.ui","@timestamp":"2025-02-25T19:06:09.194367Z","change":{"resource":{"addr":"random_password.password","module":"","resource":"random_password.password","implied_provider":"random","resource_type":"random_password","resource_name":"password","resource_key":null},"action":"create"},"type":"planned_change"}
{"@level":"info","@message":"aws_db_instance.test: Plan to create","@module":"terraform.ui","@timestamp":"2025-02-25T19:06:09.194426Z","change":{"resource":{"addr":"aws_db_instance.test","module":"","resource":"aws_db_instance.test","implied_provider":"aws","resource_type":"aws_db_instance","resource_name":"test","resource_key":null},"action":"create"},"type":"planned_change"}
{"@level":"info","@message":"Plan: 2 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","@timestamp":"2025-02-25T19:06:09.194431Z","changes":{"add":2,"change":0,"import":0,"remove":0,"operation":"plan"},"type":"change_summary"}
{"@level":"warn","@message":"Warning: Available Write-only Attribute Alternative","@module":"terraform.ui","@timestamp":"2025-02-25T19:06:09.194604Z","diagnostic":{"severity":"warning","summary":"Available Write-only Attribute Alternative","detail":"The attribute password has a write-only alternative password_wo available. Use the write-only alternative of the attribute when possible.","address":"aws_db_instance.test","range":{"filename":"main.tf","start":{"line":30,"column":25,"byte":638},"end":{"line":30,"column":56,"byte":669}},"snippet":{"context":"resource \"aws_db_instance\" \"test\"","code":"  password            = random_password.password.result","start_line":30,"highlight_start_offset":24,"highlight_end_offset":55,"values":[]}},"type":"diagnostic"}
{"@level":"warn","@message":"Warning: Available Write-only Attribute Alternative","@module":"terraform.ui","@timestamp":"2025-02-25T19:06:09.194801Z","diagnostic":{"severity":"warning","summary":"Available Write-only Attribute Alternative","detail":"The attribute password has a write-only alternative password_wo available. Use the write-only alternative of the attribute when possible.","address":"aws_db_instance.test","range":{"filename":"main.tf","start":{"line":30,"column":25,"byte":638},"end":{"line":30,"column":56,"byte":669}},"snippet":{"context":"resource \"aws_db_instance\" \"test\"","code":"  password            = random_password.password.result","start_line":30,"highlight_start_offset":24,"highlight_end_offset":55,"values":[]}},"type":"diagnostic"}

---

This PR aims to address duplicates such as the one shown above.

As mentioned elsewhere I am not particularly excited about the nature of the solution, where we filter out duplicates. I think ideally we just shouldn't be producing the duplicates but considering the complexity we are dealing with - where the many diagnostics can be produced in many different places and in different goroutines - I find it relatively difficult to implement such a solution in any reasonable time.

On a more positive note, we already have business of comparing diagnostics for equality in tests and so we can now make use of this new logic I'm adding in the PR.

Target Release

1.12.x

CHANGELOG entry

• This change is user-facing and I added a changelog entry.
• This change is not user-facing.

[jbardin]

Looks good! I think this will be a good fix for some of the annoying repeated diagnostics we already have and had no way to effectively filter. I would just add some unit tests for equality/inequality.

[jbardin]

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

[radeksimko]

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.

[jbardin]

since diagnosticBase can't implement reliable equality because it doesn't contain the location data, we should probably remove this method

[radeksimko]

That would mean sourceless diagnostics become incomparable - what do you suggest we do in tests that need to test for equality then?

terraform/internal/tfdiags/sourceless.go

Lines 10 to 16 in 43ee22f

	func Sourceless(severity Severity, summary, detail string) Diagnostic {
	return diagnosticBase{
	severity: severity,
	summary: summary,
	detail: detail,
	}
	}

[radeksimko]

On a second look I don't see any test failures anymore - not sure where I saw them before. Either way, I removed the comparison method.

[radeksimko]

I don't know why I cannot reproduce those failures locally but the CI does still show 4 failing tests.

We can implement a separate comparison with cmp.Diff for those tests I suppose - does that sound like a sensible way forward?

[SarahFrench]

TestBackendConfig_Proxy currently uses cmp.Diff directly, does anything change if tfdiags.AssertDiagnosticsMatch is used instead?

Edit: It looks like the test passes if the test helper is used instead, which is due to the helper converting diags to RPC-friendly diags before comparison.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.