lds: prefix ranges and source IP/port matching.#49
Conversation
* Allow multiple CIDR ranges to be provided for the prefix match. This
is useful when you have the same listener config in multiple subnets,
each subnet with the same suffix allocation of VIPs and the CIDR prefix
assigned by region/zone, e.g. when using the GCP subnetworks feature.
E.g. the service is on 0.0.0.37 in subnetworks 10.1.0.0 in US and
10.2.0.0 in Asia.
* Add source IP/port matching similar to existing TCP proxy filter. This
moves the route configuration from the TCP proxy filter to the
FilterChainMatch, making it also available to HTTP connections.
Fixes #6 and #36.
mattklein123
left a comment
There was a problem hiding this comment.
looks good, small comment nit
api/lds.proto
Outdated
| @@ -66,13 +66,27 @@ message FilterChainMatch { | |||
|
|
|||
| // If non-empty, an IP address and prefix length to match addresses when the | |||
| // listener is bound to 0.0.0.0/::. | |||
There was a problem hiding this comment.
I think we already fixed this in some other place, but this comment is misleading since this is still a useful feature even when binding to a specific address, but using USE_ORIGINAL_DST. I would clarify slightly. Same at line 73.
There was a problem hiding this comment.
Here's my current understanding. When using use_original_dst, the findListenerByAddress() method is used to determine the Listener based on the original destination address. In LDS, I assume we will follow a similar path. This means we get to the FilterChainMatch after either (1) a 0.0.0.0/:: bind or (2) an exact match on the bound address and original destination address. (1) is covered above, with (2) we wouldn't ever specify a prefix/suffix explicitly, since there's only 1 valid IP for the listener.
I might be missing something, so please let me know if that makes sense.
There was a problem hiding this comment.
Your are right, but I would actually argue that in v2 we lose findListenerByAddress() and do everything via the filter chain match infra. I think this is conceptually much simpler to understand since everything is now in 1 place.
There was a problem hiding this comment.
Are you suggesting that if the original destination is IP X but we receive it on a Listener bound to IP Y that we use the FilterChainMatch table for IP Y to determine the filter chain for IP X, rather than going back to the Listeners and trying to find a Listener matching IP X (which might also exist)?
There was a problem hiding this comment.
Yes, I'm proposing that as an option. Is that simpler to understand? Not sure. Happy to leave it this way for now if we want to open an issue to think about it.
There was a problem hiding this comment.
I think it's simpler to understand. My only question is what happens if we decide to pick filter chain A from IP Y's FilterChainMatch table and there is also a Listener for IP X with filter chain B? Do we expect the user to ensure A and B are consistent? Or do we expect that we won't have a listener for IP X when using use_original_dst this way?
There was a problem hiding this comment.
IMO the latter. Basically we make it clear in docs that we no longer going to jump listeners. Once you hit a listener, you stay there. IMO much simpler.
mattklein123
left a comment
There was a problem hiding this comment.
LGTM other than the discussion on how original_dst should work which can be covered in a different issue.
TCP proxy filter now has an idle timeout and the source match (and rest of route match as well) are now in the FilterChainMatch in envoyproxy#49. Fixes #23, #45.
…. (#51) TCP proxy filter now has an idle timeout and the source match (and rest of route match as well) are now in the FilterChainMatch in envoyproxy/data-plane-api#49. Fixes #23, #45.
Allow multiple CIDR ranges to be provided for the prefix match. This
is useful when you have the same listener config in multiple subnets,
each subnet with the same suffix allocation of VIPs and the CIDR prefix
assigned by region/zone, e.g. when using the GCP subnetworks feature.
E.g. the service is on 0.0.0.37 in subnetworks 10.1.0.0 in US and
10.2.0.0 in Asia.
Add source IP/port matching similar to existing TCP proxy filter. This
moves the route configuration from the TCP proxy filter to the
FilterChainMatch, making it also available to HTTP connections.
Fixes #6 and #36.