From 59b116f7bfa857def1c7b221f6cf1f6317a29708 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 29 Sep 2020 17:51:39 -0700 Subject: [PATCH] command/init: Remove support for legacy provider addresses We no longer need to support 0.12-and-earlier-style provider addresses because users should've upgraded their existing configurations and states on Terraform 0.13 already. For now this is only checked in the "init" command, because various test shims are still relying on the idea of legacy providers the core layer. However, rejecting these during init is sufficient grounds to avoid supporting legacy provider addresses in the new dependency lock file format, and thus sets the stage for a more severe removal of legacy provider support in a later commit. --- command/e2etest/init_test.go | 3 +- command/init.go | 191 +++----------------- command/init_test.go | 21 +-- internal/getproviders/legacy_lookup.go | 136 -------------- internal/getproviders/legacy_lookup_test.go | 102 ----------- internal/getproviders/registry_client.go | 81 --------- internal/getproviders/registry_source.go | 23 --- 7 files changed, 31 insertions(+), 526 deletions(-) delete mode 100644 internal/getproviders/legacy_lookup.go delete mode 100644 internal/getproviders/legacy_lookup_test.go diff --git a/command/e2etest/init_test.go b/command/e2etest/init_test.go index ed5e112d5454..84e8ee378351 100644 --- a/command/e2etest/init_test.go +++ b/command/e2etest/init_test.go @@ -340,7 +340,8 @@ func TestInitProviderNotFound(t *testing.T) { t.Fatal("expected error, got success") } - if !strings.Contains(stderr, "provider registry\nregistry.terraform.io does not have a provider named\nregistry.terraform.io/hashicorp/nonexist") { + oneLineStderr := strings.ReplaceAll(stderr, "\n", " ") + if !strings.Contains(oneLineStderr, "provider registry registry.terraform.io does not have a provider named registry.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 edb49540e4fd..003cc9d7c69e 100644 --- a/command/init.go +++ b/command/init.go @@ -433,6 +433,22 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, reqs = reqs.Merge(stateReqs) } + for providerAddr := range reqs { + if providerAddr.IsLegacy() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid legacy provider address", + fmt.Sprintf( + "This configuration or its associated state refers to the unqualified provider %q.\n\nYou must complete the Terraform 0.13 upgrade process before upgrading to later versions.", + providerAddr.Type, + ), + )) + } + } + if diags.HasErrors() { + return false, true, diags + } + var inst *providercache.Installer if len(pluginDirs) == 0 { // By default we use a source that looks for providers in all of the @@ -452,16 +468,6 @@ 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) - - // Legacy provider addresses required by source probably refer to in-house - // providers. Capture these for later analysis also, to suggest how to use - // the state replace-provider command to fix this problem. - stateLegacyProviderErrors := 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 @@ -515,27 +521,13 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, ), )) case getproviders.ErrRegistryProviderNotKnown: - if provider.IsDefault() { - // 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. - missingProviderErrors[provider] = err - } else if _, ok := stateReqs[provider]; ok && provider.IsLegacy() { - // Legacy provider, from state, not found from any source: - // probably an in-house provider. Record this here to - // faciliate a useful suggestion later. - stateLegacyProviderErrors[provider] = err - } else { - // Otherwise maybe this provider really doesn't exist? Shrug! - 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, - ), - )) - } + 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, + ), + )) case getproviders.ErrHostNoProviders: switch { case errorTy.Hostname == svchost.Hostname("github.com") && !errorTy.HasOtherVersion: @@ -745,143 +737,6 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, return true, true, diags } if err != nil { - // Build a map of provider address to modules using the provider, - // so that we can later show diagnostics about affected modules - reqs, _ := config.ProviderRequirementsByModule() - providerToReqs := make(map[addrs.Provider][]*configs.ModuleRequirements) - c.populateProviderToReqs(providerToReqs, reqs) - - // 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, redirect, err := getproviders.LookupLegacyProvider(ctx, addr, source) - if err == nil { - if redirect.IsZero() { - foundProviders[provider] = p - } else { - foundProviders[provider] = redirect - } - } 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 { - // Build list of provider suggestions, and track a list of local - // and remote modules which need to be upgraded - var providerSuggestions string - localModules := make(map[string]struct{}) - remoteModules := make(map[*configs.ModuleRequirements]struct{}) - for missingProvider, foundProvider := range foundProviders { - providerSuggestions += fmt.Sprintf(" %s -> %s\n", missingProvider.ForDisplay(), foundProvider.ForDisplay()) - exists := struct{}{} - for _, reqs := range providerToReqs[missingProvider] { - src := reqs.SourceAddr - // Treat the root module and any others with local source - // addresses as fixable with 0.13upgrade. Remote modules - // must be upgraded elsewhere and therefore are listed - // separately - if src == "" || isLocalSourceAddr(src) { - localModules[reqs.SourceDir] = exists - } else { - remoteModules[reqs] = exists - } - } - } - - // Create sorted list of 0.13upgrade commands with the affected - // source dirs - var upgradeCommands []string - for dir := range localModules { - upgradeCommands = append(upgradeCommands, fmt.Sprintf("terraform 0.13upgrade %s", dir)) - } - sort.Strings(upgradeCommands) - command := "command" - if len(upgradeCommands) > 1 { - command = "commands" - } - - // Display detailed diagnostic results, including the missing and - // found provider FQNs, and the suggested series of upgrade - // commands to fix this - var detail strings.Builder - - fmt.Fprintf(&detail, "Could not find required providers, but found possible alternatives:\n\n%s\n", providerSuggestions) - - fmt.Fprintf(&detail, "If these suggestions look correct, upgrade your configuration with the following %s:", command) - for _, upgradeCommand := range upgradeCommands { - fmt.Fprintf(&detail, "\n %s", upgradeCommand) - } - - if len(remoteModules) > 0 { - fmt.Fprintf(&detail, "\n\nThe following remote modules must also be upgraded for Terraform 0.13 compatibility:") - for remoteModule := range remoteModules { - fmt.Fprintf(&detail, "\n- module.%s at %s", remoteModule.Name, remoteModule.SourceAddr) - } - } - - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Failed to install providers", - detail.String(), - )) - } - - // Legacy providers required by state which could not be installed are - // probably in-house providers. If the user has completed the necessary - // steps to make their custom provider available for installation, then - // there should be a provider with the same type selected after the - // installation process completed. - // - // If we detect this specific situation, we can confidently suggest - // that the next step is to run the state replace-provider command to - // update state. We build a map of provider replacements here to ensure - // that we're as concise as possible with the diagnostic. - stateReplaceProviders := make(map[addrs.Provider]addrs.Provider) - for provider, fetchErr := range stateLegacyProviderErrors { - var sameType []addrs.Provider - for p := range selected { - if p.Type == provider.Type { - sameType = append(sameType, p) - } - } - if len(sameType) == 1 { - stateReplaceProviders[provider] = sameType[0] - } else { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Failed to install provider", - fmt.Sprintf("Error while installing %s: %s", provider.ForDisplay(), fetchErr), - )) - } - } - if len(stateReplaceProviders) > 0 { - var detail strings.Builder - command := "command" - if len(stateReplaceProviders) > 1 { - command = "commands" - } - - fmt.Fprintf(&detail, "Found unresolvable legacy provider references in state. It looks like these refer to in-house providers. You can update the resources in state with the following %s:\n", command) - for legacy, replacement := range stateReplaceProviders { - fmt.Fprintf(&detail, "\n terraform state replace-provider %s %s", legacy, replacement) - } - - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Failed to install legacy providers required by state", - detail.String(), - )) - } - // 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. diff --git a/command/init_test.go b/command/init_test.go index 27b8346bb5e9..796d24570c39 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -1055,8 +1055,8 @@ func TestInit_getProviderLegacyFromState(t *testing.T) { // Expect this diagnostic output wants := []string{ - "Found unresolvable legacy provider references in state", - "terraform state replace-provider registry.terraform.io/-/alpha registry.terraform.io/acme/alpha", + "Invalid legacy provider address", + "You must complete the Terraform 0.13 upgrade process", } got := ui.ErrorWriter.String() for _, want := range wants { @@ -1064,12 +1064,6 @@ func TestInit_getProviderLegacyFromState(t *testing.T) { t.Fatalf("expected output to contain %q, got:\n\n%s", want, got) } } - - // Should still install the alpha provider - exactPath := fmt.Sprintf(".terraform/plugins/registry.terraform.io/acme/alpha/1.2.3/%s", getproviders.CurrentPlatform) - if _, err := os.Stat(exactPath); os.IsNotExist(err) { - t.Fatal("provider 'alpha' not downloaded") - } } func TestInit_getProviderInvalidPackage(t *testing.T) { @@ -1187,13 +1181,10 @@ func TestInit_getProviderDetectedLegacy(t *testing.T) { // error output is the main focus of this test errOutput := ui.ErrorWriter.String() errors := []string{ - "Error while installing hashicorp/frob:", - "Could not find required providers, but found possible alternatives", - "hashicorp/baz -> terraform-providers/baz", - "terraform 0.13upgrade .", - "terraform 0.13upgrade child", - "The following remote modules must also be upgraded", - "- module.dicerolls at acme/bar/random", + "Failed to query available provider packages", + "Could not retrieve the list of available versions", + "registry.terraform.io/hashicorp/baz", + "registry.terraform.io/hashicorp/frob", } for _, want := range errors { if !strings.Contains(errOutput, want) { diff --git a/internal/getproviders/legacy_lookup.go b/internal/getproviders/legacy_lookup.go deleted file mode 100644 index 234ef3becec2..000000000000 --- a/internal/getproviders/legacy_lookup.go +++ /dev/null @@ -1,136 +0,0 @@ -package getproviders - -import ( - "context" - "fmt" - - svchost "github.com/hashicorp/terraform-svchost" - - "github.com/hashicorp/terraform/addrs" -) - -// LookupLegacyProvider attempts to resolve a legacy provider address (whose -// registry host and namespace are implied, rather than explicit) into a -// fully-qualified provider address, by asking the main Terraform registry -// to resolve it. -// -// If the given address is not a legacy provider address then it will just be -// returned verbatim without making any outgoing requests. -// -// Legacy provider lookup is possible only if the given source is either a -// *RegistrySource directly or if it is a MultiSource containing a -// *RegistrySource whose selector matching patterns include the -// public registry hostname registry.terraform.io. -// -// This is a backward-compatibility mechanism for compatibility with existing -// 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(ctx context.Context, addr addrs.Provider, source Source) (addrs.Provider, addrs.Provider, error) { - if addr.Namespace != "-" { - 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{}, 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, - // either directly or indirectly. This will not be possible if the user - // has configured Terraform to disable direct installation from - // registry.terraform.io; in that case, fully-qualified provider addresses - // are always required. - regSource := findLegacyProviderLookupSource(addr.Hostname, source) - if regSource == nil { - // This error message is assuming that the given Source was produced - // 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{}, 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, redirectNamespace, err := regSource.LookupLegacyProviderNamespace(ctx, addr.Hostname, addr.Type) - if err != nil { - return addrs.Provider{}, addrs.Provider{}, err - } - provider := addrs.Provider{ - Hostname: addr.Hostname, - Namespace: defaultNamespace, - Type: addr.Type, - } - 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 -// to the given registry host in the given Source. It might be given directly, -// or it might be given indirectly via a MultiSource where the selector -// includes a wildcard for registry.terraform.io. -// -// Returns nil if the given source does not have any configured way to talk -// directly to the given host. -// -// If the given source contains multiple sources that can talk to the given -// host directly, the first one in the sequence takes preference. In practice -// it's pointless to have two direct installation sources that match the same -// hostname anyway, so this shouldn't arise in normal use. -func findLegacyProviderLookupSource(host svchost.Hostname, source Source) *RegistrySource { - switch source := source.(type) { - - case *RegistrySource: - // Easy case: the source is a registry source directly, and so we'll - // just use it. - return source - - case *MemoizeSource: - // Also easy: the source is a memoize wrapper, so defer to its - // underlying source. - return findLegacyProviderLookupSource(host, source.underlying) - - case MultiSource: - // Trickier case: if it's a multisource then we need to scan over - // its selectors until we find one that is a *RegistrySource _and_ - // that is configured to accept arbitrary providers from the - // given hostname. - - // For our matching purposes we'll use an address that would not be - // valid as a real provider FQN and thus can only match a selector - // that has no filters at all or a selector that wildcards everything - // except the hostname, like "registry.terraform.io/*/*" - matchAddr := addrs.Provider{ - Hostname: host, - // Other fields are intentionally left empty, to make this invalid - // as a specific provider address. - } - - for _, selector := range source { - // If this source has suitable matching patterns to install from - // the given hostname then we'll recursively search inside it - // for *RegistrySource objects. - if selector.CanHandleProvider(matchAddr) { - ret := findLegacyProviderLookupSource(host, selector.Source) - if ret != nil { - return ret - } - } - } - - // If we get here then there were no selectors that are both configured - // to handle modules from the given hostname and that are registry - // sources, so we fail. - return nil - - default: - // This source cannot be and cannot contain a *RegistrySource, so - // we fail. - return nil - } -} diff --git a/internal/getproviders/legacy_lookup_test.go b/internal/getproviders/legacy_lookup_test.go deleted file mode 100644 index c53e57baa121..000000000000 --- a/internal/getproviders/legacy_lookup_test.go +++ /dev/null @@ -1,102 +0,0 @@ -package getproviders - -import ( - "context" - "strings" - "testing" - - "github.com/hashicorp/terraform/addrs" -) - -func TestLookupLegacyProvider(t *testing.T) { - source, _, close := testRegistrySource(t) - defer close() - - got, gotMoved, err := LookupLegacyProvider( - context.Background(), - addrs.NewLegacyProvider("legacy"), - source, - ) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - want := addrs.Provider{ - Hostname: defaultRegistryHost, - Namespace: "legacycorp", - Type: "legacy", - } - 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( - context.Background(), - 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( - context.Background(), - 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( - context.Background(), - 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 cea30d0dc3a4..8c0256661b99 100644 --- a/internal/getproviders/registry_client.go +++ b/internal/getproviders/registry_client.go @@ -408,87 +408,6 @@ FindMatch: return match, nil } -// 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. -// -// 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(ctx context.Context, 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 - } - endpointURL := c.baseURL.ResolveReference(endpointPath) - - req, err := retryablehttp.NewRequest("GET", endpointURL.String(), nil) - if err != nil { - return "", "", err - } - req = req.WithContext(ctx) - c.addHeadersToRequest(req.Request) - - // This is just to give us something to return in error messages. It's - // not a proper provider address. - placeholderProviderAddr := addrs.NewLegacyProvider(typeName) - - resp, err := c.httpClient.Do(req) - if err != nil { - return "", "", c.errQueryFailed(placeholderProviderAddr, err) - } - defer resp.Body.Close() - - switch resp.StatusCode { - case http.StatusOK: - // Great! - case http.StatusNotFound: - return "", "", ErrProviderNotFound{ - Provider: placeholderProviderAddr, - } - case http.StatusUnauthorized, http.StatusForbidden: - return "", "", c.errUnauthorized(placeholderProviderAddr.Hostname) - default: - return "", "", c.errQueryFailed(placeholderProviderAddr, errors.New(resp.Status)) - } - - type ResponseBody struct { - 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) - } - - 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) - } - - 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) { if c.creds != nil { c.creds.PrepareRequest(req) diff --git a/internal/getproviders/registry_source.go b/internal/getproviders/registry_source.go index 0225a0d6faad..e227438c599a 100644 --- a/internal/getproviders/registry_source.go +++ b/internal/getproviders/registry_source.go @@ -104,29 +104,6 @@ func (s *RegistrySource) PackageMeta(ctx context.Context, provider addrs.Provide return client.PackageMeta(ctx, provider, version, target) } -// LookupLegacyProviderNamespace is a special method available only on -// RegistrySource which can deal with legacy provider addresses that contain -// only a type and leave the namespace implied. -// -// It asks the registry at the given hostname to provide a default namespace -// for the given provider type, which can be combined with the given hostname -// and type name to produce a fully-qualified provider address. -// -// Not all unqualified type names can be resolved to a default namespace. If -// the request fails, this method returns an error describing the failure. -// -// 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, and this fallback mechanism will likely be removed altogether -// in a future Terraform version. -func (s *RegistrySource) LookupLegacyProviderNamespace(ctx context.Context, hostname svchost.Hostname, typeName string) (string, string, error) { - client, err := s.registryClient(hostname) - if err != nil { - return "", "", err - } - return client.LegacyProviderDefaultNamespace(ctx, typeName) -} - func (s *RegistrySource) registryClient(hostname svchost.Hostname) (*registryClient, error) { host, err := s.services.Discover(hostname) if err != nil {