From 24e13d8ec106586c673ef227fad45d55416826b3 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Wed, 13 Mar 2019 12:52:15 -0400 Subject: [PATCH 1/2] 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 From e6316c9de64409d5bc93527be2f8465da7a74b0e Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Wed, 13 Mar 2019 12:52:52 -0400 Subject: [PATCH 2/2] plugin/discovery: Parse warnings from TF Registry Terraform Registry (and other registry implementations) can now return an array of warnings with the versions response. These warnings are now displayed to the user during a `terraform init`. --- command/plugins_test.go | 2 +- plugin/discovery/get.go | 21 +++++++++++++++++++++ registry/response/terraform_provider.go | 1 + 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/command/plugins_test.go b/command/plugins_test.go index c95d36330..4a4153df0 100644 --- a/command/plugins_test.go +++ b/command/plugins_test.go @@ -197,7 +197,7 @@ func (i *mockProviderInstaller) PurgeUnused(map[string]discovery.PluginMeta) (di return ret, nil } -type callbackPluginInstaller func(provider string, req discovery.Constraints) (discovery.PluginMeta, error) +type callbackPluginInstaller func(provider string, req discovery.Constraints) (discovery.PluginMeta, tfdiags.Diagnostics, 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 f9ad9a432..1702bff46 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -136,6 +136,17 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, t } return PluginMeta{}, diags, ErrorNoSuchProvider } + + // Add any warnings from the response to diags + for _, warning := range allVersions.Warnings { + hostname, err := i.hostname() + if err != nil { + return PluginMeta{}, diags, err + } + diag := tfdiags.SimpleWarning(fmt.Sprintf("%s: %s", hostname, warning)) + diags = diags.Append(diag) + } + if len(allVersions.Versions) == 0 { return PluginMeta{}, diags, ErrorNoSuitableVersion } @@ -423,6 +434,16 @@ func (i *ProviderInstaller) getProviderChecksum(urls *response.TerraformProvider return checksumForFile(shasums, urls.Filename), nil } +func (i *ProviderInstaller) hostname() (string, error) { + provider := regsrc.NewTerraformProvider("", i.OS, i.Arch) + svchost, err := provider.SvcHost() + if err != nil { + return "", err + } + + return svchost.ForDisplay(), nil +} + // list all versions available for the named provider func (i *ProviderInstaller) listProviderVersions(name string) (*response.TerraformProviderVersions, error) { provider := regsrc.NewTerraformProvider(name, i.OS, i.Arch) diff --git a/registry/response/terraform_provider.go b/registry/response/terraform_provider.go index 08d382a48..64e454a6c 100644 --- a/registry/response/terraform_provider.go +++ b/registry/response/terraform_provider.go @@ -32,6 +32,7 @@ type TerraformProviderVersion struct { type TerraformProviderVersions struct { ID string `json:"id"` Versions []*TerraformProviderVersion `json:"versions"` + Warnings []string `json:"warnings"` } // TerraformProviderPlatform is the Terraform-specific response structure for a