Refactor upgrade machinery to use semver versions (#278)

This change starts using semver version of plugin manifests to:
- determine if upgrade is needed
- determine if a plugin is installed
by looking at receipts installation directory.

Other commands are probably not yet using receipts entirely.

Signed-off-by: Ahmet Alp Balkan <[email protected]>

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 {

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{}

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,33 +64,32 @@ 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
 	}
 
 	// 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)
 	err = receipt.Store(plugin, p.PluginInstallReceiptPath(plugin.Name))
 	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 {

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.

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