Detect and reject unknown attributes in "connection" blocks

Since the validation of connection blocks is delegated to the communicator
selected by "type", we were not previously doing any validation of the
attribute names in these blocks until running provisioners during apply.

Proper validation here requires us to already have the instance state,
since the final connection info is a merge of values provided in config
with values assigned automatically by the resource. However, we can do
some basic name validation to catch typos during the validation pass, even
though semantic validation and checking for missing attributes will still
wait until the provisioner is instantiated.

This fixes #6582 as much as we reasonably can.

terraform/eval_validate.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/terraform/config"
+	"github.com/mitchellh/mapstructure"
 )
 
 // EvalValidateError is the error structure returned if there were
@@ -85,12 +86,31 @@ func (n *EvalValidateProvider) Eval(ctx EvalContext) (interface{}, error) {
 type EvalValidateProvisioner struct {
 	Provisioner *ResourceProvisioner
 	Config      **ResourceConfig
+	ConnConfig  **ResourceConfig
 }
 
 func (n *EvalValidateProvisioner) Eval(ctx EvalContext) (interface{}, error) {
 	provisioner := *n.Provisioner
 	config := *n.Config
-	warns, errs := provisioner.Validate(config)
+	var warns []string
+	var errs []error
+
+	{
+		// Validate the provisioner's own config first
+		w, e := provisioner.Validate(config)
+		warns = append(warns, w...)
+		errs = append(errs, e...)
+	}
+
+	{
+		// Now validate the connection config, which might either be from
+		// the provisioner block itself or inherited from the resource's
+		// shared connection info.
+		w, e := n.validateConnConfig(*n.ConnConfig)
+		warns = append(warns, w...)
+		errs = append(errs, e...)
+	}
+
 	if len(warns) == 0 && len(errs) == 0 {
 		return nil, nil
 	}
@@ -101,6 +121,64 @@ func (n *EvalValidateProvisioner) Eval(ctx EvalContext) (interface{}, error) {
 	}
 }
 
+func (n *EvalValidateProvisioner) validateConnConfig(connConfig *ResourceConfig) (warns []string, errs []error) {
+	// We can't comprehensively validate the connection config since its
+	// final structure is decided by the communicator and we can't instantiate
+	// that until we have a complete instance state. However, we *can* catch
+	// configuration keys that are not valid for *any* communicator, catching
+	// typos early rather than waiting until we actually try to run one of
+	// the resource's provisioners.
+
+	type connConfigSuperset struct {
+		// All attribute types are interface{} here because at this point we
+		// may still have unresolved interpolation expressions, which will
+		// appear as strings regardless of the final goal type.
+
+		Type       interface{} `mapstructure:"type"`
+		User       interface{} `mapstructure:"user"`
+		Password   interface{} `mapstructure:"password"`
+		Host       interface{} `mapstructure:"host"`
+		Port       interface{} `mapstructure:"port"`
+		Timeout    interface{} `mapstructure:"timeout"`
+		ScriptPath interface{} `mapstructure:"script_path"`
+
+		// For type=ssh only (enforced in ssh communicator)
+		PrivateKey        interface{} `mapstructure:"private_key"`
+		Agent             interface{} `mapstructure:"agent"`
+		BastionHost       interface{} `mapstructure:"bastion_host"`
+		BastionPort       interface{} `mapstructure:"bastion_port"`
+		BastionUser       interface{} `mapstructure:"bastion_user"`
+		BastionPassword   interface{} `mapstructure:"bastion_password"`
+		BastionPrivateKey interface{} `mapstructure:"bastion_private_key"`
+
+		// For type=winrm only (enforced in winrm communicator)
+		HTTPS    interface{} `mapstructure:"https"`
+		Insecure interface{} `mapstructure:"insecure"`
+		CACert   interface{} `mapstructure:"cacert"`
+	}
+
+	var metadata mapstructure.Metadata
+	decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
+		Metadata: &metadata,
+		Result:   &connConfigSuperset{}, // result is disregarded; we only care about unused keys
+	})
+	if err != nil {
+		// should never happen
+		errs = append(errs, err)
+		return
+	}
+
+	if err := decoder.Decode(connConfig.Config); err != nil {
+		errs = append(errs, err)
+		return
+	}
+
+	for _, attrName := range metadata.Unused {
+		errs = append(errs, fmt.Errorf("unknown 'connection' argument %q", attrName))
+	}
+	return
+}
+
 // EvalValidateResource is an EvalNode implementation that validates
 // the configuration of a resource.
 type EvalValidateResource struct {

terraform/eval_validate_test.go

@@ -178,3 +178,117 @@ func TestEvalValidateResource_ignoreWarnings(t *testing.T) {
 		t.Fatalf("Expected no error, got: %s", err)
 	}
 }
