From d09510a8fbcb4121d5b67d160d3d1ee4811eac9f Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 1 Oct 2021 14:51:06 -0700 Subject: [PATCH] command: Early error message for missing cache entries of locked providers In the original incarnation of Meta.providerFactories we were returning into a Meta.contextOpts whose signature didn't allow it to return an error directly, and so we had compromised by making the provider factory functions themselves return errors once called. Subsequent work made Meta.contextOpts need to return an error anyway, but at the time we neglected to update our handling of the providerFactories result, having it still defer the error handling until we finally instantiate a provider. Although that did ultimately get the expected result anyway, the error ended up being reported from deep in the guts of a Terraform Core graph walk, in whichever concurrently-visited graph node happened to try to instantiate the plugin first. This meant that the exact phrasing of the error message would vary between runs and the reporting codepath didn't have enough context to given an actionable suggestion on how to proceed. In this commit we make Meta.contextOpts pass through directly any error that Meta.providerFactories produces, and then make Meta.providerFactories produce a special error type so that Meta.Backend can ultimately return a user-friendly diagnostic message containing a specific suggestion to run "terraform init", along with a short explanation of what a provider plugin is. The reliance here on an implied contract between two functions that are not directly connected in the callstack is non-ideal, and so hopefully we'll revisit this further in future work on the overall architecture of the CLI layer. To try to make this robust in the meantime though, I wrote it to use the errors.As function to potentially unwrap a wrapped version of our special error type, in case one of the intervening layers is changed at some point to wrap the downstream error before returning it. --- internal/command/meta.go | 15 +------------- internal/command/meta_backend.go | 29 +++++++++++++++++++++++++- internal/command/meta_providers.go | 33 +++++++++++++++++++++++++++--- 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/internal/command/meta.go b/internal/command/meta.go index 0a4029ef0..cd9387173 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -461,20 +461,7 @@ func (m *Meta) contextOpts() (*terraform.ContextOpts, error) { } else { providerFactories, err := m.providerFactories() if err != nil { - // providerFactories can fail if the plugin selections file is - // invalid in some way, but we don't have any way to report that - // from here so we'll just behave as if no providers are available - // in that case. However, we will produce a warning in case this - // shows up unexpectedly and prompts a bug report. - // This situation shouldn't arise commonly in practice because - // the selections file is generated programmatically. - log.Printf("[WARN] Failed to determine selected providers: %s", err) - - // variable providerFactories may now be incomplete, which could - // lead to errors reported downstream from here. providerFactories - // tries to populate as many providers as possible even in an - // error case, so that operations not using problematic providers - // can still succeed. + return nil, err } opts.Providers = providerFactories opts.Provisioners = m.provisionerFactories() diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index 5d2aa5ed3..382870817 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -4,8 +4,10 @@ package command // exported and private. import ( + "bytes" "context" "encoding/json" + "errors" "fmt" "log" "path/filepath" @@ -105,7 +107,32 @@ func (m *Meta) Backend(opts *BackendOpts) (backend.Enhanced, tfdiags.Diagnostics // Set up the CLI opts we pass into backends that support it. cliOpts, err := m.backendCLIOpts() if err != nil { - diags = diags.Append(err) + if errs := providerPluginErrors(nil); errors.As(err, &errs) { + // This is a special type returned by m.providerFactories, which + // indicates one or more inconsistencies between the dependency + // lock file and the provider plugins actually available in the + // local cache directory. + var buf bytes.Buffer + for addr, err := range errs { + fmt.Fprintf(&buf, "\n - %s: %s", addr, err) + } + suggestion := "To download the plugins required for this configuration, run:\n terraform init" + if m.RunningInAutomation { + // Don't mention "terraform init" specifically if we're running in an automation wrapper + suggestion = "You must install the required plugins before running Terraform operations." + } + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Required plugins are not installed", + fmt.Sprintf( + "The installed provider plugins are not consistent with the packages selected in the dependency lock file:%s\n\nTerraform uses external plugins to integrate with a variety of different infrastructure services. %s", + buf.String(), suggestion, + ), + )) + } else { + // All other errors just get generic handling. + diags = diags.Append(err) + } return nil, diags } cliOpts.Validation = true diff --git a/internal/command/meta_providers.go b/internal/command/meta_providers.go index ffe52ffde..c406e4745 100644 --- a/internal/command/meta_providers.go +++ b/internal/command/meta_providers.go @@ -1,6 +1,7 @@ package command import ( + "bytes" "errors" "fmt" "log" @@ -8,7 +9,6 @@ import ( "os/exec" "strings" - "github.com/hashicorp/go-multierror" plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/terraform/internal/addrs" @@ -236,7 +236,7 @@ func (m *Meta) providerFactories() (map[addrs.Provider]providers.Factory, error) // where appropriate and so that callers can potentially make use of the // partial result we return if e.g. they want to enumerate which providers // are available, or call into one of the providers that didn't fail. - var err error + errs := make(map[addrs.Provider]error) // For the providers from the lock file, we expect them to be already // available in the provider cache because "terraform init" should already @@ -274,7 +274,7 @@ func (m *Meta) providerFactories() (map[addrs.Provider]providers.Factory, error) } for provider, lock := range providerLocks { reportError := func(thisErr error) { - err = multierror.Append(err, thisErr) + errs[provider] = thisErr // We'll populate a provider factory that just echoes our error // again if called, which allows us to still report a helpful // error even if it gets detected downstream somewhere from the @@ -324,6 +324,11 @@ func (m *Meta) providerFactories() (map[addrs.Provider]providers.Factory, error) for provider, reattach := range unmanagedProviders { factories[provider] = unmanagedProviderFactory(provider, reattach) } + + var err error + if len(errs) > 0 { + err = providerPluginErrors(errs) + } return factories, err } @@ -475,3 +480,25 @@ func providerFactoryError(err error) providers.Factory { return nil, err } } + +// providerPluginErrors is an error implementation we can return from +// Meta.providerFactories to capture potentially multiple errors about the +// locally-cached plugins (or lack thereof) for particular external providers. +// +// Some functions closer to the UI layer can sniff for this error type in order +// to return a more helpful error message. +type providerPluginErrors map[addrs.Provider]error + +func (errs providerPluginErrors) Error() string { + if len(errs) == 1 { + for addr, err := range errs { + return fmt.Sprintf("%s: %s", addr, err) + } + } + var buf bytes.Buffer + fmt.Fprintf(&buf, "missing or corrupted provider plugins:") + for addr, err := range errs { + fmt.Fprintf(&buf, "\n - %s: %s", addr, err) + } + return buf.String() +}