Ensure DownloadPath is always clean #447

Removed DownloadPath function.
Used ioutil.TempDir each time a tmpDir was needed.
Removed old tests on DownloadPath.

cmd/krew/cmd/root.go

@@ -78,7 +78,6 @@ func init() {
 
 	paths = environment.MustGetKrewPaths()
 	if err := ensureDirs(paths.BasePath(),
-		paths.DownloadPath(),
 		paths.InstallPath(),
 		paths.BinPath(),
 		paths.InstallReceiptsPath()); err != nil {

cmd/krew/cmd/version.go

@@ -36,7 +36,6 @@ Remarks:
   - BasePath is the root directory for krew installation.
   - IndexPath is the directory that stores the local copy of the index git repository.
   - InstallPath is the directory for plugin installations.
-  - DownloadPath is the directory for temporarily downloading plugins.
   - BinPath is the directory for the symbolic links to the installed plugin executables.`,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		conf := [][]string{
@@ -46,7 +45,6 @@ Remarks:
 			{"BasePath", paths.BasePath()},
 			{"IndexPath", paths.IndexPath()},
 			{"InstallPath", paths.InstallPath()},
-			{"DownloadPath", paths.DownloadPath()},
 			{"BinPath", paths.BinPath()},
 		}
 		return printTable(os.Stdout, []string{"OPTION", "VALUE"}, conf)

integration_test/version_test.go

@@ -35,7 +35,6 @@ func TestKrewVersion(t *testing.T) {
 		`IndexURI\s+https://github.com/kubernetes-sigs/krew-index.git`,
 		"IndexPath",
 		"InstallPath",
-		"DownloadPath",
 		"BinPath",
 	}
 

internal/environment/environment.go

@@ -75,10 +75,6 @@ func (p Paths) InstallReceiptsPath() string { return filepath.Join(p.base, "rece
 // e.g. {BasePath}/bin
 func (p Paths) BinPath() string { return filepath.Join(p.base, "bin") }
 
-// DownloadPath returns a temporary directory for downloading plugins. It does
-// not create a new directory on each call.
-func (p Paths) DownloadPath() string { return filepath.Join(p.tmp, "krew-downloads") }
-
 // InstallPath returns the base directory for plugin installations.
 //
 // e.g. {BasePath}/store

internal/environment/environment_test.go

@@ -69,9 +69,6 @@ func TestPaths(t *testing.T) {
 	if got, expected := p.PluginVersionInstallPath("my-plugin", "v1"), filepath.FromSlash("/foo/store/my-plugin/v1"); got != expected {
 		t.Fatalf("PluginVersionInstallPath()=%s; expected=%s", got, expected)
 	}
-	if got := p.DownloadPath(); !strings.HasSuffix(got, "krew-downloads") {
-		t.Fatalf("DownloadPath()=%s; expected suffix 'krew-downloads'", got)
-	}
 	if got := p.InstallReceiptsPath(); !strings.HasSuffix(got, filepath.FromSlash("receipts")) {
 		t.Fatalf("InstallReceiptsPath()=%s; expected suffix 'receipts'", got)
 	}

internal/installation/install.go

@@ -41,9 +41,8 @@ type installOperation struct {
 	pluginName string
 	platform   index.Platform
 
-	downloadStagingDir string
-	installDir         string
-	binDir             string
+	installDir string
+	binDir     string
 }
 
 // Plugin lifecycle errors
@@ -80,9 +79,8 @@ func Install(p environment.Paths, plugin index.Plugin, opts InstallOpts) error {
 		pluginName: plugin.Name,
 		platform:   candidate,
 
-		downloadStagingDir: filepath.Join(p.DownloadPath(), plugin.Name),
-		binDir:             p.BinPath(),
-		installDir:         p.PluginVersionInstallPath(plugin.Name, plugin.Spec.Version),
+		binDir:     p.BinPath(),
+		installDir: p.PluginVersionInstallPath(plugin.Name, plugin.Spec.Version),
 	}, opts); err != nil {
 		return errors.Wrap(err, "install failed")
 	}
@@ -93,22 +91,24 @@ func Install(p environment.Paths, plugin index.Plugin, opts InstallOpts) error {
 
 func install(op installOperation, opts InstallOpts) error {
 	// Download and extract
-	klog.V(3).Infof("Creating download staging directory %q", op.downloadStagingDir)
-	if err := os.MkdirAll(op.downloadStagingDir, 0755); err != nil {
-		return errors.Wrapf(err, "could not create staging dir %q", op.downloadStagingDir)
+	klog.V(3).Infof("Creating download staging directory")
+	downloadStagingDir, err := ioutil.TempDir("", "krew-downloads")
+	if err != nil {
+		return errors.Wrapf(err, "could not create staging dir %q", downloadStagingDir)
 	}
+	klog.V(3).Infof("Successfully created download staging directory %q", downloadStagingDir)
 	defer func() {
-		klog.V(3).Infof("Deleting the download staging directory %s", op.downloadStagingDir)
-		if err := os.RemoveAll(op.downloadStagingDir); err != nil {
+		klog.V(3).Infof("Deleting the download staging directory %s", downloadStagingDir)
+		if err := os.RemoveAll(downloadStagingDir); err != nil {
 			klog.Warningf("failed to clean up download staging directory: %s", err)
 		}
 	}()
-	if err := downloadAndExtract(op.downloadStagingDir, op.platform.URI, op.platform.Sha256, opts.ArchiveFileOverride); err != nil {
+	if err := downloadAndExtract(downloadStagingDir, op.platform.URI, op.platform.Sha256, opts.ArchiveFileOverride); err != nil {
 		return errors.Wrap(err, "failed to unpack into staging dir")
 	}
 
 	applyDefaults(&op.platform)
-	if err := moveToInstallDir(op.downloadStagingDir, op.installDir, op.platform.Files); err != nil {
+	if err := moveToInstallDir(downloadStagingDir, op.installDir, op.platform.Files); err != nil {
 		return errors.Wrap(err, "failed while moving files to the installation directory")
 	}
 

internal/installation/upgrade.go

@@ -16,7 +16,6 @@ package installation
 
 import (
 	"os"
-	"path/filepath"
 
 	"github.com/pkg/errors"
 	"k8s.io/klog"
@@ -74,11 +73,11 @@ func Upgrade(p environment.Paths, plugin index.Plugin) error {
 	// Re-Install
 	klog.V(1).Infof("Installing new version %s", newVersion)
 	if err := install(installOperation{
-		pluginName:         plugin.Name,
-		platform:           candidate,
-		downloadStagingDir: filepath.Join(p.DownloadPath(), plugin.Name),
-		installDir:         p.PluginVersionInstallPath(plugin.Name, newVersion),
-		binDir:             p.BinPath(),
+		pluginName: plugin.Name,
+		platform:   candidate,
+
+		installDir: p.PluginVersionInstallPath(plugin.Name, newVersion),
+		binDir:     p.BinPath(),
 	}, InstallOpts{}); err != nil {
 		return errors.Wrap(err, "failed to install new version")
 	}