xds: Implementation of Unified Matcher#12640
Conversation
082243d to
f472c63
Compare
l46kok
left a comment
There was a problem hiding this comment.
Sorry about the drive-by -- I happened to notice this while putting together an unrelated PR in CEL-Java. I'll hold off on submitting it on our end.
Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 871541021
Also removes antlr dependency from `dev.cel:runtime` artifact Fixes: #566 See also grpc/grpc-java#12640 PiperOrigin-RevId: 872010923
asheshvidyut
left a comment
There was a problem hiding this comment.
Reviewing for learning purposes.
8515729 to
f312c9c
Compare
kannanjgithub
left a comment
There was a problem hiding this comment.
Reviewed source code, yet to review tests.
There was a problem hiding this comment.
Pull request overview
Implements gRFC A106 “xDS Unified Matcher” in the io.grpc.xds.internal.matcher package, adding a proto-driven matcher runtime (list/tree forms), pluggable input/matcher registries, and CEL-based matching support, along with extensive validation and behavior tests.
Changes:
- Added
UnifiedMatcherruntime withMatcherList/MatcherTreeexecution,OnMatchhandling, recursion-depth validation, andMatchResultaggregation semantics. - Introduced pluggable registries (
MatchInputRegistry,MatcherRegistry) and concrete implementations for header inputs and CEL-state matching. - Added comprehensive unit tests covering validation, keep-matching semantics, matcher-tree behaviors, and CEL matching/extraction.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| xds/src/test/java/io/grpc/xds/internal/matcher/UnifiedMatcherValidationTest.java | New validation tests for unified matcher parsing and input/matcher constraints. |
| xds/src/test/java/io/grpc/xds/internal/matcher/UnifiedMatcherTest.java | New behavioral tests for matcher list semantics, inputs, and runner behavior. |
| xds/src/test/java/io/grpc/xds/internal/matcher/MatcherTreeTest.java | New behavioral tests for matcher tree exact/prefix matching and keep-matching behavior. |
| xds/src/test/java/io/grpc/xds/internal/matcher/CelStateMatcherTest.java | New tests for CEL matcher integration, type mismatches, and safe failure modes. |
| xds/src/test/java/io/grpc/xds/internal/matcher/CelMatcherTestHelper.java | Test helper updates to compile CEL matchers/extractors for test cases. |
| xds/src/main/java/io/grpc/xds/internal/Matchers.java | Extended internal string matcher to support contains with ignoreCase. |
| xds/src/main/java/io/grpc/xds/internal/MatcherParser.java | Added parsing for com.github.xds.type.matcher.v3.StringMatcher and ignoreCase contains. |
| xds/src/main/java/io/grpc/xds/internal/matcher/UnifiedMatcher.java | New core unified matcher entry point and recursion-depth validation. |
| xds/src/main/java/io/grpc/xds/internal/matcher/PredicateEvaluator.java | New predicate evaluation implementation for single/or/and/not matchers. |
| xds/src/main/java/io/grpc/xds/internal/matcher/OnMatch.java | New “action vs nested matcher” handling and action type validation hook. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatchResult.java | New immutable match result carrying actions and terminal-match state. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatchInputRegistry.java | New registry for match input providers (header, CEL env). |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatchInputProvider.java | New provider interface for building MatchInput from typed configs. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatchInput.java | New abstraction for extracting typed values from a MatchContext. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatcherTree.java | New matcher-tree implementation with exact map and prefix trie matching. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatcherRunner.java | New helper to run matchers and return terminal actions. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatcherRegistry.java | New registry for custom matcher providers (e.g., CEL). |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatcherProvider.java | New provider interface for building Matcher implementations from typed configs. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatcherList.java | New matcher-list implementation with keep-matching and on-no-match behavior. |
| xds/src/main/java/io/grpc/xds/internal/matcher/Matcher.java | New matcher interface with optional declared input type. |
| xds/src/main/java/io/grpc/xds/internal/matcher/MatchContext.java | Extended match context to include request id and builder support. |
| xds/src/main/java/io/grpc/xds/internal/matcher/HttpAttributesCelMatchInput.java | New input that produces a CEL variable resolver from the match context. |
| xds/src/main/java/io/grpc/xds/internal/matcher/HeaderMatchInput.java | New header input implementation with binary/ASCII handling and validation. |
| xds/src/main/java/io/grpc/xds/internal/matcher/CelStateMatcher.java | New matcher provider for xDS CEL matcher extension with safe eval failure behavior. |
| xds/src/main/java/io/grpc/xds/internal/matcher/CelMatcher.java | New CEL matcher runtime wrapper for boolean expressions. |
Comments suppressed due to low confidence (1)
xds/src/test/java/io/grpc/xds/internal/matcher/CelMatcherTestHelper.java:67
CelMatcherTestHelper.compile()/compileStringExtractor()call production compile methods that declarethrows CelEvaluationException, but these helper methods don't declare or handle that checked exception. This will fail compilation ifCelEvaluationExceptionis checked (as it is in this codebase).
public static CelMatcher compile(String expression)
throws dev.cel.common.CelException {
CelAbstractSyntaxTree ast = COMPILER.compile(expression).getAst();
return CelMatcher.compile(ast);
}
public static CelStringExtractor compileStringExtractor(String expression)
throws dev.cel.common.CelException {
CelAbstractSyntaxTree ast = COMPILER.compile(expression).getAst();
return CelStringExtractor.compile(ast);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fine to merge with Kannan's review.
Implements gRFC A106: xDS Unified Matcher
grpc/proposal#520