Give suggestions & remind users to use required_providers when provider not in registry

[pselle]

When a user has any module, or child module, that has a ErrRegistryProviderNotKnown error, users are often confused why they don't get inheritance from other modules.

Because Terraform explicitly loads modules as units with their own provider requirements (required_providers), and adds a suggestion of a provider "did you mean" if they have a provider of the same type (but not the default namespace) in their requirements.

This is what the new suggestion looks like in practice (with suggestion):

This is what the new suggestion looks like in practice (without suggestion/no existing provider requirements):

Open to comments on improving this suggestion to be more user-friendly (for lack of a better term)

My hope is that this can address the (very valid!) concerns and comments surrounding issues like #27920 and #27701 . In #27920, Martin suggests an improvement where we could suggest a similar-name provider we may know about, but because of how the code is currently structured, I don't have confidence in picking an accurate (successful) name, so I am suggesting a more general suggestion here.

Fixes #27920
Fixes #27701

[codecov]

Codecov Report

Merging #28014 (05a8456) into main (98899df) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
command/init.go	53.83% <100.00%> (ø)
internal/getproviders/didyoumean.go	55.42% <100.00%> (+1.10%)	⬆️
terraform/evaluate.go	52.47% <0.00%> (-0.42%)	⬇️
states/statefile/version4.go	58.19% <0.00%> (+0.23%)	⬆️
terraform/node_resource_plan.go	98.05% <0.00%> (+1.94%)	⬆️

[jbardin]

This LGTM if it fits with the product goals. I agree it's hard to do something more specific here, so at least having a hint at required_providers is nice.

[alisdair]

This looks like a good improvement to me!

I haven't fully thought through the consequences of this, but did you decide against a type-based lookup of similar providers based on the requirements? For example, we could add something like this right after the call to getproviders.MissingProviderSuggestion:

if alternative == provider && provider.IsDefault() {
	for req := range reqs {
		if req != provider && req.Type == provider.Type {
			alternative = req
			break
		}
	}
}

This results in the "Did you intend to use …?" message being printed in cases like #27920:

[pselle]

@alisdair Thanks Alisdair! Seeing it in practice ... it does look pretty good, and could be a nice addition to the default here. I got stuck thinking I needed to provide a working provider suggestion ... using reqs, there's no guarantee that the provider found in the reqs exists/is good/etc. Any thought on that? ("Seems like it'd be fine" is a valid response).

(thinking: if we found a req that matched, but then that req failed, we'd give an error on both and in one of those errors, we would be suggesting to use the other erroring provider 🙃 )

[alisdair]

if we found a req that matched, but then that req failed, we'd give an error on both and in one of those errors, we would be suggesting to use the other erroring provider

Ah, yes, I hadn't thought of that, and trying to only recommend providers which successfully installed would be much harder.

The case you describe is unfortunate and the result is not very clear, but to me it's not worse than what we have now. For example, if I misspell the scottwinkler/shell provider as scotwinkler/shell, I get:

Initializing provider plugins...
- Finding hashicorp/external versions matching "~> 1.2"...
- Finding scotwinkler/shell versions matching "1.7.7"...
- Finding latest version of hashicorp/shell...
- Installing hashicorp/external v1.2.0...
- Installed hashicorp/external v1.2.0 (signed by HashiCorp)
╷
│ Error: Failed to query available provider packages
│
│ Could not retrieve the list of available versions for provider scotwinkler/shell: provider registry registry.terraform.io does not have a provider named
│ registry.terraform.io/scotwinkler/shell
│
│ All modules should specify their required_providers so that external consumers will get the correct providers when using a module. To see which modules are
│ currently depending on scotwinkler/shell, run the following command:
│     terraform providers
╵

╷
│ Error: Failed to query available provider packages
│
│ Could not retrieve the list of available versions for provider hashicorp/shell: provider registry registry.terraform.io does not have a provider named
│ registry.terraform.io/hashicorp/shell
│
│ Did you intend to use scotwinkler/shell? If so, you must specify that source address in each module which requires that provider. To see which modules are
│ currently depending on hashicorp/shell, run the following command:
│     terraform providers
╵

[apparentlymart]

Yeah, indeed my thinking with what I was considering in that comment over in #27920 is that if there's a non-hashicorp/ provider in the set of requirements then we can assume that somebody intentionally wrote it out (because there's no default for that situation) and so it's more likely that they intended to type the same thing in both modules than to use the non-existing HashiCorp one alongside it, separately from the fact that the thing they wrote out might not have been valid itself.

By structuring this a bit differently I suppose we could consider only providers that successfully installed, but we'd need to do it as a followup extra message at the end of the whole install process rather than as part of the main error message, because we might not have tried to install the non-hashicorp/ one yet.

While a single message covering both problems would be ideal, I agree with @alisdair that the pair of error messages is still an improvement over what happens today, and I think the worst case if someone takes it totally literally is that they put the typo source address in their other module too and then they will still get the other error about the provider not existing to prompt a second debugging step.

With that said, I think it would be most effective if we include something like the heuristic I described in my comment over there so that we only produce this additional hint in the case where the failing provider is a default provider and there's another provider of the same type elsewhere in the configuration, because then the extra hint would appear only on the error message it applies to and not be a red herring for the secondary problem:

╷
│ Error: Failed to query available provider packages
│
│ Could not retrieve the list of available versions for provider scotwinkler/shell: provider registry registry.terraform.io does not have a provider named
│ registry.terraform.io/scotwinkler/shell.
╵

╷
│ Error: Failed to query available provider packages
│
│ Could not retrieve the list of available versions for provider hashicorp/shell: provider registry registry.terraform.io does not have a provider named
│ registry.terraform.io/hashicorp/shell
│
│ Did you intend to use scotwinkler/shell? If so, you must specify that source address in each module which requires that provider. To see which modules are
│ currently depending on hashicorp/shell, run the following command:
│     terraform providers
╵

I'd hope that by not including the "did you mean" in the first of these we won't bias the user towards thinking that a missing dependency is the problem, and thus give them more room to consider the possibility of a typo as the cause.

[pselle]

Thanks for the comments @alisdair and @apparentlymart! I've added the suggestion and integrated the addition into the tests. I am leaving the "You need required_providers always" default message (when there is no suggestion) because I think it is generally useful, and I'd rather provide another signpost than not.

[alisdair]

✅ from me!

I was a bit surprised to see the switch from IsDefault to directly inspecting the provider addr namespace. More details in an inline comment, but it's totally possible I'm missing something and I'm leaving a checkmark in case that's so.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.