From 9a5c865040d006fc128c02853d5644fe88431ada Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 6 Dec 2017 16:41:48 -0800 Subject: [PATCH] command: validate config as part of loading it Previously we required callers to separately call .Validate on the root module to determine if there were any value errors, but we did that inconsistently and would thus see crashes in some cases where later code would try to use invalid configuration as if it were valid. Now we run .Validate automatically after config loading, returning the resulting diagnostics. Since we return a diagnostics here, it's possible to return both warnings and errors. We return the loaded module even if it's invalid, so callers are free to ignore returned errors and try to work with the config anyway, though they will need to be defensive against invalid configuration themselves in that case. As a result of this, all of the commands that load configuration now need to use diagnostic printing to signal errors. For the moment this just allows us to return potentially-multiple config errors/warnings in full fidelity, but also sets us up for later when more subsystems are able to produce rich diagnostics so we can show them all together. Finally, this commit also removes some stale, commented-out code for the "legacy" (pre-0.8) graph implementation, which has not been available for some time. --- command/apply.go | 36 +++++++++++++++--------------------- command/console.go | 9 ++++++--- command/graph.go | 20 +++++++++++++++++--- command/import.go | 21 ++++++++++++++++----- command/init.go | 19 +++++++------------ command/meta_new.go | 20 ++++++++++++++++---- command/plan.go | 27 +++++++++++++-------------- command/providers.go | 19 ++++++++----------- command/push.go | 12 +++++++++--- command/refresh.go | 17 +++++++++++------ command/validate.go | 33 ++++++++++++++++++++++----------- 11 files changed, 140 insertions(+), 93 deletions(-) diff --git a/command/apply.go b/command/apply.go index 84b16ce17f..c65b2df517 100644 --- a/command/apply.go +++ b/command/apply.go @@ -8,7 +8,8 @@ import ( "sort" "strings" - "github.com/hashicorp/errwrap" + "github.com/hashicorp/terraform/tfdiags" + "github.com/hashicorp/go-getter" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" @@ -118,31 +119,20 @@ func (c *ApplyCommand) Run(args []string) int { configPath = "" } + var diags tfdiags.Diagnostics + // Load the module if we don't have one yet (not running from plan) var mod *module.Tree if plan == nil { - mod, err = c.Module(configPath) - if err != nil { - err = errwrap.Wrapf("Failed to load root config module: {{err}}", err) - c.showDiagnostics(err) + var modDiags tfdiags.Diagnostics + mod, modDiags = c.Module(configPath) + diags = diags.Append(modDiags) + if modDiags.HasErrors() { + c.showDiagnostics(diags) return 1 } } - /* - terraform.SetDebugInfo(DefaultDataDir) - - // Check for the legacy graph - if experiment.Enabled(experiment.X_legacyGraph) { - c.Ui.Output(c.Colorize().Color( - "[reset][bold][yellow]" + - "Legacy graph enabled! This will use the graph from Terraform 0.7.x\n" + - "to execute this operation. This will be removed in the future so\n" + - "please report any issues causing you to use this to the Terraform\n" + - "project.\n\n")) - } - */ - var conf *config.Config if mod != nil { conf = mod.Config() @@ -198,11 +188,15 @@ func (c *ApplyCommand) Run(args []string) int { } case <-op.Done(): if err := op.Err; err != nil { - c.showDiagnostics(err) - return 1 + diags = diags.Append(err) } } + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } + if !c.Destroy { // Get the right module that we used. If we ran a plan, then use // that module. diff --git a/command/console.go b/command/console.go index b1361c26fd..cf7e15f61e 100644 --- a/command/console.go +++ b/command/console.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/helper/wrappedstreams" "github.com/hashicorp/terraform/repl" + "github.com/hashicorp/terraform/tfdiags" "github.com/mitchellh/cli" ) @@ -38,10 +39,12 @@ func (c *ConsoleCommand) Run(args []string) int { return 1 } + var diags tfdiags.Diagnostics + // Load the module - mod, err := c.Module(configPath) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err)) + mod, diags := c.Module(configPath) + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } diff --git a/command/graph.go b/command/graph.go index 2fb29c7b0a..7723043e8f 100644 --- a/command/graph.go +++ b/command/graph.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" + "github.com/hashicorp/terraform/tfdiags" + "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" @@ -56,12 +58,16 @@ func (c *GraphCommand) Run(args []string) int { configPath = "" } + var diags tfdiags.Diagnostics + // Load the module var mod *module.Tree if plan == nil { - mod, err = c.Module(configPath) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err)) + var modDiags tfdiags.Diagnostics + mod, modDiags = c.Module(configPath) + diags = diags.Append(modDiags) + if modDiags.HasErrors() { + c.showDiagnostics(diags) return 1 } } @@ -143,6 +149,14 @@ func (c *GraphCommand) Run(args []string) int { return 1 } + if diags.HasErrors() { + // For this command we only show diagnostics if there are errors, + // because printing out naked warnings could upset a naive program + // consuming our dot output. + c.showDiagnostics(diags) + return 1 + } + c.Ui.Output(graphStr) return 0 diff --git a/command/import.go b/command/import.go index 0dc2003a02..5123a032f9 100644 --- a/command/import.go +++ b/command/import.go @@ -6,6 +6,8 @@ import ( "os" "strings" + "github.com/hashicorp/terraform/tfdiags" + "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" @@ -71,13 +73,16 @@ func (c *ImportCommand) Run(args []string) int { return 1 } + var diags tfdiags.Diagnostics + // Load the module var mod *module.Tree if configPath != "" { - var err error - mod, err = c.Module(configPath) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err)) + var modDiags tfdiags.Diagnostics + mod, modDiags = c.Module(configPath) + diags = diags.Append(modDiags) + if modDiags.HasErrors() { + c.showDiagnostics(diags) return 1 } } @@ -166,7 +171,8 @@ func (c *ImportCommand) Run(args []string) int { }, }) if err != nil { - c.Ui.Error(fmt.Sprintf("Error importing: %s", err)) + diags = diags.Append(err) + c.showDiagnostics(diags) return 1 } @@ -187,6 +193,11 @@ func (c *ImportCommand) Run(args []string) int { c.Ui.Output(c.Colorize().Color("[reset][yellow]\n" + importCommandAllowMissingResourceMsg)) } + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } + return 0 } diff --git a/command/init.go b/command/init.go index 57e5b387db..66b1c7028a 100644 --- a/command/init.go +++ b/command/init.go @@ -274,20 +274,15 @@ func (c *InitCommand) Run(args []string) int { // Load the complete module tree, and fetch any missing providers. // This method outputs its own Ui. func (c *InitCommand) getProviders(path string, state *terraform.State, upgrade bool) error { - mod, err := c.Module(path) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error getting plugins: %s", err)) - return err - } - - if diags := mod.Validate(); diags.HasErrors() { - err := diags.Err() - c.Ui.Error(fmt.Sprintf("Error getting plugins: %s", err)) - return err + mod, diags := c.Module(path) + if diags.HasErrors() { + c.showDiagnostics(diags) + return diags.Err() } if err := terraform.CheckRequiredVersion(mod); err != nil { - c.Ui.Error(err.Error()) + diags = diags.Append(err) + c.showDiagnostics(diags) return err } @@ -397,7 +392,7 @@ func (c *InitCommand) getProviders(path string, state *terraform.State, upgrade digests[name] = nil } } - err = c.providerPluginsLock().Write(digests) + err := c.providerPluginsLock().Write(digests) if err != nil { c.Ui.Error(fmt.Sprintf("failed to save provider manifest: %s", err)) return err diff --git a/command/meta_new.go b/command/meta_new.go index 5fc3cdca36..9a935a3ca0 100644 --- a/command/meta_new.go +++ b/command/meta_new.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/terraform" + "github.com/hashicorp/terraform/tfdiags" ) // NOTE: Temporary file until this branch is cleaned up. @@ -34,7 +35,14 @@ func (m *Meta) Input() bool { // // It expects the modules to already be downloaded. This will never // download any modules. -func (m *Meta) Module(path string) (*module.Tree, error) { +// +// The configuration is validated before returning, so the returned diagnostics +// may contain warnings and/or errors. If the diagnostics contains only +// warnings, the caller may treat the returned module.Tree as valid after +// presenting the warnings to the user. +func (m *Meta) Module(path string) (*module.Tree, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + mod, err := module.NewTreeModule("", path) if err != nil { // Check for the error where we have no config files @@ -42,15 +50,19 @@ func (m *Meta) Module(path string) (*module.Tree, error) { return nil, nil } - return nil, err + diags = diags.Append(err) + return nil, diags } err = mod.Load(m.moduleStorage(m.DataDir(), module.GetModeNone)) if err != nil { - return nil, errwrap.Wrapf("Error loading modules: {{err}}", err) + diags = diags.Append(errwrap.Wrapf("Error loading modules: {{err}}", err)) + return nil, diags } - return mod, nil + diags = diags.Append(mod.Validate()) + + return mod, diags } // Config loads the root config for the path specified. Path may be a directory diff --git a/command/plan.go b/command/plan.go index bef831e16d..ec882b6393 100644 --- a/command/plan.go +++ b/command/plan.go @@ -5,10 +5,10 @@ import ( "fmt" "strings" - "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/tfdiags" ) // PlanCommand is a Command implementation that compares a Terraform @@ -69,13 +69,16 @@ func (c *PlanCommand) Run(args []string) int { configPath = "" } + var diags tfdiags.Diagnostics + // Load the module if we don't have one yet (not running from plan) var mod *module.Tree if plan == nil { - mod, err = c.Module(configPath) - if err != nil { - err = errwrap.Wrapf("Failed to load root config module: {{err}}", err) - c.showDiagnostics(err) + var modDiags tfdiags.Diagnostics + mod, modDiags = c.Module(configPath) + diags = diags.Append(modDiags) + if modDiags.HasErrors() { + c.showDiagnostics(diags) return 1 } } @@ -131,18 +134,14 @@ func (c *PlanCommand) Run(args []string) int { } case <-op.Done(): if err := op.Err; err != nil { - c.showDiagnostics(err) - return 1 + diags = diags.Append(err) } } - /* - err = terraform.SetDebugInfo(DefaultDataDir) - if err != nil { - c.Ui.Error(err.Error()) - return 1 - } - */ + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } if detailed && !op.PlanEmpty { return 2 diff --git a/command/providers.go b/command/providers.go index a89136a046..49d43962ea 100644 --- a/command/providers.go +++ b/command/providers.go @@ -39,9 +39,9 @@ func (c *ProvidersCommand) Run(args []string) int { } // Load the config - root, err := c.Module(configPath) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err)) + root, diags := c.Module(configPath) + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } if root == nil { @@ -52,14 +52,6 @@ func (c *ProvidersCommand) Run(args []string) int { return 1 } - // Validate the config (to ensure the version constraints are valid) - if diags := root.Validate(); len(diags) != 0 { - c.showDiagnostics(diags) - if diags.HasErrors() { - return 1 - } - } - // Load the backend b, err := c.Backend(&BackendOpts{ Config: root.Config(), @@ -91,6 +83,11 @@ func (c *ProvidersCommand) Run(args []string) int { c.Ui.Output(printRoot.String()) + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } + return 0 } diff --git a/command/push.go b/command/push.go index 1cced2d91a..039696fd38 100644 --- a/command/push.go +++ b/command/push.go @@ -89,9 +89,9 @@ func (c *PushCommand) Run(args []string) int { } // Load the module - mod, err := c.Module(configPath) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err)) + mod, diags := c.Module(configPath) + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } if mod == nil { @@ -347,6 +347,12 @@ func (c *PushCommand) Run(args []string) int { c.Ui.Output(c.Colorize().Color(fmt.Sprintf( "[reset][bold][green]Configuration %q uploaded! (v%d)", name, vsn))) + + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } + return 0 } diff --git a/command/refresh.go b/command/refresh.go index f4b9921f14..eec74281ce 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -5,10 +5,10 @@ import ( "fmt" "strings" - "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/terraform" + "github.com/hashicorp/terraform/tfdiags" ) // RefreshCommand is a cli.Command implementation that refreshes the state @@ -41,11 +41,12 @@ func (c *RefreshCommand) Run(args []string) int { return 1 } + var diags tfdiags.Diagnostics + // Load the module - mod, err := c.Module(configPath) - if err != nil { - err = errwrap.Wrapf("Failed to load root config module: {{err}}", err) - c.showDiagnostics(err) + mod, diags := c.Module(configPath) + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } @@ -84,7 +85,11 @@ func (c *RefreshCommand) Run(args []string) int { // Wait for the operation to complete <-op.Done() if err := op.Err; err != nil { - c.showDiagnostics(err) + diags = diags.Append(err) + } + + c.showDiagnostics(diags) + if diags.HasErrors() { return 1 } diff --git a/command/validate.go b/command/validate.go index 335ebe6185..f48d38e4a5 100644 --- a/command/validate.go +++ b/command/validate.go @@ -5,6 +5,8 @@ import ( "path/filepath" "strings" + "github.com/hashicorp/terraform/tfdiags" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/terraform" ) @@ -95,22 +97,27 @@ Options: } func (c *ValidateCommand) validate(dir string, checkVars bool) int { + var diags tfdiags.Diagnostics + cfg, err := config.LoadDir(dir) if err != nil { + diags = diags.Append(err) c.showDiagnostics(err) return 1 } - if diags := cfg.Validate(); len(diags) != 0 { + + diags = diags.Append(cfg.Validate()) + + if diags.HasErrors() { c.showDiagnostics(diags) - if diags.HasErrors() { - return 1 - } + return 1 } if checkVars { - mod, err := c.Module(dir) - if err != nil { - c.showDiagnostics(err) + mod, modDiags := c.Module(dir) + diags = diags.Append(modDiags) + if modDiags.HasErrors() { + c.showDiagnostics(diags) return 1 } @@ -119,13 +126,17 @@ func (c *ValidateCommand) validate(dir string, checkVars bool) int { tfCtx, err := terraform.NewContext(opts) if err != nil { - c.showDiagnostics(err) + diags = diags.Append(err) + c.showDiagnostics(diags) return 1 } - if !c.validateContext(tfCtx) { - return 1 - } + diags = diags.Append(tfCtx.Validate()) + } + + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 } return 0