Stop the validate node transforming the original config (#34026)

Author: liamcervante

internal/terraform/node_resource_validate.go

@@ -56,14 +56,7 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) (di
 	if managed := n.Config.Managed; managed != nil {
 		// Validate all the provisioners
 		for _, p := range managed.Provisioners {
-			if p.Connection == nil {
-				p.Connection = n.Config.Managed.Connection
-			} else if n.Config.Managed.Connection != nil {
-				p.Connection.Config = configs.MergeBodies(n.Config.Managed.Connection.Config, p.Connection.Config)
-			}
-
-			// Validate Provisioner Config
-			diags = diags.Append(n.validateProvisioner(ctx, p))
+			diags = diags.Append(n.validateProvisioner(ctx, p, n.Config.Managed.Connection))
 			if diags.HasErrors() {
 				return diags
 			}
@@ -75,7 +68,7 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) (di
 // validateProvisioner validates the configuration of a provisioner belonging to
 // a resource. The provisioner config is expected to contain the merged
 // connection configurations.
-func (n *NodeValidatableResource) validateProvisioner(ctx EvalContext, p *configs.Provisioner) tfdiags.Diagnostics {
+func (n *NodeValidatableResource) validateProvisioner(ctx EvalContext, p *configs.Provisioner, baseConn *configs.Connection) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
 
 	provisioner, err := ctx.Provisioner(p.Type)
@@ -120,8 +113,21 @@ func (n *NodeValidatableResource) validateProvisioner(ctx EvalContext, p *config
 		// 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.
-		_, _, connDiags := n.evaluateBlock(ctx, p.Connection.Config, connectionBlockSupersetSchema)
+
+		cfg := p.Connection.Config
+		if baseConn != nil {
+			// Merge the local config into the base connection config, if we
+			// both specified.
+			cfg = configs.MergeBodies(baseConn.Config, cfg)
+		}
+
+		_, _, connDiags := n.evaluateBlock(ctx, cfg, connectionBlockSupersetSchema)
+		diags = diags.Append(connDiags)
+	} else if baseConn != nil {
+		// Just validate the baseConn directly.
+		_, _, connDiags := n.evaluateBlock(ctx, baseConn.Config, connectionBlockSupersetSchema)
 		diags = diags.Append(connDiags)
+
 	}
 	return diags
 }

internal/terraform/node_resource_validate_test.go

@@ -10,14 +10,15 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hcltest"
+	"github.com/zclconf/go-cty/cty"
+
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/lang/marks"
 	"github.com/hashicorp/terraform/internal/providers"
 	"github.com/hashicorp/terraform/internal/provisioners"
 	"github.com/hashicorp/terraform/internal/tfdiags"
-	"github.com/zclconf/go-cty/cty"
 )
 
 func TestNodeValidatableResource_ValidateProvisioner_valid(t *testing.T) {
@@ -54,7 +55,7 @@ func TestNodeValidatableResource_ValidateProvisioner_valid(t *testing.T) {
 		},
 	}
 
-	diags := node.validateProvisioner(ctx, pc)
+	diags := node.validateProvisioner(ctx, pc, nil)
 	if diags.HasErrors() {
 		t.Fatalf("node.Eval failed: %s", diags.Err())
 	}
@@ -99,7 +100,7 @@ func TestNodeValidatableResource_ValidateProvisioner__warning(t *testing.T) {
 		}
 	}
 
-	diags := node.validateProvisioner(ctx, pc)
+	diags := node.validateProvisioner(ctx, pc, nil)
 	if len(diags) != 1 {
 		t.Fatalf("wrong number of diagnostics in %s; want one warning", diags.ErrWithWarnings())
 	}
@@ -144,7 +145,57 @@ func TestNodeValidatableResource_ValidateProvisioner__connectionInvalid(t *testi
 		},
 	}
 
-	diags := node.validateProvisioner(ctx, pc)
+	diags := node.validateProvisioner(ctx, pc, nil)
+	if !diags.HasErrors() {
+		t.Fatalf("node.Eval succeeded; want error")
+	}
+	if len(diags) != 3 {
+		t.Fatalf("wrong number of diagnostics; want two errors\n\n%s", diags.Err())
+	}
+
+	errStr := diags.Err().Error()
+	if !(strings.Contains(errStr, "bananananananana") && strings.Contains(errStr, "bazaz")) {
+		t.Fatalf("wrong errors %q; want something about each of our invalid connInfo keys", errStr)
+	}
+}
+
+func TestNodeValidatableResource_ValidateProvisioner_baseConnInvalid(t *testing.T) {
+	ctx := &MockEvalContext{}
+	ctx.installSimpleEval()
+	mp := &MockProvisioner{}
+	ps := &configschema.Block{}
+	ctx.ProvisionerSchemaSchema = ps
+	ctx.ProvisionerProvisioner = mp
+
+	pc := &configs.Provisioner{
+		Type:   "baz",
+		Config: hcl.EmptyBody(),
+	}
+
+	baseConn := &configs.Connection{
+		Config: configs.SynthBody("", map[string]cty.Value{
+			"type":             cty.StringVal("ssh"),
+			"bananananananana": cty.StringVal("foo"),
+			"bazaz":            cty.StringVal("bar"),
+		}),
+	}
+
+	rc := &configs.Resource{
+		Mode:    addrs.ManagedResourceMode,
+		Type:    "test_foo",
+		Name:    "bar",
+		Config:  configs.SynthBody("", map[string]cty.Value{}),
+		Managed: &configs.ManagedResource{},
+	}
+
+	node := NodeValidatableResource{
+		NodeAbstractResource: &NodeAbstractResource{
+			Addr:   mustConfigResourceAddr("test_foo.bar"),
+			Config: rc,
+		},
+	}
+
+	diags := node.validateProvisioner(ctx, pc, baseConn)
 	if !diags.HasErrors() {
 		t.Fatalf("node.Eval succeeded; want error")
 	}