feat(gateway): add WebSocket and gRPC protocol processors#88
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @OliverWolffGIP, your pull request is larger than the review limit of 150000 diff characters
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe change adds WebSocket and gRPC protocol support across configuration, routing, edge dispatch, relaying, rejection handling, integration infrastructure, benchmarks, tests, and documentation. ChangesProtocol support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements native support for WebSocket and gRPC protocols in the API Sheriff gateway, introducing dedicated protocol processors, a fail-closed WebSocket Origin allowlist, idle timeout management, and trailers-only gRPC rejection mapping. The code review highlighted several critical compilation errors caused by calling non-existent Vert.x 4 methods (ended() and isClosed()). Additionally, feedback was provided regarding a potential hanging connection on upstream pipe failure, a WebSocket protocol violation where pong frames are incorrectly converted to binary frames, and a backpressure issue where the drainHandler is not cleared after execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.java (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnsure the standard methods set is immutable.
Returning
GRPC_METHODSdirectly fromstandardMethods()exposes a mutableEnumSetto callers, allowing unintended modification of the processor's supported methods state. UseSet.of()to guarantee immutability.♻️ Proposed fix
- private static final Set<HttpMethod> GRPC_METHODS = EnumSet.of(HttpMethod.POST); + private static final Set<HttpMethod> GRPC_METHODS = Set.of(HttpMethod.POST);🤖 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 `@api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.java` at line 37, Make the GRPC_METHODS set used by standardMethods() immutable by replacing the mutable EnumSet initialization with Set.of(HttpMethod.POST). Preserve the existing supported-method contents and standardMethods() behavior.api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.java (1)
171-171: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSkip evaluating WebSocket origins for non-WebSocket routes.
If a user erroneously includes a
websocketblock on a non-WebSocket route,effectiveAllowedOriginswill still parse and carry that state. Consider gating this toProtocol.WEBSOCKETjust likeidleTimeoutto ensure clean runtime objects.♻️ Proposed fix
- Set<String> allowedOrigins = effectiveAllowedOrigins(route); + Set<String> allowedOrigins = protocol == Protocol.WEBSOCKET ? effectiveAllowedOrigins(route) : Set.of();🤖 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 `@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.java` at line 171, Update the route-building logic around RouteTableBuilder’s effectiveAllowedOrigins call to evaluate WebSocket origins only when the route protocol is Protocol.WEBSOCKET. For non-WebSocket routes, skip parsing and carrying websocket origin configuration, matching the existing idleTimeout protocol gating and leaving the resulting runtime objects clean.integration-tests/src/main/docker/grpc-echo/Dockerfile (1)
25-28: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueSpecify a non-root user for the runtime container.
Running as the
rootuser is not recommended, even for test services. Consider dropping privileges before executing the Quarkus fast-jar.🛡️ Proposed fix to drop privileges
WORKDIR /app # Quarkus fast-jar layout: quarkus-run.jar plus the lib/, app/ and quarkus/ trees. COPY --from=build /src/integration-tests/target/quarkus-app/ /app/ + +USER 1001:1001 # Separate gRPC server, bound on all interfaces so the gateway can reach it by service name.🤖 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 `@integration-tests/src/main/docker/grpc-echo/Dockerfile` around lines 25 - 28, Update the runtime stage of the Dockerfile after the Quarkus fast-jar files are copied to create or select a non-root user and configure the container to run as that user, preserving the existing WORKDIR and application startup behavior.Source: Linters/SAST tools
benchmarks/src/main/resources/k6-scripts/grpc_unary.js (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard against null response in k6 checks.
If
responseisnull(e.g., due to a connection drop or network error), evaluatingr.statusinside the check will throw aTypeError. Consider adding a null-guard to ensure the check evaluates cleanly.💡 Proposed defensive check
- 'status is OK': (r) => r.status === grpc.StatusOK, + 'status is OK': (r) => r && r.status === grpc.StatusOK,🤖 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 `@benchmarks/src/main/resources/k6-scripts/grpc_unary.js` at line 69, Update the `'status is OK'` check in the gRPC unary script to guard against a null response before accessing its status, while preserving the existing grpc.StatusOK comparison for non-null responses.integration-tests/src/test/java/de/cuioss/sheriff/api/integration/WebSocketProxyIT.java (1)
86-86: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueUse explicitly secure TLS versions to silence SAST warnings.
Static analysis tools (SAST) often flag
SSLContext.getInstance("TLS")as a weak SSL context because it could theoretically negotiate older, insecure protocols. Consider explicitly specifying"TLSv1.3"or"TLSv1.2"to ensure secure defaults and silence SAST warnings, even in test environments.💡 Proposed refactor
- SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLContext sslContext = SSLContext.getInstance("TLSv1.3");🤖 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 `@integration-tests/src/test/java/de/cuioss/sheriff/api/integration/WebSocketProxyIT.java` at line 86, Update the SSLContext initialization in WebSocketProxyIT to request an explicitly secure protocol, such as TLSv1.3 or TLSv1.2, instead of the generic "TLS" value, while preserving the existing test behavior.Source: Linters/SAST tools
integration-tests/src/main/proto/echo.proto (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMatch
.protofile paths to their package names.Buf lint rule
PACKAGE_DIRECTORY_MATCHexpects.protofiles to be placed in directories that match their package names to avoid collisions and maintain consistency. If strict Protobuf linting is enforced in your CI pipeline, consider relocating these files; otherwise, this can be safely ignored if the flat layout is intentional for test scaffolding.
integration-tests/src/main/proto/echo.proto#L7-L7: Relocate tode/cuioss/sheriff/api/integration/grpc/echo.proto.benchmarks/src/main/resources/k6-scripts/echo.proto#L8-L8: Relocate tode/cuioss/sheriff/api/integration/grpc/echo.proto.🤖 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 `@integration-tests/src/main/proto/echo.proto` at line 7, Relocate both proto files to directories matching their package de.cuioss.sheriff.api.integration.grpc: move integration-tests/src/main/proto/echo.proto to de/cuioss/sheriff/api/integration/grpc/echo.proto and benchmarks/src/main/resources/k6-scripts/echo.proto to de/cuioss/sheriff/api/integration/grpc/echo.proto. Update any references to the files so builds and tests continue resolving them.Source: Linters/SAST tools
🤖 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
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.java`:
- Around line 297-310: Resolve the iteration-order mismatch between
effectiveAllowedOrigins and ResolvedRoute: either preserve declaration order
through the defensive copy by retaining an insertion-ordered set, or remove the
“preserving declaration order” claim if order is not required for exact-match
validation. Keep the existing lower-casing and de-duplication behavior
unchanged.
In `@api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.java`:
- Around line 463-473: Preserve stage-0 security headers across WebSocket
handshake failures: in
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.java:463-473,
pass an immutable copy of request.responseHeaders() to
WebSocketRelayStage.relay; in
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/WebSocketRelayStage.java:100-117,
extend the relay API to retain that map; and in
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/WebSocketRelayStage.java:132-152,
apply the retained headers before setting the failure status and ending the
response.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssembler.java`:
- Around line 146-147: The guard identity and generated circuit-breaker name in
guardFor(...) must distinguish routes by scheme and forcedHttp2 in addition to
host, port, and retry settings. Update the relevant ResilienceShape/name
construction so HTTP and gRPC routes targeting the same upstream produce unique
breaker names, and add a regression test covering mixed HTTP/gRPC routes.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessor.java`:
- Line 36: Update the UPGRADE_METHODS constant in WebSocketProtocolProcessor to
use an immutable single-element set instead of the mutable EnumSet, while
preserving the existing GET-only contents and standardMethods() behavior.
In `@doc/adr/0015-websocket-origin-failclosed-allowlist.adoc`:
- Around line 42-44: Update the origin-matching policy text to cite RFC 6454 for
the web origin model and RFC 3986 §3.2.2 for case-insensitive host comparison.
Preserve the existing exact scheme-and-port matching and no-wildcard policy,
replacing only the RFC 6066 reference.
In `@doc/development/protocol-processors.adoc`:
- Around line 121-127: The “To add a future protocol” guidance is incomplete and
incorrectly implies that registry registration alone removes boot rejection.
Update this section to require adding the protocol enum, configuration
schema/model support, validation, and any applicable dispatch-path handling,
while retaining SPI implementation and ProtocolProcessorRegistry registration as
required steps.
In `@doc/plan/05-protocol-processors.adoc`:
- Around line 41-45: The plan note contradicts itself by denying config-model
changes while describing additive fields. Update the note to state that there is
no config-model restructuring, while explicitly identifying allowed_origins and
websocket.idle_timeout_seconds as additive fields on the existing ResolvedRoute
model.
- Around line 202-207: Remove or rewrite the preceding benchmark work-breakdown
steps that instruct contributors to use ghz, using the k6 scripts and aspect
harness as the sole workflow for both WebSocket and gRPC benchmarks. Preserve
the existing APISIX comparison integration through the ASPECT_SCRIPTS map and
eliminate contradictory ghz guidance.
In `@integration-tests/docker-compose.apisix.yml`:
- Around line 68-76: Update the APISIX service’s depends_on configuration to map
syntax, preserving nginx-static and go-httpbin dependencies while setting
grpc-echo’s condition to service_healthy. Match the existing api-sheriff
dependency configuration and retain the current service relationships.
In `@integration-tests/scripts/verify-invalid-config-fails.sh`:
- Around line 133-175: Update the docker run invocation inside
assert_fails_to_boot to pass QUARKUS_PROFILE=it, ensuring the
bearer-authenticated WebSocket configuration uses the integration test token
issuers and fails exclusively on the missing allowlist validation.
---
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.java`:
- Line 171: Update the route-building logic around RouteTableBuilder’s
effectiveAllowedOrigins call to evaluate WebSocket origins only when the route
protocol is Protocol.WEBSOCKET. For non-WebSocket routes, skip parsing and
carrying websocket origin configuration, matching the existing idleTimeout
protocol gating and leaving the resulting runtime objects clean.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.java`:
- Line 37: Make the GRPC_METHODS set used by standardMethods() immutable by
replacing the mutable EnumSet initialization with Set.of(HttpMethod.POST).
Preserve the existing supported-method contents and standardMethods() behavior.
In `@benchmarks/src/main/resources/k6-scripts/grpc_unary.js`:
- Line 69: Update the `'status is OK'` check in the gRPC unary script to guard
against a null response before accessing its status, while preserving the
existing grpc.StatusOK comparison for non-null responses.
In `@integration-tests/src/main/docker/grpc-echo/Dockerfile`:
- Around line 25-28: Update the runtime stage of the Dockerfile after the
Quarkus fast-jar files are copied to create or select a non-root user and
configure the container to run as that user, preserving the existing WORKDIR and
application startup behavior.
In `@integration-tests/src/main/proto/echo.proto`:
- Line 7: Relocate both proto files to directories matching their package
de.cuioss.sheriff.api.integration.grpc: move
integration-tests/src/main/proto/echo.proto to
de/cuioss/sheriff/api/integration/grpc/echo.proto and
benchmarks/src/main/resources/k6-scripts/echo.proto to
de/cuioss/sheriff/api/integration/grpc/echo.proto. Update any references to the
files so builds and tests continue resolving them.
In
`@integration-tests/src/test/java/de/cuioss/sheriff/api/integration/WebSocketProxyIT.java`:
- Line 86: Update the SSLContext initialization in WebSocketProxyIT to request
an explicitly secure protocol, such as TLSv1.3 or TLSv1.2, instead of the
generic "TLS" value, while preserving the existing test 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: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b9a30a38-8878-42e0-a147-f82b7256024f
📒 Files selected for processing (70)
api-sheriff/src/main/java/de/cuioss/sheriff/api/ApiSheriffLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/WebSocketConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcDispatchStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcStatusMapper.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/ResponseStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssembler.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/WebSocketRelayStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/EventType.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/OriginValidationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigModelReflection.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapper.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/ProtocolProcessorRegistry.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/RouteRuntime.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessor.javaapi-sheriff/src/main/resources/schema/endpoint.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GatewayEdgeRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GrpcDispatchStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GrpcStatusMapperTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssemblerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/WebSocketRelayStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/OriginValidationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapperTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/RouteRuntimeTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessorTest.javabenchmarks/README.adocbenchmarks/pom.xmlbenchmarks/scripts/run-comparison.shbenchmarks/src/main/resources/k6-scripts/echo.protobenchmarks/src/main/resources/k6-scripts/grpc_unary.jsbenchmarks/src/main/resources/k6-scripts/lib/summary.jsbenchmarks/src/main/resources/k6-scripts/lib/target.jsbenchmarks/src/main/resources/k6-scripts/websocket_echo.jsbenchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriterTest.javadoc/LogMessages.adocdoc/README.adocdoc/adr/0015-websocket-origin-failclosed-allowlist.adocdoc/adr/0016-grpc-trailers-only-rejection-mapping.adocdoc/architecture.adocdoc/configuration.adocdoc/development/README.adocdoc/development/protocol-processors.adocdoc/plan/05-protocol-processors.adocdoc/plan/README.adocdoc/user/README.adocdoc/user/protocol-routes.adocdoc/variants/01-base-gateway.adocintegration-tests/docker-compose.apisix.ymlintegration-tests/docker-compose.ymlintegration-tests/pom.xmlintegration-tests/scripts/verify-invalid-config-fails.shintegration-tests/src/main/docker/apisix/apisix.yamlintegration-tests/src/main/docker/grpc-echo/Dockerfileintegration-tests/src/main/docker/health-check.shintegration-tests/src/main/docker/sheriff-config/endpoints/grpc.yamlintegration-tests/src/main/docker/sheriff-config/endpoints/websocket.yamlintegration-tests/src/main/docker/sheriff-config/topology.propertiesintegration-tests/src/main/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoService.javaintegration-tests/src/main/proto/echo.protointegration-tests/src/test/java/de/cuioss/sheriff/api/integration/GrpcProxyIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/WebSocketProxyIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoServiceTest.java
💤 Files with no reviewable changes (2)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapper.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapperTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@integration-tests/scripts/start-integration-container.sh`:
- Around line 75-84: Update the LOG_TARGET_DIR setup in
start-integration-container.sh to use a dedicated log subdirectory under the
project target directory, rather than the entire target directory. Create that
directory and apply chmod 0777 only to it, preserving the container’s ability to
write quarkus.log while leaving other build artifacts protected.
🪄 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: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ab72f65e-ff39-481a-bb11-9e446e7e2311
📒 Files selected for processing (3)
integration-tests/scripts/start-integration-container.shintegration-tests/src/main/docker/sheriff-config/endpoints/grpc.yamlintegration-tests/src/main/docker/sheriff-config/endpoints/websocket.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- integration-tests/src/main/docker/sheriff-config/endpoints/websocket.yaml
…onds Author the two doc-first WebSocket config keys in doc/configuration.adoc before the implementing code: - allowed_origins: security_headers-adjacent, per-route Origin allowlist on bearer WebSocket routes; exact-match origins (scheme+host+port), host compared case-insensitively, no wildcards, fail-closed (absent/empty allowlist on a bearer WS route fails the boot). - websocket.idle_timeout_seconds: per-route integer, default 300, bounding an established relay; ping/pong counts as activity. Covered across the schema section, the field reference (type/default/allowed values/validation), and the validation-rules summary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…RPC protocol notes Author, doc-first before the code, the gRPC error contract and the WebSocket and gRPC protocol-support structural notes in doc/architecture.adoc: - gRPC rejections (trailers-only): a gateway rejection on a protocol: grpc route is emitted as a trailers-only gRPC response (a gRPC client cannot consume application/problem+json), with the canonical status mapping 400->INVALID_ARGUMENT, 401->UNAUTHENTICATED, 403->PERMISSION_DENIED, 404->NOT_FOUND, 405->UNIMPLEMENTED, 502/503->UNAVAILABLE (also h2 negotiation failure at dispatch), 504->DEADLINE_EXCEEDED. - Protocol Support: WebSocket handshake-time auth, Origin allowlist validation, opaque post-101 relay, idle-timeout reclaim; gRPC forced-h2 upstream, trailer relay, no protobuf inspection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Auditability layer for the protocol-processors work, authored doc-first: - doc/adr/0015: WebSocket Origin fail-closed exact-match allowlist decision (GW-09/CSWSH rationale, no "any origin" default, why a new field over CORS, rejected alternatives). - doc/adr/0016: gRPC trailers-only rejection status-mapping decision (why a gRPC client cannot consume problem+json, canonical mapping, h2 failure -> UNAVAILABLE, rejected alternatives). - doc/plan/05-protocol-processors.adoc: orchestration notes (k6 supersedes the ghz/k6 split per Clarification 1, config model final via PR #84, parent-POM 1.5.4, two folded work items). - doc/variants/01-base-gateway.adoc: protocol scope complete (ADR-0002 promise closed with WS + gRPC). - doc/plan/README.adoc, doc/README.adoc: Plan 05 delivered; ADR-0015/0016 rows. - doc/LogMessages.adoc: doc-first WS LogRecords (INFO relay established / idle reclaim; WARN Origin rejection), %s-only templates in range. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The data-plane edge (GatewayEdgeRoute.renderProblem) renders RFC 9457 problem+json directly on the Vert.x route and never throws GatewayException up to JAX-RS. The live-wiring stop-guard confirms there are no JAX-RS @path resources in api-sheriff and no production reference to the mapper's render/translate, so the @Provider ExceptionMapper is dead. Remove it and its companion test outright (pre-1.0: remove, never deprecate). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… not pom) Investigation outcome (deliverable 8, TASK-006/007): an empirical probe (a deliberately-erroring test) proves the reactor Maven build already fails correctly on a test error (BUILD FAILURE, Errors: 1) — there is no testFailureIgnore in any pom, so the pom is not the culprit and needs no change. The observed "green verify with errored tests" reproduces at the plan-marshall:build-maven wrapper, which reports status: success while Maven reports BUILD FAILURE for a test error; that is plan-marshall infrastructure, out of scope for this project's pom. CI (the cuioss-organization reusable build) keys off Maven's exit code and is unaffected. No proving test is added (it would permanently redden the suite); the finding stands as the recorded outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ble 9) Add a tiny Quarkus gRPC echo service backing the gRPC integration-test matrix: unary echo, server-streaming echo, and a deliberately-failing method (non-OK grpc-status) for trailer/status relay assertions. - echo.proto: the three-method Echo contract. - GrpcEchoService: Mutiny @GrpcService implementing the contract. - grpc-echo/Dockerfile + docker-compose: build the echo fast-jar and run it as the grpc-echo upstream; gateway depends_on gates on its service_healthy probe. - pom.xml: quarkus-grpc extension (BOM-managed, operator-approved). The inherited Quarkus 'default' execution is disabled and replaced by a 'grpc-codegen' execution so echo.proto compiles without packaging the module; a 'grpc-echo' profile re-enables the 'build' goal for the container fast-jar. api-sheriff is scoped 'provided' so the gateway is not bundled into the echo app. - health-check.sh: document that gateway readiness deliberately does not gate on upstream availability (enforced at the compose layer instead). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Isolated JUnit 5 tests for GrpcEchoService, awaiting its Mutiny results (no gRPC transport): unary echo at index 0, server-streaming fan-out with rising indices and non-positive-count clamping, and the deliberate FAILED_PRECONDITION failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dation Add WebSocketConfig (allowed_origins + idle_timeout_seconds), thread it onto RouteConfig and the materialized ResolvedRoute (effective lower-cased origin allowlist + effective idle timeout, default 300s), and register it for native-image reflection. Enforce the ADR-0015 fail-closed contract in ConfigValidator: a bearer WebSocket route with an absent/empty allowed_origins is rejected at boot, wildcard origins are rejected, and idle_timeout_seconds must be positive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Register a WebSocketProtocolProcessor for Protocol.WEBSOCKET (removing the boot rejection; session-auth WebSocket routes stay rejected until Plan 07). Add the protocol-dispatch seam in GatewayEdgeRoute: a WebSocket route runs the fail-closed OriginValidationStage (GW-09 / CSWSH, HTTP 403 before dial) then hands the upgrade to the new WebSocketRelayStage instead of the HTTP DispatchStage. The relay dials the upstream first, upgrades the client only on a 101, and relays frames opaquely and bidirectionally with backpressure, transparent close/half-close, and a per-route idle-timeout reclaim (close 1001) that ping/pong activity resets; an unreachable upstream maps to 502/504 before the upgrade. Thread the effective allowed-origins set and idle timeout onto RouteRuntime/RouteRuntimeAssembler, and add the WebSocket event types (403 Origin rejection, 1001 idle reclaim) and LogRecords. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ailer relay, and trailers-only rejection mapping Implements deliverable 6: the protocol: grpc dispatch path. - GrpcProtocolProcessor serves the gRPC POST call verb; registered in ProtocolProcessorRegistry for Protocol.GRPC (boot rejection removed). - GrpcStatusMapper maps each rejection cause to its canonical gRPC status (INVALID_ARGUMENT / UNAUTHENTICATED / PERMISSION_DENIED / NOT_FOUND / UNIMPLEMENTED / UNAVAILABLE / DEADLINE_EXCEEDED / UNKNOWN) and renders a trailers-only gRPC rejection (HTTP 200, application/grpc, grpc-status / grpc-message). - GrpcDispatchStage forces the upstream to HTTP/2 (the h2 flag joins the client-sharing tuple in RouteRuntimeAssembler.UpstreamTarget), streams opaque length-prefixed frames via the shared byte-capped DispatchStage, and maps an h2-negotiation failure to UNAVAILABLE. - ResponseStage.relayWithTrailers relays upstream grpc-status / grpc-message trailers to the client; GatewayEdgeRoute routes the GRPC arm through the gRPC dispatch + trailers-only rejection seam. Tests: GrpcProtocolProcessorTest, GrpcStatusMapperTest (one row per mapping), GrpcDispatchStageTest (forced-h2 wiring, UNAVAILABLE mapping, trailer relay, shared body-cap GW-08 bounds). Migrates the three now-stale gRPC-fails-boot assertions to assert the gRPC route now assembles cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements deliverable 13: a task-oriented operator guide at doc/user/protocol-routes.adoc covering how to configure protocol: websocket routes (allowed_origins, websocket.idle_timeout_seconds) and protocol: grpc routes, cross-linking the configuration reference and the architecture gRPC error contract rather than duplicating rationale. Linked into doc/user/README.adoc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…I and dispatch seam Implements deliverable 14: a contributor-facing guide at doc/development/protocol-processors.adoc describing the ProtocolProcessor SPI, the boot-time registry, and the edge protocol-dispatch seam (WebSocket handshake/relay and gRPC forced-h2/trailer paths), plus how to add a new processor. Linked into doc/development/README.adoc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e 10) Add WebSocketProxyIT exercising the protocol: websocket dispatch path end-to-end over the public HTTPS edge: an allow-listed-Origin echo round-trip through the opaque relay; a tokenless bearer handshake rejected 401 before any upstream dial; foreign / absent Origin rejected 403 by the fail-closed Origin gate (GW-09/CSWSH); idle-reclaim (close 1001) versus a heartbeated relay surviving; and an unmatched WS path 404. The suite is a black-box JDK WebSocket client; following the BearerValidationIT precedent it drives the successful echo over a public route and the bearer route via its rejection path (no signing key to mint a token). Add endpoints/websocket.yaml (ws-echo public + Origin allowlist, ws-bearer route- level bearer, ws-idle short idle_timeout_seconds) and the WS_UPSTREAM topology alias to the go-httpbin /websocket/echo backend. Complete the ADR-0015 fail-closed contract that deliverable 4 left unwired: add the websocket route block (allowed_origins + idle_timeout_seconds) to endpoint.schema.json so a WebSocket route config validates at boot instead of being rejected by the route object's additionalProperties: false. Extend verify-invalid-config-fails.sh with a bearer-WS-empty-allowlist case so the fail-closed boot rejection is proven end-to-end (it cannot coexist with the bootable running stack). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add GrpcProxyIT exercising the protocol: grpc dispatch path end-to-end through the
gateway against the in-repo Quarkus gRPC echo upstream: a unary echo round-trip with
client-attached and gateway-injected metadata; a server-streaming echo relaying every
frame in order; the grpc-status / grpc-message trailer relay from the intentionally
failing method; and a tokenless bearer gRPC call rejected UNAUTHENTICATED as a
trailers-only response before any upstream dial. The black-box client is grpc-netty
over ALPN h2 with trust-all TLS, and a path-prefixing ClientInterceptor rides each
call under the gateway's /grpc/<aspect> anchor prefix (which the gateway strips so the
upstream sees the bare /{package}.Echo/{Method} path).
Add endpoints/grpc.yaml (public grpc-echo route allow-listing the gRPC framing metadata
plus a set_headers injection, and a route-level bearer grpc-secure route) and the
GRPC_ECHO topology alias to the grpc-echo:9000 forced-h2 upstream.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l (deliverable 11)
Operator decision 2026-07-21: gRPC routes use the bare service path rather than a
synthetic /grpc/<aspect> anchor prefix. A route matches the service-rooted
/de.cuioss.sheriff.api.integration.grpc.Echo prefix and sets an identical upstream.path,
so the gateway's remainder-append rewrite reconstructs the full /{package}.Echo/{Method}
path unchanged end-to-end. This is pure route-yaml authoring — the config model already
supports it.
Rework endpoints/grpc.yaml to the bare-service-path shape (public + bearer routes) and
remove the client-side path-prefixing ClientInterceptor from GrpcProxyIT so a stock
grpc-java channel dials the real service path as-is. The public and bearer routes share
the one Echo service path, so the bearer route is selected by a stock x-sheriff-route:
secure request-metadata header the bearer stub attaches (declared first so the
longest-prefix-first, declaration-order-stable route sort evaluates it ahead of the
unconstrained public route). All four IT assertions (unary echo, server-streaming echo,
grpc-status/trailer relay, tokenless UNAUTHENTICATED trailers-only) are preserved.
Update the topology.properties GRPC_ECHO comment to describe the bare-service-path model.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Echo service path
Follow-up to the bare-service-path rework: a public Echo route and a bearer route on the
SAME /{package}.Echo path are not statically disjoint (a public gRPC client carries no
distinguishing host/header, and a lone header matcher establishes nothing), so the gateway
rejects the pair at boot — route precedence deliberately never depends on declaration order.
Add a distinct SecureEcho service to echo.proto and move the bearer route onto its bare
/{package}.SecureEcho path (identical upstream.path), keeping the two routes disjoint by
prefix. SecureEcho is defined in the proto but not implemented upstream — the bearer route
rejects a tokenless call UNAUTHENTICATED before any dial, so it is never reached. GrpcProxyIT
drives it with the stock generated SecureEchoGrpc stub (no path rewriting), and the earlier
x-sheriff-route header-matcher selector is removed.
Carry the routing-model rationale (chosen bare-service-path shape + rejected synthetic-anchor
and shared-path-discriminator alternatives) into the plan doc (D2b) and correct the gRPC route
example in doc/user/protocol-routes.adoc to the schema-accurate base_url + upstream.path shape.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…outes (deliverable 12) Add two protocol aspects to the k6 benchmark matrix: * websocket_echo.js — WebSocket upgrade + opaque frame relay round-trip through the gateway to go-httpbin's /websocket/echo (Origin-gated /ws/echo route), reporting round-trips/sec and round-trip latency. * grpc_unary.js — unary gRPC call over the forced-h2 relay on the bare service path against the in-repo grpc-echo service, reporting calls/sec and call latency. Per Clarification 1 this k6 gRPC benchmark supersedes the plan doc's ghz proposal. echo.proto is shipped to the k6 script dir (bundled into the k6 image) so the stock k6 grpc client loads the descriptor. Generalize lib/summary.js with durationMetric/requestsMetric/failuresMetric overrides so the non-HTTP aspects source their headline figures from the metrics that characterise them (ws_rtt/ws_roundtrips/ws_errors, grpc_req_duration/grpc_calls/grpc_req_failed) — the http_* defaults keep every existing HTTP script unchanged. Add wsUrl/grpcAddress target helpers. ComparisonSummaryWriter needs no change: it is data-driven and renders any aspect's summary generically (verified by the TASK-21 tests). Extend run-comparison.sh (ASPECT_SCRIPTS + ALL_ASPECTS) and the -Pbenchmark pom executions with the ws and grpc aspects. Complete the APISIX ws/grpc parity routes (previously pre-provisioned stubs) with protocol-appropriate backends — go-httpbin WebSocket echo and the grpc-scheme grpc-echo upstream — instead of the static fairness backend, and wire both into the apisix overlay's depends_on. FAIRNESS CAVEAT (documented in README.adoc and apisix.yaml): unlike the six HTTP aspects that ride the single static fairness backend, the ws and grpc aspects fold a real protocol backend's echo cost into the number. Both gateways target the same two protocol backends, so the rows stay symmetric, but read them as protocol-relay comparisons, not backend-free gateway-overhead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ummary (deliverable 12) Extend ComparisonSummaryWriterTest with value-asserting fixtures for the two protocol aspects, proving the data-driven writer renders websocketEcho and grpcUnary as generic RPS rows (not MBps, not throughput-only) with both P50/P99 pairs, and renders n/a for a protocol aspect only one target produced. These pin the writer's ws/grpc coverage without any aspect-specific code change to the writer itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ble 9) The integration-tests module is a test coordinator, not a published/consumed artifact. Its quarkus-grpc generated protobuf stubs are not on the javadoc tool's sourcepath, so the attach-javadocs jar goal failed under verify -Ppre-commit (Q-Gate finding c0faa2). Set maven.javadoc.skip=true in the module properties to suppress javadoc jar attachment module-scoped, leaving api-sheriff javadoc untouched.
…PC sources Mechanical normalization (EnumSet.of for enum sets, List.getFirst(), import de-qualification, continuation-indent) produced by the verify -Ppre-commit recipe run over files authored by this plan; verified green in the same run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…th posture
The api-sheriff native container refused to start (5 config violations) because
endpoints/websocket.yaml and endpoints/grpc.yaml declared public routes under the
access:public ws/grpc anchors without any resolvable auth posture: per ADR-0013 a
public anchor carries no auth block, so the require:none posture must be declared on
the endpoint (as endpoints/httpbin.yaml already does for the public api anchor).
Add an endpoint-level `auth: { require: none }` to both files so every public route
(ws-echo, ws-idle, grpc-echo) inherits a resolvable none posture while the bearer
routes keep their route-level override (route auth wins over endpoint/anchor auth,
ADR-0007). This clears all 5 boot validation violations so WebSocketProxyIT and
GrpcProxyIT can run.
Also chmod 0777 the host LOG_TARGET_DIR in start-integration-container.sh so the
distroless container (uid 1001) can write /logs/quarkus.log to the bind mount,
clearing the non-fatal "FileNotFoundException: /logs/quarkus.log (Permission denied)"
log-sink error without weakening the container's no-new-privileges / cap_drop posture.
Co-Authored-By: Claude <noreply@anthropic.com>
A protocol: grpc route rides the service-rooted /{package}.{Service}/{Method}
path, whose service component is a single opaque path segment (dots, no
slashes). No non-root anchor path_prefix can contain such a path on a segment
boundary, and stock gRPC clients cannot prepend a gateway namespace prefix — so
the native IT container refused to boot with two ApiSheriff-200 violations from
ConfigValidator.validateAnchorNamespaceMembership rule 3 (grpc-echo, grpc-bearer).
Skip both containment checks (rule 3 declared-anchor containment, rule 4
undeclared squatter) for protocol: grpc routes. Only the path-prefix geometry is
exempt: a gRPC route still declares an anchor for its ADR-0013 type/access
classification and ADR-0007 auth floor, and every other anchor rule
(declared-anchor existence, pairwise-disjoint prefixes, non-weakenable auth
floor, access→auth matrix) stays enforced unchanged. websocket/http routes keep
full containment enforcement.
Adds ConfigValidatorTest coverage for the exemption and the enforced-for-non-gRPC
contrast, and records the carve-out in ADR-0007.
Co-Authored-By: Claude <noreply@anthropic.com>
Inline dispositions from the unified wait-region triage: method references over lambdas (S1612), Set.of over EnumSet for immutable protocol sets, assertThrows single-invocation hoists (S5778), suppressed load-bearing Thread.sleep idle test with rationale (S2925), doc citation and wording fixes, and apisix compose depends_on healthcheck map form. Co-Authored-By: Claude <noreply@anthropic.com>
…e failures The WebSocket relay failure path (WebSocketRelayStage.onUpstreamFailure) set a 502/504 status and ended the response WITHOUT the accumulated stage-0 security headers that the HTTP (ResponseStage.relay) and gRPC (GrpcStatusMapper.renderRejection) rejection paths both apply. Thread the stage-0 security headers (request.responseHeaders()) into WebSocketRelayStage.relay, retain them across the async dial, and apply them in onUpstreamFailure before setStatusCode/end (only when !headWritten), mirroring the HTTP/gRPC contract. Add a regression test asserting a failed WS handshake response carries the stage-0 X-Content-Type-Options / X-Frame-Options headers. Addresses CodeRabbit finding 154f63 (Major/Security) on PR #88. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
start-integration-container.sh made the entire Maven target directory
world-writable (chmod 0777 ${PROJECT_DIR}/target) so the uid-1001 distroless
container could write /logs/quarkus.log. Apply least privilege: grant 0777 only
on a dedicated 'quarkus-logs' subdirectory (LOG_TARGET_ROOT/quarkus-logs), not
the whole target tree.
Thread the relocation through the start script, the docker-compose.yml and
docker-compose.jfr.yml bind-mount fallbacks (${LOG_TARGET_DIR:-./target/quarkus-logs}),
and the dump-keycloak-logs.sh path comment. The container-internal mount target
(/logs/quarkus.log) is unchanged.
Addresses CodeRabbit finding 570bf6 and security-audit finding 0c8354 (both
Major/Security, world-writable chmod) on PR #88.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Align shouldFailBootForSessionAndAssembleProtocolRoutes with the hoisted-local pattern PR #89 established (and the file's dominant style) after the rebase conflict resolution kept the plan-side test semantics. Co-Authored-By: Claude <noreply@anthropic.com>
OliverWolffGIP
force-pushed
the
feature/protocol-processors-ws-grpc
branch
from
July 21, 2026 20:19
9baafda to
195a2ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
`@api-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.java`:
- Around line 786-810: Update the route validation logic around the protocol
check in ConfigValidator so route.websocket() is rejected when the route
protocol is not WEBSOCKET, reporting a ConfigError for the invalid configuration
before continuing. Preserve the existing allowed-origins and idle-timeout
validations for actual WebSocket routes.
In `@benchmarks/src/main/resources/k6-scripts/echo.proto`:
- Line 8: The Echo proto is duplicated across
benchmarks/src/main/resources/k6-scripts/echo.proto:8 and
integration-tests/src/main/proto/echo.proto:7, causing duplicate symbols during
Buf lint. Deduplicate the Echo definition by retaining a single proto source, or
configure Buf to exclude the benchmark copy while preserving both files if
required.
In `@integration-tests/scripts/dump-keycloak-logs.sh`:
- Line 7: Update the log-path note in the script to identify
target/quarkus-logs/quarkus.log as the default location and mention that
LOG_TARGET_DIR can override the target directory.
In `@integration-tests/src/main/docker/grpc-echo/Dockerfile`:
- Around line 18-37: Update the final image configuration near the ENTRYPOINT to
create or use a non-root runtime user and set it with a USER directive before
the entrypoint. Ensure the user can read and execute the copied Quarkus
application under /app while preserving the existing JVM startup configuration.
🪄 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: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f39ad88e-761f-4827-95de-f218c12671b6
📒 Files selected for processing (74)
api-sheriff/src/main/java/de/cuioss/sheriff/api/ApiSheriffLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/WebSocketConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcDispatchStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcStatusMapper.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/ResponseStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssembler.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/edge/WebSocketRelayStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/events/EventType.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/OriginValidationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigModelReflection.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapper.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/ProtocolProcessorRegistry.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/RouteRuntime.javaapi-sheriff/src/main/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessor.javaapi-sheriff/src/main/resources/schema/endpoint.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GatewayEdgeRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GrpcDispatchStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GrpcStatusMapperTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssemblerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/edge/WebSocketRelayStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/OriginValidationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapperTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/RouteRuntimeTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessorTest.javabenchmarks/README.adocbenchmarks/pom.xmlbenchmarks/scripts/run-comparison.shbenchmarks/src/main/resources/k6-scripts/echo.protobenchmarks/src/main/resources/k6-scripts/grpc_unary.jsbenchmarks/src/main/resources/k6-scripts/lib/summary.jsbenchmarks/src/main/resources/k6-scripts/lib/target.jsbenchmarks/src/main/resources/k6-scripts/websocket_echo.jsbenchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriterTest.javadoc/LogMessages.adocdoc/README.adocdoc/adr/0007-anchor-scoped-policy.adocdoc/adr/0015-websocket-origin-failclosed-allowlist.adocdoc/adr/0016-grpc-trailers-only-rejection-mapping.adocdoc/architecture.adocdoc/configuration.adocdoc/development/README.adocdoc/development/protocol-processors.adocdoc/plan/05-protocol-processors.adocdoc/plan/README.adocdoc/user/README.adocdoc/user/protocol-routes.adocdoc/variants/01-base-gateway.adocintegration-tests/docker-compose.apisix.ymlintegration-tests/docker-compose.jfr.ymlintegration-tests/docker-compose.ymlintegration-tests/pom.xmlintegration-tests/scripts/dump-keycloak-logs.shintegration-tests/scripts/start-integration-container.shintegration-tests/scripts/verify-invalid-config-fails.shintegration-tests/src/main/docker/apisix/apisix.yamlintegration-tests/src/main/docker/grpc-echo/Dockerfileintegration-tests/src/main/docker/health-check.shintegration-tests/src/main/docker/sheriff-config/endpoints/grpc.yamlintegration-tests/src/main/docker/sheriff-config/endpoints/websocket.yamlintegration-tests/src/main/docker/sheriff-config/topology.propertiesintegration-tests/src/main/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoService.javaintegration-tests/src/main/proto/echo.protointegration-tests/src/test/java/de/cuioss/sheriff/api/integration/GrpcProxyIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/WebSocketProxyIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoServiceTest.java
💤 Files with no reviewable changes (2)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapper.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapperTest.java
🚧 Files skipped from review as they are similar to previous changes (59)
- api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/ConfigModelReflection.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessorTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/RouteConfig.java
- doc/plan/README.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessor.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessorTest.java
- doc/variants/01-base-gateway.adoc
- doc/LogMessages.adoc
- integration-tests/src/main/docker/sheriff-config/topology.properties
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/WebSocketConfig.java
- doc/plan/05-protocol-processors.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/RouteRuntime.java
- doc/development/README.adoc
- integration-tests/src/main/docker/health-check.sh
- integration-tests/src/main/docker/sheriff-config/endpoints/grpc.yaml
- doc/README.adoc
- integration-tests/src/main/docker/sheriff-config/endpoints/websocket.yaml
- api-sheriff/src/test/java/de/cuioss/sheriff/api/pipeline/OriginValidationStageTest.java
- doc/adr/0016-grpc-trailers-only-rejection-mapping.adoc
- benchmarks/src/main/resources/k6-scripts/websocket_echo.js
- integration-tests/docker-compose.yml
- benchmarks/src/main/resources/k6-scripts/lib/target.js
- doc/architecture.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/ResponseStage.java
- benchmarks/pom.xml
- api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/GatewayEdgeRouteTest.java
- integration-tests/src/test/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoServiceTest.java
- integration-tests/scripts/verify-invalid-config-fails.sh
- doc/user/protocol-routes.adoc
- benchmarks/scripts/run-comparison.sh
- doc/configuration.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcStatusMapper.java
- benchmarks/README.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/ProtocolProcessorRegistry.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/OriginValidationStage.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcDispatchStage.java
- benchmarks/src/main/resources/k6-scripts/grpc_unary.js
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/model/ResolvedRoute.java
- integration-tests/pom.xml
- benchmarks/src/main/resources/k6-scripts/lib/summary.js
- integration-tests/src/test/java/de/cuioss/sheriff/api/integration/GrpcProxyIT.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/WebSocketRelayStage.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/config/RouteTableBuilder.java
- integration-tests/src/main/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoService.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/events/EventType.java
- doc/adr/0007-anchor-scoped-policy.adoc
- doc/development/protocol-processors.adoc
- integration-tests/src/main/docker/apisix/apisix.yaml
- api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssemblerTest.java
- doc/adr/0015-websocket-origin-failclosed-allowlist.adoc
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GatewayEdgeRoute.java
- benchmarks/src/test/java/de/cuioss/sheriff/api/k6/benchmark/ComparisonSummaryWriterTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/edge/WebSocketRelayStageTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/config/validation/ConfigValidatorTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/config/model/ConfigModelContractTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/api/routing/RouteRuntimeTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/RouteRuntimeAssembler.java
- integration-tests/docker-compose.apisix.yml
Thread the route-level upstream.path into the per-route ResolvedUpstream base
path at route-table assembly so the forward URI is reconstructed as
stripTrailingSlash(upstream.path) + remainder-after-prefix — the documented
bare-service routing model. A gRPC route's /{package}.{Service} segment (and a
benchmark route's /anything/<aspect> rewrite) now reaches the upstream instead
of being stripped, fixing GrpcProxyIT's UNIMPLEMENTED failures. The route
upstream.path replaces the alias-derived base path wholesale (not appended); a
route without upstream.path keeps the alias base path — the default proxy
behavior. The alias host/port/scheme are carried through unchanged, so the
client/guard-sharing tuple is unaffected. Adds RouteTableBuilder unit coverage
for the materialize / replace / keep-alias / blank cases.
Co-Authored-By: Claude <noreply@anthropic.com>
Extract validateWebSocketRoute to clear the S3776 cognitive-complexity finding, reject a websocket block on a non-websocket route (with test), run the grpc-echo container as a non-root user with chowned app files, and correct the dump-keycloak-logs path note. Co-Authored-By: Claude <noreply@anthropic.com>
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
1 participant
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.
Summary
Delivers
doc/plan/05-protocol-processors.adocin full: WebSocket and gRPC protocolprocessors for the API Sheriff gateway, backed by an in-repo gRPC echo test upstream,
WS/gRPC integration tests, k6 benchmarks with an APISIX comparison-matrix extension,
and three-layer documentation (reference, operator, developer).
Changes
(
api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/WebSocketProtocolProcessor.java).GW-09
Originallowlist enforced fail-closed before dialing the upstream(
api-sheriff/src/main/java/de/cuioss/sheriff/api/pipeline/OriginValidationStage.java).Opaque bidirectional relay with idle-timeout reclaim
(
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/WebSocketRelayStage.java).(
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcDispatchStage.java),grpc-status/grpc-messagetrailer relay and trailers-only rejection mapping(
api-sheriff/src/main/java/de/cuioss/sheriff/api/edge/GrpcStatusMapper.java),with GW-08 HTTP/2 abuse bounds held on the gRPC path
(
api-sheriff/src/main/java/de/cuioss/sheriff/api/routing/GrpcProtocolProcessor.java).WebSocketConfig,ResolvedRoute/RouteConfigextensions,RouteTableBuilder,ConfigValidator, andendpoint.schema.jsonupdates tosupport the new
allowed_originsandwebsocket.idle_timeout_secondskeys.(
integration-tests/src/main/java/de/cuioss/sheriff/api/integration/grpc/GrpcEchoService.java)wired into docker-compose with a healthcheck, plus
integration-tests/src/test/java/de/cuioss/sheriff/api/integration/WebSocketProxyIT.javaand
integration-tests/src/test/java/de/cuioss/sheriff/api/integration/GrpcProxyIT.java.(
benchmarks/src/main/resources/k6-scripts/websocket_echo.js,benchmarks/src/main/resources/k6-scripts/grpc_unary.js) on the existing k6harness, and an APISIX comparison-matrix extension covering both aspects
(
benchmarks/scripts/run-comparison.sh).GatewayExceptionMapper(
api-sheriff/src/main/java/de/cuioss/sheriff/api/quarkus/GatewayExceptionMapper.java)per pre-1.0 rules (remove, never deprecate) — the stop-guard confirmed no live
consumer before removal.
confirmed the project POM configuration is correct — the defect is in the
plan-marshall
build-mavenwrapper's result parsing, not this project's build.Recorded as a finding for the plugin maintainers; CI itself is unaffected.
doc/configuration.adoc,doc/architecture.adoc,doc/LogMessages.adoc), operator guide(
doc/user/protocol-routes.adoc), developer guide(
doc/development/protocol-processors.adoc).doc/adr/0015-websocket-origin-failclosed-allowlist.adocanddoc/adr/0016-grpc-trailers-only-rejection-mapping.adoc./grpc/<aspect>prefix, since standard gRPC clients cannot inject one); therationale and rejected alternatives are carried into
doc/plan/05-protocol-processors.adocfor audit.cui-java-parent 1.5.4landed upstream via #86 during this plan; the redundantbaseline-bump commit dropped out cleanly on rebase.
Test Plan
verify -Ppre-commit)verify)verify -Pintegration-tests -pl integration-tests -am)Related Issues
None
🤖 Generated with Claude Code
Summary by CodeRabbit
Originallowlisting (no wildcards) and idle-timeout reclamation (default 300s).websocketandgrpc, including forced HTTP/2 upstream handling and gRPC trailer relaying.