From 80ab867e5794bf33aae70ca315cf2c16fd8a7743 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 7 Jul 2020 15:19:41 -0700 Subject: [PATCH] command/init: Remove special 0.12upgrade heuristic For Terraform v0.12 we introduced a special loading mode where we would use the 0.11-syntax-compatible "earlyconfig" package as a heuristic to identify situations where it was likely that the user was trying to use 0.11-only syntax that the upgrade tool might help with. However, as the language has moved on that is no longer a suitable heuristic in Terraform 0.13 and later: other new additions to the language can cause the main loader to disagree with earlyconfig, which would lead us to give poor advice about how to respond. Instead, we'll now return the same generic "there are errors" message in all syntax error cases. We have an extra message for errors in this case (as compared to other commands) because "terraform init" is usually the first command a new user interacts with and so this message gives some extra explanation about what "terraform init" will do with the configuration once it's valid. This also includes a reset control character in the output of the message as part of our ongoing mission to stop Terraform printing out whole paragraphs of colored text, which can often be hard to read for various reasons. --- command/init.go | 59 ++++++++----------- command/init_test.go | 32 ---------- .../init-sniff-version-error.tf | 9 --- 3 files changed, 24 insertions(+), 76 deletions(-) delete mode 100644 command/testdata/init-sniff-version-error/init-sniff-version-error.tf diff --git a/command/init.go b/command/init.go index 2f93c1b4e..65d9acdd1 100644 --- a/command/init.go +++ b/command/init.go @@ -145,39 +145,41 @@ func (c *InitCommand) Run(args []string) int { return 0 } - // Before we do anything else, we'll try loading configuration with both - // our "normal" and "early" configuration codepaths. If early succeeds - // while normal fails, that strongly suggests that the configuration is - // using syntax that worked in 0.11 but no longer in v0.12. + // For Terraform v0.12 we introduced a special loading mode where we would + // use the 0.11-syntax-compatible "earlyconfig" package as a heuristic to + // identify situations where it was likely that the user was trying to use + // 0.11-only syntax that the upgrade tool might help with. + // + // However, as the language has moved on that is no longer a suitable + // heuristic in Terraform 0.13 and later: other new additions to the + // language can cause the main loader to disagree with earlyconfig, which + // would lead us to give poor advice about how to respond. + // + // For that reason, we no longer use a different error message in that + // situation, but for now we still use both codepaths because some of our + // initialization functionality remains built around "earlyconfig" and + // so we need to still load the module via that mechanism anyway until we + // can do some more invasive refactoring here. rootMod, confDiags := c.loadSingleModule(path) rootModEarly, earlyConfDiags := c.loadSingleModuleEarly(path) if confDiags.HasErrors() { - if earlyConfDiags.HasErrors() { - // If both parsers produced errors then we'll assume the config - // is _truly_ invalid and produce error messages as normal. - // Since this may be the user's first ever interaction with Terraform, - // we'll provide some additional context in this case. - c.Ui.Error(strings.TrimSpace(errInitConfigError)) - diags = diags.Append(confDiags) - c.showDiagnostics(diags) - return 1 - } - // If _only_ the main loader produced errors then that suggests the - // configuration is written in 0.11-style syntax. We will return an - // error suggesting the user upgrade their config manually or with - // Terraform v0.12 - c.Ui.Error(strings.TrimSpace(errInitConfigErrorMaybeLegacySyntax)) + c.Ui.Error(c.Colorize().Color(strings.TrimSpace(errInitConfigError))) + // TODO: It would be nice to check the version constraints in + // rootModEarly.RequiredCore and print out a hint if the module is + // declaring that it's not compatible with this version of Terraform, + // though we're deferring that for now because we're intending to + // refactor our use of "earlyconfig" here anyway and so whatever we + // might do here right now would likely be invalidated by that. c.showDiagnostics(confDiags) return 1 } - // If _only_ the early loader encountered errors then that's unusual // (it should generally be a superset of the normal loader) but we'll // return those errors anyway since otherwise we'll probably get // some weird behavior downstream. Errors from the early loader are // generally not as high-quality since it has less context to work with. if earlyConfDiags.HasErrors() { - c.Ui.Error(strings.TrimSpace(errInitConfigError)) + c.Ui.Error(c.Colorize().Color(strings.TrimSpace(errInitConfigError))) // Errors from the early loader are generally not as high-quality since // it has less context to work with. diags = diags.Append(confDiags) @@ -940,25 +942,12 @@ func (c *InitCommand) Synopsis() string { } const errInitConfigError = ` -There are some problems with the configuration, described below. +[reset]There are some problems with the configuration, described below. The Terraform configuration must be valid before initialization so that Terraform can determine which modules and providers need to be installed. ` -const errInitConfigErrorMaybeLegacySyntax = ` -There are some problems with the configuration, described below. - -Terraform found syntax errors in the configuration that prevented full -initialization. If you've recently upgraded to Terraform v0.13 from Terraform -v0.11, this may be because your configuration uses syntax constructs that are no -longer valid, and so must be updated before full initialization is possible. - -Manually update your configuration syntax, or install Terraform v0.12 and run -terraform init for this configuration at a shell prompt for more information -on how to update it for Terraform v0.12+ compatibility. -` - const errInitCopyNotEmpty = ` The working directory already contains files. The -from-module option requires an empty directory into which a copy of the referenced module will be placed. diff --git a/command/init_test.go b/command/init_test.go index 9fdc29ba0..1b05ce29f 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -1657,38 +1657,6 @@ func TestInit_invalidBuiltInProviders(t *testing.T) { } } -// The module in this test uses terraform 0.11-style syntax. We expect that the -// earlyconfig will succeed but the main loader fail, and return an error that -// indicates that syntax upgrades may be required. -func TestInit_syntaxErrorUpgradeHint(t *testing.T) { - // Create a temporary working directory that is empty - td := tempDir(t) - - // This module - copy.CopyDir(testFixturePath("init-sniff-version-error"), td) - defer os.RemoveAll(td) - defer testChdir(t, td)() - - ui := new(cli.MockUi) - c := &InitCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, - }, - } - - args := []string{} - if code := c.Run(args); code != 1 { - t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) - } - - // Check output. - output := ui.ErrorWriter.String() - if got, want := output, "If you've recently upgraded to Terraform v0.13 from Terraform\nv0.11, this may be because your configuration uses syntax constructs that are no\nlonger valid"; !strings.Contains(got, want) { - t.Fatalf("wrong output\ngot:\n%s\n\nwant: message containing %q", got, want) - } -} - // newMockProviderSource is a helper to succinctly construct a mock provider // source that contains a set of packages matching the given provider versions // that are available for installation (from temporary local files). diff --git a/command/testdata/init-sniff-version-error/init-sniff-version-error.tf b/command/testdata/init-sniff-version-error/init-sniff-version-error.tf deleted file mode 100644 index d797e5144..000000000 --- a/command/testdata/init-sniff-version-error/init-sniff-version-error.tf +++ /dev/null @@ -1,9 +0,0 @@ -# The following is invalid because we don't permit multiple nested blocks -# all one one line. Instead, we require the backend block to be on a line -# of its own. -# The purpose of this test case is to see that HCL still produces a valid-enough -# AST that we can try to sniff in this block for a terraform_version argument -# without crashing, since we do that during init to try to give a better -# error message if we detect that the configuration is for a newer Terraform -# version. -terraform { backend "local" {} }