Conversation
Prepare for the gRFC A74 XdsDependencyManager, which must select the channel’s virtual host before resolving its cluster dependencies and publishing an atomic XdsConfig snapshot. Select virtual hosts using ASCII case-insensitive domain matching, wildcard precedence, and longest-pattern ordering. Reject malformed domain patterns during route configuration validation. Separate validated string and full-match regex types from runtime matching for reuse by routing and future SAN matching.
|
Hi @dfawley @arjan-bal this is the first PR of the |
|
Accidentally closed when I'm cleaning up my fork's branches, reopening |
arjan-bal
left a comment
arjan-bal
left a comment
There was a problem hiding this comment.
Took another pass. We use the assignee field to track whose turn it is to act (author or reviewer). Please assign this back to me when you're ready for another review.
| if domains_view.is_empty() { | ||
| return Err(Error::Validation(format!( | ||
| "virtual host '{name}' has no domains" | ||
| ))); | ||
| } |
There was a problem hiding this comment.
Go doesn't seem to fail validation for empty domains, and the proto field's docs don't mention any constraints on length. I think we should allow empty lists here unless there's a strong precedent to block them.
There was a problem hiding this comment.
I think this validation rule in proto enforces it? https://github.com/envoyproxy/envoy/blob/main/api/envoy/config/route/v3/route_components.proto#L92
C++ does error on it: https://github.com/grpc/grpc/blob/48a59c06db710c6a536636fc7ddc99719cffd08e/src/core/xds/grpc/xds_route_config_parser.cc#L745-L748
Seems like gRPC Go and Java doesn't. I'm ok with either, let me know what's your opinion.
|
Thanks @arjan-bal , I've addressed the above comments, ptal. Assigning back to you. |
2 participants
Motivation
Ref: #2754
For gRFC A74 XdsDependencyManager, to resolve the channel's virtual host, we'll need to run string matching per Envoy
StringMatcherspec.Solution
To reduce the size of the upcoming A74 PR, I'm sending this
StringMatcherPR first. The implementation mostly follows the design we used intonic-xds, with these highlights:StringMatchervalidated resource type and a purematchermodule.DomainMatchScoreandSafeRegex: the former encapsulate the match ordering semantics, and the latter to reject empty regex pattern and conform to the "only match the entire input" Envoy spec.