Mirroristas/etcd
2
0
Fork 0
mirror of https://github.com/etcd-io/etcd.git synced 2024-09-27 06:25:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

pkg: fix SetFlagsFromEnv behaviour

Browse Source 
This function was fundamentally buggy, as a panic could be trivially
triggered by setting the wrong environment variable (e.g.
ETCD_BIND_ADDR=foo). Instead, let's propagate the error and present it
to the user in a cleaner way.
This commit is contained in:
Jonathan Boulle 2014-11-06 14:39:30 -08:00
parent 087e0e8b62
commit 321d65c4ac
3 changed files with 21 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
etcdmain/etcd.go
View File

@ -161,9 +161,11 @@ func Main() {
os.Exit(0) os.Exit(0)
} }
flags.SetFlagsFromEnv(fs) err := flags.SetFlagsFromEnv(fs)
if err != nil {
log.Fatalf("etcd: %v", err)
}
var err error
shouldProxy := proxyFlag.String() != proxyFlagOff shouldProxy := proxyFlag.String() != proxyFlagOff
if !shouldProxy { if !shouldProxy {
err = startEtcd() err = startEtcd()

8
pkg/flags/flag.go
View File

@ -85,7 +85,7 @@ func UsageWithIgnoredFlagsFunc(fs *flag.FlagSet, ignore []string) func() {
// environment variables. Environment variables take the name of the flag but // environment variables. Environment variables take the name of the flag but
// are UPPERCASE, have the prefix "ETCD_", and any dashes are replaced by // are UPPERCASE, have the prefix "ETCD_", and any dashes are replaced by
// underscores - for example: some-flag => ETCD_SOME_FLAG // underscores - for example: some-flag => ETCD_SOME_FLAG
func SetFlagsFromEnv(fs *flag.FlagSet) { func SetFlagsFromEnv(fs *flag.FlagSet) (err error) {
alreadySet := make(map[string]bool) alreadySet := make(map[string]bool)
fs.Visit(func(f *flag.Flag) { fs.Visit(func(f *flag.Flag) {
alreadySet[f.Name] = true alreadySet[f.Name] = true
@ -95,13 +95,13 @@ func SetFlagsFromEnv(fs *flag.FlagSet) {
key := "ETCD_" + strings.ToUpper(strings.Replace(f.Name, "-", "_", -1)) key := "ETCD_" + strings.ToUpper(strings.Replace(f.Name, "-", "_", -1))
val := os.Getenv(key) val := os.Getenv(key)
if val != "" { if val != "" {
if err := fs.Set(f.Name, val); err != nil { if serr := fs.Set(f.Name, val); serr != nil {
// Should never happen err = fmt.Errorf("invalid value %q for %s: %v", val, key, serr)
log.Panicf("error setting flag from env: %v", err)
} }
} }
} }
}) })
return
} }
// URLsFromFlags decides what URLs should be using two different flags // URLsFromFlags decides what URLs should be using two different flags

14
pkg/flags/flag_test.go
View File

@ -58,7 +58,10 @@ func TestSetFlagsFromEnv(t *testing.T) {
} }
// now read the env and verify flags were updated as expected // now read the env and verify flags were updated as expected
SetFlagsFromEnv(fs) err := SetFlagsFromEnv(fs)
if err != nil {
t.Errorf("err=%v, want nil", err)
}
for f, want := range map[string]string{ for f, want := range map[string]string{
"a": "foo", "a": "foo",
"b": "bar", "b": "bar",
@ -68,6 +71,15 @@ func TestSetFlagsFromEnv(t *testing.T) {
t.Errorf("flag %q=%q, want %q", f, got, want) t.Errorf("flag %q=%q, want %q", f, got, want)
} }
} }
// now verify that an error is propagated
fs = flag.NewFlagSet("testing", flag.ExitOnError)
fs.Int("x", 0, "")
os.Setenv("ETCD_X", "not_a_number")
err = SetFlagsFromEnv(fs)
if err == nil {
t.Errorf("err=nil, want != nil")
}
} }
func TestURLsFromFlags(t *testing.T) { func TestURLsFromFlags(t *testing.T) {