diff --git a/command/013_config_upgrade.go b/command/013_config_upgrade.go index 06ef7d2d6..2de8d3733 100644 --- a/command/013_config_upgrade.go +++ b/command/013_config_upgrade.go @@ -9,6 +9,7 @@ import ( "sort" "strings" + "github.com/apparentlymart/go-versions/versions" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hclwrite" @@ -207,6 +208,7 @@ command and dealing with them before running this command again. // Build up a list of required providers, uniquely by local name requiredProviders := make(map[string]*configs.RequiredProvider) rewritePaths := make(map[string]bool) + allProviderConstraints := make(map[string]getproviders.VersionConstraints) // Step 1: copy all explicit provider requirements across for path, file := range files { @@ -232,6 +234,24 @@ command and dealing with them before running this command again. Requirement: rp.Requirement, DeclRange: rp.DeclRange, } + + // Parse and store version constraints for later use when + // processing the provider redirect + constraints, err := getproviders.ParseVersionConstraints(rp.Requirement.Required.String()) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + // The errors returned by ParseVersionConstraint + // already include the section of input that was + // incorrect, so we don't need to + // include that here. + Detail: fmt.Sprintf("Incorrect version constraint syntax: %s.", err.Error()), + Subject: rp.Requirement.DeclRange.Ptr(), + }) + } else { + allProviderConstraints[rp.Name] = append(allProviderConstraints[rp.Name], constraints...) + } } } } @@ -253,6 +273,23 @@ command and dealing with them before running this command again. Name: p.Name, } } + // Parse and store version constraints for later use when + // processing the provider redirect + constraints, err := getproviders.ParseVersionConstraints(p.Version.Required.String()) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + // The errors returned by ParseVersionConstraint + // already include the section of input that was + // incorrect, so we don't need to + // include that here. + Detail: fmt.Sprintf("Incorrect version constraint syntax: %s.", err.Error()), + Subject: p.Version.DeclRange.Ptr(), + }) + } else { + allProviderConstraints[p.Name] = append(allProviderConstraints[p.Name], constraints...) + } } // Step 3: add missing provider requirements from resources @@ -291,7 +328,7 @@ command and dealing with them before running this command again. // stated in the config. If there are any providers, attempt to detect // their sources, and rewrite the config. if len(requiredProviders) > 0 { - detectDiags := c.detectProviderSources(requiredProviders) + detectDiags := c.detectProviderSources(requiredProviders, allProviderConstraints) diags = diags.Append(detectDiags) if diags.HasErrors() { c.Ui.Error("Unable to detect sources for providers") @@ -584,10 +621,11 @@ necessary adjustments, and then commit. } // For providers which need a source attribute, detect the source -func (c *ZeroThirteenUpgradeCommand) detectProviderSources(requiredProviders map[string]*configs.RequiredProvider) tfdiags.Diagnostics { +func (c *ZeroThirteenUpgradeCommand) detectProviderSources(requiredProviders map[string]*configs.RequiredProvider, allProviderConstraints map[string]getproviders.VersionConstraints) tfdiags.Diagnostics { source := c.providerInstallSource() var diags tfdiags.Diagnostics +providers: for name, rp := range requiredProviders { // If there's already an explicit source, skip it if rp.Source != "" { @@ -599,9 +637,70 @@ func (c *ZeroThirteenUpgradeCommand) detectProviderSources(requiredProviders map // parse process, because we know that without an explicit source it is // not explicitly specified. addr := addrs.NewLegacyProvider(name) - p, err := getproviders.LookupLegacyProvider(addr, source) + p, moved, err := getproviders.LookupLegacyProvider(addr, source) if err == nil { rp.Type = p + + if !moved.IsZero() { + constraints, ok := allProviderConstraints[name] + // If there's no version constraint, always use the redirect + // target as there should be at least one version we can + // install + if !ok { + rp.Type = moved + continue providers + } + + // Check that the redirect target has a version meeting our + // constraints + acceptable := versions.MeetingConstraints(constraints) + available, _, err := source.AvailableVersions(moved) + // If something goes wrong with the registry lookup here, fall + // back to the non-redirect provider + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Failed to query available provider packages", + fmt.Sprintf("Could not retrieve the list of available versions for provider %s: %s", + moved.ForDisplay(), err), + )) + continue providers + } + + // Walk backwards to consider newer versions first + for i := len(available) - 1; i >= 0; i-- { + if acceptable.Has(available[i]) { + // Success! Provider redirect target has a version + // meeting our constraints, so we can use it + rp.Type = moved + continue providers + } + } + + // Find the last version available at the old location + oldAvailable, _, err := source.AvailableVersions(p) + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Failed to query available provider packages", + fmt.Sprintf("Could not retrieve the list of available versions for provider %s: %s", + p.ForDisplay(), err), + )) + continue providers + } + lastAvailable := oldAvailable[len(oldAvailable)-1] + + // If we fall through here, no versions at the target meet our + // version constraints, so warn the user + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Provider has moved", + fmt.Sprintf( + "Provider %q has moved to %q. No action is required to continue using %q (%s), but if you want to upgrade beyond version %s, you must also update the source.", + moved.Type, moved.ForDisplay(), p.ForDisplay(), + getproviders.VersionConstraintsString(constraints), lastAvailable), + )) + } } else { // Setting the provider address to a zero value struct // indicates that there is no known FQN for this provider, diff --git a/command/013_config_upgrade_test.go b/command/013_config_upgrade_test.go index d2e9b9a41..f6553fd3d 100644 --- a/command/013_config_upgrade_test.go +++ b/command/013_config_upgrade_test.go @@ -57,7 +57,7 @@ func verifyExpectedFiles(t *testing.T, expectedPath string) { t.Fatalf("failed to read expected %s: %s", filePath, err) } - if diff := cmp.Diff(expected, output); diff != "" { + if diff := cmp.Diff(string(expected), string(output)); diff != "" { t.Fatalf("expected and output file for %s do not match\n%s", filePath, diff) } } @@ -81,6 +81,8 @@ func TestZeroThirteenUpgrade_success(t *testing.T) { "multiple files": "013upgrade-multiple-files", "existing versions.tf": "013upgrade-existing-versions-tf", "skipped files": "013upgrade-skipped-files", + "provider redirect": "013upgrade-provider-redirect", + "version unavailable": "013upgrade-provider-redirect-version-unavailable", } for name, testPath := range testCases { t.Run(name, func(t *testing.T) { diff --git a/command/command_test.go b/command/command_test.go index 2a08d6afc..6c62d1e69 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -894,6 +894,12 @@ var legacyProviderNamespaces = map[string]string{ "foo": "hashicorp", "bar": "hashicorp", "baz": "terraform-providers", + "qux": "hashicorp", +} + +// This map is used to mock the provider redirect feature. +var movedProviderNamespaces = map[string]string{ + "qux": "acme", } // testServices starts up a local HTTP server running a fake provider registry @@ -945,16 +951,36 @@ func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) { return } - if pathParts[0] != "-" || pathParts[2] != "versions" { + if pathParts[2] != "versions" { resp.WriteHeader(404) resp.Write([]byte(`this registry only supports legacy namespace lookup requests`)) + return } name := pathParts[1] - if namespace, ok := legacyProviderNamespaces[name]; ok { + + // Legacy lookup + if pathParts[0] == "-" { + if namespace, ok := legacyProviderNamespaces[name]; ok { + resp.Header().Set("Content-Type", "application/json") + resp.WriteHeader(200) + if movedNamespace, ok := movedProviderNamespaces[name]; ok { + resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s","moved_to":"%s/%s","versions":[{"version":"1.0.0","protocols":["4"]}]}`, namespace, name, movedNamespace, name))) + } else { + resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s","versions":[{"version":"1.0.0","protocols":["4"]}]}`, namespace, name))) + } + } else { + resp.WriteHeader(404) + resp.Write([]byte(`provider not found`)) + } + return + } + + // Also return versions for redirect target + if namespace, ok := movedProviderNamespaces[name]; ok && pathParts[0] == namespace { resp.Header().Set("Content-Type", "application/json") resp.WriteHeader(200) - resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s"}`, namespace, name))) + resp.Write([]byte(fmt.Sprintf(`{"id":"%s/%s","versions":[{"version":"1.0.0","protocols":["4"]}]}`, namespace, name))) } else { resp.WriteHeader(404) resp.Write([]byte(`provider not found`)) diff --git a/command/init.go b/command/init.go index f328509b5..7bf9f3d1b 100644 --- a/command/init.go +++ b/command/init.go @@ -695,9 +695,13 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, source := c.providerInstallSource() for provider, fetchErr := range missingProviderErrors { addr := addrs.NewLegacyProvider(provider.Type) - p, err := getproviders.LookupLegacyProvider(addr, source) + p, redirect, err := getproviders.LookupLegacyProvider(addr, source) if err == nil { - foundProviders[provider] = p + if redirect.IsZero() { + foundProviders[provider] = p + } else { + foundProviders[provider] = redirect + } } else { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, diff --git a/command/testdata/013upgrade-provider-redirect-version-unavailable/expected/main.tf b/command/testdata/013upgrade-provider-redirect-version-unavailable/expected/main.tf new file mode 100644 index 000000000..2e6a3b2c9 --- /dev/null +++ b/command/testdata/013upgrade-provider-redirect-version-unavailable/expected/main.tf @@ -0,0 +1,3 @@ +provider "qux" { + version = "~> 0.9.0" +} diff --git a/command/testdata/013upgrade-provider-redirect-version-unavailable/expected/versions.tf b/command/testdata/013upgrade-provider-redirect-version-unavailable/expected/versions.tf new file mode 100644 index 000000000..21c925ecd --- /dev/null +++ b/command/testdata/013upgrade-provider-redirect-version-unavailable/expected/versions.tf @@ -0,0 +1,8 @@ +terraform { + required_providers { + qux = { + source = "hashicorp/qux" + } + } + required_version = ">= 0.13" +} diff --git a/command/testdata/013upgrade-provider-redirect-version-unavailable/input/main.tf b/command/testdata/013upgrade-provider-redirect-version-unavailable/input/main.tf new file mode 100644 index 000000000..2e6a3b2c9 --- /dev/null +++ b/command/testdata/013upgrade-provider-redirect-version-unavailable/input/main.tf @@ -0,0 +1,3 @@ +provider "qux" { + version = "~> 0.9.0" +} diff --git a/command/testdata/013upgrade-provider-redirect/expected/main.tf b/command/testdata/013upgrade-provider-redirect/expected/main.tf new file mode 100644 index 000000000..d8b5dfb03 --- /dev/null +++ b/command/testdata/013upgrade-provider-redirect/expected/main.tf @@ -0,0 +1,3 @@ +provider "qux" { + version = ">= 1.0.0" +} diff --git a/command/testdata/013upgrade-provider-redirect/expected/versions.tf b/command/testdata/013upgrade-provider-redirect/expected/versions.tf new file mode 100644 index 000000000..c0b9ee574 --- /dev/null +++ b/command/testdata/013upgrade-provider-redirect/expected/versions.tf @@ -0,0 +1,8 @@ +terraform { + required_providers { + qux = { + source = "acme/qux" + } + } + required_version = ">= 0.13" +} diff --git a/command/testdata/013upgrade-provider-redirect/input/main.tf b/command/testdata/013upgrade-provider-redirect/input/main.tf new file mode 100644 index 000000000..d8b5dfb03 --- /dev/null +++ b/command/testdata/013upgrade-provider-redirect/input/main.tf @@ -0,0 +1,3 @@ +provider "qux" { + version = ">= 1.0.0" +} diff --git a/internal/getproviders/legacy_lookup.go b/internal/getproviders/legacy_lookup.go index 71d9b15cd..5f955d1f9 100644 --- a/internal/getproviders/legacy_lookup.go +++ b/internal/getproviders/legacy_lookup.go @@ -25,13 +25,13 @@ import ( // configurations that don't include explicit provider source addresses. New // configurations should not rely on it, and this fallback mechanism is // likely to be removed altogether in a future Terraform version. -func LookupLegacyProvider(addr addrs.Provider, source Source) (addrs.Provider, error) { +func LookupLegacyProvider(addr addrs.Provider, source Source) (addrs.Provider, addrs.Provider, error) { if addr.Namespace != "-" { - return addr, nil + return addr, addrs.Provider{}, nil } if addr.Hostname != defaultRegistryHost { // condition above assures namespace is also "-" // Legacy providers must always belong to the default registry host. - return addrs.Provider{}, fmt.Errorf("invalid provider type %q: legacy provider addresses must always belong to %s", addr, defaultRegistryHost) + return addrs.Provider{}, addrs.Provider{}, fmt.Errorf("invalid provider type %q: legacy provider addresses must always belong to %s", addr, defaultRegistryHost) } // Now we need to derive a suitable *RegistrySource from the given source, @@ -45,19 +45,28 @@ func LookupLegacyProvider(addr addrs.Provider, source Source) (addrs.Provider, e // based on the CLI configuration, which isn't necessarily true but // is true in all cases where this error message will ultimately be // presented to an end-user, so good enough for now. - return addrs.Provider{}, fmt.Errorf("unqualified provider type %q cannot be resolved because direct installation from %s is disabled in the CLI configuration; declare an explicit provider namespace for this provider", addr.Type, addr.Hostname) + return addrs.Provider{}, addrs.Provider{}, fmt.Errorf("unqualified provider type %q cannot be resolved because direct installation from %s is disabled in the CLI configuration; declare an explicit provider namespace for this provider", addr.Type, addr.Hostname) } - defaultNamespace, err := regSource.LookupLegacyProviderNamespace(addr.Hostname, addr.Type) + defaultNamespace, redirectNamespace, err := regSource.LookupLegacyProviderNamespace(addr.Hostname, addr.Type) if err != nil { - return addrs.Provider{}, err + return addrs.Provider{}, addrs.Provider{}, err } - - return addrs.Provider{ + provider := addrs.Provider{ Hostname: addr.Hostname, Namespace: defaultNamespace, Type: addr.Type, - }, nil + } + var redirect addrs.Provider + if redirectNamespace != "" { + redirect = addrs.Provider{ + Hostname: addr.Hostname, + Namespace: redirectNamespace, + Type: addr.Type, + } + } + + return provider, redirect, nil } // findLegacyProviderLookupSource tries to find a *RegistrySource that can talk diff --git a/internal/getproviders/legacy_lookup_test.go b/internal/getproviders/legacy_lookup_test.go index 2701b0a5d..1869a8d95 100644 --- a/internal/getproviders/legacy_lookup_test.go +++ b/internal/getproviders/legacy_lookup_test.go @@ -11,7 +11,7 @@ func TestLookupLegacyProvider(t *testing.T) { source, _, close := testRegistrySource(t) defer close() - got, err := LookupLegacyProvider( + got, gotMoved, err := LookupLegacyProvider( addrs.NewLegacyProvider("legacy"), source, ) @@ -27,13 +27,46 @@ func TestLookupLegacyProvider(t *testing.T) { if got != want { t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, want) } + if !gotMoved.IsZero() { + t.Errorf("wrong moved result\ngot: %#v\nwant: %#v", gotMoved, addrs.Provider{}) + } +} + +func TestLookupLegacyProvider_moved(t *testing.T) { + source, _, close := testRegistrySource(t) + defer close() + + got, gotMoved, err := LookupLegacyProvider( + addrs.NewLegacyProvider("moved"), + source, + ) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + want := addrs.Provider{ + Hostname: defaultRegistryHost, + Namespace: "hashicorp", + Type: "moved", + } + wantMoved := addrs.Provider{ + Hostname: defaultRegistryHost, + Namespace: "acme", + Type: "moved", + } + if got != want { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, want) + } + if gotMoved != wantMoved { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", gotMoved, wantMoved) + } } func TestLookupLegacyProvider_invalidResponse(t *testing.T) { source, _, close := testRegistrySource(t) defer close() - got, err := LookupLegacyProvider( + got, _, err := LookupLegacyProvider( addrs.NewLegacyProvider("invalid"), source, ) @@ -50,7 +83,7 @@ func TestLookupLegacyProvider_unexpectedTypeChange(t *testing.T) { source, _, close := testRegistrySource(t) defer close() - got, err := LookupLegacyProvider( + got, _, err := LookupLegacyProvider( addrs.NewLegacyProvider("changetype"), source, ) diff --git a/internal/getproviders/registry_client.go b/internal/getproviders/registry_client.go index b3f6a27c5..54c8a18c0 100644 --- a/internal/getproviders/registry_client.go +++ b/internal/getproviders/registry_client.go @@ -408,18 +408,18 @@ FindMatch: // This method exists only to allow compatibility with unqualified names // in older configurations. New configurations should be written so as not to // depend on it. -func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string, error) { +func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string, string, error) { endpointPath, err := url.Parse(path.Join("-", typeName, "versions")) if err != nil { // Should never happen because we're constructing this from // already-validated components. - return "", err + return "", "", err } endpointURL := c.baseURL.ResolveReference(endpointPath) req, err := retryablehttp.NewRequest("GET", endpointURL.String(), nil) if err != nil { - return "", err + return "", "", err } c.addHeadersToRequest(req.Request) @@ -429,7 +429,7 @@ func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string resp, err := c.httpClient.Do(req) if err != nil { - return "", c.errQueryFailed(placeholderProviderAddr, err) + return "", "", c.errQueryFailed(placeholderProviderAddr, err) } defer resp.Body.Close() @@ -437,35 +437,48 @@ func (c *registryClient) LegacyProviderDefaultNamespace(typeName string) (string case http.StatusOK: // Great! case http.StatusNotFound: - return "", ErrProviderNotFound{ + return "", "", ErrProviderNotFound{ Provider: placeholderProviderAddr, } case http.StatusUnauthorized, http.StatusForbidden: - return "", c.errUnauthorized(placeholderProviderAddr.Hostname) + return "", "", c.errUnauthorized(placeholderProviderAddr.Hostname) default: - return "", c.errQueryFailed(placeholderProviderAddr, errors.New(resp.Status)) + return "", "", c.errQueryFailed(placeholderProviderAddr, errors.New(resp.Status)) } type ResponseBody struct { - Id string + Id string `json:"id"` + MovedTo string `json:"moved_to"` } var body ResponseBody dec := json.NewDecoder(resp.Body) if err := dec.Decode(&body); err != nil { - return "", c.errQueryFailed(placeholderProviderAddr, err) + return "", "", c.errQueryFailed(placeholderProviderAddr, err) } provider, diags := addrs.ParseProviderSourceString(body.Id) if diags.HasErrors() { - return "", fmt.Errorf("Error parsing provider ID from Registry: %s", diags.Err()) + 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 "", "", fmt.Errorf("Registry returned provider with type %q, expected %q", provider.Type, typeName) } - return provider.Namespace, nil + var movedTo addrs.Provider + if body.MovedTo != "" { + movedTo, diags = addrs.ParseProviderSourceString(body.MovedTo) + if diags.HasErrors() { + return "", "", fmt.Errorf("Error parsing provider ID from Registry: %s", diags.Err()) + } + + if movedTo.Type != typeName { + return "", "", fmt.Errorf("Registry returned provider with type %q, expected %q", movedTo.Type, typeName) + } + } + + return provider.Namespace, movedTo.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 aad11fd5d..b7cbb2ee8 100644 --- a/internal/getproviders/registry_client_test.go +++ b/internal/getproviders/registry_client_test.go @@ -226,6 +226,11 @@ func fakeRegistryHandler(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(200) // This response is used for testing LookupLegacyProvider resp.Write([]byte(`{"id":"legacycorp/legacy"}`)) + case "-/moved": + resp.Header().Set("Content-Type", "application/json") + resp.WriteHeader(200) + // This response is used for testing LookupLegacyProvider + resp.Write([]byte(`{"id":"hashicorp/moved","moved_to":"acme/moved"}`)) case "-/changetype": resp.Header().Set("Content-Type", "application/json") resp.WriteHeader(200) diff --git a/internal/getproviders/registry_source.go b/internal/getproviders/registry_source.go index 57773699f..4f03f6a8c 100644 --- a/internal/getproviders/registry_source.go +++ b/internal/getproviders/registry_source.go @@ -118,10 +118,10 @@ func (s *RegistrySource) PackageMeta(provider addrs.Provider, version Version, t // in older configurations. New configurations should be written so as not to // depend on it, and this fallback mechanism will likely be removed altogether // in a future Terraform version. -func (s *RegistrySource) LookupLegacyProviderNamespace(hostname svchost.Hostname, typeName string) (string, error) { +func (s *RegistrySource) LookupLegacyProviderNamespace(hostname svchost.Hostname, typeName string) (string, string, error) { client, err := s.registryClient(hostname) if err != nil { - return "", err + return "", "", err } return client.LegacyProviderDefaultNamespace(typeName) }