Merge pull request #1699 from shipwright-io/fix/permission

Fix issue with non-writable sub-directories

pkg/bundle/bundle.go

@@ -231,12 +231,34 @@ func Pack(directory string) (io.ReadCloser, error) {
 // Unpack reads a tar stream and writes the content into the local file system
 // with all files and directories.
 func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
+	type chmod struct {
+		name string
+		mode os.FileMode
+	}
+
+	// Make sure the target path exists and is a directory
+	if stat, err := os.Stat(targetPath); err != nil {
+		if err := os.MkdirAll(targetPath, os.FileMode(0755)); err != nil {
+			return nil, err
+		}
+	} else if !stat.IsDir() {
+		return nil, fmt.Errorf("target %q exists, but it's not a directory", targetPath)
+	}
+
+	var chmods []chmod
 	var details = UnpackDetails{}
 	var tr = tar.NewReader(in)
 	for {
 		header, err := tr.Next()
 		switch {
 		case err == io.EOF:
+			// before leaving, make sure to set the file permissions to the ones specified in the tar stream
+			for _, chmod := range chmods {
+				if err := os.Chmod(chmod.name, chmod.mode); err != nil {
+					return nil, err
+				}
+			}
+
 			return &details, nil
 
 		case err != nil:
@@ -253,10 +275,17 @@ func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
 
 		switch header.Typeflag {
 		case tar.TypeDir:
-			if err := os.MkdirAll(target, fileMode(header)); err != nil {
+			// Skip the root directory, since it already exists
+			if target == targetPath {
+				continue
+			}
+
+			if err := os.MkdirAll(target, os.FileMode(0777)); err != nil {
 				return nil, err
 			}
 
+			chmods = append(chmods, chmod{name: target, mode: fileMode(header)})
+
 		case tar.TypeReg:
 			// Edge case in which that tarball did not have a directory entry
 			dir, _ := filepath.Split(target)

pkg/bundle/bundle_test.go

@@ -55,15 +55,36 @@ var _ = Describe("Bundle", func() {
 				Expect(r).ToNot(BeNil())
 
 				details, err := Unpack(r, tempDir)
-				Expect(details).ToNot(BeNil())
 				Expect(err).ToNot(HaveOccurred())
+				Expect(details).ToNot(BeNil())
 
 				Expect(filepath.Join(tempDir, "README.md")).To(BeAnExistingFile())
 				Expect(filepath.Join(tempDir, ".someToolDir", "config.json")).ToNot(BeAnExistingFile())
 				Expect(filepath.Join(tempDir, "somefile")).To(BeAnExistingFile())
 				Expect(filepath.Join(tempDir, "linktofile")).To(BeAnExistingFile())
 			})
 		})
+
+		It("should pack and unpack a directory including all files even if a sub-directory is non-writable", func() {
+			withTempDir(func(source string) {
+				// Setup a case where there are files in a directory, which is non-writable
+				Expect(os.Mkdir(filepath.Join(source, "some-dir"), os.FileMode(0777))).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(source, "some-dir", "some-file"), []byte(`foobar`), os.FileMode(0644))).To(Succeed())
+				Expect(os.Chmod(filepath.Join(source, "some-dir"), os.FileMode(0555))).To(Succeed())
+
+				r, err := Pack(source)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(r).ToNot(BeNil())
+
+				withTempDir(func(target string) {
+					details, err := Unpack(r, target)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(details).ToNot(BeNil())
+
+					Expect(filepath.Join(target, "some-dir", "some-file")).To(BeAnExistingFile())
+				})
+			})
+		})
 	})
 
 	Context("packing/pushing and pulling/unpacking", func() {