Merge "veyron/runtimes/google/naming: Bugfix in error propagation and test setup."
diff --git a/lib/flags/doc.go b/lib/flags/doc.go
index 8681ac4..ff98c9d 100644
--- a/lib/flags/doc.go
+++ b/lib/flags/doc.go
@@ -1,3 +1,36 @@
-// Package flags provides implementations of the flag.Value interface
-// that are commonly used.
+// Package flags provides flag definitions for commonly used flags and
+// and, where appropriate, implementations of the flag.Value interface
+// for those flags to ensure that only valid values of those flags
+// are supplied.
+// In general, this package will be used by veyron profiles and the
+// runtime implementations, but can also be used by any application
+// that wants access to the flags and environment variables it supports.
+//
+// TODO(cnicolaou): move reading of environment variables to here also,
+// flags will override the environment variable settings.
package flags
+
+import "flag"
+
+type Flags struct {
+ ListenProtocolFlag TCPProtocolFlag
+ ListenAddressFlag IPHostPortFlag
+ ListenProxyFlag string
+ NamespaceRootsFlag string // TODO(cnicolaou): provide flag.Value impl
+ CredentialsFlag string // TODO(cnicolaou): provide flag.Value impl
+}
+
+// New returns a new instance of flags.Flags. Calling Parse on the supplied
+// flagSet will populate the fields of the return flags.Flags with the appropriate
+// values.
+func New(fs *flag.FlagSet) *Flags {
+ t := &Flags{}
+ t.ListenProtocolFlag = TCPProtocolFlag{"tcp"}
+ t.ListenAddressFlag = IPHostPortFlag{Port: "0"}
+ fs.Var(&t.ListenProtocolFlag, "veyron.tcp.protocol", "protocol to listen with")
+ fs.Var(&t.ListenAddressFlag, "veyron.tcp.address", "address to listen on")
+ fs.StringVar(&t.ListenProxyFlag, "veyron.proxy", "", "object name of proxy service to use to export services across network boundaries")
+ fs.StringVar(&t.NamespaceRootsFlag, "veyron.namespace.roots", "", ": separated list of roots for the local namespace")
+ fs.StringVar(&t.CredentialsFlag, "veyron.credentials", "", "directory to use for storing security credentials")
+ return t
+}
diff --git a/lib/flags/flags_test.go b/lib/flags/flags_test.go
new file mode 100644
index 0000000..4030af6
--- /dev/null
+++ b/lib/flags/flags_test.go
@@ -0,0 +1,24 @@
+package flags_test
+
+import (
+ "flag"
+ "testing"
+
+ "veyron.io/veyron/veyron/lib/flags"
+)
+
+func TestFlags(t *testing.T) {
+ fs := flag.NewFlagSet("test", flag.ContinueOnError)
+ fl := flags.New(fs)
+
+ addr := "192.168.10.1:0"
+ roots := "ab:cd:ef"
+ args := []string{"--veyron.tcp.address=" + addr, "--veyron.namespace.roots=" + roots}
+ fs.Parse(args)
+ if got, want := fl.NamespaceRootsFlag, roots; got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ if got, want := fl.ListenAddressFlag.String(), addr; got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+}
diff --git a/profiles/roaming/roaming.go b/profiles/roaming/roaming.go
index d9314bb..2f9381f 100644
--- a/profiles/roaming/roaming.go
+++ b/profiles/roaming/roaming.go
@@ -30,19 +30,14 @@
)
var (
- listenProtocolFlag = flags.TCPProtocolFlag{"tcp"}
- listenAddressFlag = flags.IPHostPortFlag{Port: "0"}
- listenProxyFlag string
-
+ commonFlags *flags.Flags
// ListenSpec is an initialized instance of ipc.ListenSpec that can
// be used with ipc.Listen.
ListenSpec ipc.ListenSpec
)
func init() {
- flag.Var(&listenProtocolFlag, "veyron.tcp.protocol", "protocol to listen with")
- flag.Var(&listenAddressFlag, "veyron.tcp.address", "address to listen on")
- flag.StringVar(&listenProxyFlag, "veyron.proxy", "", "object name of proxy service to use to export services across network boundaries")
+ commonFlags = flags.New(flag.CommandLine)
rt.RegisterProfile(New())
}
@@ -75,9 +70,9 @@
log := rt.Logger()
ListenSpec = ipc.ListenSpec{
- Protocol: listenProtocolFlag.Protocol,
- Address: listenAddressFlag.String(),
- Proxy: listenProxyFlag,
+ Protocol: commonFlags.ListenProtocolFlag.Protocol,
+ Address: commonFlags.ListenAddressFlag.String(),
+ Proxy: commonFlags.ListenProxyFlag,
}
// Our address is private, so we test for running on GCE and for its
diff --git a/profiles/static/static.go b/profiles/static/static.go
index 960f347..4997720 100644
--- a/profiles/static/static.go
+++ b/profiles/static/static.go
@@ -18,19 +18,14 @@
)
var (
- listenProtocolFlag = flags.TCPProtocolFlag{"tcp"}
- listenAddressFlag = flags.IPHostPortFlag{Port: "0"}
- listenProxyFlag string
-
+ commonFlags *flags.Flags
// ListenSpec is an initialized instance of ipc.ListenSpec that can
// be used with ipc.Listen.
ListenSpec ipc.ListenSpec
)
func init() {
- flag.Var(&listenProtocolFlag, "veyron.tcp.protocol", "protocol to listen with")
- flag.Var(&listenAddressFlag, "veyron.tcp.address", "address to listen on")
- flag.StringVar(&listenProxyFlag, "veyron.proxy", "", "object name of proxy service to use to export services across network boundaries")
+ commonFlags = flags.New(flag.CommandLine)
rt.RegisterProfile(New())
}
@@ -61,9 +56,9 @@
log := rt.Logger()
ListenSpec = ipc.ListenSpec{
- Protocol: listenProtocolFlag.Protocol,
- Address: listenAddressFlag.String(),
- Proxy: listenProxyFlag,
+ Protocol: commonFlags.ListenProtocolFlag.Protocol,
+ Address: commonFlags.ListenAddressFlag.String(),
+ Proxy: commonFlags.ListenProxyFlag,
}
// Our address is private, so we test for running on GCE and for its
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index a0d207d..514377f 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -37,10 +37,6 @@
errServerStopped = verror.Abortedf("ipc: server is stopped")
)
-func errNotAuthorized(err error) verror.E {
- return verror.NoAccessf("ipc: not authorized(%v)", err)
-}
-
type server struct {
sync.Mutex
ctx context.T // context used by the server to make internal RPCs.
@@ -850,21 +846,20 @@
}
if remoteID := fs.flow.RemoteID(); remoteID != nil {
// TODO(ashankar): This whole check goes away once the old security model is ripped out.
- if fs.authorizedRemoteID, err = remoteID.Authorize(isecurity.NewContext(
- isecurity.ContextArgs{
- LocalID: fs.flow.LocalID(),
- RemoteID: fs.flow.RemoteID(),
- Method: fs.method,
- Suffix: fs.suffix,
- Discharges: fs.discharges,
- Label: fs.label})); err != nil {
- return nil, errNotAuthorized(err)
+ if fs.authorizedRemoteID, err = remoteID.Authorize(isecurity.NewContext(isecurity.ContextArgs{
+ LocalID: fs.flow.LocalID(),
+ RemoteID: fs.flow.RemoteID(),
+ Method: fs.method,
+ Suffix: fs.suffix,
+ Discharges: fs.discharges,
+ Label: fs.label,
+ })); err != nil {
+ return nil, verror.NoAccessf("%v is not authorized to call %q.%q (%v)", fs.RemoteID(), fs.Name(), fs.Method(), err)
}
}
// Check application's authorization policy and invoke the method.
if err := fs.authorize(auth); err != nil {
- // TODO(ataly, ashankar): For privacy reasons, should we hide the authorizer error (err)?
- return nil, errNotAuthorized(fmt.Errorf("%v (PublicID:%v) not authorized for %q.%q: %v", fs.RemoteBlessings(), fs.RemoteID(), fs.Name(), fs.Method(), err))
+ return nil, err
}
// Check if the caller is permitted to view debug information.
fs.allowDebug = fs.authorizeForDebug(auth) == nil
@@ -955,7 +950,7 @@
return verror.Makef(verror.ErrorID(err), "%s", err)
}
if err := i.fs.authorize(auth); err != nil {
- return errNotAuthorized(fmt.Errorf("%q not authorized for method %q: %v", i.fs.RemoteID(), i.fs.Method(), err))
+ return err
}
leafCall := &localServerCall{call, prefix}
argptrs[0] = &pattern
@@ -997,11 +992,18 @@
return c.ServerCall.Send(me)
}
-func (fs *flowServer) authorize(auth security.Authorizer) error {
+func (fs *flowServer) authorize(auth security.Authorizer) verror.E {
if auth == nil {
auth = defaultAuthorizer(fs)
}
- return auth.Authorize(fs)
+ if err := auth.Authorize(fs); err != nil {
+ // TODO(ataly, ashankar): For privacy reasons, should we hide the authorizer error?
+ if fs.RemoteBlessings() != nil {
+ return verror.NoAccessf("ipc: %v not authorized to call %q.%q (%v)", fs.RemoteBlessings(), fs.Name(), fs.Method(), err)
+ }
+ return verror.NoAccessf("ipc: %v (deprecated security model) is not authorized to call %q.%q (%v)", fs.RemoteID(), fs.Name(), fs.Method(), err)
+ }
+ return nil
}
// debugContext is a context which wraps another context but always returns
