From 24e13d8ec106586c673ef227fad45d55416826b3 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Wed, 13 Mar 2019 12:52:15 -0400 Subject: [PATCH] plugin/discovery: Return tfdiags from Get Allows us to surface warnings to the user using the tfdiags interfaces. --- command/init.go | 3 ++- command/init_test.go | 9 +++++---- command/plugins_test.go | 16 +++++++++------- plugin/discovery/get.go | 32 ++++++++++++++++--------------- plugin/discovery/get_test.go | 10 +++++----- tools/terraform-bundle/package.go | 2 +- 6 files changed, 39 insertions(+), 33 deletions(-) diff --git a/command/init.go b/command/init.go index 1801e1a51..70cfabaee 100644 --- a/command/init.go +++ b/command/init.go @@ -494,7 +494,8 @@ func (c *InitCommand) getProviders(earlyConfig *earlyconfig.Config, state *state } for provider, reqd := range missing { - _, err := c.providerInstaller.Get(provider, reqd.Versions) + _, providerDiags, err := c.providerInstaller.Get(provider, reqd.Versions) + diags = diags.Append(providerDiags) if err != nil { constraint := reqd.Versions.String() diff --git a/command/init_test.go b/command/init_test.go index c0a198452..761a0774d 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -24,6 +24,7 @@ import ( "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states/statemgr" "github.com/hashicorp/terraform/terraform" + "github.com/hashicorp/terraform/tfdiags" ) func TestInit_empty(t *testing.T) { @@ -964,8 +965,8 @@ func TestInit_getProviderHaveLegacyVersion(t *testing.T) { testingOverrides: metaOverridesForProvider(testProvider()), Ui: ui, }, - providerInstaller: callbackPluginInstaller(func(provider string, req discovery.Constraints) (discovery.PluginMeta, error) { - return discovery.PluginMeta{}, fmt.Errorf("EXPECTED PROVIDER ERROR %s", provider) + providerInstaller: callbackPluginInstaller(func(provider string, req discovery.Constraints) (discovery.PluginMeta, tfdiags.Diagnostics, error) { + return discovery.PluginMeta{}, tfdiags.Diagnostics{}, fmt.Errorf("EXPECTED PROVIDER ERROR %s", provider) }), } @@ -1174,9 +1175,9 @@ func TestInit_pluginDirProvidersDoesNotGet(t *testing.T) { c := &InitCommand{ Meta: m, - providerInstaller: callbackPluginInstaller(func(provider string, req discovery.Constraints) (discovery.PluginMeta, error) { + providerInstaller: callbackPluginInstaller(func(provider string, req discovery.Constraints) (discovery.PluginMeta, tfdiags.Diagnostics, error) { t.Fatalf("plugin installer should not have been called for %q", provider) - return discovery.PluginMeta{}, nil + return discovery.PluginMeta{}, tfdiags.Diagnostics{}, nil }), } diff --git a/command/plugins_test.go b/command/plugins_test.go index 28977044f..c95d36330 100644 --- a/command/plugins_test.go +++ b/command/plugins_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/plugin/discovery" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/terraform" + "github.com/hashicorp/terraform/tfdiags" ) func TestMultiVersionProviderResolver(t *testing.T) { @@ -146,16 +147,17 @@ func (i *mockProviderInstaller) FileName(provider, version string) string { return fmt.Sprintf("terraform-provider-%s_v%s_x4", provider, version) } -func (i *mockProviderInstaller) Get(provider string, req discovery.Constraints) (discovery.PluginMeta, error) { +func (i *mockProviderInstaller) Get(provider string, req discovery.Constraints) (discovery.PluginMeta, tfdiags.Diagnostics, error) { + var diags tfdiags.Diagnostics noMeta := discovery.PluginMeta{} versions := i.Providers[provider] if len(versions) == 0 { - return noMeta, fmt.Errorf("provider %q not found", provider) + return noMeta, diags, fmt.Errorf("provider %q not found", provider) } err := os.MkdirAll(i.Dir, 0755) if err != nil { - return noMeta, fmt.Errorf("error creating plugins directory: %s", err) + return noMeta, diags, fmt.Errorf("error creating plugins directory: %s", err) } for _, v := range versions { @@ -170,18 +172,18 @@ func (i *mockProviderInstaller) Get(provider string, req discovery.Constraints) path := filepath.Join(i.Dir, name) f, err := os.Create(path) if err != nil { - return noMeta, fmt.Errorf("error fetching provider: %s", err) + return noMeta, diags, fmt.Errorf("error fetching provider: %s", err) } f.Close() return discovery.PluginMeta{ Name: provider, Version: discovery.VersionStr(v), Path: path, - }, nil + }, diags, nil } } - return noMeta, fmt.Errorf("no suitable version for provider %q found with constraints %s", provider, req) + return noMeta, diags, fmt.Errorf("no suitable version for provider %q found with constraints %s", provider, req) } func (i *mockProviderInstaller) PurgeUnused(map[string]discovery.PluginMeta) (discovery.PluginMetaSet, error) { @@ -197,7 +199,7 @@ func (i *mockProviderInstaller) PurgeUnused(map[string]discovery.PluginMeta) (di type callbackPluginInstaller func(provider string, req discovery.Constraints) (discovery.PluginMeta, error) -func (cb callbackPluginInstaller) Get(provider string, req discovery.Constraints) (discovery.PluginMeta, error) { +func (cb callbackPluginInstaller) Get(provider string, req discovery.Constraints) (discovery.PluginMeta, tfdiags.Diagnostics, error) { return cb(provider, req) } diff --git a/plugin/discovery/get.go b/plugin/discovery/get.go index 40f416da5..f9ad9a432 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/terraform/registry/regsrc" "github.com/hashicorp/terraform/registry/response" "github.com/hashicorp/terraform/svchost/disco" + "github.com/hashicorp/terraform/tfdiags" tfversion "github.com/hashicorp/terraform/version" "github.com/mitchellh/cli" ) @@ -54,7 +55,7 @@ func init() { // An Installer maintains a local cache of plugins by downloading plugins // from an online repository. type Installer interface { - Get(name string, req Constraints) (PluginMeta, error) + Get(name string, req Constraints) (PluginMeta, tfdiags.Diagnostics, error) PurgeUnused(used map[string]PluginMeta) (removed PluginMetaSet, err error) } @@ -111,7 +112,9 @@ type ProviderInstaller struct { // are produced under the assumption that if presented to the user they will // be presented alongside context about what is being installed, and thus the // error messages do not redundantly include such information. -func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, error) { +func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, tfdiags.Diagnostics, error) { + var diags tfdiags.Diagnostics + // a little bit of initialization. if i.OS == "" { i.OS = runtime.GOOS @@ -129,19 +132,19 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e // TODO: return multiple errors if err != nil { if registry.IsServiceNotProvided(err) { - return PluginMeta{}, err + return PluginMeta{}, diags, err } - return PluginMeta{}, ErrorNoSuchProvider + return PluginMeta{}, diags, ErrorNoSuchProvider } if len(allVersions.Versions) == 0 { - return PluginMeta{}, ErrorNoSuitableVersion + return PluginMeta{}, diags, ErrorNoSuitableVersion } providerSource := allVersions.ID // Filter the list of plugin versions to those which meet the version constraints versions := allowedVersions(allVersions, req) if len(versions) == 0 { - return PluginMeta{}, ErrorNoSuitableVersion + return PluginMeta{}, diags, ErrorNoSuitableVersion } // sort them newest to oldest. The newest version wins! @@ -152,7 +155,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e if err := i.checkPlatformCompatibility(versions[0]); err != nil { versions = i.platformCompatibleVersions(versions) if len(versions) == 0 { - return PluginMeta{}, ErrorNoVersionCompatibleWithPlatform + return PluginMeta{}, diags, ErrorNoVersionCompatibleWithPlatform } } @@ -165,7 +168,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e closestMatch, err := i.findClosestProtocolCompatibleVersion(allVersions.Versions) if err != nil { // No operation here if we can't find a version with compatible protocol - return PluginMeta{}, err + return PluginMeta{}, diags, err } // Prompt version suggestion to UI based on closest protocol match @@ -182,7 +185,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e constraintStr = "(any version)" } - return PluginMeta{}, errwrap.Wrap(ErrorVersionIncompatible, fmt.Errorf(fmt.Sprintf( + return PluginMeta{}, diags, errwrap.Wrap(ErrorVersionIncompatible, fmt.Errorf(fmt.Sprintf( errMsg, provider, v.String(), tfversion.String(), closestVersion.String(), closestVersion.MinorUpgradeConstraintStr(), constraintStr))) } @@ -193,7 +196,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e if !i.SkipVerify { sha256, err := i.getProviderChecksum(downloadURLs) if err != nil { - return PluginMeta{}, err + return PluginMeta{}, diags, err } // add the checksum parameter for go-getter to verify the download for us. @@ -207,7 +210,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e log.Printf("[DEBUG] getting provider %q version %q", printedProviderName, versionMeta.Version) err = i.install(provider, v, providerURL) if err != nil { - return PluginMeta{}, err + return PluginMeta{}, diags, err } // Find what we just installed @@ -224,7 +227,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e // This should never happen. Suggests that the release archive // contains an executable file whose name doesn't match the // expected convention. - return PluginMeta{}, fmt.Errorf( + return PluginMeta{}, diags, fmt.Errorf( "failed to find installed plugin version %s; this is a bug in Terraform and should be reported", versionMeta.Version, ) @@ -235,7 +238,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e // particular version was re-released with a different // executable filename. We consider releases as immutable, so // this is an error. - return PluginMeta{}, fmt.Errorf( + return PluginMeta{}, diags, fmt.Errorf( "multiple plugins installed for version %s; this is a bug in Terraform and should be reported", versionMeta.Version, ) @@ -243,8 +246,7 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e // By now we know we have exactly one meta, and so "Newest" will // return that one. - return metas.Newest(), nil - + return metas.Newest(), diags, nil } func (i *ProviderInstaller) install(provider string, version Version, url string) error { diff --git a/plugin/discovery/get_test.go b/plugin/discovery/get_test.go index 870595dd4..f4ee8bef3 100644 --- a/plugin/discovery/get_test.go +++ b/plugin/discovery/get_test.go @@ -415,7 +415,7 @@ func TestProviderInstallerGet(t *testing.T) { Ui: cli.NewMockUi(), registry: registry.NewClient(Disco(server), nil), } - _, err = i.Get("test", AllVersions) + _, _, err = i.Get("test", AllVersions) if err != ErrorNoVersionCompatibleWithPlatform { t.Fatal("want error for incompatible version") @@ -432,20 +432,20 @@ func TestProviderInstallerGet(t *testing.T) { } { - _, err := i.Get("test", ConstraintStr(">9.0.0").MustParse()) + _, _, err := i.Get("test", ConstraintStr(">9.0.0").MustParse()) if err != ErrorNoSuitableVersion { t.Fatal("want error for mismatching constraints") } } { - _, err := i.Get("nonexist", AllVersions) + _, _, err := i.Get("nonexist", AllVersions) if err != ErrorNoSuchProvider { t.Fatal("want error for no such provider") } } - gotMeta, err := i.Get("test", AllVersions) + gotMeta, _, err := i.Get("test", AllVersions) if err != nil { t.Fatal(err) } @@ -503,7 +503,7 @@ func TestProviderInstallerGet_cache(t *testing.T) { Arch: "mockarch", } - gotMeta, err := i.Get("test", AllVersions) + gotMeta, _, err := i.Get("test", AllVersions) if err != nil { t.Fatal(err) } diff --git a/tools/terraform-bundle/package.go b/tools/terraform-bundle/package.go index 62ebc8f7c..5d0bf3b65 100644 --- a/tools/terraform-bundle/package.go +++ b/tools/terraform-bundle/package.go @@ -181,7 +181,7 @@ func (c *PackageCommand) Run(args []string) int { } else { //attempt to get from the public registry if not found locally c.ui.Output(fmt.Sprintf("- Checking for provider plugin on %s...", releaseHost)) - _, err := installer.Get(name, constraint) + _, _, err := installer.Get(name, constraint) if err != nil { c.ui.Error(fmt.Sprintf("- Failed to resolve %s provider %s: %s", name, constraint, err)) return 1