From 62b0cbed1294528c3de7d9fa5135fa09f3600f95 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 8 May 2020 11:40:55 -0400 Subject: [PATCH] internal: Fix LookupLegacyProvider When looking up the namespace for a legacy provider source, we need to use the /v1/providers/-/{name}/versions endpoint. For non-HashiCorp providers, the /v1/providers/-/{name} endpoint returns a 404. This commit updates the LegacyProviderDefaultNamespace method and the mock registry servers accordingly. --- command/013_config_upgrade_test.go | 10 +++-- internal/getproviders/legacy_lookup_test.go | 35 +++++++++++++++++ internal/getproviders/registry_client.go | 17 ++++++-- internal/getproviders/registry_client_test.go | 39 ++++++++----------- 4 files changed, 70 insertions(+), 31 deletions(-) diff --git a/command/013_config_upgrade_test.go b/command/013_config_upgrade_test.go index 2bc472fe1..d0a7fa23f 100644 --- a/command/013_config_upgrade_test.go +++ b/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`)) diff --git a/internal/getproviders/legacy_lookup_test.go b/internal/getproviders/legacy_lookup_test.go index eac853b63..2701b0a5d 100644 --- a/internal/getproviders/legacy_lookup_test.go +++ b/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) + } +} diff --git a/internal/getproviders/registry_client.go b/internal/getproviders/registry_client.go index 87ba337b5..7179ab08d 100644 --- a/internal/getproviders/registry_client.go +++ b/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) { diff --git a/internal/getproviders/registry_client_test.go b/internal/getproviders/registry_client_test.go index 454b9b407..128b88474 100644 --- a/internal/getproviders/registry_client_test.go +++ b/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`))