From a59c2fe1b9c3b5c1742df4699708e1027cdba09e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 31 Aug 2021 15:44:07 -0700 Subject: [PATCH] core: EvalContextBuiltin no longer has a "Schemas" By tolerating ProviderSchema and ProvisionerSchema potentially returning errors, we can slightly simplify EvalContextBuiltin by having it retrieve individual schemas when needed directly from the "Plugins" object. EvalContextBuiltin already needs to be holding a contextPlugins instance for other reasons anyway, so this allows us to get the same result with fewer moving parts. --- internal/terraform/eval_context.go | 4 +-- internal/terraform/eval_context_builtin.go | 28 +++++++++---------- internal/terraform/eval_context_mock.go | 10 ++++--- internal/terraform/eval_provider.go | 5 +++- internal/terraform/graph_walk_context.go | 1 - .../node_resource_abstract_instance.go | 9 +++++- internal/terraform/node_resource_validate.go | 7 +++-- 7 files changed, 38 insertions(+), 26 deletions(-) diff --git a/internal/terraform/eval_context.go b/internal/terraform/eval_context.go index 2cee1b0711..61b4f2448f 100644 --- a/internal/terraform/eval_context.go +++ b/internal/terraform/eval_context.go @@ -53,7 +53,7 @@ type EvalContext interface { // // This method expects an _absolute_ provider configuration address, since // resources in one module are able to use providers from other modules. - ProviderSchema(addrs.AbsProviderConfig) *ProviderSchema + ProviderSchema(addrs.AbsProviderConfig) (*ProviderSchema, error) // CloseProvider closes provider connections that aren't needed anymore. // @@ -84,7 +84,7 @@ type EvalContext interface { // ProvisionerSchema retrieves the main configuration schema for a // particular provisioner, which must have already been initialized with // InitProvisioner. - ProvisionerSchema(string) *configschema.Block + ProvisionerSchema(string) (*configschema.Block, error) // CloseProvisioner closes all provisioner plugins. CloseProvisioners() error diff --git a/internal/terraform/eval_context_builtin.go b/internal/terraform/eval_context_builtin.go index 6bcf916630..ea83e82799 100644 --- a/internal/terraform/eval_context_builtin.go +++ b/internal/terraform/eval_context_builtin.go @@ -44,15 +44,6 @@ type BuiltinEvalContext struct { // eval context. Evaluator *Evaluator - // Schemas is a repository of all of the schemas we should need to - // decode configuration blocks and expressions. This must be constructed by - // the caller to include schemas for all of the providers, resource types, - // data sources and provisioners used by the given configuration and - // state. - // - // This must not be mutated during evaluation. - Schemas *Schemas - // VariableValues contains the variable values across all modules. This // structure is shared across the entire containing context, and so it // may be accessed only when holding VariableValuesLock. @@ -62,7 +53,10 @@ type BuiltinEvalContext struct { VariableValues map[string]map[string]cty.Value VariableValuesLock *sync.Mutex - Plugins *contextPlugins + // Plugins is a library of plugin components (providers and provisioners) + // available for use during a graph walk. + Plugins *contextPlugins + Hooks []Hook InputValue UIInput ProviderCache map[string]providers.Interface @@ -152,8 +146,8 @@ func (ctx *BuiltinEvalContext) Provider(addr addrs.AbsProviderConfig) providers. return ctx.ProviderCache[addr.String()] } -func (ctx *BuiltinEvalContext) ProviderSchema(addr addrs.AbsProviderConfig) *ProviderSchema { - return ctx.Schemas.ProviderSchema(addr.Provider) +func (ctx *BuiltinEvalContext) ProviderSchema(addr addrs.AbsProviderConfig) (*ProviderSchema, error) { + return ctx.Plugins.ProviderSchema(addr.Provider) } func (ctx *BuiltinEvalContext) CloseProvider(addr addrs.AbsProviderConfig) error { @@ -184,7 +178,11 @@ func (ctx *BuiltinEvalContext) ConfigureProvider(addr addrs.AbsProviderConfig, c return diags } - providerSchema := ctx.ProviderSchema(addr) + providerSchema, err := ctx.ProviderSchema(addr) + if err != nil { + diags = diags.Append(fmt.Errorf("failed to read schema for %s: %s", addr, err)) + return diags + } if providerSchema == nil { diags = diags.Append(fmt.Errorf("schema for %s is not available", addr)) return diags @@ -249,8 +247,8 @@ func (ctx *BuiltinEvalContext) Provisioner(n string) (provisioners.Interface, er return p, nil } -func (ctx *BuiltinEvalContext) ProvisionerSchema(n string) *configschema.Block { - return ctx.Schemas.ProvisionerConfig(n) +func (ctx *BuiltinEvalContext) ProvisionerSchema(n string) (*configschema.Block, error) { + return ctx.Plugins.ProvisionerSchema(n) } func (ctx *BuiltinEvalContext) CloseProvisioners() error { diff --git a/internal/terraform/eval_context_mock.go b/internal/terraform/eval_context_mock.go index 0e4fd32d69..52a06c3ee7 100644 --- a/internal/terraform/eval_context_mock.go +++ b/internal/terraform/eval_context_mock.go @@ -43,6 +43,7 @@ type MockEvalContext struct { ProviderSchemaCalled bool ProviderSchemaAddr addrs.AbsProviderConfig ProviderSchemaSchema *ProviderSchema + ProviderSchemaError error CloseProviderCalled bool CloseProviderAddr addrs.AbsProviderConfig @@ -71,6 +72,7 @@ type MockEvalContext struct { ProvisionerSchemaCalled bool ProvisionerSchemaName string ProvisionerSchemaSchema *configschema.Block + ProvisionerSchemaError error CloseProvisionersCalled bool @@ -173,10 +175,10 @@ func (c *MockEvalContext) Provider(addr addrs.AbsProviderConfig) providers.Inter return c.ProviderProvider } -func (c *MockEvalContext) ProviderSchema(addr addrs.AbsProviderConfig) *ProviderSchema { +func (c *MockEvalContext) ProviderSchema(addr addrs.AbsProviderConfig) (*ProviderSchema, error) { c.ProviderSchemaCalled = true c.ProviderSchemaAddr = addr - return c.ProviderSchemaSchema + return c.ProviderSchemaSchema, c.ProviderSchemaError } func (c *MockEvalContext) CloseProvider(addr addrs.AbsProviderConfig) error { @@ -214,10 +216,10 @@ func (c *MockEvalContext) Provisioner(n string) (provisioners.Interface, error) return c.ProvisionerProvisioner, nil } -func (c *MockEvalContext) ProvisionerSchema(n string) *configschema.Block { +func (c *MockEvalContext) ProvisionerSchema(n string) (*configschema.Block, error) { c.ProvisionerSchemaCalled = true c.ProvisionerSchemaName = n - return c.ProvisionerSchemaSchema + return c.ProvisionerSchemaSchema, c.ProvisionerSchemaError } func (c *MockEvalContext) CloseProvisioners() error { diff --git a/internal/terraform/eval_provider.go b/internal/terraform/eval_provider.go index 31f7b0453a..a97f347e40 100644 --- a/internal/terraform/eval_provider.go +++ b/internal/terraform/eval_provider.go @@ -51,6 +51,9 @@ func getProvider(ctx EvalContext, addr addrs.AbsProviderConfig) (providers.Inter } // Not all callers require a schema, so we will leave checking for a nil // schema to the callers. - schema := ctx.ProviderSchema(addr) + schema, err := ctx.ProviderSchema(addr) + if err != nil { + return nil, &ProviderSchema{}, fmt.Errorf("failed to read schema for provider %s: %w", addr, err) + } return provider, schema, nil } diff --git a/internal/terraform/graph_walk_context.go b/internal/terraform/graph_walk_context.go index 3561ab983b..6fd790e92f 100644 --- a/internal/terraform/graph_walk_context.go +++ b/internal/terraform/graph_walk_context.go @@ -92,7 +92,6 @@ func (w *ContextGraphWalker) EvalContext() EvalContext { InputValue: w.Context.uiInput, InstanceExpanderValue: w.InstanceExpander, Plugins: w.Context.plugins, - Schemas: w.Schemas, MoveResultsValue: w.MoveResults, ProviderCache: w.providerCache, ProviderInputConfig: w.Context.providerInputConfig, diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 9f07068e39..e73939a8e3 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -1809,7 +1809,14 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state return diags.Append(err) } - schema := ctx.ProvisionerSchema(prov.Type) + schema, err := ctx.ProvisionerSchema(prov.Type) + if err != nil { + // This error probably won't be a great diagnostic, but in practice + // we typically catch this problem long before we get here, so + // it should be rare to return via this codepath. + diags = diags.Append(err) + return diags + } config, configDiags := evalScope(ctx, prov.Config, self, schema) diags = diags.Append(configDiags) diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index ebf597c066..cd4ec91fe8 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -77,9 +77,12 @@ func (n *NodeValidatableResource) validateProvisioner(ctx EvalContext, p *config if provisioner == nil { return diags.Append(fmt.Errorf("provisioner %s not initialized", p.Type)) } - provisionerSchema := ctx.ProvisionerSchema(p.Type) + provisionerSchema, err := ctx.ProvisionerSchema(p.Type) + if err != nil { + return diags.Append(fmt.Errorf("failed to read schema for provisioner %s: %s", p.Type, err)) + } if provisionerSchema == nil { - return diags.Append(fmt.Errorf("provisioner %s not initialized", p.Type)) + return diags.Append(fmt.Errorf("provisioner %s has no schema", p.Type)) } // Validate the provisioner's own config first