fix(config.go)

return a valid URL struct from sanitizeURL()
pass the URL struct above to sanitizeBindAddr()

Since url.Parse() will return an error when parsing an already-parsed
ipv6 url string, (e.g. [http://[fe80::6203:8ff:fe9e:ace%25eth0]:7001),
so I just return the valid URL struct from sanitizeURL() and send it to
sanitizeBindAddr(), then there is no need to parse it again in sanitizeBindAddr().

Besides, for IPV6 url, the percent sign should be escaped, see:
http://en.wikipedia.org/wiki/IPv6_address#Link-local_addresses_and_zone_indices
release-0.4
Yifan Gu 2014-05-18 01:38:06 -07:00
parent ad9155c82a
commit b4e4bf4b75
2 changed files with 75 additions and 40 deletions

View File

@ -373,18 +373,19 @@ func (c *Config) Reset() error {
// Sanitize cleans the input fields. // Sanitize cleans the input fields.
func (c *Config) Sanitize() error { func (c *Config) Sanitize() error {
var err error var err error
var url *url.URL
// Sanitize the URLs first. // Sanitize the URLs first.
if c.Addr, err = sanitizeURL(c.Addr, c.EtcdTLSInfo().Scheme()); err != nil { if c.Addr, url, err = sanitizeURL(c.Addr, c.EtcdTLSInfo().Scheme()); err != nil {
return fmt.Errorf("Advertised URL: %s", err) return fmt.Errorf("Advertised URL: %s", err)
} }
if c.BindAddr, err = sanitizeBindAddr(c.BindAddr, c.Addr); err != nil { if c.BindAddr, err = sanitizeBindAddr(c.BindAddr, url); err != nil {
return fmt.Errorf("Listen Host: %s", err) return fmt.Errorf("Listen Host: %s", err)
} }
if c.Peer.Addr, err = sanitizeURL(c.Peer.Addr, c.PeerTLSInfo().Scheme()); err != nil { if c.Peer.Addr, url, err = sanitizeURL(c.Peer.Addr, c.PeerTLSInfo().Scheme()); err != nil {
return fmt.Errorf("Peer Advertised URL: %s", err) return fmt.Errorf("Peer Advertised URL: %s", err)
} }
if c.Peer.BindAddr, err = sanitizeBindAddr(c.Peer.BindAddr, c.Peer.Addr); err != nil { if c.Peer.BindAddr, err = sanitizeBindAddr(c.Peer.BindAddr, url); err != nil {
return fmt.Errorf("Peer Listen Host: %s", err) return fmt.Errorf("Peer Listen Host: %s", err)
} }
@ -440,35 +441,30 @@ func (c *Config) ClusterConfig() *server.ClusterConfig {
// sanitizeURL will cleanup a host string in the format hostname[:port] and // sanitizeURL will cleanup a host string in the format hostname[:port] and
// attach a schema. // attach a schema.
func sanitizeURL(host string, defaultScheme string) (string, error) { func sanitizeURL(host string, defaultScheme string) (string, *url.URL, error) {
// Blank URLs are fine input, just return it // Blank URLs are fine input, just return it
if len(host) == 0 { if len(host) == 0 {
return host, nil return host, &url.URL{}, nil
} }
p, err := url.Parse(host) p, err := url.Parse(host)
if err != nil { if err != nil {
return "", err return "", nil, err
} }
// Make sure the host is in Host:Port format // Make sure the host is in Host:Port format
_, _, err = net.SplitHostPort(host) _, _, err = net.SplitHostPort(host)
if err != nil { if err != nil {
return "", err return "", nil, err
} }
p = &url.URL{Host: host, Scheme: defaultScheme} p = &url.URL{Host: host, Scheme: defaultScheme}
return p.String(), nil return p.String(), p, nil
} }
// sanitizeBindAddr cleans up the BindAddr parameter and appends a port // sanitizeBindAddr cleans up the BindAddr parameter and appends a port
// if necessary based on the advertised port. // if necessary based on the advertised port.
func sanitizeBindAddr(bindAddr string, addr string) (string, error) { func sanitizeBindAddr(bindAddr string, aurl *url.URL) (string, error) {
aurl, err := url.Parse(addr)
if err != nil {
return "", err
}
// If it is a valid host:port simply return with no further checks. // If it is a valid host:port simply return with no further checks.
bhost, bport, err := net.SplitHostPort(bindAddr) bhost, bport, err := net.SplitHostPort(bindAddr)
if err == nil && bhost != "" { if err == nil && bhost != "" {

View File

@ -165,7 +165,7 @@ func TestConfigAbbreviatedForceFlag(t *testing.T) {
assert.True(t, c.Force) assert.True(t, c.Force)
} }
// Ensures that a the advertised url can be parsed from the environment. // Ensures that the advertised url can be parsed from the environment.
func TestConfigAddrEnv(t *testing.T) { func TestConfigAddrEnv(t *testing.T) {
withEnv("ETCD_ADDR", "127.0.0.1:4002", func(c *Config) { withEnv("ETCD_ADDR", "127.0.0.1:4002", func(c *Config) {
assert.Nil(t, c.LoadEnv(), "") assert.Nil(t, c.LoadEnv(), "")
@ -173,14 +173,14 @@ func TestConfigAddrEnv(t *testing.T) {
}) })
} }
// Ensures that a the advertised flag can be parsed. // Ensures that the advertised flag can be parsed.
func TestConfigAddrFlag(t *testing.T) { func TestConfigAddrFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4002"}), "") assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4002"}), "")
assert.Equal(t, c.Addr, "127.0.0.1:4002", "") assert.Equal(t, c.Addr, "127.0.0.1:4002", "")
} }
// Ensures that a the CA file can be parsed from the environment. // Ensures that the CA file can be parsed from the environment.
func TestConfigCAFileEnv(t *testing.T) { func TestConfigCAFileEnv(t *testing.T) {
withEnv("ETCD_CA_FILE", "/tmp/file.ca", func(c *Config) { withEnv("ETCD_CA_FILE", "/tmp/file.ca", func(c *Config) {
assert.Nil(t, c.LoadEnv(), "") assert.Nil(t, c.LoadEnv(), "")
@ -188,14 +188,14 @@ func TestConfigCAFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the CA file flag can be parsed. // Ensures that the CA file flag can be parsed.
func TestConfigCAFileFlag(t *testing.T) { func TestConfigCAFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-ca-file", "/tmp/file.ca"}), "") assert.Nil(t, c.LoadFlags([]string{"-ca-file", "/tmp/file.ca"}), "")
assert.Equal(t, c.CAFile, "/tmp/file.ca", "") assert.Equal(t, c.CAFile, "/tmp/file.ca", "")
} }
// Ensures that a the CA file can be parsed from the environment. // Ensures that the CA file can be parsed from the environment.
func TestConfigCertFileEnv(t *testing.T) { func TestConfigCertFileEnv(t *testing.T) {
withEnv("ETCD_CERT_FILE", "/tmp/file.cert", func(c *Config) { withEnv("ETCD_CERT_FILE", "/tmp/file.cert", func(c *Config) {
assert.Nil(t, c.LoadEnv(), "") assert.Nil(t, c.LoadEnv(), "")
@ -203,14 +203,14 @@ func TestConfigCertFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the Cert file flag can be parsed. // Ensures that the Cert file flag can be parsed.
func TestConfigCertFileFlag(t *testing.T) { func TestConfigCertFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-cert-file", "/tmp/file.cert"}), "") assert.Nil(t, c.LoadFlags([]string{"-cert-file", "/tmp/file.cert"}), "")
assert.Equal(t, c.CertFile, "/tmp/file.cert", "") assert.Equal(t, c.CertFile, "/tmp/file.cert", "")
} }
// Ensures that a the Key file can be parsed from the environment. // Ensures that the Key file can be parsed from the environment.
func TestConfigKeyFileEnv(t *testing.T) { func TestConfigKeyFileEnv(t *testing.T) {
withEnv("ETCD_KEY_FILE", "/tmp/file.key", func(c *Config) { withEnv("ETCD_KEY_FILE", "/tmp/file.key", func(c *Config) {
assert.Nil(t, c.LoadEnv(), "") assert.Nil(t, c.LoadEnv(), "")
@ -218,14 +218,14 @@ func TestConfigKeyFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the Key file flag can be parsed. // Ensures that the Key file flag can be parsed.
func TestConfigKeyFileFlag(t *testing.T) { func TestConfigKeyFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-key-file", "/tmp/file.key"}), "") assert.Nil(t, c.LoadFlags([]string{"-key-file", "/tmp/file.key"}), "")
assert.Equal(t, c.KeyFile, "/tmp/file.key", "") assert.Equal(t, c.KeyFile, "/tmp/file.key", "")
} }
// Ensures that a the Listen Host can be parsed from the environment. // Ensures that the Listen Host can be parsed from the environment.
func TestConfigBindAddrEnv(t *testing.T) { func TestConfigBindAddrEnv(t *testing.T) {
withEnv("ETCD_BIND_ADDR", "127.0.0.1:4003", func(c *Config) { withEnv("ETCD_BIND_ADDR", "127.0.0.1:4003", func(c *Config) {
assert.Nil(t, c.LoadEnv(), "") assert.Nil(t, c.LoadEnv(), "")
@ -233,14 +233,14 @@ func TestConfigBindAddrEnv(t *testing.T) {
}) })
} }
// Ensures that a the Listen Host file flag can be parsed. // Ensures that the Listen Host file flag can be parsed.
func TestConfigBindAddrFlag(t *testing.T) { func TestConfigBindAddrFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-bind-addr", "127.0.0.1:4003"}), "") assert.Nil(t, c.LoadFlags([]string{"-bind-addr", "127.0.0.1:4003"}), "")
assert.Equal(t, c.BindAddr, "127.0.0.1:4003", "") assert.Equal(t, c.BindAddr, "127.0.0.1:4003", "")
} }
// Ensures that a the Listen Host port overrides the advertised port // Ensures that the Listen Host port overrides the advertised port
func TestConfigBindAddrOverride(t *testing.T) { func TestConfigBindAddrOverride(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", "127.0.0.1:4010"}), "") assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", "127.0.0.1:4010"}), "")
@ -248,7 +248,23 @@ func TestConfigBindAddrOverride(t *testing.T) {
assert.Equal(t, c.BindAddr, "127.0.0.1:4010", "") assert.Equal(t, c.BindAddr, "127.0.0.1:4010", "")
} }
// Ensures that a the Listen Host inherits its port from the advertised addr // Ensures that the Listen Host port overrides the advertised port
func TestConfigBindIPv6AddrOverride(t *testing.T) {
c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "[::1]:4009", "-bind-addr", "[::1]:4010"}), "")
assert.Nil(t, c.Sanitize())
assert.Equal(t, c.BindAddr, "[::1]:4010", "")
}
// Ensures that the Listen Host port overrides the advertised port
func TestConfigBindIPv6WithZoneAddrOverride(t *testing.T) {
c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "[::1%25lo]:4009", "-bind-addr", "[::1%25lo]:4010"}), "")
assert.Nil(t, c.Sanitize())
assert.Equal(t, c.BindAddr, "[::1%25lo]:4010", "")
}
// Ensures that the Listen Host inherits its port from the advertised addr
func TestConfigBindAddrInheritPort(t *testing.T) { func TestConfigBindAddrInheritPort(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", "127.0.0.1"}), "") assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", "127.0.0.1"}), "")
@ -256,6 +272,22 @@ func TestConfigBindAddrInheritPort(t *testing.T) {
assert.Equal(t, c.BindAddr, "127.0.0.1:4009", "") assert.Equal(t, c.BindAddr, "127.0.0.1:4009", "")
} }
// Ensures that the Listen Host inherits its port from the advertised addr
func TestConfigBindIPv6AddrInheritPort(t *testing.T) {
c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "[::1]:4009", "-bind-addr", "::1"}), "")
assert.Nil(t, c.Sanitize())
assert.Equal(t, c.BindAddr, "[::1]:4009", "")
}
// Ensures that the Listen Host inherits its port from the advertised addr
func TestConfigBindIPv6WithZoneAddrInheritPort(t *testing.T) {
c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "[::1%25lo]:4009", "-bind-addr", "::1%25lo"}), "")
assert.Nil(t, c.Sanitize())
assert.Equal(t, c.BindAddr, "[::1%25lo]:4009", "")
}
// Ensures that a port only argument errors out // Ensures that a port only argument errors out
func TestConfigBindAddrErrorOnNoHost(t *testing.T) { func TestConfigBindAddrErrorOnNoHost(t *testing.T) {
c := New() c := New()
@ -263,6 +295,13 @@ func TestConfigBindAddrErrorOnNoHost(t *testing.T) {
assert.Error(t, c.Sanitize()) assert.Error(t, c.Sanitize())
} }
// Ensures that a bad IPv6 address will raise an error
func TestConfigBindAddrErrorOnBadIPv6Addr(t *testing.T) {
c := New()
assert.Nil(t, c.LoadFlags([]string{"-addr", "[::1%lo]:4009"}), "")
assert.Error(t, c.Sanitize())
}
// Ensures that the peers can be parsed from the environment. // Ensures that the peers can be parsed from the environment.
func TestConfigPeersEnv(t *testing.T) { func TestConfigPeersEnv(t *testing.T) {
withEnv("ETCD_PEERS", "coreos.com:4001,coreos.com:4002", func(c *Config) { withEnv("ETCD_PEERS", "coreos.com:4001,coreos.com:4002", func(c *Config) {
@ -271,7 +310,7 @@ func TestConfigPeersEnv(t *testing.T) {
}) })
} }
// Ensures that a the Peers flag can be parsed. // Ensures that the Peers flag can be parsed.
func TestConfigPeersFlag(t *testing.T) { func TestConfigPeersFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peers", "coreos.com:4001,coreos.com:4002"}), "") assert.Nil(t, c.LoadFlags([]string{"-peers", "coreos.com:4001,coreos.com:4002"}), "")
@ -286,7 +325,7 @@ func TestConfigPeersFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the Peers File flag can be parsed. // Ensures that the Peers File flag can be parsed.
func TestConfigPeersFileFlag(t *testing.T) { func TestConfigPeersFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peers-file", "/tmp/peers"}), "") assert.Nil(t, c.LoadFlags([]string{"-peers-file", "/tmp/peers"}), "")
@ -301,7 +340,7 @@ func TestConfigMaxResultBufferEnv(t *testing.T) {
}) })
} }
// Ensures that a the Max Result Buffer flag can be parsed. // Ensures that the Max Result Buffer flag can be parsed.
func TestConfigMaxResultBufferFlag(t *testing.T) { func TestConfigMaxResultBufferFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-max-result-buffer", "512"}), "") assert.Nil(t, c.LoadFlags([]string{"-max-result-buffer", "512"}), "")
@ -316,7 +355,7 @@ func TestConfigMaxRetryAttemptsEnv(t *testing.T) {
}) })
} }
// Ensures that a the Max Retry Attempts flag can be parsed. // Ensures that the Max Retry Attempts flag can be parsed.
func TestConfigMaxRetryAttemptsFlag(t *testing.T) { func TestConfigMaxRetryAttemptsFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-max-retry-attempts", "10"}), "") assert.Nil(t, c.LoadFlags([]string{"-max-retry-attempts", "10"}), "")
@ -331,7 +370,7 @@ func TestConfigNameEnv(t *testing.T) {
}) })
} }
// Ensures that a the Name flag can be parsed. // Ensures that the Name flag can be parsed.
func TestConfigNameFlag(t *testing.T) { func TestConfigNameFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-name", "test-name"}), "") assert.Nil(t, c.LoadFlags([]string{"-name", "test-name"}), "")
@ -364,7 +403,7 @@ func TestConfigSnapshotEnv(t *testing.T) {
}) })
} }
// Ensures that a the Snapshot flag can be parsed. // Ensures that the Snapshot flag can be parsed.
func TestConfigSnapshotFlag(t *testing.T) { func TestConfigSnapshotFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-snapshot"}), "") assert.Nil(t, c.LoadFlags([]string{"-snapshot"}), "")
@ -379,7 +418,7 @@ func TestConfigVerboseEnv(t *testing.T) {
}) })
} }
// Ensures that a the Verbose flag can be parsed. // Ensures that the Verbose flag can be parsed.
func TestConfigVerboseFlag(t *testing.T) { func TestConfigVerboseFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-v"}), "") assert.Nil(t, c.LoadFlags([]string{"-v"}), "")
@ -394,7 +433,7 @@ func TestConfigVeryVerboseEnv(t *testing.T) {
}) })
} }
// Ensures that a the Very Verbose flag can be parsed. // Ensures that the Very Verbose flag can be parsed.
func TestConfigVeryVerboseFlag(t *testing.T) { func TestConfigVeryVerboseFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-vv"}), "") assert.Nil(t, c.LoadFlags([]string{"-vv"}), "")
@ -409,7 +448,7 @@ func TestConfigPeerAddrEnv(t *testing.T) {
}) })
} }
// Ensures that a the Peer Advertised URL flag can be parsed. // Ensures that the Peer Advertised URL flag can be parsed.
func TestConfigPeerAddrFlag(t *testing.T) { func TestConfigPeerAddrFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peer-addr", "localhost:7002"}), "") assert.Nil(t, c.LoadFlags([]string{"-peer-addr", "localhost:7002"}), "")
@ -424,7 +463,7 @@ func TestConfigPeerCAFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the Peer CA file flag can be parsed. // Ensures that the Peer CA file flag can be parsed.
func TestConfigPeerCAFileFlag(t *testing.T) { func TestConfigPeerCAFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peer-ca-file", "/tmp/peer/file.ca"}), "") assert.Nil(t, c.LoadFlags([]string{"-peer-ca-file", "/tmp/peer/file.ca"}), "")
@ -439,7 +478,7 @@ func TestConfigPeerCertFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the Cert file flag can be parsed. // Ensures that the Cert file flag can be parsed.
func TestConfigPeerCertFileFlag(t *testing.T) { func TestConfigPeerCertFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peer-cert-file", "/tmp/peer/file.cert"}), "") assert.Nil(t, c.LoadFlags([]string{"-peer-cert-file", "/tmp/peer/file.cert"}), "")
@ -454,7 +493,7 @@ func TestConfigPeerKeyFileEnv(t *testing.T) {
}) })
} }
// Ensures that a the Peer Key file flag can be parsed. // Ensures that the Peer Key file flag can be parsed.
func TestConfigPeerKeyFileFlag(t *testing.T) { func TestConfigPeerKeyFileFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peer-key-file", "/tmp/peer/file.key"}), "") assert.Nil(t, c.LoadFlags([]string{"-peer-key-file", "/tmp/peer/file.key"}), "")
@ -477,7 +516,7 @@ func TestConfigBadFlag(t *testing.T) {
assert.Equal(t, err.Error(), `flag provided but not defined: -no-such-flag`) assert.Equal(t, err.Error(), `flag provided but not defined: -no-such-flag`)
} }
// Ensures that a the Peer Listen Host file flag can be parsed. // Ensures that the Peer Listen Host file flag can be parsed.
func TestConfigPeerBindAddrFlag(t *testing.T) { func TestConfigPeerBindAddrFlag(t *testing.T) {
c := New() c := New()
assert.Nil(t, c.LoadFlags([]string{"-peer-bind-addr", "127.0.0.1:4003"}), "") assert.Nil(t, c.LoadFlags([]string{"-peer-bind-addr", "127.0.0.1:4003"}), "")