jurmous / etcd4j

Java / Netty client for etcd, the highly-available key value store for shared configuration and service discovery.
Apache License 2.0
267 stars 83 forks source link

fix netty connect port if uri does not have port or behind some proxy like nginx #64

Closed burakdede closed 8 years ago

burakdede commented 8 years ago
    java.lang.IllegalArgumentException: port out of range:-1
at java.net.InetSocketAddress.checkPort(InetSocketAddress.java:143)
at java.net.InetSocketAddress.createUnresolved(InetSocketAddress.java:254)
at io.netty.bootstrap.Bootstrap.connect(Bootstrap.java:120)
at mousio.etcd4j.transport.EtcdNettyClient.connect(EtcdNettyClient.java:223)
at mousio.etcd4j.transport.EtcdNettyClient.send(EtcdNettyClient.java:178)
at mousio.etcd4j.requests.EtcdKeyRequest.send(EtcdKeyRequest.java:75)

This tries to fix problem when given uri does not have port information attached on EtcdNettyClient.connect. This happens when you put your nodes in cluster behind some proxy like nginx to enable basic authentication (which by default listens port 80).

Of course sending port information attached to URI when creating EtcdClient explicitly is another solution, let me know which one works for you.

dengshuan commented 8 years ago

I got the same error. But i was using http://my_etcd_container:2379 to create an EtcdClient. So at first i thought this PR would not solve the problem. But my problem was that i contains underscore in my hostname, that is invalid. So java.net.URI can't parse it.

But i'm wondering if using 80 as default port is good enough, because etcd itself uses 4001 for client and 7001 for server as default port

lburgazzoli commented 8 years ago

I think that if the port is not provided it should eventually default to etcd defaults, not port 80.

An additional option is to retrieve connection info from env like etcdctl does

burakdede commented 8 years ago

Yes, my point was etcd node may sit behind of some kind of proxy and it may not have explicit port information (80 does not make sense and work for everyone, thats true) but at least it should not fail because no explicit port information provided.

lburgazzoli commented 8 years ago

@burakdd would you be able to rework this PR to behave according to the discussion ?

See etcdct- endpoint

burakdede commented 8 years ago

@lburgazzoli

lburgazzoli commented 8 years ago

Thx for the PR

dengshuan commented 8 years ago

:+1: thanks