Merge pull request #3723 from hashicorp/b-3702-chown-dirs

chown dirs when migrating ephemeral_disk data

CHANGELOG.md

@@ -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)]
 

client/alloc_watcher.go

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

client/alloc_watcher_test.go

@@ -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"
 )
 
@@ -73,6 +75,7 @@ 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 {
@@ -80,18 +83,29 @@ func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) {
 	}
 	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 {
@@ -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)
 	}
@@ -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 {