Repository: kubernetes-sigs/krew
Pull request #558 (2020-03-27 19:49:18)

Refactor ListInstalledPlugins
Small refactor that I think will be necessary for updating search/list/upgrade to work with custom indexes.

Related issue: #483 
<!-- For proposed features, make sure there's an issue it's discussed first -->


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/558#" title="Author self-approved">chriskim06</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **soltysh**
You can assign the PR to them by writing `/assign @soltysh` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["soltysh"]} -->
ahmetb: /lgtm
/approve
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/558#issuecomment-605400508" title="Approved">ahmetb</a>*, *<a href="https://github.com/kubernetes-sigs/krew/pull/558#" title="Author self-approved">chriskim06</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)~~ [ahmetb]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/cmd/krew/cmd/list.go b/cmd/krew/cmd/list.go
index a7b400c..6a3e4c7 100644
--- a/cmd/krew/cmd/list.go
+++ b/cmd/krew/cmd/list.go
@@ -40,7 +40,7 @@ Remarks:
   the names of the plugins installed. This output can be piped back to the
   "install" command.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			plugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+			receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 			if err != nil {
 				return errors.Wrap(err, "failed to find all installed versions")
 			}
@@ -48,8 +48,8 @@ Remarks:
 			// return sorted list of plugin names when piped to other commands or file
 			if !isTerminal(os.Stdout) {
 				var names []string
-				for name := range plugins {
-					names = append(names, name)
+				for _, receipt := range receipts {
+					names = append(names, receipt.Name)
 				}
 				sort.Strings(names)
 				fmt.Fprintln(os.Stdout, strings.Join(names, "\n"))
@@ -58,8 +58,8 @@ Remarks:
 
 			// print table
 			var rows [][]string
-			for p, version := range plugins {
-				rows = append(rows, []string{p, version})
+			for _, receipt := range receipts {
+				rows = append(rows, []string{receipt.Name, receipt.Spec.Version})
 			}
 			rows = sortByFirstColumn(rows)
 			return printTable(os.Stdout, []string{"PLUGIN", "VERSION"}, rows)
diff --git a/cmd/krew/cmd/search.go b/cmd/krew/cmd/search.go
index d82b8bf..8a8570c 100644
--- a/cmd/krew/cmd/search.go
+++ b/cmd/krew/cmd/search.go
@@ -54,11 +54,17 @@ Examples:
 			pluginMap[p.Name] = p
 		}
 
-		installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+		receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 		if err != nil {
 			return errors.Wrap(err, "failed to load installed plugins")
 		}
 
+		// TODO(chriskim06) include index name when refactoring for custom indexes
+		installed := make(map[string]string)
+		for _, receipt := range receipts {
+			installed[receipt.Name] = receipt.Spec.Version
+		}
+
 		var matchNames []string
 		if len(args) > 0 {
 			matches := fuzzy.Find(strings.Join(args, ""), names)
diff --git a/cmd/krew/cmd/update.go b/cmd/krew/cmd/update.go
index e44adb5..6345fc8 100644
--- a/cmd/krew/cmd/update.go
+++ b/cmd/krew/cmd/update.go
@@ -120,11 +120,16 @@ func ensureIndexUpdated(_ *cobra.Command, _ []string) error {
 		return errors.Wrap(err, "failed to load plugin index after update")
 	}
 
-	installedPlugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+	receipts, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 	if err != nil {
 		return errors.Wrap(err, "failed to load installed plugins list after update")
 	}
+	installedPlugins := make(map[string]string)
+	for _, receipt := range receipts {
+		installedPlugins[receipt.Name] = receipt.Spec.Version
+	}
 
+	// TODO(chriskim06) consider commenting this out when refactoring for custom indexes
 	showUpdatedPlugins(os.Stderr, preUpdateIndex, posUpdateIndex, installedPlugins)
 
 	return nil
diff --git a/cmd/krew/cmd/upgrade.go b/cmd/krew/cmd/upgrade.go
index 8df9806..760ec27 100644
--- a/cmd/krew/cmd/upgrade.go
+++ b/cmd/krew/cmd/upgrade.go
@@ -47,12 +47,12 @@ kubectl krew upgrade foo bar"`,
 			var pluginNames []string
 			if len(args) == 0 {
 				// Upgrade all plugins.
-				installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
+				installed, err := installation.GetInstalledPluginReceipts(paths.InstallReceiptsPath())
 				if err != nil {
 					return errors.Wrap(err, "failed to find all installed versions")
 				}
-				for name := range installed {
-					pluginNames = append(pluginNames, name)
+				for _, receipt := range installed {
+					pluginNames = append(pluginNames, receipt.Name)
 				}
 				ignoreUpgraded = true
 				skipErrors = true
diff --git a/internal/installation/util.go b/internal/installation/util.go
index 31b56ac..833772b 100644
--- a/internal/installation/util.go
+++ b/internal/installation/util.go
@@ -25,24 +25,10 @@ import (
 	"sigs.k8s.io/krew/pkg/index"
 )
 
-// ListInstalledPlugins returns a list of all install plugins in a
-// name:version format based on the install receipts at the specified dir.
-func ListInstalledPlugins(receiptsDir string) (map[string]string, error) {
-	installed := make(map[string]string)
-	receipts, err := getInstalledPluginReceipts(receiptsDir)
-	if err != nil {
-		return nil, err
-	}
-	for _, r := range receipts {
-		installed[r.GetObjectMeta().GetName()] = r.Spec.Version
-	}
-	return installed, nil
-}
-
 // InstalledPluginsFromIndex returns a list of all install plugins from a particular index.
 func InstalledPluginsFromIndex(receiptsDir, indexName string) ([]index.Receipt, error) {
 	var out []index.Receipt
-	receipts, err := getInstalledPluginReceipts(receiptsDir)
+	receipts, err := GetInstalledPluginReceipts(receiptsDir)
 	if err != nil {
 		return nil, err
 	}
@@ -54,7 +40,8 @@ func InstalledPluginsFromIndex(receiptsDir, indexName string) ([]index.Receipt,
 	return out, nil
 }
 
-func getInstalledPluginReceipts(receiptsDir string) ([]index.Receipt, error) {
+// GetInstalledPluginReceipts returns a list of receipts.
+func GetInstalledPluginReceipts(receiptsDir string) ([]index.Receipt, error) {
 	files, err := filepath.Glob(filepath.Join(receiptsDir, "*"+constants.ManifestExtension))
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to glob receipts directory (%s) for manifests", receiptsDir)


Test patch:
diff --git a/internal/installation/util_test.go b/internal/installation/util_test.go
index ad7c341..45e466e 100644
--- a/internal/installation/util_test.go
+++ b/internal/installation/util_test.go
@@ -33,31 +33,24 @@ func testdataPath(t *testing.T) string {
 	return filepath.Join(pwd, "testdata")
 }
 
-func TestListInstalledPlugins(t *testing.T) {
+func TestGetInstalledPluginReceipts(t *testing.T) {
 	tests := []struct {
 		name     string
-		plugins  []index.Receipt
-		expected map[string]string
+		receipts []index.Receipt
 	}{
 		{
 			name: "single plugin",
-			plugins: []index.Receipt{
+			receipts: []index.Receipt{
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("test").WithVersion("v0.0.1").V()).V(),
 			},
-			expected: map[string]string{"test": "v0.0.1"},
 		},
 		{
 			name: "multiple plugins",
-			plugins: []index.Receipt{
+			receipts: []index.Receipt{
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("plugin-a").WithVersion("v0.0.1").V()).V(),
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("plugin-b").WithVersion("v0.1.0").V()).V(),
 				testutil.NewReceipt().WithPlugin(testutil.NewPlugin().WithName("plugin-c").WithVersion("v1.0.0").V()).V(),
 			},
-			expected: map[string]string{
-				"plugin-a": "v0.0.1",
-				"plugin-b": "v0.1.0",
-				"plugin-c": "v1.0.0",
-			},
 		},
 	}
 
@@ -66,15 +59,15 @@ func TestListInstalledPlugins(t *testing.T) {
 			tempDir, cleanup := testutil.NewTempDir(t)
 			defer cleanup()
 
-			for _, plugin := range test.plugins {
+			for _, plugin := range test.receipts {
 				tempDir.WriteYAML(plugin.Name+constants.ManifestExtension, plugin)
 			}
 
-			actual, err := ListInstalledPlugins(tempDir.Root())
+			actual, err := GetInstalledPluginReceipts(tempDir.Root())
 			if err != nil {
 				t.Fatal(err)
 			}
-			if diff := cmp.Diff(test.expected, actual); diff != "" {
+			if diff := cmp.Diff(test.receipts, actual); diff != "" {
 				t.Error(diff)
 			}
 		})