Skip to content

Implement basic route matching for xDS clients - #6333

Merged
ikhoon merged 3 commits into
line:mainfrom
jrhee17:feat/xds-route-matching
Aug 4, 2025
Merged

Implement basic route matching for xDS clients#6333
ikhoon merged 3 commits into
line:mainfrom
jrhee17:feat/xds-route-matching

Conversation

@jrhee17

@jrhee17 jrhee17 commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Motivation:

Following #6322, this changeset attempts to implement route matching (Route#match).
ref: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-route

Implementation is focused on following upstream behavior as closely as possible.
The corresponding implementation can be found here: https://github.com/envoyproxy/envoy/blob/4bb05db55ecd583b6f451d81445d94210445176f/source/common/router/config_impl.cc#L831-L881

Matching rules that aren't implemented are:

  • tls_context: Since the client doesn't act as a proxy yet, there are no downstream tls contexts.
  • dynamic_metadata: While this may be useful, users don't have a way to supply a metadata directly for a request yet.
  • runtime_fraction: This will be implemented when rtds is implemented.
  • filter_state: may consider this when the set_filter_state filter is introduced.
    Overall, the above rules have been verified to not being used in basic istio cases.

Modifications:

Result:

  • The xDS integration can now route requests based on the path, headers, and query parameters.

@jrhee17 jrhee17 added this to the 1.33.0 milestone Jul 30, 2025
@codecov

codecov Bot commented Jul 30, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.83333% with 152 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.46%. Comparing base (8150425) to head (13fa4fe).
⚠️ Report is 131 commits behind head on main.

Files with missing lines Patch % Lines
...armeria/xds/client/endpoint/RouteEntryMatcher.java 18.13% 136 Missing and 13 partials ⚠️
...ecorp/armeria/xds/client/endpoint/RouteConfig.java 57.14% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##               main    #6333     +/-   ##
===========================================
  Coverage     74.46%   74.46%             
- Complexity    22234    22694    +460     
===========================================
  Files          1963     2031     +68     
  Lines         82437    84336   +1899     
  Branches      10764    11000    +236     
===========================================
+ Hits          61385    62803   +1418     
- Misses        15918    16279    +361     
- Partials       5134     5254    +120     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jrhee17
jrhee17 force-pushed the feat/xds-route-matching branch from a0e8543 to 875f64d Compare July 30, 2025 06:21
@jrhee17
jrhee17 force-pushed the feat/xds-route-matching branch from 14effb3 to 13fa4fe Compare July 31, 2025 02:05
@jrhee17
jrhee17 marked this pull request as ready for review July 31, 2025 02:24

@ikhoon ikhoon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! 👍 👍

Comment on lines +218 to +219
final String joined = COMMA_JOINER.join(allHeaders);
return stringMatcher.match(joined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, the order of header values is crucial to match multiple headers.

Comment thread xds/src/main/java/com/linecorp/armeria/xds/client/endpoint/RouteEntryMatcher.java Outdated
final StringMatcherImpl matcherImpl = new StringMatcherImpl(matcher);
predicate = ctx -> matcherImpl.match(ctx.path());
} else if (pathSpecifierCase == PathSpecifierCase.CONNECT_MATCHER) {
predicate = ctx -> ctx.method() == HttpMethod.CONNECT;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this will not match other Extended CONNECT requests (WebSocket and the like) as they are normalized in Envoy as HTTP/1.1 style upgrades.

I understand that the CONNECT_MATCHER implementation does not follow the Envoy's specification, as CONNECT is used for WebSocket in Armeria.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CONNECT is used for WebSocket in Armeria.

I don't really see a difference since for HTTP1 CONNECT isn't used either (and http2 websockets aren't supported in envoy).

Nuances on the request state may differ, but the condition is more or less the same.

Comment thread xds/src/main/java/com/linecorp/armeria/xds/client/endpoint/RouteEntryMatcher.java Outdated

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

@ikhoon
ikhoon merged commit d562343 into line:main Aug 4, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants