Merge "vdl/javascript: Change the generator to use the new package layout."
diff --git a/cmd/debug/debug_v23_test.go b/cmd/debug/debug_v23_test.go
index a56051c..600eb58 100644
--- a/cmd/debug/debug_v23_test.go
+++ b/cmd/debug/debug_v23_test.go
@@ -18,7 +18,7 @@
func V23TestDebugGlob(i *v23tests.T) {
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
inv := binary.Start("glob", "__debug/*")
var want string
@@ -35,7 +35,7 @@
// Create a temp file before we list the logs.
fileName := filepath.Base(i.NewTempFile().Name())
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
output := binary.Start("glob", "__debug/logs/*").Output()
// The output should contain the filename.
@@ -49,7 +49,7 @@
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
path := "__debug/stats/system/hostname"
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
got := binary.Start("stats", "read", path).Output()
hostname, err := os.Hostname()
if err != nil {
@@ -72,7 +72,7 @@
func V23TestLogSize(i *v23tests.T) {
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
testLogData := "This is a test log file"
file := createTestLogFile(i, testLogData)
@@ -91,7 +91,7 @@
func V23TestStatsRead(i *v23tests.T) {
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
testLogData := "This is a test log file\n"
file := createTestLogFile(i, testLogData)
logName := filepath.Base(file.Name())
@@ -111,7 +111,7 @@
func V23TestStatsWatch(i *v23tests.T) {
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
testLogData := "This is a test log file\n"
file := createTestLogFile(i, testLogData)
logName := filepath.Base(file.Name())
@@ -150,7 +150,7 @@
func V23TestVTrace(i *v23tests.T) {
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
logContent := "Hello, world!\n"
logPath := "__debug/logs/" + filepath.Base(createTestLogFile(i, logContent).Name())
// Create a log file with tracing, read it and check that the resulting trace exists.
@@ -203,7 +203,7 @@
func V23TestPprof(i *v23tests.T) {
v23tests.RunRootMT(i, "--veyron.tcp.address=127.0.0.1:0")
- binary := i.BuildGoPkg("v.io/x/ref/cmd/debug")
+ binary := i.BuildV23Pkg("v.io/x/ref/cmd/debug")
inv := binary.Start("pprof", "run", "__debug/pprof", "heap", "--text")
// Assert that a profile indicating the heap size was written out.
diff --git a/cmd/mgmt/device/doc.go b/cmd/mgmt/device/doc.go
index 653d474..45f0d8b 100644
--- a/cmd/mgmt/device/doc.go
+++ b/cmd/mgmt/device/doc.go
@@ -351,9 +351,9 @@
Name of application service.
-binserv=binaryd
Name of binary service.
- -goarch=amd64
+ -goarch=$GOARCH
GOARCH for application.
- -goos=linux
+ -goos=$GOOS
GOOS for application.
-readers=dev.v.io
If non-empty, comma-separated blessing patterns to add to Read and Resolve
diff --git a/cmd/mgmt/device/impl/local_install.go b/cmd/mgmt/device/impl/local_install.go
index 1781dc5..78e06f2 100644
--- a/cmd/mgmt/device/impl/local_install.go
+++ b/cmd/mgmt/device/impl/local_install.go
@@ -82,10 +82,6 @@
return nil, nil, err
}
spec := v23.GetListenSpec(ctx)
- endpoints, err := server.Listen(spec)
- if err != nil {
- return nil, nil, err
- }
var name string
if spec.Proxy != "" {
id, err := uniqueid.Random()
@@ -93,6 +89,14 @@
return nil, nil, err
}
name = id.String()
+ // Disable listening on local addresses to avoid publishing
+ // local endpoints to the mount table. The only thing published
+ // should be the proxied endpoint.
+ spec.Addrs = nil
+ }
+ endpoints, err := server.Listen(spec)
+ if err != nil {
+ return nil, nil, err
}
dispatcher := make(mapDispatcher)
if err := server.ServeDispatcher(name, dispatcher); err != nil {
@@ -137,6 +141,11 @@
func (i binaryInvoker) Download(call repository.BinaryDownloadServerCall, _ int32) error {
fileName := string(i)
+ fStat, err := os.Stat(fileName)
+ if err != nil {
+ return err
+ }
+ vlog.VI(1).Infof("Download commenced for %v (%v bytes)", fileName, fStat.Size())
file, err := os.Open(fileName)
if err != nil {
return err
@@ -145,15 +154,22 @@
bufferLength := 4096
buffer := make([]byte, bufferLength)
sender := call.SendStream()
+ var sentTotal int64
+ const logChunk = 1 << 20
for {
n, err := file.Read(buffer)
switch err {
case io.EOF:
+ vlog.VI(1).Infof("Download complete for %v (%v bytes)", fileName, fStat.Size())
return nil
case nil:
if err := sender.Send(buffer[:n]); err != nil {
return err
}
+ if sentTotal/logChunk < (sentTotal+int64(n))/logChunk {
+ vlog.VI(1).Infof("Download progress for %v: %v/%v", fileName, sentTotal+int64(n), fStat.Size())
+ }
+ sentTotal += int64(n)
default:
return err
}
@@ -277,6 +293,7 @@
if err != nil {
return err
}
+ vlog.VI(1).Infof("binary %v serving as %v", binary, envelope.Binary.File)
// For each package dir/file specified in the arguments list, set up an
// object in the binary service to serve that package, and add the
diff --git a/cmd/mgmt/device/impl/publish.go b/cmd/mgmt/device/impl/publish.go
index 35621e4..21b3bf0 100644
--- a/cmd/mgmt/device/impl/publish.go
+++ b/cmd/mgmt/device/impl/publish.go
@@ -40,11 +40,32 @@
var binaryService, applicationService, goos, goarch, readBlessings string
+const (
+ defaultArch = "$GOARCH"
+ defaultOS = "$GOOS"
+)
+
+var (
+ flagArch string
+ flagOS string
+)
+
+// SubstituteVarsInFlags substitutes environment variables in default
+// values of relevant flags.
+func SubstituteVarsInFlags() {
+ if flagArch == defaultArch {
+ flagArch = runtime.GOARCH
+ }
+ if flagOS == defaultOS {
+ flagOS = runtime.GOOS
+ }
+}
+
func init() {
cmdPublish.Flags.StringVar(&binaryService, "binserv", "binaryd", "Name of binary service.")
cmdPublish.Flags.StringVar(&applicationService, "appserv", "applicationd", "Name of application service.")
- cmdPublish.Flags.StringVar(&goos, "goos", runtime.GOOS, "GOOS for application.")
- cmdPublish.Flags.StringVar(&goarch, "goarch", runtime.GOARCH, "GOARCH for application.")
+ cmdPublish.Flags.StringVar(&goos, "goos", defaultOS, "GOOS for application.")
+ cmdPublish.Flags.StringVar(&goarch, "goarch", defaultArch, "GOARCH for application.")
cmdPublish.Flags.StringVar(&readBlessings, "readers", "dev.v.io", "If non-empty, comma-separated blessing patterns to add to Read and Resolve ACL.")
}
diff --git a/cmd/mgmt/device/main.go b/cmd/mgmt/device/main.go
index c5714e2..93fd359 100644
--- a/cmd/mgmt/device/main.go
+++ b/cmd/mgmt/device/main.go
@@ -15,6 +15,7 @@
func main() {
gctx, shutdown := v23.Init()
impl.SetGlobalContext(gctx)
+ impl.SubstituteVarsInFlags()
exitCode := impl.Root().Main()
shutdown()
os.Exit(exitCode)
diff --git a/cmd/naming/simulator/driver.go b/cmd/naming/simulator/driver.go
index f562216..ec5fbce 100644
--- a/cmd/naming/simulator/driver.go
+++ b/cmd/naming/simulator/driver.go
@@ -117,7 +117,7 @@
// Subprocesses commands are run by fork/execing this binary
// so we must test to see if this instance is a subprocess or the
// the original command line instance.
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
shutdown()
// Subprocess, run the requested command.
if err := modules.Dispatch(); err != nil {
diff --git a/cmd/naming/simulator/v23_test.go b/cmd/naming/simulator/v23_test.go
index c374413..94328a4 100644
--- a/cmd/naming/simulator/v23_test.go
+++ b/cmd/naming/simulator/v23_test.go
@@ -16,7 +16,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/cmd/principal/principal_v23_test.go b/cmd/principal/principal_v23_test.go
index fb7f5ac..41b6433 100644
--- a/cmd/principal/principal_v23_test.go
+++ b/cmd/principal/principal_v23_test.go
@@ -44,7 +44,7 @@
)
bin := t.BuildGoPkg("v.io/x/ref/cmd/principal")
- bin.Start("create", aliceDir, "alice").WaitOrDie(os.Stdout, os.Stderr)
+ bin.Run("create", aliceDir, "alice")
bin = bin.WithEnv("VEYRON_CREDENTIALS=" + aliceDir)
redirect(t, bin.Start("blessself", "alicereborn"), aliceBlessingFile)
diff --git a/cmd/servicerunner/main.go b/cmd/servicerunner/main.go
index 21ac2f2..4d33f26 100644
--- a/cmd/servicerunner/main.go
+++ b/cmd/servicerunner/main.go
@@ -56,7 +56,7 @@
}
func main() {
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
panicOnError(modules.Dispatch())
return
}
diff --git a/examples/rps/rpsbot/v23_internal_test.go b/examples/rps/rpsbot/v23_internal_test.go
index 3a3370a..6ab5fba 100644
--- a/examples/rps/rpsbot/v23_internal_test.go
+++ b/examples/rps/rpsbot/v23_internal_test.go
@@ -15,7 +15,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/examples/tunnel/tunneld/tunneld_v23_test.go b/examples/tunnel/tunneld/tunneld_v23_test.go
index a141ce4..be0361a 100644
--- a/examples/tunnel/tunneld/tunneld_v23_test.go
+++ b/examples/tunnel/tunneld/tunneld_v23_test.go
@@ -15,9 +15,9 @@
func V23TestTunneld(t *v23tests.T) {
v23tests.RunRootMT(t, "--veyron.tcp.address=127.0.0.1:0")
- tunneldBin := t.BuildGoPkg("v.io/x/ref/examples/tunnel/tunneld")
- vsh := t.BuildGoPkg("v.io/x/ref/examples/tunnel/vsh")
- mounttableBin := t.BuildGoPkg("v.io/x/ref/cmd/mounttable")
+ tunneldBin := t.BuildV23Pkg("v.io/x/ref/examples/tunnel/tunneld")
+ vsh := t.BuildV23Pkg("v.io/x/ref/examples/tunnel/vsh")
+ mounttableBin := t.BuildV23Pkg("v.io/x/ref/cmd/mounttable")
// Start tunneld with a known endpoint.
tunnelEndpoint := tunneldBin.Start("--veyron.tcp.address=127.0.0.1:0").ExpectVar("NAME")
diff --git a/lib/exec/parent.go b/lib/exec/parent.go
index ddb3d5b..326d45b 100644
--- a/lib/exec/parent.go
+++ b/lib/exec/parent.go
@@ -179,7 +179,6 @@
return err
}
return nil
-
}
// copy is like io.Copy, but it also treats the receipt of the special eofChar
diff --git a/lib/modules/core/core_test.go b/lib/modules/core/core_test.go
index 0112757..559d611 100644
--- a/lib/modules/core/core_test.go
+++ b/lib/modules/core/core_test.go
@@ -8,7 +8,6 @@
"sort"
"strconv"
"testing"
- "time"
"v.io/x/lib/vlog"
@@ -16,7 +15,6 @@
"v.io/x/ref/lib/modules"
"v.io/x/ref/lib/modules/core"
"v.io/x/ref/lib/testutil"
- "v.io/x/ref/lib/testutil/expect"
_ "v.io/x/ref/profiles"
)
@@ -24,7 +22,7 @@
// recognize that this requires modules.Dispatch().
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
@@ -48,14 +46,12 @@
func newShell(t *testing.T) (*modules.Shell, func()) {
ctx, shutdown := testutil.InitForTest()
-
- sh, err := modules.NewShell(ctx, nil)
+ sh, err := modules.NewExpectShell(ctx, nil, t, testing.Verbose())
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
return sh, func() {
if testing.Verbose() {
- vlog.Infof("------ cleanup ------")
sh.Cleanup(os.Stderr, os.Stderr)
} else {
sh.Cleanup(nil, nil)
@@ -76,9 +72,8 @@
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
- s := expect.NewSession(t, root.Stdout(), time.Second)
- s.ExpectVar("PID")
- s.ExpectVar("MT_NAME")
+ root.ExpectVar("PID")
+ root.ExpectVar("MT_NAME")
root.CloseStdin()
}
@@ -89,11 +84,10 @@
t.Fatalf("unexpected error for root mt: %s", err)
}
sh.Forget(root)
- rootSession := expect.NewSession(t, root.Stdout(), time.Minute)
- rootSession.ExpectVar("PID")
- rootName := rootSession.ExpectVar("MT_NAME")
+ root.ExpectVar("PID")
+ rootName := root.ExpectVar("MT_NAME")
if t.Failed() {
- return nil, nil, rootSession.Error()
+ return nil, nil, root.Error()
}
sh.SetVar(consts.NamespaceRootPrefix, rootName)
mountAddrs := make(map[string]string)
@@ -105,13 +99,12 @@
if err != nil {
return nil, nil, fmt.Errorf("unexpected error for mt %q: %s", mp, err)
}
- s := expect.NewSession(t, h.Stdout(), time.Minute)
// Wait until each mount table has at least called Serve to
// mount itself.
- s.ExpectVar("PID")
- mountAddrs[mp] = s.ExpectVar("MT_NAME")
- if s.Failed() {
- return nil, nil, s.Error()
+ h.ExpectVar("PID")
+ mountAddrs[mp] = h.ExpectVar("MT_NAME")
+ if h.Failed() {
+ return nil, nil, h.Error()
}
}
deferFn := func() {
@@ -159,10 +152,8 @@
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
- lseSession := expect.NewSession(t, lse.Stdout(), time.Minute)
- lseSession.SetVerbosity(testing.Verbose())
- if got, want := lseSession.ExpectVar("RN"), strconv.Itoa(len(mountPoints)); got != want {
+ if got, want := lse.ExpectVar("RN"), strconv.Itoa(len(mountPoints)); got != want {
t.Fatalf("got %v, want %v", got, want)
}
@@ -177,7 +168,7 @@
pattern = pattern[:len(pattern)-1]
found := []string{}
for i := 0; i < len(mountPoints); i++ {
- found = append(found, getMatchingMountpoint(lseSession.ExpectRE(pattern, 1)))
+ found = append(found, getMatchingMountpoint(lse.ExpectRE(pattern, 1)))
}
sort.Strings(found)
sort.Strings(mountPoints)
@@ -194,9 +185,8 @@
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
- srvSession := expect.NewSession(t, srv.Stdout(), time.Minute)
- srvSession.ExpectVar("PID")
- name := srvSession.ExpectVar("NAME")
+ srv.ExpectVar("PID")
+ name := srv.ExpectVar("NAME")
if len(name) == 0 {
t.Fatalf("failed to get name")
}
@@ -204,14 +194,13 @@
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
- cltSession := expect.NewSession(t, clt.Stdout(), time.Minute)
- cltSession.Expect("test: a message")
+ clt.Expect("test: a message")
}
func TestExec(t *testing.T) {
sh, cleanup := newShell(t)
defer cleanup()
- h, err := sh.StartExternalCommand(nil, nil, []string{"/bin/sh", "-c", "echo hello world"}...)
+ h, err := sh.StartWithOpts(sh.DefaultStartOpts().NoExecCommand(), nil, "/bin/sh", "-c", "echo hello world")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@@ -227,7 +216,7 @@
func TestExecWithEnv(t *testing.T) {
sh, cleanup := newShell(t)
defer cleanup()
- h, err := sh.StartExternalCommand(nil, []string{"BLAH=hello world"}, "/bin/sh", "-c", "printenv BLAH")
+ h, err := sh.StartWithOpts(sh.DefaultStartOpts().NoExecCommand(), []string{"BLAH=hello world"}, "/bin/sh", "-c", "printenv BLAH")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
diff --git a/lib/modules/examples_test.go b/lib/modules/examples_test.go
index 5b18040..f9ae282 100644
--- a/lib/modules/examples_test.go
+++ b/lib/modules/examples_test.go
@@ -23,7 +23,7 @@
func ExampleDispatch() {
ctx, shutdown := testutil.InitForTest()
defer shutdown()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
// Child process. Dispatch will invoke the 'echo' command
if err := modules.Dispatch(); err != nil {
panic(fmt.Sprintf("unexpected error: %s", err))
diff --git a/lib/modules/exec.go b/lib/modules/exec.go
index 08b14b5..ad494fc 100644
--- a/lib/modules/exec.go
+++ b/lib/modules/exec.go
@@ -13,22 +13,23 @@
"v.io/v23/mgmt"
"v.io/x/lib/vlog"
vexec "v.io/x/ref/lib/exec"
+ "v.io/x/ref/lib/testutil/expect"
)
// execHandle implements both the command and Handle interfaces.
type execHandle struct {
+ *expect.Session
mu sync.Mutex
cmd *exec.Cmd
name string
entryPoint string
handle *vexec.ParentHandle
sh *Shell
- childStdin io.Reader
stderr *os.File
stdout io.ReadCloser
stdin io.WriteCloser
procErrCh chan error
- external bool
+ opts *StartOpts
}
func testFlags() []string {
@@ -43,7 +44,6 @@
return fl
}
// must be a go test binary
- fl = append(fl, "-test.run=TestHelperProcess")
val := timeout.Value.(flag.Getter).Get().(time.Duration)
if val.String() != timeout.DefValue {
// use supplied command value for subprocesses
@@ -57,23 +57,12 @@
return fl
}
-// IsTestHelperProcess returns true if it is called in via
-// -run=TestHelperProcess which normally only ever happens for subprocesses
-// run from tests.
-func IsTestHelperProcess() bool {
- runFlag := flag.Lookup("test.run")
- if runFlag == nil {
- return false
- }
- return runFlag.Value.String() == "TestHelperProcess"
-}
-
func newExecHandle(name string) command {
return &execHandle{name: name, entryPoint: shellEntryPoint + "=" + name, procErrCh: make(chan error, 1)}
}
-func newExecHandleForExternalCommand(name string, stdin io.Reader) command {
- return &execHandle{name: name, external: true, childStdin: stdin, procErrCh: make(chan error, 1)}
+func newExecHandleForExternalCommand(name string) command {
+ return &execHandle{name: name, procErrCh: make(chan error, 1)}
}
func (eh *execHandle) Stdout() io.Reader {
@@ -116,15 +105,16 @@
return newargs, append(cleaned, eh.entryPoint)
}
-func (eh *execHandle) start(sh *Shell, agentfd *os.File, env []string, args ...string) (Handle, error) {
+func (eh *execHandle) start(sh *Shell, agentfd *os.File, opts *StartOpts, env []string, args ...string) (Handle, error) {
eh.mu.Lock()
defer eh.mu.Unlock()
eh.sh = sh
+ eh.opts = opts
cmdPath := args[0]
newargs, newenv := args, env
// If an entry point is specified, use the envelope execution environment.
- if eh.entryPoint != "" {
+ if len(eh.entryPoint) > 0 {
cmdPath = os.Args[0]
newargs, newenv = eh.envelope(sh, env, args[1:]...)
}
@@ -149,8 +139,8 @@
// If we have an explicit stdin to pass to the child, use that,
// otherwise create a pipe and return the write side of that pipe
// in the handle.
- if eh.childStdin != nil {
- cmd.Stdin = eh.childStdin
+ if eh.opts.Stdin != nil {
+ cmd.Stdin = eh.opts.Stdin
} else {
stdin, err := cmd.StdinPipe()
if err != nil {
@@ -159,22 +149,29 @@
eh.stdin = stdin
}
config := vexec.NewConfig()
- serialized, err := sh.config.Serialize()
- if err != nil {
- return nil, err
- }
- config.MergeFrom(serialized)
- if agentfd != nil {
- childfd := len(cmd.ExtraFiles) + vexec.FileOffset
- config.Set(mgmt.SecurityAgentFDConfigKey, strconv.Itoa(childfd))
- cmd.ExtraFiles = append(cmd.ExtraFiles, agentfd)
- defer agentfd.Close()
+
+ execOpts := []vexec.ParentHandleOpt{}
+ if !eh.opts.ExecProtocol {
+ execOpts = append(execOpts, vexec.UseExecProtocolOpt(false))
+ } else {
+ serialized, err := sh.config.Serialize()
+ if err != nil {
+ return nil, err
+ }
+ config.MergeFrom(serialized)
+ if agentfd != nil {
+ childfd := len(cmd.ExtraFiles) + vexec.FileOffset
+ config.Set(mgmt.SecurityAgentFDConfigKey, strconv.Itoa(childfd))
+ cmd.ExtraFiles = append(cmd.ExtraFiles, agentfd)
+ defer agentfd.Close()
+ }
+ execOpts = append(execOpts, vexec.ConfigOpt{Config: config})
}
// TODO(cnicolaou): for external commands, vexec should either not be
// used or it should taken an option to not use its protocol, and in
// particular to share secrets with children.
- handle := vexec.NewParentHandle(cmd, vexec.ConfigOpt{Config: config})
+ handle := vexec.NewParentHandle(cmd, execOpts...)
eh.stdout = stdout
eh.stderr = stderr
eh.handle = handle
@@ -183,13 +180,25 @@
vlog.VI(1).Infof("Start: %q args: %v", eh.name, cmd.Args)
vlog.VI(2).Infof("Start: %q env: %v", eh.name, cmd.Env)
if err := handle.Start(); err != nil {
- return nil, err
+ // The child process failed to start, either because of some setup
+ // error (e.g. creating pipes for it to use), or a bad binary etc.
+ // A handle is returned, so that Shutdown etc may be called, hence
+ // the error must be sent over eh.procErrCh to allow Shutdown to
+ // terminate.
+ eh.procErrCh <- err
+ return eh, err
}
- // TODO(cnicolaou): we should really call handle.WaitForReady here,
- // but if we do, we'll no longer be able to use this interface
- // for non Vanadium processes. We should extend the API to distinguish
- // between Vanadium and non-Vanadium processes and then the various
- // clients of this API will need to be changed accordingly.
+ if eh.opts.ExecProtocol {
+ if err := eh.handle.WaitForReady(eh.opts.StartTimeout); err != nil {
+ // The child failed to call SetReady, most likely because of bad
+ // command line arguments or some other early exit in the child
+ // process.
+ // As per Start above, a handle is returned and the error
+ // sent over eh.procErrCh.
+ eh.procErrCh <- err
+ return eh, err
+ }
+ }
vlog.VI(1).Infof("Started: %q, pid %d", eh.name, cmd.Process.Pid)
go func() {
eh.procErrCh <- eh.handle.Wait(0)
@@ -199,7 +208,8 @@
// up (they'll receive EOF).
eh.stdout.Close()
}()
-
+ eh.Session = expect.NewSession(opts.ExpectTesting, stdout, opts.ExpectTimeout)
+ eh.Session.SetVerbosity(eh.sh.sessionVerbosity)
return eh, nil
}
@@ -232,7 +242,7 @@
select {
case procErr = <-eh.procErrCh:
// The child has exited already.
- case <-time.After(eh.sh.waitTimeout):
+ case <-time.After(eh.opts.ShutdownTimeout):
// Time out waiting for child to exit.
procErr = vexec.ErrTimeout
// Force close stdout to unblock any readers of stdout
@@ -247,7 +257,3 @@
return procErr
}
-
-func (eh *execHandle) WaitForReady(timeout time.Duration) error {
- return eh.handle.WaitForReady(timeout)
-}
diff --git a/lib/modules/func.go b/lib/modules/func.go
index 04dd7b3..ff0ebd9 100644
--- a/lib/modules/func.go
+++ b/lib/modules/func.go
@@ -8,12 +8,19 @@
"time"
"v.io/x/lib/vlog"
+ "v.io/x/ref/lib/testutil/expect"
)
type pipe struct {
- r, w *os.File
+ // We use the *os.File when we create pipe's that we need
+ // to close etc, but we use the io.Reader when we have
+ // a stdin stream specified via StartOpts. In that case we
+ // don't have a *os.File that we can close.
+ rf, wf *os.File
+ r io.Reader
}
type functionHandle struct {
+ *expect.Session
mu sync.Mutex
name string
main Main
@@ -21,6 +28,7 @@
stderr *os.File
err error
sh *Shell
+ opts *StartOpts
wg sync.WaitGroup
}
@@ -43,12 +51,14 @@
func (fh *functionHandle) Stdin() io.Writer {
fh.mu.Lock()
defer fh.mu.Unlock()
- return fh.stdin.w
+ return fh.stdin.wf
}
func (fh *functionHandle) CloseStdin() {
fh.mu.Lock()
- fh.stdin.w.Close()
+ if fh.stdin.wf != nil {
+ fh.stdin.wf.Close()
+ }
fh.mu.Unlock()
}
@@ -56,20 +66,32 @@
return args, env
}
-func (fh *functionHandle) start(sh *Shell, agent *os.File, env []string, args ...string) (Handle, error) {
+func (fh *functionHandle) start(sh *Shell, agent *os.File, opts *StartOpts, env []string, args ...string) (Handle, error) {
fh.mu.Lock()
defer fh.mu.Unlock()
+ fh.opts = opts
// In process commands need their own reference to a principal.
if agent != nil {
agent.Close()
}
fh.sh = sh
- for _, p := range []*pipe{&fh.stdin, &fh.stdout} {
+
+ var pipes []*pipe
+ if fh.opts.Stdin != nil {
+ pipes = []*pipe{&fh.stdout}
+ fh.stdin.r = fh.opts.Stdin
+ } else {
+ pipes = []*pipe{&fh.stdin, &fh.stdout}
+ }
+
+ for _, p := range pipes {
var err error
- if p.r, p.w, err = os.Pipe(); err != nil {
+ if p.rf, p.wf, err = os.Pipe(); err != nil {
return nil, err
}
+ p.r, _ = p.rf, p.wf
}
+
stderr, err := newLogfile("stderr", args[0])
if err != nil {
return nil, err
@@ -80,7 +102,7 @@
go func(env []string) {
fh.mu.Lock()
stdin := fh.stdin.r
- stdout := fh.stdout.w
+ stdout := io.Writer(fh.stdout.wf)
stderr := fh.stderr
main := fh.main
fh.mu.Unlock()
@@ -94,12 +116,16 @@
}
fh.mu.Lock()
- fh.stdin.r.Close()
- fh.stdout.w.Close()
+ if fh.stdin.rf != nil {
+ fh.stdin.rf.Close()
+ }
+ fh.stdout.wf.Close()
fh.err = err
fh.mu.Unlock()
fh.wg.Done()
}(env)
+ fh.Session = expect.NewSession(opts.ExpectTesting, fh.stdout.r, opts.ExpectTimeout)
+ fh.Session.SetVerbosity(fh.sh.sessionVerbosity)
return fh, nil
}
@@ -110,7 +136,9 @@
func (fh *functionHandle) Shutdown(stdout_w, stderr_w io.Writer) error {
fh.mu.Lock()
vlog.VI(1).Infof("Shutdown: %q", fh.name)
- fh.stdin.w.Close()
+ if fh.stdin.wf != nil {
+ fh.stdin.wf.Close()
+ }
stdout := fh.stdout.r
stderr := fh.stderr
fh.mu.Unlock()
@@ -139,7 +167,9 @@
}
fh.mu.Lock()
- fh.stdout.r.Close()
+ if fh.stdout.rf != nil {
+ fh.stdout.rf.Close()
+ }
fh.sh.Forget(fh)
fh.mu.Unlock()
return funcErr
diff --git a/lib/modules/modules_test.go b/lib/modules/modules_test.go
index 092765d..9a8c8b0 100644
--- a/lib/modules/modules_test.go
+++ b/lib/modules/modules_test.go
@@ -5,9 +5,11 @@
"bytes"
"fmt"
"io"
+ "math/rand"
"os"
"reflect"
"sort"
+ "strconv"
"strings"
"syscall"
"testing"
@@ -34,17 +36,20 @@
modules.RegisterChild("echos", "[args]*", Echo)
modules.RegisterChild("errortestChild", "", ErrorMain)
modules.RegisterChild("ignores_stdin", "", ignoresStdin)
+ modules.RegisterChild("pipeProc", "", pipeEcho)
+ modules.RegisterChild("lifo", "", lifo)
modules.RegisterFunction("envtestf", "envtest: <variables to print>...", PrintFromEnv)
modules.RegisterFunction("echof", "[args]*", Echo)
modules.RegisterFunction("errortestFunc", "", ErrorMain)
+ modules.RegisterFunction("pipeFunc", "", pipeEcho)
}
// We must call Testmain ourselves because using v23 test generate
// creates an import cycle for this package.
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
@@ -67,6 +72,23 @@
return nil
}
+func pipeEcho(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
+ scanner := bufio.NewScanner(stdin)
+ for scanner.Scan() {
+ fmt.Fprintf(stdout, "%p: %s\n", pipeEcho, scanner.Text())
+ }
+ return nil
+}
+
+func lifo(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
+ scanner := bufio.NewScanner(stdin)
+ scanner.Scan()
+ msg := scanner.Text()
+ modules.WaitForEOF(stdin)
+ fmt.Fprintf(stdout, "%p: %s\n", lifo, msg)
+ return nil
+}
+
func PrintBlessing(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
ctx, shutdown := testutil.InitForTest()
defer shutdown()
@@ -169,7 +191,7 @@
}
func getCustomBlessing(t *testing.T, sh *modules.Shell, creds *modules.CustomCredentials) string {
- h, err := sh.StartWithCredentials("printblessing", nil, creds)
+ h, err := sh.StartWithOpts(sh.DefaultStartOpts().WithCustomCredentials(creds), nil, "printblessing")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@@ -297,13 +319,10 @@
}
func TestChildNoRegistration(t *testing.T) {
- // TODO(jsimsa): Re-enable this test when it is no longer flaky.
- // https://github.com/veyron/release-issues/issues/1205
- t.SkipNow()
-
ctx, shutdown := testutil.InitForTest()
defer shutdown()
+ //fmt.Fprintf(os.Stderr, "B\n")
sh, err := modules.NewShell(ctx, nil)
if err != nil {
t.Fatalf("unexpected error: %s", err)
@@ -314,6 +333,7 @@
testCommand(t, sh, "envtest", key, val)
_, err = sh.Start("non-existent-command", nil, "random", "args")
if err == nil {
+ fmt.Fprintf(os.Stderr, "Failed: %v\n", err)
t.Fatalf("expected error")
}
}
@@ -398,8 +418,9 @@
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
- sh.SetWaitTimeout(time.Second)
- h, err := sh.Start("ignores_stdin", nil)
+ opts := sh.DefaultStartOpts()
+ opts.ShutdownTimeout = time.Second
+ h, err := sh.StartWithOpts(opts, nil, "ignores_stdin")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@@ -426,8 +447,9 @@
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
- sh.SetWaitTimeout(time.Second)
- h, err := sh.Start("ignores_stdin", nil)
+ opts := sh.DefaultStartOpts()
+ opts.ShutdownTimeout = time.Second
+ h, err := sh.StartWithOpts(opts, nil, "ignores_stdin")
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
@@ -596,4 +618,190 @@
}
}
-// TODO(cnicolaou): test for error return from cleanup
+func TestNoExec(t *testing.T) {
+ ctx, shutdown := testutil.InitForTest()
+ defer shutdown()
+
+ sh, err := modules.NewShell(ctx, nil)
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ defer sh.Cleanup(nil, nil)
+ h, err := sh.StartWithOpts(sh.DefaultStartOpts().NoExecCommand(), nil, "/bin/echo", "hello", "world")
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ scanner := bufio.NewScanner(h.Stdout())
+ scanner.Scan()
+ if got, want := scanner.Text(), "hello world"; got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+}
+
+func TestExternal(t *testing.T) {
+ ctx, shutdown := testutil.InitForTest()
+ defer shutdown()
+
+ sh, err := modules.NewShell(ctx, nil)
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ defer sh.Cleanup(nil, nil)
+ cookie := strconv.Itoa(rand.Int())
+ sh.SetConfigKey("cookie", cookie)
+ h, err := sh.StartWithOpts(sh.DefaultStartOpts().ExternalCommand(), nil, os.Args[0], "--test.run=TestExternalTestHelper")
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ scanner := bufio.NewScanner(h.Stdout())
+ scanner.Scan()
+ if got, want := scanner.Text(), fmt.Sprintf("cookie: %s", cookie); got != want {
+ h.Shutdown(os.Stderr, os.Stderr)
+ t.Fatalf("got %v, want %v", got, want)
+ }
+}
+
+// TestExternalTestHelper is used by TestExternal above and has not utility
+// as a test in it's own right.
+func TestExternalTestHelper(t *testing.T) {
+ child, err := exec.GetChildHandle()
+ if err != nil {
+ return
+ }
+ child.SetReady()
+ val, err := child.Config.Get("cookie")
+ if err != nil {
+ t.Fatalf("failed to get child handle: %s", err)
+ }
+ fmt.Printf("cookie: %s\n", val)
+}
+
+func TestPipe(t *testing.T) {
+ ctx, shutdown := testutil.InitForTest()
+ defer shutdown()
+
+ sh, err := modules.NewShell(ctx, nil)
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ defer sh.Cleanup(nil, nil)
+
+ for _, cmd := range []string{"pipeProc", "pipeFunc"} {
+ r, w, err := os.Pipe()
+ if err != nil {
+ t.Fatal(err)
+ }
+ opts := sh.DefaultStartOpts()
+ opts.Stdin = r
+ h, err := sh.StartWithOpts(opts, nil, cmd)
+ if err != nil {
+ t.Fatal(err)
+ }
+ cookie := strconv.Itoa(rand.Int())
+ go func(w *os.File, s string) {
+ fmt.Fprintf(w, "hello world\n")
+ fmt.Fprintf(w, "%s\n", s)
+ w.Close()
+ }(w, cookie)
+
+ scanner := bufio.NewScanner(h.Stdout())
+ want := []string{
+ fmt.Sprintf("%p: hello world", pipeEcho),
+ fmt.Sprintf("%p: %s", pipeEcho, cookie),
+ }
+ i := 0
+ for scanner.Scan() {
+ if got, want := scanner.Text(), want[i]; got != want {
+ t.Fatalf("%s: got %v, want %v", cmd, got, want)
+ }
+ i++
+ }
+ if got, want := i, 2; got != want {
+ t.Fatalf("%s: got %v, want %v", cmd, got, want)
+ }
+ if err := h.Shutdown(os.Stderr, os.Stderr); err != nil {
+ t.Fatal(err)
+ }
+ r.Close()
+ }
+}
+
+func TestLIFO(t *testing.T) {
+ ctx, shutdown := testutil.InitForTest()
+ defer shutdown()
+ sh, err := modules.NewShell(ctx, nil)
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ defer sh.Cleanup(nil, nil)
+
+ cases := []string{"a", "b", "c"}
+ for _, msg := range cases {
+ h, err := sh.Start("lifo", nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ fmt.Fprintf(h.Stdin(), "%s\n", msg)
+ }
+ var buf bytes.Buffer
+ if err := sh.Cleanup(&buf, nil); err != nil {
+ t.Fatal(err)
+ }
+ lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n")
+ if got, want := len(lines), len(cases); got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+ sort.Sort(sort.Reverse(sort.StringSlice(cases)))
+ for i, msg := range cases {
+ if got, want := lines[i], fmt.Sprintf("%p: %s", lifo, msg); got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+ }
+}
+
+func TestStartOpts(t *testing.T) {
+ sh, err := modules.NewShell(nil, nil)
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ opts := modules.StartOpts{
+ External: true,
+ }
+ sh.SetDefaultStartOpts(opts)
+ def := sh.DefaultStartOpts()
+ if got, want := def.External, opts.External; got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+ def.External = false
+ if got, want := def, (modules.StartOpts{}); !reflect.DeepEqual(got, want) {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+
+ // Verify that the shell retains a copy.
+ opts.External = false
+ opts.ExecProtocol = true
+ def = sh.DefaultStartOpts()
+ if got, want := def.External, true; got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+ if got, want := def.ExecProtocol, false; got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+
+ sh.SetDefaultStartOpts(opts)
+ def = sh.DefaultStartOpts()
+ if got, want := def.ExecProtocol, true; got != want {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+}
+
+func TestEmbeddedSession(t *testing.T) {
+ sh, err := modules.NewExpectShell(nil, nil, t, testing.Verbose())
+ if err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ def := sh.DefaultStartOpts()
+ if def.ExpectTesting == nil {
+ t.Fatalf("ExpectTesting should be non nil")
+ }
+}
diff --git a/lib/modules/registry.go b/lib/modules/registry.go
index 110c145..85c75c1 100644
--- a/lib/modules/registry.go
+++ b/lib/modules/registry.go
@@ -39,6 +39,13 @@
return r.cmds[name]
}
+func (r *cmdRegistry) getExternalCommand(name string) *commandDesc {
+ h := newExecHandleForExternalCommand(name)
+ return &commandDesc{
+ factory: func() command { return h },
+ }
+}
+
// RegisterChild adds a new command to the registry that will be run
// as a subprocess. It must be called before Dispatch or DispatchInTest is
// called, typically from an init function.
@@ -79,9 +86,9 @@
const shellEntryPoint = "VEYRON_SHELL_HELPER_PROCESS_ENTRY_POINT"
-// IsModulesProcess returns true if this process is run using
+// IsModulesChildProcess returns true if this process was started by
// the modules package.
-func IsModulesProcess() bool {
+func IsModulesChildProcess() bool {
return os.Getenv(shellEntryPoint) != ""
}
@@ -104,7 +111,7 @@
// process that does not specify an entry point in its environment.
//
// func main() {
-// if modules.IsModulesProcess() {
+// if modules.IsModulesChildProcess() {
// if err := modules.Dispatch(); err != nil {
// panic("error")
// }
@@ -113,25 +120,12 @@
// parent code...
//
func Dispatch() error {
- if !IsModulesProcess() {
+ if !IsModulesChildProcess() {
return nil
}
return registry.dispatch()
}
-// TODO(cnicolaou): delete this in a subsequent CL.
-// DispatchInTest will execute the requested subproccess command from within
-// a unit test run as a subprocess.
-func DispatchInTest() {
- if !IsTestHelperProcess() {
- return
- }
- if err := registry.dispatch(); err != nil {
- vlog.Fatalf("Failed: %s", err)
- }
- os.Exit(0)
-}
-
// DispatchAndExit is like Dispatch except that it will call os.Exit(0)
// when executed within a child process and the command succeeds, or panic
// on encountering an error.
@@ -141,12 +135,9 @@
// parent code...
//
func DispatchAndExit() {
- if !IsModulesProcess() {
+ if !IsModulesChildProcess() {
return
}
- if IsTestHelperProcess() {
- panic("use DispatchInTest in unittests")
- }
if err := registry.dispatch(); err != nil {
panic(fmt.Sprintf("unexpected error: %s", err))
}
diff --git a/lib/modules/shell.go b/lib/modules/shell.go
index 55be3b3..9965aae 100644
--- a/lib/modules/shell.go
+++ b/lib/modules/shell.go
@@ -1,43 +1,139 @@
// Package modules provides a mechanism for running commonly used services
// as subprocesses and client functionality for accessing those services.
-// Such services and functions are collectively called 'commands' and are
-// registered with a single, per-process, registry, but executed within a
-// context, defined by the Shell type. The Shell is analagous to the original
-// UNIX shell and maintains a key, value store of variables that is accessible
-// to all of the commands that it hosts. These variables may be referenced by
-// the arguments passed to commands.
+// Such services and functions are collectively called 'commands' and
+// are managed by a 'Registry'. The Shell is analagous to the UNIX shell and
+// maintains a key, value store of environment variables and config settings
+// that are accessible to the commands that it hosts. Simple variable
+// expansion is supported.
//
-// Commands are added to the registry in two ways:
-// - via RegisterChild for a subprocess
-// - via RegisterFunction for an in-process function
+// Four types of 'commands' may be invoked via a Shell.
//
-// In all cases commands are started by invoking the Start method on the
-// Shell with the name of the command to run. An instance of the Handle
-// interface is returned which can be used to interact with the function
-// or subprocess, and in particular to read/write data from/to it using io
-// channels that follow the stdin, stdout, stderr convention.
+// - functions of type Shell.Main as subprocesses via fork/exec
+// - functions of type Shell.Main as functions within the current process
+// - arbitrary non-Vanadium commands available on the underlying
+// operating system such as '/bin/cp', 'bash' etc.
+// - arbtirary Vanadium commands available on the underlying
+// operating system such as precompiled Vanadium services.
//
-// A simple protocol must be followed by all commands, namely, they
-// should wait for their stdin stream to be closed before exiting. The
-// caller can then coordinate with any command by writing to that stdin
-// stream and reading responses from the stdout stream, and it can close
-// stdin when it's ready for the command to exit using the CloseStdin method
-// on the command's handle.
-//
+// The first two types require that the function to be executed is compiled
+// into the binary executing the calls to the Shell. These functions
+// are registered with a single, per-process, registry.
// The signature of the function that implements the command is the
// same for both types of command and is defined by the Main function type.
// In particular stdin, stdout and stderr are provided as parameters, as is
// a map representation of the shell's environment.
//
+// The second two types allow for arbitrary binaries to be executed. The
+// distinction between a Vanadium and non-Vanadium command is that the
+// Vanadium command implements the protocol used by v.io/x/ref/lib/exec
+// package to synchronise between the parent and child processes and to share
+// information such as the ConfigKey key,value store supported by the Shell,
+// a shared secret etc.
+//
+// The registry provides the following functions:
+// - RegisterChild: generally called from an init function to register a
+// shell.Main to be executed in a subprocess by fork/exec'ing the calling
+// process.
+// - Dispatch: which must be called in the child process to lookup the
+// requested function in the registry and to invoke it. This will typically
+// be called from a TestMain. modules.IsModulesChildProcess can be used
+// to determine if the calling process is a child started via this package.
+// - DispatchAndExit: essentially the same as Dispatch but intended to be
+// called as the first thing in a main function.
+// - RegisterFunction: for an in-process function that will be invoked
+// via function call.
+//
+// The v23 tool can automate generation of TestMain and calls to RegisterChild,
+// and RegisterFunction. Adding the comment below to a test file will
+// generate the appropriate code.
+//
+// //go:generate v23 test generate .
+//
+// Use 'v23 test generate --help' to get a complete explanation.
+//
+// In all cases commands are started by invoking the StartWithOpts method
+// on the Shell with the name of the command to run. An instance of the Handle
+// interface is returned which can be used to interact with the function
+// or subprocess, and in particular to read/write data from/to it using io
+// channels that follow the stdin, stdout, stderr convention. The StartOpts
+// struct is used to control the detailed behaviour of each such invocation.
+// Various helper functions are provided both for creating appropriate
+// instances of StartOpts and for common uses of StartWithOpts.
+//
+// Each successful call to StartWithOpts returns a handle representing
+// the running command. This handle can be used to gain access to that
+// command's stdin, stdout, stderr and to request or synchronize with
+// its termination via the Shutdown method. The Shutdown method can
+// optionally be used to read any remaining output from the commands stdout
+// and stderr.
+// The Shell maintains a record of all such handles and will call Shutdown
+// on them in LIFO order when the Shell's Cleanup method is called.
+//
+// A simple protocol must be followed by all modules.Main commands,
+// in particular, they should wait for their stdin stream to be closed
+// before exiting. The caller can then coordinate with any command by writing
+// to that stdin stream and reading responses from the stdout stream, and it
+// can close stdin when it's ready for the command to exit using the
+// CloseStdin method on the command's handle. Any binary or script that
+// follows this protocol can be used as well.
+//
// By default, every Shell created by NewShell starts a security agent
-// to manage principals for child processes. These default
-// credentials can be overridden by passing a nil context to NewShell
-// then specifying VeyronCredentials in the environment provided as a
-// parameter to the Start method.
+// to manage principals for child processes. These default credentials
+// can be overridden by passing a nil context to NewShell then specifying
+// VeyronCredentials in the environment provided as a parameter to the
+// StartWithOpts method. It is also possible to specify custom credentials
+// via StartOpts.
+//
+// Interacting with Commands:
+//
+// Handle.Stdout(), Stdin(), Stderr():
+// StartWithOpts returns a Handle which can be used to interact with the
+// running command. In particular, its Stdin() and Stdout() methods give
+// access to the running process' corresponding stdin and stdout and hence
+// can be used to communicate with it. Stderr is handled differently and is
+// configured so that the child's stderr is written to a log file rather
+// than a pipe. This is in order to maximise the liklihood of capturing
+// stderr output from a crashed child process.
+//
+// Handle.Shutdown(stdout, stderr io.Writer):
+// The Shutdown method is used to gracefully shutdown a command and to
+// synchronise with its termination. In particular, Shutdown can be used
+// to read any unread output from the command's stdout and stderr. Note that
+// since Stderr is buffered to a file, Shutdown is able to return the entire
+// contents of that file. This is useful for debugging misbehaving/crashing
+// child processes.
+//
+// Shell.Cleanup(stdout, stderr io.Writer):
+// The Shell keeps track of all Handles that it has issued and in particular
+// if Shutdown (or Forget) have not been called, it will call Shutdown for
+// each such Handle in LIFO order. This ensures that all commands will be
+// Shutdown even if the developer does not explicitly take care to do so
+// for every invocation.
+//
+// Pipes:
+// StartWithOpts allows the caller to pass an io.Reader to the command
+// (StartOpts.Stdin) for it to read from, rather than creating a new pipe
+// internally. This makes it possible to connect the output of one
+// command to the input of another directly.
+//
+// Command Line Arguments:
+// The arguments passed in calls to Start are appended to any system required
+// ones (e.g. for propagating test timeouts, verbosity etc) and the child
+// process will call the command with the result of flag.Args(). In this way
+// the caller can provide flags used by libraries in the child process
+// as well as those specific to the command and the command will only
+// receive the args specific to it. The usual "--" convention can be
+// used to override this default behaviour.
+//
+// Caveats:
+//
+// Handle.Shutdown assumes that the child command/process will terminate
+// when its stdin stream is closed. This assumption is unlikely to be valid
+// for 'external' commands (e.g. /bin/cp) and in these cases Kill or some other
+// application specific mechanism will need to be used.
package modules
import (
- "errors"
"fmt"
"io"
"io/ioutil"
@@ -46,12 +142,12 @@
"syscall"
"time"
- "v.io/v23/security"
-
"v.io/v23"
"v.io/v23/context"
+ "v.io/v23/security"
"v.io/x/ref/lib/exec"
"v.io/x/ref/lib/flags/consts"
+ "v.io/x/ref/lib/testutil/expect"
"v.io/x/ref/security/agent"
"v.io/x/ref/security/agent/keymgr"
)
@@ -59,22 +155,35 @@
const (
shellBlessingExtension = "test-shell"
childBlessingExtension = "child"
+
+ defaultStartTimeout = time.Minute
+ defaultShutdownTimeout = time.Minute
+ defaultExpectTimeout = time.Minute
)
+var defaultStartOpts = StartOpts{
+ StartTimeout: defaultStartTimeout,
+ ShutdownTimeout: defaultShutdownTimeout,
+ ExpectTimeout: defaultExpectTimeout,
+ ExecProtocol: true,
+}
+
// Shell represents the context within which commands are run.
type Shell struct {
- mu sync.Mutex
- env map[string]string
- handles map[Handle]struct{}
+ mu sync.Mutex
+ env map[string]string
+ handles map[Handle]struct{}
+ lifoHandles []Handle
+ defaultStartOpts StartOpts
// tmpCredDir is the temporary directory created by this
// shell. This must be removed when the shell is cleaned up.
- tempCredDir string
- startTimeout, waitTimeout time.Duration
- config exec.Config
- principal security.Principal
- agent *keymgr.Agent
- ctx *context.T
- cancelCtx func()
+ tempCredDir string
+ config exec.Config
+ principal security.Principal
+ agent *keymgr.Agent
+ ctx *context.T
+ sessionVerbosity bool
+ cancelCtx func()
}
// NewShell creates a new instance of Shell.
@@ -86,11 +195,10 @@
// principal blessed by the default blessings of ctx's principal.
func NewShell(ctx *context.T, p security.Principal) (*Shell, error) {
sh := &Shell{
- env: make(map[string]string),
- handles: make(map[Handle]struct{}),
- startTimeout: time.Minute,
- waitTimeout: 10 * time.Second,
- config: exec.NewConfig(),
+ env: make(map[string]string),
+ handles: make(map[Handle]struct{}),
+ config: exec.NewConfig(),
+ defaultStartOpts: defaultStartOpts,
}
if ctx == nil {
return sh, nil
@@ -115,6 +223,28 @@
return sh, nil
}
+// NewExpectShell creates a new instance of Shell as per NewShell, but with
+// the default StartOpts set to include the specified expect.Testing parameter.
+func NewExpectShell(ctx *context.T, p security.Principal, t expect.Testing, verbosity bool) (*Shell, error) {
+ sh, err := NewShell(ctx, p)
+ if err != nil {
+ return nil, err
+ }
+ sh.sessionVerbosity = verbosity
+ sh.SetDefaultStartOpts(DefaultStartOpts().WithSessions(t, time.Minute))
+ return sh, nil
+}
+
+// DefaultStartOpts returns the current StartOpts stored with the Shell.
+func (sh *Shell) DefaultStartOpts() StartOpts {
+ return sh.defaultStartOpts
+}
+
+// SetDefaultStartOpts sets the default StartOpts stored with the Shell.
+func (sh *Shell) SetDefaultStartOpts(opts StartOpts) {
+ sh.defaultStartOpts = opts
+}
+
// CustomCredentials encapsulates a Principal which can be shared with
// one or more processes run by a Shell.
type CustomCredentials struct {
@@ -147,7 +277,7 @@
return fd, nil
}
-// NewCustomCredentials creates a new Principal for StartWithCredentials.
+// NewCustomCredentials creates a new Principal for StartWithOpts.
// Returns nil if the shell is not managing principals.
func (sh *Shell) NewCustomCredentials() (cred *CustomCredentials, err error) {
// Create child principal.
@@ -216,8 +346,110 @@
return registry.help(command)
}
-// Start starts the specified command, it returns a Handle which can be
-// used for interacting with that command.
+// Start is shorthand for StartWithOpts(sh.DefaultStartOpts(), ...)
+func (sh *Shell) Start(name string, env []string, args ...string) (Handle, error) {
+ return sh.StartWithOpts(sh.DefaultStartOpts(), env, name, args...)
+}
+
+// StartOpts represents the options that can be passed to the
+// StartWithOpts method.
+type StartOpts struct {
+ // Error is set when creating/intializing instances of StartOpts
+ // via one of the factory methods and returned when StartWithOpts
+ // is called. This allows usage of of the form:
+ //
+ // err := sh.StartWithOpts(sh.DefaultStartOpts()...)
+ //
+ // as opposed to:
+ //
+ // opts, err := sh.DefaultStartOpts(....)
+ // if err != nil {
+ // panic(...)
+ // }
+ // sh.StartWithOpts(opts, ....)
+ Error error
+
+ // Stdin, if non-nil, will be used as the stdin for the child process.
+ // If this option is set, then the Stdin() method on the returned Handle
+ // will return nil. The client of this API maintains ownership of stdin
+ // and must close it, i.e. the shell will not do so.
+ Stdin io.Reader
+ // Credentials, if non-nil, will be used as the credentials for the
+ // child process. If the creds are nil or the shell is not managing
+ // principals, the credentials are ignored.
+ Credentials *CustomCredentials
+ // ExecProtocol indicates whether the child process is expected to
+ // implement the v.io/x.ref/lib/exec parent/child protocol.
+ // It should be set to false when running non-vanadium commands
+ // (e.g. /bin/cp).
+ ExecProtocol bool
+ // External indicates if the command is an external process rather than
+ // a Shell.Main function.
+ External bool
+ // StartTimeout specifies the amount of time to wait for the
+ // child process to signal its correct intialization for Vanadium
+ // processes that implement the exec parent/child protocol. It has no
+ // effect if External is set to true.
+ StartTimeout time.Duration
+ // ShutdownTimeout specifics the amount of time to wait for the child
+ // process to exit when the Shutdown method is called on that
+ // child's handle.
+ ShutdownTimeout time.Duration
+ // ExpectTesting is used when creating an instance of expect.Session
+ // to embed in Handle.
+ ExpectTesting expect.Testing
+ // ExpectTimeout is the timeout to use with expect.Session.
+ ExpectTimeout time.Duration
+}
+
+// DefaultStartOpts returns an instance of Startops with the current default
+// values. The defaults have values for timeouts, no credentials
+// (StartWithOpts will then create credentials each time it is called),
+// and with ExecProtocol set to true.
+// This is expected to be the common use case.
+func DefaultStartOpts() StartOpts {
+ return defaultStartOpts
+}
+
+// WithCustomCredentials returns an instance of StartOpts with the specified
+// credentials. All other options are set to the current defaults.
+func (opts StartOpts) WithCustomCredentials(creds *CustomCredentials) StartOpts {
+ opts.Credentials = creds
+ return opts
+}
+
+// WithSessions returns a copy of opts with the specified expect.Testing and
+// associated timeout.
+func (opts StartOpts) WithSessions(t expect.Testing, timeout time.Duration) StartOpts {
+ opts.ExpectTesting = t
+ opts.ExpectTimeout = timeout
+ return opts
+}
+
+// WithStdin returns a copy of opts with the specified Stdin io.Reader.
+func (opts StartOpts) WithStdin(stdin io.Reader) StartOpts {
+ opts.Stdin = stdin
+ return opts
+}
+
+// NoExecCommand returns a copy of opts with the External option
+// enabled and ExecProtocol disabled.
+func (opts StartOpts) NoExecCommand() StartOpts {
+ opts.External = true
+ opts.ExecProtocol = false
+ return opts
+}
+
+// ExternalCommand returns a copy of StartOpts with the
+// External option enabled.
+func (opts StartOpts) ExternalCommand() StartOpts {
+ opts.External = true
+ return opts
+}
+
+// StartWithOpts starts the specified command according to the supplied
+// StartOpts and returns a Handle which can be used for interacting with
+// that command.
//
// The environment variables for the command are set by merging variables
// from the OS environment, those in this Shell and those provided as a
@@ -226,8 +458,8 @@
// and agent FdEnvVar variables will never use the value from the Shell or OS.
//
// If the shell is managing principals, the command is configured to
-// connect to the shell's agent.
-// To override this, or if the shell is not managing principals, set
+// connect to the shell's agent. Custom credentials may be specified
+// via StartOpts. If the shell is not managing principals, set
// the VeyronCredentials environment variable in the 'env' parameter.
//
// The Shell tracks all of the Handles that it creates so that it can shut
@@ -235,100 +467,62 @@
// error is returned, in which case it may be used to retrieve any output
// from the failed command.
//
-// Commands must have already been registered using RegisterFunction
-// or RegisterChild.
-func (sh *Shell) Start(name string, env []string, args ...string) (Handle, error) {
- creds, err := sh.NewChildCredentials()
- if err != nil {
- return nil, err
- }
- return sh.StartWithCredentials(name, env, creds, args...)
-}
-
-// StartWithCredentials starts a command with the specified credentials, it returns a Handle which can be
-// used for interacting with that command. The specified Principal is used directly without modification.
+// StartWithOpts will return a valid handle for errors that occur during the
+// child processes startup process. It is thus possible to call Shutdown
+// to obtain the error output. Handle will be nil if the error is due to
+// some other reason, such as failure to create pipes/files before starting
+// the child process. A common use will therefore be:
//
-// If the creds is nil or the shell is not managing principals, the credentials are ignored.
-// See 'Start' for the meaning of the other parameters and return values.
-func (sh *Shell) StartWithCredentials(name string, env []string, creds *CustomCredentials, args ...string) (Handle, error) {
- cmd := registry.getCommand(name)
- if cmd == nil {
+// h, err := sh.Start(env, "/bin/echo", "hello")
+// if err != nil {
+// if h != nil {
+// h.Shutdown(nil,os.Stderr)
+// }
+// t.Fatal(err)
+// }
+func (sh *Shell) StartWithOpts(opts StartOpts, env []string, name string, args ...string) (Handle, error) {
+ if opts.Error != nil {
+ return nil, opts.Error
+ }
+ var desc *commandDesc
+ if opts.External {
+ desc = registry.getExternalCommand(name)
+ } else if desc = registry.getCommand(name); desc == nil {
return nil, fmt.Errorf("%s: not registered", name)
}
+ if opts.Credentials == nil {
+ var err error
+ opts.Credentials, err = sh.NewChildCredentials()
+ if err != nil {
+ return nil, err
+ }
+ }
+ cmd := desc.factory()
expanded := append([]string{name}, sh.expand(args...)...)
- c := cmd.factory()
- h, err := sh.startCommand(c, env, creds, expanded...)
- if err != nil {
- // If the error is a timeout, then h can be used to recover
- // any output from the process.
- return h, err
- }
-
- if err := h.WaitForReady(sh.waitTimeout); err != nil {
- return h, err
- }
- return h, nil
-}
-
-// StartExternalCommand starts the specified external, non-Vanadium, command;
-// it returns a Handle which can be used for interacting with that command.
-// A non-Vanadium command does not implement the parent-child protocol
-// implemented by the veyron/lib/exec library, thus this method can be used
-// to start any command (e.g. /bin/cp).
-//
-// StartExternalCommand takes an io.Reader which, if non-nil, will be used as
-// the stdin for the child process. If this parameter is supplied, then the
-// Stdin() method on the returned Handle will return nil. The client of this
-// API maintains ownership of stdin and must close it, i.e. the shell
-// will not do so.
-// The env and args parameters are handled in the same way as Start.
-func (sh *Shell) StartExternalCommand(stdin io.Reader, env []string, args ...string) (Handle, error) {
- if len(args) == 0 {
- return nil, errors.New("no arguments specified to StartExternalCommand")
- }
- c := newExecHandleForExternalCommand(args[0], stdin)
- expanded := sh.expand(args...)
- creds, err := sh.NewChildCredentials()
- if err != nil {
- return nil, err
- }
- return sh.startCommand(c, env, creds, expanded...)
-}
-
-func (sh *Shell) startCommand(c command, env []string, creds *CustomCredentials, args ...string) (Handle, error) {
cenv, err := sh.setupCommandEnv(env)
if err != nil {
return nil, err
}
var p *os.File
- if creds != nil {
- p, err = creds.File()
+ if opts.Credentials != nil {
+ p, err = opts.Credentials.File()
if err != nil {
return nil, err
}
}
- h, err := c.start(sh, p, cenv, args...)
+ h, err := cmd.start(sh, p, &opts, cenv, expanded...)
if err != nil {
- return nil, err
+ return h, err
}
sh.mu.Lock()
sh.handles[h] = struct{}{}
+ sh.lifoHandles = append(sh.lifoHandles, h)
sh.mu.Unlock()
return h, nil
}
-// SetStartTimeout sets the timeout for starting subcommands.
-func (sh *Shell) SetStartTimeout(d time.Duration) {
- sh.startTimeout = d
-}
-
-// SetWaitTimeout sets the timeout for waiting on subcommands to complete.
-func (sh *Shell) SetWaitTimeout(d time.Duration) {
- sh.waitTimeout = d
-}
-
// CommandEnvelope returns the command line and environment that would be
// used for running the subprocess or function if it were started with the
// specifed arguments.
@@ -429,18 +623,57 @@
// Shutdown routines are executed is not defined.
func (sh *Shell) Cleanup(stdout, stderr io.Writer) error {
sh.mu.Lock()
- handles := make(map[Handle]struct{})
- for k, v := range sh.handles {
- handles[k] = v
+ verbose := sh.sessionVerbosity
+ sh.mu.Unlock()
+
+ writeMsg := func(format string, args ...interface{}) {
+ if !verbose {
+ return
+ }
+ if stderr != nil {
+ fmt.Fprintf(stderr, format, args...)
+ }
+ }
+
+ if verbose {
+ writeMsg("---- Shell Cleanup ----\n")
+ }
+ sh.mu.Lock()
+ handles := make([]Handle, 0, len(sh.lifoHandles))
+ for _, h := range sh.lifoHandles {
+ if _, present := sh.handles[h]; present {
+ handles = append(handles, h)
+ }
}
sh.handles = make(map[Handle]struct{})
+ sh.lifoHandles = nil
sh.mu.Unlock()
var err error
- for k, _ := range handles {
- cerr := k.Shutdown(stdout, stderr)
+ for i := len(handles); i > 0; i-- {
+ h := handles[i-1]
+ switch v := h.(type) {
+ case *functionHandle:
+ writeMsg("---- Cleanup calling Shutdown on function %q\n", v.name)
+ case *execHandle:
+ writeMsg("---- Cleanup calling Shutdown on command %q\n", v.name)
+ }
+ cerr := h.Shutdown(stdout, stderr)
if cerr != nil {
err = cerr
}
+ fn := func() string {
+ if cerr == nil {
+ return ": done"
+ } else {
+ return ": error: " + err.Error()
+ }
+ }
+ switch v := h.(type) {
+ case *functionHandle:
+ writeMsg("---- Shutdown on function %q%s\n", v.name, fn())
+ case *execHandle:
+ writeMsg("---- Shutdown on command %q%s\n", v.name, fn())
+ }
}
if sh.cancelCtx != nil {
@@ -475,8 +708,27 @@
return r, nil
}
+// ExpectSession is a subset of v.io/x/ref/lib/testutil/expect.Session's methods
+// that are embedded in Handle.
+type ExpectSession interface {
+ Expect(expected string)
+ ExpectEOF() error
+ ExpectRE(pattern string, n int) [][]string
+ ExpectSetEventuallyRE(expected ...string) [][]string
+ ExpectSetRE(expected ...string) [][]string
+ ExpectVar(name string) string
+ Expectf(format string, args ...interface{})
+ ReadAll() (string, error)
+ ReadLine() string
+ SetVerbosity(bool)
+ Failed() bool
+ Error() error
+}
+
// Handle represents a running command.
type Handle interface {
+ ExpectSession
+
// Stdout returns a reader to the running command's stdout stream.
Stdout() io.Reader
@@ -503,16 +755,11 @@
// Pid returns the pid of the process running the command
Pid() int
-
- // WaitForReady waits until the child process signals to us that it is
- // ready. If this does not occur within the given timeout duration, a
- // timeout error is returned.
- WaitForReady(timeout time.Duration) error
}
// command is used to abstract the implementations of inprocess and subprocess
// commands.
type command interface {
envelope(sh *Shell, env []string, args ...string) ([]string, []string)
- start(sh *Shell, agent *os.File, env []string, args ...string) (Handle, error)
+ start(sh *Shell, agent *os.File, opts *StartOpts, env []string, args ...string) (Handle, error)
}
diff --git a/lib/signals/signals_test.go b/lib/signals/signals_test.go
index b04b54f..b74bbc0 100644
--- a/lib/signals/signals_test.go
+++ b/lib/signals/signals_test.go
@@ -44,7 +44,6 @@
func program(stdin io.Reader, stdout io.Writer, signals ...os.Signal) {
ctx, shutdown := testutil.InitForTest()
-
closeStopLoop := make(chan struct{})
go stopLoop(v23.GetAppCycle(ctx).Stop, stdin, closeStopLoop)
wait := ShutdownOnSignals(ctx, signals...)
diff --git a/lib/signals/v23_internal_test.go b/lib/signals/v23_internal_test.go
index 51d914f..3fbd22d 100644
--- a/lib/signals/v23_internal_test.go
+++ b/lib/signals/v23_internal_test.go
@@ -22,7 +22,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/lib/testutil/v23tests/binary.go b/lib/testutil/v23tests/binary.go
new file mode 100644
index 0000000..2997735
--- /dev/null
+++ b/lib/testutil/v23tests/binary.go
@@ -0,0 +1,140 @@
+package v23tests
+
+import (
+ "bytes"
+ "io"
+ "os"
+ "path"
+ "strings"
+
+ "v.io/x/lib/vlog"
+
+ "v.io/x/ref/lib/modules"
+)
+
+// Binary represents an executable program that will be executed during a
+// test. A binary may be invoked multiple times by calling Start, each call
+// will return a new Invocation.
+//
+// Binary instances are typically obtained from a T by calling BuldV23Pkg,
+// BuildGoPkg (for Vanadium and other Go binaries) or BinaryFromPath (to
+// start binaries that are already present on the system).
+type Binary struct {
+ // The environment to which this binary belongs.
+ env *T
+
+ // The path to the binary.
+ path string
+
+ // StartOpts
+ opts modules.StartOpts
+
+ // Environment variables that will be used when creating invocations
+ // via Start.
+ envVars []string
+}
+
+func (b *Binary) cleanup() {
+ binaryDir := path.Dir(b.path)
+ vlog.Infof("cleaning up %s", binaryDir)
+ if err := os.RemoveAll(binaryDir); err != nil {
+ vlog.Infof("WARNING: RemoveAll(%s) failed (%v)", binaryDir, err)
+ }
+}
+
+// StartOpts returns the current the StartOpts
+func (b *Binary) StartOpts() modules.StartOpts {
+ return b.opts
+}
+
+// Path returns the path to the binary.
+func (b *Binary) Path() string {
+ return b.path
+}
+
+// Start starts the given binary with the given arguments.
+func (b *Binary) Start(args ...string) *Invocation {
+ return b.start(1, args...)
+}
+
+func (b *Binary) start(skip int, args ...string) *Invocation {
+ vlog.Infof("%s: starting %s %s", Caller(skip+1), b.Path(), strings.Join(args, " "))
+ opts := b.opts
+ if opts.Credentials == nil {
+ opts.Credentials, opts.Error = b.env.shell.NewChildCredentials()
+ }
+ handle, err := b.env.shell.StartWithOpts(opts, b.envVars, b.Path(), args...)
+ if err != nil {
+ if handle != nil {
+ vlog.Infof("%s: start failed", Caller(skip+1))
+ handle.Shutdown(nil, os.Stderr)
+ }
+ // TODO(cnicolaou): calling Fatalf etc from a goroutine often leads
+ // to deadlock. Need to make sure that we handle this here. Maybe
+ // it's best to just return an error? Or provide a StartWithError
+ // call for use from goroutines.
+ b.env.Fatalf("%s: StartWithOpts(%v, %v) failed: %v", Caller(skip+1), b.Path(), strings.Join(args, ", "), err)
+ }
+ vlog.Infof("started PID %d\n", handle.Pid())
+ inv := &Invocation{
+ env: b.env,
+ path: b.path,
+ args: args,
+ shutdownErr: errNotShutdown,
+ Handle: handle,
+ }
+ b.env.appendInvocation(inv)
+ return inv
+}
+
+func (b *Binary) run(args ...string) string {
+ inv := b.start(2, args...)
+ var stdout, stderr bytes.Buffer
+ err := inv.Wait(&stdout, &stderr)
+ if err != nil {
+ a := strings.Join(args, ", ")
+ b.env.Fatalf("%s: Run(%s): failed: %v: \n%s\n", Caller(2), a, err, stderr.String())
+ }
+ return strings.TrimRight(stdout.String(), "\n")
+}
+
+// Run runs the binary with the specified arguments to completion. On
+// success it returns the contents of stdout, on failure it terminates the
+// test with an error message containing the error and the contents of
+// stderr.
+func (b *Binary) Run(args ...string) string {
+ return b.run(args...)
+}
+
+// WithStdin returns a copy of this binary that, when Start is called,
+// will read its input from the given reader. Once the reader returns
+// EOF, the returned invocation's standard input will be closed (see
+// Invocation.CloseStdin).
+func (b *Binary) WithStdin(r io.Reader) *Binary {
+ opts := b.opts
+ opts.Stdin = r
+ return b.WithStartOpts(opts)
+}
+
+// WithEnv returns a copy of this binary that, when Start is called, will use
+// the given environment variables. Each environment variable should be
+// in "key=value" form. For example:
+//
+// bin.WithEnv("EXAMPLE_ENV=/tmp/something").Start(...)
+func (b *Binary) WithEnv(env ...string) *Binary {
+ newBin := *b
+ newBin.envVars = env
+ return &newBin
+}
+
+// WithStartOpts eturns a copy of this binary that, when Start is called, will
+// use the given StartOpts.
+//
+// bin.WithStartOpts(opts).Start(...)
+// or
+// bin.WithStartOpts().Run(...)
+func (b *Binary) WithStartOpts(opts modules.StartOpts) *Binary {
+ newBin := *b
+ newBin.opts = opts
+ return &newBin
+}
diff --git a/lib/testutil/v23tests/invocation.go b/lib/testutil/v23tests/invocation.go
new file mode 100644
index 0000000..8879aa3
--- /dev/null
+++ b/lib/testutil/v23tests/invocation.go
@@ -0,0 +1,108 @@
+package v23tests
+
+import (
+ "bytes"
+ "container/list"
+ "io"
+ "syscall"
+
+ "v.io/x/lib/vlog"
+
+ "v.io/x/ref/lib/modules"
+)
+
+// Invocation represents a single invocation of a Binary.
+//
+// Any bytes written by the invocation to its standard error may be recovered
+// using the Wait or WaitOrDie functions.
+//
+// For example:
+// bin := env.BinaryFromPath("/bin/bash")
+// inv := bin.Start("-c", "echo hello world 1>&2")
+// var stderr bytes.Buffer
+// inv.WaitOrDie(nil, &stderr)
+// // stderr.Bytes() now contains 'hello world\n'
+type Invocation struct {
+ // The handle to the process that was run when this invocation was started.
+ modules.Handle
+
+ // The environment to which this invocation belongs.
+ env *T
+
+ // The element representing this invocation in the list of
+ // invocations stored in the environment
+ el *list.Element
+
+ // The path of the binary used for this invocation.
+ path string
+
+ // The args the binary was started with
+ args []string
+
+ // True if the process has been shutdown
+ hasShutdown bool
+
+ // The error, if any, as determined when the invocation was
+ // shutdown. It must be set to a default initial value of
+ // errNotShutdown rather than nil to allow us to distinguish between
+ // a successful shutdown or an error.
+ shutdownErr error
+}
+
+// Path returns the path to the binary that was used for this invocation.
+func (i *Invocation) Path() string {
+ return i.path
+}
+
+// Exists returns true if the invocation still exists.
+func (i *Invocation) Exists() bool {
+ return syscall.Kill(i.Handle.Pid(), 0) == nil
+}
+
+// Kill sends the given signal to this invocation. It is up to the test
+// author to decide whether failure to deliver the signal is fatal to
+// the test.
+func (i *Invocation) Kill(sig syscall.Signal) error {
+ pid := i.Handle.Pid()
+ vlog.VI(1).Infof("sending signal %v to PID %d", sig, pid)
+ return syscall.Kill(pid, sig)
+}
+
+// Output reads the invocation's stdout until EOF and then returns what
+// was read as a string.
+func (i *Invocation) Output() string {
+ buf := bytes.Buffer{}
+ _, err := buf.ReadFrom(i.Stdout())
+ if err != nil {
+ i.env.Fatalf("%s: ReadFrom() failed: %v", Caller(1), err)
+ }
+ return buf.String()
+}
+
+// Wait waits for this invocation to finish. If either stdout or stderr
+// is non-nil, any remaining unread output from those sources will be
+// written to the corresponding writer. The returned error represents
+// the exit status of the underlying command.
+func (i *Invocation) Wait(stdout, stderr io.Writer) error {
+ err := i.Handle.Shutdown(stdout, stderr)
+ i.hasShutdown = true
+ i.shutdownErr = err
+ return err
+}
+
+// WaitOrDie waits for this invocation to finish. If either stdout or stderr
+// is non-nil, any remaining unread output from those sources will be
+// written to the corresponding writer. If the underlying command
+// exited with anything but success (exit status 0), this function will
+// cause the current test to fail.
+func (i *Invocation) WaitOrDie(stdout, stderr io.Writer) {
+ if err := i.Wait(stdout, stderr); err != nil {
+ i.env.Fatalf("%s: FATAL: Wait() for pid %d failed: %v", Caller(1), i.Handle.Pid(), err)
+ }
+}
+
+// Environment returns the instance of the test environment that this
+// invocation was from.
+func (i *Invocation) Environment() *T {
+ return i.env
+}
diff --git a/lib/testutil/v23tests/v23tests.go b/lib/testutil/v23tests/v23tests.go
index 292eb60..6fa05b7 100644
--- a/lib/testutil/v23tests/v23tests.go
+++ b/lib/testutil/v23tests/v23tests.go
@@ -71,11 +71,8 @@
package v23tests
import (
- "bytes"
- "container/list"
"errors"
"fmt"
- "io"
"io/ioutil"
"os"
"os/exec"
@@ -93,7 +90,6 @@
"v.io/x/ref/lib/modules"
"v.io/x/ref/lib/testutil"
- "v.io/x/ref/lib/testutil/expect"
tsecurity "v.io/x/ref/lib/testutil/security"
"v.io/x/ref/security/agent"
)
@@ -142,105 +138,8 @@
invocations []*Invocation
}
-// Binary represents an executable program that will be executed during a
-// test. A binary may be invoked multiple times by calling Start, each call
-// will return a new Invocation.
-//
-// Binary instances are typically obtained from a T by calling BuildGoPkg
-// (for Vanadium and other Go binaries) or BinaryFromPath (to start binaries
-// that are already present on the system).
-type Binary struct {
- // The environment to which this binary belongs.
- env *T
-
- // The path to the binary.
- path string
-
- // Environment variables that will be used when creating invocations
- // via Start.
- envVars []string
-
- // The reader who is supplying the bytes we're going to send to our stdin.
- inputReader io.Reader
-}
-
-// Invocation represents a single invocation of a Binary.
-//
-// Any bytes written by the invocation to its standard error may be recovered
-// using the Wait or WaitOrDie functions.
-//
-// For example:
-// bin := env.BinaryFromPath("/bin/bash")
-// inv := bin.Start("-c", "echo hello world 1>&2")
-// var stderr bytes.Buffer
-// inv.WaitOrDie(nil, &stderr)
-// // stderr.Bytes() now contains 'hello world\n'
-type Invocation struct {
- *expect.Session
-
- // The environment to which this invocation belongs.
- env *T
-
- // The handle to the process that was run when this invocation was started.
- handle modules.Handle
-
- // The element representing this invocation in the list of
- // invocations stored in the environment
- el *list.Element
-
- // The path of the binary used for this invocation.
- path string
-
- // The args the binary was started with
- args []string
-
- // True if the process has been shutdown
- hasShutdown bool
-
- // The error, if any, as determined when the invocation was
- // shutdown. It must be set to a default initial value of
- // errNotShutdown rather than nil to allow us to distinguish between
- // a successful shutdown or an error.
- shutdownErr error
-}
-
var errNotShutdown = errors.New("has not been shutdown")
-// Stdin returns this invocations Stdin stream.
-func (i *Invocation) Stdin() io.Writer {
- return i.handle.Stdin()
-}
-
-// CloseStdin closes the write-side of the pipe to the invocation's
-// standard input.
-func (i *Invocation) CloseStdin() {
- i.handle.CloseStdin()
-}
-
-// Stdout returns this invocations Stdout stream.
-func (i *Invocation) Stdout() io.Reader {
- return i.handle.Stdout()
-}
-
-// Path returns the path to the binary that was used for this invocation.
-func (i *Invocation) Path() string {
- return i.path
-}
-
-// Exists returns true if the invocation still exists.
-func (i *Invocation) Exists() bool {
- return syscall.Kill(i.handle.Pid(), 0) == nil
-}
-
-// Sends the given signal to this invocation. It is up to the test
-// author to decide whether failure to deliver the signal is fatal to
-// the test.
-func (i *Invocation) Kill(sig syscall.Signal) error {
- pid := i.handle.Pid()
- vlog.VI(1).Infof("sending signal %v to PID %d", sig, pid)
- return syscall.Kill(pid, sig)
-}
-
// Caller returns a string of the form <filename>:<lineno> for the
// caller specified by skip, where skip is as per runtime.Caller.
func Caller(skip int) string {
@@ -248,108 +147,16 @@
return fmt.Sprintf("%s:%d", filepath.Base(file), line)
}
-// Output reads the invocation's stdout until EOF and then returns what
-// was read as a string.
-func (i *Invocation) Output() string {
- buf := bytes.Buffer{}
- _, err := buf.ReadFrom(i.Stdout())
- if err != nil {
- i.env.Fatalf("%s: ReadFrom() failed: %v", Caller(1), err)
- }
- return buf.String()
-}
-
-// Wait waits for this invocation to finish. If either stdout or stderr
-// is non-nil, any remaining unread output from those sources will be
-// written to the corresponding writer. The returned error represents
-// the exit status of the underlying command.
-func (i *Invocation) Wait(stdout, stderr io.Writer) error {
- err := i.handle.Shutdown(stdout, stderr)
- i.hasShutdown = true
- i.shutdownErr = err
- return err
-}
-
-// Wait waits for this invocation to finish. If either stdout or stderr
-// is non-nil, any remaining unread output from those sources will be
-// written to the corresponding writer. If the underlying command
-// exited with anything but success (exit status 0), this function will
-// cause the current test to fail.
-func (i *Invocation) WaitOrDie(stdout, stderr io.Writer) {
- if err := i.Wait(stdout, stderr); err != nil {
- i.env.Fatalf("%s: FATAL: Wait() for pid %d failed: %v", Caller(1), i.handle.Pid(), err)
- }
-}
-
-// Environment returns the instance of the test environment that this
-// invocation was from.
-func (i *Invocation) Environment() *T {
- return i.env
-}
-
-func (b *Binary) cleanup() {
- binaryDir := path.Dir(b.path)
- vlog.Infof("cleaning up %s", binaryDir)
- if err := os.RemoveAll(binaryDir); err != nil {
- vlog.Infof("WARNING: RemoveAll(%s) failed (%v)", binaryDir, err)
- }
-}
-
-// Path returns the path to the binary.
-func (b *Binary) Path() string {
- return b.path
-}
-
-// Start starts the given binary with the given arguments.
-func (b *Binary) Start(args ...string) *Invocation {
- return b.start(1, args...)
-}
-
-func (b *Binary) start(skip int, args ...string) *Invocation {
- vlog.Infof("%s: starting %s %s", Caller(skip+1), b.Path(), strings.Join(args, " "))
- handle, err := b.env.shell.StartExternalCommand(b.inputReader, b.envVars, append([]string{b.Path()}, args...)...)
- if err != nil {
- // TODO(cnicolaou): calling Fatalf etc from a goroutine often leads
- // to deadlock. Need to make sure that we handle this here. Maybe
- // it's best to just return an error? Or provide a StartWithError
- // call for use from goroutines.
- b.env.Fatalf("%s: StartExternalCommand(%v, %v) failed: %v", Caller(skip+1), b.Path(), strings.Join(args, ", "), err)
- }
- vlog.Infof("started PID %d\n", handle.Pid())
- inv := &Invocation{
- env: b.env,
- handle: handle,
- path: b.path,
- args: args,
- shutdownErr: errNotShutdown,
- Session: expect.NewSession(b.env, handle.Stdout(), 5*time.Minute),
- }
- b.env.appendInvocation(inv)
- return inv
-}
-
-func (b *Binary) run(args ...string) string {
- inv := b.start(2, args...)
- var stdout, stderr bytes.Buffer
- err := inv.Wait(&stdout, &stderr)
- if err != nil {
- a := strings.Join(args, ", ")
- b.env.Fatalf("%s: Run(%s): failed: %v: \n%s\n", Caller(2), a, err, stderr.String())
- }
- return strings.TrimRight(stdout.String(), "\n")
-}
-
-// Run runs the binary with the specified arguments to completion. On
-// success it returns the contents of stdout, on failure it terminates the
-// test with an error message containing the error and the contents of
-// stderr.
-func (b *Binary) Run(args ...string) string {
- return b.run(args...)
-}
-
// Run constructs a Binary for path and invokes Run on it.
-func (e *T) Run(path string, args ...string) string {
- return e.BinaryFromPath(path).run(args...)
+func (t *T) Run(path string, args ...string) string {
+ return t.BinaryFromPath(path).run(args...)
+}
+
+// Run constructs a Binary for path and invokes Run on it using
+// the specified StartOpts
+func (t *T) RunWithOpts(opts modules.StartOpts, path string, args ...string) string {
+ b := t.BinaryFromPath(path)
+ return b.WithStartOpts(opts).run(args...)
}
// WaitFunc is the type of the functions to be used in conjunction
@@ -370,7 +177,7 @@
// WaitFor will always run fn at least once to completion and hence it will
// hang if that first iteration of fn hangs. If this behaviour is not
// appropriate, then WaitForAsync should be used.
-func (e *T) WaitFor(fn WaitFunc, delay, timeout time.Duration) interface{} {
+func (t *T) WaitFor(fn WaitFunc, delay, timeout time.Duration) interface{} {
deadline := time.Now().Add(timeout)
for {
val, err := fn()
@@ -378,10 +185,10 @@
return val
}
if err != nil {
- e.Fatalf("%s: the WaitFunc returned an error: %v", Caller(1), err)
+ t.Fatalf("%s: the WaitFunc returned an error: %v", Caller(1), err)
}
if time.Now().After(deadline) {
- e.Fatalf("%s: timed out after %s", Caller(1), timeout)
+ t.Fatalf("%s: timed out after %s", Caller(1), timeout)
}
time.Sleep(delay)
}
@@ -389,7 +196,7 @@
// WaitForAsync is like WaitFor except that it calls fn in a goroutine
// and can timeout during the execution of fn.
-func (e *T) WaitForAsync(fn WaitFunc, delay, timeout time.Duration) interface{} {
+func (t *T) WaitForAsync(fn WaitFunc, delay, timeout time.Duration) interface{} {
resultCh := make(chan interface{})
errCh := make(chan interface{})
go func() {
@@ -408,11 +215,11 @@
}()
select {
case err := <-errCh:
- e.Fatalf("%s: the WaitFunc returned error: %v", Caller(1), err)
+ t.Fatalf("%s: the WaitFunc returned error: %v", Caller(1), err)
case result := <-resultCh:
return result
case <-time.After(timeout):
- e.Fatalf("%s: timed out after %s", Caller(1), timeout)
+ t.Fatalf("%s: timed out after %s", Caller(1), timeout)
}
return nil
}
@@ -420,30 +227,30 @@
// Pushd pushes the current working directory to the stack of
// directories, returning it as its result, and changes the working
// directory to dir.
-func (e *T) Pushd(dir string) string {
+func (t *T) Pushd(dir string) string {
cwd, err := os.Getwd()
if err != nil {
- e.Fatalf("%s: Getwd failed: %s", Caller(1), err)
+ t.Fatalf("%s: Getwd failed: %s", Caller(1), err)
}
if err := os.Chdir(dir); err != nil {
- e.Fatalf("%s: Chdir failed: %s", Caller(1), err)
+ t.Fatalf("%s: Chdir failed: %s", Caller(1), err)
}
vlog.VI(1).Infof("Pushd: %s -> %s", cwd, dir)
- e.dirStack = append(e.dirStack, cwd)
+ t.dirStack = append(t.dirStack, cwd)
return cwd
}
// Popd pops the most recent entry from the directory stack and changes
// the working directory to that directory. It returns the new working
// directory as its result.
-func (e *T) Popd() string {
- if len(e.dirStack) == 0 {
- e.Fatalf("%s: directory stack empty", Caller(1))
+func (t *T) Popd() string {
+ if len(t.dirStack) == 0 {
+ t.Fatalf("%s: directory stack empty", Caller(1))
}
- dir := e.dirStack[len(e.dirStack)-1]
- e.dirStack = e.dirStack[:len(e.dirStack)-1]
+ dir := t.dirStack[len(t.dirStack)-1]
+ t.dirStack = t.dirStack[:len(t.dirStack)-1]
if err := os.Chdir(dir); err != nil {
- e.Fatalf("%s: Chdir failed: %s", Caller(1), err)
+ t.Fatalf("%s: Chdir failed: %s", Caller(1), err)
}
vlog.VI(1).Infof("Popd: -> %s", dir)
return dir
@@ -451,34 +258,13 @@
// Caller returns a string of the form <filename>:<lineno> for the
// caller specified by skip, where skip is as per runtime.Caller.
-func (e *T) Caller(skip int) string {
+func (t *T) Caller(skip int) string {
return Caller(skip + 1)
}
-// WithStdin returns a copy of this binary that, when Start is called,
-// will read its input from the given reader. Once the reader returns
-// EOF, the returned invocation's standard input will be closed (see
-// Invocation.CloseStdin).
-func (b *Binary) WithStdin(r io.Reader) *Binary {
- newBin := *b
- newBin.inputReader = r
- return &newBin
-}
-
-// Returns a copy of this binary that, when Start is called, will use
-// the given environment variables. Each environment variable should be
-// in "key=value" form. For example:
-//
-// bin.WithEnv("EXAMPLE_ENV=/tmp/something").Start(...)
-func (b *Binary) WithEnv(env ...string) *Binary {
- newBin := *b
- newBin.envVars = env
- return &newBin
-}
-
// Principal returns the security principal of this environment.
-func (e *T) Principal() security.Principal {
- return e.principal
+func (t *T) Principal() security.Principal {
+ return t.principal
}
// Cleanup cleans up the environment, deletes all its artifacts and
@@ -487,23 +273,23 @@
// as to the state of the processes it was asked to invoke up to that
// point and optionally, if the --v23.tests.shell-on-fail flag is set
// then it will run a debug shell before cleaning up its state.
-func (e *T) Cleanup() {
- if e.Failed() {
+func (t *T) Cleanup() {
+ if t.Failed() {
if testutil.IntegrationTestsDebugShellOnError {
- e.DebugShell()
+ t.DebugSystemShell()
}
// Print out a summary of the invocations and their status.
- for i, inv := range e.invocations {
+ for i, inv := range t.invocations {
if inv.hasShutdown && inv.Exists() {
m := fmt.Sprintf("%d: %s has been shutdown but still exists: %v", i, inv.path, inv.shutdownErr)
- e.Log(m)
+ t.Log(m)
vlog.VI(1).Info(m)
vlog.VI(2).Infof("%d: %s %v", i, inv.path, inv.args)
continue
}
if inv.shutdownErr != nil {
m := fmt.Sprintf("%d: %s: shutdown status: %v", i, inv.path, inv.shutdownErr)
- e.Log(m)
+ t.Log(m)
vlog.VI(1).Info(m)
vlog.VI(2).Infof("%d: %s %v", i, inv.path, inv.args)
}
@@ -514,8 +300,8 @@
// Shut down all processes in LIFO order before attempting to delete any
// files/directories to avoid potential 'file system busy' problems
// on non-unix systems.
- for i := len(e.invocations); i > 0; i-- {
- inv := e.invocations[i-1]
+ for i := len(t.invocations); i > 0; i-- {
+ inv := t.invocations[i-1]
if inv.hasShutdown {
vlog.VI(1).Infof("V23Test.Cleanup: %q has been shutdown", inv.Path())
continue
@@ -527,13 +313,13 @@
}
vlog.VI(1).Infof("V23Test.Cleanup: all invocations taken care of.")
- if err := e.shell.Cleanup(os.Stdout, os.Stderr); err != nil {
- e.Fatalf("WARNING: could not clean up shell (%v)", err)
+ if err := t.shell.Cleanup(os.Stdout, os.Stderr); err != nil {
+ t.Fatalf("WARNING: could not clean up shell (%v)", err)
}
vlog.VI(1).Infof("V23Test.Cleanup: cleaning up binaries & files")
- for _, tempFile := range e.tempFiles {
+ for _, tempFile := range t.tempFiles {
vlog.VI(1).Infof("V23Test.Cleanup: cleaning up %s", tempFile.Name())
if err := tempFile.Close(); err != nil {
vlog.Errorf("WARNING: Close(%q) failed: %v", tempFile.Name(), err)
@@ -543,7 +329,7 @@
}
}
- for _, tempDir := range e.tempDirs {
+ for _, tempDir := range t.tempDirs {
vlog.VI(1).Infof("V23Test.Cleanup: cleaning up %s", tempDir)
if err := os.RemoveAll(tempDir); err != nil {
vlog.Errorf("WARNING: RemoveAll(%q) failed: %v", tempDir, err)
@@ -551,23 +337,23 @@
}
// shutdown the runtime
- e.shutdown()
+ t.shutdown()
}
// GetVar returns the variable associated with the specified key
// and an indication of whether it is defined or not.
-func (e *T) GetVar(key string) (string, bool) {
- return e.shell.GetVar(key)
+func (t *T) GetVar(key string) (string, bool) {
+ return t.shell.GetVar(key)
}
// SetVar sets the value to be associated with key.
-func (e *T) SetVar(key, value string) {
- e.shell.SetVar(key, value)
+func (t *T) SetVar(key, value string) {
+ t.shell.SetVar(key, value)
}
// ClearVar removes the speficied variable from the Shell's environment
-func (e *T) ClearVar(key string) {
- e.shell.ClearVar(key)
+func (t *T) ClearVar(key string) {
+ t.shell.ClearVar(key)
}
func writeStringOrDie(t *T, f *os.File, s string) {
@@ -576,18 +362,19 @@
}
}
-// DebugShell drops the user into a debug shell with any environment
-// variables specified in env... (in VAR=VAL format) available to it.
-// If there is no controlling TTY, DebugShell will emit a warning message
-// and take no futher action. The DebugShell also sets some environment
+// DebugSystemShell drops the user into a debug system shell (e.g. bash)
+// with any environment variables specified in env... (in VAR=VAL format)
+// available to it.
+// If there is no controlling TTY, DebugSystemShell will emit a warning message
+// and take no futher action. The DebugSystemShell also sets some environment
// variables that relate to the running test:
// - V23_TMP_DIR<#> contains the name of each temp directory created.
// - V23_BIN_DIR contains the name of the directory containing binaries.
-func (e *T) DebugShell(env ...string) {
+func (t *T) DebugSystemShell(env ...string) {
// Get the current working directory.
cwd, err := os.Getwd()
if err != nil {
- e.Fatalf("Getwd() failed: %v", err)
+ t.Fatalf("Getwd() failed: %v", err)
}
// Transfer stdin, stdout, and stderr to the new process
@@ -600,7 +387,7 @@
}
var agentFile *os.File
- if creds, err := e.shell.NewChildCredentials(); err == nil {
+ if creds, err := t.shell.NewChildCredentials(); err == nil {
if agentFile, err = creds.File(); err != nil {
vlog.Errorf("WARNING: failed to obtain credentials for the debug shell: %v", err)
}
@@ -618,32 +405,32 @@
attr.Env = append(attr.Env, fmt.Sprintf("%s=%d", agent.FdVarName, len(attr.Files)-1))
// Set up environment for Child.
- for _, v := range e.shell.Env() {
+ for _, v := range t.shell.Env() {
attr.Env = append(attr.Env, v)
}
- for i, td := range e.tempDirs {
+ for i, td := range t.tempDirs {
attr.Env = append(attr.Env, fmt.Sprintf("V23_TMP_DIR%d=%s", i, td))
}
- if len(e.cachedBinDir) > 0 {
- attr.Env = append(attr.Env, "V23_BIN_DIR="+e.BinDir())
+ if len(t.cachedBinDir) > 0 {
+ attr.Env = append(attr.Env, "V23_BIN_DIR="+t.BinDir())
}
attr.Env = append(attr.Env, env...)
// Start up a new shell.
- writeStringOrDie(e, file, ">> Starting a new interactive shell\n")
- writeStringOrDie(e, file, "Hit CTRL-D to resume the test\n")
- if len(e.builtBinaries) > 0 {
- writeStringOrDie(e, file, "Built binaries:\n")
- for _, value := range e.builtBinaries {
- writeStringOrDie(e, file, "\t"+value.Path()+"\n")
+ writeStringOrDie(t, file, ">> Starting a new interactive shell\n")
+ writeStringOrDie(t, file, "Hit CTRL-D to resume the test\n")
+ if len(t.builtBinaries) > 0 {
+ writeStringOrDie(t, file, "Built binaries:\n")
+ for _, value := range t.builtBinaries {
+ writeStringOrDie(t, file, "\t"+value.Path()+"\n")
}
}
- if len(e.cachedBinDir) > 0 {
- writeStringOrDie(e, file, fmt.Sprintf("Binaries are cached in %q\n", e.cachedBinDir))
+ if len(t.cachedBinDir) > 0 {
+ writeStringOrDie(t, file, fmt.Sprintf("Binaries are cached in %q\n", t.cachedBinDir))
} else {
- writeStringOrDie(e, file, fmt.Sprintf("Caching of binaries was not enabled, being written to %q\n", e.binDir))
+ writeStringOrDie(t, file, fmt.Sprintf("Caching of binaries was not enabled, being written to %q\n", t.binDir))
}
shellPath := "/bin/sh"
@@ -652,44 +439,59 @@
}
proc, err := os.StartProcess(shellPath, []string{}, &attr)
if err != nil {
- e.Fatalf("StartProcess(%q) failed: %v", shellPath, err)
+ t.Fatalf("StartProcess(%q) failed: %v", shellPath, err)
}
// Wait until user exits the shell
state, err := proc.Wait()
if err != nil {
- e.Fatalf("Wait(%v) failed: %v", shellPath, err)
+ t.Fatalf("Wait(%v) failed: %v", shellPath, err)
}
- writeStringOrDie(e, file, fmt.Sprintf("<< Exited shell: %s\n", state.String()))
+ writeStringOrDie(t, file, fmt.Sprintf("<< Exited shell: %s\n", state.String()))
}
// BinaryFromPath returns a new Binary that, when started, will
// execute the executable or script at the given path.
//
// E.g. env.BinaryFromPath("/bin/bash").Start("-c", "echo hello world").Output() -> "hello world"
-func (e *T) BinaryFromPath(path string) *Binary {
+func (t *T) BinaryFromPath(path string) *Binary {
return &Binary{
- env: e,
+ env: t,
envVars: nil,
path: path,
+ opts: t.shell.DefaultStartOpts().NoExecCommand(),
}
}
// BuildGoPkg expects a Go package path that identifies a "main"
// package and returns a Binary representing the newly built
-// binary.
-func (e *T) BuildGoPkg(pkg string) *Binary {
+// binary. This binary does not use the exec protocol defined
+// in v.io/x/ref/lib/exec. Use this for command line tools and non
+// Vanadium servers.
+func (t *T) BuildGoPkg(pkg string) *Binary {
+ return t.buildPkg(pkg)
+}
+
+// BuildV23 is like BuildGoPkg, but instead assumes that the resulting
+// binary is a Vanadium application and does implement the exec protocol
+// defined in v.io/x/ref/lib/exec. Use this for Vanadium servers.
+func (t *T) BuildV23Pkg(pkg string) *Binary {
+ b := t.buildPkg(pkg)
+ b.opts = t.shell.DefaultStartOpts().ExternalCommand()
+ return b
+}
+
+func (t *T) buildPkg(pkg string) *Binary {
then := time.Now()
loc := Caller(1)
- cached, built_path, err := buildPkg(e.BinDir(), pkg)
+ cached, built_path, err := buildPkg(t.BinDir(), pkg)
if err != nil {
- e.Fatalf("%s: buildPkg(%s) failed: %v", loc, pkg, err)
+ t.Fatalf("%s: buildPkg(%s) failed: %v", loc, pkg, err)
return nil
}
-
if _, err := os.Stat(built_path); err != nil {
- e.Fatalf("%s: buildPkg(%s) failed to stat %q", loc, pkg, built_path)
+ t.Fatalf("%s: buildPkg(%s) failed to stat %q", loc, pkg, built_path)
}
taken := time.Now().Sub(then)
if cached {
@@ -697,44 +499,44 @@
} else {
vlog.Infof("%s: built %s, written to %s in %s.", loc, pkg, built_path, taken)
}
-
binary := &Binary{
- env: e,
+ env: t,
envVars: nil,
path: built_path,
+ opts: t.shell.DefaultStartOpts().NoExecCommand(),
}
- e.builtBinaries[pkg] = binary
+ t.builtBinaries[pkg] = binary
return binary
}
// NewTempFile creates a temporary file. Temporary files will be deleted
// by Cleanup.
-func (e *T) NewTempFile() *os.File {
+func (t *T) NewTempFile() *os.File {
loc := Caller(1)
f, err := ioutil.TempFile("", "")
if err != nil {
- e.Fatalf("%s: TempFile() failed: %v", loc, err)
+ t.Fatalf("%s: TempFile() failed: %v", loc, err)
}
vlog.Infof("%s: created temporary file at %s", loc, f.Name())
- e.tempFiles = append(e.tempFiles, f)
+ t.tempFiles = append(t.tempFiles, f)
return f
}
// NewTempDir creates a temporary directory. Temporary directories and
// their contents will be deleted by Cleanup.
-func (e *T) NewTempDir() string {
+func (t *T) NewTempDir() string {
loc := Caller(1)
f, err := ioutil.TempDir("", "")
if err != nil {
- e.Fatalf("%s: TempDir() failed: %v", loc, err)
+ t.Fatalf("%s: TempDir() failed: %v", loc, err)
}
vlog.Infof("%s: created temporary directory at %s", loc, f)
- e.tempDirs = append(e.tempDirs, f)
+ t.tempDirs = append(t.tempDirs, f)
return f
}
-func (e *T) appendInvocation(inv *Invocation) {
- e.invocations = append(e.invocations, inv)
+func (t *T) appendInvocation(inv *Invocation) {
+ t.invocations = append(t.invocations, inv)
}
// Creates a new local testing environment. A local testing environment has a
@@ -763,8 +565,10 @@
if err != nil {
t.Fatalf("NewShell() failed: %v", err)
}
- shell.SetStartTimeout(1 * time.Minute)
- shell.SetWaitTimeout(5 * time.Minute)
+ opts := modules.DefaultStartOpts()
+ opts.StartTimeout = time.Minute
+ opts.ShutdownTimeout = 5 * time.Minute
+ shell.SetDefaultStartOpts(opts)
// The V23_BIN_DIR environment variable can be
// used to identify a directory that multiple integration
@@ -788,12 +592,16 @@
return e
}
+func (t *T) Shell() *modules.Shell {
+ return t.shell
+}
+
// BinDir returns the directory that binarie files are stored in.
-func (e *T) BinDir() string {
- if len(e.cachedBinDir) > 0 {
- return e.cachedBinDir
+func (t *T) BinDir() string {
+ if len(t.cachedBinDir) > 0 {
+ return t.cachedBinDir
}
- return e.binDir
+ return t.binDir
}
// BuildPkg returns a path to a directory that contains the built binary for
@@ -833,7 +641,7 @@
// the NAMESPACE_ROOT variable in the test environment so that all subsequent
// invocations will access this root mount table.
func RunRootMT(i *T, args ...string) (*Binary, *Invocation) {
- b := i.BuildGoPkg("v.io/x/ref/services/mounttable/mounttabled")
+ b := i.BuildV23Pkg("v.io/x/ref/services/mounttable/mounttabled")
inv := b.start(1, args...)
name := inv.ExpectVar("NAME")
inv.Environment().SetVar("NAMESPACE_ROOT", name)
diff --git a/lib/testutil/v23tests/v23tests_test.go b/lib/testutil/v23tests/v23tests_test.go
index 4937d26..b253ad9 100644
--- a/lib/testutil/v23tests/v23tests_test.go
+++ b/lib/testutil/v23tests/v23tests_test.go
@@ -18,14 +18,10 @@
"v.io/x/ref/lib/modules"
"v.io/x/ref/lib/testutil"
- "v.io/x/ref/lib/testutil/expect"
"v.io/x/ref/lib/testutil/v23tests"
_ "v.io/x/ref/profiles"
)
-// TODO(sjr): add more unit tests, especially for errors cases.
-// TODO(sjr): need to make sure processes don't get left lying around.
-
func TestBinaryFromPath(t *testing.T) {
env := v23tests.New(t)
defer env.Cleanup()
@@ -48,7 +44,7 @@
defer env.Cleanup()
v23tests.RunRootMT(env, "--veyron.tcp.address=127.0.0.1:0")
- proxyBin := env.BuildGoPkg("v.io/x/ref/services/proxy/proxyd")
+ proxyBin := env.BuildV23Pkg("v.io/x/ref/services/proxy/proxyd")
nsBin := env.BuildGoPkg("v.io/x/ref/cmd/namespace")
mt, ok := env.GetVar("NAMESPACE_ROOT")
@@ -84,8 +80,6 @@
// can examine their output, but not in the parent process. We use the
// modules framework to do so, with the added twist that we need access
// to an instance of testing.T which we obtain via a global variable.
-// TODO(cnicolaou): this will need to change once we switch to using
-// TestMain.
func IntegrationTestInChild(i *v23tests.T) {
fmt.Println("Hello")
sleep := i.BinaryFromPath("/bin/sleep")
@@ -102,7 +96,7 @@
func TestHelperProcess(t *testing.T) {
globalT = t
- modules.DispatchInTest()
+ modules.Dispatch()
}
func RunIntegrationTestInChild(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
@@ -117,19 +111,18 @@
func TestDeferHandling(t *testing.T) {
sh, _ := modules.NewShell(nil, nil)
- child, err := sh.Start("RunIntegrationTestInChild", nil, "--v23.tests")
+ child, err := sh.Start("RunIntegrationTestInChild", nil, "--test.run=TestHelperProcess", "--v23.tests")
if err != nil {
t.Fatal(err)
}
- s := expect.NewSession(t, child.Stdout(), time.Minute)
- s.Expect("Hello")
- s.ExpectRE("--- FAIL: TestHelperProcess", -1)
+ child.Expect("Hello")
+ child.ExpectRE("--- FAIL: TestHelperProcess", -1)
for _, e := range []string{
".* 0: /bin/sleep: shutdown status: has not been shutdown",
".* 1: /bin/sleep: shutdown status: signal: terminated",
".* 2: /bin/sleep: shutdown status: has not been shutdown",
} {
- s.ExpectRE(e, -1)
+ child.ExpectRE(e, -1)
}
var stderr bytes.Buffer
if err := child.Shutdown(nil, &stderr); err != nil {
@@ -285,7 +278,7 @@
msg := recover().(string)
// this, and the tests below are intended to ensure that line #s
// are captured and reported correctly.
- if got, want := msg, "v23tests_test.go:295"; !strings.Contains(got, want) {
+ if got, want := msg, "v23tests_test.go:288"; !strings.Contains(got, want) {
t.Fatalf("%q does not contain %q", got, want)
}
if got, want := msg, "fork/exec /bin/echox: no such file or directory"; !strings.Contains(got, want) {
@@ -300,17 +293,21 @@
env := v23tests.New(mock)
defer env.Cleanup()
- _, inv := v23tests.RunRootMT(env, "--xxveyron.tcp.address=127.0.0.1:0")
+ // Fail fast.
+ sh := env.Shell()
+ opts := sh.DefaultStartOpts()
+ opts.StartTimeout = 100 * time.Millisecond
+ sh.SetDefaultStartOpts(opts)
defer func() {
msg := recover().(string)
- if got, want := msg, "v23tests_test.go:313"; !strings.Contains(got, want) {
+ if got, want := msg, "v23tests_test.go:310"; !strings.Contains(got, want) {
t.Fatalf("%q does not contain %q", got, want)
}
- if got, want := msg, "exit status 2"; !strings.Contains(got, want) {
+ if got, want := msg, "StartWithOpts"; !strings.Contains(got, want) {
t.Fatalf("%q does not contain %q", got, want)
}
}()
- inv.WaitOrDie(nil, nil)
+ v23tests.RunRootMT(env, "--xxveyron.tcp.address=127.0.0.1:0")
}
func TestWaitTimeout(t *testing.T) {
@@ -327,13 +324,11 @@
if iterations == 0 {
t.Fatalf("our sleeper didn't get to run")
}
- if got, want := recover().(string), "v23tests_test.go:335: timed out"; !strings.Contains(got, want) {
+ if got, want := recover().(string), "v23tests_test.go:331: timed out"; !strings.Contains(got, want) {
t.Fatalf("%q does not contain %q", got, want)
}
}()
-
env.WaitFor(sleeper, time.Millisecond, 50*time.Millisecond)
-
}
func TestWaitAsyncTimeout(t *testing.T) {
@@ -351,11 +346,10 @@
if iterations != 0 {
t.Fatalf("our sleeper got to run")
}
- if got, want := recover().(string), "v23tests_test.go:359: timed out"; !strings.Contains(got, want) {
+ if got, want := recover().(string), "v23tests_test.go:353: timed out"; !strings.Contains(got, want) {
t.Fatalf("%q does not contain %q", got, want)
}
}()
-
env.WaitForAsync(sleeper, time.Millisecond, 50*time.Millisecond)
}
diff --git a/profiles/internal/ipc/cancel_test.go b/profiles/internal/ipc/cancel_test.go
index 6a510ab..4154aaa 100644
--- a/profiles/internal/ipc/cancel_test.go
+++ b/profiles/internal/ipc/cancel_test.go
@@ -3,6 +3,8 @@
import (
"testing"
+ tsecurity "v.io/x/ref/lib/testutil/security"
+ "v.io/x/ref/profiles/internal/ipc/stream"
"v.io/x/ref/profiles/internal/ipc/stream/manager"
tnaming "v.io/x/ref/profiles/internal/testing/mocks/naming"
@@ -12,7 +14,6 @@
"v.io/v23/naming/ns"
"v.io/v23/security"
"v.io/x/lib/vlog"
- "v.io/x/ref/profiles/internal/ipc/stream"
)
type fakeAuthorizer int
@@ -57,7 +58,7 @@
func makeCanceld(ns ns.Namespace, name, child string) (*canceld, error) {
sm := manager.InternalNew(naming.FixedRoutingID(0x111111111))
ctx := testContext()
- s, err := testInternalNewServer(ctx, sm, ns)
+ s, err := testInternalNewServer(ctx, sm, ns, tsecurity.NewPrincipal("test"))
if err != nil {
return nil, err
}
diff --git a/profiles/internal/ipc/client_test.go b/profiles/internal/ipc/client_test.go
index b16c1d9..01807eb 100644
--- a/profiles/internal/ipc/client_test.go
+++ b/profiles/internal/ipc/client_test.go
@@ -496,7 +496,6 @@
t.Errorf("Got (%q, %v) want (%q, nil)", result, err, expected)
}
// Kill the server, verify client can't talk to it anymore.
- sh.SetWaitTimeout(time.Minute)
if err := server.Shutdown(os.Stderr, os.Stderr); err != nil {
t.Fatalf("unexpected error: %s", err)
}
diff --git a/profiles/internal/ipc/debug_test.go b/profiles/internal/ipc/debug_test.go
index 71f21b7..502995d 100644
--- a/profiles/internal/ipc/debug_test.go
+++ b/profiles/internal/ipc/debug_test.go
@@ -36,7 +36,7 @@
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns, ReservedNameDispatcher{debugDisp}, vc.LocalPrincipal{pserver})
+ server, err := testInternalNewServer(ctx, sm, ns, pserver, ReservedNameDispatcher{debugDisp})
if err != nil {
t.Fatalf("InternalNewServer failed: %v", err)
}
diff --git a/profiles/internal/ipc/discharges.go b/profiles/internal/ipc/discharges.go
index b45912a..49a0996 100644
--- a/profiles/internal/ipc/discharges.go
+++ b/profiles/internal/ipc/discharges.go
@@ -1,6 +1,9 @@
package ipc
import (
+ "reflect"
+ "sort"
+ "strings"
"sync"
"time"
@@ -42,7 +45,10 @@
return &dischargeClient{
c: client,
defaultCtx: defaultCtx,
- cache: dischargeCache{cache: make(map[string]security.Discharge)},
+ cache: dischargeCache{
+ cache: make(map[dischargeCacheKey]security.Discharge),
+ idToKeys: make(map[string][]dischargeCacheKey),
+ },
dischargeExpiryBuffer: dischargeExpiryBuffer,
}
}
@@ -61,18 +67,20 @@
}
// Make a copy since this copy will be mutated.
var caveats []security.Caveat
+ var filteredImpetuses []security.DischargeImpetus
for _, cav := range forcaveats {
// It shouldn't happen, but in case there are non-third-party
// caveats, drop them.
if tp := cav.ThirdPartyDetails(); tp != nil {
caveats = append(caveats, cav)
+ filteredImpetuses = append(filteredImpetuses, filteredImpetus(tp.Requirements(), impetus))
}
}
// Gather discharges from cache.
// (Collect a set of pointers, where nil implies a missing discharge)
discharges := make([]*security.Discharge, len(caveats))
- if d.cache.Discharges(caveats, discharges) > 0 {
+ if d.cache.Discharges(caveats, filteredImpetuses, discharges) > 0 {
// Fetch discharges for caveats for which no discharges were
// found in the cache.
if ctx == nil {
@@ -83,7 +91,7 @@
ctx, span = vtrace.SetNewSpan(ctx, "Fetching Discharges")
defer span.Finish()
}
- d.fetchDischarges(ctx, caveats, impetus, discharges)
+ d.fetchDischarges(ctx, caveats, filteredImpetuses, discharges)
}
for _, d := range discharges {
if d != nil {
@@ -102,12 +110,13 @@
// fetched or no new discharges are fetched.
// REQUIRES: len(caveats) == len(out)
// REQUIRES: caveats[i].ThirdPartyDetails() != nil for 0 <= i < len(caveats)
-func (d *dischargeClient) fetchDischarges(ctx *context.T, caveats []security.Caveat, impetus security.DischargeImpetus, out []*security.Discharge) {
+func (d *dischargeClient) fetchDischarges(ctx *context.T, caveats []security.Caveat, impetuses []security.DischargeImpetus, out []*security.Discharge) {
var wg sync.WaitGroup
for {
type fetched struct {
idx int
discharge *security.Discharge
+ impetus security.DischargeImpetus
}
discharges := make(chan fetched, len(caveats))
want := 0
@@ -121,7 +130,7 @@
defer wg.Done()
tp := cav.ThirdPartyDetails()
vlog.VI(3).Infof("Fetching discharge for %v", tp)
- call, err := d.c.StartCall(ctx, tp.Location(), "Discharge", []interface{}{cav, filteredImpetus(tp.Requirements(), impetus)}, NoDischarges{})
+ call, err := d.c.StartCall(ctx, tp.Location(), "Discharge", []interface{}{cav, impetuses[i]}, NoDischarges{})
if err != nil {
vlog.VI(3).Infof("Discharge fetch for %v failed: %v", tp, err)
return
@@ -131,14 +140,14 @@
vlog.VI(3).Infof("Discharge fetch for %v failed: (%v)", cav, err)
return
}
- discharges <- fetched{i, &d}
+ discharges <- fetched{i, &d, impetuses[i]}
}(i, ctx, caveats[i])
}
wg.Wait()
close(discharges)
var got int
for fetched := range discharges {
- d.cache.Add(*fetched.discharge)
+ d.cache.Add(*fetched.discharge, fetched.impetus)
out[fetched.idx] = fetched.discharge
got++
}
@@ -165,38 +174,68 @@
// dischargeCache is a concurrency-safe cache for third party caveat discharges.
// TODO(suharshs,ataly,ashankar): This should be keyed by filtered impetus as well.
type dischargeCache struct {
- mu sync.RWMutex
- cache map[string]security.Discharge // GUARDED_BY(mu)
+ mu sync.RWMutex
+ cache map[dischargeCacheKey]security.Discharge // GUARDED_BY(mu)
+ idToKeys map[string][]dischargeCacheKey // GUARDED_BY(mu)
}
-// Add inserts the argument to the cache, possibly overwriting previous
-// discharges for the same caveat.
-func (dcc *dischargeCache) Add(discharges ...security.Discharge) {
- dcc.mu.Lock()
- for _, d := range discharges {
- dcc.cache[d.ID()] = d
+type dischargeCacheKey struct {
+ id, method, serverPatterns string
+}
+
+func (dcc *dischargeCache) cacheKey(id string, impetus security.DischargeImpetus) dischargeCacheKey {
+ // We currently do not cache on impetus.Arguments because there it seems there is no
+ // universal way to generate a key from them.
+ // Add sorted BlessingPatterns to the key.
+ var bps []string
+ for _, bp := range impetus.Server {
+ bps = append(bps, string(bp))
}
+ sort.Strings(bps)
+ return dischargeCacheKey{
+ id: id,
+ method: impetus.Method,
+ serverPatterns: strings.Join(bps, ","), // "," is restricted in blessingPatterns.
+ }
+}
+
+// Add inserts the argument to the cache, the previous discharge for the same caveat.
+func (dcc *dischargeCache) Add(d security.Discharge, filteredImpetus security.DischargeImpetus) {
+ // Only add to the cache if the caveat did not require arguments.
+ if len(filteredImpetus.Arguments) > 0 {
+ return
+ }
+ id := d.ID()
+ dcc.mu.Lock()
+ dcc.cache[dcc.cacheKey(id, filteredImpetus)] = d
+ if _, ok := dcc.idToKeys[id]; !ok {
+ dcc.idToKeys[id] = []dischargeCacheKey{}
+ }
+ dcc.idToKeys[id] = append(dcc.idToKeys[id], dcc.cacheKey(id, filteredImpetus))
dcc.mu.Unlock()
}
-// Discharges takes a slice of caveats and a slice of discharges of the same
-// length and fills in nil entries in the discharges slice with pointers to
-// cached discharges (if there are any).
+// Discharges takes a slice of caveats, a slice of filtered Discharge impetuses
+// corresponding to the caveats, and a slice of discharges of the same length and
+// fills in nil entries in the discharges slice with pointers to cached discharges
+// (if there are any).
//
-// REQUIRES: len(caveats) == len(out)
+// REQUIRES: len(caveats) == len(impetuses) == len(out)
// REQUIRES: caveats[i].ThirdPartyDetails() != nil, for all 0 <= i < len(caveats)
-func (dcc *dischargeCache) Discharges(caveats []security.Caveat, out []*security.Discharge) (remaining int) {
+func (dcc *dischargeCache) Discharges(caveats []security.Caveat, impetuses []security.DischargeImpetus, out []*security.Discharge) (remaining int) {
dcc.mu.Lock()
for i, d := range out {
if d != nil {
continue
}
- if cached, exists := dcc.cache[caveats[i].ThirdPartyDetails().ID()]; exists {
+ id := caveats[i].ThirdPartyDetails().ID()
+ key := dcc.cacheKey(id, impetuses[i])
+ if cached, exists := dcc.cache[key]; exists {
out[i] = &cached
// If the discharge has expired, purge it from the cache.
if hasDischargeExpired(out[i]) {
out[i] = nil
- delete(dcc.cache, cached.ID())
+ delete(dcc.cache, key)
remaining++
}
} else {
@@ -218,15 +257,19 @@
func (dcc *dischargeCache) invalidate(discharges ...security.Discharge) {
dcc.mu.Lock()
for _, d := range discharges {
- // TODO(ashankar,ataly): The cached discharge might have been
- // replaced by the time invalidate is called.
- // Should we have an "Equals" function defined on "Discharge"
- // and use that? (Could use reflect.DeepEqual as well, but
- // that will likely be expensive)
- // if cached := dcc.cache[d.ID()]; cached.Equals(d) {
- // delete(dcc.cache, d.ID())
- // }
- delete(dcc.cache, d.ID())
+ if keys, ok := dcc.idToKeys[d.ID()]; ok {
+ var newKeys []dischargeCacheKey
+ for _, k := range keys {
+ // TODO(suharshs,ataly,ashankar): Should we have an "Equals" function
+ // defined on "Discharge" and use that instead of reflect.DeepEqual?
+ if cached := dcc.cache[k]; reflect.DeepEqual(cached, d) {
+ delete(dcc.cache, k)
+ } else {
+ newKeys = append(newKeys, k)
+ }
+ }
+ dcc.idToKeys[d.ID()] = newKeys
+ }
}
dcc.mu.Unlock()
}
diff --git a/profiles/internal/ipc/discharges_test.go b/profiles/internal/ipc/discharges_test.go
new file mode 100644
index 0000000..230b8bf
--- /dev/null
+++ b/profiles/internal/ipc/discharges_test.go
@@ -0,0 +1,102 @@
+package ipc
+
+import (
+ "testing"
+ "time"
+
+ tsecurity "v.io/x/ref/lib/testutil/security"
+
+ "v.io/v23/security"
+ "v.io/v23/vdl"
+)
+
+func TestDischargeClientCache(t *testing.T) {
+ dcc := &dischargeCache{
+ cache: make(map[dischargeCacheKey]security.Discharge),
+ idToKeys: make(map[string][]dischargeCacheKey),
+ }
+
+ var (
+ discharger = tsecurity.NewPrincipal("discharger")
+ expiredCav = mkCaveat(security.NewPublicKeyCaveat(discharger.PublicKey(), "moline", security.ThirdPartyRequirements{}, security.UnconstrainedUse()))
+ argsCav = mkCaveat(security.NewPublicKeyCaveat(discharger.PublicKey(), "moline", security.ThirdPartyRequirements{}, security.UnconstrainedUse()))
+ methodCav = mkCaveat(security.NewPublicKeyCaveat(discharger.PublicKey(), "moline", security.ThirdPartyRequirements{}, security.UnconstrainedUse()))
+ serverCav = mkCaveat(security.NewPublicKeyCaveat(discharger.PublicKey(), "moline", security.ThirdPartyRequirements{}, security.UnconstrainedUse()))
+
+ dExpired = mkDischarge(discharger.MintDischarge(expiredCav, mkCaveat(security.ExpiryCaveat(time.Now().Add(-1*time.Minute)))))
+ dArgs = mkDischarge(discharger.MintDischarge(argsCav, security.UnconstrainedUse()))
+ dMethod = mkDischarge(discharger.MintDischarge(methodCav, security.UnconstrainedUse()))
+ dServer = mkDischarge(discharger.MintDischarge(serverCav, security.UnconstrainedUse()))
+
+ emptyImp = security.DischargeImpetus{}
+ argsImp = security.DischargeImpetus{Arguments: []*vdl.Value{&vdl.Value{}}}
+ methodImp = security.DischargeImpetus{Method: "foo"}
+ otherMethodImp = security.DischargeImpetus{Method: "bar"}
+ serverImp = security.DischargeImpetus{Server: []security.BlessingPattern{security.BlessingPattern("fooserver")}}
+ otherServerImp = security.DischargeImpetus{Server: []security.BlessingPattern{security.BlessingPattern("barserver")}}
+ )
+
+ // Discharges for different cavs should not be cached.
+ d := mkDischarge(discharger.MintDischarge(argsCav, security.UnconstrainedUse()))
+ dcc.Add(d, emptyImp)
+ outdis := make([]*security.Discharge, 1)
+ if remaining := dcc.Discharges([]security.Caveat{methodCav}, []security.DischargeImpetus{emptyImp}, outdis); remaining == 0 {
+ t.Errorf("Discharge for different caveat should not have been in cache")
+ }
+ dcc.invalidate(d)
+
+ // Add some discharges into the cache.
+ dcc.Add(dArgs, argsImp)
+ dcc.Add(dMethod, methodImp)
+ dcc.Add(dServer, serverImp)
+ dcc.Add(dExpired, emptyImp)
+
+ testCases := []struct {
+ caveat security.Caveat // caveat that we are fetching discharges for.
+ queryImpetus security.DischargeImpetus // Impetus used to query the cache.
+ cachedDischarge *security.Discharge // Discharge that we expect to be returned from the cache, nil if the discharge should not be cached.
+ }{
+ // Expired discharges should not be returned by the cache.
+ {expiredCav, emptyImp, nil},
+
+ // Discharges with Impetuses that have Arguments should not be cached.
+ {argsCav, argsImp, nil},
+
+ {methodCav, methodImp, &dMethod},
+ {methodCav, otherMethodImp, nil},
+ {methodCav, emptyImp, nil},
+
+ {serverCav, serverImp, &dServer},
+ {serverCav, otherServerImp, nil},
+ {serverCav, emptyImp, nil},
+ }
+
+ for i, test := range testCases {
+ out := make([]*security.Discharge, 1)
+ remaining := dcc.Discharges([]security.Caveat{test.caveat}, []security.DischargeImpetus{test.queryImpetus}, out)
+ if test.cachedDischarge != nil {
+ got := "nil"
+ if remaining == 0 {
+ got = out[0].ID()
+ }
+ if got != test.cachedDischarge.ID() {
+ t.Errorf("#%d: got discharge %v, want %v, queried with %v", i, got, test.cachedDischarge.ID(), test.queryImpetus)
+ }
+ } else if remaining == 0 {
+ t.Errorf("#%d: discharge %v should not have been in cache, queried with %v", i, out[0].ID(), test.queryImpetus)
+ }
+ }
+ if t.Failed() {
+ t.Logf("dArgs.ID(): %v", dArgs.ID())
+ t.Logf("dMethod.ID(): %v", dMethod.ID())
+ t.Logf("dServer.ID(): %v", dServer.ID())
+ t.Logf("dExpired.ID(): %v", dExpired.ID())
+ }
+}
+
+func mkDischarge(d security.Discharge, err error) security.Discharge {
+ if err != nil {
+ panic(err)
+ }
+ return d
+}
diff --git a/profiles/internal/ipc/full_test.go b/profiles/internal/ipc/full_test.go
index 8adb4bf..a8e0077 100644
--- a/profiles/internal/ipc/full_test.go
+++ b/profiles/internal/ipc/full_test.go
@@ -81,12 +81,12 @@
return ctx
}
-func testInternalNewServer(ctx *context.T, streamMgr stream.Manager, ns ns.Namespace, opts ...ipc.ServerOpt) (ipc.Server, error) {
+func testInternalNewServer(ctx *context.T, streamMgr stream.Manager, ns ns.Namespace, principal security.Principal, opts ...ipc.ServerOpt) (ipc.Server, error) {
client, err := InternalNewClient(streamMgr, ns)
if err != nil {
return nil, err
}
- return InternalNewServer(ctx, streamMgr, ns, client, opts...)
+ return InternalNewServer(ctx, streamMgr, ns, client, principal, opts...)
}
type userType string
@@ -254,9 +254,8 @@
func startServerWS(t *testing.T, principal security.Principal, sm stream.Manager, ns ns.Namespace, name string, disp ipc.Dispatcher, shouldUseWebsocket websocketMode, opts ...ipc.ServerOpt) (naming.Endpoint, ipc.Server) {
vlog.VI(1).Info("InternalNewServer")
- opts = append(opts, vc.LocalPrincipal{principal})
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns, opts...)
+ server, err := testInternalNewServer(ctx, sm, ns, principal, opts...)
if err != nil {
t.Errorf("InternalNewServer failed: %v", err)
}
@@ -399,13 +398,13 @@
return verror.Is(err, id.ID)
}
-func runServer(t *testing.T, ns ns.Namespace, name string, obj interface{}, opts ...ipc.ServerOpt) stream.Manager {
+func runServer(t *testing.T, ns ns.Namespace, principal security.Principal, name string, obj interface{}, opts ...ipc.ServerOpt) stream.Manager {
rid, err := naming.NewRoutingID()
if err != nil {
t.Fatal(err)
}
sm := imanager.InternalNew(rid)
- server, err := testInternalNewServer(testContext(), sm, ns, opts...)
+ server, err := testInternalNewServer(testContext(), sm, ns, principal, opts...)
if err != nil {
t.Fatal(err)
}
@@ -422,7 +421,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x555555555))
ns := tnaming.NewSimpleNamespace()
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns, vc.LocalPrincipal{tsecurity.NewPrincipal("server")})
+ server, err := testInternalNewServer(ctx, sm, ns, tsecurity.NewPrincipal("server"))
if err != nil {
t.Errorf("InternalNewServer failed: %v", err)
}
@@ -618,7 +617,7 @@
// namespace that the client will use, provide it with a
// different namespace).
tnaming.NewSimpleNamespace(),
- vc.LocalPrincipal{tsecurity.NewPrincipal("attacker")})
+ tsecurity.NewPrincipal("attacker"))
if err != nil {
t.Fatal(err)
}
@@ -922,7 +921,7 @@
// Setup the discharge server.
var tester dischargeTestServer
ctx := testContext()
- dischargeServer, err := testInternalNewServer(ctx, sm, ns, vc.LocalPrincipal{pdischarger})
+ dischargeServer, err := testInternalNewServer(ctx, sm, ns, pdischarger)
if err != nil {
t.Fatal(err)
}
@@ -935,7 +934,7 @@
}
// Setup the application server.
- appServer, err := testInternalNewServer(ctx, sm, ns, vc.LocalPrincipal{pserver})
+ appServer, err := testInternalNewServer(ctx, sm, ns, pserver)
if err != nil {
t.Fatal(err)
}
@@ -1527,7 +1526,7 @@
return []ipc.Address{&netstate.AddrIfc{Addr: a}}, nil
}
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns, vc.LocalPrincipal{tsecurity.NewPrincipal("server")})
+ server, err := testInternalNewServer(ctx, sm, ns, tsecurity.NewPrincipal("server"))
if err != nil {
t.Errorf("InternalNewServer failed: %v", err)
}
@@ -1569,7 +1568,7 @@
return nil, fmt.Errorf("oops")
}
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns, vc.LocalPrincipal{tsecurity.NewPrincipal("server")})
+ server, err := testInternalNewServer(ctx, sm, ns, tsecurity.NewPrincipal("server"))
if err != nil {
t.Errorf("InternalNewServer failed: %v", err)
}
@@ -1598,7 +1597,7 @@
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns, options.VCSecurityNone)
+ server, err := testInternalNewServer(ctx, sm, ns, nil, options.VCSecurityNone)
if err != nil {
t.Fatalf("InternalNewServer failed: %v", err)
}
@@ -1664,9 +1663,8 @@
// to act as batman (as opposed to using the default blessing).
ns := tnaming.NewSimpleNamespace()
- popt := vc.LocalPrincipal{pserver}
- defer runServer(t, ns, "mountpoint/batman", &testServer{}, popt, options.ServerBlessings{batman}).Shutdown()
- defer runServer(t, ns, "mountpoint/default", &testServer{}, popt).Shutdown()
+ defer runServer(t, ns, pserver, "mountpoint/batman", &testServer{}, options.ServerBlessings{batman}).Shutdown()
+ defer runServer(t, ns, pserver, "mountpoint/default", &testServer{}).Shutdown()
// And finally, make an RPC and see that the client sees "batman"
runClient := func(server string) ([]string, error) {
@@ -1726,8 +1724,8 @@
// Setup the disharger and test server.
discharger := &dischargeServer{}
- defer runServer(t, ns, "mountpoint/discharger", discharger, vc.LocalPrincipal{pdischarger}).Shutdown()
- defer runServer(t, ns, "mountpoint/testServer", &testServer{}, vc.LocalPrincipal{pserver}).Shutdown()
+ defer runServer(t, ns, pdischarger, "mountpoint/discharger", discharger).Shutdown()
+ defer runServer(t, ns, pserver, "mountpoint/testServer", &testServer{}).Shutdown()
runClient := func(noDischarges bool) {
rid, err := naming.NewRoutingID()
@@ -1784,8 +1782,8 @@
// Setup the disharger and test server.
discharger1 := &dischargeServer{}
discharger2 := &dischargeServer{}
- defer runServer(t, ns, "mountpoint/discharger1", discharger1, vc.LocalPrincipal{pdischarger1}).Shutdown()
- defer runServer(t, ns, "mountpoint/discharger2", discharger2, vc.LocalPrincipal{pdischarger2}).Shutdown()
+ defer runServer(t, ns, pdischarger1, "mountpoint/discharger1", discharger1).Shutdown()
+ defer runServer(t, ns, pdischarger2, "mountpoint/discharger2", discharger2).Shutdown()
rid, err := naming.NewRoutingID()
if err != nil {
@@ -1827,7 +1825,7 @@
ns := tnaming.NewSimpleNamespace()
- serverSM := runServer(t, ns, "mountpoint/testServer", &testServer{}, vc.LocalPrincipal{pserver})
+ serverSM := runServer(t, ns, pserver, "mountpoint/testServer", &testServer{})
defer serverSM.Shutdown()
rid := serverSM.RoutingID()
@@ -1912,7 +1910,7 @@
mountName := "mountpoint/default"
// Start a server with pserver.
- defer runServer(t, ns, mountName, &testServer{}, vc.LocalPrincipal{pserver}).Shutdown()
+ defer runServer(t, ns, pserver, mountName, &testServer{}).Shutdown()
smc := imanager.InternalNew(naming.FixedRoutingID(0xc))
client, err := InternalNewClient(
@@ -1976,7 +1974,7 @@
// Setup the disharge server.
discharger := &expiryDischarger{}
- defer runServer(t, ns, "mountpoint/discharger", discharger, vc.LocalPrincipal{pdischarger}).Shutdown()
+ defer runServer(t, ns, pdischarger, "mountpoint/discharger", discharger).Shutdown()
// Create a discharge client.
rid, err := naming.NewRoutingID()
diff --git a/profiles/internal/ipc/proxy_test.go b/profiles/internal/ipc/proxy_test.go
index 58259d9..6eed70a 100644
--- a/profiles/internal/ipc/proxy_test.go
+++ b/profiles/internal/ipc/proxy_test.go
@@ -185,7 +185,7 @@
}
defer client.Close()
serverCtx, _ := v23.SetPrincipal(ctx, pserver)
- server, err := iipc.InternalNewServer(serverCtx, smserver, ns, nil, vc.LocalPrincipal{pserver})
+ server, err := iipc.InternalNewServer(serverCtx, smserver, ns, nil, pserver)
if err != nil {
t.Fatal(err)
}
diff --git a/profiles/internal/ipc/server.go b/profiles/internal/ipc/server.go
index 6ee0b8f..c289c3d 100644
--- a/profiles/internal/ipc/server.go
+++ b/profiles/internal/ipc/server.go
@@ -74,6 +74,7 @@
publisher publisher.Publisher // publisher to publish mounttable mounts.
listenerOpts []stream.ListenerOpt // listener opts for Listen.
dhcpState *dhcpState // dhcpState, nil if not using dhcp
+ principal security.Principal
// maps that contain state on listeners.
listenState map[*listenState]struct{}
@@ -150,15 +151,15 @@
var _ ipc.Server = (*server)(nil)
-func InternalNewServer(ctx *context.T, streamMgr stream.Manager, ns ns.Namespace, client ipc.Client, opts ...ipc.ServerOpt) (ipc.Server, error) {
+func InternalNewServer(ctx *context.T, streamMgr stream.Manager, ns ns.Namespace, client ipc.Client, principal security.Principal, opts ...ipc.ServerOpt) (ipc.Server, error) {
ctx, cancel := context.WithRootCancel(ctx)
ctx, _ = vtrace.SetNewSpan(ctx, "NewServer")
statsPrefix := naming.Join("ipc", "server", "routing-id", streamMgr.RoutingID().String())
s := &server{
-
ctx: ctx,
cancel: cancel,
streamMgr: streamMgr,
+ principal: principal,
publisher: publisher.New(ctx, ns, publishPeriod),
listenState: make(map[*listenState]struct{}),
listeners: make(map[stream.Listener]struct{}),
@@ -169,7 +170,6 @@
stats: newIPCStats(statsPrefix),
}
var (
- principal security.Principal
blessings security.Blessings
dischargeExpiryBuffer = vc.DefaultServerDischargeExpiryBuffer
)
@@ -179,8 +179,6 @@
// Collect all ServerOpts that are also ListenerOpts.
s.listenerOpts = append(s.listenerOpts, opt)
switch opt := opt.(type) {
- case vc.LocalPrincipal:
- principal = opt.Principal
case options.ServerBlessings:
blessings = opt.Blessings
case vc.DischargeExpiryBuffer:
@@ -391,7 +389,7 @@
protocol: addr.Protocol,
address: addr.Address,
}
- ls.ln, ls.lep, ls.lnerr = s.streamMgr.Listen(addr.Protocol, addr.Address, s.listenerOpts...)
+ ls.ln, ls.lep, ls.lnerr = s.streamMgr.Listen(addr.Protocol, addr.Address, s.principal, s.listenerOpts...)
lnState = append(lnState, ls)
if ls.lnerr != nil {
vlog.VI(2).Infof("Listen(%q, %q, ...) failed: %v", addr.Protocol, addr.Address, ls.lnerr)
@@ -463,7 +461,7 @@
if err != nil {
return nil, nil, fmt.Errorf("Failed to resolve proxy %q (%v)", proxy, err)
}
- ln, ep, err := s.streamMgr.Listen(inaming.Network, resolved, s.listenerOpts...)
+ ln, ep, err := s.streamMgr.Listen(inaming.Network, resolved, s.principal, s.listenerOpts...)
if err != nil {
return nil, nil, fmt.Errorf("failed to listen on %q: %s", resolved, err)
}
diff --git a/profiles/internal/ipc/server_test.go b/profiles/internal/ipc/server_test.go
index 469cb59..2670900 100644
--- a/profiles/internal/ipc/server_test.go
+++ b/profiles/internal/ipc/server_test.go
@@ -45,7 +45,7 @@
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
ctx := testContext()
- server, err := testInternalNewServer(ctx, sm, ns)
+ server, err := testInternalNewServer(ctx, sm, ns, tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -87,7 +87,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x555555555))
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
- server, err := InternalNewServer(testContext(), sm, ns, nil)
+ server, err := testInternalNewServer(testContext(), sm, ns, tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -134,7 +134,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x555555555))
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
- principal := vc.LocalPrincipal{tsecurity.NewPrincipal("testServerStatus")}
+ principal := tsecurity.NewPrincipal("testServerStatus")
server, err := testInternalNewServer(ctx, sm, ns, principal)
if err != nil {
t.Fatal(err)
@@ -162,7 +162,7 @@
progress := make(chan error)
- client, err := InternalNewClient(sm, ns, principal)
+ client, err := InternalNewClient(sm, ns, vc.LocalPrincipal{principal})
makeCall := func(ctx *context.T) {
call, err := client.StartCall(ctx, "test", "Hang", nil)
progress <- err
@@ -236,7 +236,7 @@
}
}
- server, err := testInternalNewServer(ctx, sm, ns)
+ server, err := testInternalNewServer(ctx, sm, ns, tsecurity.NewPrincipal("test"))
expectNoError(err)
defer server.Stop()
@@ -289,7 +289,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x555555555))
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
- server, err := testInternalNewServer(testContext(), sm, ns)
+ server, err := testInternalNewServer(testContext(), sm, ns, tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -411,7 +411,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x555555555))
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
- server, err := testInternalNewServer(testContext(), sm, ns)
+ server, err := testInternalNewServer(testContext(), sm, ns, tsecurity.NewPrincipal("test"))
defer server.Stop()
if err != nil {
@@ -560,7 +560,7 @@
sm := imanager.InternalNew(naming.FixedRoutingID(0x555555555))
defer sm.Shutdown()
ns := tnaming.NewSimpleNamespace()
- server, err := testInternalNewServer(testContext(), sm, ns)
+ server, err := testInternalNewServer(testContext(), sm, ns, tsecurity.NewPrincipal("test"))
defer server.Stop()
if err != nil {
diff --git a/profiles/internal/ipc/stream/benchmark/dial_vc.go b/profiles/internal/ipc/stream/benchmark/dial_vc.go
index 68cfdd5..ca94f11 100644
--- a/profiles/internal/ipc/stream/benchmark/dial_vc.go
+++ b/profiles/internal/ipc/stream/benchmark/dial_vc.go
@@ -5,6 +5,7 @@
"time"
"v.io/x/ref/lib/testutil/benchmark"
+ tsecurity "v.io/x/ref/lib/testutil/security"
"v.io/x/ref/profiles/internal/ipc/stream/manager"
_ "v.io/x/ref/profiles/static"
@@ -19,7 +20,7 @@
server := manager.InternalNew(naming.FixedRoutingID(0x5))
client := manager.InternalNew(naming.FixedRoutingID(0xc))
- _, ep, err := server.Listen("tcp", "127.0.0.1:0", mode)
+ _, ep, err := server.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("test"), mode)
if err != nil {
b.Fatal(err)
}
diff --git a/profiles/internal/ipc/stream/benchmark/dial_vif.go b/profiles/internal/ipc/stream/benchmark/dial_vif.go
index 6bd7d7d..fd1d47e 100644
--- a/profiles/internal/ipc/stream/benchmark/dial_vif.go
+++ b/profiles/internal/ipc/stream/benchmark/dial_vif.go
@@ -6,6 +6,7 @@
"time"
"v.io/x/ref/lib/testutil/benchmark"
+ tsecurity "v.io/x/ref/lib/testutil/security"
"v.io/x/ref/profiles/internal/ipc/stream/vif"
"v.io/v23/naming"
@@ -15,6 +16,7 @@
// benchmarkDialVIF measures VIF creation time over the underlying net connection.
func benchmarkDialVIF(b *testing.B, mode options.VCSecurityLevel) {
stats := benchmark.AddStats(b, 16)
+ principal := tsecurity.NewPrincipal("test")
b.ResetTimer() // Exclude setup time from measurement.
@@ -22,7 +24,7 @@
b.StopTimer()
nc, ns := net.Pipe()
- server, err := vif.InternalNewAcceptedVIF(ns, naming.FixedRoutingID(0x5), nil, mode)
+ server, err := vif.InternalNewAcceptedVIF(ns, naming.FixedRoutingID(0x5), principal, nil, mode)
if err != nil {
b.Fatal(err)
}
diff --git a/profiles/internal/ipc/stream/benchmark/throughput_flow.go b/profiles/internal/ipc/stream/benchmark/throughput_flow.go
index e0191d0..82e4b9c 100644
--- a/profiles/internal/ipc/stream/benchmark/throughput_flow.go
+++ b/profiles/internal/ipc/stream/benchmark/throughput_flow.go
@@ -8,6 +8,7 @@
"v.io/v23/naming"
"v.io/v23/options"
+ tsecurity "v.io/x/ref/lib/testutil/security"
"v.io/x/ref/profiles/internal/ipc/stream"
)
@@ -28,7 +29,7 @@
func createListeners(mode options.VCSecurityLevel, m stream.Manager, N int) (servers []listener, err error) {
for i := 0; i < N; i++ {
var l listener
- if l.ln, l.ep, err = m.Listen("tcp", "127.0.0.1:0", mode); err != nil {
+ if l.ln, l.ep, err = m.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("test"), mode); err != nil {
return
}
servers = append(servers, l)
diff --git a/profiles/internal/ipc/stream/manager/listener.go b/profiles/internal/ipc/stream/manager/listener.go
index 6c23880..07fde42 100644
--- a/profiles/internal/ipc/stream/manager/listener.go
+++ b/profiles/internal/ipc/stream/manager/listener.go
@@ -8,11 +8,13 @@
"sync"
"v.io/x/ref/profiles/internal/ipc/stream/proxy"
+ "v.io/x/ref/profiles/internal/ipc/stream/vc"
"v.io/x/ref/profiles/internal/ipc/stream/vif"
"v.io/x/ref/profiles/internal/lib/upcqueue"
inaming "v.io/x/ref/profiles/internal/naming"
"v.io/v23/naming"
+ "v.io/v23/security"
"v.io/v23/verror"
"v.io/v23/vom"
"v.io/x/lib/vlog"
@@ -44,15 +46,16 @@
// proxyListener implements the listener interface by connecting to a remote
// proxy (typically used to "listen" across network domains).
type proxyListener struct {
- q *upcqueue.T
- proxyEP naming.Endpoint
- manager *manager
- opts []stream.ListenerOpt
+ q *upcqueue.T
+ proxyEP naming.Endpoint
+ manager *manager
+ principal security.Principal
+ opts []stream.ListenerOpt
}
var _ stream.Listener = (*proxyListener)(nil)
-func newNetListener(m *manager, netLn net.Listener, opts []stream.ListenerOpt) listener {
+func newNetListener(m *manager, netLn net.Listener, principal security.Principal, opts []stream.ListenerOpt) listener {
ln := &netListener{
q: upcqueue.New(),
manager: m,
@@ -60,11 +63,11 @@
vifs: vif.NewSet(),
}
ln.netLoop.Add(1)
- go ln.netAcceptLoop(opts)
+ go ln.netAcceptLoop(principal, opts)
return ln
}
-func (ln *netListener) netAcceptLoop(listenerOpts []stream.ListenerOpt) {
+func (ln *netListener) netAcceptLoop(principal security.Principal, listenerOpts []stream.ListenerOpt) {
defer ln.netLoop.Done()
for {
conn, err := ln.netLn.Accept()
@@ -73,7 +76,7 @@
return
}
vlog.VI(1).Infof("New net.Conn accepted from %s (local address: %s)", conn.RemoteAddr(), conn.LocalAddr())
- vf, err := vif.InternalNewAcceptedVIF(conn, ln.manager.rid, nil, listenerOpts...)
+ vf, err := vif.InternalNewAcceptedVIF(conn, ln.manager.rid, principal, nil, listenerOpts...)
if err != nil {
vlog.Infof("Shutting down conn from %s (local address: %s) as a VIF could not be created: %v", conn.RemoteAddr(), conn.LocalAddr(), err)
conn.Close()
@@ -137,12 +140,13 @@
return strings.Join(ret, "\n")
}
-func newProxyListener(m *manager, proxyEP naming.Endpoint, opts []stream.ListenerOpt) (listener, *inaming.Endpoint, error) {
+func newProxyListener(m *manager, proxyEP naming.Endpoint, principal security.Principal, opts []stream.ListenerOpt) (listener, *inaming.Endpoint, error) {
ln := &proxyListener{
- q: upcqueue.New(),
- proxyEP: proxyEP,
- manager: m,
- opts: opts,
+ q: upcqueue.New(),
+ proxyEP: proxyEP,
+ manager: m,
+ opts: opts,
+ principal: principal,
}
vf, ep, err := ln.connect()
if err != nil {
@@ -163,6 +167,8 @@
}
}
// TODO(cnicolaou, ashankar): probably want to set a timeout here. (is this covered by opts?)
+ // TODO(suharshs): Pass the principal explicitly to FindOrDialVIF and remove vc.LocalPrincipal.
+ dialOpts = append(dialOpts, vc.LocalPrincipal{ln.principal})
vf, err := ln.manager.FindOrDialVIF(ln.proxyEP, dialOpts...)
if err != nil {
return nil, nil, err
diff --git a/profiles/internal/ipc/stream/manager/manager.go b/profiles/internal/ipc/stream/manager/manager.go
index b8e657a..e7c42e1 100644
--- a/profiles/internal/ipc/stream/manager/manager.go
+++ b/profiles/internal/ipc/stream/manager/manager.go
@@ -19,7 +19,6 @@
"v.io/x/ref/lib/stats"
"v.io/x/ref/profiles/internal/ipc/stream"
"v.io/x/ref/profiles/internal/ipc/stream/crypto"
- "v.io/x/ref/profiles/internal/ipc/stream/vc"
"v.io/x/ref/profiles/internal/ipc/stream/vif"
"v.io/x/ref/profiles/internal/ipc/version"
inaming "v.io/x/ref/profiles/internal/naming"
@@ -160,12 +159,12 @@
return nil, fmt.Errorf("unknown network %s", protocol)
}
-func (m *manager) Listen(protocol, address string, opts ...stream.ListenerOpt) (stream.Listener, naming.Endpoint, error) {
- blessings, err := extractBlessings(opts)
+func (m *manager) Listen(protocol, address string, principal security.Principal, opts ...stream.ListenerOpt) (stream.Listener, naming.Endpoint, error) {
+ blessings, err := extractBlessings(principal, opts)
if err != nil {
return nil, nil, err
}
- ln, ep, err := m.internalListen(protocol, address, opts...)
+ ln, ep, err := m.internalListen(protocol, address, principal, opts...)
if err != nil {
return nil, nil, err
}
@@ -173,7 +172,7 @@
return ln, ep, nil
}
-func (m *manager) internalListen(protocol, address string, opts ...stream.ListenerOpt) (stream.Listener, *inaming.Endpoint, error) {
+func (m *manager) internalListen(protocol, address string, principal security.Principal, opts ...stream.ListenerOpt) (stream.Listener, *inaming.Endpoint, error) {
m.muListeners.Lock()
if m.shutdown {
m.muListeners.Unlock()
@@ -187,7 +186,7 @@
if err != nil {
return nil, nil, fmt.Errorf("failed to parse endpoint %q: %v", address, err)
}
- return m.remoteListen(ep, opts)
+ return m.remoteListen(ep, principal, opts)
}
netln, err := listen(protocol, address)
if err != nil {
@@ -201,14 +200,14 @@
return nil, nil, errShutDown
}
- ln := newNetListener(m, netln, opts)
+ ln := newNetListener(m, netln, principal, opts)
m.listeners[ln] = true
m.muListeners.Unlock()
return ln, version.Endpoint(protocol, netln.Addr().String(), m.rid), nil
}
-func (m *manager) remoteListen(proxy naming.Endpoint, listenerOpts []stream.ListenerOpt) (stream.Listener, *inaming.Endpoint, error) {
- ln, ep, err := newProxyListener(m, proxy, listenerOpts)
+func (m *manager) remoteListen(proxy naming.Endpoint, principal security.Principal, listenerOpts []stream.ListenerOpt) (stream.Listener, *inaming.Endpoint, error) {
+ ln, ep, err := newProxyListener(m, proxy, principal, listenerOpts)
if err != nil {
return nil, nil, err
}
@@ -298,15 +297,10 @@
return strings.Join(l, "\n")
}
-func extractBlessings(opts []stream.ListenerOpt) ([]string, error) {
- var (
- p security.Principal
- b security.Blessings
- )
+func extractBlessings(p security.Principal, opts []stream.ListenerOpt) ([]string, error) {
+ var b security.Blessings
for _, o := range opts {
switch v := o.(type) {
- case vc.LocalPrincipal:
- p = v.Principal
case options.ServerBlessings:
b = v.Blessings
}
diff --git a/profiles/internal/ipc/stream/manager/manager_test.go b/profiles/internal/ipc/stream/manager/manager_test.go
index dbff17d..3d7a1a5 100644
--- a/profiles/internal/ipc/stream/manager/manager_test.go
+++ b/profiles/internal/ipc/stream/manager/manager_test.go
@@ -31,10 +31,6 @@
inaming "v.io/x/ref/profiles/internal/naming"
)
-func newPrincipal(defaultBlessing string) vc.LocalPrincipal {
- return vc.LocalPrincipal{tsecurity.NewPrincipal(defaultBlessing)}
-}
-
func init() {
modules.RegisterChild("runServer", "", runServer)
}
@@ -48,7 +44,7 @@
// condition that occurs when closing the server; also, using 1 cpu
// introduces less variance in the behavior of the test.
runtime.GOMAXPROCS(1)
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
@@ -62,7 +58,7 @@
server := InternalNew(naming.FixedRoutingID(0x55555555))
client := InternalNew(naming.FixedRoutingID(0xcccccccc))
- ln, ep, err := server.Listen(protocol, "127.0.0.1:0")
+ ln, ep, err := server.Listen(protocol, "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -176,10 +172,10 @@
server = InternalNew(naming.FixedRoutingID(0x55555555))
client = InternalNew(naming.FixedRoutingID(0xcccccccc))
- clientPrincipal = newPrincipal("client")
- serverPrincipal = newPrincipal("server")
- clientKey = clientPrincipal.Principal.PublicKey()
- serverBlessings = serverPrincipal.Principal.BlessingStore().Default()
+ clientPrincipal = tsecurity.NewPrincipal("client")
+ serverPrincipal = tsecurity.NewPrincipal("server")
+ clientKey = clientPrincipal.PublicKey()
+ serverBlessings = serverPrincipal.BlessingStore().Default()
)
// VCSecurityLevel is intentionally not provided to Listen - to test
// default behavior.
@@ -223,7 +219,7 @@
go func() {
// VCSecurityLevel is intentionally not provided to Dial - to
// test default behavior.
- vc, err := client.Dial(ep, clientPrincipal)
+ vc, err := client.Dial(ep, vc.LocalPrincipal{clientPrincipal})
if err != nil {
errs <- err
return
@@ -259,8 +255,8 @@
func TestListenEndpoints(t *testing.T) {
server := InternalNew(naming.FixedRoutingID(0xcafe))
- ln1, ep1, err1 := server.Listen("tcp", "127.0.0.1:0")
- ln2, ep2, err2 := server.Listen("tcp", "127.0.0.1:0")
+ ln1, ep1, err1 := server.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
+ ln2, ep2, err2 := server.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
// Since "127.0.0.1:0" was used as the network address, a random port will be
// assigned in each case. The endpoint should include that random port.
if err1 != nil {
@@ -306,7 +302,7 @@
func testCloseListener(t *testing.T, protocol string) {
server := InternalNew(naming.FixedRoutingID(0x5e97e9))
- ln, ep, err := server.Listen(protocol, "127.0.0.1:0")
+ ln, ep, err := server.Listen(protocol, "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -325,7 +321,7 @@
func TestShutdown(t *testing.T) {
server := InternalNew(naming.FixedRoutingID(0x5e97e9))
- ln, _, err := server.Listen("tcp", "127.0.0.1:0")
+ ln, _, err := server.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -335,7 +331,7 @@
t.Errorf("expecting %d listeners, got %d for %s", n, expect, debugString(server))
}
server.Shutdown()
- if _, _, err := server.Listen("tcp", "127.0.0.1:0"); err == nil {
+ if _, _, err := server.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("test")); err == nil {
t.Error("server should have shut down")
}
if n, expect := numListeners(server), 0; n != expect {
@@ -355,7 +351,7 @@
server := InternalNew(naming.FixedRoutingID(0x55555555))
client := InternalNew(naming.FixedRoutingID(0xcccccccc))
- ln, ep, err := server.Listen(protocol, "127.0.0.1:0")
+ ln, ep, err := server.Listen(protocol, "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -379,13 +375,13 @@
/* TLS + resumption + channel bindings is broken: <https://secure-resumption.com/#channelbindings>.
func TestSessionTicketCache(t *testing.T) {
server := InternalNew(naming.FixedRoutingID(0x55555555))
- _, ep, err := server.Listen("tcp", "127.0.0.1:0", newPrincipal("server"))
+ _, ep, err := server.Listen("tcp", "127.0.0.1:0", tsecurity.NewPrincipal("server"))
if err != nil {
t.Fatal(err)
}
client := InternalNew(naming.FixedRoutingID(0xcccccccc))
- if _, err = client.Dial(ep, newPrincipal("TestSessionTicketCacheClient")); err != nil {
+ if _, err = client.Dial(ep, tsecurity.NewPrincipal("TestSessionTicketCacheClient")); err != nil {
t.Fatalf("Dial(%q) failed: %v", ep, err)
}
@@ -404,7 +400,7 @@
// Have the server read from each flow and write to rchan.
rchan := make(chan string)
- ln, ep, err := server.Listen(protocol, "127.0.0.1:0")
+ ln, ep, err := server.Listen(protocol, "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -492,7 +488,7 @@
// resolve IPv6 addresses.
// As of April 2014, https://developers.google.com/compute/docs/networking
// said that IPv6 is not yet supported.
- ln, ep, err := server.Listen("tcp4", "127.0.0.1:0")
+ ln, ep, err := server.Listen("tcp4", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -576,7 +572,7 @@
func runServer(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
server := InternalNew(naming.FixedRoutingID(0x55555555))
- _, ep, err := server.Listen(args[0], args[1])
+ _, ep, err := server.Listen(args[0], args[1], tsecurity.NewPrincipal("test"))
if err != nil {
fmt.Fprintln(stderr, err)
return err
@@ -623,12 +619,12 @@
}
ipc.RegisterProtocol("tn", dialer, listener)
- _, _, err := server.Listen("tnx", "127.0.0.1:0")
+ _, _, err := server.Listen("tnx", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err == nil || !strings.Contains(err.Error(), "unknown network tnx") {
t.Fatal("expected error is missing (%v)", err)
}
- _, _, err = server.Listen("tn", "127.0.0.1:0")
+ _, _, err = server.Listen("tn", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err == nil || !strings.Contains(err.Error(), "tn.Listen") {
t.Fatal("expected error is missing (%v)", err)
}
@@ -642,7 +638,7 @@
t.Errorf("got %t, want %t", got, want)
}
- _, ep, err := server.Listen("tn", "127.0.0.1:0")
+ _, ep, err := server.Listen("tn", "127.0.0.1:0", tsecurity.NewPrincipal("test"))
if err != nil {
t.Errorf("unexpected error %s", err)
}
@@ -655,45 +651,49 @@
func TestBlessingNamesInEndpoint(t *testing.T) {
var (
- p = newPrincipal("default")
- b1, _ = p.Principal.BlessSelf("dev.v.io/users/foo@bar.com/devices/desktop/app/myapp")
- b2, _ = p.Principal.BlessSelf("otherblessing")
+ p = tsecurity.NewPrincipal("default")
+ b1, _ = p.BlessSelf("dev.v.io/users/foo@bar.com/devices/desktop/app/myapp")
+ b2, _ = p.BlessSelf("otherblessing")
b, _ = security.UnionOfBlessings(b1, b2)
bopt = options.ServerBlessings{b}
server = InternalNew(naming.FixedRoutingID(0x1))
tests = []struct {
+ principal security.Principal
opts []stream.ListenerOpt
blessings []string
err bool
}{
{
// Use the default blessings when only a principal is provided
- opts: []stream.ListenerOpt{p},
+ principal: p,
blessings: []string{"default"},
},
{
// Respect options.ServerBlessings if provided
- opts: []stream.ListenerOpt{p, bopt},
+ principal: p,
+ opts: []stream.ListenerOpt{bopt},
blessings: []string{"dev.v.io/users/foo@bar.com/devices/desktop/app/myapp", "otherblessing"},
},
{
- // It is an error to provide options.ServerBlessings without vc.LocalPrincipal
- opts: []stream.ListenerOpt{bopt},
- err: true,
+ // It is an error to provide options.ServerBlessings without passing a principal
+ principal: nil,
+ opts: []stream.ListenerOpt{bopt},
+ err: true,
},
{
- // It is an error to provide inconsistent options.ServerBlessings and vc.LocalPrincipal
- opts: []stream.ListenerOpt{newPrincipal("random"), bopt},
- err: true,
+ // It is an error to provide inconsistent options.ServerBlessings and principal
+ principal: tsecurity.NewPrincipal("random"),
+ opts: []stream.ListenerOpt{bopt},
+ err: true,
},
}
)
// p must recognize its own blessings!
p.AddToRoots(bopt.Blessings)
for idx, test := range tests {
- ln, ep, err := server.Listen("tcp", "127.0.0.1:0", test.opts...)
+ ln, ep, err := server.Listen("tcp", "127.0.0.1:0", test.principal, test.opts...)
if (err != nil) != test.err {
t.Errorf("test #%d: Got error %v, wanted error: %v", idx, err, test.err)
}
diff --git a/profiles/internal/ipc/stream/model.go b/profiles/internal/ipc/stream/model.go
index 630fe1e..ddbc8e5 100644
--- a/profiles/internal/ipc/stream/model.go
+++ b/profiles/internal/ipc/stream/model.go
@@ -111,14 +111,20 @@
// with the provided network address.
//
// For example:
- // ln, ep, err := Listen("tcp", ":0")
+ // ln, ep, err := Listen("tcp", ":0", principal)
// for {
// flow, err := ln.Accept()
// // process flow
// }
// can be used to accept Flows initiated by remote processes to the endpoint
// identified by the returned Endpoint.
- Listen(protocol, address string, opts ...ListenerOpt) (Listener, naming.Endpoint, error)
+ //
+ // principal is used during authentication. If principal is nil, then the Listener
+ // expects to be used for unauthenticated, unencrypted communication.
+ // If no Blessings are provided via v23.options.ServerBlessings, the principal's
+ // default Blessings will be presented to the Client.
+ // TODO(suharshs): Pass Blessings in explicitly instead of relying on ServerBlessings opt.
+ Listen(protocol, address string, principal security.Principal, opts ...ListenerOpt) (Listener, naming.Endpoint, error)
// Dial creates a VC to the provided remote endpoint.
Dial(remote naming.Endpoint, opts ...VCOpt) (VC, error)
diff --git a/profiles/internal/ipc/stream/proxy/proxy.go b/profiles/internal/ipc/stream/proxy/proxy.go
index f3c50ac..8189083 100644
--- a/profiles/internal/ipc/stream/proxy/proxy.go
+++ b/profiles/internal/ipc/stream/proxy/proxy.go
@@ -563,7 +563,7 @@
p.proxy.routeCounters(p, m.Counters)
if vcObj != nil {
server := &server{Process: p, VC: vcObj}
- go p.proxy.runServer(server, vcObj.HandshakeAcceptedVC(vc.LocalPrincipal{p.proxy.principal}))
+ go p.proxy.runServer(server, vcObj.HandshakeAcceptedVC(p.proxy.principal))
}
break
}
diff --git a/profiles/internal/ipc/stream/proxy/proxy_test.go b/profiles/internal/ipc/stream/proxy/proxy_test.go
index 7cb86e0..cc6d906 100644
--- a/profiles/internal/ipc/stream/proxy/proxy_test.go
+++ b/profiles/internal/ipc/stream/proxy/proxy_test.go
@@ -15,13 +15,13 @@
_ "v.io/x/ref/profiles"
"v.io/x/ref/profiles/internal/ipc/stream/manager"
"v.io/x/ref/profiles/internal/ipc/stream/proxy"
- "v.io/x/ref/profiles/internal/ipc/stream/vc"
)
//go:generate v23 test generate
func TestProxy(t *testing.T) {
- shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), nil, "tcp", "127.0.0.1:0", "")
+ pproxy := tsecurity.NewPrincipal("proxy")
+ shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), pproxy, "tcp", "127.0.0.1:0", "")
if err != nil {
t.Fatal(err)
}
@@ -32,7 +32,7 @@
defer server1.Shutdown()
// Setup a stream.Listener that will accept VCs and Flows routed
// through the proxy.
- ln1, ep1, err := server1.Listen(proxyEp.Network(), proxyEp.String())
+ ln1, ep1, err := server1.Listen(proxyEp.Network(), proxyEp.String(), tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -43,7 +43,7 @@
defer server2.Shutdown()
// Setup a stream.Listener that will accept VCs and Flows routed
// through the proxy.
- ln2, ep2, err := server2.Listen(proxyEp.Network(), proxyEp.String())
+ ln2, ep2, err := server2.Listen(proxyEp.Network(), proxyEp.String(), tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -81,7 +81,8 @@
}
func TestDuplicateRoutingID(t *testing.T) {
- shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), nil, "tcp", "127.0.0.1:0", "")
+ pproxy := tsecurity.NewPrincipal("proxy")
+ shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), pproxy, "tcp", "127.0.0.1:0", "")
if err != nil {
t.Fatal(err)
}
@@ -95,13 +96,13 @@
defer server2.Shutdown()
// First server to claim serverRID should win.
- ln1, ep1, err := server1.Listen(proxyEp.Network(), proxyEp.String())
+ ln1, ep1, err := server1.Listen(proxyEp.Network(), proxyEp.String(), tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
defer ln1.Close()
- ln2, ep2, err := server2.Listen(proxyEp.Network(), proxyEp.String())
+ ln2, ep2, err := server2.Listen(proxyEp.Network(), proxyEp.String(), tsecurity.NewPrincipal("test"))
if pattern := "routing id 00000000000000005555555555555555 is already being proxied"; err == nil || !strings.Contains(err.Error(), pattern) {
t.Errorf("Got (%v, %v, %v) want error \"...%v\" (ep1:%v)", ln2, ep2, err, pattern, ep1)
}
@@ -151,7 +152,7 @@
server := manager.InternalNew(naming.FixedRoutingID(0x5555555555555555))
defer server.Shutdown()
- ln, ep, err := server.Listen(proxyEp.Network(), proxyEp.String(), vc.LocalPrincipal{pserver})
+ ln, ep, err := server.Listen(proxyEp.Network(), proxyEp.String(), pserver)
if err != nil {
t.Fatal(err)
}
@@ -183,7 +184,8 @@
}
func TestHostPort(t *testing.T) {
- shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), nil, "tcp", "127.0.0.1:0", "")
+ pproxy := tsecurity.NewPrincipal("proxy")
+ shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), pproxy, "tcp", "127.0.0.1:0", "")
if err != nil {
t.Fatal(err)
}
@@ -192,7 +194,7 @@
defer server.Shutdown()
addr := proxyEp.Addr().String()
port := addr[strings.LastIndex(addr, ":"):]
- ln, _, err := server.Listen("veyron", "127.0.0.1"+port)
+ ln, _, err := server.Listen("veyron", "127.0.0.1"+port, tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -200,7 +202,8 @@
}
func TestClientBecomesServer(t *testing.T) {
- shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), nil, "tcp", "127.0.0.1:0", "")
+ pproxy := tsecurity.NewPrincipal("proxy")
+ shutdown, proxyEp, err := proxy.InternalNew(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), pproxy, "tcp", "127.0.0.1:0", "")
if err != nil {
t.Fatal(err)
}
@@ -212,7 +215,7 @@
defer client1.Shutdown()
defer client2.Shutdown()
- lnS, epS, err := server.Listen(proxyEp.Network(), proxyEp.String())
+ lnS, epS, err := server.Listen(proxyEp.Network(), proxyEp.String(), tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
@@ -233,7 +236,7 @@
}
// Now client1 becomes a server
- lnC, epC, err := client1.Listen(proxyEp.Network(), proxyEp.String())
+ lnC, epC, err := client1.Listen(proxyEp.Network(), proxyEp.String(), tsecurity.NewPrincipal("test"))
if err != nil {
t.Fatal(err)
}
diff --git a/profiles/internal/ipc/stream/vc/vc.go b/profiles/internal/ipc/stream/vc/vc.go
index c994b3a..bbad982 100644
--- a/profiles/internal/ipc/stream/vc/vc.go
+++ b/profiles/internal/ipc/stream/vc/vc.go
@@ -142,10 +142,8 @@
// when establishing virtual circuits.
type LocalPrincipal struct{ security.Principal }
-func (LocalPrincipal) IPCStreamListenerOpt() {}
-func (LocalPrincipal) IPCStreamVCOpt() {}
-func (LocalPrincipal) IPCClientOpt() {}
-func (LocalPrincipal) IPCServerOpt() {}
+func (LocalPrincipal) IPCStreamVCOpt() {}
+func (LocalPrincipal) IPCClientOpt() {}
// DischargeClient is an interface for obtaining discharges for a set of third-party
// caveats.
@@ -504,14 +502,18 @@
// Since the handshaking process might involve several round trips, a bulk of the work
// is done asynchronously and the result of the handshake is written to the
// channel returned by this method.
-func (vc *VC) HandshakeAcceptedVC(opts ...stream.ListenerOpt) <-chan HandshakeResult {
+//
+// principal is server's used during authentication. If principal is nil, then the VC
+// expects to be used for unauthenticated, unencrypted communication.
+// If no Blessings are provided via v23.options.ServerBlessings, the principal's
+// default Blessings will be presented to the Client.
+func (vc *VC) HandshakeAcceptedVC(principal security.Principal, opts ...stream.ListenerOpt) <-chan HandshakeResult {
result := make(chan HandshakeResult, 1)
finish := func(ln stream.Listener, err error) chan HandshakeResult {
result <- HandshakeResult{ln, err}
return result
}
var (
- principal security.Principal
securityLevel options.VCSecurityLevel
dischargeClient DischargeClient
lBlessings security.Blessings
@@ -522,8 +524,6 @@
switch v := o.(type) {
case DischargeClient:
dischargeClient = v
- case LocalPrincipal:
- principal = v.Principal
case options.VCSecurityLevel:
securityLevel = v
case options.ServerBlessings:
@@ -543,7 +543,7 @@
switch securityLevel {
case options.VCSecurityConfidential:
if principal == nil {
- principal = AnonymousPrincipal
+ return finish(nil, fmt.Errorf("principal required for VCSecurityConfidential"))
}
if lBlessings.IsZero() {
lBlessings = principal.BlessingStore().Default()
diff --git a/profiles/internal/ipc/stream/vc/vc_test.go b/profiles/internal/ipc/stream/vc/vc_test.go
index 9a9ff00..40d549d 100644
--- a/profiles/internal/ipc/stream/vc/vc_test.go
+++ b/profiles/internal/ipc/stream/vc/vc_test.go
@@ -472,7 +472,7 @@
go clientH.pipeLoop(serverH.VC)
go serverH.pipeLoop(clientH.VC)
- lopts := []stream.ListenerOpt{vc.LocalPrincipal{server}, security}
+ lopts := []stream.ListenerOpt{security}
vcopts := []stream.VCOpt{vc.LocalPrincipal{client}, security}
if dischargeClient != nil {
@@ -482,7 +482,7 @@
vcopts = append(vcopts, auth)
}
- c := serverH.VC.HandshakeAcceptedVC(lopts...)
+ c := serverH.VC.HandshakeAcceptedVC(server, lopts...)
if err := clientH.VC.HandshakeDialedVC(vcopts...); err != nil {
go func() { <-c }()
return nil, nil, err
diff --git a/profiles/internal/ipc/stream/vif/auth.go b/profiles/internal/ipc/stream/vif/auth.go
index 62914ab..25a5414 100644
--- a/profiles/internal/ipc/stream/vif/auth.go
+++ b/profiles/internal/ipc/stream/vif/auth.go
@@ -187,15 +187,17 @@
return c, nil
}
-// serverAuthOptions extracts the Principal from the options list.
-func serverAuthOptions(lopts []stream.ListenerOpt) (principal security.Principal, lBlessings security.Blessings, dischargeClient vc.DischargeClient, err error) {
- var securityLevel options.VCSecurityLevel
+// serverAuthOptions returns credentials for VIF authentication, based on the provided principal and options list.
+func serverAuthOptions(principal security.Principal, lopts []stream.ListenerOpt) (security.Principal, security.Blessings, vc.DischargeClient, error) {
+ var (
+ securityLevel options.VCSecurityLevel
+ dischargeClient vc.DischargeClient
+ lBlessings security.Blessings
+ )
for _, o := range lopts {
switch v := o.(type) {
case vc.DischargeClient:
dischargeClient = v
- case vc.LocalPrincipal:
- principal = v.Principal
case options.VCSecurityLevel:
securityLevel = v
case options.ServerBlessings:
@@ -204,18 +206,15 @@
}
switch securityLevel {
case options.VCSecurityConfidential:
- if principal == nil {
- principal = vc.AnonymousPrincipal
- }
if lBlessings.IsZero() {
lBlessings = principal.BlessingStore().Default()
}
+ return principal, lBlessings, dischargeClient, nil
case options.VCSecurityNone:
- principal = nil
+ return nil, security.Blessings{}, nil, nil
default:
- err = fmt.Errorf("unrecognized VC security level: %v", securityLevel)
+ return nil, security.Blessings{}, nil, fmt.Errorf("unrecognized VC security level: %v", securityLevel)
}
- return
}
// makeHopSetup constructs the options that this process can support.
diff --git a/profiles/internal/ipc/stream/vif/set_test.go b/profiles/internal/ipc/stream/vif/set_test.go
index 0a45f12..e00a51b 100644
--- a/profiles/internal/ipc/stream/vif/set_test.go
+++ b/profiles/internal/ipc/stream/vif/set_test.go
@@ -12,6 +12,7 @@
"v.io/v23/ipc"
"v.io/v23/naming"
+ tsecurity "v.io/x/ref/lib/testutil/security"
_ "v.io/x/ref/profiles"
"v.io/x/ref/profiles/internal/ipc/stream/vif"
)
@@ -59,7 +60,7 @@
func newVIF(c, s net.Conn) (*vif.VIF, *vif.VIF, error) {
done := make(chan *vif.VIF)
go func() {
- vf, err := vif.InternalNewAcceptedVIF(s, naming.FixedRoutingID(0x5), nil)
+ vf, err := vif.InternalNewAcceptedVIF(s, naming.FixedRoutingID(0x5), tsecurity.NewPrincipal("test"), nil)
if err != nil {
panic(err)
}
diff --git a/profiles/internal/ipc/stream/vif/vif.go b/profiles/internal/ipc/stream/vif/vif.go
index dd2c703..97bbe65 100644
--- a/profiles/internal/ipc/stream/vif/vif.go
+++ b/profiles/internal/ipc/stream/vif/vif.go
@@ -65,6 +65,7 @@
muListen sync.Mutex
acceptor *upcqueue.T // GUARDED_BY(muListen)
listenerOpts []stream.ListenerOpt // GUARDED_BY(muListen)
+ principal security.Principal
muNextVCI sync.Mutex
nextVCI id.VC
@@ -153,7 +154,7 @@
if err != nil {
return nil, err
}
- return internalNew(conn, pool, reader, rid, id.VC(vc.NumReservedVCs), versions, nil, nil, c)
+ return internalNew(conn, pool, reader, rid, id.VC(vc.NumReservedVCs), versions, principal, nil, nil, c)
}
// InternalNewAcceptedVIF creates a new virtual interface over the provided
@@ -166,13 +167,13 @@
// As the name suggests, this method is intended for use only within packages
// placed inside veyron/profiles/internal. Code outside the
// veyron/profiles/internal/* packages should never call this method.
-func InternalNewAcceptedVIF(conn net.Conn, rid naming.RoutingID, versions *version.Range, lopts ...stream.ListenerOpt) (*VIF, error) {
+func InternalNewAcceptedVIF(conn net.Conn, rid naming.RoutingID, principal security.Principal, versions *version.Range, lopts ...stream.ListenerOpt) (*VIF, error) {
pool := iobuf.NewPool(0)
reader := iobuf.NewReader(pool, conn)
- return internalNew(conn, pool, reader, rid, id.VC(vc.NumReservedVCs)+1, versions, upcqueue.New(), lopts, &crypto.NullControlCipher{})
+ return internalNew(conn, pool, reader, rid, id.VC(vc.NumReservedVCs)+1, versions, principal, upcqueue.New(), lopts, &crypto.NullControlCipher{})
}
-func internalNew(conn net.Conn, pool *iobuf.Pool, reader *iobuf.Reader, rid naming.RoutingID, initialVCI id.VC, versions *version.Range, acceptor *upcqueue.T, listenerOpts []stream.ListenerOpt, c crypto.ControlCipher) (*VIF, error) {
+func internalNew(conn net.Conn, pool *iobuf.Pool, reader *iobuf.Reader, rid naming.RoutingID, initialVCI id.VC, versions *version.Range, principal security.Principal, acceptor *upcqueue.T, listenerOpts []stream.ListenerOpt, c crypto.ControlCipher) (*VIF, error) {
var (
// Choose IDs that will not conflict with any other (VC, Flow)
// pairs. VCI 0 is never used by the application (it is
@@ -210,6 +211,7 @@
vcMap: newVCMap(),
acceptor: acceptor,
listenerOpts: listenerOpts,
+ principal: principal,
localEP: localEP(conn, rid, versions),
nextVCI: initialVCI,
outgoing: outgoing,
@@ -448,7 +450,7 @@
})
return nil
}
- go vif.acceptFlowsLoop(vc, vc.HandshakeAcceptedVC(lopts...))
+ go vif.acceptFlowsLoop(vc, vc.HandshakeAcceptedVC(vif.principal, lopts...))
case *message.SetupVC:
// TODO(ashankar,mattr): Handle this! See comment about SetupVC
// in vif.Dial
@@ -491,7 +493,7 @@
return errAlreadySetup
}
vif.muListen.Lock()
- principal, lBlessings, dischargeClient, err := serverAuthOptions(vif.listenerOpts)
+ principal, lBlessings, dischargeClient, err := serverAuthOptions(vif.principal, vif.listenerOpts)
vif.muListen.Unlock()
if err != nil {
return errVersionNegotiationFailed
diff --git a/profiles/internal/ipc/stream/vif/vif_test.go b/profiles/internal/ipc/stream/vif/vif_test.go
index 132aa11..959ae0b 100644
--- a/profiles/internal/ipc/stream/vif/vif_test.go
+++ b/profiles/internal/ipc/stream/vif/vif_test.go
@@ -389,7 +389,7 @@
}
result <- client
}()
- server, err := vif.InternalNewAcceptedVIF(c2, naming.FixedRoutingID(0x5), nil)
+ server, err := vif.InternalNewAcceptedVIF(c2, naming.FixedRoutingID(0x5), tsecurity.NewPrincipal("test"), nil)
if err != nil {
t.Fatal(err)
}
@@ -500,7 +500,7 @@
cl <- c
}
}()
- s, err := vif.InternalNewAcceptedVIF(c2, naming.FixedRoutingID(0x5), serverVersions, newPrincipal("server"))
+ s, err := vif.InternalNewAcceptedVIF(c2, naming.FixedRoutingID(0x5), tsecurity.NewPrincipal("server"), serverVersions)
c, ok := <-cl
if err != nil {
verr = err
diff --git a/profiles/internal/ipc/v23_test.go b/profiles/internal/ipc/v23_test.go
index e416039..a5def16 100644
--- a/profiles/internal/ipc/v23_test.go
+++ b/profiles/internal/ipc/v23_test.go
@@ -20,7 +20,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/profiles/internal/rt/ipc_test.go b/profiles/internal/rt/ipc_test.go
index eaa9615..36ec91f 100644
--- a/profiles/internal/rt/ipc_test.go
+++ b/profiles/internal/rt/ipc_test.go
@@ -239,7 +239,9 @@
// no proxies were started. Anyway, just to express the
// intent...
for _, ep := range status.Endpoints {
- if got := ep.BlessingNames(); !reflect.DeepEqual(got, want) {
+ got := ep.BlessingNames()
+ sort.Strings(got)
+ if !reflect.DeepEqual(got, want) {
t.Errorf("test #%d: endpoint=%q: Got blessings %v, want %v", idx, ep, got, want)
}
}
diff --git a/profiles/internal/rt/mgmt.go b/profiles/internal/rt/mgmt.go
index 34f1b4d..935120c 100644
--- a/profiles/internal/rt/mgmt.go
+++ b/profiles/internal/rt/mgmt.go
@@ -24,10 +24,14 @@
} else if err != nil {
return err
}
-
parentName, err := handle.Config.Get(mgmt.ParentNameConfigKey)
if err != nil {
- return nil
+ // If the ParentNameConfigKey is not set, then this process has
+ // not been started by the device manager, but the parent process
+ // is still a Vanadium process using the exec library so we
+ // call SetReady to let it know that this child process has
+ // successfully started.
+ return handle.SetReady()
}
listenSpec, err := getListenSpec(handle)
if err != nil {
@@ -58,7 +62,6 @@
server.Stop()
return err
}
-
return handle.SetReady()
}
diff --git a/profiles/internal/rt/runtime.go b/profiles/internal/rt/runtime.go
index d065cb0..8eba35a 100644
--- a/profiles/internal/rt/runtime.go
+++ b/profiles/internal/rt/runtime.go
@@ -232,7 +232,6 @@
client, _ := ctx.Value(clientKey).(ipc.Client)
otherOpts := append([]ipc.ServerOpt{}, opts...)
- otherOpts = append(otherOpts, vc.LocalPrincipal{principal})
if reserved, ok := ctx.Value(reservedNameKey).(*reservedNameDispatcher); ok {
otherOpts = append(otherOpts, iipc.ReservedNameDispatcher{reserved.dispatcher})
otherOpts = append(otherOpts, reserved.opts...)
@@ -244,7 +243,7 @@
if !hasServerBlessingsOpt(opts) && principal != nil {
otherOpts = append(otherOpts, options.ServerBlessings{principal.BlessingStore().Default()})
}
- server, err := iipc.InternalNewServer(ctx, sm, ns, r.GetClient(ctx), otherOpts...)
+ server, err := iipc.InternalNewServer(ctx, sm, ns, r.GetClient(ctx), principal, otherOpts...)
if err != nil {
return nil, err
}
diff --git a/profiles/internal/rt/v23_test.go b/profiles/internal/rt/v23_test.go
index 4ca1965..f1d3bd1 100644
--- a/profiles/internal/rt/v23_test.go
+++ b/profiles/internal/rt/v23_test.go
@@ -35,7 +35,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/profiles/internal/vtrace/vtrace_test.go b/profiles/internal/vtrace/vtrace_test.go
index 14afa92..834af71 100644
--- a/profiles/internal/vtrace/vtrace_test.go
+++ b/profiles/internal/vtrace/vtrace_test.go
@@ -15,6 +15,7 @@
"v.io/x/lib/vlog"
"v.io/x/ref/lib/testutil"
+ tsecurity "v.io/x/ref/lib/testutil/security"
_ "v.io/x/ref/profiles"
iipc "v.io/x/ref/profiles/internal/ipc"
"v.io/x/ref/profiles/internal/ipc/stream"
@@ -90,7 +91,7 @@
if err != nil {
return nil, err
}
- s, err := iipc.InternalNewServer(ctx, sm, ns, client)
+ s, err := iipc.InternalNewServer(ctx, sm, ns, client, tsecurity.NewPrincipal("test"))
if err != nil {
return nil, err
}
diff --git a/security/agent/v23_test.go b/security/agent/v23_test.go
index bdcc989..68dcf3c 100644
--- a/security/agent/v23_test.go
+++ b/security/agent/v23_test.go
@@ -20,7 +20,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/security/flag/v23_internal_test.go b/security/flag/v23_internal_test.go
index 9a2e9f0..1f17192 100644
--- a/security/flag/v23_internal_test.go
+++ b/security/flag/v23_internal_test.go
@@ -19,7 +19,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/services/identity/oauth/googleoauth.go b/services/identity/oauth/googleoauth.go
index 5feb74e..a543d5e 100644
--- a/services/identity/oauth/googleoauth.go
+++ b/services/identity/oauth/googleoauth.go
@@ -79,7 +79,9 @@
if err := json.NewDecoder(tinfo.Body).Decode(>oken); err != nil {
return "", fmt.Errorf("invalid JSON response from Google's tokeninfo API: %v", err)
}
- if !gtoken.VerifiedEmail {
+ // We check both "verified_email" and "email_verified" here because the token response sometimes
+ // contains one and sometimes contains the other.
+ if !gtoken.VerifiedEmail && !gtoken.EmailVerified {
return "", fmt.Errorf("email not verified: %#v", gtoken)
}
if gtoken.Issuer != "accounts.google.com" {
@@ -116,6 +118,7 @@
ExpiresIn int64 `json:"expires_in"`
Email string `json:"email"`
VerifiedEmail bool `json:"verified_email"`
+ EmailVerified bool `json:"email_verified"`
AccessType string `json:"access_type"`
}
if err := json.NewDecoder(tokeninfo.Body).Decode(&token); err != nil {
@@ -134,7 +137,9 @@
vlog.Infof("Got access token [%+v], wanted one of client ids %v", token, accessTokenClients)
return "", "", fmt.Errorf("token not meant for this purpose, confused deputy? https://developers.google.com/accounts/docs/OAuth2UserAgent#validatetoken")
}
- if !token.VerifiedEmail {
+ // We check both "verified_email" and "email_verified" here because the token response sometimes
+ // contains one and sometimes contains the other.
+ if !token.VerifiedEmail && !token.EmailVerified {
return "", "", fmt.Errorf("email not verified")
}
return token.Email, client.Name, nil
@@ -184,4 +189,5 @@
IssuedAt int64 `json:"issued_at"`
Email string `json:"email"`
VerifiedEmail bool `json:"verified_email"`
+ EmailVerified bool `json:"email_verified"`
}
diff --git a/services/mgmt/application/impl/acl_test.go b/services/mgmt/application/impl/acl_test.go
index 9715785..ad9c964 100644
--- a/services/mgmt/application/impl/acl_test.go
+++ b/services/mgmt/application/impl/acl_test.go
@@ -85,8 +85,8 @@
storedir, cleanup := mgmttest.SetupRootDir(t, "application")
defer cleanup()
- _, nms := mgmttest.RunShellCommand(t, sh, nil, repoCmd, "repo", storedir)
- pid := mgmttest.ReadPID(t, nms)
+ nmh := mgmttest.RunCommand(t, sh, nil, repoCmd, "repo", storedir)
+ pid := mgmttest.ReadPID(t, nmh)
defer syscall.Kill(pid, syscall.SIGINT)
otherCtx, err := v23.SetPrincipal(ctx, tsecurity.NewPrincipal())
@@ -222,8 +222,8 @@
t.Fatal(err)
}
- _, nms := mgmttest.RunShellCommand(t, sh, nil, repoCmd, "repo", storedir)
- pid := mgmttest.ReadPID(t, nms)
+ nmh := mgmttest.RunCommand(t, sh, nil, repoCmd, "repo", storedir)
+ pid := mgmttest.ReadPID(t, nmh)
defer syscall.Kill(pid, syscall.SIGINT)
// Create example envelope.
diff --git a/services/mgmt/application/impl/v23_test.go b/services/mgmt/application/impl/v23_test.go
index ad4161e..125eb9e 100644
--- a/services/mgmt/application/impl/v23_test.go
+++ b/services/mgmt/application/impl/v23_test.go
@@ -19,7 +19,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/services/mgmt/binary/impl/acl_test.go b/services/mgmt/binary/impl/acl_test.go
index 281a594..62393b5 100644
--- a/services/mgmt/binary/impl/acl_test.go
+++ b/services/mgmt/binary/impl/acl_test.go
@@ -97,8 +97,8 @@
defer cleanup()
prepDirectory(t, storedir)
- _, nms := mgmttest.RunShellCommand(t, sh, nil, binaryCmd, "bini", storedir)
- pid := mgmttest.ReadPID(t, nms)
+ nmh := mgmttest.RunCommand(t, sh, nil, binaryCmd, "bini", storedir)
+ pid := mgmttest.ReadPID(t, nmh)
defer syscall.Kill(pid, syscall.SIGINT)
vlog.VI(2).Infof("Self uploads a shared and private binary.")
@@ -154,8 +154,8 @@
t.Fatalf("SetPrincipal() failed: %v", err)
}
- _, nms := mgmttest.RunShellCommand(t, sh, nil, binaryCmd, "bini", storedir)
- pid := mgmttest.ReadPID(t, nms)
+ nmh := mgmttest.RunCommand(t, sh, nil, binaryCmd, "bini", storedir)
+ pid := mgmttest.ReadPID(t, nmh)
defer syscall.Kill(pid, syscall.SIGINT)
vlog.VI(2).Infof("Self uploads a shared and private binary.")
@@ -434,8 +434,8 @@
t.Fatalf("otherPrincipal.AddToRoots() failed: %v", err)
}
- _, nms := mgmttest.RunShellCommand(t, sh, nil, binaryCmd, "bini", storedir)
- pid := mgmttest.ReadPID(t, nms)
+ nmh := mgmttest.RunCommand(t, sh, nil, binaryCmd, "bini", storedir)
+ pid := mgmttest.ReadPID(t, nmh)
defer syscall.Kill(pid, syscall.SIGINT)
acl, tag, err := b("bini").GetACL(selfCtx)
diff --git a/services/mgmt/binary/impl/v23_test.go b/services/mgmt/binary/impl/v23_test.go
index 9c0642e..a55f54f 100644
--- a/services/mgmt/binary/impl/v23_test.go
+++ b/services/mgmt/binary/impl/v23_test.go
@@ -19,7 +19,7 @@
func TestMain(m *testing.M) {
testutil.Init()
- if modules.IsModulesProcess() {
+ if modules.IsModulesChildProcess() {
if err := modules.Dispatch(); err != nil {
fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
os.Exit(1)
diff --git a/services/mgmt/device/impl/debug_acls_test.go b/services/mgmt/device/impl/debug_acls_test.go
new file mode 100644
index 0000000..f6811c2
--- /dev/null
+++ b/services/mgmt/device/impl/debug_acls_test.go
@@ -0,0 +1,112 @@
+package impl_test
+
+import (
+ "syscall"
+ "testing"
+
+ "v.io/v23/context"
+ "v.io/v23/security"
+ "v.io/v23/services/security/access"
+ "v.io/v23/vdl"
+
+ mgmttest "v.io/x/ref/services/mgmt/lib/testutil"
+)
+
+func updateACL(t *testing.T, ctx *context.T, blessing, right string, name ...string) {
+ acl, etag, err := appStub(name...).GetACL(ctx)
+ if err != nil {
+ t.Fatalf("GetACL(%v) failed %v", name, err)
+ }
+ acl.Add(security.BlessingPattern(blessing), right)
+ if err = appStub(name...).SetACL(ctx, acl, etag); err != nil {
+ t.Fatalf("SetACL(%v, %v, %v) failed: %v", name, blessing, right, err)
+ }
+}
+
+func TestDebugACLPropagation(t *testing.T) {
+ cleanup, ctx, sh, envelope, root, helperPath, idp := startupHelper(t)
+ defer cleanup()
+
+ // Set up the device manager.
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ mgmttest.ReadPID(t, dmh)
+ claimDevice(t, ctx, "dm", "mydevice", noPairingToken)
+
+ // Create the local server that the app uses to let us know it's ready.
+ pingCh, cleanup := setupPingServer(t, ctx)
+ defer cleanup()
+ resolve(t, ctx, "pingserver", 1)
+
+ // Make some users.
+ selfCtx := ctx
+ bobCtx := ctxWithNewPrincipal(t, selfCtx, idp, "bob")
+ hjCtx := ctxWithNewPrincipal(t, selfCtx, idp, "hackerjoe")
+ aliceCtx := ctxWithNewPrincipal(t, selfCtx, idp, "alice")
+
+ // TODO(rjkroege): Set ACLs here that conflict with the one provided by the device
+ // manager and show that the one set here is overridden.
+ // Create the envelope for the first version of the app.
+ *envelope = envelopeFromShell(sh, nil, appCmd, "google naps", "appV1")
+
+ // Install the app.
+ appID := installApp(t, ctx)
+
+ // Give bob rights to start an app.
+ updateACL(t, selfCtx, "root/bob/$", string(access.Read), appID)
+
+ // Bob starts an instance of the app.
+ bobApp := startApp(t, bobCtx, appID)
+ verifyPingArgs(t, pingCh, userName(t), "default", "")
+
+ // Bob permits Alice to read from his app.
+ updateACL(t, bobCtx, "root/alice/$", string(access.Read), appID, bobApp)
+
+ // Confirm that self can access stats name (i.e. apps proxied from
+ // the __debug space of the app.
+ // TODO(rjkroege): validate each of the services under __debug.
+ v, err := statsStub(appID, bobApp, "stats/system/pid").Value(ctx)
+ if err != nil {
+ t.Fatalf("Value() failed: %v\n", err)
+ }
+ var pid int
+ if err := vdl.Convert(&pid, v); err != nil {
+ t.Fatalf("pid returned from stats interface is not an int: %v", err)
+ }
+
+ // Bob has an issue with his app and tries to use the debug output to figure it out.
+ // TODO(rjkroege): Invert this test when ACLs are correctly propagated.
+ if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(bobCtx); err == nil {
+ t.Fatalf("This ought not to work yet! Something is wrong.")
+ }
+
+ // But Bob can't figure it out and hopes that hackerjoe can debug it.
+ updateACL(t, bobCtx, "root/hackerjoe/$", string(access.Debug), appID, bobApp)
+
+ // But the device manager doesn't support this so hackerjoe can't help.
+ // TODO(rjkroege): Invert this test when ACLs are propagated..
+ if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(hjCtx); err == nil {
+ t.Fatalf("This ought not to work yet! Something is wrong.")
+ }
+
+ // Alice might be able to help but Bob didn't give Alice access to the debug ACLs.
+ if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(hjCtx); err == nil {
+ t.Fatalf("Alice could wrongly access the stats without perms.")
+ }
+
+ // Bob changes the permissions so that Alice can help debug too.
+ updateACL(t, bobCtx, "root/alice/$", string(access.Debug), appID, bobApp)
+
+ // But the device manager doesn't support this so Alice can't help either.
+ // TODO(rjkroege): Invert this test when ACLs are propagated..
+ if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(aliceCtx); err == nil {
+ t.Fatalf("This ought not to work yet! Something is wrong.")
+ }
+
+ // Bob is glum so stops his app.
+ stopApp(t, bobCtx, appID, bobApp)
+
+ // Cleanly shut down the device manager.
+ syscall.Kill(dmh.Pid(), syscall.SIGINT)
+ dmh.Expect("dm terminated")
+ dmh.ExpectEOF()
+}
diff --git a/services/mgmt/device/impl/impl_test.go b/services/mgmt/device/impl/impl_test.go
index 4302dec..0aa8c45 100644
--- a/services/mgmt/device/impl/impl_test.go
+++ b/services/mgmt/device/impl/impl_test.go
@@ -58,9 +58,11 @@
suidhelper "v.io/x/ref/services/mgmt/suidhelper/impl"
)
+//go:generate v23 test generate .
+
const (
// Modules command names.
- execScriptCmd = "execScriptCmd"
+ execScriptCmd = "execScript"
deviceManagerCmd = "deviceManager"
deviceManagerV10Cmd = "deviceManagerV10" // deviceManager with a different major version number
appCmd = "app"
@@ -82,28 +84,21 @@
// The installer sets this flag on the installed device manager, so we
// need to ensure it's defined.
flag.String("name", "", "")
-
- modules.RegisterChild(execScriptCmd, "", execScript)
- modules.RegisterChild(deviceManagerCmd, "", deviceManager)
- modules.RegisterChild(deviceManagerV10Cmd, "", deviceManagerV10)
- modules.RegisterChild(appCmd, "", app)
-
- if modules.IsModulesProcess() {
- return
- }
}
func TestMain(m *testing.M) {
testutil.Init()
+ isSuidHelper := len(os.Getenv("VEYRON_SUIDHELPER_TEST")) > 0
+ if modules.IsModulesChildProcess() && !isSuidHelper {
+ if err := modules.Dispatch(); err != nil {
+ fmt.Fprintf(os.Stderr, "modules.Dispatch failed: %v\n", err)
+ os.Exit(1)
+ }
+ return
+ }
os.Exit(m.Run())
}
-// TestHelperProcess is the entrypoint for the modules commands in a
-// a test subprocess.
-func TestHelperProcess(t *testing.T) {
- modules.DispatchInTest()
-}
-
// TestSuidHelper is testing boilerplate for suidhelper that does not
// create a runtime because the suidhelper is not a Veyron application.
func TestSuidHelper(t *testing.T) {
@@ -111,7 +106,6 @@
return
}
vlog.VI(1).Infof("TestSuidHelper starting")
-
if err := suidhelper.Run(os.Environ()); err != nil {
vlog.Fatalf("Failed to Run() setuidhelper: %v", err)
}
@@ -401,12 +395,12 @@
// demonstrates that the initial device manager could be started by hand
// as long as the right initial configuration is passed into the device
// manager implementation.
- dmh, dms := mgmttest.RunShellCommand(t, sh, dmPauseBeforeStopEnv, deviceManagerCmd, dmArgs...)
+ dmh := mgmttest.RunCommand(t, sh, dmPauseBeforeStopEnv, deviceManagerCmd, dmArgs...)
defer func() {
syscall.Kill(dmh.Pid(), syscall.SIGINT)
}()
- mgmttest.ReadPID(t, dms)
+ mgmttest.ReadPID(t, dmh)
// Brand new device manager must be claimed first.
claimDevice(t, ctx, "factoryDM", "mydevice", noPairingToken)
@@ -439,17 +433,17 @@
dmh.CloseStdin()
- dms.Expect("restart handler")
- dms.Expect("factoryDM terminated")
+ dmh.Expect("restart handler")
+ dmh.Expect("factoryDM terminated")
dmh.Shutdown(os.Stderr, os.Stderr)
// A successful update means the device manager has stopped itself. We
// relaunch it from the current link.
resolveExpectNotFound(t, ctx, "v2DM") // Ensure a clean slate.
- dmh, dms = mgmttest.RunShellCommand(t, sh, dmEnv, execScriptCmd, currLink)
+ dmh = mgmttest.RunCommand(t, sh, dmEnv, execScriptCmd, currLink)
- mgmttest.ReadPID(t, dms)
+ mgmttest.ReadPID(t, dmh)
resolve(t, ctx, "v2DM", 1) // Current link should have been launching v2.
// Try issuing an update without changing the envelope in the
@@ -480,8 +474,8 @@
t.Fatalf("current link didn't change")
}
- dms.Expect("restart handler")
- dms.Expect("v2DM terminated")
+ dmh.Expect("restart handler")
+ dmh.Expect("v2DM terminated")
dmh.Shutdown(os.Stderr, os.Stderr)
@@ -490,17 +484,17 @@
// Re-lanuch the device manager from current link. We instruct the
// device manager to pause before stopping its server, so that we can
// verify that a second revert fails while a revert is in progress.
- dmh, dms = mgmttest.RunShellCommand(t, sh, dmPauseBeforeStopEnv, execScriptCmd, currLink)
+ dmh = mgmttest.RunCommand(t, sh, dmPauseBeforeStopEnv, execScriptCmd, currLink)
- mgmttest.ReadPID(t, dms)
+ mgmttest.ReadPID(t, dmh)
resolve(t, ctx, "v3DM", 1) // Current link should have been launching v3.
// Revert the device manager to its previous version (v2).
revertDevice(t, ctx, "v3DM")
revertDeviceExpectError(t, ctx, "v3DM", impl.ErrOperationInProgress.ID) // Revert already in progress.
dmh.CloseStdin()
- dms.Expect("restart handler")
- dms.Expect("v3DM terminated")
+ dmh.Expect("restart handler")
+ dmh.Expect("v3DM terminated")
if evalLink() != scriptPathV2 {
t.Fatalf("current link was not reverted correctly")
}
@@ -508,14 +502,14 @@
resolveExpectNotFound(t, ctx, "v2DM") // Ensure a clean slate.
- dmh, dms = mgmttest.RunShellCommand(t, sh, dmEnv, execScriptCmd, currLink)
- mgmttest.ReadPID(t, dms)
+ dmh = mgmttest.RunCommand(t, sh, dmEnv, execScriptCmd, currLink)
+ mgmttest.ReadPID(t, dmh)
resolve(t, ctx, "v2DM", 1) // Current link should have been launching v2.
// Revert the device manager to its previous version (factory).
revertDevice(t, ctx, "v2DM")
- dms.Expect("restart handler")
- dms.Expect("v2DM terminated")
+ dmh.Expect("restart handler")
+ dmh.Expect("v2DM terminated")
if evalLink() != scriptPathFactory {
t.Fatalf("current link was not reverted correctly")
}
@@ -523,22 +517,22 @@
resolveExpectNotFound(t, ctx, "factoryDM") // Ensure a clean slate.
- dmh, dms = mgmttest.RunShellCommand(t, sh, dmEnv, execScriptCmd, currLink)
- mgmttest.ReadPID(t, dms)
+ dmh = mgmttest.RunCommand(t, sh, dmEnv, execScriptCmd, currLink)
+ mgmttest.ReadPID(t, dmh)
resolve(t, ctx, "factoryDM", 1) // Current link should have been launching factory version.
stopDevice(t, ctx, "factoryDM")
- dms.Expect("factoryDM terminated")
- dms.ExpectEOF()
+ dmh.Expect("factoryDM terminated")
+ dmh.ExpectEOF()
// Re-launch the device manager, to exercise the behavior of Suspend.
resolveExpectNotFound(t, ctx, "factoryDM") // Ensure a clean slate.
- dmh, dms = mgmttest.RunShellCommand(t, sh, dmEnv, execScriptCmd, currLink)
- mgmttest.ReadPID(t, dms)
+ dmh = mgmttest.RunCommand(t, sh, dmEnv, execScriptCmd, currLink)
+ mgmttest.ReadPID(t, dmh)
resolve(t, ctx, "factoryDM", 1)
suspendDevice(t, ctx, "factoryDM")
- dms.Expect("restart handler")
- dms.Expect("factoryDM terminated")
- dms.ExpectEOF()
+ dmh.Expect("restart handler")
+ dmh.Expect("factoryDM terminated")
+ dmh.ExpectEOF()
}
type pingServer chan<- pingArgs
@@ -647,8 +641,8 @@
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
- dmh, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ mgmttest.ReadPID(t, dmh)
claimDevice(t, ctx, "dm", "mydevice", noPairingToken)
// Create the local server that the app uses to let us know it's ready.
@@ -840,8 +834,8 @@
// Cleanly shut down the device manager.
syscall.Kill(dmh.Pid(), syscall.SIGINT)
- dms.Expect("dm terminated")
- dms.ExpectEOF()
+ dmh.Expect("dm terminated")
+ dmh.ExpectEOF()
}
func startRealBinaryRepository(t *testing.T, ctx *context.T, von string) func() {
@@ -903,8 +897,8 @@
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
pairingToken := "abcxyz"
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link", pairingToken)
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link", pairingToken)
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
*envelope = envelopeFromShell(sh, nil, appCmd, "google naps", "trapp")
@@ -987,8 +981,8 @@
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, "unused_helper", "unused_app_repo_name", "unused_curr_link")
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, "unused_helper", "unused_app_repo_name", "unused_curr_link")
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
// Create an envelope for an app.
@@ -1135,8 +1129,8 @@
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
// Create the local server that the app uses to let us know it's ready.
@@ -1341,8 +1335,8 @@
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
// Create the local server that the app uses to let us know it's ready.
@@ -1455,8 +1449,8 @@
}
otherCtx := ctxWithNewPrincipal(t, selfCtx, idp, "other")
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, "unused_helper", "unused_app_repo_name", "unused_curr_link")
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, "unused_helper", "unused_app_repo_name", "unused_curr_link")
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
deviceStub := device.DeviceClient("dm/device")
@@ -1554,8 +1548,8 @@
// Create a script wrapping the test target that implements suidhelper.
helperPath := generateSuidHelperScript(t, root)
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "-mocksetuid", "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "-mocksetuid", "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
// Claim the devicemanager with selfCtx as root/self/alice
claimDevice(t, selfCtx, "dm", "alice", noPairingToken)
@@ -1708,8 +1702,8 @@
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
- _, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- pid := mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ pid := mgmttest.ReadPID(t, dmh)
defer syscall.Kill(pid, syscall.SIGINT)
claimDevice(t, ctx, "dm", "mydevice", noPairingToken)
diff --git a/services/mgmt/device/impl/instance_reaping_test.go b/services/mgmt/device/impl/instance_reaping_test.go
index 0f52a0b..cc00312 100644
--- a/services/mgmt/device/impl/instance_reaping_test.go
+++ b/services/mgmt/device/impl/instance_reaping_test.go
@@ -7,55 +7,23 @@
"syscall"
"testing"
- "v.io/v23"
"v.io/v23/context"
"v.io/v23/naming"
- "v.io/v23/services/mgmt/application"
"v.io/v23/services/mgmt/stats"
"v.io/v23/vdl"
"v.io/x/ref/lib/flags/consts"
- "v.io/x/ref/lib/modules"
- "v.io/x/ref/lib/testutil"
- "v.io/x/ref/services/mgmt/device/impl"
mgmttest "v.io/x/ref/services/mgmt/lib/testutil"
)
-// TODO(rjkroege): This helper is generally useful. Move to util_test.go
-// and use it to reduce boiler plate across all tests here.
-func startupHelper(t *testing.T) (func(), *context.T, *modules.Shell, *application.Envelope, string, string) {
- ctx, shutdown := testutil.InitForTest()
- v23.GetNamespace(ctx).CacheCtl(naming.DisableCache(true))
-
- sh, deferFn := mgmttest.CreateShellAndMountTable(t, ctx, nil)
-
- // Set up mock application and binary repositories.
- envelope, envCleanup := startMockRepos(t, ctx)
-
- root, rootCleanup := mgmttest.SetupRootDir(t, "devicemanager")
- if err := impl.SaveCreatorInfo(root); err != nil {
- t.Fatal(err)
- }
-
- // Create a script wrapping the test target that implements suidhelper.
- helperPath := generateSuidHelperScript(t, root)
-
- return func() {
- rootCleanup()
- envCleanup()
- deferFn()
- shutdown()
- }, ctx, sh, envelope, root, helperPath
-}
-
func TestReaperNoticesAppDeath(t *testing.T) {
- cleanup, ctx, sh, envelope, root, helperPath := startupHelper(t)
+ cleanup, ctx, sh, envelope, root, helperPath, _ := startupHelper(t)
defer cleanup()
// Set up the device manager. Since we won't do device manager updates,
// don't worry about its application envelope and current link.
- dmh, dms := mgmttest.RunShellCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, nil, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ mgmttest.ReadPID(t, dmh)
claimDevice(t, ctx, "dm", "mydevice", noPairingToken)
// Create the local server that the app uses to let us know it's ready.
@@ -105,8 +73,8 @@
// Cleanly shut down the device manager.
syscall.Kill(dmh.Pid(), syscall.SIGINT)
- dms.Expect("dm terminated")
- dms.ExpectEOF()
+ dmh.Expect("dm terminated")
+ dmh.ExpectEOF()
}
func getPid(t *testing.T, ctx *context.T, appID, instanceID string) int {
@@ -120,7 +88,7 @@
}
func TestReapReconciliation(t *testing.T) {
- cleanup, ctx, sh, envelope, root, helperPath := startupHelper(t)
+ cleanup, ctx, sh, envelope, root, helperPath, _ := startupHelper(t)
defer cleanup()
// Start a device manager.
@@ -133,8 +101,8 @@
defer os.RemoveAll(dmCreds)
dmEnv := []string{fmt.Sprintf("%v=%v", consts.VeyronCredentials, dmCreds)}
- dmh, dms := mgmttest.RunShellCommand(t, sh, dmEnv, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- mgmttest.ReadPID(t, dms)
+ dmh := mgmttest.RunCommand(t, sh, dmEnv, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ mgmttest.ReadPID(t, dmh)
claimDevice(t, ctx, "dm", "mydevice", noPairingToken)
// Create the local server that the app uses to let us know it's ready.
@@ -160,8 +128,8 @@
// Shutdown the first device manager.
syscall.Kill(dmh.Pid(), syscall.SIGINT)
- dms.Expect("dm terminated")
- dms.ExpectEOF()
+ dmh.Expect("dm terminated")
+ dmh.ExpectEOF()
dmh.Shutdown(os.Stderr, os.Stderr)
resolveExpectNotFound(t, ctx, "dm") // Ensure a clean slate.
@@ -174,8 +142,8 @@
}
// Run another device manager to replace the dead one.
- dmh, dms = mgmttest.RunShellCommand(t, sh, dmEnv, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
- mgmttest.ReadPID(t, dms)
+ dmh = mgmttest.RunCommand(t, sh, dmEnv, deviceManagerCmd, "dm", root, helperPath, "unused_app_repo_name", "unused_curr_link")
+ mgmttest.ReadPID(t, dmh)
resolve(t, ctx, "dm", 1) // Verify the device manager has published itself.
// By now, we've reconciled the state of the tree with which processes are actually
@@ -218,6 +186,6 @@
// TODO(rjkroege): Should be in a defer to ensure that the device manager
// is cleaned up even if the test fails in an exceptional way.
syscall.Kill(dmh.Pid(), syscall.SIGINT)
- dms.Expect("dm terminated")
- dms.ExpectEOF()
+ dmh.Expect("dm terminated")
+ dmh.ExpectEOF()
}
diff --git a/services/mgmt/device/impl/util_test.go b/services/mgmt/device/impl/util_test.go
index 6e8fe15..9d4b7eb 100644
--- a/services/mgmt/device/impl/util_test.go
+++ b/services/mgmt/device/impl/util_test.go
@@ -18,6 +18,7 @@
"v.io/v23/security"
"v.io/v23/services/mgmt/application"
"v.io/v23/services/mgmt/device"
+ "v.io/v23/services/mgmt/stats"
"v.io/v23/verror"
"v.io/x/lib/vlog"
tsecurity "v.io/x/ref/lib/testutil/security"
@@ -26,6 +27,7 @@
"v.io/x/ref/lib/testutil"
_ "v.io/x/ref/profiles/roaming"
"v.io/x/ref/services/mgmt/device/impl"
+ mgmttest "v.io/x/ref/services/mgmt/lib/testutil"
)
const (
@@ -181,6 +183,12 @@
return device.ApplicationClient(appName)
}
+func statsStub(nameComponents ...string) stats.StatsClientMethods {
+ baseName := "dm/apps"
+ statsName := naming.Join(append([]string{baseName}, nameComponents...)...)
+ return stats.StatsClient(statsName)
+}
+
func installApp(t *testing.T, ctx *context.T, opt ...interface{}) string {
appID, err := appStub().Install(ctx, mockApplicationRepoName, ocfg(opt), opkg(opt))
if err != nil {
@@ -383,3 +391,34 @@
}
return ret
}
+
+// TODO(rjkroege): This helper is generally useful. Use it to reduce
+// boiler plate across all device manager tests.
+func startupHelper(t *testing.T) (func(), *context.T, *modules.Shell, *application.Envelope, string, string, *tsecurity.IDProvider) {
+ ctx, shutdown := testutil.InitForTest()
+ v23.GetNamespace(ctx).CacheCtl(naming.DisableCache(true))
+
+ // Make a new identity context.
+ idp := tsecurity.NewIDProvider("root")
+ ctx = ctxWithNewPrincipal(t, ctx, idp, "self")
+
+ sh, deferFn := mgmttest.CreateShellAndMountTable(t, ctx, nil)
+
+ // Set up mock application and binary repositories.
+ envelope, envCleanup := startMockRepos(t, ctx)
+
+ root, rootCleanup := mgmttest.SetupRootDir(t, "devicemanager")
+ if err := impl.SaveCreatorInfo(root); err != nil {
+ t.Fatal(err)
+ }
+
+ // Create a script wrapping the test target that implements suidhelper.
+ helperPath := generateSuidHelperScript(t, root)
+
+ return func() {
+ rootCleanup()
+ envCleanup()
+ deferFn()
+ shutdown()
+ }, ctx, sh, envelope, root, helperPath, idp
+}
diff --git a/services/mgmt/device/impl/v23_test.go b/services/mgmt/device/impl/v23_test.go
new file mode 100644
index 0000000..aa08bfe
--- /dev/null
+++ b/services/mgmt/device/impl/v23_test.go
@@ -0,0 +1,18 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This file was auto-generated via go generate.
+// DO NOT UPDATE MANUALLY
+package impl_test
+
+import "v.io/x/ref/lib/modules"
+
+func init() {
+ modules.RegisterChild("execScript", `execScript launches the script passed as argument.`, execScript)
+ modules.RegisterChild("deviceManager", `deviceManager sets up a device manager server. It accepts the name to
+publish the server under as an argument. Additional arguments can optionally
+specify device manager config settings.`, deviceManager)
+ modules.RegisterChild("deviceManagerV10", `This is the same as deviceManager above, except that it has a different major version number`, deviceManagerV10)
+ modules.RegisterChild("app", ``, app)
+}
diff --git a/services/mgmt/device/starter/starter.go b/services/mgmt/device/starter/starter.go
index 4ad22f8..f1fc91d 100644
--- a/services/mgmt/device/starter/starter.go
+++ b/services/mgmt/device/starter/starter.go
@@ -117,6 +117,10 @@
}
func startClaimableDevice(ctx *context.T, dispatcher ipc.Dispatcher, args Args) (func(), error) {
+ ctx, err := setNamespaceRootsForUnclaimedDevice(ctx)
+ if err != nil {
+ return nil, err
+ }
// TODO(caprita,ashankar): We create a context with a new stream manager
// that we can cancel once the device has been claimed. This gets around
// the following issue: if we publish the claimable server to the local
@@ -129,8 +133,7 @@
// gets confused trying to reuse the old connection and doesn't attempt
// to create a new connection). We should get to the bottom of it.
ctx, cancel := context.WithCancel(ctx)
- ctx, err := v23.SetNewStreamManager(ctx)
- if err != nil {
+ if ctx, err = v23.SetNewStreamManager(ctx); err != nil {
cancel()
return nil, err
}
@@ -362,3 +365,44 @@
remoteBlessings, _ := call.RemoteBlessings()
return remoteBlessings, nil
}
+
+// Unclaimed devices typically have Principals that recognize no other
+// authoritative public keys than their own. As a result, they will fail to
+// authorize any other services.
+//
+// With no information to authenticate or authorize peers (including the
+// mounttable at the namespace root), this unclaimed device manager will be
+// unable to make any outgoing RPCs.
+//
+// As a workaround, reconfigure it to "authorize any root mounttable" by
+// removing references to the expected blessings of the namespace root. This
+// will allow the unclaimed device manager to mount itself.
+//
+// TODO(ashankar,caprita): The more secure fix would be to ensure that an
+// unclaimed device is configured to recognize the blessings presented by the
+// mounttable it is configured to talk to. Of course, if the root mounttable is
+// "discovered" as opposed to "configured", then this new device will have to
+// return to either not mounting itself (and being claimed via some discovery
+// protocol like mdns or bluetooth) or ignoring the blessings of the namespace
+// root.
+func setNamespaceRootsForUnclaimedDevice(ctx *context.T) (*context.T, error) {
+ origroots := v23.GetNamespace(ctx).Roots()
+ roots := make([]string, len(origroots))
+ for i, orig := range origroots {
+ addr, suffix := naming.SplitAddressName(orig)
+ origep, err := v23.NewEndpoint(addr)
+ if err != nil {
+ return nil, fmt.Errorf("failed to create endpoint from namespace root %q: %v", orig, err)
+ }
+ ep := naming.FormatEndpoint(
+ origep.Addr().Network(),
+ origep.Addr().String(),
+ origep.RoutingID(),
+ origep.IPCVersionRange(),
+ naming.ServesMountTableOpt(origep.ServesMountTable()))
+ roots[i] = naming.JoinAddressName(ep, suffix)
+ }
+ vlog.Infof("Changing namespace roots from %v to %v", origroots, roots)
+ ctx, _, err := v23.SetNewNamespace(ctx, roots...)
+ return ctx, err
+}
diff --git a/services/mgmt/lib/testutil/modules.go b/services/mgmt/lib/testutil/modules.go
index b03dcb9..1736135 100644
--- a/services/mgmt/lib/testutil/modules.go
+++ b/services/mgmt/lib/testutil/modules.go
@@ -17,7 +17,6 @@
"v.io/x/ref/lib/modules"
"v.io/x/ref/lib/modules/core"
"v.io/x/ref/lib/testutil"
- "v.io/x/ref/lib/testutil/expect"
tsecurity "v.io/x/ref/lib/testutil/security"
)
@@ -35,11 +34,10 @@
if err != nil {
t.Fatalf("failed to start root mount table: %s", err)
}
- s := expect.NewSession(t, h.Stdout(), ExpectTimeout)
- s.ExpectVar("PID")
- rootName := s.ExpectVar("MT_NAME")
+ h.ExpectVar("PID")
+ rootName := h.ExpectVar("MT_NAME")
if t.Failed() {
- t.Fatalf("failed to read mt name: %s", s.Error())
+ t.Fatalf("failed to read mt name: %s", h.Error())
}
return rootName, h
}
@@ -61,10 +59,13 @@
// CreateShellAndMountTable builds a new modules shell and its
// associated mount table.
func CreateShellAndMountTable(t *testing.T, ctx *context.T, p security.Principal) (*modules.Shell, func()) {
- sh, err := modules.NewShell(ctx, p)
+ sh, err := modules.NewExpectShell(ctx, p, t, testing.Verbose())
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
+ opts := sh.DefaultStartOpts()
+ opts.ExpectTimeout = ExpectTimeout
+ sh.SetDefaultStartOpts(opts)
// The shell, will, by default share credentials with its children.
sh.ClearVar(consts.VeyronCredentials)
@@ -100,16 +101,25 @@
return sh, fn
}
+// RunShellCommand runs an external, non Vanadium command using the modules system.
+func RunShellCommand(t *testing.T, sh *modules.Shell, env []string, cmd string, args ...string) modules.Handle {
+ return runWithOpts(t, sh, sh.DefaultStartOpts().NoExecCommand(), env, cmd, args...)
+}
+
+// RunCommand runs a modules command.
+func RunCommand(t *testing.T, sh *modules.Shell, env []string, cmd string, args ...string) modules.Handle {
+ return runWithOpts(t, sh, sh.DefaultStartOpts(), env, cmd, args...)
+}
+
// RunShellCommand runs an external command using the modules system.
-func RunShellCommand(t *testing.T, sh *modules.Shell, env []string, cmd string, args ...string) (modules.Handle, *expect.Session) {
- h, err := sh.Start(cmd, env, args...)
+func runWithOpts(t *testing.T, sh *modules.Shell, opts modules.StartOpts, env []string, cmd string, args ...string) modules.Handle {
+ h, err := sh.StartWithOpts(opts, env, cmd, args...)
if err != nil {
t.Fatalf(testutil.FormatLogLine(2, "failed to start %q: %s", cmd, err))
- return nil, nil
+ return nil
}
- s := expect.NewSession(t, h.Stdout(), ExpectTimeout)
- s.SetVerbosity(testing.Verbose())
- return h, s
+ h.SetVerbosity(testing.Verbose())
+ return h
}
// NewServer creates a new server.
@@ -128,8 +138,8 @@
// 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)
+func ReadPID(t *testing.T, h modules.ExpectSession) int {
+ m := h.ExpectRE("ready:([0-9]+)", -1)
if len(m) == 1 && len(m[0]) == 2 {
pid, err := strconv.Atoi(m[0][1])
if err != nil {
diff --git a/services/mgmt/profile/profiled/profiled_v23_test.go b/services/mgmt/profile/profiled/profiled_v23_test.go
index 5ef410e..a7152a6 100644
--- a/services/mgmt/profile/profiled/profiled_v23_test.go
+++ b/services/mgmt/profile/profiled/profiled_v23_test.go
@@ -50,9 +50,9 @@
"-name=" + profileRepoName, "-store=" + profileRepoStore,
"-veyron.tcp.address=127.0.0.1:0",
}
- i.BuildGoPkg("v.io/x/ref/services/mgmt/profile/profiled").Start(args...)
+ i.BuildV23Pkg("v.io/x/ref/services/mgmt/profile/profiled").Start(args...)
- clientBin := i.BuildGoPkg("v.io/x/ref/cmd/profile")
+ clientBin := i.BuildV23Pkg("v.io/x/ref/cmd/profile")
// Create a profile.
const profile = "test-profile"
diff --git a/services/mgmt/suidhelper/impl/args.go b/services/mgmt/suidhelper/impl/args.go
index 3703e22..04933fb 100644
--- a/services/mgmt/suidhelper/impl/args.go
+++ b/services/mgmt/suidhelper/impl/args.go
@@ -8,6 +8,7 @@
"os"
"os/user"
"strconv"
+ "strings"
sflag "v.io/x/ref/services/mgmt/suidhelper/impl/flag"
)
@@ -57,6 +58,16 @@
flagProgName = sflag.ProgName
}
+func cleanEnv(env []string) []string {
+ nenv := []string{}
+ for _, e := range env {
+ if !strings.HasPrefix(e, "VEYRON_SUIDHELPER_TEST") {
+ nenv = append(nenv, e)
+ }
+ }
+ return nenv
+}
+
// ParseArguments populates the WorkParameter object from the provided args
// and env strings.
func (wp *WorkParameters) ProcessArguments(fs *flag.FlagSet, env []string) error {
@@ -96,6 +107,7 @@
// Preserve the arguments for examination by the test harness if executed
// in the course of a test.
if os.Getenv("VEYRON_SUIDHELPER_TEST") != "" {
+ env = cleanEnv(env)
b := new(bytes.Buffer)
enc := json.NewEncoder(b)
enc.Encode(ArgsSavedForTest{
