Update plugin listings to use install receipts (#276)

* Update plugin listings to use install receipts

- Changed the way we determine the list of "installed plugins" by looking at
  install receipts directory.
- Added a receipt.Load() method for reading saved install receipts.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>

* Add TODOs about requested fixes.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>

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.InstallPath(), paths.BinPath())
+			plugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
 			if err != nil {
 				return errors.Wrap(err, "failed to find all installed versions")
 			}

cmd/krew/cmd/search.go

@@ -53,7 +53,7 @@ Examples:
 			pluginMap[p.Name] = p
 		}
 
-		installed, err := installation.ListInstalledPlugins(paths.InstallPath(), paths.BinPath())
+		installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
 		if err != nil {
 			return errors.Wrap(err, "failed to load installed plugins")
 		}

cmd/krew/cmd/upgrade.go

@@ -40,7 +40,7 @@ kubectl krew upgrade foo bar"`,
 		var pluginNames []string
 		// Upgrade all plugins.
 		if len(args) == 0 {
-			installed, err := installation.ListInstalledPlugins(paths.InstallPath(), paths.BinPath())
+			installed, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath())
 			if err != nil {
 				return errors.Wrap(err, "failed to find all installed versions")
 			}

pkg/installation/util.go

@@ -15,16 +15,17 @@
 package installation
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 
+	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
 	"sigs.k8s.io/krew/pkg/pathutil"
+	"sigs.k8s.io/krew/pkg/receipt"
 )
 
 func findInstalledPluginVersion(installPath, binDir, pluginName string) (name string, installed bool, err error) {
@@ -79,27 +80,26 @@ func getDownloadTarget(index index.Plugin) (version, uri string, fos []index.Fil
 	return version, uri, p.Files, p.Bin, nil
 }
 
-// ListInstalledPlugins returns a list of all name:version for all plugins.
-func ListInstalledPlugins(installDir, binDir string) (map[string]string, error) {
-	installed := make(map[string]string)
-	plugins, err := ioutil.ReadDir(installDir)
+// 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) {
+	// TODO(ahmetb): Write unit tests for this method. Currently blocked by
+	// lack of an in-memory recipt object (issue#270) that we can use to save
+	// receipts to a tempdir that can be read from unit tests.
+
+	matches, err := filepath.Glob(filepath.Join(receiptsDir, "*"+constants.ManifestExtension))
 	if err != nil {
-		return installed, errors.Wrap(err, "failed to read install dir")
+		return nil, errors.Wrapf(err, "failed to grab receipts directory (%s) for manifests", receiptsDir)
 	}
-	glog.V(4).Infof("Read installation directory: %s (%d items)", installDir, len(plugins))
-	for _, plugin := range plugins {
-		if !plugin.IsDir() {
-			glog.V(4).Infof("Skip non-directory item: %s", plugin.Name())
-			continue
-		}
-		version, ok, err := findInstalledPluginVersion(installDir, binDir, plugin.Name())
+	glog.V(4).Infof("Found %d install receipts in %s", len(matches), receiptsDir)
+	installed := make(map[string]string)
+	for _, m := range matches {
+		r, err := receipt.Load(m)
 		if err != nil {
-			return installed, errors.Wrap(err, "failed to get plugin version")
-		}
-		if ok {
-			installed[plugin.Name()] = version
-			glog.V(4).Infof("Found %q, with version %s", plugin.Name(), version)
+			return nil, errors.Wrapf(err, "failed to parse plugin install receipt %s", m)
 		}
+		glog.V(4).Infof("parsed receipt for %s: version=%s", r.GetObjectMeta().GetName(), r.Spec.Version)
+		installed[r.GetObjectMeta().GetName()] = r.Spec.Version
 	}
 	return installed, nil
 }

pkg/receipt/receipt.go

@@ -21,6 +21,7 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"sigs.k8s.io/krew/pkg/index"
+	"sigs.k8s.io/krew/pkg/index/indexscanner"
 )
 
 // Store saves the given plugin at the destination.
@@ -34,3 +35,9 @@ func Store(plugin index.Plugin, dest string) error {
 	err = ioutil.WriteFile(dest, yamlBytes, 0644)
 	return errors.Wrapf(err, "write plugin receipt %q", dest)
 }
+
+// Load reads the plugin receipt at the specified destination.
+// If not found, it returns os.IsNotExist error.
+func Load(path string) (index.Plugin, error) {
+	return indexscanner.ReadPluginFile(path)
+}

pkg/receipt/receipt_test.go

@@ -15,6 +15,8 @@
 package receipt
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -68,3 +70,20 @@ func TestStore(t *testing.T) {
 		t.Fatal(diff)
 	}
 }
+
+func TestLoad(t *testing.T) {
+	// TODO(ahmetb): Avoid reading test data from other packages. It would be
+	// good to have an in-memory Plugin object (issue#270) that we can Store()
+	// first then load here.
+	_, err := Load(filepath.Join("..", "..", "integration_test", "testdata", "foo.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestLoad_preservesNonExistsError(t *testing.T) {
+	_, err := Load(filepath.Join("foo", "non-existing.yaml"))
+	if !os.IsNotExist(err) {
+		t.Fatalf("returned error is not ENOENT: %+v", err)
+	}
+}