diff --git a/main.go b/main.go index 1835b73f5..38268f6cb 100644 --- a/main.go +++ b/main.go @@ -5,7 +5,6 @@ import ( "fmt" "log" "net/http" - "net/url" "os" "path" "strings" @@ -38,10 +37,6 @@ var ( printVersion = flag.Bool("version", false, "Print the version and exit") cluster = &etcdserver.Cluster{} - lcurls = &flagtypes.URLs{} - acurls = &flagtypes.URLs{} - lpurls = &flagtypes.URLs{} - apurls = &flagtypes.URLs{} cors = &pkg.CORSInfo{} proxyFlag = new(flagtypes.Proxy) @@ -49,7 +44,6 @@ var ( peerTLSInfo = transport.TLSInfo{} deprecated = []string{ - "addr", "cluster-active-size", "cluster-remove-delay", "cluster-sync-interval", @@ -57,7 +51,6 @@ var ( "force", "max-result-buffer", "max-retry-attempts", - "peer-addr", "peer-heartbeat-interval", "peer-election-timeout", "retry-interval", @@ -69,19 +62,16 @@ var ( func init() { flag.Var(cluster, "bootstrap-config", "Initial cluster configuration for bootstrapping") - flag.Var(apurls, "advertise-peer-urls", "List of this member's peer URLs to advertise to the rest of the cluster") - flag.Var(acurls, "advertise-client-urls", "List of this member's client URLs to advertise to the rest of the cluster") - flag.Var(lpurls, "listen-peer-urls", "List of this URLs to listen on for peer traffic") - flag.Var(lcurls, "listen-client-urls", "List of this URLs to listen on for client traffic") - flag.Var(cors, "cors", "Comma-separated white list of origins for CORS (cross-origin resource sharing).") - flag.Var(proxyFlag, "proxy", fmt.Sprintf("Valid values include %s", strings.Join(flagtypes.ProxyValues, ", "))) - cluster.Set("default=http://localhost:2380,default=http://localhost:7001") - lcurls.Set("http://localhost:2379,http://localhost:4001") - acurls.Set("http://localhost:2379,http://localhost:4001") - lpurls.Set("http://localhost:2380,http://localhost:7001") - apurls.Set("http://localhost:2380,http://localhost:7001") + flag.Var(flagtypes.NewURLs("http://localhost:2380,http://localhost:7001"), "advertise-peer-urls", "List of this member's peer URLs to advertise to the rest of the cluster") + flag.Var(flagtypes.NewURLs("http://localhost:2379,http://localhost:4001"), "advertise-client-urls", "List of this member's client URLs to advertise to the rest of the cluster") + flag.Var(flagtypes.NewURLs("http://localhost:2380,http://localhost:7001"), "listen-peer-urls", "List of this URLs to listen on for peer traffic") + flag.Var(flagtypes.NewURLs("http://localhost:2379,http://localhost:4001"), "listen-client-urls", "List of this URLs to listen on for client traffic") + + flag.Var(cors, "cors", "Comma-separated white list of origins for CORS (cross-origin resource sharing).") + + flag.Var(proxyFlag, "proxy", fmt.Sprintf("Valid values include %s", strings.Join(flagtypes.ProxyValues, ", "))) proxyFlag.Set(flagtypes.ProxyValueOff) flag.StringVar(&clientTLSInfo.CAFile, "ca-file", "", "Path to the client server TLS CA file.") @@ -92,6 +82,12 @@ func init() { flag.StringVar(&peerTLSInfo.CertFile, "peer-cert-file", "", "Path to the peer server TLS cert file.") flag.StringVar(&peerTLSInfo.KeyFile, "peer-key-file", "", "Path to the peer server TLS key file.") + // backwards-compatibility with v0.4.6 + flag.Var(&flagtypes.IPAddressPort{}, "addr", "DEPRECATED: Use -advertise-client-urls instead.") + flag.Var(&flagtypes.IPAddressPort{}, "bind-addr", "DEPRECATED: Use -listen-client-urls instead.") + flag.Var(&flagtypes.IPAddressPort{}, "peer-addr", "DEPRECATED: Use -advertise-peer-urls instead.") + flag.Var(&flagtypes.IPAddressPort{}, "peer-bind-addr", "DEPRECATED: Use -listen-peer-urls instead.") + for _, f := range deprecated { flag.Var(&pkg.DeprecatedFlag{f}, f, "") } @@ -215,7 +211,12 @@ func startEtcd() { } ph := etcdhttp.NewPeerHandler(s) - for _, u := range []url.URL(*lpurls) { + lpurls, err := pkg.URLsFromFlags(flag.CommandLine, "listen-peer-urls", "peer-bind-addr", clientTLSInfo) + if err != nil { + log.Fatal(err.Error()) + } + + for _, u := range lpurls { l, err := transport.NewListener(u.Host, peerTLSInfo) if err != nil { log.Fatal(err) @@ -229,8 +230,13 @@ func startEtcd() { }() } + lcurls, err := pkg.URLsFromFlags(flag.CommandLine, "listen-client-urls", "bind-addr", clientTLSInfo) + if err != nil { + log.Fatal(err.Error()) + } + // Start a client server goroutine for each listen address - for _, u := range []url.URL(*lcurls) { + for _, u := range lcurls { l, err := transport.NewListener(u.Host, clientTLSInfo) if err != nil { log.Fatal(err) @@ -265,8 +271,12 @@ func startProxy() { ph = proxy.NewReadonlyHandler(ph) } + lcurls, err := pkg.URLsFromFlags(flag.CommandLine, "listen-client-urls", "bind-addr", clientTLSInfo) + if err != nil { + log.Fatal(err.Error()) + } // Start a proxy server goroutine for each listen address - for _, u := range []url.URL(*lcurls) { + for _, u := range lcurls { l, err := transport.NewListener(u.Host, clientTLSInfo) if err != nil { log.Fatal(err) diff --git a/pkg/flag.go b/pkg/flag.go index b09374268..3eb66bef5 100644 --- a/pkg/flag.go +++ b/pkg/flag.go @@ -4,8 +4,12 @@ import ( "flag" "fmt" "log" + "net/url" "os" "strings" + + "github.com/coreos/etcd/pkg/flags" + "github.com/coreos/etcd/pkg/transport" ) type DeprecatedFlag struct { @@ -64,3 +68,38 @@ func SetFlagsFromEnv(fs *flag.FlagSet) { } }) } + +// URLsFromFlags decides what URLs should be using two different flags +// as datasources. The first flag's Value must be of type URLs, while +// the second must be of type IPAddressPort. If both of these flags +// are set, an error will be returned. If only the first flag is set, +// the underlying url.URL objects will be returned unmodified. If the +// second flag happens to be set, the underlying IPAddressPort will be +// converted to a url.URL and returned. The Scheme of the returned +// url.URL will be http unless the provided TLSInfo object is non-empty. +// If neither of the flags have been explicitly set, the default value +// of the first flag will be returned unmodified. +func URLsFromFlags(fs *flag.FlagSet, urlsFlagName string, addrFlagName string, tlsInfo transport.TLSInfo) ([]url.URL, error) { + visited := make(map[string]struct{}) + fs.Visit(func(f *flag.Flag) { + visited[f.Name] = struct{}{} + }) + + _, urlsFlagIsSet := visited[urlsFlagName] + _, addrFlagIsSet := visited[addrFlagName] + + if addrFlagIsSet { + if urlsFlagIsSet { + return nil, fmt.Errorf("Set only one of flags -%s and -%s", urlsFlagName, addrFlagName) + } + + addr := *fs.Lookup(addrFlagName).Value.(*flags.IPAddressPort) + addrURL := url.URL{Scheme: "http", Host: addr.String()} + if !tlsInfo.Empty() { + addrURL.Scheme = "https" + } + return []url.URL{addrURL}, nil + } + + return []url.URL(*fs.Lookup(urlsFlagName).Value.(*flags.URLs)), nil +} diff --git a/pkg/flag_test.go b/pkg/flag_test.go index 0ae424e6d..32fd84a67 100644 --- a/pkg/flag_test.go +++ b/pkg/flag_test.go @@ -2,8 +2,13 @@ package pkg import ( "flag" + "net/url" "os" + "reflect" "testing" + + "github.com/coreos/etcd/pkg/flags" + "github.com/coreos/etcd/pkg/transport" ) func TestSetFlagsFromEnv(t *testing.T) { @@ -49,3 +54,85 @@ func TestSetFlagsFromEnv(t *testing.T) { } } } + +func TestURLsFromFlags(t *testing.T) { + tests := []struct { + args []string + tlsInfo transport.TLSInfo + wantURLs []url.URL + wantFail bool + }{ + // use -urls default when no flags defined + { + args: []string{}, + tlsInfo: transport.TLSInfo{}, + wantURLs: []url.URL{ + url.URL{Scheme: "http", Host: "127.0.0.1:2379"}, + }, + wantFail: false, + }, + + // explicitly setting -urls should carry through + { + args: []string{"-urls=https://192.0.3.17:2930,http://127.0.0.1:1024"}, + tlsInfo: transport.TLSInfo{}, + wantURLs: []url.URL{ + url.URL{Scheme: "https", Host: "192.0.3.17:2930"}, + url.URL{Scheme: "http", Host: "127.0.0.1:1024"}, + }, + wantFail: false, + }, + + // explicitly setting -addr should carry through + { + args: []string{"-addr=192.0.2.3:1024"}, + tlsInfo: transport.TLSInfo{}, + wantURLs: []url.URL{ + url.URL{Scheme: "http", Host: "192.0.2.3:1024"}, + }, + wantFail: false, + }, + + // scheme prepended to -addr should be https if TLSInfo non-empty + { + args: []string{"-addr=192.0.2.3:1024"}, + tlsInfo: transport.TLSInfo{ + CertFile: "/tmp/foo", + KeyFile: "/tmp/bar", + }, + wantURLs: []url.URL{ + url.URL{Scheme: "https", Host: "192.0.2.3:1024"}, + }, + wantFail: false, + }, + + // explicitly setting both -urls and -addr should fail + { + args: []string{"-urls=https://127.0.0.1:1024", "-addr=192.0.2.3:1024"}, + tlsInfo: transport.TLSInfo{}, + wantURLs: nil, + wantFail: true, + }, + } + + for i, tt := range tests { + fs := flag.NewFlagSet("test", flag.PanicOnError) + fs.Var(flags.NewURLs("http://127.0.0.1:2379"), "urls", "") + fs.Var(&flags.IPAddressPort{}, "addr", "") + + if err := fs.Parse(tt.args); err != nil { + t.Errorf("#%d: failed to parse flags: %v", i, err) + continue + } + + gotURLs, err := URLsFromFlags(fs, "urls", "addr", tt.tlsInfo) + if tt.wantFail != (err != nil) { + t.Errorf("#%d: wantFail=%t, got err=%v", i, tt.wantFail, err) + continue + } + + if !reflect.DeepEqual(tt.wantURLs, gotURLs) { + t.Errorf("#%d: incorrect URLs\nwant=%#v\ngot=%#v", i, tt.wantURLs, gotURLs) + } + } +} diff --git a/pkg/flags/addrs.go b/pkg/flags/addrs.go deleted file mode 100644 index b9892831a..000000000 --- a/pkg/flags/addrs.go +++ /dev/null @@ -1,51 +0,0 @@ -package flags - -import ( - "errors" - "net" - "strconv" - "strings" -) - -// Addrs implements the flag.Value interface to allow users to define multiple -// listen addresses on the command-line -type Addrs []string - -// Set parses a command line set of listen addresses, formatted like: -// 127.0.0.1:7001,10.1.1.2:80 -func (as *Addrs) Set(s string) error { - parsed := make([]string, 0) - for _, in := range strings.Split(s, ",") { - a := strings.TrimSpace(in) - if err := validateAddr(a); err != nil { - return err - } - parsed = append(parsed, a) - } - if len(parsed) == 0 { - return errors.New("no valid addresses given!") - } - *as = parsed - return nil -} - -func (as *Addrs) String() string { - return strings.Join(*as, ",") -} - -// validateAddr ensures that the provided string is a valid address. Valid -// addresses are of the form IP:port. -// Returns an error if the address is invalid, else nil. -func validateAddr(s string) error { - parts := strings.SplitN(s, ":", 2) - if len(parts) != 2 { - return errors.New("bad format in address specification") - } - if net.ParseIP(parts[0]) == nil { - return errors.New("bad IP in address specification") - } - if _, err := strconv.Atoi(parts[1]); err != nil { - return errors.New("bad port in address specification") - } - return nil -} diff --git a/pkg/flags/addrs_test.go b/pkg/flags/addrs_test.go deleted file mode 100644 index 092a5504d..000000000 --- a/pkg/flags/addrs_test.go +++ /dev/null @@ -1,42 +0,0 @@ -package flags - -import ( - "testing" -) - -func TestBadValidateAddr(t *testing.T) { - tests := []string{ - // bad IP specification - ":4001", - "127.0:8080", - "123:456", - // bad port specification - "127.0.0.1:foo", - "127.0.0.1:", - // unix sockets not supported - "unix://", - "unix://tmp/etcd.sock", - // bad strings - "somewhere", - "234#$", - "file://foo/bar", - "http://hello", - } - for i, in := range tests { - if err := validateAddr(in); err == nil { - t.Errorf(`#%d: unexpected nil error for in=%q`, i, in) - } - } -} - -func TestValidateAddr(t *testing.T) { - tests := []string{ - "1.2.3.4:8080", - "10.1.1.1:80", - } - for i, in := range tests { - if err := validateAddr(in); err != nil { - t.Errorf("#%d: err=%v, want nil for in=%q", i, err, in) - } - } -} diff --git a/pkg/flags/ipaddressport.go b/pkg/flags/ipaddressport.go new file mode 100644 index 000000000..41ad15210 --- /dev/null +++ b/pkg/flags/ipaddressport.go @@ -0,0 +1,43 @@ +package flags + +import ( + "errors" + "fmt" + "net" + "strconv" + "strings" +) + +// IPAddressPort implements the flag.Value interface. The argument +// is validated as "ip:port". +type IPAddressPort struct { + IP string + Port int +} + +func (a *IPAddressPort) Set(arg string) error { + arg = strings.TrimSpace(arg) + + parts := strings.SplitN(arg, ":", 2) + if len(parts) != 2 { + return errors.New("bad format in address specification") + } + + if net.ParseIP(parts[0]) == nil { + return errors.New("bad IP in address specification") + } + + port, err := strconv.Atoi(parts[1]) + if err != nil { + return errors.New("bad port in address specification") + } + + a.IP = parts[0] + a.Port = port + + return nil +} + +func (a *IPAddressPort) String() string { + return fmt.Sprintf("%s:%d", a.IP, a.Port) +} diff --git a/pkg/flags/ipaddressport_test.go b/pkg/flags/ipaddressport_test.go new file mode 100644 index 000000000..d9d60f9b2 --- /dev/null +++ b/pkg/flags/ipaddressport_test.go @@ -0,0 +1,57 @@ +package flags + +import ( + "testing" +) + +func TestIPAddressPortSet(t *testing.T) { + pass := []string{ + "1.2.3.4:8080", + "10.1.1.1:80", + } + + fail := []string{ + // bad IP specification + ":4001", + "127.0:8080", + "123:456", + // bad port specification + "127.0.0.1:foo", + "127.0.0.1:", + // unix sockets not supported + "unix://", + "unix://tmp/etcd.sock", + // bad strings + "somewhere", + "234#$", + "file://foo/bar", + "http://hello", + } + + for i, tt := range pass { + f := &IPAddressPort{} + if err := f.Set(tt); err != nil { + t.Errorf("#%d: unexpected error from IPAddressPort.Set(%q): %v", i, tt, err) + } + } + + for i, tt := range fail { + f := &IPAddressPort{} + if err := f.Set(tt); err == nil { + t.Errorf("#%d: expected error from IPAddressPort.Set(%q)", i, tt, err) + } + } +} + +func TestIPAddressPortString(t *testing.T) { + f := &IPAddressPort{} + if err := f.Set("127.0.0.1:4001"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := "127.0.0.1:4001" + got := f.String() + if want != got { + t.Fatalf("IPAddressPort.String() value should be %q, got %q", want, got) + } +} diff --git a/pkg/flags/urls.go b/pkg/flags/urls.go index af2121d89..7ed69ff3c 100644 --- a/pkg/flags/urls.go +++ b/pkg/flags/urls.go @@ -48,3 +48,9 @@ func (us *URLs) String() string { } return strings.Join(all, ",") } + +func NewURLs(init string) *URLs { + v := &URLs{} + v.Set(init) + return v +}