CLI: Fix version selection for plugin install (#19498)

pkg/cmd/grafana-cli/commands/install_command.go

@@ -157,32 +157,34 @@ func latestSupportedVersion(plugin *m.Plugin) *m.Version {
 
 // SelectVersion returns latest version if none is specified or the specified version. If the version string is not
 // matched to existing version it errors out. It also errors out if version that is matched is not available for current
-// os and platform.
+// os and platform. It expects plugin.Versions to be sorted so the newest version is first.
 func SelectVersion(plugin *m.Plugin, version string) (*m.Version, error) {
-	var ver *m.Version
+	var ver m.Version
-	if version == "" {
-		ver = &plugin.Versions[0]
-	}
-
-	for _, v := range plugin.Versions {
-		if v.Version == version {
-			ver = &v
-		}
-	}
-
-	if ver == nil {
-		return nil, xerrors.New("Could not find the version you're looking for")
-	}
 
 	latestForArch := latestSupportedVersion(plugin)
 	if latestForArch == nil {
 		return nil, xerrors.New("Plugin is not supported on your architecture and os.")
 	}
 
-	if latestForArch.Version == ver.Version {
+	if version == "" {
-		return ver, nil
+		return latestForArch, nil
 	}
+	for _, v := range plugin.Versions {
+		if v.Version == version {
+			ver = v
+			break
+		}
+	}
+
+	if len(ver.Version) == 0 {
+		return nil, xerrors.New("Could not find the version you're looking for")
+	}
+
+	if !supportsCurrentArch(&ver) {
 		return nil, xerrors.Errorf("Version you want is not supported on your architecture and os. Latest suitable version is %v", latestForArch.Version)
+	}
+
+	return &ver, nil
 }
 
 func RemoveGitBuildFromName(pluginName, filename string) string {

pkg/cmd/grafana-cli/commands/install_command_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestFoldernameReplacement(t *testing.T) {
+func TestFolderNameReplacement(t *testing.T) {
 	Convey("path containing git commit path", t, func() {
 		pluginName := "datasource-plugin-kairosdb"
 
@@ -134,7 +134,68 @@ func TestIsPathSafe(t *testing.T) {
 		assert.False(t, isPathSafe("../../", dest))
 		assert.False(t, isPathSafe("../../test", dest))
 	})
+}
 
+func TestSelectVersion(t *testing.T) {
+	t.Run("Should return error when requested version does not exist", func(t *testing.T) {
+		_, err := SelectVersion(
+			makePluginWithVersions(versionArg{Version: "version"}),
+			"1.1.1",
+		)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("Should return error when no version supports current arch", func(t *testing.T) {
+		_, err := SelectVersion(
+			makePluginWithVersions(versionArg{Version: "version", Arch: []string{"non-existent"}}),
+			"",
+		)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("Should return error when requested version does not support current arch", func(t *testing.T) {
+		_, err := SelectVersion(
+			makePluginWithVersions(
+				versionArg{Version: "2.0.0"},
+				versionArg{Version: "1.1.1", Arch: []string{"non-existent"}},
+			),
+			"1.1.1",
+		)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("Should return latest available for arch when no version specified", func(t *testing.T) {
+		ver, err := SelectVersion(
+			makePluginWithVersions(
+				versionArg{Version: "2.0.0", Arch: []string{"non-existent"}},
+				versionArg{Version: "1.0.0"},
+			),
+			"",
+		)
+		assert.Nil(t, err)
+		assert.Equal(t, "1.0.0", ver.Version)
+	})
+
+	t.Run("Should return latest version when no version specified", func(t *testing.T) {
+		ver, err := SelectVersion(
+			makePluginWithVersions(versionArg{Version: "2.0.0"}, versionArg{Version: "1.0.0"}),
+			"",
+		)
+		assert.Nil(t, err)
+		assert.Equal(t, "2.0.0", ver.Version)
+	})
+
+	t.Run("Should return requested version", func(t *testing.T) {
+		ver, err := SelectVersion(
+			makePluginWithVersions(
+				versionArg{Version: "2.0.0"},
+				versionArg{Version: "1.0.0"},
+			),
+			"1.0.0",
+		)
+		assert.Nil(t, err)
+		assert.Equal(t, "1.0.0", ver.Version)
+	})
 }
 
 func setupPluginInstallCmd(t *testing.T, pluginDir string) utils.CommandLine {
@@ -199,3 +260,35 @@ func skipWindows(t *testing.T) {
 		t.Skip("Skipping test on Windows")
 	}
 }
+
+type versionArg struct {
+	Version string
+	Arch    []string
+}
+
+func makePluginWithVersions(versions ...versionArg) *models.Plugin {
+	plugin := &models.Plugin{
+		Id:       "",
+		Category: "",
+		Versions: []models.Version{},
+	}
+
+	for _, version := range versions {
+		ver := models.Version{
+			Version: version.Version,
+			Commit:  fmt.Sprintf("commit_%s", version.Version),
+			Url:     fmt.Sprintf("url_%s", version.Version),
+		}
+		if version.Arch != nil {
+			ver.Arch = map[string]models.ArchMeta{}
+			for _, arch := range version.Arch {
+				ver.Arch[arch] = models.ArchMeta{
+					Md5: fmt.Sprintf("md5_%s", arch),
+				}
+			}
+		}
+		plugin.Versions = append(plugin.Versions, ver)
+	}
+
+	return plugin
+}