Merge pull request #1678 from hashicorp/revert-1671-f-secret-dir2

Revert "Introduce a Secret/ directory"

Author: dadgar

client/alloc_runner.go

@@ -12,7 +12,6 @@ import (
 	"github.com/hashicorp/nomad/client/allocdir"
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/driver"
-	"github.com/hashicorp/nomad/client/secretdir"
 	"github.com/hashicorp/nomad/nomad/structs"
 
 	cstructs "github.com/hashicorp/nomad/client/structs"
@@ -52,20 +51,17 @@ type AllocRunner struct {
 
 	dirtyCh chan struct{}
 
-	ctx     *driver.ExecContext
-	ctxLock sync.Mutex
+	ctx        *driver.ExecContext
+	ctxLock    sync.Mutex
+	tasks      map[string]*TaskRunner
+	taskStates map[string]*structs.TaskState
+	restored   map[string]struct{}
+	taskLock   sync.RWMutex
 
-	tasks    map[string]*TaskRunner
-	restored map[string]struct{}
-	taskLock sync.RWMutex
-
-	taskStates     map[string]*structs.TaskState
 	taskStatusLock sync.RWMutex
 
 	updateCh chan *structs.Allocation
 
-	secretDir secretdir.SecretDirectory
-
 	destroy     bool
 	destroyCh   chan struct{}
 	destroyLock sync.Mutex
@@ -84,13 +80,12 @@ type allocRunnerState struct {
 
 // NewAllocRunner is used to create a new allocation context
 func NewAllocRunner(logger *log.Logger, config *config.Config, updater AllocStateUpdater,
-	alloc *structs.Allocation, secretDir secretdir.SecretDirectory) *AllocRunner {
+	alloc *structs.Allocation) *AllocRunner {
 	ar := &AllocRunner{
 		config:     config,
 		updater:    updater,
 		logger:     logger,
 		alloc:      alloc,
-		secretDir:  secretDir,
 		dirtyCh:    make(chan struct{}, 1),
 		tasks:      make(map[string]*TaskRunner),
 		taskStates: copyTaskStates(alloc.TaskStates),
@@ -131,8 +126,6 @@ func (r *AllocRunner) RestoreState() error {
 	}
 	if r.ctx == nil {
 		snapshotErrors.Errors = append(snapshotErrors.Errors, fmt.Errorf("alloc_runner snapshot includes a nil context"))
-	} else {
-		r.ctx.AllocDir.SetSecretDirFn(r.secretDir.CreateFor)
 	}
 	if e := snapshotErrors.ErrorOrNil(); e != nil {
 		return e
@@ -393,11 +386,9 @@ func (r *AllocRunner) Run() {
 	// Create the execution context
 	r.ctxLock.Lock()
 	if r.ctx == nil {
-		path := filepath.Join(r.config.AllocDir, r.alloc.ID)
-		size := r.Alloc().Resources.DiskMB
-		allocDir := allocdir.NewAllocDir(r.alloc.ID, path, size, r.secretDir.CreateFor)
+		allocDir := allocdir.NewAllocDir(filepath.Join(r.config.AllocDir, r.alloc.ID), r.Alloc().Resources.DiskMB)
 		if err := allocDir.Build(tg.Tasks); err != nil {
-			r.logger.Printf("[ERR] client: failed to build task directories: %v", err)
+			r.logger.Printf("[WARN] client: failed to build task directories: %v", err)
 			r.setStatus(structs.AllocClientStatusFailed, fmt.Sprintf("failed to build task dirs for '%s'", alloc.TaskGroup))
 			r.ctxLock.Unlock()
 			return

client/alloc_runner_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/hashicorp/nomad/testutil"
 
 	"github.com/hashicorp/nomad/client/config"
-	"github.com/hashicorp/nomad/client/secretdir"
 	ctestutil "github.com/hashicorp/nomad/client/testutil"
 )
 
@@ -26,7 +25,7 @@ func (m *MockAllocStateUpdater) Update(alloc *structs.Allocation) {
 	m.Allocs = append(m.Allocs, alloc)
 }
 
-func testAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) {
+func testAllocRunnerFromAlloc(alloc *structs.Allocation, restarts bool) (*MockAllocStateUpdater, *AllocRunner) {
 	logger := testLogger()
 	conf := config.DefaultConfig()
 	conf.StateDir = os.TempDir()
@@ -36,17 +35,17 @@ func testAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts
 		*alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0}
 		alloc.Job.Type = structs.JobTypeBatch
 	}
-	ar := NewAllocRunner(logger, conf, upd.Update, alloc, secretdir.NewTestSecretDir(t))
+	ar := NewAllocRunner(logger, conf, upd.Update, alloc)
 	return upd, ar
 }
 
-func testAllocRunner(t *testing.T, restarts bool) (*MockAllocStateUpdater, *AllocRunner) {
-	return testAllocRunnerFromAlloc(t, mock.Alloc(), restarts)
+func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) {
+	return testAllocRunnerFromAlloc(mock.Alloc(), restarts)
 }
 
 func TestAllocRunner_SimpleRun(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 	go ar.Run()
 	defer ar.Destroy()
 
@@ -83,7 +82,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) {
 	}
 
 	alloc.Job.TaskGroups[0].Tasks = append(alloc.Job.TaskGroups[0].Tasks, badtask)
-	upd, ar := testAllocRunnerFromAlloc(t, alloc, true)
+	upd, ar := testAllocRunnerFromAlloc(alloc, true)
 	go ar.Run()
 	defer ar.Destroy()
 
@@ -119,7 +118,7 @@ func TestAllocRunner_RetryArtifact(t *testing.T) {
 
 func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 
 	// Ensure task takes some time
 	task := ar.alloc.Job.TaskGroups[0].Tasks[0]
@@ -208,7 +207,7 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) {
 
 func TestAllocRunner_DiskExceeded_Destroy(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 
 	// Ensure task takes some time
 	task := ar.alloc.Job.TaskGroups[0].Tasks[0]
@@ -314,7 +313,7 @@ func TestAllocRunner_DiskExceeded_Destroy(t *testing.T) {
 }
 func TestAllocRunner_Destroy(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 
 	// Ensure task takes some time
 	task := ar.alloc.Job.TaskGroups[0].Tasks[0]
@@ -366,7 +365,7 @@ func TestAllocRunner_Destroy(t *testing.T) {
 
 func TestAllocRunner_Update(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	_, ar := testAllocRunner(t, false)
+	_, ar := testAllocRunner(false)
 
 	// Ensure task takes some time
 	task := ar.alloc.Job.TaskGroups[0].Tasks[0]
@@ -392,7 +391,7 @@ func TestAllocRunner_Update(t *testing.T) {
 
 func TestAllocRunner_SaveRestoreState(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 
 	// Ensure task takes some time
 	task := ar.alloc.Job.TaskGroups[0].Tasks[0]
@@ -414,7 +413,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) {
 
 	// Create a new alloc runner
 	ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update,
-		&structs.Allocation{ID: ar.alloc.ID}, ar.secretDir)
+		&structs.Allocation{ID: ar.alloc.ID})
 	err = ar2.RestoreState()
 	if err != nil {
 		t.Fatalf("err: %v", err)
@@ -442,7 +441,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) {
 
 func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 	ar.logger = prefixedTestLogger("ar1: ")
 
 	// Ensure task takes some time
@@ -486,7 +485,7 @@ func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) {
 
 	// Create a new alloc runner
 	ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update,
-		&structs.Allocation{ID: ar.alloc.ID}, ar.secretDir)
+		&structs.Allocation{ID: ar.alloc.ID})
 	ar2.logger = prefixedTestLogger("ar2: ")
 	err = ar2.RestoreState()
 	if err != nil {
@@ -548,7 +547,7 @@ func TestAllocRunner_SaveRestoreState_TerminalAlloc(t *testing.T) {
 
 func TestAllocRunner_TaskFailed_KillTG(t *testing.T) {
 	ctestutil.ExecCompatible(t)
-	upd, ar := testAllocRunner(t, false)
+	upd, ar := testAllocRunner(false)
 
 	// Create two tasks in the task group
 	task := ar.alloc.Job.TaskGroups[0].Tasks[0]

client/allocdir/alloc_dir.go

@@ -46,24 +46,11 @@ var (
 	// regardless of driver.
 	TaskLocal = "local"
 
-	// TaskSecrets is the the name of the secret directory inside each task
-	// directory
-	TaskSecrets = "secrets"
-
 	// TaskDirs is the set of directories created in each tasks directory.
 	TaskDirs = []string{"tmp"}
 )
 
-type CreateSecretDirFn func(allocID, task string) (path string, err error)
-
 type AllocDir struct {
-	// AllocID is the allocation ID for this directory
-	AllocID string
-
-	// createSecretDirFn is used to create a secret directory and retrieve the
-	// path to it so that it can be mounted in the task directory.
-	createSecretDirFn CreateSecretDirFn
-
 	// AllocDir is the directory used for storing any state
 	// of this allocation. It will be purged on alloc destroy.
 	AllocDir string
@@ -122,30 +109,20 @@ type AllocDirFS interface {
 }
 
 // NewAllocDir initializes the AllocDir struct with allocDir as base path for
-// the allocation directory and maxSize as the maximum allowed size in
-// megabytes. The secretDirFn is used to create secret directories and get their
-// path which will then be mounted into the task directory
-func NewAllocDir(allocID, allocDir string, maxSize int, secretDirFn CreateSecretDirFn) *AllocDir {
+// the allocation directory and maxSize as the maximum allowed size in megabytes.
+func NewAllocDir(allocDir string, maxSize int) *AllocDir {
 	d := &AllocDir{
-		AllocID:                   allocID,
 		AllocDir:                  allocDir,
 		MaxCheckDiskInterval:      maxCheckDiskInterval,
 		MinCheckDiskInterval:      minCheckDiskInterval,
 		CheckDiskMaxEnforcePeriod: checkDiskMaxEnforcePeriod,
 		TaskDirs:                  make(map[string]string),
 		MaxSize:                   maxSize,
-		createSecretDirFn:         secretDirFn,
 	}
 	d.SharedDir = filepath.Join(d.AllocDir, SharedAllocName)
 	return d
 }
 
-// SetSecretDirFn is used to set the function used to create secret
-// directories.
-func (d *AllocDir) SetSecretDirFn(fn CreateSecretDirFn) {
-	d.createSecretDirFn = fn
-}
-
 // Tears down previously build directory structure.
 func (d *AllocDir) Destroy() error {
 
@@ -168,7 +145,7 @@ func (d *AllocDir) UnmountAll() error {
 		// Check if the directory has the shared alloc mounted.
 		taskAlloc := filepath.Join(dir, SharedAllocName)
 		if d.pathExists(taskAlloc) {
-			if err := d.unmount(taskAlloc); err != nil {
+			if err := d.unmountSharedDir(taskAlloc); err != nil {
 				mErr.Errors = append(mErr.Errors,
 					fmt.Errorf("failed to unmount shared alloc dir %q: %v", taskAlloc, err))
 			} else if err := os.RemoveAll(taskAlloc); err != nil {
@@ -177,18 +154,6 @@ func (d *AllocDir) UnmountAll() error {
 			}
 		}
 
-		// Remove the secrets dir
-		taskSecrets := filepath.Join(dir, TaskSecrets)
-		if d.pathExists(taskSecrets) {
-			if err := d.unmount(taskSecrets); err != nil {
-				mErr.Errors = append(mErr.Errors,
-					fmt.Errorf("failed to unmount secrets dir %q: %v", taskSecrets, err))
-			} else if err := os.RemoveAll(taskSecrets); err != nil {
-				mErr.Errors = append(mErr.Errors,
-					fmt.Errorf("failed to delete secrets dir %q: %v", taskSecrets, err))
-			}
-		}
-
 		// Unmount dev/ and proc/ have been mounted.
 		d.unmountSpecialDirs(dir)
 	}
@@ -258,24 +223,6 @@ func (d *AllocDir) Build(tasks []*structs.Task) error {
 				return err
 			}
 		}
-
-		// Get the secret directory
-		if d.createSecretDirFn != nil {
-			sdir, err := d.createSecretDirFn(d.AllocID, t.Name)
-			if err != nil {
-				return fmt.Errorf("Creating secret directory for task %q failed: %v", t.Name, err)
-			}
-
-			// Mount the secret directory
-			taskSecret := filepath.Join(taskDir, TaskSecrets)
-			if err := d.mount(sdir, taskSecret); err != nil {
-				return fmt.Errorf("failed to mount secret directory for task %q: %v", t.Name, err)
-			}
-
-			if err := d.dropDirPermissions(taskSecret); err != nil {
-				return err
-			}
-		}
 	}
 
 	return nil
@@ -385,7 +332,7 @@ func (d *AllocDir) MountSharedDir(task string) error {
 	}
 
 	taskLoc := filepath.Join(taskDir, SharedAllocName)
-	if err := d.mount(d.SharedDir, taskLoc); err != nil {
+	if err := d.mountSharedDir(taskLoc); err != nil {
 		return fmt.Errorf("Failed to mount shared directory for task %v: %v", task, err)
 	}
 

client/allocdir/alloc_dir_darwin.go

@@ -4,13 +4,13 @@ import (
 	"syscall"
 )
 
-// Hardlinks the shared directory. As a side-effect the src and dest directory
-// must be on the same filesystem.
-func (d *AllocDir) mount(src, dest string) error {
-	return syscall.Link(src, dest)
+// Hardlinks the shared directory. As a side-effect the shared directory and
+// task directory must be on the same filesystem.
+func (d *AllocDir) mountSharedDir(dir string) error {
+	return syscall.Link(d.SharedDir, dir)
 }
 
-func (d *AllocDir) unmount(dir string) error {
+func (d *AllocDir) unmountSharedDir(dir string) error {
 	return syscall.Unlink(dir)
 }
 

client/allocdir/alloc_dir_freebsd.go

@@ -4,13 +4,13 @@ import (
 	"syscall"
 )
 
-// Hardlinks the shared directory. As a side-effect the src and dest directory
-// must be on the same filesystem.
-func (d *AllocDir) mount(src, dest string) error {
-	return syscall.Link(src, dest)
+// Hardlinks the shared directory. As a side-effect the shared directory and
+// task directory must be on the same filesystem.
+func (d *AllocDir) mountSharedDir(dir string) error {
+	return syscall.Link(d.SharedDir, dir)
 }
 
-func (d *AllocDir) unmount(dir string) error {
+func (d *AllocDir) unmountSharedDir(dir string) error {
 	return syscall.Unlink(dir)
 }
 

client/allocdir/alloc_dir_linux.go

@@ -9,15 +9,17 @@ import (
 	"github.com/hashicorp/go-multierror"
 )
 
-func (d *AllocDir) mount(src, dest string) error {
-	if err := os.MkdirAll(dest, 0777); err != nil {
+// Bind mounts the shared directory into the task directory. Must be root to
+// run.
+func (d *AllocDir) mountSharedDir(taskDir string) error {
+	if err := os.MkdirAll(taskDir, 0777); err != nil {
 		return err
 	}
 
-	return syscall.Mount(src, dest, "", syscall.MS_BIND, "")
+	return syscall.Mount(d.SharedDir, taskDir, "", syscall.MS_BIND, "")
 }
 
-func (d *AllocDir) unmount(dir string) error {
+func (d *AllocDir) unmountSharedDir(dir string) error {
 	return syscall.Unmount(dir, 0)
 }