From 3547f9e368185121528494536ddf9fc35643aa2b Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 1 Sep 2020 13:58:23 -0400 Subject: [PATCH 1/3] format: Don't wrap space-prefixed diag details Diagnostic detail lines sometimes contain lines which include commands suggested for the user to execute. By convention, these start with leading whitespace to indicate that they are not prose. This commit changes the diagnostic formatter to wrap each line of the detail separately, and skips word wrapping for lines prefixed with space. This prevents ugly and confusing wrapping of long command lines. --- command/format/diagnostic.go | 12 +++++++++--- command/format/diagnostic_test.go | 32 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/command/format/diagnostic.go b/command/format/diagnostic.go index fa7584a4b..6ceb30894 100644 --- a/command/format/diagnostic.go +++ b/command/format/diagnostic.go @@ -175,11 +175,17 @@ func Diagnostic(diag tfdiags.Diagnostic, sources map[string][]byte, color *color } if desc.Detail != "" { - detail := desc.Detail if width != 0 { - detail = wordwrap.WrapString(detail, uint(width)) + lines := strings.Split(desc.Detail, "\n") + for _, line := range lines { + if !strings.HasPrefix(line, " ") { + line = wordwrap.WrapString(line, uint(width)) + } + fmt.Fprintf(&buf, "%s\n", line) + } + } else { + fmt.Fprintf(&buf, "%s\n", desc.Detail) } - fmt.Fprintf(&buf, "%s\n", detail) } return buf.String() diff --git a/command/format/diagnostic_test.go b/command/format/diagnostic_test.go index 98b879d38..2062f633c 100644 --- a/command/format/diagnostic_test.go +++ b/command/format/diagnostic_test.go @@ -167,3 +167,35 @@ Error: Some error t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected) } } + +func TestDiagnostic_wrapDetailIncludingCommand(t *testing.T) { + var diags tfdiags.Diagnostics + + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Everything went wrong", + Detail: "This is a very long sentence about whatever went wrong which is supposed to wrap onto multiple lines. Thank-you very much for listening.\n\nTo fix this, run this very long command:\n terraform read-my-mind -please -thanks -but-do-not-wrap-this-line-because-it-is-prefixed-with-spaces\n\nHere is a coda which is also long enough to wrap and so it should eventually make it onto multiple lines. THE END", + }) + color := &colorstring.Colorize{ + Colors: colorstring.DefaultColors, + Reset: true, + Disable: true, + } + expected := ` +Error: Everything went wrong + +This is a very long sentence about whatever went wrong which is supposed to +wrap onto multiple lines. Thank-you very much for listening. + +To fix this, run this very long command: + terraform read-my-mind -please -thanks -but-do-not-wrap-this-line-because-it-is-prefixed-with-spaces + +Here is a coda which is also long enough to wrap and so it should eventually +make it onto multiple lines. THE END +` + output := Diagnostic(diags[0], nil, color, 76) + + if output != expected { + t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected) + } +} From 9f824c53a5b1400ec0a610fe9a917c4bc2ce2ced Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Mon, 31 Aug 2020 17:02:05 -0400 Subject: [PATCH 2/3] command: Better in-house provider install errors When init attempts to install a legacy provider required by state and fails, but another provider with the same type is successfully installed, this almost definitely means that the user is migrating an in-house provider. The solution here is to use the `terraform state replace-provider` subcommand. This commit makes that next step clearer, by detecting this specific case, and displaying a list of commands to fix the existing state provider references. --- command/init.go | 69 +++++++++++++++++-- command/init_test.go | 46 +++++++++++++ .../main.tf | 12 ++++ .../terraform.tfstate | 25 +++++++ 4 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 command/testdata/init-get-provider-legacy-from-state/main.tf create mode 100644 command/testdata/init-get-provider-legacy-from-state/terraform.tfstate diff --git a/command/init.go b/command/init.go index 7bf9f3d1b..392fb5e14 100644 --- a/command/init.go +++ b/command/init.go @@ -427,8 +427,9 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, if moreDiags.HasErrors() { return false, diags } + stateReqs := make(getproviders.Requirements, 0) if state != nil { - stateReqs := state.ProviderRequirements() + stateReqs = state.ProviderRequirements() reqs = reqs.Merge(stateReqs) } @@ -456,6 +457,11 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, // 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 @@ -509,13 +515,19 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, ), )) 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() { + // 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", @@ -771,6 +783,53 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State, )) } + // 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 0b5d7bdb8..1eb5b8aef 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -1026,6 +1026,52 @@ func TestInit_getProviderSource(t *testing.T) { } } +func TestInit_getProviderLegacyFromState(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + copy.CopyDir(testFixturePath("init-get-provider-legacy-from-state"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + overrides := metaOverridesForProvider(testProvider()) + ui := new(cli.MockUi) + providerSource, close := newMockProviderSource(t, map[string][]string{ + "acme/alpha": {"1.2.3"}, + }) + defer close() + m := Meta{ + testingOverrides: overrides, + Ui: ui, + ProviderSource: providerSource, + } + + c := &InitCommand{ + Meta: m, + } + + if code := c.Run(nil); code != 1 { + t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, ui.ErrorWriter.String(), ui.OutputWriter.String()) + } + + // 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", + } + got := ui.ErrorWriter.String() + for _, want := range wants { + if !strings.Contains(got, want) { + 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) { // Create a temporary working directory that is empty td := tempDir(t) diff --git a/command/testdata/init-get-provider-legacy-from-state/main.tf b/command/testdata/init-get-provider-legacy-from-state/main.tf new file mode 100644 index 000000000..366518192 --- /dev/null +++ b/command/testdata/init-get-provider-legacy-from-state/main.tf @@ -0,0 +1,12 @@ +terraform { + required_providers { + alpha = { + source = "acme/alpha" + version = "1.2.3" + } + } +} + +resource "alpha_resource" "a" { + index = 1 +} diff --git a/command/testdata/init-get-provider-legacy-from-state/terraform.tfstate b/command/testdata/init-get-provider-legacy-from-state/terraform.tfstate new file mode 100644 index 000000000..33b2fb0c5 --- /dev/null +++ b/command/testdata/init-get-provider-legacy-from-state/terraform.tfstate @@ -0,0 +1,25 @@ +{ + "version": 4, + "terraform_version": "0.12.28", + "serial": 1, + "lineage": "481bf512-f245-4c60-42dc-7005f4fa9181", + "outputs": {}, + "resources": [ + { + "mode": "managed", + "type": "alpha_resource", + "name": "a", + "provider": "provider.alpha", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "a", + "index": 1 + }, + "private": "bnVsbA==" + } + ] + } + ] +} From f795083225ba55d8e3d8e409e6e06c3a938009d0 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 1 Sep 2020 10:19:10 -0400 Subject: [PATCH 3/3] website: Update 0.13 upgrade for legacy providers The error diagnostic shown when legacy state contains resources from in-house providers has changed, so update references to it in the 0.13 upgrade guide. --- website/upgrade-guides/0-13.html.markdown | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/website/upgrade-guides/0-13.html.markdown b/website/upgrade-guides/0-13.html.markdown index 2e306d6c1..2d1bf63b1 100644 --- a/website/upgrade-guides/0-13.html.markdown +++ b/website/upgrade-guides/0-13.html.markdown @@ -302,11 +302,13 @@ situation, `terraform init` will produce the following error message after you complete the configuration changes described above: ``` -Error: Failed to query available provider packages +Error: Failed to install legacy providers required by state -Could not retrieve the list of available versions for provider -/happycloud: -provider registry registry.terraform.io does not have a provider named -registry.terraform.io/-/happycloud +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 command: + + terraform state replace-provider registry.terraform.io/-/happycloud terraform.example.com/awesomecorp/happycloud ``` Provider source addresses starting with `registry.terraform.io/-/` are a special