Skip to content

Commit

Permalink
Merge pull request #3723 from hashicorp/b-3702-chown-dirs
Browse files Browse the repository at this point in the history
chown dirs when migrating ephemeral_disk data
  • Loading branch information
schmichael committed Jan 9, 2018
2 parents 34d3bba + 58f7623 commit 7e7c799
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ BUG FIXES:
* core: Fix search endpoint forwarding for multi-region clusters [[GH-3680](https://github.com/hashicorp/nomad/issues/3680)]
* core: Fix an issue in which batch jobs with queued placements and lost
allocations could result in improper placement counts [[GH-3717](https://github.com/hashicorp/nomad/issues/3717)]
* client: Migrated ephemeral_disk's maintain directory permissions [[GH-3723](https://github.com/hashicorp/nomad/issues/3723)]
* config: Revert minimum CPU limit back to 20 from 100.
* ui: Fix requests using client-side certificates in Firefox. [[GH-3728](https://github.com/hashicorp/nomad/pull/3728)]

Expand Down
13 changes: 10 additions & 3 deletions client/alloc_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,15 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser

// If the header is for a directory we create the directory
if hdr.Typeflag == tar.TypeDir {
os.MkdirAll(filepath.Join(dest, hdr.Name), os.FileMode(hdr.Mode))
name := filepath.Join(dest, hdr.Name)
os.MkdirAll(name, os.FileMode(hdr.Mode))

// Can't change owner if not root or on Windows.
if euid == 0 {
if err := os.Chown(name, hdr.Uid, hdr.Gid); err != nil {
return fmt.Errorf("error chowning directory %v", err)
}
}
continue
}
// If the header is for a symlink we create the symlink
Expand All @@ -522,8 +530,7 @@ func (p *remotePrevAlloc) streamAllocDir(ctx context.Context, resp io.ReadCloser
return fmt.Errorf("error chmoding file %v", err)
}

// Can't change owner if not root. Returns false on
// Windows as Chown always errors there.
// Can't change owner if not root or on Windows.
if euid == 0 {
if err := f.Chown(hdr.Uid, hdr.Gid); err != nil {
f.Close()
Expand Down
29 changes: 25 additions & 4 deletions client/alloc_watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import (
"os"
"path/filepath"
"strings"
"syscall"
"testing"
"time"

"github.com/hashicorp/nomad/client/allocdir"
"github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/client/testutil"
"github.com/hashicorp/nomad/nomad/mock"
)

Expand Down Expand Up @@ -73,25 +75,37 @@ func TestPrevAlloc_LocalPrevAlloc(t *testing.T) {
// TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir
// works.
func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
testutil.RequireRoot(t)
t.Parallel()
dir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatalf("err: %v", err)
}
defer os.RemoveAll(dir)

if err := os.Mkdir(filepath.Join(dir, "foo"), 0777); err != nil {
// Create foo/
fooDir := filepath.Join(dir, "foo")
if err := os.Mkdir(fooDir, 0777); err != nil {
t.Fatalf("err: %v", err)
}
dirInfo, err := os.Stat(filepath.Join(dir, "foo"))

// Change ownership of foo/ to test #3702 (any non-root user is fine)
const uid, gid = 1, 1
if err := os.Chown(fooDir, uid, gid); err != nil {
t.Fatalf("err : %v", err)
}

dirInfo, err := os.Stat(fooDir)
if err != nil {
t.Fatalf("err: %v", err)
}
f, err := os.Create(filepath.Join(dir, "foo", "bar"))

// Create foo/bar
f, err := os.Create(filepath.Join(fooDir, "bar"))
if err != nil {
t.Fatalf("err: %v", err)
}
if _, err := f.WriteString("foo"); err != nil {
if _, err := f.WriteString("123"); err != nil {
t.Fatalf("err: %v", err)
}
if err := f.Chmod(0644); err != nil {
Expand All @@ -102,6 +116,8 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
t.Fatalf("err: %v", err)
}
f.Close()

// Create foo/baz -> bar symlink
if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil {
t.Fatalf("err: %v", err)
}
Expand Down Expand Up @@ -181,6 +197,11 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
if fi.Mode() != dirInfo.Mode() {
t.Fatalf("mode: %v", fi.Mode())
}
stat := fi.Sys().(*syscall.Stat_t)
if stat.Uid != uid || stat.Gid != gid {
t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d",
uid, gid, stat.Uid, stat.Gid)
}

fi1, err := os.Stat(filepath.Join(dir1, "bar"))
if err != nil {
Expand Down

0 comments on commit 7e7c799

Please sign in to comment.