From 3c8b46fffe538d1e4edf28f769bf89402eee1531 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 26 Mar 2019 09:11:43 -0400 Subject: [PATCH] merge connection blocks for validation The resource connection block was not being validated. Merge the two bodies, with the provider as the override, before validation. --- terraform/context_validate_test.go | 68 +++++++++++++++++++ terraform/eval_validate.go | 11 ++- terraform/eval_validate_test.go | 37 +++++----- terraform/node_resource_validate.go | 9 ++- .../validate-bad-prov-connection/main.tf | 8 +++ .../validate-bad-resource-connection/main.tf | 8 +++ 6 files changed, 116 insertions(+), 25 deletions(-) create mode 100644 terraform/test-fixtures/validate-bad-prov-connection/main.tf create mode 100644 terraform/test-fixtures/validate-bad-resource-connection/main.tf diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index e1518ee1c..1b6d571af 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -719,6 +719,74 @@ func TestContext2Validate_provisionerConfig_bad(t *testing.T) { } } +func TestContext2Validate_badResourceConnection(t *testing.T) { + m := testModule(t, "validate-bad-resource-connection") + p := testProvider("aws") + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "aws_instance": { + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true}, + }, + }, + }, + } + + pr := simpleMockProvisioner() + + c := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + Provisioners: map[string]ProvisionerFactory{ + "shell": testProvisionerFuncFixed(pr), + }, + }) + + diags := c.Validate() + t.Log(diags.Err()) + if !diags.HasErrors() { + t.Fatalf("succeeded; want error") + } +} + +func TestContext2Validate_badProvisionerConnection(t *testing.T) { + m := testModule(t, "validate-bad-prov-connection") + p := testProvider("aws") + p.GetSchemaReturn = &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "aws_instance": { + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true}, + }, + }, + }, + } + + pr := simpleMockProvisioner() + + c := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + Provisioners: map[string]ProvisionerFactory{ + "shell": testProvisionerFuncFixed(pr), + }, + }) + + diags := c.Validate() + t.Log(diags.Err()) + if !diags.HasErrors() { + t.Fatalf("succeeded; want error") + } +} + func TestContext2Validate_provisionerConfig_good(t *testing.T) { m := testModule(t, "validate-bad-prov-conf") p := testProvider("aws") diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index a7e625612..0033e01ac 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -109,13 +109,13 @@ func (n *EvalValidateProvider) Eval(ctx EvalContext) (interface{}, error) { } // EvalValidateProvisioner is an EvalNode implementation that validates -// the configuration of a provisioner belonging to a resource. +// the configuration of a provisioner belonging to a resource. The provisioner +// config is expected to contain the merged connection configurations. type EvalValidateProvisioner struct { ResourceAddr addrs.Resource Provisioner *provisioners.Interface Schema **configschema.Block Config *configs.Provisioner - ConnConfig *configs.Connection ResourceHasCount bool } @@ -149,10 +149,9 @@ func (n *EvalValidateProvisioner) Eval(ctx EvalContext) (interface{}, error) { } { - // Now validate the connection config, which might either be from - // the provisioner block itself or inherited from the resource's - // shared connection info. - connDiags := n.validateConnConfig(ctx, n.ConnConfig, n.ResourceAddr) + // Now validate the connection config, which contains the merged bodies + // of the resource and provisioner connection blocks. + connDiags := n.validateConnConfig(ctx, config.Connection, n.ResourceAddr) diags = diags.Append(connDiags) } diff --git a/terraform/eval_validate_test.go b/terraform/eval_validate_test.go index 3f5861a64..7a49c451a 100644 --- a/terraform/eval_validate_test.go +++ b/terraform/eval_validate_test.go @@ -372,11 +372,12 @@ func TestEvalValidateProvisioner_valid(t *testing.T) { Config: &configs.Provisioner{ Type: "baz", Config: hcl.EmptyBody(), - }, - ConnConfig: &configs.Connection{ - Config: configs.SynthBody("", map[string]cty.Value{ - "host": cty.StringVal("foo"), - }), + Connection: &configs.Connection{ + Config: configs.SynthBody("", map[string]cty.Value{ + "host": cty.StringVal("localhost"), + "type": cty.StringVal("ssh"), + }), + }, }, } @@ -419,12 +420,12 @@ func TestEvalValidateProvisioner_warning(t *testing.T) { Config: &configs.Provisioner{ Type: "baz", Config: hcl.EmptyBody(), - }, - ConnConfig: &configs.Connection{ - Config: configs.SynthBody("", map[string]cty.Value{ - "host": cty.StringVal("localhost"), - "type": cty.StringVal("ssh"), - }), + Connection: &configs.Connection{ + Config: configs.SynthBody("", map[string]cty.Value{ + "host": cty.StringVal("localhost"), + "type": cty.StringVal("ssh"), + }), + }, }, } @@ -477,13 +478,13 @@ func TestEvalValidateProvisioner_connectionInvalid(t *testing.T) { Config: &configs.Provisioner{ Type: "baz", Config: hcl.EmptyBody(), - }, - ConnConfig: &configs.Connection{ - Config: configs.SynthBody("", map[string]cty.Value{ - "type": cty.StringVal("ssh"), - "bananananananana": cty.StringVal("foo"), - "bazaz": cty.StringVal("bar"), - }), + Connection: &configs.Connection{ + Config: configs.SynthBody("", map[string]cty.Value{ + "type": cty.StringVal("ssh"), + "bananananananana": cty.StringVal("foo"), + "bazaz": cty.StringVal("bar"), + }), + }, }, } diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go index c1e10a6b2..734ec9e2b 100644 --- a/terraform/node_resource_validate.go +++ b/terraform/node_resource_validate.go @@ -1,6 +1,7 @@ package terraform import ( + "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/provisioners" @@ -58,6 +59,13 @@ func (n *NodeValidatableResource) EvalTree() EvalNode { for _, p := range managed.Provisioners { var provisioner provisioners.Interface var provisionerSchema *configschema.Block + + if p.Connection == nil { + p.Connection = config.Managed.Connection + } else if config.Managed.Connection != nil { + p.Connection.Config = configs.MergeBodies(config.Managed.Connection.Config, p.Connection.Config) + } + seq.Nodes = append( seq.Nodes, &EvalGetProvisioner{ @@ -71,7 +79,6 @@ func (n *NodeValidatableResource) EvalTree() EvalNode { Schema: &provisionerSchema, Config: p, ResourceHasCount: hasCount, - ConnConfig: p.Connection, }, ) } diff --git a/terraform/test-fixtures/validate-bad-prov-connection/main.tf b/terraform/test-fixtures/validate-bad-prov-connection/main.tf new file mode 100644 index 000000000..550714ff1 --- /dev/null +++ b/terraform/test-fixtures/validate-bad-prov-connection/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + provisioner "shell" { + test_string = "test" + connection { + user = "test" + } + } +} diff --git a/terraform/test-fixtures/validate-bad-resource-connection/main.tf b/terraform/test-fixtures/validate-bad-resource-connection/main.tf new file mode 100644 index 000000000..46a167175 --- /dev/null +++ b/terraform/test-fixtures/validate-bad-resource-connection/main.tf @@ -0,0 +1,8 @@ +resource "aws_instance" "foo" { + connection { + user = "test" + } + provisioner "shell" { + test_string = "test" + } +}