policy: limit TCPRoute to one per policy response #13272
Conversation
Signed-off-by: Zahari Dichev <[email protected]>
| pub(crate) fn protocol( | ||
| default_backend: outbound::Backend, | ||
| routes: impl Iterator<Item = (GroupKindNamespaceName, TcpRoute)>, | ||
| parent_info: &ParentInfo, | ||
| original_dst: Option<SocketAddr>, | ||
| ) -> outbound::proxy_protocol::Kind { | ||
| let mut routes = routes | ||
| .take(MAXIMUM_ALLOWED_TCP_ROUTES) |
There was a problem hiding this comment.
Are the routes sorted in any particular way at this point when we take the first one? Is there a deterministic way to know which TCP route will take affect and which ones will be ignored?
If there are multiple TCP routes attached, should we be setting a RouteConflict and/or Accepted=false status on them?
There was a problem hiding this comment.
I'd have to consult more of the impl to tell you how/where this is happening, but the routes must be sorted according to preference (i.e. according to the route sorting rules dictated by the gateway API--naming, creation timestamp, etc). This change, as it is, simply encodes what the proxy would already do: use the first matching route. The proxy has no concept of conflicts or accepted=false, so all of that ordering must be handled in the policy controller.
Given that this function is building the actual protobuf types, I believe that must have already happened.
There was a problem hiding this comment.
The sorting happens before this is called:
linkerd2/policy-controller/grpc/src/outbound.rs
Lines 436 to 437 in 8fb22d6
There was a problem hiding this comment.
As pointed out sorting happens before this is called. In general the GW api spec does not prescribe setting conflict statuses in these cases. These are prescribed when there are routes with higher specificity (ie GRPC > HTTP). In cases when there are more than one route we need to follow disambiguation rules. In cases like HTTPRoute that would be more specific hostname, header and method matches. In the case of TCP these things are not present so we revert to sorting on route age and naming. For reference:
Proxy or Load Balancer routing configuration generated from HTTPRoutes MUST prioritize matches based on the following criteria, continuing on ties. Across all rules specified on applicable Routes, precedence must be given to the match having:
- “Exact” path match.
- “Prefix” path match with largest number of characters.
- Method match.
- Largest number of header matches.
- Largest number of query param matches.
Note: The precedence of RegularExpression path matches are implementation-specific.
If ties still exist across multiple Routes, matching precedence MUST be determined in order of the following criteria, continuing on ties:
- The oldest Route based on creation timestamp.
- The Route appearing first in alphabetical order by “{namespace}/{name}”.
If ties still exist within an HTTPRoute, matching precedence MUST be granted to the FIRST matching rule (in list order) with a match meeting the above criteria.
When no rules matching a request have been successfully attached to the parent a request is coming from, a HTTP 404 status code MUST be returned.
Based on the feedback received on some proxy work and the fact that there is no real way to select over
TCPRouteobjects at the moment, this PR limits the amount ofTCPRoutesthat are returned per response to 1.Signed-off-by: Zahari Dichev [email protected]