Add host address to BindTransportException message #51269
Conversation
When bind fails, show the host address in addition to the port. This helps debugging cases with wrong "network.host" values. Closes elastic#48001
original-brownbear
added
the
:Distributed Coordination/Network
|
Pinging @elastic/es-distributed (:Distributed/Network) |
|
@elasticmachine test this please |
| }); | ||
| if (!success) { | ||
| throw new BindHttpException("Failed to bind to [" + port.getPortRangeString() + "]", lastException.get()); | ||
| throw new BindHttpException( |
There was a problem hiding this comment.
This is failing forbidden APIs checks, see the logs:
Forbidden method invocation: java.net.InetAddress#getHostAddress() [use NetworkAddress format/formatAddress to print IP or IP+ports]
in org.elasticsearch.transport.TcpTransport (TcpTransport.java:381)
Forbidden method invocation: java.net.InetAddress#getHostAddress() [use NetworkAddress format/formatAddress to print IP or IP+ports]
in org.elasticsearch.http.AbstractHttpServerTransport (AbstractHttpServerTransport.java:178)
Scanned 5715 class file(s) for forbidden API invocations (in 5.20s), 2 error(s).
There was a problem hiding this comment.
Hi @original-brownbear, thanks for reviewing this. I ran the tests but forgot to run the check task. Will fix this ASAP.
There was a problem hiding this comment.
@mariaral FYI, you can just run these simple static code checks via:
./gradlew precommit
no need to run a full check for those
There was a problem hiding this comment.
Thanks @original-brownbear for the tip. I didn't know of the precommit task.
I'm thinking of extending NetworkAddress.format() to be able to handle PortsRange objects and use that instead of coping the same message in five places. I propose using the following format:
IPv4 no port: 127.0.0.1
IPv4 single port: 127.0.0.1:9300
IPv4 multiple ports: 127.0.0.1:[9300-9400]
IPv6 multiple ports: [::1]:[9300-9400]
What do you think?
There was a problem hiding this comment.
I only count two spots where we print the PortsRange but fine by me to keep it consistent via a utility method :)
There was a problem hiding this comment.
I pushed a new commit. Please take a look.
Extend NetworkAddress.format() to handle port ranges and use this utility method instead of forbidden InetAddress.getHostAddress().
|
@elasticmachine test this please |
server/src/main/java/org/elasticsearch/common/network/NetworkAddress.java
Show resolved
Hide resolved
|
@elasticmachine update branch |
|
@elasticmachine ok to test |
|
Failures look unrelated. @elasticmachine test this please |
|
@elasticmachine update branch |
When bind fails, show the host address in addition to the port. This helps debugging cases with wrong "network.host" values. Closes #48001
|
This is merged and backported to 7.x now. Thanks for your contribution @mariaral. |
5 participants
When bind fails, show the host address in addition to the port. This
helps debugging cases with wrong "network.host" values.
Closes #48001