Merge "veyron2/vdl: New API for generated Go interfaces."
diff --git a/services/mgmt/node/impl/app_invoker.go b/services/mgmt/node/impl/app_invoker.go
index 247d7d0..6ff19f7 100644
--- a/services/mgmt/node/impl/app_invoker.go
+++ b/services/mgmt/node/impl/app_invoker.go
@@ -300,7 +300,7 @@
installationID := generateID()
installationDir := filepath.Join(i.config.Root, applicationDirName(envelope.Title), installationDirName(installationID))
deferrer := func() {
- cleanupDir(installationDir)
+ cleanupDir(installationDir, "")
}
defer func() {
if deferrer != nil {
@@ -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, "--")
@@ -559,25 +558,26 @@
}
func (i *appInvoker) Start(call ipc.ServerContext) ([]string, error) {
+ helper := i.config.Helper
instanceDir, instanceID, err := i.newInstance()
if err != nil {
- cleanupDir(instanceDir)
+ cleanupDir(instanceDir, helper)
return nil, err
}
- systemName, err := systemAccountForHelper(i.config.Helper, call.RemoteBlessings().ForContext(call), i.uat)
+ systemName, err := systemAccountForHelper(helper, call.RemoteBlessings().ForContext(call), i.uat)
if err != nil {
- cleanupDir(instanceDir)
+ cleanupDir(instanceDir, helper)
return nil, err
}
if err := saveSystemNameForInstance(instanceDir, systemName); err != nil {
- cleanupDir(instanceDir)
+ cleanupDir(instanceDir, helper)
return nil, err
}
if err = i.run(instanceDir, systemName); err != nil {
- cleanupDir(instanceDir)
+ cleanupDir(instanceDir, helper)
return nil, err
}
return []string{instanceID}, nil
@@ -743,7 +743,7 @@
}
versionDir, err := newVersion(installationDir, newEnvelope, oldVersionDir)
if err != nil {
- cleanupDir(versionDir)
+ cleanupDir(versionDir, "")
return err
}
return nil
diff --git a/services/mgmt/node/impl/impl_helper_test.go b/services/mgmt/node/impl/impl_helper_test.go
new file mode 100644
index 0000000..1eaaeda
--- /dev/null
+++ b/services/mgmt/node/impl/impl_helper_test.go
@@ -0,0 +1,45 @@
+package impl_test
+
+// Separate from impl_test to avoid contributing further to impl_test bloat.
+// TODO(rjkroege): Move all helper-related tests to here.
+
+import (
+ "io/ioutil"
+ "os"
+ "path"
+ "testing"
+
+ "veyron.io/veyron/veyron/services/mgmt/node/impl"
+)
+
+func TestBaseCleanupDir(t *testing.T) {
+ dir, err := ioutil.TempDir("", "impl_helper_test")
+ if err != nil {
+ t.Fatalf("ioutil.TempDir() failed: %v", err)
+ }
+ defer os.RemoveAll(dir)
+
+ // Setup some files to delete.
+ helperTarget := path.Join(dir, "helper_target")
+ if err := os.MkdirAll(helperTarget, os.FileMode(0700)); err != nil {
+ t.Fatalf("os.MkdirAll(%s) failed: %v", helperTarget, err)
+ }
+
+ nohelperTarget := path.Join(dir, "nohelper_target")
+ if err := os.MkdirAll(nohelperTarget, os.FileMode(0700)); err != nil {
+ t.Fatalf("os.MkdirAll(%s) failed: %v", nohelperTarget, err)
+ }
+
+ // Setup a helper.
+ helper := generateSuidHelperScript(t, dir)
+
+ impl.WrapBaseCleanupDir(helperTarget, helper)
+ if _, err := os.Stat(helperTarget); err == nil || os.IsExist(err) {
+ t.Fatalf("%s should be missing but isn't", helperTarget)
+ }
+
+ impl.WrapBaseCleanupDir(nohelperTarget, "")
+ if _, err := os.Stat(nohelperTarget); err == nil || os.IsExist(err) {
+ t.Fatalf("%s should be missing but isn't", nohelperTarget)
+ }
+}
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index f35721c..8f44059 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -276,30 +276,7 @@
return path
}
-// generateSuidHelperScript builds a script to execute the test target as
-// a suidhelper instance and installs it in <root>/helper.
-func generateSuidHelperScript(t *testing.T, root string) string {
- output := "#!/bin/bash\n"
- output += "VEYRON_SUIDHELPER_TEST=1"
- output += " "
- output += "exec" + " " + os.Args[0] + " " + "-minuid=1" + " " + "-test.run=TestSuidHelper $*"
- output += "\n"
-
- vlog.VI(1).Infof("script\n%s", output)
-
- if err := os.MkdirAll(root, 0755); err != nil {
- t.Fatalf("MkdirAll failed: %v", err)
- }
- // Helper does not need to live under the node manager's root dir, but
- // we put it there for convenience.
- path := filepath.Join(root, "helper.sh")
- if err := ioutil.WriteFile(path, []byte(output), 0755); err != nil {
- t.Fatalf("WriteFile(%v) failed: %v", path, err)
- }
- return path
-}
-
-// readPID waits for the "ready:<PID>" line from the child and parses out the
+/// readPID waits for the "ready:<PID>" line from the child and parses out the
// PID of the child.
func readPID(t *testing.T, s *expect.Session) int {
m := s.ExpectRE("ready:([0-9]+)", -1)
diff --git a/services/mgmt/node/impl/node_invoker.go b/services/mgmt/node/impl/node_invoker.go
index b2952f9..0e50fdd 100644
--- a/services/mgmt/node/impl/node_invoker.go
+++ b/services/mgmt/node/impl/node_invoker.go
@@ -339,7 +339,7 @@
}
deferrer := func() {
- cleanupDir(workspace)
+ cleanupDir(workspace, "")
}
defer func() {
if deferrer != nil {
diff --git a/services/mgmt/node/impl/only_for_test.go b/services/mgmt/node/impl/only_for_test.go
index 3261c42..b110699 100644
--- a/services/mgmt/node/impl/only_for_test.go
+++ b/services/mgmt/node/impl/only_for_test.go
@@ -24,9 +24,14 @@
}
func init() {
- cleanupDir = func(dir string) {
+ cleanupDir = func(dir, helper string) {
parentDir, base := filepath.Dir(dir), filepath.Base(dir)
- renamed := filepath.Join(parentDir, "deleted_"+base)
+ var renamed string
+ if helper != "" {
+ renamed = filepath.Join(parentDir, "helper_deleted_"+base)
+ } else {
+ renamed = filepath.Join(parentDir, "deleted_"+base)
+ }
if err := os.Rename(dir, renamed); err != nil {
vlog.Errorf("Rename(%v, %v) failed: %v", dir, renamed, err)
}
@@ -38,3 +43,7 @@
vlog.VI(2).Infof("Mock isSetuid is reporting: %v", *mockIsSetuid)
return *mockIsSetuid
}
+
+func WrapBaseCleanupDir(path, helper string) {
+ baseCleanupDir(path, helper)
+}
diff --git a/services/mgmt/node/impl/util.go b/services/mgmt/node/impl/util.go
index d8cca9d..29ca3da 100644
--- a/services/mgmt/node/impl/util.go
+++ b/services/mgmt/node/impl/util.go
@@ -3,6 +3,7 @@
import (
"io/ioutil"
"os"
+ "os/exec"
"path/filepath"
"time"
@@ -76,10 +77,24 @@
return nil
}
-// cleanupDir is defined like this so we can override its implementation for
-// tests.
-var cleanupDir = func(dir string) {
- if err := os.RemoveAll(dir); err != nil {
- vlog.Errorf("RemoveAll(%v) failed: %v", dir, err)
+func baseCleanupDir(path, helper string) {
+ if helper != "" {
+ out, err := exec.Command(helper, "--rm", path).CombinedOutput()
+ if err != nil {
+ vlog.Errorf("exec.Command(%s %s %s).CombinedOutput() failed: %v", helper, "--rm", path, err)
+ return
+ }
+ if len(out) != 0 {
+ vlog.Errorf("exec.Command(%s %s %s).CombinedOutput() generated output: %v", helper, "--rm", path, string(out))
+ }
+ } else {
+ if err := os.RemoveAll(path); err != nil {
+ vlog.Errorf("RemoveAll(%v) failed: %v", path, err)
+ }
}
}
+
+// cleanupDir is defined like this so we can override its implementation for
+// tests. cleanupDir will use the helper to delete application state possibly
+// owned by different accounts if helper is provided.
+var cleanupDir = baseCleanupDir
diff --git a/services/mgmt/node/impl/util_test.go b/services/mgmt/node/impl/util_test.go
index fce2764..00762a5 100644
--- a/services/mgmt/node/impl/util_test.go
+++ b/services/mgmt/node/impl/util_test.go
@@ -330,3 +330,26 @@
t.Fatalf("ListAssociations() got %v, expected %v", got, expected)
}
}
+
+// generateSuidHelperScript builds a script to execute the test target as
+// a suidhelper instance and returns the path to the script.
+func generateSuidHelperScript(t *testing.T, root string) string {
+ output := "#!/bin/bash\n"
+ output += "VEYRON_SUIDHELPER_TEST=1"
+ output += " "
+ output += "exec" + " " + os.Args[0] + " " + "-minuid=1" + " " + "-test.run=TestSuidHelper $*"
+ output += "\n"
+
+ vlog.VI(1).Infof("script\n%s", output)
+
+ if err := os.MkdirAll(root, 0755); err != nil {
+ t.Fatalf("MkdirAll failed: %v", err)
+ }
+ // Helper does not need to live under the node manager's root dir, but
+ // we put it there for convenience.
+ path := filepath.Join(root, "helper.sh")
+ if err := ioutil.WriteFile(path, []byte(output), 0755); err != nil {
+ t.Fatalf("WriteFile(%v) failed: %v", path, err)
+ }
+ return path
+}
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)
+ }
+ }
+}
