From e9dc9071cbf620d6c109ce7fb1af6777f3bc480c Mon Sep 17 00:00:00 2001 From: aimbot31 Date: Mon, 13 Jan 2020 11:47:19 +0100 Subject: [PATCH] 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 | 1 - cmd/krew/cmd/version.go | 2 -- integration_test/version_test.go | 1 - internal/environment/environment.go | 4 ---- internal/environment/environment_test.go | 3 --- internal/installation/install.go | 26 ++++++++++++------------ internal/installation/upgrade.go | 11 +++++----- 7 files changed, 18 insertions(+), 30 deletions(-) diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go index 1679eb49..a5888899 100644 --- a/cmd/krew/cmd/root.go +++ b/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 { diff --git a/cmd/krew/cmd/version.go b/cmd/krew/cmd/version.go index 2f7a92c5..7bc2618d 100644 --- a/cmd/krew/cmd/version.go +++ b/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) diff --git a/integration_test/version_test.go b/integration_test/version_test.go index dfc2e639..0da795a5 100644 --- a/integration_test/version_test.go +++ b/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", } diff --git a/internal/environment/environment.go b/internal/environment/environment.go index 7df4991c..9ebbf890 100644 --- a/internal/environment/environment.go +++ b/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 diff --git a/internal/environment/environment_test.go b/internal/environment/environment_test.go index 7c0c24e1..d80a34cf 100644 --- a/internal/environment/environment_test.go +++ b/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) } diff --git a/internal/installation/install.go b/internal/installation/install.go index c2837780..4d7ab8ca 100644 --- a/internal/installation/install.go +++ b/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") } diff --git a/internal/installation/upgrade.go b/internal/installation/upgrade.go index b329a985..7ad5ab09 100644 --- a/internal/installation/upgrade.go +++ b/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") }