From 242f31963872e8f0faeac31ec9c24c29070686b9 Mon Sep 17 00:00:00 2001 From: Pam Selle Date: Thu, 11 Mar 2021 08:54:18 -0500 Subject: [PATCH] Give suggestions & remind users to use required_providers when provider not in registry (#28014) * Add helper suggestion when failed registry err When someone has a failed registry error on init, remind them that they should have required_providers in every module * Give suggestion for a provider based on reqs Suggest another provider on a registry error, from the list of requirements we have on init. This skips the legacy lookup process if there is a similar provider existing in requirements. --- command/e2etest/init_test.go | 10 +++ .../provider-not-found-non-default/main.tf | 7 ++ command/init.go | 4 +- internal/getproviders/didyoumean.go | 11 ++- internal/getproviders/didyoumean_test.go | 83 +++++++++++++++++-- 5 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 command/e2etest/testdata/provider-not-found-non-default/main.tf diff --git a/command/e2etest/init_test.go b/command/e2etest/init_test.go index a3b3a0b08..1f0e3fc53 100644 --- a/command/e2etest/init_test.go +++ b/command/e2etest/init_test.go @@ -344,6 +344,10 @@ func TestInitProviderNotFound(t *testing.T) { if !strings.Contains(oneLineStderr, "provider registry registry.terraform.io does not have a provider named registry.terraform.io/hashicorp/nonexist") { t.Errorf("expected error message is missing from output:\n%s", stderr) } + + if !strings.Contains(oneLineStderr, "All modules should specify their required_providers") { + t.Errorf("expected error message is missing from output:\n%s", stderr) + } }) t.Run("local provider not found", func(t *testing.T) { @@ -375,6 +379,12 @@ func TestInitProviderNotFound(t *testing.T) { │ Could not retrieve the list of available versions for provider │ hashicorp/nonexist: provider registry registry.terraform.io does not have a │ provider named registry.terraform.io/hashicorp/nonexist +│ +│ 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 hashicorp/nonexist, run the following +│ command: +│ terraform providers ╵ ` diff --git a/command/e2etest/testdata/provider-not-found-non-default/main.tf b/command/e2etest/testdata/provider-not-found-non-default/main.tf new file mode 100644 index 000000000..fe5112730 --- /dev/null +++ b/command/e2etest/testdata/provider-not-found-non-default/main.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + nonexist = { + source = "teamterraform/nonexist" + } + } +} diff --git a/command/init.go b/command/init.go index 3931e9713..670a4ae72 100644 --- a/command/init.go +++ b/command/init.go @@ -513,8 +513,8 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, case getproviders.ErrRegistryProviderNotKnown: // We might be able to suggest an alternative provider to use // instead of this one. - var suggestion string - alternative := getproviders.MissingProviderSuggestion(ctx, provider, inst.ProviderSource()) + suggestion := fmt.Sprintf("\n\nAll 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 %s, run the following command:\n terraform providers", provider.ForDisplay()) + alternative := getproviders.MissingProviderSuggestion(ctx, provider, inst.ProviderSource(), reqs) if alternative != provider { suggestion = fmt.Sprintf( "\n\nDid you intend to use %s? If so, you must specify that source address in each module which requires that provider. To see which modules are currently depending on %s, run the following command:\n terraform providers", diff --git a/internal/getproviders/didyoumean.go b/internal/getproviders/didyoumean.go index c91955264..b34bc995b 100644 --- a/internal/getproviders/didyoumean.go +++ b/internal/getproviders/didyoumean.go @@ -39,11 +39,20 @@ import ( // If the given context is cancelled then this function might not return a // renaming suggestion even if one would've been available for a completed // request. -func MissingProviderSuggestion(ctx context.Context, addr addrs.Provider, source Source) addrs.Provider { +func MissingProviderSuggestion(ctx context.Context, addr addrs.Provider, source Source, reqs Requirements) addrs.Provider { if !addr.IsDefault() { return addr } + // Before possibly looking up legacy naming, see if the user has another provider + // named in their requirements that is of the same type, and offer that + // as a suggestion + for req := range reqs { + if req != addr && req.Type == addr.Type { + return req + } + } + // Our strategy here, for a default provider, is to use the default // registry's special API for looking up "legacy" providers and try looking // for a legacy provider whose type name matches the type of the given diff --git a/internal/getproviders/didyoumean_test.go b/internal/getproviders/didyoumean_test.go index 05c315018..85bc4582c 100644 --- a/internal/getproviders/didyoumean_test.go +++ b/internal/getproviders/didyoumean_test.go @@ -21,10 +21,14 @@ func TestMissingProviderSuggestion(t *testing.T) { // testRegistrySource handles -/legacy as a valid legacy provider // lookup mapping to legacycorp/legacy. + legacyAddr := addrs.NewDefaultProvider("legacy") got := MissingProviderSuggestion( ctx, addrs.NewDefaultProvider("legacy"), source, + Requirements{ + legacyAddr: MustParseVersionConstraints(">= 1.0.0"), + }, ) want := addrs.Provider{ @@ -48,20 +52,49 @@ func TestMissingProviderSuggestion(t *testing.T) { // copy in some other namespace for v0.13 or later to use. Our naming // suggestions ignore the v0.12-compatible one and suggest the // other one. - got := MissingProviderSuggestion( - ctx, - addrs.NewDefaultProvider("moved"), - source, - ) - + moved := addrs.NewDefaultProvider("moved") want := addrs.Provider{ Hostname: defaultRegistryHost, Namespace: "acme", Type: "moved", } + + got := MissingProviderSuggestion( + ctx, + moved, + source, + Requirements{ + moved: MustParseVersionConstraints(">= 1.0.0"), + }, + ) + if got != want { t.Errorf("wrong result\ngot: %s\nwant: %s", got, want) } + + // If a provider has moved, but there's provider requirements + // for something of the same type, we'll return that one + // and skip the legacy lookup process. In practice, + // hopefully this is also "acme" but it's "zcme" here to + // exercise the codepath + want2 := addrs.Provider{ + Hostname: defaultRegistryHost, + Namespace: "zcme", + Type: "moved", + } + got2 := MissingProviderSuggestion( + ctx, + moved, + source, + Requirements{ + moved: MustParseVersionConstraints(">= 1.0.0"), + want2: MustParseVersionConstraints(">= 1.0.0"), + }, + ) + + if got2 != want2 { + t.Errorf("wrong result\ngot: %s\nwant: %s", got2, want2) + } }) t.Run("invalid response", func(t *testing.T) { ctx := context.Background() @@ -76,6 +109,9 @@ func TestMissingProviderSuggestion(t *testing.T) { ctx, want, source, + Requirements{ + want: MustParseVersionConstraints(">= 1.0.0"), + }, ) if got != want { t.Errorf("wrong result\ngot: %s\nwant: %s", got, want) @@ -98,6 +134,9 @@ func TestMissingProviderSuggestion(t *testing.T) { ctx, want, source, + Requirements{ + want: MustParseVersionConstraints(">= 1.0.0"), + }, ) if got != want { t.Errorf("wrong result\ngot: %s\nwant: %s", got, want) @@ -109,8 +148,8 @@ func TestMissingProviderSuggestion(t *testing.T) { defer close() // Because this provider address isn't in - // registry.terraform.io/hashicorp/..., MissingProviderSuggestion won't - // even attempt to make a suggestion for it. + // registry.terraform.io/hashicorp/..., MissingProviderSuggestion + // will provide the same addr since there's no alternative in Requirements want := addrs.Provider{ Hostname: defaultRegistryHost, Namespace: "whatever", @@ -120,9 +159,37 @@ func TestMissingProviderSuggestion(t *testing.T) { ctx, want, source, + Requirements{ + want: MustParseVersionConstraints(">= 1.0.0"), + }, ) if got != want { t.Errorf("wrong result\ngot: %s\nwant: %s", got, want) } + + // If there is a provider required that has the same type, + // but different namespace, we can suggest that + foo := addrs.Provider{ + Hostname: defaultRegistryHost, + Namespace: "hashicorp", + Type: "foo", + } + realFoo := addrs.Provider{ + Hostname: defaultRegistryHost, + Namespace: "acme", + Type: "foo", + } + got2 := MissingProviderSuggestion( + ctx, + foo, + source, + Requirements{ + foo: MustParseVersionConstraints(">= 1.0.0"), + realFoo: MustParseVersionConstraints(">= 1.0.0"), + }, + ) + if got2 != realFoo { + t.Errorf("wrong result\ngot: %s\nwant: %s", got2, realFoo) + } }) }