+
+func TestEvalValidateProvisioner_valid(t *testing.T) {
+	mp := &MockResourceProvisioner{}
+	var p ResourceProvisioner = mp
+	ctx := &MockEvalContext{}
+
+	cfg := &ResourceConfig{}
+	connInfo, err := config.NewRawConfig(map[string]interface{}{})
+	if err != nil {
+		t.Fatalf("failed to make connInfo: %s", err)
+	}
+	connConfig := NewResourceConfig(connInfo)
+
+	node := &EvalValidateProvisioner{
+		Provisioner: &p,
+		Config:      &cfg,
+		ConnConfig:  &connConfig,
+	}
+
+	result, err := node.Eval(ctx)
+	if err != nil {
+		t.Fatalf("node.Eval failed: %s", err)
+	}
+	if result != nil {
+		t.Errorf("node.Eval returned non-nil result")
+	}
+
+	if !mp.ValidateCalled {
+		t.Fatalf("p.Config not called")
+	}
+	if mp.ValidateConfig != cfg {
+		t.Errorf("p.Config called with wrong config")
+	}
+}
+
+func TestEvalValidateProvisioner_warning(t *testing.T) {
+	mp := &MockResourceProvisioner{}
+	var p ResourceProvisioner = mp
+	ctx := &MockEvalContext{}
+
+	cfg := &ResourceConfig{}
+	connInfo, err := config.NewRawConfig(map[string]interface{}{})
+	if err != nil {
+		t.Fatalf("failed to make connInfo: %s", err)
+	}
+	connConfig := NewResourceConfig(connInfo)
+
+	node := &EvalValidateProvisioner{
+		Provisioner: &p,
+		Config:      &cfg,
+		ConnConfig:  &connConfig,
+	}
+
+	mp.ValidateReturnWarns = []string{"foo is deprecated"}
+
+	_, err = node.Eval(ctx)
+	if err == nil {
+		t.Fatalf("node.Eval succeeded; want error")
+	}
+
+	valErr, ok := err.(*EvalValidateError)
+	if !ok {
+		t.Fatalf("node.Eval error is %#v; want *EvalValidateError", valErr)
+	}
+
+	warns := valErr.Warnings
+	if warns == nil || len(warns) != 1 {
+		t.Fatalf("wrong number of warnings in %#v; want one warning", warns)
+	}
+	if warns[0] != mp.ValidateReturnWarns[0] {
+		t.Fatalf("wrong warning %q; want %q", warns[0], mp.ValidateReturnWarns[0])
+	}
+}
+
+func TestEvalValidateProvisioner_connectionInvalid(t *testing.T) {
+	var p ResourceProvisioner = &MockResourceProvisioner{}
+	ctx := &MockEvalContext{}
+
+	cfg := &ResourceConfig{}
+	connInfo, err := config.NewRawConfig(map[string]interface{}{
+		"bananananananana": "foo",
+		"bazaz":            "bar",
+	})
+	if err != nil {
+		t.Fatalf("failed to make connInfo: %s", err)
+	}
+	connConfig := NewResourceConfig(connInfo)
+
+	node := &EvalValidateProvisioner{
+		Provisioner: &p,
+		Config:      &cfg,
+		ConnConfig:  &connConfig,
+	}
+
+	_, err = node.Eval(ctx)
+	if err == nil {
+		t.Fatalf("node.Eval succeeded; want error")
+	}
+
+	valErr, ok := err.(*EvalValidateError)
+	if !ok {
+		t.Fatalf("node.Eval error is %#v; want *EvalValidateError", valErr)
+	}
+
+	errs := valErr.Errors
+	if errs == nil || len(errs) != 2 {
+		t.Fatalf("wrong number of errors in %#v; want two errors", errs)
+	}
+
+	errStr := errs[0].Error()
+	if !(strings.Contains(errStr, "bananananananana") || strings.Contains(errStr, "bazaz")) {
+		t.Fatalf("wrong first error %q; want something about our invalid connInfo keys", errStr)
+	}
+}

terraform/node_resource_validate.go

@@ -129,17 +129,29 @@ func (n *NodeValidatableResourceInstance) EvalTree() EvalNode {
 	// Validate all the provisioners
 	for _, p := range n.Config.Provisioners {
 		var provisioner ResourceProvisioner
-		seq.Nodes = append(seq.Nodes, &EvalGetProvisioner{
-			Name:   p.Type,
-			Output: &provisioner,
-		}, &EvalInterpolate{
-			Config:   p.RawConfig.Copy(),
-			Resource: resource,
-			Output:   &config,
-		}, &EvalValidateProvisioner{
-			Provisioner: &provisioner,
-			Config:      &config,
-		})
+		var connConfig *ResourceConfig
+		seq.Nodes = append(
+			seq.Nodes,
+			&EvalGetProvisioner{
+				Name:   p.Type,
+				Output: &provisioner,
+			},
+			&EvalInterpolate{
+				Config:   p.RawConfig.Copy(),
+				Resource: resource,
+				Output:   &config,
+			},
+			&EvalInterpolate{
+				Config:   p.ConnInfo.Copy(),
+				Resource: resource,
+				Output:   &connConfig,
+			},
+			&EvalValidateProvisioner{
+				Provisioner: &provisioner,
+				Config:      &config,
+				ConnConfig:  &connConfig,
+			},
+		)
 	}
 
 	return seq