Merge pull request #24903 from hashicorp/alisdair/lookup-legacy-provider-fix

internal: Fix LookupLegacyProvider

command/013_config_upgrade_test.go

@@ -1,6 +1,7 @@
 package command
 
 import (
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -361,21 +362,22 @@ func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) {
 
 	pathParts := strings.Split(path, "/")[3:]
 
-	if len(pathParts) != 2 {
+	if len(pathParts) != 3 {
 		resp.WriteHeader(404)
 		resp.Write([]byte(`unrecognized path scheme`))
 		return
 	}
 
-	if pathParts[0] != "-" {
+	if pathParts[0] != "-" || pathParts[2] != "versions" {
 		resp.WriteHeader(404)
 		resp.Write([]byte(`this registry only supports legacy namespace lookup requests`))
 	}
 
-	if namespace, ok := legacyProviderNamespaces[pathParts[1]]; ok {
+	name := pathParts[1]
+	if namespace, ok := legacyProviderNamespaces[name]; ok {
 		resp.Header().Set("Content-Type", "application/json")
 		resp.WriteHeader(200)
-		resp.Write([]byte(`{"namespace":"` + namespace + `"}`))
+		resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s"}`, namespace, name)))
 	} else {
 		resp.WriteHeader(404)
 		resp.Write([]byte(`provider not found`))

internal/getproviders/legacy_lookup_test.go

@@ -1,6 +1,7 @@
 package getproviders
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/addrs"
@@ -27,3 +28,37 @@ func TestLookupLegacyProvider(t *testing.T) {
 		t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, want)
 	}
 }
+
+func TestLookupLegacyProvider_invalidResponse(t *testing.T) {
+	source, _, close := testRegistrySource(t)
+	defer close()
+
+	got, err := LookupLegacyProvider(
+		addrs.NewLegacyProvider("invalid"),
+		source,
+	)
+	if !got.IsZero() {
+		t.Errorf("got non-zero addr\ngot:  %#v\nwant: %#v", got, nil)
+	}
+	wantErr := "Error parsing provider ID from Registry: Invalid provider source string"
+	if gotErr := err.Error(); !strings.Contains(gotErr, wantErr) {
+		t.Fatalf("unexpected error: got %q, want %q", gotErr, wantErr)
+	}
+}
+
+func TestLookupLegacyProvider_unexpectedTypeChange(t *testing.T) {
+	source, _, close := testRegistrySource(t)
+	defer close()
+
+	got, err := LookupLegacyProvider(
+		addrs.NewLegacyProvider("changetype"),
+		source,
+	)
+	if !got.IsZero() {
+		t.Errorf("got non-zero addr\ngot:  %#v\nwant: %#v", got, nil)
+	}
+	wantErr := `Registry returned provider with type "newtype", expected "changetype"`
+	if gotErr := err.Error(); gotErr != wantErr {
+		t.Fatalf("unexpected error: got %q, want %q", gotErr, wantErr)
+	}
+}

internal/getproviders/registry_client.go

@@ -292,7 +292,7 @@ func (c *registryClient) PackageMeta(provider addrs.Provider, version Version, t
 	return ret, nil
 }
 
-// LegacyProviderCanonicalAddress returns the raw address strings produced by
+// LegacyProviderDefaultNamespace returns the raw address strings produced by
 // the registry when asked about the given unqualified provider type name.
 // The returned namespace string is taken verbatim from the registry's response.
 //
@@ -300,7 +300,7 @@ func (c *registryClient) PackageMeta(provider addrs.Provider, version Version, t
 // in older configurations. New configurations should be written so as not to
 // depend on it.
 func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string, error) {
-	endpointPath, err := url.Parse(path.Join("-", typeName))
+	endpointPath, err := url.Parse(path.Join("-", typeName, "versions"))
 	if err != nil {
 		// Should never happen because we're constructing this from
 		// already-validated components.
@@ -338,7 +338,7 @@ func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string
 	}
 
 	type ResponseBody struct {
-		Namespace string
+		Id string
 	}
 	var body ResponseBody
 
@@ -347,7 +347,16 @@ func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string
 		return "", c.errQueryFailed(placeholderProviderAddr, err)
 	}
 
-	return body.Namespace, nil
+	provider, diags := addrs.ParseProviderSourceString(body.Id)
+	if diags.HasErrors() {
+		return "", fmt.Errorf("Error parsing provider ID from Registry: %s", diags.Err())
+	}
+
+	if provider.Type != typeName {
+		return "", fmt.Errorf("Registry returned provider with type %q, expected %q", provider.Type, typeName)
+	}
+
+	return provider.Namespace, nil
 }
 
 func (c *registryClient) addHeadersToRequest(req *http.Request) {

internal/getproviders/registry_client_test.go

@@ -114,34 +114,12 @@ func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) {
 	}
 
 	pathParts := strings.Split(path, "/")[3:]
-	if len(pathParts) < 2 {
-		resp.WriteHeader(404)
-		resp.Write([]byte(`unexpected number of path parts`))
-		return
-	}
-	log.Printf("[TRACE] fake provider registry request for %#v", pathParts)
-	if len(pathParts) == 2 {
-		switch pathParts[0] + "/" + pathParts[1] {
-
-		case "-/legacy":
-			// NOTE: This legacy lookup endpoint is specific to
-			// registry.terraform.io and not expected to work on any other
-			// registry host.
-			resp.Header().Set("Content-Type", "application/json")
-			resp.WriteHeader(200)
-			resp.Write([]byte(`{"namespace":"legacycorp"}`))
-
-		default:
-			resp.WriteHeader(404)
-			resp.Write([]byte(`unknown namespace or provider type for direct lookup`))
-		}
-	}
-
 	if len(pathParts) < 3 {
 		resp.WriteHeader(404)
 		resp.Write([]byte(`unexpected number of path parts`))
 		return
 	}
+	log.Printf("[TRACE] fake provider registry request for %#v", pathParts)
 
 	if pathParts[2] == "versions" {
 		if len(pathParts) != 3 {
@@ -162,6 +140,21 @@ func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) {
 			resp.Header().Set("Content-Type", "application/json")
 			resp.WriteHeader(200)
 			resp.Write([]byte(`{"versions":[]}`))
+		case "-/legacy":
+			resp.Header().Set("Content-Type", "application/json")
+			resp.WriteHeader(200)
+			// This response is used for testing LookupLegacyProvider
+			resp.Write([]byte(`{"id":"legacycorp/legacy"}`))
+		case "-/changetype":
+			resp.Header().Set("Content-Type", "application/json")
+			resp.WriteHeader(200)
+			// This (unrealistic) response is used for error handling code coverage
+			resp.Write([]byte(`{"id":"legacycorp/newtype"}`))
+		case "-/invalid":
+			resp.Header().Set("Content-Type", "application/json")
+			resp.WriteHeader(200)
+			// This (unrealistic) response is used for error handling code coverage
+			resp.Write([]byte(`{"id":"some/invalid/id/string"}`))
 		default:
 			resp.WriteHeader(404)
 			resp.Write([]byte(`unknown namespace or provider type`))