required_providers: warn on legacy version syntax, missing source

[bendrucker]

Tightens enforcement of required_providers:

• Rejects the legacy string syntax from Terraform 0.12.25 and earlier in favor of the object syntax which includes a source
• Enforces that source is defined with the object syntax

This is a breaking change, since previously these legacy syntaxes were accepted. It seems appropriate to focus on enforcement for modern TF versions and stop accommodating old/unsupported TF versions. While omitting a version constraint can lead to major issues down the road, omitting source is also a common hazard, albeit more of an immediate one.

https://developer.hashicorp.com/terraform/language/providers/requirements#source-addresses

If you omit the source argument..., Terraform uses an implied source address... This is a backward compatibility feature... in modules that require 0.13 or later, we recommend using explicit source addresses for all providers.

Closes #62

[wata727]

Thank you for working on this.

Agreed that we should warn against the legacy syntax and omitted sources, but I wonder if these should be enforced as a default for all users. Perhaps many users only use official providers such as AWS or Google, and explicitly declaring the source can simply feel redundant.

Nonetheless, since the behavior of omitting the source is clearly explained as a backward compatibility feature, the user should originally declare the source. The issue is that when updating TFLint, all users will have to manually fix every required_providers declaration by hand.

I believe that providing an auto-fix is the desired behavior when forcing such a fix for low-impact legacy syntax, as Terraform provided the 0.12upgrade subcommand. See terraform-linters/tflint#266

Another option might be to provide it as another rule or control the behavior with an option of this rule.

What do you think?

[bendrucker]

I'd hesitate to split out too many individual rules, enforcing source and version are things we could make configuration options for this rule that users could disable. That at least allows for a quick way to globally revert to the prior behavior if they don't want to update.

Auto-fixing would be nice if we had the framework to offer it. Arguably it should make a network call to check that the given provider exists in the hashicorp namespace before writing anything out.

The cost of omitting source is that users who go to add a third-party provider don't recognize the need to specify it and get a lengthy error about hashicorp/<my-provider> not existing.

[wata727]

OK. Agreed with this change.

[bendrucker]

I'll add config options for source and version before merging this so that users can optionally enforce one or the other. Setting source = false would revert to the previous behavior but version = false has concrete uses that aren't just preserving legacy behavior. In a test module you might want to have the latest version of every provider but still enforce the presence of source.

[wata727]

👍