Add an example for xDS integration - #6886
Open
jrhee17 wants to merge 3 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a runnable three-layer Armeria xDS example with profile-based YAML resources, TLS and fault scenarios, a browser dashboard, Prometheus/Grafana monitoring, integration tests, and route-over-virtual-host retry policy resolution. ChangesxDS Example Application
xDS Retry Policy Precedence
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant DashboardService
participant ServiceMesh
participant Layer2Server
participant Layer3Server
Browser->>DashboardService: request traffic or profile update
DashboardService->>ServiceMesh: send request or apply profile
ServiceMesh->>Layer2Server: forward Layer 1 traffic
Layer2Server->>Layer3Server: forward Layer 2 traffic
Layer3Server-->>ServiceMesh: return HTTP response
ServiceMesh-->>DashboardService: return operation result
DashboardService-->>Browser: return API response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java (1)
68-73: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winShutdown order: dependencies closed before the server that uses them. Both
Layer2Server.close()andLayer3Server.close()close theirxdsBootstrap(and, for Layer2,preprocessor) before callingserver.stop().join(), even though the still-running server's route handler andXdsServerPlugindepend on those resources. In-flight requests during the stop/drain window can fail as a result.
examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java#L68-L73: callserver.stop().join()first, thenpreprocessor.close()andxdsBootstrap.close().examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java#L52-L56: callserver.stop().join()first, thenxdsBootstrap.close().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java` around lines 68 - 73, Update Layer2Server.close() in examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java:68-73 to call server.stop().join() before closing preprocessor and xdsBootstrap. Apply the same shutdown-order change in Layer3Server.close() at examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java:52-56, stopping the server before closing xdsBootstrap.xds/src/main/java/com/linecorp/armeria/xds/filter/FaultInjectionFilterFactory.java (1)
165-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate control flow in
httpDecorator/serviceDecorator.Both decorators repeat the same header-match → maybeFault → maybeDelay sequence, differing only in the delegate invocation.
♻️ Proposed refactor to share the fault-application logic
+ private HttpResponse applyFault(HttpHeaders headers, EventExecutor executor, + java.util.function.Supplier<HttpResponse> delegateCall) { + if (!headersMatch(headers)) { + return delegateCall.get(); + } + final HttpResponse faultResponse = maybeFault(); + if (faultResponse != null) { + return maybeDelay(faultResponse, executor); + } + return maybeDelay(delegateCall.get(), executor); + } + `@Override` public DecoratingHttpClientFunction httpDecorator() { - return (delegate, ctx, req) -> { - if (!headersMatch(req.headers())) { - return delegate.execute(ctx, req); - } - final HttpResponse faultResponse = maybeFault(); - if (faultResponse != null) { - return maybeDelay(faultResponse, ctx.eventLoop()); - } - return maybeDelay(delegate.execute(ctx, req), ctx.eventLoop()); - }; + return (delegate, ctx, req) -> + applyFault(req.headers(), ctx.eventLoop(), () -> delegate.execute(ctx, req)); } `@Override` public DecoratingHttpServiceFunction serviceDecorator() { - return (delegate, ctx, req) -> { - if (!headersMatch(req.headers())) { - return delegate.serve(ctx, req); - } - final HttpResponse faultResponse = maybeFault(); - if (faultResponse != null) { - return maybeDelay(faultResponse, ctx.eventLoop()); - } - return maybeDelay(delegate.serve(ctx, req), ctx.eventLoop()); - }; + return (delegate, ctx, req) -> + applyFault(req.headers(), ctx.eventLoop(), () -> delegate.serve(ctx, req)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xds/src/main/java/com/linecorp/armeria/xds/filter/FaultInjectionFilterFactory.java` around lines 165 - 191, Extract the shared header-match, maybeFault, and maybeDelay sequence from httpDecorator and serviceDecorator into a reusable helper in FaultInjectionFilterFactory, parameterizing only the delegate execution. Update both decorators to call this helper while preserving their respective delegate.execute and delegate.serve behavior.examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java (1)
99-107: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHeader building happens outside the try/catch.
headers.forEach(reqBuilder::add)(lines 99-103) runs before thetryblock starts at line 105. If any header name/value is rejected byRequestHeadersBuilder(invalid characters, reserved names, etc.), the exception escapes the per-iteration error handling that every other failure path in this method relies on (lines 120-123), aborting the whole request instead of reporting a per-iterationTrafficResulterror.♻️ Proposed fix
final RequestHeadersBuilder reqBuilder = RequestHeaders.builder(HttpMethod.GET, "/"); - if (headers != null) { - headers.forEach(reqBuilder::add); - } - try { + if (headers != null) { + headers.forEach(reqBuilder::add); + } final AggregatedHttpResponse response = layer1Client.execute(reqBuilder.build());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java` around lines 99 - 107, Move the RequestHeadersBuilder initialization and headers.forEach(reqBuilder::add) logic inside the existing try block in the per-iteration flow of DashboardService. Preserve the current catch handling so header-construction exceptions are converted into that iteration’s TrafficResult error instead of escaping the request loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/xds-example/docker-compose.yml`:
- Line 4: Update the startup command in the usage comment to reference the
registered Gradle module path :examples:xds-example:run instead of
:examples:xds:run.
In
`@examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java`:
- Around line 92-126: Update sendTraffic to enforce a finite upper bound on the
requested count before entering the synchronous request loop. Clamp or reject
values above the chosen maximum while preserving the existing default for
non-positive counts, and ensure the response count matches the bounded number of
requests performed.
- Around line 174-210: Harden updateFile by validating the resolved path remains
within configDir before writing, rejecting absolute or traversal-based file
values with BAD_REQUEST; use a normalized configDir and resolved path
containment check. Also validate the first resource’s type is non-null before
the switch, returning BAD_REQUEST instead of allowing a null-type NPE. Keep the
existing resource conversion and response behavior unchanged for valid requests.
In `@examples/xds-example/src/main/java/example/armeria/xds/Main.java`:
- Around line 19-53: Update Main.main() to ensure the ServiceMesh instance is
closed both during normal JVM shutdown and if dashboard.start().join() fails
after mesh creation. Register mesh with the shutdown lifecycle alongside the
dashboard Server, or use a try/finally structure that closes mesh while
preserving dashboard startup behavior.
In `@examples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.java`:
- Around line 57-180: Update the ServiceMesh constructor to guard the entire
initialization sequence, including server startup and clientFactory/XDS setup,
with failure cleanup. If any later step throws, best-effort close every
already-created Layer3Server and Layer2Server plus clientFactory and other
initialized resources before rethrowing the original failure; preserve normal
construction behavior and avoid masking the original exception with cleanup
failures.
- Around line 210-218: Update ServiceMesh.close() to close every resource even
when an individual close operation throws. Replace the fail-fast
layer2Servers/layer3Servers iteration with per-resource exception isolation, and
ensure later servers and meterRegistry.close() are still attempted; preserve
closure of the existing resources and the current close ordering.
- Around line 23-24: Replace the direct SelfSignedCertificate and
SignedCertificate usage in ServiceMesh with the public TlsKeyPair.ofSelfSigned()
API when populating certVars. Remove the internal TLS imports and preserve the
example’s existing certificate configuration behavior.
In
`@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java`:
- Around line 68-84: The writeDiscoveryResponse method must publish YAML
atomically instead of writing directly to path. Create a sibling temporary file
in the same directory, write the complete UTF-8 content to it, then move it over
path using an atomic replacement rename; clean up the temporary file if any step
fails while preserving the existing IOException wrapping.
- Around line 117-133: Replace the Path-based classpath enumeration in
scanValidProfiles() with an explicit/generated template index or a
resource-scanning approach that supports both directory and JAR classpath
entries. Preserve the existing profile-to-template mapping, sorting, and
immutable return behavior while ensuring packaged /xds/ resources can be
discovered without requiring a mounted filesystem.
In `@examples/xds-example/src/main/resources/index.html`:
- Around line 322-351: Apply the existing escHtml encoder to every dynamic value
interpolated into innerHTML: the counter key and value display in renderStats,
r.name and label in populateDropdown, and the corresponding selected-resource
fields in showSelectedResource. Keep numeric styling and behavior unchanged
while ensuring all server-derived text is escaped before assignment.
In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/FaultInjectionFilterTest.java`:
- Around line 19-41: Add the missing explicit import for XdsResourceReader in
FaultInjectionFilterTest so both XdsResourceReader.fromYaml(...) usages resolve
from the com.linecorp.armeria.xds package.
In `@xds/src/main/java/com/linecorp/armeria/xds/server/XdsServerPlugin.java`:
- Around line 176-177: Update the TLS consistency check in XdsServerPlugin to
use SessionProtocol.isTls() instead of comparing only against
SessionProtocol.HTTPS, so HTTPS, H1, and H2 over TLS are consistently recognized
as TLS by FilterChainMatcher.
---
Nitpick comments:
In
`@examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java`:
- Around line 99-107: Move the RequestHeadersBuilder initialization and
headers.forEach(reqBuilder::add) logic inside the existing try block in the
per-iteration flow of DashboardService. Preserve the current catch handling so
header-construction exceptions are converted into that iteration’s TrafficResult
error instead of escaping the request loop.
In `@examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java`:
- Around line 68-73: Update Layer2Server.close() in
examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java:68-73
to call server.stop().join() before closing preprocessor and xdsBootstrap. Apply
the same shutdown-order change in Layer3Server.close() at
examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java:52-56,
stopping the server before closing xdsBootstrap.
In
`@xds/src/main/java/com/linecorp/armeria/xds/filter/FaultInjectionFilterFactory.java`:
- Around line 165-191: Extract the shared header-match, maybeFault, and
maybeDelay sequence from httpDecorator and serviceDecorator into a reusable
helper in FaultInjectionFilterFactory, parameterizing only the delegate
execution. Update both decorators to call this helper while preserving their
respective delegate.execute and delegate.serve behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7bafc09c-86cd-424d-9eb4-b06f4ae81cf9
📒 Files selected for processing (55)
examples/xds-example/build.gradleexamples/xds-example/docker-compose.ymlexamples/xds-example/grafana/dashboards/xds-traffic.jsonexamples/xds-example/grafana/provisioning/dashboards/provider.ymlexamples/xds-example/grafana/provisioning/datasources/datasource.ymlexamples/xds-example/prometheus.ymlexamples/xds-example/src/main/java/example/armeria/xds/DashboardService.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer2Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer3Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Main.javaexamples/xds-example/src/main/java/example/armeria/xds/ProfileState.javaexamples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.javaexamples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.javaexamples/xds-example/src/main/java/example/armeria/xds/TrafficCounter.javaexamples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.javaexamples/xds-example/src/main/resources/index.htmlexamples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-eds.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-eds.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault-retry/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault-retry/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/layer1-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer2-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer2-eds.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer3-eds.template.yamlexamples/xds-example/src/test/java/example/armeria/xds/ServiceMeshTest.javait/xds-client/src/test/java/com/linecorp/armeria/xds/RouteEntryMatcherTest.javait/xds-client/src/test/java/com/linecorp/armeria/xds/it/FaultInjectionFilterTest.javait/xds-client/src/test/java/com/linecorp/armeria/xds/it/server/ServerFilterChainMatchTest.javasettings.gradlexds-api/src/main/proto/envoy/config/core/v3/base.protoxds-api/src/main/proto/envoy/extensions/filters/common/fault/v3/fault.protoxds-api/src/main/proto/envoy/extensions/filters/http/fault/v3/fault.protoxds-api/src/main/proto/envoy/type/v3/percent.protoxds/src/main/java/com/linecorp/armeria/xds/RetryStateFactory.javaxds/src/main/java/com/linecorp/armeria/xds/RouteEntryMatcher.javaxds/src/main/java/com/linecorp/armeria/xds/StateCoordinator.javaxds/src/main/java/com/linecorp/armeria/xds/XdsExtensionRegistry.javaxds/src/main/java/com/linecorp/armeria/xds/XdsResourceReader.javaxds/src/main/java/com/linecorp/armeria/xds/client/endpoint/LocalityRoutingStateFactory.javaxds/src/main/java/com/linecorp/armeria/xds/filter/FaultInjectionFilterFactory.javaxds/src/main/java/com/linecorp/armeria/xds/internal/XdsHeaderMatcher.javaxds/src/main/java/com/linecorp/armeria/xds/server/XdsServerPlugin.java
Contributor
jrhee17
force-pushed
the
feat/xds-examples
branch
from
August 4, 2026 07:50
36c0a4c to
d9b75e0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/xds-example/src/main/java/example/armeria/xds/ProfileState.java`:
- Line 1: Add the standard LY Corporation Apache 2.0 copyright header before the
package declaration in ProfileState.java, TrafficCounter.java, and
ServiceMesh.java; make no other changes.
- Around line 28-34: Validate each profile override in ProfileState.mergeFrom()
or its request acceptor against XdsTemplateReader.VALID_PROFILES for the
corresponding resource before storing it, while preserving valid overrides.
Update the /apply-profile validation to allow the OVERRIDE profile value, and
reject invalid values before applyProfile() invokes profileFor(resource).
In `@examples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.java`:
- Around line 65-66: Update ServiceMesh.close() to delete the temporary
directory stored in configDir after all server and resource cleanup completes.
Remove the directory tree recursively, including generated files, while
preserving existing close behavior and handling the cleanup through the method’s
established exception strategy.
In `@examples/xds-example/src/main/resources/index.html`:
- Around line 505-528: Update the apply-profile request in the update flow after
`/api/update-file` succeeds to validate its response status and report a partial
failure when it is non-2xx, rather than calling `loadProfiles()` as if both
mutations succeeded. If feasible, consolidate the `/api/update-file` and
`/api/apply-profile` operations behind one server endpoint that performs them
atomically.
- Around line 363-369: Update loadConfig and the corresponding reload logic
around the referenced resource list so the current resource’s stable identity
(file, type, and name) is captured before replacing allResources, then restore
the matching resource after repopulating instead of reusing its old index; if no
match exists, clear the selection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6813af8a-4614-4a58-8594-d0bc908d0dae
📒 Files selected for processing (39)
examples/xds-example/build.gradleexamples/xds-example/docker-compose.ymlexamples/xds-example/grafana/dashboards/xds-traffic.jsonexamples/xds-example/grafana/provisioning/dashboards/provider.ymlexamples/xds-example/grafana/provisioning/datasources/datasource.ymlexamples/xds-example/prometheus.ymlexamples/xds-example/src/main/java/example/armeria/xds/DashboardService.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer2Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer3Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Main.javaexamples/xds-example/src/main/java/example/armeria/xds/ProfileState.javaexamples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.javaexamples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.javaexamples/xds-example/src/main/java/example/armeria/xds/TrafficCounter.javaexamples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.javaexamples/xds-example/src/main/resources/index.htmlexamples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-eds.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-eds.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault-retry/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault-retry/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/layer1-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer2-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer2-eds.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer3-eds.template.yamlexamples/xds-example/src/test/java/example/armeria/xds/ServiceMeshTest.javasettings.gradle
🚧 Files skipped from review as they are similar to previous changes (34)
- settings.gradle
- examples/xds-example/prometheus.yml
- examples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/unhealthy/layer2-eds.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/layer2-bootstrap.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java
- examples/xds-example/build.gradle
- examples/xds-example/docker-compose.yml
- examples/xds-example/grafana/provisioning/dashboards/provider.yml
- examples/xds-example/grafana/provisioning/datasources/datasource.yml
- examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java
- examples/xds-example/src/test/java/example/armeria/xds/ServiceMeshTest.java
- examples/xds-example/src/main/java/example/armeria/xds/Main.java
- examples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.java
- examples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/layer1-bootstrap.template.yaml
- examples/xds-example/src/main/resources/xds/unhealthy/layer3-eds.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer3-eds.template.yaml
- examples/xds-example/src/main/resources/xds/fault/layer1-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-eds.template.yaml
- examples/xds-example/src/main/resources/xds/fault-retry/layer2-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/fault/layer2-client-listener.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java
- examples/xds-example/src/main/resources/xds/fault-retry/layer1-client-listener.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java (2)
127-129: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClose both
DirectoryStreamloops.
Files.newDirectoryStreamopens aDirectoryStream, and enhancedforloops do not close it. Wrap the outer and inner streams in try-with-resources inscanValidProfiles().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java` around lines 127 - 129, The nested for loops in scanValidProfiles() use Files.newDirectoryStream() which opens DirectoryStream resources that are not closed by enhanced for loops, causing resource leaks. Wrap both the outer Files.newDirectoryStream(base, Files::isDirectory) and inner Files.newDirectoryStream(dir, "*.template.yaml") calls in separate try-with-resources statements to ensure each DirectoryStream is properly closed, while preserving the existing nested loop structure and logic.
71-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the canonical xDS type URL when packing resources.
DiscoveryResponse.type_urlusestype.googleapis.com/..., butAny.pack(resource, "")sets the packed resource type URL to the resource full qualified name without the canonical prefix. Envoy requires the packed resource URL to match the response type URL. UseAny.pack(resource)or pass"type.googleapis.com"explicitly.Proposed fix
- builder.addResources(Any.pack(resource, "")); + builder.addResources(Any.pack(resource));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java` around lines 71 - 76, The Any.pack call in the resources loop is using an empty string as the type URL prefix instead of the canonical xDS prefix that the DiscoveryResponse typeUrl field expects. Update the builder.addResources(Any.pack(resource, "")) line to either omit the second parameter and call Any.pack(resource) to use the default canonical prefix, or explicitly pass "type.googleapis.com" as the prefix parameter. This ensures the packed resource URL matches the canonical format in the response type URL.
♻️ Duplicate comments (2)
examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java (2)
82-87: 🩺 Stability & Availability | 🟠 MajorDelete the temporary file after a failed publication.
If
Files.writeStringorFiles.movefails,tmpremains in the configuration directory. Repeated profile update failures can accumulate.xds-*.tmpfiles and exhaust disk space. Delete the temporary file in the failure path while preserving the original exception.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java` around lines 82 - 87, Update the temporary-file publication flow in XdsTemplateReader to delete tmp when Files.writeString or Files.move fails, while preserving and rethrowing the original exception. Use a failure-safe cleanup mechanism around the existing write-and-move operations so cleanup errors do not replace the publication failure.Source: Linters/SAST tools
123-139: 🩺 Stability & Availability | 🟠 MajorSupport profile discovery from packaged JARs.
If
/xds/resolves to ajar:URI,Path.of(...)requires an existing filesystem for that URI. The static initializer can then fail before the dashboard starts. Use an explicit template index or a scanner that supports both directory and JAR classpath entries. The JDK documentsFileSystemNotFoundExceptionwhen the filesystem identified by a URI is unavailable. (docs.oracle.com)This remains the issue reported in the previous review.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java` around lines 123 - 139, Update scanValidProfiles() to discover templates when the /xds/ classpath resource is packaged in a JAR, not only when it resolves to a directory Path. Use an explicit template index or classpath scanner that handles both file and jar URLs, while preserving the existing profile/name mapping and sorting behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java`:
- Around line 69-74: Update Layer2Server.closeAsync() to replace the synchronous
preprocessor.close() and xdsBootstrap.close() calls with their asynchronous
close APIs. Combine both returned futures with server.stop() and return the
combined shutdown future without blocking.
---
Outside diff comments:
In
`@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java`:
- Around line 127-129: The nested for loops in scanValidProfiles() use
Files.newDirectoryStream() which opens DirectoryStream resources that are not
closed by enhanced for loops, causing resource leaks. Wrap both the outer
Files.newDirectoryStream(base, Files::isDirectory) and inner
Files.newDirectoryStream(dir, "*.template.yaml") calls in separate
try-with-resources statements to ensure each DirectoryStream is properly closed,
while preserving the existing nested loop structure and logic.
- Around line 71-76: The Any.pack call in the resources loop is using an empty
string as the type URL prefix instead of the canonical xDS prefix that the
DiscoveryResponse typeUrl field expects. Update the
builder.addResources(Any.pack(resource, "")) line to either omit the second
parameter and call Any.pack(resource) to use the default canonical prefix, or
explicitly pass "type.googleapis.com" as the prefix parameter. This ensures the
packed resource URL matches the canonical format in the response type URL.
---
Duplicate comments:
In
`@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java`:
- Around line 82-87: Update the temporary-file publication flow in
XdsTemplateReader to delete tmp when Files.writeString or Files.move fails,
while preserving and rethrowing the original exception. Use a failure-safe
cleanup mechanism around the existing write-and-move operations so cleanup
errors do not replace the publication failure.
- Around line 123-139: Update scanValidProfiles() to discover templates when the
/xds/ classpath resource is packaged in a JAR, not only when it resolves to a
directory Path. Use an explicit template index or classpath scanner that handles
both file and jar URLs, while preserving the existing profile/name mapping and
sorting behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d1a67a7-4d01-486f-9848-a5dc7eec3e77
📒 Files selected for processing (4)
examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer3Server.javaexamples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.javaexamples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.java
- examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java
jrhee17
force-pushed
the
feat/xds-examples
branch
from
August 4, 2026 09:30
409403f to
bd4c7ff
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
examples/xds-example/src/main/resources/index.html (1)
91-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPopulate the profile options from the server.
The
profileSelectoptions are hardcoded tobasic,tls,fault, andfault-retry. The server derives profiles from thexdstemplate directories, which also includeunhealthy. The list drifts whenever a template directory is added or removed. Build the options from the/api/profilesresponse inloadProfiles, and keepprofileDescriptionsas a lookup for known profiles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/resources/index.html` around lines 91 - 96, Update loadProfiles to fetch profiles from /api/profiles and dynamically populate profileSelect instead of relying on hardcoded option elements. Preserve profileDescriptions as the lookup for known profile descriptions, while allowing server-provided profiles such as unhealthy and reflecting template additions or removals.examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java (2)
53-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the template substitution loop.
readandreadBootstraprepeat the same${key}replacement loop. Extract one private helper and call it from both methods.♻️ Proposed refactor
static <T extends GeneratedMessageV3> T read( ProfileState state, String name, Map<String, String> vars, Class<T> type) { - String yaml = readTemplate(state.profileFor(name) + '/' + name); - for (var entry : vars.entrySet()) { - yaml = yaml.replace("${" + entry.getKey() + '}', entry.getValue()); - } - return XdsResourceReader.from(yaml, type); + final String yaml = substitute(readTemplate(state.profileFor(name) + '/' + name), vars); + return XdsResourceReader.from(yaml, type); } static Bootstrap readBootstrap(String name, Map<String, String> vars) { - String yaml = readTemplate(name); - for (var entry : vars.entrySet()) { - yaml = yaml.replace("${" + entry.getKey() + '}', entry.getValue()); - } - return XdsResourceReader.from(yaml, Bootstrap.class); + return XdsResourceReader.from(substitute(readTemplate(name), vars), Bootstrap.class); } + + private static String substitute(String template, Map<String, String> vars) { + String yaml = template; + for (Map.Entry<String, String> entry : vars.entrySet()) { + yaml = yaml.replace("${" + entry.getKey() + '}', entry.getValue()); + } + return yaml; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java` around lines 53 - 68, Extract the duplicated variable-substitution loop from read and readBootstrap into a private helper that accepts the YAML template and vars map, performs each ${key} replacement, and returns the updated YAML. Replace both inline loops with calls to this helper while preserving the existing parsing behavior and method signatures.
103-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNarrow the catch to
IOException.
JsonFormat.Printer.printthrowsInvalidProtocolBufferExceptionand the Jackson calls throwJsonProcessingException. Both areIOExceptionsubtypes. CatchIOExceptionand throwUncheckedIOException, which matches the other methods in this class and avoids swallowing unrelated runtime exceptions.♻️ Proposed refactor
static String printProtoYaml(GeneratedMessageV3 msg) { try { final String json = PROTO_PRINTER.print(msg); final JsonNode tree = JSON_MAPPER.readTree(json); return YAML_MAPPER.writeValueAsString(tree); - } catch (Exception e) { - throw new RuntimeException(e); + } catch (IOException e) { + throw new UncheckedIOException("Failed to print: " + msg.getDescriptorForType().getFullName(), e); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java` around lines 103 - 111, Update printProtoYaml to catch only IOException, covering the checked failures from PROTO_PRINTER.print and the Jackson read/write calls, and rethrow it as UncheckedIOException. Leave unrelated runtime exceptions uncaught and match the exception-handling pattern used by the other methods in XdsTemplateReader.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java`:
- Around line 35-36: Annotate DashboardService with `@UnstableApi` so its public
API methods inherit the required stability marker, and add Javadoc to every
public endpoint method in the class, including those at the referenced
locations. Preserve the existing endpoint behavior and ensure the annotation
import is added if needed.
- Around line 205-209: Update applyProfile in DashboardService to synchronize
the profileState merge and XdsTemplateReader.applyProfile call under a single
lock. Ensure concurrent requests cannot read-modify-write profileState or apply
profile files simultaneously, while preserving the existing response behavior.
In `@examples/xds-example/src/main/java/example/armeria/xds/Main.java`:
- Around line 19-25: Annotate the public Main class with `@UnstableApi` and add
Javadoc documenting the public main(String[] args) entry point. Ensure the
required annotation import is present and retain the existing startup behavior.
In
`@examples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.java`:
- Line 14: Annotate the newly added public API in RouteMetricsClient with
`@UnstableApi`, applying it to the class or execute() method, and add Javadoc
documenting execute(). Ensure every public method in the affected class has the
required documentation.
In
`@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java`:
- Around line 83-91: Update the temp-file handling in XdsTemplateReader so the
Path created by Files.createTempFile is available for cleanup, and ensure it is
deleted when writing or atomic publishing fails. Keep successful moves intact,
and perform cleanup without masking the original IOException.
- Around line 120-127: The outer and inner Files.newDirectoryStream calls in the
nested loops are not being closed, causing OS handle leaks. Wrap the outer for
loop that iterates over Files.newDirectoryStream(base, Files::isDirectory) with
a try-with-resources statement that declares the returned DirectoryStream, and
similarly wrap the inner for loop that iterates over
Files.newDirectoryStream(dir, "*.template.yaml") with its own try-with-resources
statement. Ensure both try-with-resources blocks are properly nested to close
both streams in the correct order.
In `@examples/xds-example/src/main/resources/index.html`:
- Around line 533-535: Chain the initial load calls so loadConfig executes only
after loadProfiles has completed and populated validProfiles, then run
refreshStats after the dependent loads finish. Update the startup sequence
around loadProfiles, loadConfig, and refreshStats while preserving their
existing behavior.
---
Nitpick comments:
In
`@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java`:
- Around line 53-68: Extract the duplicated variable-substitution loop from read
and readBootstrap into a private helper that accepts the YAML template and vars
map, performs each ${key} replacement, and returns the updated YAML. Replace
both inline loops with calls to this helper while preserving the existing
parsing behavior and method signatures.
- Around line 103-111: Update printProtoYaml to catch only IOException, covering
the checked failures from PROTO_PRINTER.print and the Jackson read/write calls,
and rethrow it as UncheckedIOException. Leave unrelated runtime exceptions
uncaught and match the exception-handling pattern used by the other methods in
XdsTemplateReader.
In `@examples/xds-example/src/main/resources/index.html`:
- Around line 91-96: Update loadProfiles to fetch profiles from /api/profiles
and dynamically populate profileSelect instead of relying on hardcoded option
elements. Preserve profileDescriptions as the lookup for known profile
descriptions, while allowing server-provided profiles such as unhealthy and
reflecting template additions or removals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 72dd5669-8344-4af8-8d17-72ae4f2b430e
📒 Files selected for processing (39)
examples/xds-example/build.gradleexamples/xds-example/docker-compose.ymlexamples/xds-example/grafana/dashboards/xds-traffic.jsonexamples/xds-example/grafana/provisioning/dashboards/provider.ymlexamples/xds-example/grafana/provisioning/datasources/datasource.ymlexamples/xds-example/prometheus.ymlexamples/xds-example/src/main/java/example/armeria/xds/DashboardService.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer2Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer3Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Main.javaexamples/xds-example/src/main/java/example/armeria/xds/ProfileState.javaexamples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.javaexamples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.javaexamples/xds-example/src/main/java/example/armeria/xds/TrafficCounter.javaexamples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.javaexamples/xds-example/src/main/resources/index.htmlexamples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-eds.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-eds.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault-retry/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault-retry/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/layer1-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer2-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer2-eds.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer3-eds.template.yamlexamples/xds-example/src/test/java/example/armeria/xds/ServiceMeshTest.javasettings.gradle
🚧 Files skipped from review as they are similar to previous changes (33)
- settings.gradle
- examples/xds-example/src/main/resources/xds/layer1-bootstrap.template.yaml
- examples/xds-example/build.gradle
- examples/xds-example/grafana/provisioning/dashboards/provider.yml
- examples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/fault-retry/layer2-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/fault-retry/layer1-client-listener.template.yaml
- examples/xds-example/grafana/provisioning/datasources/datasource.yml
- examples/xds-example/src/main/resources/xds/fault/layer1-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-eds.template.yaml
- examples/xds-example/docker-compose.yml
- examples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/TrafficCounter.java
- examples/xds-example/prometheus.yml
- examples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yaml
- examples/xds-example/src/test/java/example/armeria/xds/ServiceMeshTest.java
- examples/xds-example/src/main/resources/xds/unhealthy/layer2-eds.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer3-eds.template.yaml
- examples/xds-example/grafana/dashboards/xds-traffic.json
- examples/xds-example/src/main/resources/xds/layer2-bootstrap.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/unhealthy/layer3-eds.template.yaml
- examples/xds-example/src/main/resources/xds/fault/layer2-client-listener.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java
- examples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.java
- examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java
jrhee17
force-pushed
the
feat/xds-examples
branch
from
August 5, 2026 09:29
bd4c7ff to
d97fc61
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
xds/src/main/java/com/linecorp/armeria/xds/RouteStream.java (1)
178-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse protobuf field-presence checks for retry-policy precedence.
getRetryPolicy()returns the default message when the field is absent, so comparing withRetryPolicy.getDefaultInstance()treats non-absent configured policies as absent and can skip route-level precedence. UsehasRetryPolicy()for the route action and virtual host before building the retry decoration.Proposed change
- if (routeRetryPolicy != RetryPolicy.getDefaultInstance()) { + if (route.getRoute().hasRetryPolicy()) { effectiveRetryPolicy = routeRetryPolicy; - } else if (vhostRetryPolicy != RetryPolicy.getDefaultInstance()) { + } else if (vhostResource.resource().hasRetryPolicy()) { effectiveRetryPolicy = vhostRetryPolicy;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xds/src/main/java/com/linecorp/armeria/xds/RouteStream.java` around lines 178 - 188, Update the retry-policy selection in RouteStream to use protobuf field-presence checks: prefer the route action’s policy when route.getRoute().hasRetryPolicy() is true, otherwise use the virtual host policy when vhostResource.resource().hasRetryPolicy() is true, and retain null when neither field is present before building the retry decoration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/xds-example/grafana/dashboards/xds-traffic.json`:
- Around line 258-260: Remove the trailing comma from the single-element
transformations array in the xds-traffic dashboard JSON so the organize
transformation entry is the last item without a dangling comma; keep the
existing transformations structure and only update the JSON syntax around the
transformations field.
In
`@examples/xds-example/src/main/java/example/armeria/xds/DashboardService.java`:
- Line 1: Add the repository-standard copyright header before the package
declaration in DashboardService.java and ServiceMeshTest.java at the specified
sites; leave the existing package declarations and file contents unchanged.
- Around line 167-171: Update the type dispatch in DashboardService’s preview
handling to treat unsupported type values as a BAD_REQUEST response directly,
matching the existing missing-field behavior. Replace the
IllegalArgumentException path in the switch default with the established
response flow while preserving the listener, cluster, and endpoint handling.
In
`@examples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.java`:
- Line 1: Insert the standard project copyright header at the top of
RouteMetricsClient before the package declaration, using the same header format
used by other files in the example.armeria.xds package.
In
`@examples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.java`:
- Line 1: Add the project-standard copyright header at the beginning of
XdsTemplateReader.java, before the package declaration, matching the header
format used by other Java source files in the project.
In
`@examples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yaml`:
- Around line 18-20: Add match_typed_subject_alt_names under validation_context
to require the Layer 2 service identity in the upstream peer certificate, while
retaining trusted_ca validation. Update the corresponding Layer 2 certificate
issuance configuration to include the same identity so Envoy’s SAN check
succeeds.
---
Nitpick comments:
In `@xds/src/main/java/com/linecorp/armeria/xds/RouteStream.java`:
- Around line 178-188: Update the retry-policy selection in RouteStream to use
protobuf field-presence checks: prefer the route action’s policy when
route.getRoute().hasRetryPolicy() is true, otherwise use the virtual host policy
when vhostResource.resource().hasRetryPolicy() is true, and retain null when
neither field is present before building the retry decoration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ff9ddd4-d6eb-4a14-89c1-7eda4eec4b47
📒 Files selected for processing (41)
examples/xds-example/build.gradleexamples/xds-example/docker-compose.ymlexamples/xds-example/grafana/dashboards/xds-traffic.jsonexamples/xds-example/grafana/provisioning/dashboards/provider.ymlexamples/xds-example/grafana/provisioning/datasources/datasource.ymlexamples/xds-example/prometheus.ymlexamples/xds-example/src/main/java/example/armeria/xds/DashboardService.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer2Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Layer3Server.javaexamples/xds-example/src/main/java/example/armeria/xds/Main.javaexamples/xds-example/src/main/java/example/armeria/xds/ProfileState.javaexamples/xds-example/src/main/java/example/armeria/xds/RouteMetricsClient.javaexamples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.javaexamples/xds-example/src/main/java/example/armeria/xds/XdsTemplateReader.javaexamples/xds-example/src/main/resources/index.htmlexamples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer1-endpoints.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-endpoints.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/fault/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/layer1-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer2-client-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer2-server-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yamlexamples/xds-example/src/main/resources/xds/retry/layer1-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/retry/layer2-client-listener.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-client-cluster.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer1-endpoints.template.yamlexamples/xds-example/src/main/resources/xds/unhealthy/layer2-endpoints.template.yamlexamples/xds-example/src/test/java/example/armeria/xds/ServiceMeshTest.javait/xds-client/src/test/java/com/linecorp/armeria/xds/it/RetryTest.javasettings.gradlexds/src/main/java/com/linecorp/armeria/xds/RouteStream.java
🚧 Files skipped from review as they are similar to previous changes (20)
- settings.gradle
- examples/xds-example/src/main/resources/xds/tls/layer1-client-cluster.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer1-client-cluster.template.yaml
- examples/xds-example/docker-compose.yml
- examples/xds-example/build.gradle
- examples/xds-example/grafana/provisioning/dashboards/provider.yml
- examples/xds-example/src/main/resources/xds/basic/layer1-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/layer3-bootstrap.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-client-listener.template.yaml
- examples/xds-example/src/main/resources/xds/tls/layer3-server-listener.template.yaml
- examples/xds-example/grafana/provisioning/datasources/datasource.yml
- examples/xds-example/src/main/resources/xds/tls/layer2-server-listener.template.yaml
- examples/xds-example/src/main/resources/xds/basic/layer2-server-listener.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/ProfileState.java
- examples/xds-example/src/main/java/example/armeria/xds/Main.java
- examples/xds-example/prometheus.yml
- examples/xds-example/src/main/java/example/armeria/xds/ServiceMesh.java
- examples/xds-example/src/main/resources/xds/basic/layer3-server-listener.template.yaml
- examples/xds-example/src/main/java/example/armeria/xds/Layer2Server.java
- examples/xds-example/src/main/java/example/armeria/xds/Layer3Server.java
jrhee17
force-pushed
the
feat/xds-examples
branch
4 times, most recently
from
August 5, 2026 10:16
d7f0f7c to
4d08628
Compare
jrhee17
changed the title
jrhee17
force-pushed
the
feat/xds-examples
branch
from
August 5, 2026 12:52
4d08628 to
4526719
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6886 +/- ##
============================================
+ Coverage 74.46% 75.09% +0.63%
- Complexity 22234 25567 +3333
============================================
Files 1963 2275 +312
Lines 82437 94978 +12541
Branches 10764 12417 +1653
============================================
+ Hits 61385 71323 +9938
- Misses 15918 17771 +1853
- Partials 5134 5884 +750 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jrhee17
marked this pull request as ready for review
August 6, 2026 01:18
|
Tick the box to add this pull request to the merge queue (same as
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Armeria's xDS client only checked the per-route
retry_policyand ignored the virtual host levelretry_policy. In Envoy, a virtual host level retry policy acts as a default that applies to all routes unless a route explicitly overrides it. Users who configured retry at the virtual host level (a common pattern) got no retry behavior.Additionally, Armeria's xDS support has grown significantly (file-based config, zone-aware routing, subset routing, fault injection, mTLS, server-side xDS) but there was no end-to-end example demonstrating these features together.
Modifications:
RouteStreamto fall back to the virtual host levelretry_policywhen no route-level policy is set. Route-level takes precedence over virtual host level, matching Envoy's behavior.RetryTest:virtualHostLevelRetryPolicy: verifies that a vhost-levelretry_on: "5xx"withnum_retries: 2triggers retries on 503 responses.routeLevelRetryPolicyOverridesVirtualHost: verifies that a route-levelretry_on: "gateway-error"overrides a vhost-levelretry_on: "5xx", so a 500 response does not trigger retries.examples/xds-examplemodule — a three-layer service mesh demo with:us-east,us-west,eu-west)basic,tls,fault,retry)Result:
retry_policyis now correctly used as a fallback when no route-level policy is configured, matching Envoy's behavior../gradlew :examples:xds-example:runanddocker compose upto get a fully working xDS service mesh with a visual dashboard and Grafana metrics, making it easy to explore Armeria's xDS capabilities interactively.