veyron/lib/flags: add flag groups.
Change-Id: Ia490369e153564b79d673feaee64d8e3c0e91564
diff --git a/lib/flags/doc.go b/lib/flags/doc.go
index ef8c2c5..49a7360 100644
--- a/lib/flags/doc.go
+++ b/lib/flags/doc.go
@@ -1,13 +1,17 @@
-// 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
+// Package flags provides definitions for commonly used flags and where
+// appropriate, implementations of the flag.Value interface for those flags
+// to ensure that only valid values of those flags are supplied. Some of these
+// flags may also be specified using environment variables directly for
+// are documented accordingly; in these cases the command line value takes
+// precedence over the environment variable.
+// Flags are defined as 'groups' of related flags so that caller may choose
+// which ones to use without having to be burdened with the full set. The
+// groups may be used directly or via the Flags type that aggregates multiple
+// groups. In all cases, the flags are registered with a supplied
+// flag.FlagSet and hence are not forced onto the command line
+// unless the caller passes in flag.CommandLine as the flag.FlagSet to use.
+//
+// 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.
-//
-// TODO(cnicolaou): implement simply subsetting of the flags to be defined
-// and parsed - e.g. for server configs, basic runtime configs/security etc.
package flags
diff --git a/lib/flags/flags.go b/lib/flags/flags.go
index 270b72f..f6c2ef1 100644
--- a/lib/flags/flags.go
+++ b/lib/flags/flags.go
@@ -1,44 +1,170 @@
package flags
-import "flag"
+import (
+ "flag"
+ "fmt"
+ "os"
+ "strings"
+)
+// FlagGroup is the type for identifying groups of related flags.
+type FlagGroup int
+
+const (
+ // Essential identifies the flags and associated environment variables
+ // required by all Vanadium processes. Namely:
+ // --veyron.namespace.root (which may be repeated to supply multiple values)
+ // --veyron.credentials
+ Essential FlagGroup = iota
+ // Listen identifies the flags typically required to configure
+ // ipc.ListenSpec. Namely:
+ // --veyron.tcp.protocol
+ // --veyron.tcp.address
+ // --veyron.proxy
+ Listen
+)
+
+// Flags represents the set of flag groups created by a call to
+// CreateAndRegister.
type Flags struct {
- // The FlagSet passed in as a parameter to New
FlagSet *flag.FlagSet
-
- // As needed to initialize ipc.ListenSpec.
- ListenProtocolFlag TCPProtocolFlag
- ListenAddressFlag IPHostPortFlag
- ListenProxyFlag string
-
- // TODO(cnicolaou): implement these.
- NamespaceRootsFlag string // TODO(cnicolaou): provide flag.Value impl
- CredentialsFlag string // TODO(cnicolaou): provide flag.Value impl
+ groups map[FlagGroup]interface{}
}
-// New returns a new instance of flags.Flags. Calling Parse on the supplied
-// flagSet will populate the fields of the returned flags.Flags with the
-// appropriate values. The currently supported set of flags only allows
-// for configuring one service at a time, but in the future this will
-// be expanded to support multiple services.
-func New(fs *flag.FlagSet) *Flags {
- t := &Flags{FlagSet: fs}
- 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", "", "colon separated list of roots for the local namespace")
- fs.StringVar(&t.CredentialsFlag, "veyron.credentials", "", "directory to use for storing security credentials")
- return t
+type namespaceRootFlagVar struct {
+ roots []string
}
-// Args returns the unparsed args, as per flag.Args
-func (fs *Flags) Args() []string {
- return fs.FlagSet.Args()
+func (nsr *namespaceRootFlagVar) String() string {
+ return fmt.Sprintf("%v", nsr.roots)
+}
+
+func (nsr *namespaceRootFlagVar) Set(v string) error {
+ nsr.roots = append(nsr.roots, v)
+ return nil
+}
+
+// EssentialFlags contains the values of the Essential flag group.
+type EssentialFlags struct {
+ // NamespaceRoots may be initialized by NAMESPACE_ROOT* enivornment
+ // variables as well as --veyron.namespace.root. The command line
+ // will override the environment.
+ NamespaceRoots []string // TODO(cnicolaou): provide flag.Value impl
+
+ // Credentials may be initialized by the the VEYRON_CREDENTIALS
+ // environment variable. The command line will override the environment.
+ Credentials string // TODO(cnicolaou): provide flag.Value impl
+
+ namespaceRootsFlag namespaceRootFlagVar
+}
+
+// ListenFlags contains the values of the Listen flag group.
+type ListenFlags struct {
+ ListenProtocol TCPProtocolFlag
+ ListenAddress IPHostPortFlag
+ ListenProxy string
+}
+
+// createAndRegisterEssentialFlags creates and registers the EssentialFlags
+// group with the supplied flag.FlagSet.
+func createAndRegisterEssentialFlags(fs *flag.FlagSet) *EssentialFlags {
+ f := &EssentialFlags{}
+ fs.Var(&f.namespaceRootsFlag, "veyron.namespace.root", "local namespace root; can be repeated to provided multiple roots")
+ fs.StringVar(&f.Credentials, "veyron.credentials", "", "directory to use for storing security credentials")
+ return f
+}
+
+// createAndRegisterListenFlags creates and registers the ListenFlags
+// group with the supplied flag.FlagSet.
+func createAndRegisterListenFlags(fs *flag.FlagSet) *ListenFlags {
+ f := &ListenFlags{}
+ f.ListenProtocol = TCPProtocolFlag{"tcp"}
+ f.ListenAddress = IPHostPortFlag{Port: "0"}
+ fs.Var(&f.ListenProtocol, "veyron.tcp.protocol", "protocol to listen with")
+ fs.Var(&f.ListenAddress, "veyron.tcp.address", "address to listen on")
+ fs.StringVar(&f.ListenProxy, "veyron.proxy", "", "object name of proxy service to use to export services across network boundaries")
+ return f
+}
+
+// CreateAndRegister creates a new set of flag groups as specified by the
+// supplied flag group parameters and registers them with the supplied
+// flag.Flagset. The Essential flag group is always included.
+func CreateAndRegister(fs *flag.FlagSet, groups ...FlagGroup) *Flags {
+ f := &Flags{FlagSet: fs, groups: make(map[FlagGroup]interface{})}
+ f.groups[Essential] = createAndRegisterEssentialFlags(fs)
+ for _, s := range groups {
+ switch s {
+ case Essential:
+ // do nothing, always included
+ case Listen:
+ f.groups[Listen] = createAndRegisterListenFlags(fs)
+ }
+ }
+ return f
+}
+
+// EssentialFlags returns the Essential flag subset stored in its Flags
+// instance.
+func (f *Flags) EssentialFlags() EssentialFlags {
+ from := f.groups[Essential].(*EssentialFlags)
+ to := *from
+ to.NamespaceRoots = make([]string, len(from.NamespaceRoots))
+ copy(to.NamespaceRoots, from.NamespaceRoots)
+ return to
+}
+
+// ListenFlags returns a copy of the Listen flag group stored in Flags.
+// This copy will contain default values if the Listen flag group
+// was not specified when CreateAndRegister was called. The HasGroup
+// method can be used for testing to see if any given group was configured.
+func (f *Flags) ListenFlags() ListenFlags {
+ if p := f.groups[Listen]; p != nil {
+ return *(p.(*ListenFlags))
+ }
+ return ListenFlags{}
+}
+
+// HasGroup returns group if the supplied FlagGroup has been created
+// for these Flags.
+func (f *Flags) HasGroup(group FlagGroup) bool {
+ _, present := f.groups[group]
+ return present
+}
+
+// Args returns the unparsed args, as per flag.Args.
+func (f *Flags) Args() []string {
+ return f.FlagSet.Args()
+}
+
+// legacyEnvInit provides support for the legacy NAMESPACE_ROOT? and
+// VEYRON_CREDENTIALS env vars.
+func (es *EssentialFlags) legacyEnvInit() {
+ for _, ev := range os.Environ() {
+ p := strings.SplitN(ev, "=", 2)
+ if len(p) != 2 {
+ continue
+ }
+ k, v := p[0], p[1]
+ if strings.HasPrefix(k, "NAMESPACE_ROOT") {
+ es.NamespaceRoots = append(es.NamespaceRoots, v)
+ }
+ }
+ if creds := os.Getenv("VEYRON_CREDENTIALS"); creds != "" {
+ es.Credentials = creds
+ }
}
// Parse parses the supplied args, as per flag.Parse
-func (fs *Flags) Parse(args []string) error {
- return fs.FlagSet.Parse(args)
+func (f *Flags) Parse(args []string) error {
+ f.groups[Essential].(*EssentialFlags).legacyEnvInit()
+ // TODO(cnicolaou): implement a single env var 'VANADIUM_OPTS'
+ // that can be used to specify any command line.
+ if err := f.FlagSet.Parse(args); err != nil {
+ return err
+ }
+ essential := f.groups[Essential].(*EssentialFlags)
+ if len(essential.namespaceRootsFlag.roots) > 0 {
+ essential.NamespaceRoots = essential.namespaceRootsFlag.roots
+ }
+ return nil
}
diff --git a/lib/flags/flags_test.go b/lib/flags/flags_test.go
index 7b17e99..e8ca273 100644
--- a/lib/flags/flags_test.go
+++ b/lib/flags/flags_test.go
@@ -3,34 +3,41 @@
import (
"flag"
"io/ioutil"
+ "os"
+ "reflect"
"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)
+ fl := flags.CreateAndRegister(flag.NewFlagSet("test", flag.ContinueOnError))
+ creds := "creddir"
+ roots := []string{"ab:cd:ef"}
+ args := []string{"--veyron.credentials=" + creds, "--veyron.namespace.root=" + roots[0]}
+ fl.Parse(args)
+ es := fl.EssentialFlags()
+ if got, want := es.NamespaceRoots, roots; !reflect.DeepEqual(got, want) {
+ t.Errorf("got %v, want %v", got, want)
}
- if got, want := fl.ListenAddressFlag.String(), addr; got != want {
- t.Errorf("got %q, want %q", got, want)
+ if got, want := es.Credentials, creds; !reflect.DeepEqual(got, want) {
+ t.Errorf("got %v, want %v", got, want)
}
- if got, want := len(fl.Args()), 0; got != want {
- t.Errorf("got %d, want %d", got, want)
+ if got, want := fl.HasGroup(flags.Listen), false; got != want {
+ t.Errorf("got %t, want %t", got, want)
+ }
+ // Make sure we have a deep copy.
+ es.NamespaceRoots[0] = "oooh"
+ es = fl.EssentialFlags()
+ if got, want := es.NamespaceRoots, roots; !reflect.DeepEqual(got, want) {
+ t.Errorf("got %v, want %v", got, want)
}
}
func TestFlagError(t *testing.T) {
fs := flag.NewFlagSet("test", flag.ContinueOnError)
fs.SetOutput(ioutil.Discard)
- fl := flags.New(fs)
+ fl := flags.CreateAndRegister(fs)
addr := "192.168.10.1:0"
args := []string{"--xxxveyron.tcp.address=" + addr, "not an arg"}
err := fl.Parse(args)
@@ -40,5 +47,73 @@
if got, want := len(fl.Args()), 1; got != want {
t.Errorf("got %d, want %d [args: %v]", got, want, fl.Args())
}
+}
+
+func TestFlagsGroups(t *testing.T) {
+ fl := flags.CreateAndRegister(flag.NewFlagSet("test", flag.ContinueOnError), flags.Listen)
+ if got, want := fl.HasGroup(flags.Listen), true; got != want {
+ t.Errorf("got %t, want %t", got, want)
+ }
+ addr := "192.168.10.1:0"
+ roots := []string{"ab:cd:ef"}
+ args := []string{"--veyron.tcp.address=" + addr, "--veyron.namespace.root=" + roots[0]}
+ fl.Parse(args)
+ lf := fl.ListenFlags()
+ if got, want := fl.EssentialFlags().NamespaceRoots, roots; !reflect.DeepEqual(got, want) {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := lf.ListenAddress.String(), addr; got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+}
+
+const credEnvVar = "VEYRON_CREDENTIALS"
+const rootEnvVar = "NAMESPACE_ROOT"
+const rootEnvVar0 = "NAMESPACE_ROOT0"
+
+func TestEnvVars(t *testing.T) {
+ oldcreds := os.Getenv(credEnvVar)
+ defer os.Setenv(credEnvVar, oldcreds)
+
+ oldroot := os.Getenv(rootEnvVar)
+ oldroot0 := os.Getenv(rootEnvVar0)
+ defer os.Setenv(rootEnvVar, oldroot)
+ defer os.Setenv(rootEnvVar0, oldroot0)
+
+ os.Setenv(credEnvVar, "bar")
+ fl := flags.CreateAndRegister(flag.NewFlagSet("test", flag.ContinueOnError))
+ if err := fl.Parse([]string{}); err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ es := fl.EssentialFlags()
+ if got, want := es.Credentials, "bar"; got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+
+ if err := fl.Parse([]string{"--veyron.credentials=baz"}); err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ es = fl.EssentialFlags()
+ if got, want := es.Credentials, "baz"; got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+
+ os.Setenv(rootEnvVar, "a:1")
+ os.Setenv(rootEnvVar0, "a:2")
+ fl = flags.CreateAndRegister(flag.NewFlagSet("test", flag.ContinueOnError))
+ if err := fl.Parse([]string{}); err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ es = fl.EssentialFlags()
+ if got, want := es.NamespaceRoots, []string{"a:1", "a:2"}; !reflect.DeepEqual(got, want) {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ if err := fl.Parse([]string{"--veyron.namespace.root=b:1", "--veyron.namespace.root=b:2", "--veyron.namespace.root=b:3"}); err != nil {
+ t.Fatalf("unexpected error: %s", err)
+ }
+ es = fl.EssentialFlags()
+ if got, want := es.NamespaceRoots, []string{"b:1", "b:2", "b:3"}; !reflect.DeepEqual(got, want) {
+ t.Errorf("got %q, want %q", got, want)
+ }
}
diff --git a/lib/modules/core/echo.go b/lib/modules/core/echo.go
index 5fd3307..3f6ff42 100644
--- a/lib/modules/core/echo.go
+++ b/lib/modules/core/echo.go
@@ -38,11 +38,10 @@
}
func echoServer(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- fl, err := ParseCommonFlags(args)
+ fl, args, err := parseListenFlags(args)
if err != nil {
return fmt.Errorf("failed parsing args: %s", err)
}
- args = fl.Args()
if err := checkArgs(args, 2, "<message> <name>"); err != nil {
return err
}
@@ -68,14 +67,7 @@
}
func echoClient(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- fl, err := ParseCommonFlags(args)
- if err != nil {
- return fmt.Errorf("failed parsing args: %s", err)
- }
- args = fl.Args()
- if err := checkArgs(args, 2, "<name> <message>"); err != nil {
- return err
- }
+ args = args[1:]
name := args[0]
args = args[1:]
client := rt.R().Client()
diff --git a/lib/modules/core/mounttable.go b/lib/modules/core/mounttable.go
index 307dde3..9c4fec0 100644
--- a/lib/modules/core/mounttable.go
+++ b/lib/modules/core/mounttable.go
@@ -34,11 +34,11 @@
func runMT(root bool, stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
r := rt.R()
- fl, err := ParseCommonFlags(args)
+ fl, args, err := parseListenFlags(args)
if err != nil {
return fmt.Errorf("failed parsing args: %s", err)
}
- args = fl.Args()
+ // args = fl.Args()
lspec := initListenSpec(fl)
server, err := r.NewServer(options.ServesMountTable(true))
if err != nil {
diff --git a/lib/modules/core/proxy.go b/lib/modules/core/proxy.go
index 0e24fe1..7290543 100644
--- a/lib/modules/core/proxy.go
+++ b/lib/modules/core/proxy.go
@@ -18,11 +18,10 @@
}
func proxyServer(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- fl, err := ParseCommonFlags(args)
+ fl, args, err := parseListenFlags(args)
if err != nil {
return fmt.Errorf("failed parsing args: %s", err)
- }
- args = fl.Args()
+ } // args = fl.Args()
if err := checkArgs(args, -1, ""); err != nil {
return err
}
@@ -31,10 +30,10 @@
if err != nil {
return err
}
-
+ lf := fl.ListenFlags()
// TODO(ashankar): Set the second argument to r.Principal() once the
// old security model is no longer operational.
- proxy, err := proxy.New(rid, nil, fl.ListenProtocolFlag.String(), fl.ListenAddressFlag.String(), "")
+ proxy, err := proxy.New(rid, nil, lf.ListenProtocol.String(), lf.ListenAddress.String(), "")
if err != nil {
return err
}
diff --git a/lib/modules/core/util.go b/lib/modules/core/util.go
index 507b7e3..81b6101 100644
--- a/lib/modules/core/util.go
+++ b/lib/modules/core/util.go
@@ -9,23 +9,22 @@
"veyron.io/veyron/veyron/lib/flags"
)
-// ParseCommonFlags parses the supplied args for the common set of flags
-// and environment variables defined in in the veyron/lib/flags package.
-func ParseCommonFlags(args []string) (*flags.Flags, error) {
+func parseListenFlags(args []string) (*flags.Flags, []string, error) {
fs := flag.NewFlagSet("modules/core", flag.ContinueOnError)
- fl := flags.New(fs)
+ fl := flags.CreateAndRegister(fs, flags.Listen)
if len(args) == 0 {
- return fl, fmt.Errorf("no args supplied")
+ return fl, []string{}, nil
}
err := fl.Parse(args[1:])
- return fl, err
+ return fl, fl.Args(), err
}
func initListenSpec(fl *flags.Flags) ipc.ListenSpec {
+ lf := fl.ListenFlags()
return ipc.ListenSpec{
- Protocol: fl.ListenProtocolFlag.String(),
- Address: fl.ListenAddressFlag.String(),
- Proxy: fl.ListenProxyFlag,
+ Protocol: lf.ListenProtocol.String(),
+ Address: lf.ListenAddress.String(),
+ Proxy: lf.ListenProxy,
}
}
diff --git a/profiles/roaming/roaming.go b/profiles/roaming/roaming.go
index 2f9381f..816b4a1 100644
--- a/profiles/roaming/roaming.go
+++ b/profiles/roaming/roaming.go
@@ -37,7 +37,7 @@
)
func init() {
- commonFlags = flags.New(flag.CommandLine)
+ commonFlags = flags.CreateAndRegister(flag.CommandLine, flags.Listen)
rt.RegisterProfile(New())
}
@@ -69,10 +69,11 @@
func (p *profile) Init(rt veyron2.Runtime, publisher *config.Publisher) error {
log := rt.Logger()
+ lf := commonFlags.ListenFlags()
ListenSpec = ipc.ListenSpec{
- Protocol: commonFlags.ListenProtocolFlag.Protocol,
- Address: commonFlags.ListenAddressFlag.String(),
- Proxy: commonFlags.ListenProxyFlag,
+ Protocol: lf.ListenProtocol.Protocol,
+ Address: lf.ListenAddress.String(),
+ Proxy: lf.ListenProxy,
}
// 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 4997720..ba90318 100644
--- a/profiles/static/static.go
+++ b/profiles/static/static.go
@@ -25,7 +25,7 @@
)
func init() {
- commonFlags = flags.New(flag.CommandLine)
+ commonFlags = flags.CreateAndRegister(flag.CommandLine, flags.Listen)
rt.RegisterProfile(New())
}
@@ -55,10 +55,11 @@
func (p *static) Init(rt veyron2.Runtime, _ *config.Publisher) error {
log := rt.Logger()
+ lf := commonFlags.ListenFlags()
ListenSpec = ipc.ListenSpec{
- Protocol: commonFlags.ListenProtocolFlag.Protocol,
- Address: commonFlags.ListenAddressFlag.String(),
- Proxy: commonFlags.ListenProxyFlag,
+ Protocol: lf.ListenProtocol.Protocol,
+ Address: lf.ListenAddress.String(),
+ Proxy: lf.ListenProxy,
}
// Our address is private, so we test for running on GCE and for its
