From 368ac85a263cfce2ef34450748d8a69659231011 Mon Sep 17 00:00:00 2001 From: findkim Date: Thu, 10 Jan 2019 17:09:37 -0600 Subject: [PATCH] Add provider protocol compatibility UI err msg during registry discovery --- plugin/discovery/get.go | 74 ++++++++++++++++++------- plugin/discovery/get_test.go | 104 +++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 19 deletions(-) diff --git a/plugin/discovery/get.go b/plugin/discovery/get.go index 751844e17..9c095c574 100644 --- a/plugin/discovery/get.go +++ b/plugin/discovery/get.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform/registry/regsrc" "github.com/hashicorp/terraform/registry/response" "github.com/hashicorp/terraform/svchost/disco" + tfversion "github.com/hashicorp/terraform/version" "github.com/mitchellh/cli" ) @@ -160,20 +161,23 @@ func (i *ProviderInstaller) Get(provider string, req Constraints) (PluginMeta, e // check protocol compatibility if err := i.checkPluginProtocol(versionMeta); err != nil { - closestMatch, err := i.findProtocolCompatibleVersion(versions) - if err == nil { - if err := i.checkPlatformCompatibility(closestMatch); err != nil { - // At this point, we have protocol compatibility but not platform, - // and we give up trying to find a compatible version. - // This error message should be improved. - return PluginMeta{}, ErrorNoSuitableVersion - } - // TODO: This is a placeholder UI message. We must choose to send - // providerProtocolTooOld or providerProtocolTooNew message to the UI - i.Ui.Error(fmt.Sprintf("the most recent version of %s to match your platform is %s", provider, closestMatch)) - return PluginMeta{}, ErrorNoVersionCompatible + 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{}, ErrorNoVersionCompatibleWithPlatform + + // Prompt version suggestion to UI based on closest protocol match + closestVersion := VersionStr(closestMatch.Version).MustParse() + var errMsg string + if v.NewerThan(closestVersion) { + errMsg = providerProtocolTooNew + } else { + errMsg = providerProtocolTooOld + } + i.Ui.Error(fmt.Sprintf(errMsg, provider, v.String(), tfversion.String(), + closestVersion.String(), closestVersion.MinorUpgradeConstraintStr())) + return PluginMeta{}, ErrorNoVersionCompatible } downloadURLs, err := i.listProviderDownloadURLs(providerSource, versionMeta.Version) @@ -411,16 +415,48 @@ func (i *ProviderInstaller) listProviderDownloadURLs(name, version string) (*res return urls, err } -// REVIEWER QUESTION: this ends up swallowing a bunch of errors from -// checkPluginProtocol. Do they need to be percolated up better, or would -// debug messages would suffice in these situations? -func (i *ProviderInstaller) findProtocolCompatibleVersion(versions []*response.TerraformProviderVersion) (*response.TerraformProviderVersion, error) { +// findClosestProtocolCompatibleVersion searches for the provider version with the closest protocol match. +// +func (i *ProviderInstaller) findClosestProtocolCompatibleVersion(versions []*response.TerraformProviderVersion) (*response.TerraformProviderVersion, error) { + // Loop through all the provider versions to find the earliest and latest + // versions that match the installer protocol to then select the closest of the two + var latest, earliest *response.TerraformProviderVersion for _, version := range versions { if err := i.checkPluginProtocol(version); err == nil { - return version, nil + if earliest == nil { + // Found the first provider version with compatible protocol + earliest = version + } + // Update the latest protocol compatible version + latest = version } } - return nil, ErrorNoVersionCompatible + if earliest == nil { + // No compatible protocol was found for any version + return nil, ErrorNoVersionCompatible + } + + // Convert protocols to comparable types + protoString := strconv.Itoa(int(i.PluginProtocolVersion)) + protocolVersion, err := VersionStr(protoString).Parse() + if err != nil { + return nil, fmt.Errorf("invalid plugin protocol version: %q", i.PluginProtocolVersion) + } + + earliestVersionProtocol, err := VersionStr(earliest.Protocols[0]).Parse() + if err != nil { + return nil, err + } + + // Compare installer protocol version with the first protocol listed of the earliest match + // [A, B] where A is assumed the earliest compatible major version of the protocol pair + if protocolVersion.NewerThan(earliestVersionProtocol) { + // Provider protocols are too old, the closest version is the earliest compatible version + return earliest, nil + } + + // Provider protocols are too new, the closest version is the latest compatible version + return latest, nil } func (i *ProviderInstaller) checkPluginProtocol(versionMeta *response.TerraformProviderVersion) error { diff --git a/plugin/discovery/get_test.go b/plugin/discovery/get_test.go index 6b48f5f5f..ee9d59cbf 100644 --- a/plugin/discovery/get_test.go +++ b/plugin/discovery/get_test.go @@ -234,6 +234,110 @@ func TestCheckProtocolVersions(t *testing.T) { } } +func TestFindClosestProtocolCompatibleVersion(t *testing.T) { + testCases := []struct { + Name string + PluginProtocolVersion uint + ProviderVersions []*response.TerraformProviderVersion + ExpectedVersion string + Err bool + }{ + { + "no compatible version", + 5, + []*response.TerraformProviderVersion{ + &response.TerraformProviderVersion{ + Version: "1.0.0", + Protocols: []string{"4.0"}, + }, + }, + "", + true, + }, { + "equal, suggests latest", + 4, + []*response.TerraformProviderVersion{ + &response.TerraformProviderVersion{ + Version: "1.0.0", + Protocols: []string{"4.0"}, + }, + &response.TerraformProviderVersion{ + Version: "1.5.0", + Protocols: []string{"4.0"}, + }, + }, + "1.5.0", + false, + }, { + "provider protocol too old, suggests earliest", + 5, + []*response.TerraformProviderVersion{ + &response.TerraformProviderVersion{ + Version: "1.0.0", + Protocols: []string{"4.0"}, + }, + &response.TerraformProviderVersion{ + Version: "2.0.0", + Protocols: []string{"4.0", "5.0"}, + }, + &response.TerraformProviderVersion{ + Version: "2.5.0", + Protocols: []string{"4.0", "5.0"}, + }, + &response.TerraformProviderVersion{ + Version: "3.0.0", + Protocols: []string{"5.0"}, + }, + }, + "2.0.0", + false, + }, { + "provider protocol too new, suggests latest", + 4, + []*response.TerraformProviderVersion{ + &response.TerraformProviderVersion{ + Version: "1.0.0", + Protocols: []string{"4.0"}, + }, + &response.TerraformProviderVersion{ + Version: "2.0.0", + Protocols: []string{"4.0", "5.0"}, + }, + &response.TerraformProviderVersion{ + Version: "2.5.0", + Protocols: []string{"4.0", "5.0"}, + }, + &response.TerraformProviderVersion{ + Version: "3.0.0", + Protocols: []string{"5.0"}, + }, + }, + "2.5.0", + false, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + i := ProviderInstaller{ + Ui: cli.NewMockUi(), + PluginProtocolVersion: tc.PluginProtocolVersion, + } + + closestMatch, err := i.findClosestProtocolCompatibleVersion(tc.ProviderVersions) + if err != nil { + if !tc.Err { + t.Fatalf("unexpected error: %q", err) + } + return + } + if tc.ExpectedVersion != closestMatch.Version { + t.Errorf("Expected %q, got %q", tc.ExpectedVersion, closestMatch.Version) + } + }) + } +} + func TestProviderInstallerGet(t *testing.T) { server := testReleaseServer() server.Start()