Merge pull request #25055 from hashicorp/alisdair/detected-legacy-provider-on-init

command/init: Better diagnostics for provider 404s

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`))
-	}
-}

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`))
+	}
+}

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)
 		}
 	})

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
 	}
 

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)

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 {}