From 5969272c23bcd5c1327da8514598b04538478ced Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Thu, 18 Jul 2019 12:52:20 -0700 Subject: [PATCH] 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 * Add TODOs about requested fixes. Signed-off-by: Ahmet Alp Balkan --- cmd/krew/cmd/list.go | 2 +- cmd/krew/cmd/search.go | 2 +- cmd/krew/cmd/upgrade.go | 2 +- pkg/installation/util.go | 36 ++++++++++++++++++------------------ pkg/receipt/receipt.go | 7 +++++++ pkg/receipt/receipt_test.go | 19 +++++++++++++++++++ 6 files changed, 47 insertions(+), 21 deletions(-) diff --git a/cmd/krew/cmd/list.go b/cmd/krew/cmd/list.go index bfdd9dfd..794f80b2 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.InstallPath(), paths.BinPath()) + plugins, err := installation.ListInstalledPlugins(paths.InstallReceiptsPath()) if err != nil { return errors.Wrap(err, "failed to find all installed versions") } diff --git a/cmd/krew/cmd/search.go b/cmd/krew/cmd/search.go index 4751c8b9..b6ace8ec 100644 --- a/cmd/krew/cmd/search.go +++ b/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") } diff --git a/cmd/krew/cmd/upgrade.go b/cmd/krew/cmd/upgrade.go index af31cc84..8edcc14e 100644 --- a/cmd/krew/cmd/upgrade.go +++ b/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") } diff --git a/pkg/installation/util.go b/pkg/installation/util.go index 46a44cb7..59653e8e 100644 --- a/pkg/installation/util.go +++ b/pkg/installation/util.go @@ -15,7 +15,6 @@ package installation import ( - "io/ioutil" "os" "path/filepath" "strings" @@ -23,8 +22,10 @@ import ( "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 } diff --git a/pkg/receipt/receipt.go b/pkg/receipt/receipt.go index 20773c68..4fd5bb03 100644 --- a/pkg/receipt/receipt.go +++ b/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) +} diff --git a/pkg/receipt/receipt_test.go b/pkg/receipt/receipt_test.go index 37bf8456..41af47de 100644 --- a/pkg/receipt/receipt_test.go +++ b/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) + } +}