fix leaking of tmp files during release upload

Signed-off-by: Tom Viehman <[email protected]>

cmd/upload_release.go

@@ -109,6 +109,8 @@ func (c UploadReleaseCmd) uploadFile(opts UploadReleaseOpts) error {
 		}
 	}
 
+	defer release.CleanUp()
+
 	return c.uploadRelease(release, opts)
 }
 
@@ -128,6 +130,8 @@ func (c UploadReleaseCmd) uploadRelease(release boshrel.Release, opts UploadRele
 		return err
 	}
 
+	defer c.fs.RemoveAll(path)
+
 	file, err := c.releaseArchiveFactory(path).File()
 	if err != nil {
 		return bosherr.WrapErrorf(err, "Opening release")

cmd/upload_release_test.go

@@ -42,6 +42,8 @@ var _ = Describe("UploadReleaseCmd", func() {
 		}
 
 		releaseWriter = &fakerel.FakeWriter{}
+		releaseWriter.WriteReturns("/archive-path", nil)
+
 		director = &fakedir.FakeDirector{}
 		cmdRunner = fakesys.NewFakeCmdRunner()
 		fs = fakesys.NewFakeFileSystem()
@@ -274,6 +276,31 @@ var _ = Describe("UploadReleaseCmd", func() {
 				Expect(fix).To(BeFalse())
 			})
 
+			It("clean up release", func() {
+				releaseReader.ReadStub = func(path string) (boshrel.Release, error) {
+					Expect(path).To(Equal("./some-file.tgz"))
+					return release, nil
+				}
+
+				releaseWriter.WriteStub = func(rel boshrel.Release, _ []string) (string, error) {
+					Expect(rel).To(Equal(release))
+					return "/archive-path", nil
+				}
+
+				removedFiles := []string{}
+
+				fs.RemoveAllStub = func(path string) error {
+					removedFiles = append(removedFiles, path)
+					return nil
+				}
+
+				err := act()
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(release.CleanUpCallCount()).To(Equal(1))
+				Expect(removedFiles).To(Equal([]string{"/archive-path"}))
+			})
+
 			It("uploads given release with a fix flag hence does not filter out any packages", func() {
 				opts.Fix = true
 

integration/suite_test.go

@@ -13,31 +13,28 @@ import (
 
 var (
 	originalHome string
+	testHome     string
 )
 
 func TestIntegration(t *testing.T) {
 	RegisterFailHandler(Fail)
 
-	var (
-		homePath string
-	)
-
 	BeforeEach(func() {
 		originalHome = os.Getenv("HOME")
 
 		var err error
-		homePath, err = ioutil.TempDir("", "bosh-init-cli-integration")
+		testHome, err = ioutil.TempDir("", "bosh-init-cli-integration")
 		Expect(err).NotTo(HaveOccurred())
 
-		err = os.Setenv("HOME", homePath)
+		err = os.Setenv("HOME", testHome)
 		Expect(err).NotTo(HaveOccurred())
 	})
 
 	AfterEach(func() {
 		err := os.Setenv("HOME", originalHome)
 		Expect(err).NotTo(HaveOccurred())
 
-		err = os.RemoveAll(homePath)
+		err = os.RemoveAll(testHome)
 		Expect(err).NotTo(HaveOccurred())
 	})
 

integration/upload_release_test.go

@@ -181,4 +181,125 @@ blobstore:
 			Expect(fs.ReadFileString(filepath.Join(pkg1.ExtractedPath(), "in-src"))).To(Equal("in-src"))
 		}
 	})
+
+	It("can upload release tarball", func() {
+		boshTmpDir := filepath.Join(testHome, ".bosh", "tmp")
+
+		tmpDir, err := fs.TempDir("bosh-upload-release-int-test")
+		Expect(err).ToNot(HaveOccurred())
+
+		defer fs.RemoveAll(tmpDir)
+
+		{
+			execCmd([]string{"init-release", "--dir", tmpDir})
+			execCmd([]string{"generate-job", "job1", "--dir", tmpDir})
+			execCmd([]string{"generate-package", "pkg1", "--dir", tmpDir})
+		}
+
+		{ // job1 depends on both packages
+			jobSpecPath := filepath.Join(tmpDir, "jobs", "job1", "spec")
+
+			contents, err := fs.ReadFileString(jobSpecPath)
+			Expect(err).ToNot(HaveOccurred())
+
+			err = fs.WriteFileString(jobSpecPath, strings.Replace(contents, "packages: []", "packages: [pkg1]", -1))
+			Expect(err).ToNot(HaveOccurred())
+		}
+
+		{ // Add a bit of content
+			err := fs.WriteFileString(filepath.Join(tmpDir, "src", "in-src"), "in-src")
+			Expect(err).ToNot(HaveOccurred())
+
+			pkg1SpecPath := filepath.Join(tmpDir, "packages", "pkg1", "spec")
+
+			contents, err := fs.ReadFileString(pkg1SpecPath)
+			Expect(err).ToNot(HaveOccurred())
+
+			err = fs.WriteFileString(pkg1SpecPath, strings.Replace(contents, "files: []", "files:\n- in-src", -1))
+			Expect(err).ToNot(HaveOccurred())
+		}
+
+		releaseTarballFile := filepath.Join(tmpDir, "release-tarball.tgz")
+
+		{ // Create dev release
+			execCmd([]string{"create-release", "--dir", tmpDir, "--tarball", releaseTarballFile})
+		}
+
+		{ // Starting with empty tmp directory
+			matches, err := fs.RecursiveGlob(filepath.Join(boshTmpDir, "*"))
+			Expect(err).ToNot(HaveOccurred())
+			Expect(matches).ToNot(BeEmpty())
+
+			// create-release leaves dev artifacts, so clean up before upload
+			fs.RemoveAll(boshTmpDir)
+		}
+
+		uploadedReleaseFile := filepath.Join(tmpDir, "release-3.tgz")
+
+		{
+			directorCACert, director := BuildHTTPSServer()
+			defer director.Close()
+
+			director.AppendHandlers(
+				ghttp.CombineHandlers(
+					ghttp.VerifyRequest("GET", "/info"),
+					ghttp.RespondWith(http.StatusOK, `{"user_authentication":{"type":"basic","options":{}}}`),
+				),
+				ghttp.CombineHandlers(
+					ghttp.VerifyRequest("POST", "/packages/matches"),
+					ghttp.RespondWith(http.StatusOK, "[]"),
+				),
+				ghttp.CombineHandlers(
+					ghttp.VerifyRequest("POST", "/releases"),
+					ghttp.RespondWith(http.StatusOK, `{"id":123, "state":"done"}`),
+					func(w http.ResponseWriter, req *http.Request) {
+						defer req.Body.Close()
+
+						body, err := ioutil.ReadAll(req.Body)
+						Expect(err).ToNot(HaveOccurred())
+
+						err = fs.WriteFile(uploadedReleaseFile, body)
+						Expect(err).ToNot(HaveOccurred())
+					},
+				),
+				ghttp.CombineHandlers(
+					ghttp.VerifyRequest("GET", "/tasks/123"),
+					ghttp.RespondWith(http.StatusOK, `{"id":123, "state":"done"}`),
+				),
+				ghttp.CombineHandlers(
+					ghttp.VerifyRequest("GET", "/tasks/123/output", "type=event"),
+					ghttp.RespondWith(http.StatusOK, ``),
+				),
+				ghttp.CombineHandlers(
+					ghttp.VerifyRequest("GET", "/tasks/123/output", "type=result"),
+					ghttp.RespondWith(http.StatusOK, ``),
+				),
+			)
+
+			execCmd([]string{"upload-release", releaseTarballFile, "-e", director.URL(), "--ca-cert", directorCACert})
+		}
+
+		{ // Check contents of uploaded release
+			relProvider := boshrel.NewProvider(deps.CmdRunner, deps.Compressor, deps.DigestCalculator, deps.FS, deps.Logger)
+			archiveReader := relProvider.NewExtractingArchiveReader()
+
+			release, err := archiveReader.Read(uploadedReleaseFile)
+			Expect(err).ToNot(HaveOccurred())
+
+			defer release.CleanUp()
+
+			pkg1 := release.Packages()[0]
+			Expect(fs.ReadFileString(filepath.Join(pkg1.ExtractedPath(), "in-src"))).To(Equal("in-src"))
+
+			release.CleanUp()
+		}
+
+		fs.RemoveAll(tmpDir)
+
+		{ // Expect empty tmp directory to make sure we are not leaking any files
+			matches, err := fs.RecursiveGlob(filepath.Join(boshTmpDir, "*"))
+			Expect(err).ToNot(HaveOccurred())
+			Expect(matches).To(BeEmpty())
+		}
+	})
 })