From a2c59c6ecdf377849d15e85f9994bcd663edea0b Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 19 Oct 2017 16:43:18 -0700
Subject: [PATCH] main: validate credentials blocks in CLI config

We require that each "credentials" block has a valid hostname and that
there be no more than one "credentials_helper" block.

There are some more sophisticated validations we could do here, such as
checking if the same host is declared more than once, but since this
config handling will be rewritten to use HCL2 in the near future, and this
sort of check is easier to do in the HCL2 API, we just check the basic
stuff for now and plan to revisit later.
---
 config.go      | 40 +++++++++++++++++++++++++++++++
 config_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 main.go        | 23 ++++++++++++++++++
 3 files changed, 128 insertions(+)

diff --git a/config.go b/config.go
index 04334a58e..e5b3e73ad 100644
--- a/config.go
+++ b/config.go
@@ -8,7 +8,10 @@ import (
 	"os"
 
 	"github.com/hashicorp/hcl"
+
 	"github.com/hashicorp/terraform/command"
+	"github.com/hashicorp/terraform/svchost"
+	"github.com/hashicorp/terraform/tfdiags"
 )
 
 const pluginCacheDirEnvVar = "TF_PLUGIN_CACHE_DIR"
@@ -115,6 +118,43 @@ func EnvConfig() *Config {
 	return config
 }
 
+// Validate checks for errors in the configuration that cannot be detected
+// just by HCL decoding, returning any problems as diagnostics.
+//
+// On success, the returned diagnostics will return false from the HasErrors
+// method. A non-nil diagnostics is not necessarily an error, since it may
+// contain just warnings.
+func (c *Config) Validate() tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+
+	if c == nil {
+		return diags
+	}
+
+	// FIXME: Right now our config parsing doesn't retain enough information
+	// to give proper source references to any errors. We should improve
+	// on this when we change the CLI config parser to use HCL2.
+
+	// Check that all "credentials" blocks have valid hostnames.
+	for givenHost := range c.Credentials {
+		_, err := svchost.ForComparison(givenHost)
+		if err != nil {
+			diags = diags.Append(
+				fmt.Errorf("The credentials %q block has an invalid hostname: %s", givenHost, err),
+			)
+		}
+	}
+
+	// Should have zero or one "credentials_helper" blocks
+	if len(c.CredentialsHelpers) > 1 {
+		diags = diags.Append(
+			fmt.Errorf("No more than one credentials_helper block may be specified"),
+		)
+	}
+
+	return diags
+}
+
 // Merge merges two configurations and returns a third entirely
 // new configuration with the two merged.
 func (c1 *Config) Merge(c2 *Config) *Config {
diff --git a/config_test.go b/config_test.go
index 3884e9abd..28fcec92a 100644
--- a/config_test.go
+++ b/config_test.go
@@ -82,6 +82,71 @@ func TestLoadConfig_credentials(t *testing.T) {
 	}
 }
 
+func TestConfigValidate(t *testing.T) {
+	tests := map[string]struct {
+		Config    *Config
+		DiagCount int
+	}{
+		"nil": {
+			nil,
+			0,
+		},
+		"empty": {
+			&Config{},
+			0,
+		},
+		"credentials good": {
+			&Config{
+				Credentials: map[string]map[string]interface{}{
+					"example.com": map[string]interface{}{
+						"token": "foo",
+					},
+				},
+			},
+			0,
+		},
+		"credentials with bad hostname": {
+			&Config{
+				Credentials: map[string]map[string]interface{}{
+					"example..com": map[string]interface{}{
+						"token": "foo",
+					},
+				},
+			},
+			1, // credentials block has invalid hostname
+		},
+		"credentials helper good": {
+			&Config{
+				CredentialsHelpers: map[string]*ConfigCredentialsHelper{
+					"foo": {},
+				},
+			},
+			0,
+		},
+		"credentials helper too many": {
+			&Config{
+				CredentialsHelpers: map[string]*ConfigCredentialsHelper{
+					"foo": {},
+					"bar": {},
+				},
+			},
+			1, // no more than one credentials_helper block allowed
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			diags := test.Config.Validate()
+			if len(diags) != test.DiagCount {
+				t.Errorf("wrong number of diagnostics %d; want %d", len(diags), test.DiagCount)
+				for _, diag := range diags {
+					t.Logf("- %#v", diag.Description())
+				}
+			}
+		})
+	}
+}
+
 func TestConfig_Merge(t *testing.T) {
 	c1 := &Config{
 		Providers: map[string]string{
diff --git a/main.go b/main.go
index 67c4bfcb6..85a51c54a 100644
--- a/main.go
+++ b/main.go
@@ -11,9 +11,13 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/mitchellh/colorstring"
+
 	"github.com/hashicorp/go-plugin"
+	"github.com/hashicorp/terraform/command/format"
 	"github.com/hashicorp/terraform/helper/logging"
 	"github.com/hashicorp/terraform/terraform"
+	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/mattn/go-colorable"
 	"github.com/mattn/go-shellwords"
 	"github.com/mitchellh/cli"
@@ -145,6 +149,25 @@ func wrappedMain() int {
 
 	log.Printf("[DEBUG] CLI Config is %#v", config)
 
+	{
+		var diags tfdiags.Diagnostics
+		diags = diags.Append(config.Validate())
+		if len(diags) > 0 {
+			Ui.Error("There are some problems with the CLI configuration:")
+			for _, diag := range diags {
+				earlyColor := &colorstring.Colorize{
+					Colors:  colorstring.DefaultColors,
+					Disable: true, // Disable color to be conservative until we know better
+					Reset:   true,
+				}
+				Ui.Error(format.Diagnostic(diag, earlyColor, 78))
+			}
+			if diags.HasErrors() {
+				Ui.Error("As a result of the above problems, Terraform may not behave as intended.\n\n")
+			}
+		}
+	}
+
 	// In tests, Commands may already be set to provide mock commands
 	if Commands == nil {
 		initCommands(&config)