diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go index 10679647..c2cd80d4 100644 --- a/cmd/krew/cmd/install.go +++ b/cmd/krew/cmd/install.go @@ -121,7 +121,7 @@ Remarks: fmt.Fprintf(os.Stderr, "Installing plugin: %s\n", plugin.Name) err := installation.Install(paths, plugin, *forceDownloadFile) if err == installation.ErrIsAlreadyInstalled { - glog.Warningf("Skipping plugin %s, it is already installed", plugin.Name) + glog.Warningf("Skipping plugin %q, it is already installed", plugin.Name) continue } if err != nil { diff --git a/pkg/download/verifier.go b/pkg/download/verifier.go index 61c65088..459e50a9 100644 --- a/pkg/download/verifier.go +++ b/pkg/download/verifier.go @@ -53,7 +53,7 @@ func (v sha256Verifier) Verify() error { if bytes.Equal(v.wantedHash, v.Sum(nil)) { return nil } - return errors.Errorf("checksum does not match, wantReader: %x, got %x", v.wantedHash, v.Sum(nil)) + return errors.Errorf("checksum does not match, want: %x, got %x", v.wantedHash, v.Sum(nil)) } var _ Verifier = trueVerifier{} diff --git a/pkg/installation/install.go b/pkg/installation/install.go index 700cd8eb..e00399c2 100644 --- a/pkg/installation/install.go +++ b/pkg/installation/install.go @@ -41,7 +41,7 @@ const ( krewPluginName = "krew" ) -func downloadAndMove(version, uri string, fos []index.FileOperation, downloadPath, installPath, forceDownloadFile string) (dst string, err error) { +func downloadAndMove(version, sha256sum, uri string, fos []index.FileOperation, downloadPath, installPath, forceDownloadFile string) (dst string, err error) { glog.V(3).Infof("Creating download dir %q", downloadPath) if err = os.MkdirAll(downloadPath, 0755); err != nil { return "", errors.Wrapf(err, "could not create download path %q", downloadPath) @@ -53,7 +53,7 @@ func downloadAndMove(version, uri string, fos []index.FileOperation, downloadPat fetcher = download.NewFileFetcher(forceDownloadFile) } - verifier := download.NewSha256Verifier(version) + verifier := download.NewSha256Verifier(sha256sum) if err := download.NewDownloader(verifier, fetcher).Get(uri, downloadPath); err != nil { return "", errors.Wrap(err, "failed to download and verify file") } @@ -64,16 +64,15 @@ func downloadAndMove(version, uri string, fos []index.FileOperation, downloadPat // to not get the plugin dir in a bad state if it fails during the process. func Install(p environment.Paths, plugin index.Plugin, forceDownloadFile string) error { glog.V(2).Infof("Looking for installed versions") - _, ok, err := findInstalledPluginVersion(p.InstallPath(), p.BinPath(), plugin.Name) - if err != nil { - return err - } - if ok { + _, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) + if err == nil { return ErrIsAlreadyInstalled + } else if !os.IsNotExist(err) { + return errors.Wrap(err, "failed to look up plugin receipt") } glog.V(1).Infof("Finding download target for plugin %s", plugin.Name) - version, uri, fos, bin, err := getDownloadTarget(plugin) + version, sha256, uri, fos, bin, err := getDownloadTarget(plugin) if err != nil { return err } @@ -81,7 +80,7 @@ func Install(p environment.Paths, plugin index.Plugin, forceDownloadFile string) // The actual install should be the last action so that a failure during receipt // saving does not result in an installed plugin without receipt. glog.V(3).Infof("Install plugin %s", plugin.Name) - if err := install(plugin.Name, version, uri, bin, p, fos, forceDownloadFile); err != nil { + if err := install(plugin.Name, version, sha256, uri, bin, p, fos, forceDownloadFile); err != nil { return errors.Wrap(err, "install failed") } glog.V(3).Infof("Storing install receipt for plugin %s", plugin.Name) @@ -89,8 +88,8 @@ func Install(p environment.Paths, plugin index.Plugin, forceDownloadFile string) return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail") } -func install(plugin, version, uri, bin string, p environment.Paths, fos []index.FileOperation, forceDownloadFile string) error { - dst, err := downloadAndMove(version, uri, fos, filepath.Join(p.DownloadPath(), plugin), p.PluginInstallPath(plugin), forceDownloadFile) +func install(plugin, version, sha256sum, uri, bin string, p environment.Paths, fos []index.FileOperation, forceDownloadFile string) error { + dst, err := downloadAndMove(version, sha256sum, uri, fos, filepath.Join(p.DownloadPath(), plugin), p.PluginInstallPath(plugin), forceDownloadFile) if err != nil { return errors.Wrap(err, "failed to download and move during installation") } @@ -113,18 +112,22 @@ func install(plugin, version, uri, bin string, p environment.Paths, fos []index. // Uninstall will uninstall a plugin. func Uninstall(p environment.Paths, name string) error { if name == krewPluginName { - return errors.Errorf("removing krew is not allowed through krew. Please run:\n\t rm -r %s", p.BasePath()) + glog.Errorf("Removing krew through krew is not supported.") + if !isWindows() { // assume POSIX-like + glog.Errorf("If you’d like to uninstall krew altogether, run:\n\trm -rf -- %q", p.BasePath()) + } + return errors.New("self-uninstall not allowed") } glog.V(3).Infof("Finding installed version to delete") - version, installed, err := findInstalledPluginVersion(p.InstallPath(), p.BinPath(), name) - if err != nil { - return errors.Wrap(err, "can't uninstall plugin") - } - if !installed { - return ErrIsNotInstalled + + if _, err := receipt.Load(p.PluginInstallReceiptPath(name)); err != nil { + if os.IsNotExist(err) { + return ErrIsNotInstalled + } + return errors.Wrapf(err, "failed to look up install receipt for plugin %q", name) } - glog.V(1).Infof("Deleting plugin version %s", version) + glog.V(1).Infof("Deleting plugin %s", name) symlinkPath := filepath.Join(p.BinPath(), pluginNameToBin(name, isWindows())) glog.V(3).Infof("Unlink %q", symlinkPath) @@ -137,10 +140,10 @@ func Uninstall(p environment.Paths, name string) error { if err := os.RemoveAll(pluginInstallPath); err != nil { return errors.Wrapf(err, "could not remove plugin directory %q", pluginInstallPath) } - PluginInstallReceiptPath := p.PluginInstallReceiptPath(name) - glog.V(3).Infof("Deleting plugin receipt %q", PluginInstallReceiptPath) - err = os.Remove(PluginInstallReceiptPath) - return errors.Wrapf(err, "could not remove plugin receipt %q", PluginInstallReceiptPath) + pluginReceiptPath := p.PluginInstallReceiptPath(name) + glog.V(3).Infof("Deleting plugin receipt %q", pluginReceiptPath) + err := os.Remove(pluginReceiptPath) + return errors.Wrapf(err, "could not remove plugin receipt %q", pluginReceiptPath) } func createOrUpdateLink(binDir string, binary string, plugin string) error { diff --git a/pkg/installation/upgrade.go b/pkg/installation/upgrade.go index cd4b8cb2..e0abb988 100644 --- a/pkg/installation/upgrade.go +++ b/pkg/installation/upgrade.go @@ -20,6 +20,7 @@ import ( "sigs.k8s.io/krew/pkg/environment" "sigs.k8s.io/krew/pkg/index" + "sigs.k8s.io/krew/pkg/installation/semver" "sigs.k8s.io/krew/pkg/receipt" "github.com/golang/glog" @@ -29,22 +30,34 @@ import ( // Upgrade will reinstall and delete the old plugin. The operation tries // to not get the plugin dir in a bad state if it fails during the process. func Upgrade(p environment.Paths, plugin index.Plugin) error { - oldVersion, ok, err := findInstalledPluginVersion(p.InstallPath(), p.BinPath(), plugin.Name) + installReceipt, err := receipt.Load(p.PluginInstallReceiptPath(plugin.Name)) if err != nil { - return errors.Wrap(err, "could not detect installed plugin oldVersion") - } - if !ok { - return errors.Errorf("can't upgrade plugin %q, it is not installed", plugin.Name) + return errors.Wrapf(err, "failed to load install receipt for plugin %q", plugin.Name) } - // Check allowed installation - newVersion, uri, fos, binName, err := getDownloadTarget(plugin) - if oldVersion == newVersion { - return ErrIsAlreadyUpgraded + curVersion := installReceipt.Spec.Version + curv, err := semver.Parse(curVersion) + if err != nil { + return errors.Wrapf(err, "failed to parse installed version (%q) plugin %q as semantic version", curVersion, plugin.Name) } + + // Find available installation candidate + newVersion, sha256sum, uri, fos, binName, err := getDownloadTarget(plugin) if err != nil { return errors.Wrap(err, "failed to get the current download target") } + newv, err := semver.Parse(newVersion) + if err != nil { + return errors.Wrapf(err, "failed to parse installation candidate version spec (%q) for plugin %q", newVersion, plugin.Name) + } + glog.V(2).Infof("Comparing versions: current=%s target=%s", curv, newv) + + // See if it's a newer version + if !semver.Less(curv, newv) { + glog.V(3).Infof("Plugin does not need upgrade (%s ≥ %s)", curv, newv) + return ErrIsAlreadyUpgraded + } + glog.V(1).Infof("Plugin needs upgrade (%s < %s)", curv, newv) glog.V(2).Infof("Upgrading install receipt for plugin %s", plugin.Name) if err = receipt.Store(plugin, p.PluginInstallReceiptPath(plugin.Name)); err != nil { @@ -53,13 +66,13 @@ func Upgrade(p environment.Paths, plugin index.Plugin) error { // Re-Install glog.V(1).Infof("Installing new version %s", newVersion) - if err := install(plugin.Name, newVersion, uri, binName, p, fos, ""); err != nil { + if err := install(plugin.Name, newVersion, sha256sum, uri, binName, p, fos, ""); err != nil { return errors.Wrap(err, "failed to install new version") } // Clean old installations glog.V(4).Infof("Starting old version cleanup") - return removePluginVersionFromFS(p, plugin, newVersion, oldVersion) + return removePluginVersionFromFS(p, plugin, newVersion, curVersion) } // removePluginVersionFromFS will remove a plugin directly if it not krew. diff --git a/pkg/installation/util.go b/pkg/installation/util.go index 59653e8e..d8a29023 100644 --- a/pkg/installation/util.go +++ b/pkg/installation/util.go @@ -15,9 +15,7 @@ package installation import ( - "os" "path/filepath" - "strings" "github.com/golang/glog" "github.com/pkg/errors" @@ -28,31 +26,6 @@ import ( "sigs.k8s.io/krew/pkg/receipt" ) -func findInstalledPluginVersion(installPath, binDir, pluginName string) (name string, installed bool, err error) { - if !index.IsSafePluginName(pluginName) { - return "", false, errors.Errorf("the plugin name %q is not allowed", pluginName) - } - glog.V(3).Infof("Searching for installed versions of %s in %q", pluginName, binDir) - link, err := os.Readlink(filepath.Join(binDir, pluginNameToBin(pluginName, isWindows()))) - if os.IsNotExist(err) { - return "", false, nil - } else if err != nil { - return "", false, errors.Wrap(err, "could not read plugin link") - } - - if !filepath.IsAbs(link) { - if link, err = filepath.Abs(filepath.Join(binDir, link)); err != nil { - return "", true, errors.Wrapf(err, "failed to get the absolute path for the link of %q", link) - } - } - - name, err = pluginVersionFromPath(installPath, link) - if err != nil { - return "", true, errors.Wrap(err, "cloud not parse plugin version") - } - return name, true, nil -} - func pluginVersionFromPath(installPath, pluginPath string) (string, error) { // plugin path: {install_path}/{plugin_name}/{version}/... elems, ok := pathutil.IsSubPath(installPath, pluginPath) @@ -62,22 +35,23 @@ func pluginVersionFromPath(installPath, pluginPath string) (string, error) { return elems[1], nil } -func getPluginVersion(p index.Platform) (version, uri string) { - return strings.ToLower(p.Sha256), p.URI -} - -func getDownloadTarget(index index.Plugin) (version, uri string, fos []index.FileOperation, bin string, err error) { +func getDownloadTarget(index index.Plugin) (version, sha256sum, uri string, fos []index.FileOperation, bin string, err error) { + // TODO(ahmetb): We have many return values from this method, indicating + // code smell. More specifically we return all-or-nothing, so ideally this + // should be converted into a struct, like InstallOperation{} contains all + // the data needed to install a plugin. p, ok, err := index.Spec.GetMatchingPlatform() if err != nil { - return "", "", nil, p.Bin, errors.Wrap(err, "failed to get matching platforms") + return "", "", "", nil, p.Bin, errors.Wrap(err, "failed to get matching platforms") } if !ok { - return "", "", nil, p.Bin, errors.New("no matching platform found") + return "", "", "", nil, p.Bin, errors.New("no matching platform found") } - version, uri = getPluginVersion(p) - glog.V(4).Infof("Matching plugin version is %s", version) - - return version, uri, p.Files, p.Bin, nil + version = index.Spec.Version + uri = p.URI + sha256sum = p.Sha256 + glog.V(4).Infof("found a matching platform, version=%s checksum=%s", version, sha256sum) + return version, sha256sum, uri, p.Files, p.Bin, nil } // ListInstalledPlugins returns a list of all install plugins in a diff --git a/pkg/installation/util_test.go b/pkg/installation/util_test.go index 06153928..925c5465 100644 --- a/pkg/installation/util_test.go +++ b/pkg/installation/util_test.go @@ -25,23 +25,6 @@ import ( "sigs.k8s.io/krew/pkg/index" ) -func Test_getPluginVersion(t *testing.T) { - wantVersion := "deadbeef" - wantURI := "https://uri.git" - platform := index.Platform{ - URI: "https://uri.git", - Sha256: "dEaDbEeF", - } - - gotVersion, gotURI := getPluginVersion(platform) - if gotVersion != wantVersion { - t.Errorf("getPluginVersion() gotVersion = %v, want %v", gotVersion, wantVersion) - } - if gotURI != wantURI { - t.Errorf("getPluginVersion() gotURI = %v, want %v", gotURI, wantURI) - } -} - func Test_getDownloadTarget(t *testing.T) { matchingPlatform := index.Platform{ URI: "https://uri.git", @@ -58,19 +41,21 @@ func Test_getDownloadTarget(t *testing.T) { index index.Plugin } tests := []struct { - name string - args args - wantVersion string - wantURI string - wantFos []index.FileOperation - wantBin string - wantErr bool + name string + args args + wantVersion string + wantSHA256Sum string + wantURI string + wantFos []index.FileOperation + wantBin string + wantErr bool }{ { name: "Find Matching Platform", args: args{ index: index.Plugin{ Spec: index.PluginSpec{ + Version: "v1.0.1", Platforms: []index.Platform{ matchingPlatform, { @@ -85,16 +70,18 @@ func Test_getDownloadTarget(t *testing.T) { }, }, }, - wantVersion: "deadbeef", - wantURI: "https://uri.git", - wantFos: nil, - wantBin: "kubectl-foo", - wantErr: false, + wantVersion: "v1.0.1", + wantSHA256Sum: "deadbeef", + wantURI: "https://uri.git", + wantFos: nil, + wantBin: "kubectl-foo", + wantErr: false, }, { name: "No Matching Platform", args: args{ index: index.Plugin{ Spec: index.PluginSpec{ + Version: "v1.0.2", Platforms: []index.Platform{ { URI: "https://wrong.com", @@ -108,16 +95,12 @@ func Test_getDownloadTarget(t *testing.T) { }, }, }, - wantVersion: "", - wantURI: "", - wantFos: nil, - wantBin: "", - wantErr: true, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotVersion, gotURI, gotFos, bin, err := getDownloadTarget(tt.args.index) + gotVersion, gotSHA256Sum, gotURI, gotFos, bin, err := getDownloadTarget(tt.args.index) if (err != nil) != tt.wantErr { t.Errorf("getDownloadTarget() error = %v, wantErr %v", err, tt.wantErr) return @@ -125,6 +108,9 @@ func Test_getDownloadTarget(t *testing.T) { if gotVersion != tt.wantVersion { t.Errorf("getDownloadTarget() gotVersion = %v, want %v", gotVersion, tt.wantVersion) } + if gotSHA256Sum != tt.wantSHA256Sum { + t.Errorf("getDownloadTarget() gotSHA256Sum = %v, want %v", gotSHA256Sum, tt.wantSHA256Sum) + } if bin != tt.wantBin { t.Errorf("getDownloadTarget() bin = %v, want %v", bin, tt.wantBin) } @@ -138,68 +124,6 @@ func Test_getDownloadTarget(t *testing.T) { } } -func Test_findInstalledPluginVersion(t *testing.T) { - type args struct { - installPath string - binDir string - pluginName string - } - tests := []struct { - name string - args args - wantName string - wantInstalled bool - wantErr bool - }{ - { - name: "Find version", - args: args{ - installPath: filepath.Join(testdataPath(t), "index"), - binDir: filepath.Join(testdataPath(t), "bin"), - pluginName: "foo", - }, - wantName: "deadbeef", - wantInstalled: true, - wantErr: false, - }, { - name: "No installed version", - args: args{ - installPath: filepath.Join(testdataPath(t), "index"), - binDir: filepath.Join(testdataPath(t), "bin"), - pluginName: "not-found", - }, - wantName: "", - wantInstalled: false, - wantErr: false, - }, { - name: "Insecure name", - args: args{ - installPath: filepath.Join(testdataPath(t), "index"), - binDir: filepath.Join(testdataPath(t), "bin"), - pluginName: "../foo", - }, - wantName: "", - wantInstalled: false, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotName, gotInstalled, err := findInstalledPluginVersion(tt.args.installPath, tt.args.binDir, tt.args.pluginName) - if (err != nil) != tt.wantErr { - t.Errorf("getOtherInstalledVersion() error = %v, wantErr %v", err, tt.wantErr) - return - } - if gotName != tt.wantName { - t.Errorf("getOtherInstalledVersion() gotName = %v, want %v", gotName, tt.wantName) - } - if gotInstalled != tt.wantInstalled { - t.Errorf("getOtherInstalledVersion() gotInstalled = %v, want %v", gotInstalled, tt.wantInstalled) - } - }) - } -} - func testdataPath(t *testing.T) string { pwd, err := filepath.Abs(".") if err != nil {