xds: listener type validation #11933
Conversation
shivaspeaks
added
the
kokoro:run
grpc-kokoro
removed
the
kokoro:run
| InetAddress listenerIp = InetAddresses.forString(listenerAddressHnP.getHost()); | ||
| InetAddress ldsIp = InetAddresses.forString(ldsAddressHnP.getHost()); | ||
|
|
||
| if (listenerIp.isAnyLocalAddress()) { |
There was a problem hiding this comment.
As I said before, remove the wildcard handling. IPv4 and IPv6 wildcards behave differently, and you aren't checking the port here. And if we need to be comparing wildcards, we would really need to spell out how that is done in the gRFC to be consistent cross-language.
| return; | ||
| } | ||
|
|
||
| String ldsAddress = update.listener().address(); |
There was a problem hiding this comment.
For my education: what would be the "best/most appropriate" behaviour if the String ldsAddress is the empty string or null?
Currently, if ldsAddress is null, we skip ipAddressesMatch entirely and proceed with the remaining logic. Would this still be fine?
Likewise, if ldsAddress is the empty string, we check ipAddressesMatch, AFAIK, this would throw an exception and would this be fine? HostAndPort.fromString would return a HostAndPort(host="", port=-1, hasBracketlessColons=false) then InetAddresses.forString would throw an IllegalArgumentException
There was a problem hiding this comment.
Hmmm. Good point. I couldn't find any mention about this case in gRFC A36.
| @Override | ||
| public void onResourceDoesNotExist(final String resourceName) { | ||
| if (stopped) { | ||
| return; | ||
| } | ||
| StatusException statusException = Status.UNAVAILABLE.withDescription( | ||
| String.format("Listener %s unavailable, xDS node ID: %s", resourceName, | ||
| String.format("%s listener unavailable, xDS node ID: %s", resourceName, |
There was a problem hiding this comment.
nit: what would be the reason to switch the order of the error format here? I think Listener %s is slightly more common in the code base. Consistent formatting helps with searching when debugging issues.
Fixes #11737