From ca401070663bf65a57dd7bbcb7356aa37b87075a Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Mon, 25 May 2020 15:24:35 -0400 Subject: [PATCH] command/init: Better diagnostics for provider 404s Fetching a default namespace provider from the public registry can result in 404 Not Found error. This might be caused by a previously- default provider moving to a new namespace, which means that the configuration needs to be upgraded to use an explicit provider source. This commit adds a more detailed diagnostic for this situation, suggesting that the intended provider might be in a new namespace. The recommended course of action is to run the 0.13upgrade command to generate the correct required_providers configuration. --- command/013_config_upgrade_test.go | 80 ------------------- command/command_test.go | 77 ++++++++++++++++++ command/e2etest/init_test.go | 2 +- command/init.go | 53 ++++++++++++ command/init_test.go | 64 +++++++++++++++ .../init-get-provider-detected-legacy/main.tf | 10 +++ 6 files changed, 205 insertions(+), 81 deletions(-) create mode 100644 command/testdata/init-get-provider-detected-legacy/main.tf diff --git a/command/013_config_upgrade_test.go b/command/013_config_upgrade_test.go index 195c30a78f..7eab57451c 100644 --- a/command/013_config_upgrade_test.go +++ b/command/013_config_upgrade_test.go @@ -2,10 +2,7 @@ package command import ( "bytes" - "fmt" "io/ioutil" - "net/http" - "net/http/httptest" "os" "path" "path/filepath" @@ -13,22 +10,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" - svchost "github.com/hashicorp/terraform-svchost" - "github.com/hashicorp/terraform-svchost/disco" "github.com/hashicorp/terraform/helper/copy" - "github.com/hashicorp/terraform/internal/getproviders" "github.com/mitchellh/cli" ) -// This map from provider type name to namespace is used by the fake registry -// when called via LookupLegacyProvider. Providers not in this map will return -// a 404 Not Found error. -var legacyProviderNamespaces = map[string]string{ - "foo": "hashicorp", - "bar": "hashicorp", - "baz": "terraform-providers", -} - func verifyExpectedFiles(t *testing.T, expectedPath string) { // Compare output and expected file trees var outputFiles, expectedFiles []string @@ -380,68 +365,3 @@ func TestZeroThirteenUpgrade_empty(t *testing.T) { t.Fatal("unexpected error:", errMsg) } } - -// testServices starts up a local HTTP server running a fake provider registry -// service which responds only to discovery requests and legacy provider lookup -// API calls. -// -// The final return value is a function to call at the end of a test function -// to shut down the test server. After you call that function, the discovery -// object becomes useless. -func testServices(t *testing.T) (services *disco.Disco, cleanup func()) { - server := httptest.NewServer(http.HandlerFunc(fakeRegistryHandler)) - - services = disco.New() - services.ForceHostServices(svchost.Hostname("registry.terraform.io"), map[string]interface{}{ - "providers.v1": server.URL + "/providers/v1/", - }) - - return services, func() { - server.Close() - } -} - -// testRegistrySource is a wrapper around testServices that uses the created -// discovery object to produce a Source instance that is ready to use with the -// fake registry services. -// -// As with testServices, the final return value is a function to call at the end -// of your test in order to shut down the test server. -func testRegistrySource(t *testing.T) (source *getproviders.RegistrySource, cleanup func()) { - services, close := testServices(t) - source = getproviders.NewRegistrySource(services) - return source, close -} - -func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) { - path := req.URL.EscapedPath() - - if !strings.HasPrefix(path, "/providers/v1/") { - resp.WriteHeader(404) - resp.Write([]byte(`not a provider registry endpoint`)) - return - } - - pathParts := strings.Split(path, "/")[3:] - - if len(pathParts) != 3 { - resp.WriteHeader(404) - resp.Write([]byte(`unrecognized path scheme`)) - return - } - - if pathParts[0] != "-" || pathParts[2] != "versions" { - resp.WriteHeader(404) - resp.Write([]byte(`this registry only supports legacy namespace lookup requests`)) - } - - name := pathParts[1] - if namespace, ok := legacyProviderNamespaces[name]; ok { - resp.Header().Set("Content-Type", "application/json") - resp.WriteHeader(200) - resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s"}`, namespace, name))) - } else { - resp.WriteHeader(404) - resp.Write([]byte(`provider not found`)) - } -} diff --git a/command/command_test.go b/command/command_test.go index 7c87f9d6a2..2a528765a1 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -19,6 +19,9 @@ import ( "syscall" "testing" + svchost "github.com/hashicorp/terraform-svchost" + "github.com/hashicorp/terraform-svchost/disco" + "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/registry" @@ -883,3 +886,77 @@ func mustResourceAddr(s string) addrs.ConfigResource { } return addr.Config() } + +// This map from provider type name to namespace is used by the fake registry +// when called via LookupLegacyProvider. Providers not in this map will return +// a 404 Not Found error. +var legacyProviderNamespaces = map[string]string{ + "foo": "hashicorp", + "bar": "hashicorp", + "baz": "terraform-providers", +} + +// testServices starts up a local HTTP server running a fake provider registry +// service which responds only to discovery requests and legacy provider lookup +// API calls. +// +// The final return value is a function to call at the end of a test function +// to shut down the test server. After you call that function, the discovery +// object becomes useless. +func testServices(t *testing.T) (services *disco.Disco, cleanup func()) { + server := httptest.NewServer(http.HandlerFunc(fakeRegistryHandler)) + + services = disco.New() + services.ForceHostServices(svchost.Hostname("registry.terraform.io"), map[string]interface{}{ + "providers.v1": server.URL + "/providers/v1/", + }) + + return services, func() { + server.Close() + } +} + +// testRegistrySource is a wrapper around testServices that uses the created +// discovery object to produce a Source instance that is ready to use with the +// fake registry services. +// +// As with testServices, the final return value is a function to call at the end +// of your test in order to shut down the test server. +func testRegistrySource(t *testing.T) (source *getproviders.RegistrySource, cleanup func()) { + services, close := testServices(t) + source = getproviders.NewRegistrySource(services) + return source, close +} + +func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) { + path := req.URL.EscapedPath() + + if !strings.HasPrefix(path, "/providers/v1/") { + resp.WriteHeader(404) + resp.Write([]byte(`not a provider registry endpoint`)) + return + } + + pathParts := strings.Split(path, "/")[3:] + + if len(pathParts) != 3 { + resp.WriteHeader(404) + resp.Write([]byte(`unrecognized path scheme`)) + return + } + + if pathParts[0] != "-" || pathParts[2] != "versions" { + resp.WriteHeader(404) + resp.Write([]byte(`this registry only supports legacy namespace lookup requests`)) + } + + name := pathParts[1] + if namespace, ok := legacyProviderNamespaces[name]; ok { + resp.Header().Set("Content-Type", "application/json") + resp.WriteHeader(200) + resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s"}`, namespace, name))) + } else { + resp.WriteHeader(404) + resp.Write([]byte(`provider not found`)) + } +} diff --git a/command/e2etest/init_test.go b/command/e2etest/init_test.go index 5da646913e..e8bff4d7b5 100644 --- a/command/e2etest/init_test.go +++ b/command/e2etest/init_test.go @@ -329,7 +329,7 @@ func TestInitProviderNotFound(t *testing.T) { t.Fatal("expected error, got success") } - if !strings.Contains(stderr, "provider registry registry.terraform.io does not have a\nprovider named registry.terraform.io/hashicorp/nonexist") { + if !strings.Contains(stderr, "provider registry\nregistry.terraform.io does not have a provider named\nregistry.terraform.io/hashicorp/nonexist") { t.Errorf("expected error message is missing from output:\n%s", stderr) } }) diff --git a/command/init.go b/command/init.go index 2c0c18bcc8..7fa88b24ba 100644 --- a/command/init.go +++ b/command/init.go @@ -442,6 +442,11 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, log.Printf("[DEBUG] will search for provider plugins in %s", pluginDirs) } + // We capture any missing provider errors (404s from a Registry source) for + // later analysis, to provide more useful diagnostics if the providers + // appear to have been re-namespaced. + missingProviderErrors := make(map[addrs.Provider]error) + // Because we're currently just streaming a series of events sequentially // into the terminal, we're showing only a subset of the events to keep // things relatively concise. Later it'd be nice to have a progress UI @@ -494,6 +499,22 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, provider.ForDisplay(), err, strings.Join(displaySources, "\n"), ), )) + case getproviders.ErrRegistryProviderNotKnown: + // Default providers may have no explicit source, and the 404 + // error could be caused by re-namespacing. Add the provider + // and error to a map to later check for this case. We don't + // run the check here to keep this event callback simple. + if provider.IsDefault() { + missingProviderErrors[provider] = err + } else { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to query available provider packages", + fmt.Sprintf("Could not retrieve the list of available versions for provider %s: %s", + provider.ForDisplay(), err, + ), + )) + } default: diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -586,12 +607,44 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, ctx := evts.OnContext(context.TODO()) selected, err := inst.EnsureProviderVersions(ctx, reqs, mode) if err != nil { + // Try to look up any missing providers which may be redirected legacy + // providers. If we're successful, construct a "did you mean?" diag to + // suggest how to fix this. Otherwise, add a simple error diag + // explaining that the provider could not be found. + foundProviders := make(map[addrs.Provider]addrs.Provider) + source := c.providerInstallSource() + for provider, fetchErr := range missingProviderErrors { + addr := addrs.NewLegacyProvider(provider.Type) + p, err := getproviders.LookupLegacyProvider(addr, source) + if err == nil { + foundProviders[provider] = p + } else { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to install provider", + fmt.Sprintf("Error while installing %s: %s", provider.ForDisplay(), fetchErr), + )) + } + } + if len(foundProviders) > 0 { + var providerSuggestions string + for missingProvider, foundProvider := range foundProviders { + providerSuggestions += fmt.Sprintf(" %s -> %s\n", missingProvider.ForDisplay(), foundProvider.ForDisplay()) + } + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to install providers", + fmt.Sprintf("Could not find required providers, but found possible alternatives:\n\n%s\nIf these suggestions look correct, upgrade your configuration with the following command:\n terraform 0.13upgrade", providerSuggestions), + )) + } + // The errors captured in "err" should be redundant with what we // received via the InstallerEvents callbacks above, so we'll // just return those as long as we have some. if !diags.HasErrors() { diags = diags.Append(err) } + return true, diags } diff --git a/command/init_test.go b/command/init_test.go index 1e37a5ebea..194049d8b3 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -898,6 +898,70 @@ func TestInit_getProviderSource(t *testing.T) { } } +func TestInit_getProviderDetectedLegacy(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + copy.CopyDir(testFixturePath("init-get-provider-detected-legacy"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + // We need to construct a multisource with a mock source and a registry + // source: the mock source will return ErrRegistryProviderNotKnown for an + // unknown provider, and the registry source will allow us to look up the + // appropriate namespace if possible. + providerSource, psClose := newMockProviderSource(t, map[string][]string{ + "hashicorp/foo": []string{"1.2.3"}, + "terraform-providers/baz": []string{"2.3.4"}, // this will not be installed + }) + defer psClose() + registrySource, rsClose := testRegistrySource(t) + defer rsClose() + multiSource := getproviders.MultiSource{ + {Source: providerSource}, + {Source: registrySource}, + } + + ui := new(cli.MockUi) + m := Meta{ + Ui: ui, + ProviderSource: multiSource, + } + + c := &InitCommand{ + Meta: m, + } + + args := []string{ + "-backend=false", // should be possible to install plugins without backend init + } + if code := c.Run(args); code == 0 { + t.Fatalf("expected error, got output: \n%s", ui.OutputWriter.String()) + } + + // foo should be installed + fooPath := fmt.Sprintf(".terraform/plugins/registry.terraform.io/hashicorp/foo/1.2.3/%s", getproviders.CurrentPlatform) + if _, err := os.Stat(fooPath); os.IsNotExist(err) { + t.Error("provider 'foo' not installed") + } + // baz should not be installed + bazPath := fmt.Sprintf(".terraform/plugins/registry.terraform.io/terraform-providers/baz/2.3.4/%s", getproviders.CurrentPlatform) + if _, err := os.Stat(bazPath); !os.IsNotExist(err) { + t.Error("provider 'baz' installed, but should not be") + } + + // error output is the main focus of this test + errOutput := ui.ErrorWriter.String() + if !strings.Contains(errOutput, "Error while installing hashicorp/frob:") { + t.Fatalf("expected error for installing hashicorp/frob: %s", errOutput) + } + if !strings.Contains(errOutput, "Could not find required providers, but found possible alternatives") { + t.Fatalf("expected required provider suggestions: %s", errOutput) + } + if !strings.Contains(errOutput, "hashicorp/baz -> terraform-providers/baz") { + t.Fatalf("expected suggestion for hashicorp/baz: %s", errOutput) + } +} + func TestInit_providerSource(t *testing.T) { // Create a temporary working directory that is empty td := tempDir(t) diff --git a/command/testdata/init-get-provider-detected-legacy/main.tf b/command/testdata/init-get-provider-detected-legacy/main.tf new file mode 100644 index 0000000000..467ecf3be3 --- /dev/null +++ b/command/testdata/init-get-provider-detected-legacy/main.tf @@ -0,0 +1,10 @@ +// This should result in installing hashicorp/foo +provider foo {} + +// This will try to install hashicorp/baz, fail, and then suggest +// terraform-providers/baz +provider baz {} + +// This will try to install hashicrop/frob, fail, find no suggestions, and +// result in an error +provider frob {}