diff --git a/terraform/eval_context.go b/terraform/eval_context.go index 241662f7b..1448ae93d 100644 --- a/terraform/eval_context.go +++ b/terraform/eval_context.go @@ -77,11 +77,9 @@ type EvalContext interface { ProviderInput(addrs.AbsProviderConfig) map[string]cty.Value SetProviderInput(addrs.AbsProviderConfig, map[string]cty.Value) - // InitProvisioner initializes the provisioner with the given name and - // returns the implementation of the resource provisioner or an error. - // + // InitProvisioner initializes the provisioner with the given name. // It is an error to initialize the same provisioner more than once. - InitProvisioner(string) (provisioners.Interface, error) + InitProvisioner(string) error // Provisioner gets the provisioner instance with the given name (already // initialized) or returns nil if the provisioner isn't initialized. diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 4c2ff128c..efb88b310 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -227,10 +227,10 @@ func (ctx *BuiltinEvalContext) SetProviderInput(pc addrs.AbsProviderConfig, c ma ctx.ProviderLock.Unlock() } -func (ctx *BuiltinEvalContext) InitProvisioner(n string) (provisioners.Interface, error) { +func (ctx *BuiltinEvalContext) InitProvisioner(n string) error { // If we already initialized, it is an error if p := ctx.Provisioner(n); p != nil { - return nil, fmt.Errorf("Provisioner '%s' already initialized", n) + return fmt.Errorf("Provisioner '%s' already initialized", n) } // Warning: make sure to acquire these locks AFTER the call to Provisioner @@ -240,12 +240,12 @@ func (ctx *BuiltinEvalContext) InitProvisioner(n string) (provisioners.Interface p, err := ctx.Components.ResourceProvisioner(n) if err != nil { - return nil, err + return err } ctx.ProvisionerCache[n] = p - return p, nil + return nil } func (ctx *BuiltinEvalContext) Provisioner(n string) provisioners.Interface { diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go index ecb01d643..f97c3d08c 100644 --- a/terraform/eval_context_mock.go +++ b/terraform/eval_context_mock.go @@ -198,10 +198,10 @@ func (c *MockEvalContext) SetProviderInput(addr addrs.AbsProviderConfig, vals ma c.SetProviderInputValues = vals } -func (c *MockEvalContext) InitProvisioner(n string) (provisioners.Interface, error) { +func (c *MockEvalContext) InitProvisioner(n string) error { c.InitProvisionerCalled = true c.InitProvisionerName = n - return c.InitProvisionerProvisioner, c.InitProvisionerError + return c.InitProvisionerError } func (c *MockEvalContext) Provisioner(n string) provisioners.Interface { diff --git a/terraform/eval_provisioner.go b/terraform/eval_provisioner.go deleted file mode 100644 index bc6b5cc76..000000000 --- a/terraform/eval_provisioner.go +++ /dev/null @@ -1,55 +0,0 @@ -package terraform - -import ( - "fmt" - - "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/provisioners" -) - -// EvalInitProvisioner is an EvalNode implementation that initializes a provisioner -// and returns nothing. The provisioner can be retrieved again with the -// EvalGetProvisioner node. -type EvalInitProvisioner struct { - Name string -} - -func (n *EvalInitProvisioner) Eval(ctx EvalContext) (interface{}, error) { - return ctx.InitProvisioner(n.Name) -} - -// EvalCloseProvisioner is an EvalNode implementation that closes provisioner -// connections that aren't needed anymore. -type EvalCloseProvisioner struct { - Name string -} - -func (n *EvalCloseProvisioner) Eval(ctx EvalContext) (interface{}, error) { - ctx.CloseProvisioner(n.Name) - return nil, nil -} - -// EvalGetProvisioner is an EvalNode implementation that retrieves an already -// initialized provisioner instance for the given name. -type EvalGetProvisioner struct { - Name string - Output *provisioners.Interface - Schema **configschema.Block -} - -func (n *EvalGetProvisioner) Eval(ctx EvalContext) (interface{}, error) { - result := ctx.Provisioner(n.Name) - if result == nil { - return nil, fmt.Errorf("provisioner %s not initialized", n.Name) - } - - if n.Output != nil { - *n.Output = result - } - - if n.Schema != nil { - *n.Schema = ctx.ProvisionerSchema(n.Name) - } - - return result, nil -} diff --git a/terraform/eval_provisioner_test.go b/terraform/eval_provisioner_test.go deleted file mode 100644 index 5dfdf4b3a..000000000 --- a/terraform/eval_provisioner_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package terraform - -import ( - "testing" - - "github.com/hashicorp/terraform/provisioners" -) - -func TestEvalInitProvisioner_impl(t *testing.T) { - var _ EvalNode = new(EvalInitProvisioner) -} - -func TestEvalInitProvisioner(t *testing.T) { - n := &EvalInitProvisioner{Name: "foo"} - provisioner := &MockProvisioner{} - ctx := &MockEvalContext{InitProvisionerProvisioner: provisioner} - if _, err := n.Eval(ctx); err != nil { - t.Fatalf("err: %s", err) - } - - if !ctx.InitProvisionerCalled { - t.Fatal("should be called") - } - if ctx.InitProvisionerName != "foo" { - t.Fatalf("bad: %#v", ctx.InitProvisionerName) - } -} - -func TestEvalCloseProvisioner(t *testing.T) { - n := &EvalCloseProvisioner{Name: "foo"} - provisioner := &MockProvisioner{} - ctx := &MockEvalContext{CloseProvisionerProvisioner: provisioner} - if _, err := n.Eval(ctx); err != nil { - t.Fatalf("err: %s", err) - } - - if !ctx.CloseProvisionerCalled { - t.Fatal("should be called") - } - if ctx.CloseProvisionerName != "foo" { - t.Fatalf("bad: %#v", ctx.CloseProvisionerName) - } -} - -func TestEvalGetProvisioner_impl(t *testing.T) { - var _ EvalNode = new(EvalGetProvisioner) -} - -func TestEvalGetProvisioner(t *testing.T) { - var actual provisioners.Interface - n := &EvalGetProvisioner{Name: "foo", Output: &actual} - provisioner := &MockProvisioner{} - ctx := &MockEvalContext{ProvisionerProvisioner: provisioner} - if _, err := n.Eval(ctx); err != nil { - t.Fatalf("err: %s", err) - } - if actual != provisioner { - t.Fatalf("bad: %#v", actual) - } - - if !ctx.ProvisionerCalled { - t.Fatal("should be called") - } - if ctx.ProvisionerName != "foo" { - t.Fatalf("bad: %#v", ctx.ProvisionerName) - } -} diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index bdd908dff..10448ad7a 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -108,9 +108,9 @@ func (n *EvalValidateProvider) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.NonFatalErr() } -// EvalValidateProvisioner is an EvalNode implementation that validates -// the configuration of a provisioner belonging to a resource. The provisioner -// config is expected to contain the merged connection configurations. +// EvalValidateProvisioner validates 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 @@ -120,43 +120,38 @@ type EvalValidateProvisioner struct { ResourceHasForEach bool } -func (n *EvalValidateProvisioner) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalValidateProvisioner) Validate(ctx EvalContext) error { provisioner := *n.Provisioner config := *n.Config schema := *n.Schema var diags tfdiags.Diagnostics - { - // Validate the provisioner's own config first - - configVal, _, configDiags := n.evaluateBlock(ctx, config.Config, schema) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { - return nil, diags.Err() - } - - if configVal == cty.NilVal { - // Should never happen for a well-behaved EvaluateBlock implementation - return nil, fmt.Errorf("EvaluateBlock returned nil value") - } - - req := provisioners.ValidateProvisionerConfigRequest{ - Config: configVal, - } - - resp := provisioner.ValidateProvisionerConfig(req) - diags = diags.Append(resp.Diagnostics) + // Validate the provisioner's own config first + configVal, _, configDiags := n.evaluateBlock(ctx, config.Config, schema) + diags = diags.Append(configDiags) + if configDiags.HasErrors() { + return diags.Err() } - { - // 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) + if configVal == cty.NilVal { + // Should never happen for a well-behaved EvaluateBlock implementation + return fmt.Errorf("EvaluateBlock returned nil value") } - return nil, diags.NonFatalErr() + req := provisioners.ValidateProvisionerConfigRequest{ + Config: configVal, + } + + resp := provisioner.ValidateProvisionerConfig(req) + diags = diags.Append(resp.Diagnostics) + + // 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) + + return diags.NonFatalErr() } func (n *EvalValidateProvisioner) validateConnConfig(ctx EvalContext, config *configs.Connection, self addrs.Referenceable) tfdiags.Diagnostics { @@ -342,8 +337,7 @@ func ConnectionBlockSupersetSchema() *configschema.Block { return connectionBlockSupersetSchema } -// EvalValidateResource is an EvalNode implementation that validates -// the configuration of a resource. +// EvalValidateResource validates the configuration of a resource. type EvalValidateResource struct { Addr addrs.Resource Provider *providers.Interface @@ -363,9 +357,9 @@ type EvalValidateResource struct { ConfigVal *cty.Value } -func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalValidateResource) Validate(ctx EvalContext) error { if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return nil, fmt.Errorf("EvalValidateResource has nil schema for %s", n.Addr) + return fmt.Errorf("EvalValidateResource has nil schema for %s", n.Addr) } var diags tfdiags.Diagnostics @@ -450,13 +444,13 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { Detail: fmt.Sprintf("The provider %s does not support resource type %q.", cfg.ProviderConfigAddr(), cfg.Type), Subject: &cfg.TypeRange, }) - return nil, diags.Err() + return diags.Err() } configVal, _, valDiags := ctx.EvaluateBlock(cfg.Config, schema, nil, keyData) diags = diags.Append(valDiags) if valDiags.HasErrors() { - return nil, diags.Err() + return diags.Err() } if cfg.Managed != nil { // can be nil only in tests with poorly-configured mocks @@ -487,13 +481,13 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { Detail: fmt.Sprintf("The provider %s does not support data source %q.", cfg.ProviderConfigAddr(), cfg.Type), Subject: &cfg.TypeRange, }) - return nil, diags.Err() + return diags.Err() } configVal, _, valDiags := ctx.EvaluateBlock(cfg.Config, schema, nil, keyData) diags = diags.Append(valDiags) if valDiags.HasErrors() { - return nil, diags.Err() + return diags.Err() } req := providers.ValidateDataSourceConfigRequest{ @@ -508,13 +502,13 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { if n.IgnoreWarnings { // If we _only_ have warnings then we'll return nil. if diags.HasErrors() { - return nil, diags.NonFatalErr() + return diags.NonFatalErr() } - return nil, nil + return nil } else { // We'll return an error if there are any diagnostics at all, even if // some of them are warnings. - return nil, diags.NonFatalErr() + return diags.NonFatalErr() } } diff --git a/terraform/eval_validate_test.go b/terraform/eval_validate_test.go index 6adfeab92..ce23b3596 100644 --- a/terraform/eval_validate_test.go +++ b/terraform/eval_validate_test.go @@ -52,7 +52,7 @@ func TestEvalValidateResource_managedResource(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("err: %s", err) } @@ -98,7 +98,7 @@ func TestEvalValidateResource_managedResourceCount(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("err: %s", err) } @@ -144,7 +144,7 @@ func TestEvalValidateResource_dataSource(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("err: %s", err) } @@ -181,7 +181,7 @@ func TestEvalValidateResource_validReturnsNilError(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("Expected nil error, got: %s", err) } @@ -219,7 +219,7 @@ func TestEvalValidateResource_warningsAndErrorsPassedThrough(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err == nil { t.Fatal("unexpected success; want error") } @@ -271,7 +271,7 @@ func TestEvalValidateResource_ignoreWarnings(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("Expected no error, got: %s", err) } @@ -323,7 +323,7 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("error for supposedly-valid config: %s", err) } @@ -344,7 +344,7 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) { }, }) - _, err = node.Eval(ctx) + err = node.Validate(ctx) if err == nil { t.Fatal("no error for invalid depends_on") } @@ -360,7 +360,7 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) { }, }) - _, err = node.Eval(ctx) + err = node.Validate(ctx) if err == nil { t.Fatal("no error for invalid depends_on") } @@ -397,14 +397,10 @@ func TestEvalValidateProvisioner_valid(t *testing.T) { }, } - result, err := node.Eval(ctx) + err := node.Validate(ctx) if err != nil { t.Fatalf("node.Eval failed: %s", err) } - if result != nil { - t.Errorf("node.Eval returned non-nil result") - } - if !mp.ValidateProvisionerConfigCalled { t.Fatalf("p.ValidateProvisionerConfig not called") } @@ -453,7 +449,7 @@ func TestEvalValidateProvisioner_warning(t *testing.T) { } } - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err == nil { t.Fatalf("node.Eval succeeded; want error") } @@ -504,7 +500,7 @@ func TestEvalValidateProvisioner_connectionInvalid(t *testing.T) { }, } - _, err := node.Eval(ctx) + err := node.Validate(ctx) if err == nil { t.Fatalf("node.Eval succeeded; want error") } diff --git a/terraform/node_provisioner.go b/terraform/node_provisioner.go index 1160498ae..c4df9c627 100644 --- a/terraform/node_provisioner.go +++ b/terraform/node_provisioner.go @@ -16,7 +16,7 @@ type NodeProvisioner struct { var ( _ GraphNodeModuleInstance = (*NodeProvisioner)(nil) _ GraphNodeProvisioner = (*NodeProvisioner)(nil) - _ GraphNodeEvalable = (*NodeProvisioner)(nil) + _ GraphNodeExecutable = (*NodeProvisioner)(nil) ) func (n *NodeProvisioner) Name() string { @@ -38,7 +38,7 @@ func (n *NodeProvisioner) ProvisionerName() string { return n.NameValue } -// GraphNodeEvalable impl. -func (n *NodeProvisioner) EvalTree() EvalNode { - return &EvalInitProvisioner{Name: n.NameValue} +// GraphNodeExecutable impl. +func (n *NodeProvisioner) Execute(ctx EvalContext, op walkOperation) error { + return ctx.InitProvisioner(n.NameValue) } diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go index 0228e3d1c..436d30a45 100644 --- a/terraform/node_resource_validate.go +++ b/terraform/node_resource_validate.go @@ -1,11 +1,10 @@ package terraform import ( + "fmt" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" - "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/providers" - "github.com/hashicorp/terraform/provisioners" "github.com/zclconf/go-cty/cty" ) @@ -17,7 +16,7 @@ type NodeValidatableResource struct { var ( _ GraphNodeModuleInstance = (*NodeValidatableResource)(nil) - _ GraphNodeEvalable = (*NodeValidatableResource)(nil) + _ GraphNodeExecutable = (*NodeValidatableResource)(nil) _ GraphNodeReferenceable = (*NodeValidatableResource)(nil) _ GraphNodeReferencer = (*NodeValidatableResource)(nil) _ GraphNodeConfigResource = (*NodeValidatableResource)(nil) @@ -32,33 +31,30 @@ func (n *NodeValidatableResource) Path() addrs.ModuleInstance { } // GraphNodeEvalable -func (n *NodeValidatableResource) EvalTree() EvalNode { +func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) error { addr := n.ResourceAddr() config := n.Config // Declare the variables will be used are used to pass values along // the evaluation sequence below. These are written to via pointers // passed to the EvalNodes. - var provider providers.Interface - var providerSchema *ProviderSchema var configVal cty.Value + provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) + if err != nil { + return err + } - seq := &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, - &EvalValidateResource{ - Addr: addr.Resource, - Provider: &provider, - ProviderMetas: n.ProviderMetas, - ProviderSchema: &providerSchema, - Config: config, - ConfigVal: &configVal, - }, - }, + evalValidateResource := &EvalValidateResource{ + Addr: addr.Resource, + Provider: &provider, + ProviderMetas: n.ProviderMetas, + ProviderSchema: &providerSchema, + Config: config, + ConfigVal: &configVal, + } + err = evalValidateResource.Validate(ctx) + if err != nil { + return err } if managed := n.Config.Managed; managed != nil { @@ -67,33 +63,35 @@ func (n *NodeValidatableResource) EvalTree() EvalNode { // Validate all the provisioners 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{ - Name: p.Type, - Output: &provisioner, - Schema: &provisionerSchema, - }, - &EvalValidateProvisioner{ - ResourceAddr: addr.Resource, - Provisioner: &provisioner, - Schema: &provisionerSchema, - Config: p, - ResourceHasCount: hasCount, - ResourceHasForEach: hasForEach, - }, - ) + provisioner := ctx.Provisioner(p.Type) + if provisioner == nil { + return fmt.Errorf("provisioner %s not initialized", p.Type) + } + provisionerSchema := ctx.ProvisionerSchema(p.Type) + if provisionerSchema == nil { + return fmt.Errorf("provisioner %s not initialized", p.Type) + } + + // Validate Provisioner Config + validateProvisioner := &EvalValidateProvisioner{ + ResourceAddr: addr.Resource, + Provisioner: &provisioner, + Schema: &provisionerSchema, + Config: p, + ResourceHasCount: hasCount, + ResourceHasForEach: hasForEach, + } + err := validateProvisioner.Validate(ctx) + if err != nil { + return err + } } } - - return seq + return nil } diff --git a/terraform/transform_provisioner.go b/terraform/transform_provisioner.go index b31026655..638410d4a 100644 --- a/terraform/transform_provisioner.go +++ b/terraform/transform_provisioner.go @@ -169,9 +169,9 @@ func (n *graphNodeCloseProvisioner) Name() string { return fmt.Sprintf("provisioner.%s (close)", n.ProvisionerNameValue) } -// GraphNodeEvalable impl. -func (n *graphNodeCloseProvisioner) EvalTree() EvalNode { - return &EvalCloseProvisioner{Name: n.ProvisionerNameValue} +// GraphNodeExecutable impl. +func (n *graphNodeCloseProvisioner) Execute(ctx EvalContext, op walkOperation) error { + return ctx.CloseProvisioner(n.ProvisionerNameValue) } func (n *graphNodeCloseProvisioner) CloseProvisionerName() string {