Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with non-writable sub-directories #1699

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down
23 changes: 22 additions & 1 deletion pkg/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down