Bind to configured IP address on netty servers #320
Conversation
Replace call to super on the `NettyGrpcServer` and `ShaddedNettyGrpcServer` `newServerBuilder()` method, with a call to the `NettyServerBuilder.forAddress` that allows defining the IP and port to be bound to using a `SocketAddress`. The glue logic for getting the `SocketAddress` from the configured address string is in the `DefaultGrpcServerFactory` as is common to both inheritor classes. Signed-off-by: Ivan Lorca <ivan.lorca@spacewell.com> [resolves spring-projects#319]
| testIpAddress("", new InetSocketAddress(9090))); | ||
| } | ||
|
|
||
| private DynamicTest testIpAddress(String address, SocketAddress expected) { |
| .workerEventLoopGroup(new EpollEventLoopGroup()); | ||
| } | ||
| return super.newServerBuilder(); | ||
| return NettyServerBuilder.forAddress(socketAddress(), credentials()); |
There was a problem hiding this comment.
I think this replacement makes sense and is "safe" as the previous code called:
Grpc.newServerBuilderForPort(port(), credentials());which ultimately called:
ProtocolNegotiators.FromServerCredentialsResult result = ProtocolNegotiators.from(creds);
if (result.error != null) {
return NewServerBuilderResult.error(result.error);
}
return NewServerBuilderResult.serverBuilder(
new NettyServerBuilder(new InetSocketAddress(port), result.negotiator));and the new code NettyServerBuilder.forAddress(socketAddress(), credentials()); does the same effectively but passes in the hostname not just the port.
Previously we were simply disregarding any host info (as noted in your issue).
| return address().split(":")[0].trim(); | ||
| } | ||
|
|
||
| protected SocketAddress socketAddress() { |
There was a problem hiding this comment.
I would prefer moving this to GrpcUtils like we do for port. Then rather than super.hostname the subs could call GrpcUtils.hostname.
| if (address.startsWith("unix:")) { | ||
| throw new UnsupportedOperationException("Unix socket addresses not supported"); | ||
| } | ||
| if (address.startsWith("in-process:")) { |
There was a problem hiding this comment.
I don't think in-process: string every makes it to the server side and is soley used on the client/channel side (i.e. its stripped by the time it gets here).
| } | ||
|
|
||
| protected String hostName() { | ||
| return address().split(":")[0].trim(); |
There was a problem hiding this comment.
I don't think this will handle all cases as users could pass in arbitrary addresses (e.g. 12::34::....). I think we need to use a regex to cover all cases. Wdyt?
2 participants
Replace call to super on the
NettyGrpcServerandShaddedNettyGrpcServernewServerBuilder()method, with a call to theNettyServerBuilder.forAddressthat allows defining the IP and port to be bound to using aSocketAddress.The glue logic for getting the
SocketAddressfrom the configured address string is in theDefaultGrpcServerFactoryas is common to both inheritor classes.Signed-off-by: Ivan Lorca ivan.lorca@spacewell.com
[resolves #319]