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

@@ -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)
+		var modDiags tfdiags.Diagnostics
-		if err != nil {
+		mod, modDiags = c.Module(configPath)
-			err = errwrap.Wrapf("Failed to load root config module: {{err}}", err)
+		diags = diags.Append(modDiags)
-			c.showDiagnostics(err)
+		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)
+			diags = diags.Append(err)
-			return 1
 		}
 	}
 
+	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.

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)
+	mod, diags := c.Module(configPath)
-	if err != nil {
+	if diags.HasErrors() {
-		c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err))
+		c.showDiagnostics(diags)
 		return 1
 	}
 

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)
+		var modDiags tfdiags.Diagnostics
-		if err != nil {
+		mod, modDiags = c.Module(configPath)
-			c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err))
+		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

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
+		var modDiags tfdiags.Diagnostics
-		mod, err = c.Module(configPath)
+		mod, modDiags = c.Module(configPath)
-		if err != nil {
+		diags = diags.Append(modDiags)
-			c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err))
+		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
 }
 

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)
+	mod, diags := c.Module(path)
-	if err != nil {
+	if diags.HasErrors() {
-		c.Ui.Error(fmt.Sprintf("Error getting plugins: %s", err))
+		c.showDiagnostics(diags)
-		return err
+		return diags.Err()
-	}
-
-	if diags := mod.Validate(); diags.HasErrors() {
-		err := diags.Err()
-		c.Ui.Error(fmt.Sprintf("Error getting plugins: %s", err))
-		return 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

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

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)
+		var modDiags tfdiags.Diagnostics
-		if err != nil {
+		mod, modDiags = c.Module(configPath)
-			err = errwrap.Wrapf("Failed to load root config module: {{err}}", err)
+		diags = diags.Append(modDiags)
-			c.showDiagnostics(err)
+		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)
+			diags = diags.Append(err)
-			return 1
 		}
 	}
 
-	/*
+	c.showDiagnostics(diags)
-		err = terraform.SetDebugInfo(DefaultDataDir)
+	if diags.HasErrors() {
-		if err != nil {
+		return 1
-			c.Ui.Error(err.Error())
+	}
-			return 1
-		}
-	*/
 
 	if detailed && !op.PlanEmpty {
 		return 2

command/providers.go

@@ -39,9 +39,9 @@ func (c *ProvidersCommand) Run(args []string) int {
 	}
 
 	// Load the config
-	root, err := c.Module(configPath)
+	root, diags := c.Module(configPath)
-	if err != nil {
+	if diags.HasErrors() {
-		c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err))
+		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
 }
 

command/push.go

@@ -89,9 +89,9 @@ func (c *PushCommand) Run(args []string) int {
 	}
 
 	// Load the module
-	mod, err := c.Module(configPath)
+	mod, diags := c.Module(configPath)
-	if err != nil {
+	if diags.HasErrors() {
-		c.Ui.Error(fmt.Sprintf("Failed to load root config module: %s", err))
+		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
 }
 

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)
+	mod, diags := c.Module(configPath)
-	if err != nil {
+	if diags.HasErrors() {
-		err = errwrap.Wrapf("Failed to load root config module: {{err}}", err)
+		c.showDiagnostics(diags)
-		c.showDiagnostics(err)
 		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
 	}
 

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)
+		mod, modDiags := c.Module(dir)
-		if err != nil {
+		diags = diags.Append(modDiags)
-			c.showDiagnostics(err)
+		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) {
+		diags = diags.Append(tfCtx.Validate())
-			return 1
+	}
-		}
+
+	c.showDiagnostics(diags)
+	if diags.HasErrors() {
+		return 1
 	}
 
 	return 0