From 68aedd3f8f8908b50d40a89f747da193bd3f42a8 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 8 Nov 2016 10:57:29 -0800 Subject: [PATCH 1/2] Fixed permission issues on client --- client/allocdir/alloc_dir.go | 66 +++++++++++++++++++++++++++---- client/allocdir/alloc_dir_test.go | 65 ++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index c230169bf765..3acc174e6db7 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -313,13 +313,12 @@ func (d *AllocDir) Embed(task string, entries map[string]string) error { // Embedding a single file if !s.IsDir() { - destDir := filepath.Join(taskdir, filepath.Dir(dest)) - if err := os.MkdirAll(destDir, s.Mode().Perm()); err != nil { - return fmt.Errorf("Couldn't create destination directory %v: %v", destDir, err) + if err := d.createDir(taskdir, filepath.Dir(dest)); err != nil { + return fmt.Errorf("Couldn't create destination directory 11 %v: %v", dest, err) } // Copy the file. - taskEntry := filepath.Join(destDir, filepath.Base(dest)) + taskEntry := filepath.Join(taskdir, dest) if err := d.linkOrCopy(source, taskEntry, s.Mode().Perm()); err != nil { return err } @@ -327,10 +326,10 @@ func (d *AllocDir) Embed(task string, entries map[string]string) error { continue } - // Create destination directory. destDir := filepath.Join(taskdir, dest) - if err := os.MkdirAll(destDir, s.Mode().Perm()); err != nil { - return fmt.Errorf("Couldn't create destination directory %v: %v", destDir, err) + // Create destination directory. + if err := d.createDir(taskdir, dest); err != nil { + return fmt.Errorf("Couldn't create destination directory 22 %v: %v", destDir, err) } // Enumerate the files in source. @@ -565,3 +564,56 @@ func (d *AllocDir) GetSecretDir(task string) (string, error) { return filepath.Join(t, TaskSecrets), nil } } + +// createDir creates a directory structure inside the basepath. This functions +// preserves the permissions of each of the subdirectories in the relative path +// by looking up the permissions in the host. +func (d *AllocDir) createDir(basePath, relPath string) error { + filePerms, err := d.splitPath(relPath) + if err != nil { + return err + } + for i := len(filePerms) - 1; i >= 0; i-- { + fi := filePerms[i] + destDir := filepath.Join(basePath, fi.Name) + if err := os.MkdirAll(destDir, fi.Perm); err != nil { + return err + } + } + return nil +} + +// fileInfo holds the path and the permissions of a file +type fileInfo struct { + Name string + Perm os.FileMode +} + +// splitPath stats each subdirectory of a path +func (d *AllocDir) splitPath(path string) ([]fileInfo, error) { + var mode os.FileMode + i, err := os.Stat(path) + if err != nil { + mode = os.ModePerm + } else { + mode = i.Mode() + } + var dirs []fileInfo + dirs = append(dirs, fileInfo{Name: path, Perm: mode}) + currentDir := path + for { + dir := filepath.Dir(filepath.Clean(currentDir)) + if dir == currentDir { + break + } + i, err = os.Stat(dir) + if err != nil { + mode = os.ModePerm + } else { + mode = i.Mode() + } + dirs = append(dirs, fileInfo{Name: dir, Perm: mode}) + currentDir = dir + } + return dirs, nil +} diff --git a/client/allocdir/alloc_dir_test.go b/client/allocdir/alloc_dir_test.go index 948253865cbe..daf38832e7e4 100644 --- a/client/allocdir/alloc_dir_test.go +++ b/client/allocdir/alloc_dir_test.go @@ -5,6 +5,7 @@ import ( "bytes" "io" "io/ioutil" + "log" "os" "path/filepath" "reflect" @@ -417,3 +418,67 @@ func TestAllocDir_ReadAt_SecretDir(t *testing.T) { t.Fatalf("ReadAt of secret file didn't error: %v", err) } } + +func TestAllocDir_SplitPath(t *testing.T) { + dir, err := ioutil.TempDir("/tmp", "tmpdirtest") + if err != nil { + log.Fatal(err) + } + defer os.RemoveAll(dir) + + dest := filepath.Join(dir, "/foo/bar/baz") + if err := os.MkdirAll(dest, os.ModePerm); err != nil { + t.Fatalf("err: %v", err) + } + + d := NewAllocDir(dir) + defer d.Destroy() + + info, err := d.splitPath(dest) + if err != nil { + t.Fatalf("err: %v", err) + } + if len(info) != 6 { + t.Fatalf("expected: %v, actual: %v", 6, len(info)) + } +} + +func TestAllocDir_CreateDir(t *testing.T) { + dir, err := ioutil.TempDir("/tmp", "tmpdirtest") + if err != nil { + t.Fatalf("err: %v", err) + } + defer os.RemoveAll(dir) + + // create a subdir and a file + subdir := filepath.Join(dir, "subdir") + if err := os.MkdirAll(subdir, 0760); err != nil { + t.Fatalf("err: %v", err) + } + subdirMode, err := os.Stat(subdir) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Create the above hierarchy under another destination + dir1, err := ioutil.TempDir("/tmp", "tempdirdest") + if err != nil { + t.Fatalf("err: %v", err) + } + + d := NewAllocDir(dir) + defer d.Destroy() + + if err := d.createDir(dir1, subdir); err != nil { + t.Fatalf("err: %v", err) + } + + // Ensure that the subdir had the right perm + fi, err := os.Stat(filepath.Join(dir1, dir, "subdir")) + if err != nil { + t.Fatalf("err: %v", err) + } + if fi.Mode() != subdirMode.Mode() { + t.Fatalf("wrong file mode: %v, expected: %v", fi.Mode(), subdirMode.Mode()) + } +} From 0beb8c0856600e97b609f3646ac61be49ee1adfe Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 8 Nov 2016 12:55:15 -0800 Subject: [PATCH 2/2] Fixed comments --- client/allocdir/alloc_dir.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 3acc174e6db7..1f0377714484 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -314,7 +314,7 @@ func (d *AllocDir) Embed(task string, entries map[string]string) error { // Embedding a single file if !s.IsDir() { if err := d.createDir(taskdir, filepath.Dir(dest)); err != nil { - return fmt.Errorf("Couldn't create destination directory 11 %v: %v", dest, err) + return fmt.Errorf("Couldn't create destination directory %v: %v", dest, err) } // Copy the file. @@ -326,10 +326,11 @@ func (d *AllocDir) Embed(task string, entries map[string]string) error { continue } - destDir := filepath.Join(taskdir, dest) // Create destination directory. + destDir := filepath.Join(taskdir, dest) + if err := d.createDir(taskdir, dest); err != nil { - return fmt.Errorf("Couldn't create destination directory 22 %v: %v", destDir, err) + return fmt.Errorf("Couldn't create destination directory %v: %v", destDir, err) } // Enumerate the files in source. @@ -573,6 +574,9 @@ func (d *AllocDir) createDir(basePath, relPath string) error { if err != nil { return err } + + // We are going backwards since we create the root of the directory first + // and then create the entire nested structure. for i := len(filePerms) - 1; i >= 0; i-- { fi := filePerms[i] destDir := filepath.Join(basePath, fi.Name) @@ -589,10 +593,15 @@ type fileInfo struct { Perm os.FileMode } -// splitPath stats each subdirectory of a path +// splitPath stats each subdirectory of a path. The first element of the array +// is the file passed to this method, and the last element is the root of the +// path. func (d *AllocDir) splitPath(path string) ([]fileInfo, error) { var mode os.FileMode i, err := os.Stat(path) + + // If the path is not present in the host then we respond with the most + // flexible permission. if err != nil { mode = os.ModePerm } else { @@ -606,6 +615,9 @@ func (d *AllocDir) splitPath(path string) ([]fileInfo, error) { if dir == currentDir { break } + + // We try to find the permission of the file in the host. If the path is not + // present in the host then we respond with the most flexible permission. i, err = os.Stat(dir) if err != nil { mode = os.ModePerm