veyron/services/mgmt/node/impl: chown the log directory
Modify the suidhelper to give ownership of the entire log directory to
the application so that apps can create additional log files even when
running under different user names.
Change-Id: I62818bba4ab8bbbf2e85615037c1c722afc20386
diff --git a/services/mgmt/node/impl/app_invoker.go b/services/mgmt/node/impl/app_invoker.go
index 5651b7b..36556d2 100644
--- a/services/mgmt/node/impl/app_invoker.go
+++ b/services/mgmt/node/impl/app_invoker.go
@@ -476,18 +476,17 @@
if err := mkdir(logDir); err != nil {
return nil, err
}
+ cmd.Args = append(cmd.Args, "--logdir", logDir)
timestamp := time.Now().UnixNano()
+
stdoutLog := filepath.Join(logDir, fmt.Sprintf("STDOUT-%d", timestamp))
if cmd.Stdout, err = openWriteFile(stdoutLog); err != nil {
return nil, err
}
- cmd.Args = append(cmd.Args, "--stdoutlog", stdoutLog)
-
stderrLog := filepath.Join(logDir, fmt.Sprintf("STDERR-%d", timestamp))
if cmd.Stderr, err = openWriteFile(stderrLog); err != nil {
return nil, err
}
- cmd.Args = append(cmd.Args, "--stderrlog", stderrLog)
cmd.Args = append(cmd.Args, "--run", binPath)
cmd.Args = append(cmd.Args, "--")
diff --git a/services/mgmt/suidhelper/impl/args.go b/services/mgmt/suidhelper/impl/args.go
index 2cc26ae..7ab9574 100644
--- a/services/mgmt/suidhelper/impl/args.go
+++ b/services/mgmt/suidhelper/impl/args.go
@@ -16,8 +16,7 @@
uid int
gid int
workspace string
- stderrLog string
- stdoutLog string
+ logDir string
argv0 string
argv []string
envv []string
@@ -26,17 +25,16 @@
}
type ArgsSavedForTest struct {
- Uname string
- Workpace string
- Run string
- StdoutLog string
- StderrLog string
+ Uname string
+ Workpace string
+ Run string
+ LogDir string
}
const SavedArgs = "VEYRON_SAVED_ARGS"
var (
- flagUsername, flagWorkspace, flagStdoutLog, flagStderrLog, flagRun *string
+ flagUsername, flagWorkspace, flagLogDir, flagRun *string
flagMinimumUid *int64
flagRemove *bool
)
@@ -51,8 +49,7 @@
}
flagUsername = sflag.Username
flagWorkspace = sflag.Workspace
- flagStdoutLog = sflag.StdoutLog
- flagStderrLog = sflag.StderrLog
+ flagLogDir = sflag.LogDir
flagRun = sflag.Run
flagMinimumUid = sflag.MinimumUid
flagRemove = sflag.Remove
@@ -97,11 +94,10 @@
b := new(bytes.Buffer)
enc := json.NewEncoder(b)
enc.Encode(ArgsSavedForTest{
- Uname: *flagUsername,
- Workpace: *flagWorkspace,
- Run: *flagRun,
- StdoutLog: *flagStdoutLog,
- StderrLog: *flagStderrLog,
+ Uname: *flagUsername,
+ Workpace: *flagWorkspace,
+ Run: *flagRun,
+ LogDir: *flagLogDir,
})
env = append(env, SavedArgs+"="+b.String())
wp.dryrun = true
@@ -111,8 +107,7 @@
wp.gid = int(gid)
wp.workspace = *flagWorkspace
wp.argv0 = *flagRun
- wp.stdoutLog = *flagStdoutLog
- wp.stderrLog = *flagStderrLog
+ wp.logDir = *flagLogDir
wp.argv = append([]string{wp.argv0}, fs.Args()...)
// TODO(rjkroege): Reduce the environment to the absolute minimum needed.
wp.envv = env
diff --git a/services/mgmt/suidhelper/impl/args_test.go b/services/mgmt/suidhelper/impl/args_test.go
index a9f0cde..9358e9f 100644
--- a/services/mgmt/suidhelper/impl/args_test.go
+++ b/services/mgmt/suidhelper/impl/args_test.go
@@ -30,8 +30,7 @@
uid: testUid,
gid: testGid,
workspace: "",
- stderrLog: "",
- stdoutLog: "",
+ logDir: "",
argv0: "",
argv: []string{""},
envv: []string{"A=B"},
@@ -42,15 +41,14 @@
{
[]string{"setuidhelper", "--minuid", "1", "--username", testUserName, "--workspace", "/hello",
- "--stdoutlog", "/stdout", "--stderrlog", "/stderr", "--run", "/bin/veyron", "--", "one", "two"},
+ "--logdir", "/logging", "--run", "/bin/veyron", "--", "one", "two"},
[]string{"A=B"},
nil,
WorkParameters{
uid: testUid,
gid: testGid,
workspace: "/hello",
- stderrLog: "/stderr",
- stdoutLog: "/stdout",
+ logDir: "/logging",
argv0: "/bin/veyron",
argv: []string{"/bin/veyron", "one", "two"},
envv: []string{"A=B"},
@@ -73,8 +71,7 @@
uid: 0,
gid: 0,
workspace: "",
- stderrLog: "",
- stdoutLog: "",
+ logDir: "",
argv0: "",
argv: []string{"hello", "vanadium"},
envv: nil,
diff --git a/services/mgmt/suidhelper/impl/flag/flag.go b/services/mgmt/suidhelper/impl/flag/flag.go
index 674227e..6a4cdb3 100644
--- a/services/mgmt/suidhelper/impl/flag/flag.go
+++ b/services/mgmt/suidhelper/impl/flag/flag.go
@@ -13,7 +13,7 @@
import "flag"
var (
- Username, Workspace, StdoutLog, StderrLog, Run *string
+ Username, Workspace, LogDir, Run *string
MinimumUid *int64
Remove *bool
)
@@ -25,8 +25,7 @@
func SetupFlags(fs *flag.FlagSet) {
Username = fs.String("username", "", "The UNIX user name used for the other functions of this tool.")
Workspace = fs.String("workspace", "", "Path to the application's workspace directory.")
- StdoutLog = fs.String("stdoutlog", "", "Path to the stdout log file.")
- StderrLog = fs.String("stderrlog", "", "Path to the stdin log file.")
+ LogDir = fs.String("logdir", "", "Path to the log directory.")
Run = fs.String("run", "", "Path to the application to exec.")
MinimumUid = fs.Int64("minuid", uidThreshold, "UIDs cannot be less than this number.")
Remove = fs.Bool("rm", false, "Remove the file trees given as command-line arguments.")
diff --git a/services/mgmt/suidhelper/impl/system.go b/services/mgmt/suidhelper/impl/system.go
index 9df0f9b..2032f60 100644
--- a/services/mgmt/suidhelper/impl/system.go
+++ b/services/mgmt/suidhelper/impl/system.go
@@ -6,13 +6,17 @@
"fmt"
"log"
"os"
+ "path/filepath"
"syscall"
)
// Chown is only availabe on UNIX platforms so this file has a build
// restriction.
func (hw *WorkParameters) Chown() error {
- chown := func(path string) error {
+ chown := func(path string, _ os.FileInfo, inerr error) error {
+ if inerr != nil {
+ return inerr
+ }
if hw.dryrun {
log.Printf("[dryrun] os.Chown(%s, %d, %d)", path, hw.uid, hw.gid)
return nil
@@ -20,9 +24,9 @@
return os.Chown(path, hw.uid, hw.gid)
}
- for _, p := range []string{hw.workspace, hw.stdoutLog, hw.stderrLog} {
+ for _, p := range []string{hw.workspace, hw.logDir} {
// TODO(rjkroege): Ensure that the nodemanager can read log entries.
- if err := chown(p); err != nil {
+ if err := filepath.Walk(p, chown); err != nil {
return fmt.Errorf("os.Chown(%s, %d, %d) failed: %v", p, hw.uid, hw.gid, err)
}
}
diff --git a/services/mgmt/suidhelper/impl/system_test.go b/services/mgmt/suidhelper/impl/system_test.go
new file mode 100644
index 0000000..d8040f2
--- /dev/null
+++ b/services/mgmt/suidhelper/impl/system_test.go
@@ -0,0 +1,70 @@
+package impl
+
+import (
+ "bytes"
+ "io/ioutil"
+ "log"
+ "os"
+ "path"
+ "testing"
+)
+
+func TestChown(t *testing.T) {
+ dir, err := ioutil.TempDir("", "chown_test")
+ if err != nil {
+ t.Fatalf("ioutil.TempDir() failed: %v", err)
+ }
+ defer os.RemoveAll(dir)
+
+ for _, p := range []string{"a/b/c", "c/d"} {
+ fp := path.Join(dir, p)
+ if err := os.MkdirAll(fp, os.FileMode(0700)); err != nil {
+ t.Fatalf("os.MkdirAll(%s) failed: %v", fp, err)
+ }
+ }
+
+ wp := &WorkParameters{
+ uid: 42,
+ gid: 7,
+ logDir: path.Join(dir, "a"),
+ workspace: path.Join(dir, "c"),
+
+ dryrun: true,
+ }
+
+ // Collect the log entries.
+ b := new(bytes.Buffer)
+ log.SetOutput(b)
+ log.SetFlags(0)
+ defer log.SetOutput(os.Stderr)
+ defer log.SetFlags(log.LstdFlags)
+
+ // Mock-chown the tree.
+ if err := wp.Chown(); err != nil {
+ t.Fatalf("wp.Chown() wrongly failed: %v", err)
+ }
+
+ // Edit the log buffer to remove the invocation dependent output.
+ pb := bytes.TrimSpace(bytes.Replace(b.Bytes(), []byte(dir), []byte("$PATH"), -1))
+
+ cmds := bytes.Split(pb, []byte{'\n'})
+ for i, _ := range cmds {
+ cmds[i] = bytes.TrimSpace(cmds[i])
+ }
+
+ expected := []string{
+ "[dryrun] os.Chown($PATH/c, 42, 7)",
+ "[dryrun] os.Chown($PATH/c/d, 42, 7)",
+ "[dryrun] os.Chown($PATH/a, 42, 7)",
+ "[dryrun] os.Chown($PATH/a/b, 42, 7)",
+ "[dryrun] os.Chown($PATH/a/b/c, 42, 7)",
+ }
+ if got, expected := len(cmds), len(expected); got != expected {
+ t.Fatalf("bad length. got: %d, expected %d", got, expected)
+ }
+ for i, _ := range expected {
+ if expected, got := expected[i], string(cmds[i]); expected != got {
+ t.Fatalf("wp.Chown output %d: got %v, expected %v", i, got, expected)
+ }
+ }
+}
