From 2bf1de1f5dfa09bbe3daf9dd6e73016e7eaf791b Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 31 Aug 2021 11:27:07 -0700 Subject: [PATCH] core: Context.Schemas in terms of contextPlugins methods The responsibility for actually instantiating a single plugin and reading out its schema now belongs to the contextPlugins type, which memoizes the results by each plugin's unique identifier so that we can avoid retrieving the same schemas multiple times when working with the same context. This doesn't change the API of Context.Schemas but it does restore the spirit of an earlier version of terraform.Context which did all of the schema loading proactively inside terraform.NewContext. In an earlier commit we reduced the scope of terraform.NewContext, making schema loading a separate step, but in the process of doing that removed the effective memoization of the schema results that terraform.NewContext was providing. The memoization here will play out in a different way than before, because we'll be treating each plugin call as separate rather than proactively loading them all up front, but this is effectively the same because all of our operation methods on Context call Context.Schemas early in their work and thus end up forcing all of the necessary schemas to load up front nonetheless. --- internal/terraform/context_plugins.go | 105 ++++++++++++++++++++++++- internal/terraform/schemas.go | 109 +++++--------------------- 2 files changed, 122 insertions(+), 92 deletions(-) diff --git a/internal/terraform/context_plugins.go b/internal/terraform/context_plugins.go index 2bd1964d3..5711badbd 100644 --- a/internal/terraform/context_plugins.go +++ b/internal/terraform/context_plugins.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "sync" "github.com/hashicorp/terraform/internal/addrs" @@ -25,7 +26,7 @@ type contextPlugins struct { // it makes sense to preload all of them. providerSchemas map[addrs.Provider]*ProviderSchema provisionerSchemas map[string]*configschema.Block - schemasLock *sync.Mutex + schemasLock sync.Mutex } func newContextPlugins(providerFactories map[addrs.Provider]providers.Factory, provisionerFactories map[string]provisioners.Factory) *contextPlugins { @@ -60,3 +61,105 @@ func (cp *contextPlugins) NewProvisionerInstance(typ string) (provisioners.Inter return f() } + +// ProviderSchema uses a temporary instance of the provider with the given +// address to obtain the full schema for all aspects of that provider. +// +// ProviderSchema memoizes results by unique provider address, so it's fine +// to repeatedly call this method with the same address if various different +// parts of Terraform all need the same schema information. +func (cp *contextPlugins) ProviderSchema(addr addrs.Provider) (*ProviderSchema, error) { + cp.schemasLock.Lock() + defer cp.schemasLock.Unlock() + + if schema, ok := cp.providerSchemas[addr]; ok { + return schema, nil + } + + log.Printf("[TRACE] terraform.contextPlugins: Initializing provider %q to read its schema", addr) + + provider, err := cp.NewProviderInstance(addr) + if err != nil { + return nil, fmt.Errorf("failed to instantiate provider %q to obtain schema: %s", addr, err) + } + defer provider.Close() + + resp := provider.GetProviderSchema() + if resp.Diagnostics.HasErrors() { + return nil, fmt.Errorf("failed to retrieve schema from provider %q: %s", addr, resp.Diagnostics.Err()) + } + + s := &ProviderSchema{ + Provider: resp.Provider.Block, + ResourceTypes: make(map[string]*configschema.Block), + DataSources: make(map[string]*configschema.Block), + + ResourceTypeSchemaVersions: make(map[string]uint64), + } + + if resp.Provider.Version < 0 { + // We're not using the version numbers here yet, but we'll check + // for validity anyway in case we start using them in future. + return nil, fmt.Errorf("provider %s has invalid negative schema version for its configuration blocks,which is a bug in the provider ", addr) + } + + for t, r := range resp.ResourceTypes { + if err := r.Block.InternalValidate(); err != nil { + return nil, fmt.Errorf("provider %s has invalid schema for managed resource type %q, which is a bug in the provider: %q", addr, t, err) + } + s.ResourceTypes[t] = r.Block + s.ResourceTypeSchemaVersions[t] = uint64(r.Version) + if r.Version < 0 { + return nil, fmt.Errorf("provider %s has invalid negative schema version for managed resource type %q, which is a bug in the provider", addr, t) + } + } + + for t, d := range resp.DataSources { + if err := d.Block.InternalValidate(); err != nil { + return nil, fmt.Errorf("provider %s has invalid schema for data resource type %q, which is a bug in the provider: %q", addr, t, err) + } + s.DataSources[t] = d.Block + if d.Version < 0 { + // We're not using the version numbers here yet, but we'll check + // for validity anyway in case we start using them in future. + return nil, fmt.Errorf("provider %s has invalid negative schema version for data resource type %q, which is a bug in the provider", addr, t) + } + } + + if resp.ProviderMeta.Block != nil { + s.ProviderMeta = resp.ProviderMeta.Block + } + + cp.providerSchemas[addr] = s + return s, nil +} + +// ProvisionerSchema uses a temporary instance of the provisioner with the +// given type name to obtain the schema for that provisioner's configuration. +// +// ProvisionerSchema memoizes results by provisioner type name, so it's fine +// to repeatedly call this method with the same name if various different +// parts of Terraform all need the same schema information. +func (cp *contextPlugins) ProvisionerSchema(typ string) (*configschema.Block, error) { + cp.schemasLock.Lock() + defer cp.schemasLock.Unlock() + + if schema, ok := cp.provisionerSchemas[typ]; ok { + return schema, nil + } + + log.Printf("[TRACE] terraform.contextPlugins: Initializing provisioner %q to read its schema", typ) + provisioner, err := cp.NewProvisionerInstance(typ) + if err != nil { + return nil, fmt.Errorf("failed to instantiate provisioner %q to obtain schema: %s", typ, err) + } + defer provisioner.Close() + + resp := provisioner.GetSchema() + if resp.Diagnostics.HasErrors() { + return nil, fmt.Errorf("failed to retrieve schema from provisioner %q: %s", typ, resp.Diagnostics.Err()) + } + + cp.provisionerSchemas[typ] = resp.Provisioner + return resp.Provisioner, nil +} diff --git a/internal/terraform/schemas.go b/internal/terraform/schemas.go index 7b7177dbd..d09cc2cb2 100644 --- a/internal/terraform/schemas.go +++ b/internal/terraform/schemas.go @@ -100,79 +100,23 @@ func loadProviderSchemas(schemas map[addrs.Provider]*ProviderSchema, config *con } log.Printf("[TRACE] LoadSchemas: retrieving schema for provider type %q", name) - provider, err := plugins.NewProviderInstance(fqn) + schema, err := plugins.ProviderSchema(fqn) if err != nil { // We'll put a stub in the map so we won't re-attempt this on - // future calls. + // future calls, which would then repeat the same error message + // multiple times. schemas[fqn] = &ProviderSchema{} diags = diags.Append( - fmt.Errorf("failed to instantiate provider %q to obtain schema: %s", name, err), - ) - return - } - defer func() { - provider.Close() - }() - - resp := provider.GetProviderSchema() - if resp.Diagnostics.HasErrors() { - // We'll put a stub in the map so we won't re-attempt this on - // future calls. - schemas[fqn] = &ProviderSchema{} - diags = diags.Append( - fmt.Errorf("failed to retrieve schema from provider %q: %s", name, resp.Diagnostics.Err()), + tfdiags.Sourceless( + tfdiags.Error, + "Failed to obtain provider schema", + fmt.Sprintf("Could not load the schema for provider %s: %s.", fqn, err), + ), ) return } - s := &ProviderSchema{ - Provider: resp.Provider.Block, - ResourceTypes: make(map[string]*configschema.Block), - DataSources: make(map[string]*configschema.Block), - - ResourceTypeSchemaVersions: make(map[string]uint64), - } - - if resp.Provider.Version < 0 { - // We're not using the version numbers here yet, but we'll check - // for validity anyway in case we start using them in future. - diags = diags.Append( - fmt.Errorf("invalid negative schema version provider configuration for provider %q", name), - ) - } - - for t, r := range resp.ResourceTypes { - if err := r.Block.InternalValidate(); err != nil { - diags = diags.Append(fmt.Errorf(errProviderSchemaInvalid, name, "resource", t, err)) - } - s.ResourceTypes[t] = r.Block - s.ResourceTypeSchemaVersions[t] = uint64(r.Version) - if r.Version < 0 { - diags = diags.Append( - fmt.Errorf("invalid negative schema version for resource type %s in provider %q", t, name), - ) - } - } - - for t, d := range resp.DataSources { - if err := d.Block.InternalValidate(); err != nil { - diags = diags.Append(fmt.Errorf(errProviderSchemaInvalid, name, "data source", t, err)) - } - s.DataSources[t] = d.Block - if d.Version < 0 { - // We're not using the version numbers here yet, but we'll check - // for validity anyway in case we start using them in future. - diags = diags.Append( - fmt.Errorf("invalid negative schema version for data source %s in provider %q", t, name), - ) - } - } - - schemas[fqn] = s - - if resp.ProviderMeta.Block != nil { - s.ProviderMeta = resp.ProviderMeta.Block - } + schemas[fqn] = schema } if config != nil { @@ -200,32 +144,23 @@ func loadProvisionerSchemas(schemas map[string]*configschema.Block, config *conf } log.Printf("[TRACE] LoadSchemas: retrieving schema for provisioner %q", name) - provisioner, err := plugins.NewProvisionerInstance(name) + schema, err := plugins.ProvisionerSchema(name) if err != nil { // We'll put a stub in the map so we won't re-attempt this on - // future calls. + // future calls, which would then repeat the same error message + // multiple times. schemas[name] = &configschema.Block{} diags = diags.Append( - fmt.Errorf("failed to instantiate provisioner %q to obtain schema: %s", name, err), - ) - return - } - defer func() { - provisioner.Close() - }() - - resp := provisioner.GetSchema() - if resp.Diagnostics.HasErrors() { - // We'll put a stub in the map so we won't re-attempt this on - // future calls. - schemas[name] = &configschema.Block{} - diags = diags.Append( - fmt.Errorf("failed to retrieve schema from provisioner %q: %s", name, resp.Diagnostics.Err()), + tfdiags.Sourceless( + tfdiags.Error, + "Failed to obtain provisioner schema", + fmt.Sprintf("Could not load the schema for provisioner %q: %s.", name, err), + ), ) return } - schemas[name] = resp.Provisioner + schemas[name] = schema } if config != nil { @@ -280,11 +215,3 @@ func (ps *ProviderSchema) SchemaForResourceType(mode addrs.ResourceMode, typeNam func (ps *ProviderSchema) SchemaForResourceAddr(addr addrs.Resource) (schema *configschema.Block, version uint64) { return ps.SchemaForResourceType(addr.Mode, addr.Type) } - -const errProviderSchemaInvalid = ` -Internal validation of the provider failed! This is always a bug with the -provider itself, and not a user issue. Please report this bug to the -maintainers of the %q provider: - -%s %s: %s -`