Fix issue with non-writable sub-directories

There are file system setups where there is a file in a directory, but
the directory does not have the write bit set (anymore). This kind of
setup is perfect fine, however, the bundle logic is unable to unpack it.

Fix unpack logic so that it is not setting the file permissions of
directories in the moment it creates the directory, but later when all
other files are unpacked already and no more write operations are
required.

Signed-off-by: Matthias Diester <[email protected]>

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() {