From 3eb3d51094983aee71d94fe0eb79ea0975045d0b Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:50:34 +0200 Subject: [PATCH 01/11] fix(edge): release the WebSocket admission permit at relay teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GatewayEdgeRoute.handle registers its sole admission release inside ctx.addEndHandler. A completed WebSocket upgrade takes the connection over, so the HTTP response never ends, the end handler never fires, and one of the gateway's admission permits is stranded for the process lifetime — a sustained WebSocket workload degrades the gateway to blanket 503s. Stash the existing releaseAdmission CAS guard on the RoutingContext under a new key (the same idiom as ROUTE_KEY, and the seam that already crosses the virtual-thread hop), read it back in dispatchWebSocket, and hand WebSocketRelayStage a release callback. The relay invokes it on both teardown paths: the established relay's single idempotent RelaySession.closeBoth funnel, and the client-upgrade-failure branch that constructs no RelaySession at all. It is never invoked at upgrade completion, which would under-count concurrent relays. The upstream-dial-failure path is unchanged — it ends the response, so the end handler still releases through the same guard, and the shared CAS guard keeps any overlap from double-releasing. --- .../gateway/edge/GatewayEdgeRoute.java | 31 +++- .../gateway/edge/WebSocketRelayStage.java | 53 +++++-- .../gateway/edge/GatewayEdgeRouteTest.java | 118 ++++++++++++++ .../gateway/edge/WebSocketRelayStageTest.java | 148 +++++++++++++++++- 4 files changed, 336 insertions(+), 14 deletions(-) diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java index 9c28fbc2..823655c1 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java @@ -24,6 +24,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; @@ -188,6 +189,12 @@ public class GatewayEdgeRoute { * read failure or timeout, so {@link #readFormBody} reads {@code null} and the receiver fails closed * to {@code 400}. */ private static final String RESERVED_BODY_KEY = "sheriff.reservedbody"; + /** Holds the per-request admission-release CAS guard stashed in {@link #handle} so a terminal path + * that runs after the virtual-thread hop — where only the {@link RoutingContext} is in + * scope — can still release the permit exactly once. The WebSocket relay is the one such path: + * a completed upgrade takes the connection over, so the HTTP end handler never fires and + * {@link #dispatchWebSocket} must hand the relay its own release callback. */ + private static final String ADMISSION_GUARD_KEY = "sheriff.admissionguard"; private final List routes; private final ExecutorService virtualThreadExecutor; @@ -354,7 +361,13 @@ private void handle(RoutingContext ctx) { // Guard the admission accounting so it is rolled back exactly once. The end handler releases // it on the normal path; the executor-rejection path below also rolls it back, and both must // never double-release (reject() ends the response, which itself fires this end handler). + // The end handler is NOT the only release site: a protocol: websocket route that completes its + // upgrade takes the connection over, so the HTTP response never ends and this handler never + // fires. That route therefore releases through the same guard from the relay's teardown + // callback (see dispatchWebSocket / WebSocketRelayStage); the guard is stashed on the context + // under ADMISSION_GUARD_KEY because the virtual-thread hop carries only the RoutingContext. AtomicBoolean admissionReleased = new AtomicBoolean(); + ctx.put(ADMISSION_GUARD_KEY, admissionReleased); ctx.addEndHandler(result -> { releaseAdmission(admissionReleased); recordRequestMetrics(ctx, startNanos); @@ -537,8 +550,9 @@ private void dispatchProcessing(RoutingContext ctx, AtomicBoolean admissionRelea /** * Releases one admission permit and decrements the in-flight counter exactly once, guarded by - * {@code released} so the normal end-handler path and the executor-rejection rollback path can - * both call it without double-counting. + * {@code released} so the normal end-handler path, the executor-rejection rollback path and the + * WebSocket relay-teardown callback (see {@link #dispatchWebSocket}) can all call it without + * double-counting. */ private void releaseAdmission(AtomicBoolean released) { if (released.compareAndSet(false, true)) { @@ -819,6 +833,14 @@ private void dispatchAndRelay(RoutingContext ctx, PipelineRequest request, Route * before any upstream contact — GW-09 / CSWSH), then hands the upgrade to the opaque * {@link WebSocketRelayStage}. The upstream dial and the client upgrade are performed by the * relay stage, so no HTTP {@link ResponseStage} relay runs for a WebSocket route. + *

+ * Admission accounting. Because a completed upgrade takes the connection over, the + * HTTP end handler registered in {@link #handle} never fires for an established relay — the permit + * would otherwise be stranded for the process lifetime. The per-request CAS guard stashed under + * {@link #ADMISSION_GUARD_KEY} is therefore read back here and handed to the relay as a release + * callback, which the relay invokes on each of its teardown paths. Routing the release through the + * same guard keeps it idempotent against the end handler and the executor-rejection rollback, so an + * upstream-dial failure (which does end the response) can never double-release. */ private void dispatchWebSocket(RoutingContext ctx, PipelineRequest request, RouteRuntime route, ForwardPolicyStage.Result forward) { @@ -831,7 +853,10 @@ private void dispatchWebSocket(RoutingContext ctx, PipelineRequest request, Rout .orElseThrow(() -> new IllegalStateException("WebSocket dispatch requires a resolved upstream")); String uri = DispatchStage.upstreamRequestUri(upstreamTarget, remainder, query); applyStageSetCookies(ctx.response(), request.responseSetCookies()); - webSocketRelayStage.relay(ctx, route, forward.headers(), request.responseHeaders(), uri); + AtomicBoolean admissionGuard = Objects.requireNonNull(ctx.get(ADMISSION_GUARD_KEY), + "admission guard missing — handle() must stash it before dispatch"); + webSocketRelayStage.relay(ctx, route, forward.headers(), request.responseHeaders(), uri, + () -> releaseAdmission(admissionGuard)); } /** diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStage.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStage.java index 2d47ad8b..defdef02 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStage.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStage.java @@ -62,6 +62,18 @@ *

* Every socket operation is event-loop-bound; the relay hops onto the request's Vert.x context so * both legs share one event loop and the frame relay is single-threaded. + *

+ * An established relay owns an admission permit for its lifetime. A completed client + * upgrade takes the connection over, so the HTTP response never ends and the edge's end handler never + * fires — the relay is therefore handed a release callback and is the component responsible for + * returning the permit. It is invoked on every teardown path and never at upgrade completion: releasing + * on upgrade would under-count concurrent relays and re-open the exhaustion window the callback exists + * to close. Two terminal paths carry it — the established relay's single idempotent + * {@code RelaySession.closeBoth} funnel (graceful close, abrupt disconnect, idle reclaim, relay error), + * and the client-upgrade-failure branch of {@link #onUpstreamConnected}, which constructs no + * {@code RelaySession} at all. The upstream-dial-failure path ({@link #onUpstreamFailure}) does not + * invoke it: that path ends the HTTP response, so the edge's own end handler releases the permit + * through the same idempotent guard. * * @author API Sheriff Team * @since 1.0 @@ -103,14 +115,18 @@ public WebSocketRelayStage(UpstreamFailureMapper failureMapper, GatewayEventCoun * @param securityHeaders the stage-0 security headers accumulated on the response, applied to a * handshake-failure response before it is ended * @param requestUri the upstream request URI (path + allow-listed query) + * @param releaseAdmission the edge's idempotent admission-release callback, invoked once at relay + * teardown — on the established relay's {@code closeBoth} funnel and on the + * client-upgrade-failure branch — and never at upgrade completion */ public void relay(RoutingContext ctx, RouteRuntime route, Map forwardHeaders, - Map securityHeaders, String requestUri) { + Map securityHeaders, String requestUri, Runnable releaseAdmission) { Objects.requireNonNull(ctx, "ctx"); Objects.requireNonNull(route, "route"); Objects.requireNonNull(forwardHeaders, "forwardHeaders"); Objects.requireNonNull(securityHeaders, "securityHeaders"); Objects.requireNonNull(requestUri, "requestUri"); + Objects.requireNonNull(releaseAdmission, "releaseAdmission"); Map retainedSecurityHeaders = Map.copyOf(securityHeaders); HttpClient client = route.getHttpClient() .orElseThrow(() -> new IllegalStateException("WebSocket dispatch requires an upstream client")); @@ -123,18 +139,23 @@ public void relay(RoutingContext ctx, RouteRuntime route, Map fo .setURI(requestUri); forwardHeaders.forEach(options::addHeader); ctx.vertx().runOnContext(v -> client.webSocket(options) - .onSuccess(upstreamWs -> onUpstreamConnected(ctx, route, upstreamWs)) + .onSuccess(upstreamWs -> onUpstreamConnected(ctx, route, upstreamWs, releaseAdmission)) .onFailure(failure -> onUpstreamFailure(ctx, route, failure, retainedSecurityHeaders))); } - private void onUpstreamConnected(RoutingContext ctx, RouteRuntime route, WebSocket upstreamWs) { + private void onUpstreamConnected(RoutingContext ctx, RouteRuntime route, WebSocket upstreamWs, + Runnable releaseAdmission) { ctx.request().toWebSocket() - .onSuccess(clientWs -> establishRelay(ctx, route, clientWs, upstreamWs)) + .onSuccess(clientWs -> establishRelay(ctx, route, clientWs, upstreamWs, releaseAdmission)) .onFailure(failure -> { // The upstream is already upgraded but the client handshake could not complete; // there is no HTTP response to render anymore. Close the upstream leg and drop. + // No RelaySession is constructed here, so this branch owns its own admission + // release — without it the permit acquired for this request is stranded, since the + // HTTP end handler no longer has a response to fire on. LOGGER.debug(failure, "WebSocket client upgrade failed on route '%s': %s", route.getId(), failure.getMessage()); + releaseAdmission.run(); closeQuietly(upstreamWs, CLOSE_INTERNAL_ERROR, "client upgrade failed"); }); } @@ -167,11 +188,14 @@ private void onUpstreamFailure(RoutingContext ctx, RouteRuntime route, Throwable } private void establishRelay(RoutingContext ctx, RouteRuntime route, ServerWebSocket clientWs, - WebSocket upstreamWs) { + WebSocket upstreamWs, Runnable releaseAdmission) { int idleSeconds = route.getEffectiveWebSocketIdleTimeoutSeconds().orElse(DEFAULT_IDLE_TIMEOUT_SECONDS); LOGGER.info(ApiSheriffLogMessages.INFO.WEBSOCKET_RELAY_ESTABLISHED, route.getId()); eventCounter.increment(EventType.REQUEST_FORWARDED); - new RelaySession(ctx.vertx(), route.getId(), clientWs, upstreamWs, idleSeconds, eventCounter).start(); + // The admission permit stays held for the relay's whole lifetime — the session releases it from + // its single teardown funnel, never here at upgrade completion. + new RelaySession(ctx.vertx(), route.getId(), clientWs, upstreamWs, idleSeconds, eventCounter, + releaseAdmission).start(); } private static void closeQuietly(WebSocketBase ws, short code, @Nullable String reason) { @@ -181,9 +205,13 @@ private static void closeQuietly(WebSocketBase ws, short code, @Nullable String } /** - * One established relay: the two legs, the idle-timeout timer, and the frame-relay wiring. All - * callbacks run on the shared request event loop, so the mutable {@code closed} / timer state is - * single-threaded and needs no synchronization. + * One established relay: the two legs, the idle-timeout timer, the frame-relay wiring, and the + * admission permit the relay holds for its lifetime. All callbacks run on the shared request event + * loop, so the mutable {@code closed} / timer state is single-threaded and needs no synchronization. + *

+ * {@link #closeBoth} is the single idempotent teardown funnel every terminal path reaches — client + * close, upstream close, idle reclaim and relay error alike — so it is also the single site the + * admission-release callback is invoked from, exactly once. */ private static final class RelaySession { @@ -194,11 +222,12 @@ private static final class RelaySession { private final int idleSeconds; private final long idleMillis; private final GatewayEventCounter eventCounter; + private final Runnable releaseAdmission; private long idleTimerId = -1L; private boolean closed; RelaySession(Vertx vertx, String routeId, ServerWebSocket clientWs, WebSocket upstreamWs, int idleSeconds, - GatewayEventCounter eventCounter) { + GatewayEventCounter eventCounter, Runnable releaseAdmission) { this.vertx = vertx; this.routeId = routeId; this.clientWs = clientWs; @@ -206,6 +235,7 @@ private static final class RelaySession { this.idleSeconds = idleSeconds; this.idleMillis = idleSeconds * 1000L; this.eventCounter = eventCounter; + this.releaseAdmission = releaseAdmission; } void start() { @@ -289,6 +319,9 @@ private void closeBoth(short code, @Nullable String reason) { return; } closed = true; + // The relay held the admission permit for its whole lifetime; return it here, behind the + // latch, so every terminal path releases it exactly once. + releaseAdmission.run(); if (idleTimerId != -1L) { vertx.cancelTimer(idleTimerId); idleTimerId = -1L; diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java index 6c4e47d7..337431ad 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java @@ -17,15 +17,21 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.lang.annotation.Annotation; import java.time.Duration; import java.util.Iterator; import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import de.cuioss.sheriff.gateway.bff.runtime.BffRuntime; @@ -45,12 +51,19 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.quarkus.runtime.ShutdownEvent; import io.vertx.core.Vertx; +import io.vertx.core.http.HttpClient; +import io.vertx.core.http.HttpClientRequest; +import io.vertx.core.http.HttpServer; +import io.vertx.core.http.WebSocket; +import io.vertx.core.http.WebSocketClient; +import io.vertx.core.http.WebSocketConnectOptions; import io.vertx.ext.web.Router; import jakarta.enterprise.inject.Instance; import jakarta.enterprise.util.TypeLiteral; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; /** @@ -177,6 +190,111 @@ void drainsPromptlyWhenIdle() { "Graceful drain returns promptly when no request is in flight"); } + /** + * The admission-accounting plumbing a WebSocket route depends on. {@code handle()} registers its + * release inside {@code ctx.addEndHandler}, but a completed WebSocket upgrade takes the connection + * over so that handler never fires — the release CAS guard is therefore stashed on the + * {@link io.vertx.ext.web.RoutingContext} and read back by the WebSocket branch, which releases at + * relay teardown instead. These tests pin both halves of that seam against a refactor that reverts + * to the end-handler-only assumption and silently strands one permit per upgrade. + */ + @Nested + @DisplayName("admission-release guard stashed on the RoutingContext") + class AdmissionGuardPlumbing { + + /** Mirrors the private {@code GatewayEdgeRoute.ADMISSION_GUARD_KEY}. */ + private static final String ADMISSION_GUARD_KEY = "sheriff.admissionguard"; + + @Test + @DisplayName("stashes the release guard on the context before the pipeline is dispatched") + void stashesReleaseGuardBeforeDispatch() throws Exception { + // Arrange — an empty route table renders 404 without dialing anything; a probe handler + // registered ahead of the catch-all reads the stash back the moment handle() returns. + CompletableFuture stashed = new CompletableFuture<>(); + HttpServer front = startFront(new RouteTable(List.of()), stashed); + HttpClient client = vertx.createHttpClient(); + try { + // Act + client.request(io.vertx.core.http.HttpMethod.GET, front.actualPort(), "localhost", "/nothing") + .compose(HttpClientRequest::send); + + // Assert + assertInstanceOf(AtomicBoolean.class, stashed.get(15, TimeUnit.SECONDS), + "handle() stashes the admission-release CAS guard under its context key"); + } finally { + client.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + front.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + } + } + + @Test + @DisplayName("the WebSocket branch reads the guard back and releases at relay teardown") + void webSocketBranchReleasesThroughTheStashedGuard() throws Exception { + // Arrange — a real upgrade against a stub upstream, so the HTTP response never ends + HttpServer upstream = vertx.createHttpServer() + .webSocketHandler(ws -> ws.textMessageHandler(ws::writeTextMessage)) + .listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + CompletableFuture stashed = new CompletableFuture<>(); + HttpServer front = startFront(new RouteTable(List.of(webSocketRoute(upstream.actualPort()))), stashed); + WebSocketClient client = vertx.createWebSocketClient(); + try { + WebSocket socket = client.connect(new WebSocketConnectOptions() + .setHost("localhost").setPort(front.actualPort()).setURI("/w/room")) + .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + AtomicBoolean guard = assertInstanceOf(AtomicBoolean.class, stashed.get(15, TimeUnit.SECONDS), + "the WebSocket request stashes the same release guard"); + assertFalse(guard.get(), + "an established relay still holds its admission permit — release at upgrade " + + "completion would under-count concurrent relays"); + + // Act + socket.close().toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + + // Assert — nothing ever ended the HTTP response, so only the relay's teardown callback + // can have flipped the guard + awaitReleased(guard, "the WebSocket relay releases the admission permit at teardown"); + } finally { + client.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + front.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + upstream.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + } + } + + private HttpServer startFront(RouteTable table, CompletableFuture stashed) throws Exception { + Router router = Router.router(vertx); + // Registered before the edge, which adds its catch-all last: ctx.next() runs handle() + // synchronously up to the asynchronous dispatch, so the stash is visible on return. + router.route().handler(ctx -> { + ctx.next(); + stashed.complete(ctx.get(ADMISSION_GUARD_KEY)); + }); + newEdge(table).registerRoutes(router); + return vertx.createHttpServer().requestHandler(router) + .listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + } + } + + // NOSONAR java:S2925 - Thread.sleep is load-bearing: relay teardown is a real socket round-trip on + // a live Vert.x event loop with no virtual clock to advance, so the guard is polled until it flips. + @SuppressWarnings("java:S2925") + private static void awaitReleased(AtomicBoolean guard, String message) throws InterruptedException { + for (int attempt = 0; attempt < 200 && !guard.get(); attempt++) { + Thread.sleep(25); + } + assertTrue(guard.get(), message); + } + + private static ResolvedRoute webSocketRoute(int upstreamPort) { + return ResolvedRoute.builder() + .id("w") + .protocol(Protocol.WEBSOCKET) + .match(MatchConfig.builder().pathPrefix("/w").build()) + .effectiveAuth(AuthConfig.builder().require("none").build()) + .effectiveAllowedMethods(List.of(HttpMethod.GET)) + .upstream(Optional.of(new ResolvedUpstream("http", "localhost", upstreamPort, ""))) + .build(); + } + private GatewayEdgeRoute newEdge(RouteTable table) { return new GatewayEdgeRoute(table, gatewayConfig, new SingletonInstance<>(tokenValidator), vertx, virtualThreadExecutor, hardening, new SheriffMetrics(new SimpleMeterRegistry()), BffRuntime.inert()); diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java index 51ff1d9c..11bea007 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java @@ -24,6 +24,7 @@ import java.lang.annotation.Annotation; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletableFuture; @@ -45,7 +46,9 @@ import de.cuioss.sheriff.gateway.config.model.ResolvedUpstream; import de.cuioss.sheriff.gateway.config.model.RouteTable; import de.cuioss.sheriff.gateway.config.model.SecurityHeadersConfig; +import de.cuioss.sheriff.gateway.events.GatewayEventCounter; import de.cuioss.sheriff.gateway.quarkus.SheriffMetrics; +import de.cuioss.sheriff.gateway.routing.RouteRuntime; import de.cuioss.sheriff.token.validation.TokenValidator; import de.cuioss.sheriff.token.validation.test.generator.TestTokenGenerators; import de.cuioss.test.generator.junit.EnableGeneratorController; @@ -53,6 +56,8 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.vertx.core.MultiMap; import io.vertx.core.Vertx; +import io.vertx.core.http.HttpClient; +import io.vertx.core.http.HttpClientRequest; import io.vertx.core.http.HttpServer; import io.vertx.core.http.UpgradeRejectedException; import io.vertx.core.http.WebSocket; @@ -64,6 +69,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; /** @@ -88,7 +94,9 @@ class WebSocketRelayStageTest { private HttpServer upstreamServer; private HttpServer frontServer; private WebSocketClient wsClient; + private HttpClient relayUpstreamClient; private int frontPort; + private int upstreamPort; private int deadPort; private final AtomicInteger upstreamConnects = new AtomicInteger(); private final AtomicReference upstreamCustomHeader = new AtomicReference<>(); @@ -104,7 +112,8 @@ void setUp() throws Exception { upstreamCustomHeader.set(ws.headers().get("X-Custom")); ws.textMessageHandler(ws::writeTextMessage); }).listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); - int upstreamPort = upstreamServer.actualPort(); + upstreamPort = upstreamServer.actualPort(); + relayUpstreamClient = vertx.createHttpClient(); // A definitely-closed port for the unreachable-upstream case. HttpServer throwaway = vertx.createHttpServer().requestHandler(req -> req.response().end()) @@ -142,6 +151,7 @@ void setUp() throws Exception { @AfterEach void tearDown() throws Exception { wsClient.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + relayUpstreamClient.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); frontServer.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); upstreamServer.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); virtualThreadExecutor.close(); @@ -293,6 +303,142 @@ void heartbeatKeepsRelayOpen() throws Exception { assertFalse(closed.isDone(), "a heartbeated relay is not reclaimed while activity continues"); } + /** + * The admission permit a WebSocket request acquires at the edge is held for the relay's whole + * lifetime, because a completed upgrade takes the connection over and the HTTP end handler never + * fires. These tests drive {@link WebSocketRelayStage#relay} directly — bypassing the edge, so the + * release callback is a plain counter rather than a private semaphore — and pin that it runs + * exactly once on each teardown path, and not at all on the path the edge still releases itself. + */ + @Nested + @DisplayName("admission-release callback — exactly once per relay teardown") + class AdmissionReleaseCallback { + + @Test + @DisplayName("releases exactly once when an established relay closes, staying idempotent as both legs tear down") + void releasesOnceOnEstablishedRelayTeardown() throws Exception { + // Arrange — a live relay over the stub upstream, with the release callback counted + AtomicInteger releases = new AtomicInteger(); + HttpServer relayServer = startRelayOnlyServer(upstreamPort, releases::incrementAndGet); + try { + WebSocket socket = connectTo(relayServer.actualPort()); + CompletableFuture echoed = new CompletableFuture<>(); + socket.textMessageHandler(echoed::complete); + socket.writeTextMessage("live"); + echoed.get(15, TimeUnit.SECONDS); + assertEquals(0, releases.get(), "an established relay keeps holding its admission permit"); + + // Act + socket.close().toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + + // Assert — closeBoth is re-entered from the upstream leg's close handler once the first + // pass closed it, so the latch is what keeps the release at exactly one + awaitReleases(releases, "a graceful close releases the admission permit"); + assertNoFurtherRelease(releases, "the closeBoth latch makes a repeated teardown a no-op"); + } finally { + relayServer.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + } + } + + @Test + @DisplayName("releases once on the client-upgrade-failure branch, which builds no relay session") + void releasesOnceWhenTheClientUpgradeFails() throws Exception { + // Arrange — the upstream accepts the upgrade, but the client request is a plain GET, so + // ctx.request().toWebSocket() fails. That branch constructs no RelaySession and renders no + // HTTP response, so it must return the permit itself or the request strands one forever. + AtomicInteger releases = new AtomicInteger(); + HttpServer relayServer = startRelayOnlyServer(upstreamPort, releases::incrementAndGet); + HttpClient plainClient = vertx.createHttpClient(); + try { + // Act — deliberately not awaited: the branch under test ends no response, so the + // send future never completes; the release callback is the observable outcome. + plainClient.request(io.vertx.core.http.HttpMethod.GET, relayServer.actualPort(), "localhost", + "/relay").compose(HttpClientRequest::send); + + // Assert + awaitReleases(releases, "the client-upgrade-failure branch releases the admission permit"); + assertNoFurtherRelease(releases, "the upgrade-failure branch releases exactly once"); + } finally { + plainClient.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + relayServer.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + } + } + + @Test + @DisplayName("never fires on an upstream-dial failure, which the edge's own end handler already covers") + void doesNotReleaseOnUpstreamDialFailure() throws Exception { + // Arrange — the route points at a closed port, so the dial fails before any upgrade + AtomicInteger releases = new AtomicInteger(); + HttpServer relayServer = startRelayOnlyServer(deadPort, releases::incrementAndGet); + try { + // Act + ExecutionException failure = assertThrows(ExecutionException.class, + () -> connectTo(relayServer.actualPort())); + + // Assert — the dial-failure path ends the HTTP response, so the edge's end handler + // releases the permit through the same CAS guard; firing the callback too would be a + // double-release attempt rather than a fix. + assertInstanceOf(UpgradeRejectedException.class, failure.getCause()); + assertEquals(0, releases.get(), + "the dial-failure path leaves the release to the edge's end handler"); + } finally { + relayServer.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + } + } + + private WebSocket connectTo(int port) throws Exception { + return wsClient.connect(new WebSocketConnectOptions() + .setHost("localhost").setPort(port).setURI("/relay")) + .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + } + } + + /** + * Stands up a front server whose single route hands the request straight to a fresh + * {@link WebSocketRelayStage} — no edge, no pipeline — so {@code releaseAdmission} is observable as + * a plain callback instead of the edge's private admission semaphore. + */ + private HttpServer startRelayOnlyServer(int upstreamTargetPort, Runnable releaseAdmission) throws Exception { + RouteRuntime route = RouteRuntime.builder() + .id("relay-only") + .protocol(Protocol.WEBSOCKET) + .upstream(Optional.of(new ResolvedUpstream("http", "localhost", upstreamTargetPort, ""))) + .httpClient(Optional.of(relayUpstreamClient)) + .effectiveWebSocketIdleTimeoutSeconds(Optional.of(300)) + .build(); + WebSocketRelayStage stage = new WebSocketRelayStage( + new UpstreamFailureMapper(new GatewayEventCounter()), new GatewayEventCounter()); + Router router = Router.router(vertx); + router.route().handler(ctx -> { + // Mirror GatewayEdgeRoute.handle(): the request stream is paused before the dispatch, so the + // asynchronous upstream dial cannot race the body being read out from under toWebSocket(). + ctx.request().pause(); + stage.relay(ctx, route, Map.of(), Map.of(), "/", releaseAdmission); + }); + return vertx.createHttpServer().requestHandler(router) + .listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + } + + // NOSONAR java:S2925 - Thread.sleep is load-bearing: relay teardown is a real socket round-trip on + // a live Vert.x event loop and no virtual clock can advance it, so the callback count is polled + // until it settles rather than assumed to have landed. + @SuppressWarnings("java:S2925") + private static void awaitReleases(AtomicInteger releases, String message) throws InterruptedException { + for (int attempt = 0; attempt < 200 && releases.get() < 1; attempt++) { + Thread.sleep(25); + } + assertEquals(1, releases.get(), message); + } + + // NOSONAR java:S2925 - see awaitReleases: a bounded settle window is the only way to observe that + // no *further* release lands after the first one on a live event loop. + @SuppressWarnings("java:S2925") + private static void assertNoFurtherRelease(AtomicInteger releases, String message) + throws InterruptedException { + Thread.sleep(250); + assertEquals(1, releases.get(), message); + } + private WebSocket connect(String uri, String origin) throws Exception { WebSocketConnectOptions options = new WebSocketConnectOptions() .setHost("localhost").setPort(frontPort).setURI(uri).addHeader("Origin", origin); From 96d7a2126037e1140a4fc33e5e2ee533f8e71e8e Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:10:07 +0200 Subject: [PATCH 02/11] feat(config): add the edge_hardening admission budget with a WebSocket relay sub-cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A WebSocket relay now holds its admission permit for the connection's whole lifetime, so a modest number of long-lived relays could consume the general pool and starve HTTP — trading the permit leak for a slow squeeze. Bound them separately. Adds the operator-facing edge_hardening block: admission_cap (previously the hard-coded 2048 in EdgeHardeningOptions) and websocket_relay_cap, a sub-budget acquired in addition to the general permit rather than instead of it. Wired through the whole config chain — the EdgeHardeningConfig record, the GatewayConfig component, the JSON schema (whose root additionalProperties:false would otherwise refuse the block at boot), native reflection registration, and a boot validator requiring both caps >= 1 with the relay cap within the pool it draws from. EdgeHardeningOptions is now supplied by a ConfigProducer producer method behind the same buildOnce() guard, so the caps come from the single boot-time assembly; an omitted block resolves to the documented defaults, preserving today's behaviour. GatewayEdgeRoute acquires the sub-permit before the relay hand-off, refuses 503 beyond it, and releases both permits together through the deliverable-1 CAS guard so neither a refusal nor a dial failure can strand one. --- .../config/model/EdgeHardeningConfig.java | 89 +++++++++++++++++++ .../gateway/config/model/GatewayConfig.java | 6 +- .../config/validation/ConfigValidator.java | 33 ++++++- .../gateway/edge/EdgeHardeningOptions.java | 64 ++++++++++--- .../gateway/edge/GatewayEdgeRoute.java | 69 ++++++++++++-- .../quarkus/ConfigModelReflection.java | 2 + .../gateway/quarkus/ConfigProducer.java | 27 +++++- .../main/resources/schema/gateway.schema.json | 9 ++ .../config/model/ConfigModelContractTest.java | 22 ++++- .../validation/ConfigValidatorTest.java | 50 +++++++++++ .../gateway/edge/EdgeHardeningTest.java | 58 ++++++++++-- .../gateway/edge/GatewayEdgeRouteTest.java | 88 +++++++++++++++++- .../gateway/quarkus/ConfigProducerTest.java | 59 ++++++++++++ .../gateway/quarkus/SheriffMetricsTest.java | 2 +- 14 files changed, 537 insertions(+), 41 deletions(-) create mode 100644 api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java new file mode 100644 index 00000000..7c6ec29d --- /dev/null +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java @@ -0,0 +1,89 @@ +/* + * Copyright © 2026 CUI-OpenSource-Software (info@cuioss.de) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package de.cuioss.sheriff.gateway.config.model; + +import java.util.Objects; +import java.util.Optional; + +import lombok.Builder; + +/** + * The operator-facing {@code edge_hardening} block: the gateway's admission budget. + *

+ * Two caps, not one. {@code admission_cap} bounds how many requests may be in flight + * across the whole edge at once. {@code websocket_relay_cap} is a sub-budget acquired in addition + * to that general permit, never instead of it, and exists because a WebSocket relay holds its + * admission permit for the connection's entire lifetime rather than for one request/response + * round-trip. Without the sub-budget a modest number of long-lived relays would consume the general + * pool and starve ordinary HTTP traffic; with it, WebSocket pressure is bounded independently and + * HTTP keeps the remaining headroom. A {@code websocket_relay_cap} larger than {@code admission_cap} + * is meaningless — the sub-budget can never bind — and is refused at boot by the config validator. + *

+ * Both members are optional: an omitted block, or an omitted member, resolves to the + * {@link #defaults() documented defaults}, so an operator who never writes the block keeps today's + * behaviour. Values are validated at boot (both {@code >= 1}); this record only carries them. + * + * @param admissionCap the maximum number of concurrently in-flight requests the edge admits, + * empty when the operator did not declare one + * @param websocketRelayCap the maximum number of concurrently established WebSocket relays, empty + * when the operator did not declare one + * @author API Sheriff Team + * @since 1.0 + */ +@Builder +public record EdgeHardeningConfig(Optional admissionCap, Optional websocketRelayCap) { + + /** The admission cap applied when the operator declares none — the gateway's historical bound. */ + public static final int DEFAULT_ADMISSION_CAP = 2048; + + /** + * The WebSocket-relay sub-budget applied when the operator declares none: a quarter of + * {@link #DEFAULT_ADMISSION_CAP}. A relay holds its permit for the connection's lifetime, so the + * default deliberately leaves three quarters of the general pool for ordinary request/response + * traffic while still admitting far more concurrent relays than a typical deployment sustains. + */ + public static final int DEFAULT_WEBSOCKET_RELAY_CAP = 512; + + /** Canonical constructor normalizing absent optionals to {@link Optional#empty()}. */ + public EdgeHardeningConfig { + admissionCap = Objects.requireNonNullElse(admissionCap, Optional.empty()); + websocketRelayCap = Objects.requireNonNullElse(websocketRelayCap, Optional.empty()); + } + + /** + * @return the block an omitted {@code edge_hardening} resolves to — both caps at their documented + * defaults, preserving the behaviour of a gateway that never declares the block + */ + public static EdgeHardeningConfig defaults() { + return new EdgeHardeningConfig(Optional.of(DEFAULT_ADMISSION_CAP), + Optional.of(DEFAULT_WEBSOCKET_RELAY_CAP)); + } + + /** + * @return the declared admission cap, or {@link #DEFAULT_ADMISSION_CAP} when the member is absent + */ + public int effectiveAdmissionCap() { + return admissionCap.orElse(DEFAULT_ADMISSION_CAP); + } + + /** + * @return the declared WebSocket-relay sub-budget, or {@link #DEFAULT_WEBSOCKET_RELAY_CAP} when + * the member is absent + */ + public int effectiveWebsocketRelayCap() { + return websocketRelayCap.orElse(DEFAULT_WEBSOCKET_RELAY_CAP); + } +} diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.java index 0a378ed1..235c64ff 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/GatewayConfig.java @@ -48,6 +48,8 @@ * @param forwarded the forwarded-header trust policy, empty when omitted * @param tokenValidation the offline bearer-validation settings, empty when omitted * @param oidc the confidential-client settings, empty when omitted + * @param edgeHardening the admission-budget settings ({@code admission_cap} and the WebSocket + * relay sub-budget), empty when omitted — the documented defaults then apply * @author API Sheriff Team * @since 1.0 */ @@ -55,7 +57,8 @@ public record GatewayConfig(int version, Optional metadata, Optional tls, Optional securityHeaders, Optional securityDefaults, List allowedMethods, Map anchors, Optional upstreamDefaults, -Optional forwarded, Optional tokenValidation, Optional oidc) { +Optional forwarded, Optional tokenValidation, Optional oidc, +Optional edgeHardening) { /** * Canonical constructor defensively copying {@code allowedMethods} and @@ -73,5 +76,6 @@ public record GatewayConfig(int version, Optional metadata, Optional validateLoginPath(gateway, errors), (gateway, endpoints, topology, errors) -> validatePassthroughHostCollision(gateway, endpoints, errors), (gateway, endpoints, topology, errors) -> validatePassthroughAliasResolvable(gateway, topology, errors), - (gateway, endpoints, topology, errors) -> validateWebSocketConfig(gateway, endpoints, errors)); + (gateway, endpoints, topology, errors) -> validateWebSocketConfig(gateway, endpoints, errors), + (gateway, endpoints, topology, errors) -> validateEdgeHardening(gateway, errors)); private final List rules; @@ -185,6 +188,34 @@ private static void validateVersion(GatewayConfig gateway, List err } } + /** + * Bounds the operator-facing admission budget. Both caps must admit at least one request — a cap + * of zero or below would refuse every request (or every relay) and is always a misconfiguration + * rather than a deliberate posture. The WebSocket sub-budget must additionally stay within the + * general pool it draws from: a {@code websocket_relay_cap} above {@code admission_cap} can never + * bind, so it would silently disable the sub-cap the operator meant to impose. + */ + private static void validateEdgeHardening(GatewayConfig gateway, List errors) { + gateway.edgeHardening().ifPresent(hardening -> { + hardening.admissionCap() + .filter(cap -> cap < 1) + .ifPresent(cap -> errors.add(new ConfigError(GATEWAY_FILE, EDGE_HARDENING_ADMISSION_POINTER, + "admission_cap must be at least 1 (was %d)".formatted(cap)))); + hardening.websocketRelayCap() + .filter(cap -> cap < 1) + .ifPresent(cap -> errors.add(new ConfigError(GATEWAY_FILE, EDGE_HARDENING_WEBSOCKET_POINTER, + "websocket_relay_cap must be at least 1 (was %d)".formatted(cap)))); + int admission = hardening.effectiveAdmissionCap(); + int relay = hardening.effectiveWebsocketRelayCap(); + if (admission >= 1 && relay >= 1 && relay > admission) { + errors.add(new ConfigError(GATEWAY_FILE, EDGE_HARDENING_WEBSOCKET_POINTER, + "websocket_relay_cap %d must not exceed admission_cap %d — the relay budget is a " + .formatted(relay, admission) + + "sub-budget of the admission pool and could never bind")); + } + }); + } + private static void validateEndpointIdUniqueness(List endpoints, List errors) { Set seen = new HashSet<>(); for (EndpointConfig endpoint : endpoints) { diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java index 30b35f7b..75e994b5 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java @@ -17,9 +17,10 @@ import java.util.concurrent.TimeUnit; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; + import io.quarkus.vertx.http.HttpServerOptionsCustomizer; import io.vertx.core.http.HttpServerOptions; -import jakarta.enterprise.context.ApplicationScoped; /** * Edge hardening for the public data-plane HTTP server. It is BOTH the carrier of the @@ -29,25 +30,31 @@ * The transport bounds fail fast on abusive framing before a request is ever admitted to the * pipeline: an over-long request line, an over-large header block, or an over-large chunk is * rejected by the Vert.x codec, and an idle connection is reaped so a slow-loris / h2 abuse load - * cannot pin connection slots. Three further limits are consumed by {@link GatewayEdgeRoute} rather + * cannot pin connection slots. Four further limits are consumed by {@link GatewayEdgeRoute} rather * than the transport: the {@linkplain #admissionCap() admission cap} bounds the number of requests * that may be in flight at once (rejected with {@code 503} before a virtual thread is * dispatched, so a flood cannot spawn unbounded virtual threads), the - * {@linkplain #drainTimeoutMillis() drain timeout} bounds the graceful-shutdown wait for in-flight - * requests to complete on {@code SIGTERM}, and the + * {@linkplain #webSocketRelayCap() WebSocket relay cap} bounds how many of those in-flight requests + * may be established WebSocket relays, the {@linkplain #drainTimeoutMillis() drain timeout} bounds + * the graceful-shutdown wait for in-flight requests to complete on {@code SIGTERM}, and the * {@linkplain #reservedBodyMaxBytes() reserved-body ceiling} bounds the cumulative bytes the edge * will buffer for a gateway-terminated reserved POST path (rejected with {@code 413}), so an * unauthenticated caller cannot exhaust heap through the pre-authentication callback / back-channel * logout paths that never reach the per-route body cap. *

- * The values are deliberate secure defaults chosen to keep the abuse surface bounded while - * comfortably serving ordinary API traffic; the drain timeout is kept below the Quarkus default + * Transport bounds are fixed; the admission budget is operator-configurable. The + * codec limits above are deliberate secure defaults chosen to keep the abuse surface bounded while + * comfortably serving ordinary API traffic, and the drain timeout is kept below the Quarkus default * shutdown timeout so the drain completes within the shutdown window rather than being cut short. + * The two admission caps instead come from the {@code edge_hardening} block of {@code gateway.yaml} + * ({@link EdgeHardeningConfig}), because a deployment's viable concurrency depends on its container + * memory limit and its traffic mix — and, for the relay cap, on how long its WebSocket connections + * live. An omitted block resolves to {@link EdgeHardeningConfig#defaults()}, preserving the + * historical behaviour. The bean is produced (and its caps resolved) by {@code ConfigProducer}. * * @author API Sheriff Team * @since 1.0 */ -@ApplicationScoped public class EdgeHardeningOptions implements HttpServerOptionsCustomizer { /** Maximum size of the whole request header block in bytes (default 16 KiB). */ @@ -62,9 +69,6 @@ public class EdgeHardeningOptions implements HttpServerOptionsCustomizer { /** Idle-connection reap threshold in seconds — an idle connection is closed after this. */ private static final int IDLE_TIMEOUT_SECONDS = 60; - /** Maximum number of requests permitted in flight at once before a virtual thread is dispatched. */ - private static final int ADMISSION_CAP = 2048; - /** * Ceiling in bytes for a gateway-terminated reserved-path request body (the OIDC * {@code response_mode=form_post} callback and the back-channel logout receiver). @@ -77,14 +81,36 @@ public class EdgeHardeningOptions implements HttpServerOptionsCustomizer { * The ceiling is therefore defined as {@link #MAX_HEADER_SIZE_BYTES} rather than * restated as its own literal, so the gateway's single 16 KiB inbound-unit bound cannot drift * into two contradicting constants. It leaves roughly an order of magnitude of headroom over a - * real payload while bounding the worst case at the {@link #ADMISSION_CAP admission cap} to - * ~32 MiB — comfortably inside the deployed container memory limit. + * real payload while bounding the worst case at the {@linkplain #admissionCap() configured + * admission cap} to that cap times 16 KiB — ~32 MiB at the default cap of + * {@link EdgeHardeningConfig#DEFAULT_ADMISSION_CAP}, comfortably inside the deployed container + * memory limit. An operator who raises the admission cap raises that worst case proportionally. */ private static final int RESERVED_BODY_MAX_BYTES = MAX_HEADER_SIZE_BYTES; /** Bounded graceful-drain wait for in-flight requests on shutdown (below the Quarkus default). */ private static final long DRAIN_TIMEOUT_MILLIS = 25_000L; + private final int admissionCap; + private final int webSocketRelayCap; + + /** + * Resolves the operator-configured admission budget, applying the documented defaults for any + * member the {@code edge_hardening} block leaves out. + * + * @param hardening the bound {@code edge_hardening} block; never {@code null} — a gateway that + * declares no block is supplied {@link EdgeHardeningConfig#defaults()} + */ + public EdgeHardeningOptions(EdgeHardeningConfig hardening) { + this.admissionCap = hardening.effectiveAdmissionCap(); + this.webSocketRelayCap = hardening.effectiveWebsocketRelayCap(); + } + + /** Creates the options carrying the documented default admission budget. */ + public EdgeHardeningOptions() { + this(EdgeHardeningConfig.defaults()); + } + @Override public void customizeHttpServer(HttpServerOptions options) { apply(options); @@ -108,7 +134,19 @@ private static void apply(HttpServerOptions options) { * virtual thread is dispatched; a request beyond the cap is rejected {@code 503} */ public int admissionCap() { - return ADMISSION_CAP; + return admissionCap; + } + + /** + * @return the maximum number of concurrently established WebSocket relays the edge admits. This + * is a sub-budget acquired in addition to the general admission permit, never + * instead of it: a relay holds its admission permit for the connection's whole lifetime, + * so without a separate bound a handful of long-lived relays would consume the general + * pool and starve ordinary HTTP traffic. An upgrade beyond the cap is rejected + * {@code 503} + */ + public int webSocketRelayCap() { + return webSocketRelayCap; } /** diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java index 823655c1..c12039b6 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java @@ -195,6 +195,10 @@ public class GatewayEdgeRoute { * a completed upgrade takes the connection over, so the HTTP end handler never fires and * {@link #dispatchWebSocket} must hand the relay its own release callback. */ private static final String ADMISSION_GUARD_KEY = "sheriff.admissionguard"; + /** Holds the per-request release guard for the WebSocket relay sub-permit. Present ONLY once + * {@link #dispatchWebSocket} has actually acquired that sub-permit, so its absence is how every + * release site knows there is no sub-permit to return. */ + private static final String WEBSOCKET_RELAY_GUARD_KEY = "sheriff.wsrelayguard"; private final List routes; private final ExecutorService virtualThreadExecutor; @@ -223,6 +227,13 @@ public class GatewayEdgeRoute { private final GrpcStatusMapper grpcStatusMapper; private final Semaphore admission; + /** + * The WebSocket relay sub-budget: acquired in addition to {@link #admission}, never + * instead of it. A relay holds its general admission permit for the connection's whole lifetime, + * so without this second bound a handful of long-lived relays would consume the general pool and + * starve ordinary HTTP traffic — trading the permit leak for a slow squeeze. + */ + private final Semaphore webSocketRelayAdmission; private final AtomicInteger inFlight = new AtomicInteger(); private volatile boolean draining; @@ -260,6 +271,7 @@ public GatewayEdgeRoute(RouteTable routeTable, GatewayConfig gatewayConfig, this.sheriffMetrics = sheriffMetrics; this.bffRuntime = bffRuntime; this.admission = new Semaphore(hardening.admissionCap()); + this.webSocketRelayAdmission = new Semaphore(hardening.webSocketRelayCap()); SecurityEventCounter securityEventCounter = new SecurityEventCounter(); SecurityConfiguration defaultConfiguration = SecurityConfiguration.defaults(); @@ -369,7 +381,7 @@ private void handle(RoutingContext ctx) { AtomicBoolean admissionReleased = new AtomicBoolean(); ctx.put(ADMISSION_GUARD_KEY, admissionReleased); ctx.addEndHandler(result -> { - releaseAdmission(admissionReleased); + releaseAdmission(ctx, admissionReleased); recordRequestMetrics(ctx, startNanos); }); if (needsReservedBodyRead(ctx)) { @@ -543,24 +555,43 @@ private void dispatchProcessing(RoutingContext ctx, AtomicBoolean admissionRelea // NullPointerException arm is unreachable), and process(ctx) runs on the virtual thread // rather than inline, so no pipeline exception can surface here. LOGGER.debug(rejected, "Virtual-thread dispatch rejected: %s", rejected.getMessage()); - releaseAdmission(admissionReleased); + releaseAdmission(ctx, admissionReleased); reject(ctx, SERVICE_UNAVAILABLE); } } /** - * Releases one admission permit and decrements the in-flight counter exactly once, guarded by - * {@code released} so the normal end-handler path, the executor-rejection rollback path and the - * WebSocket relay-teardown callback (see {@link #dispatchWebSocket}) can all call it without - * double-counting. + * Releases this request's admission permits and decrements the in-flight counter exactly once, + * guarded by {@code released} so the normal end-handler path, the executor-rejection rollback + * path and the WebSocket relay-teardown callback (see {@link #dispatchWebSocket}) can all call it + * without double-counting. + *

+ * The general permit and the WebSocket relay sub-permit are an ordered pair that must be returned + * together: a request that acquired both would otherwise strand the sub-permit whenever the + * general permit is released by a different site than the relay callback (the upstream-dial + * failure ends the HTTP response, so the end handler gets there first). Both are therefore + * released here, each behind its own idempotence latch. */ - private void releaseAdmission(AtomicBoolean released) { + private void releaseAdmission(RoutingContext ctx, AtomicBoolean released) { + releaseWebSocketRelayPermit(ctx); if (released.compareAndSet(false, true)) { admission.release(); inFlight.decrementAndGet(); } } + /** + * Returns the WebSocket relay sub-permit exactly once, and only for a request that actually + * acquired one — the guard is stashed on the context by {@link #dispatchWebSocket} at acquisition, + * so an absent guard means an HTTP/gRPC request (or a refused upgrade) with nothing to return. + */ + private void releaseWebSocketRelayPermit(RoutingContext ctx) { + AtomicBoolean relayReleased = ctx.get(WEBSOCKET_RELAY_GUARD_KEY); + if (relayReleased != null && relayReleased.compareAndSet(false, true)) { + webSocketRelayAdmission.release(); + } + } + /** * Records the terminal {@link SheriffMetrics#REQUESTS_TOTAL request count} and * {@link SheriffMetrics#REQUEST_DURATION_SECONDS request-duration timer} for one served request @@ -841,6 +872,13 @@ private void dispatchAndRelay(RoutingContext ctx, PipelineRequest request, Route * callback, which the relay invokes on each of its teardown paths. Routing the release through the * same guard keeps it idempotent against the end handler and the executor-rejection rollback, so an * upstream-dial failure (which does end the response) can never double-release. + *

+ * Relay sub-budget. Because that permit is held for the connection's lifetime, an + * upgrade must additionally acquire the {@linkplain EdgeHardeningOptions#webSocketRelayCap() + * WebSocket relay sub-budget} before the hand-off — a second permit taken in addition to + * the general one, so long-lived relays cannot squeeze ordinary HTTP traffic out of the admission + * pool. An upgrade beyond that cap is refused {@code 503}, releasing the general permit through + * the shared guard on the way out so a refusal strands nothing. */ private void dispatchWebSocket(RoutingContext ctx, PipelineRequest request, RouteRuntime route, ForwardPolicyStage.Result forward) { @@ -852,11 +890,24 @@ private void dispatchWebSocket(RoutingContext ctx, PipelineRequest request, Rout ResolvedUpstream upstreamTarget = route.getUpstream() .orElseThrow(() -> new IllegalStateException("WebSocket dispatch requires a resolved upstream")); String uri = DispatchStage.upstreamRequestUri(upstreamTarget, remainder, query); - applyStageSetCookies(ctx.response(), request.responseSetCookies()); AtomicBoolean admissionGuard = Objects.requireNonNull(ctx.get(ADMISSION_GUARD_KEY), "admission guard missing — handle() must stash it before dispatch"); + if (!webSocketRelayAdmission.tryAcquire()) { + // The relay sub-budget is exhausted. The general admission permit is still held and this + // request will never reach the relay teardown that would return it, so release it here — + // through the shared guard, so the 503 response's own end handler cannot double-release. + LOGGER.debug("WebSocket relay budget exhausted on route '%s' — refusing the upgrade", + route.getId()); + releaseAdmission(ctx, admissionGuard); + ctx.vertx().runOnContext(v -> reject(ctx, SERVICE_UNAVAILABLE)); + return; + } + // Stashed only now, on the acquired path: every release site keys off its presence to decide + // whether there is a sub-permit to return. + ctx.put(WEBSOCKET_RELAY_GUARD_KEY, new AtomicBoolean()); + applyStageSetCookies(ctx.response(), request.responseSetCookies()); webSocketRelayStage.relay(ctx, route, forward.headers(), request.responseHeaders(), uri, - () -> releaseAdmission(admissionGuard)); + () -> releaseAdmission(ctx, admissionGuard)); } /** diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigModelReflection.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigModelReflection.java index 87bc2d15..0bfbb826 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigModelReflection.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigModelReflection.java @@ -20,6 +20,7 @@ import de.cuioss.sheriff.gateway.config.model.AnchorType; import de.cuioss.sheriff.gateway.config.model.AssetConfig; import de.cuioss.sheriff.gateway.config.model.AuthConfig; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import de.cuioss.sheriff.gateway.config.model.EndpointConfig; import de.cuioss.sheriff.gateway.config.model.ForwardConfig; import de.cuioss.sheriff.gateway.config.model.ForwardedConfig; @@ -71,6 +72,7 @@ UpstreamDefaultsConfig.class, ForwardedConfig.class, ForwardConfig.class, + EdgeHardeningConfig.class, TokenValidationConfig.class, IssuerConfig.class, IssuerConfig.Jwks.class, diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducer.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducer.java index 8acff3d2..77be87cc 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducer.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducer.java @@ -29,6 +29,7 @@ import de.cuioss.sheriff.gateway.config.load.ConfigLoader; import de.cuioss.sheriff.gateway.config.load.EnvSecretResolver; import de.cuioss.sheriff.gateway.config.model.AssetConfig; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import de.cuioss.sheriff.gateway.config.model.EndpointConfig; import de.cuioss.sheriff.gateway.config.model.GatewayConfig; import de.cuioss.sheriff.gateway.config.model.Metadata; @@ -38,6 +39,7 @@ import de.cuioss.sheriff.gateway.config.model.TlsConfig; import de.cuioss.sheriff.gateway.config.topology.TopologyResolver; import de.cuioss.sheriff.gateway.config.validation.ConfigValidator; +import de.cuioss.sheriff.gateway.edge.EdgeHardeningOptions; import de.cuioss.tools.logging.CuiLogger; import io.quarkus.runtime.StartupEvent; @@ -59,8 +61,8 @@ * {@link ConfigLogMessages} records and throws, so Quarkus exits non-zero and never * serves on partial configuration. On success it emits the {@code CONFIG_LOADED} * INFO record carrying the audit {@code config_version} and publishes the bound - * {@link GatewayConfig} and assembled {@link RouteTable} as {@code @ApplicationScoped} - * beans. + * {@link GatewayConfig}, the assembled {@link RouteTable}, the {@link ResolvedTopology} + * and the resolved {@link EdgeHardeningOptions} admission budget as beans. * * @author API Sheriff Team * @since 1.0 @@ -143,6 +145,27 @@ public ResolvedTopology resolvedTopology() { return resolvedTopology; } + /** + * Produces the edge's transport bounds and admission budget, resolving the two operator-facing + * caps from the {@code edge_hardening} block and falling back to + * {@link EdgeHardeningConfig#defaults()} when the block is absent. + *

+ * Produced here rather than declared as a bean on the class itself so the whole admission budget + * comes from the single boot-time assembly this producer guards — one configuration entry point, + * not two. {@code ApplicationScoped} is exact: {@link EdgeHardeningOptions} is a non-final class, + * so ArC can build the client proxy a normal scope requires, and the bean also carries + * {@code HttpServerOptionsCustomizer} in its bean types so the transport-customizer SPI still + * discovers it. + * + * @return the immutable {@link EdgeHardeningOptions} for this deployment + */ + @Produces + @ApplicationScoped + public EdgeHardeningOptions edgeHardeningOptions() { + buildOnce(); + return new EdgeHardeningOptions(gateway.edgeHardening().orElseGet(EdgeHardeningConfig::defaults)); + } + private synchronized void buildOnce() { if (built) { return; diff --git a/api-sheriff/src/main/resources/schema/gateway.schema.json b/api-sheriff/src/main/resources/schema/gateway.schema.json index a7fb8c40..7571fdd1 100644 --- a/api-sheriff/src/main/resources/schema/gateway.schema.json +++ b/api-sheriff/src/main/resources/schema/gateway.schema.json @@ -170,6 +170,15 @@ "emit": { "type": "string", "enum": ["x-forwarded", "both"] } } }, + "edge_hardening": { + "type": "object", + "additionalProperties": false, + "description": "Admission budget for the public data-plane edge. websocket_relay_cap is a sub-budget acquired in addition to the general admission permit (a relay holds its permit for the connection's whole lifetime) and must not exceed admission_cap.", + "properties": { + "admission_cap": { "type": "integer", "minimum": 1 }, + "websocket_relay_cap": { "type": "integer", "minimum": 1 } + } + }, "token_validation": { "type": "object", "additionalProperties": false, diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java index 29b65e0f..4e2df441 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java @@ -420,7 +420,7 @@ void anchorConfigBuilderMatchesConstructor() { void gatewayConfigBuilderMatchesConstructor() { GatewayConfig viaCtor = new GatewayConfig(2, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), List.of(HttpMethod.GET), Map.of("api", anchorConfig()), Optional.empty(), - Optional.empty(), Optional.empty(), Optional.empty()); + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); GatewayConfig viaBuilder = GatewayConfig.builder().version(2).allowedMethods(List.of(HttpMethod.GET)) .anchors(Map.of("api", anchorConfig())).build(); assertEquals(viaCtor, viaBuilder); @@ -445,7 +445,7 @@ class NullNormalization { @Test void gatewayConfigNormalizesAllAbsentComponents() { - GatewayConfig cfg = new GatewayConfig(1, null, null, null, null, null, null, null, null, null, null); + GatewayConfig cfg = new GatewayConfig(1, null, null, null, null, null, null, null, null, null, null, null); assertTrue(cfg.metadata().isEmpty()); assertTrue(cfg.tls().isEmpty()); assertTrue(cfg.securityHeaders().isEmpty()); @@ -456,6 +456,24 @@ void gatewayConfigNormalizesAllAbsentComponents() { assertTrue(cfg.forwarded().isEmpty()); assertTrue(cfg.tokenValidation().isEmpty()); assertTrue(cfg.oidc().isEmpty()); + assertTrue(cfg.edgeHardening().isEmpty()); + } + + @Test + void edgeHardeningConfigNormalizesAbsentCaps() { + EdgeHardeningConfig cfg = new EdgeHardeningConfig(null, null); + assertTrue(cfg.admissionCap().isEmpty()); + assertTrue(cfg.websocketRelayCap().isEmpty()); + assertEquals(EdgeHardeningConfig.DEFAULT_ADMISSION_CAP, cfg.effectiveAdmissionCap()); + assertEquals(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP, cfg.effectiveWebsocketRelayCap()); + } + + @Test + void edgeHardeningConfigDefaultsCarryBothCaps() { + EdgeHardeningConfig defaults = EdgeHardeningConfig.defaults(); + assertEquals(Optional.of(EdgeHardeningConfig.DEFAULT_ADMISSION_CAP), defaults.admissionCap()); + assertEquals(Optional.of(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP), + defaults.websocketRelayCap()); } @Test diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java index fb0ecd5b..d4e3d3b0 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java @@ -34,6 +34,7 @@ import de.cuioss.sheriff.gateway.config.model.AnchorType; import de.cuioss.sheriff.gateway.config.model.AssetConfig; import de.cuioss.sheriff.gateway.config.model.AuthConfig; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import de.cuioss.sheriff.gateway.config.model.EndpointConfig; import de.cuioss.sheriff.gateway.config.model.ForwardedConfig; import de.cuioss.sheriff.gateway.config.model.GatewayConfig; @@ -391,6 +392,55 @@ void shouldRejectUnsupportedVersion() { assertHasError(errors, "/version", "unsupported config version"); } + @Test + @DisplayName("Should reject an edge_hardening admission_cap below 1") + void shouldRejectAdmissionCapBelowOne() { + GatewayConfig gateway = validGateway() + .edgeHardening(Optional.of(new EdgeHardeningConfig(Optional.of(0), Optional.of(1)))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS", List.of(), route("r", HttpMethod.GET)); + + List errors = validator.validate(gateway, List.of(endpoint), topologyWith("ORDERS")); + + assertHasError(errors, "/edge_hardening/admission_cap", "admission_cap must be at least 1"); + } + + @Test + @DisplayName("Should reject an edge_hardening websocket_relay_cap below 1") + void shouldRejectWebsocketRelayCapBelowOne() { + GatewayConfig gateway = validGateway() + .edgeHardening(Optional.of(new EdgeHardeningConfig(Optional.of(8), Optional.of(0)))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS", List.of(), route("r", HttpMethod.GET)); + + List errors = validator.validate(gateway, List.of(endpoint), topologyWith("ORDERS")); + + assertHasError(errors, "/edge_hardening/websocket_relay_cap", + "websocket_relay_cap must be at least 1"); + } + + @Test + @DisplayName("Should reject a websocket_relay_cap exceeding admission_cap, which could never bind") + void shouldRejectInvertedCapPair() { + GatewayConfig gateway = validGateway() + .edgeHardening(Optional.of(new EdgeHardeningConfig(Optional.of(4), Optional.of(16)))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS", List.of(), route("r", HttpMethod.GET)); + + List errors = validator.validate(gateway, List.of(endpoint), topologyWith("ORDERS")); + + assertHasError(errors, "/edge_hardening/websocket_relay_cap", "must not exceed admission_cap"); + } + + @Test + @DisplayName("Should accept a well-formed edge_hardening block") + void shouldAcceptValidEdgeHardeningBlock() { + GatewayConfig gateway = validGateway() + .edgeHardening(Optional.of(new EdgeHardeningConfig(Optional.of(64), Optional.of(8)))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS", List.of(), route("r", HttpMethod.GET)); + + List errors = validator.validate(gateway, List.of(endpoint), topologyWith("ORDERS")); + + assertTrue(errors.isEmpty(), "A valid admission budget raises no violation, but got: " + errors); + } + @Test @DisplayName("Should reject a duplicate endpoint id across endpoint files") void shouldRejectDuplicateEndpointId() { diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java index 222d3433..3601b120 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java @@ -18,8 +18,11 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Optional; import java.util.concurrent.TimeUnit; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; + import io.vertx.core.http.HttpServerOptions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -31,7 +34,6 @@ class EdgeHardeningTest { private static final int MAX_INITIAL_LINE_LENGTH_BYTES = 8 * 1024; private static final int MAX_CHUNK_SIZE_BYTES = 16 * 1024; private static final int IDLE_TIMEOUT_SECONDS = 60; - private static final int ADMISSION_CAP = 2048; private static final long DRAIN_TIMEOUT_MILLIS = 25_000L; @Test @@ -95,18 +97,56 @@ void appliesSameBoundsToHttpsListener() { } @Test - @DisplayName("bounds in-flight requests with a positive admission cap") - void exposesPositiveAdmissionCap() { - // Arrange + @DisplayName("bounds in-flight requests and WebSocket relays with the documented default budget") + void exposesDefaultAdmissionBudget() { + // Arrange — the no-arg construction is the documented defaults path a gateway that declares + // no edge_hardening block resolves to EdgeHardeningOptions hardening = new EdgeHardeningOptions(); // Act - int cap = hardening.admissionCap(); + int admissionCap = hardening.admissionCap(); + int relayCap = hardening.webSocketRelayCap(); + + // Assert — a request beyond the admission cap is rejected 503 before a virtual thread is + // dispatched, so a flood cannot spawn unbounded virtual threads; the relay cap is a strictly + // smaller sub-budget, so long-lived relays cannot consume the whole pool. + assertEquals(EdgeHardeningConfig.DEFAULT_ADMISSION_CAP, admissionCap, + "The admission cap is the documented default"); + assertEquals(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP, relayCap, + "The WebSocket relay cap is the documented default"); + assertTrue(admissionCap > 0, "The admission cap must be a positive bound"); + assertTrue(relayCap > 0, "The WebSocket relay cap must be a positive bound"); + assertTrue(relayCap <= admissionCap, + "The relay sub-budget must stay within the admission pool it draws from"); + } + + @Test + @DisplayName("carries the operator-configured caps when the edge_hardening block declares them") + void carriesOperatorConfiguredCaps() { + // Arrange + EdgeHardeningConfig configured = new EdgeHardeningConfig(Optional.of(64), Optional.of(8)); + + // Act + EdgeHardeningOptions hardening = new EdgeHardeningOptions(configured); + + // Assert + assertEquals(64, hardening.admissionCap(), "The declared admission_cap is carried through"); + assertEquals(8, hardening.webSocketRelayCap(), "The declared websocket_relay_cap is carried through"); + } + + @Test + @DisplayName("falls back per member, so a half-declared block keeps the default for the omitted cap") + void fallsBackPerOmittedMember() { + // Arrange — only the admission cap is declared + EdgeHardeningConfig partial = new EdgeHardeningConfig(Optional.of(4096), Optional.empty()); + + // Act + EdgeHardeningOptions hardening = new EdgeHardeningOptions(partial); - // Assert — a request beyond the cap is rejected 503 before a virtual thread is dispatched, - // so a flood cannot spawn unbounded virtual threads. - assertEquals(ADMISSION_CAP, cap, "The admission cap is the documented secure default"); - assertTrue(cap > 0, "The admission cap must be a positive bound"); + // Assert — the omitted member resolves to its documented default rather than to zero + assertEquals(4096, hardening.admissionCap(), "The declared admission_cap is carried through"); + assertEquals(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP, hardening.webSocketRelayCap(), + "An omitted websocket_relay_cap resolves to the documented default"); } @Test diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java index 337431ad..65d746cb 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -28,6 +29,7 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -36,6 +38,7 @@ import de.cuioss.sheriff.gateway.bff.runtime.BffRuntime; import de.cuioss.sheriff.gateway.config.model.AuthConfig; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import de.cuioss.sheriff.gateway.config.model.GatewayConfig; import de.cuioss.sheriff.gateway.config.model.HttpMethod; import de.cuioss.sheriff.gateway.config.model.MatchConfig; @@ -54,6 +57,7 @@ import io.vertx.core.http.HttpClient; import io.vertx.core.http.HttpClientRequest; import io.vertx.core.http.HttpServer; +import io.vertx.core.http.UpgradeRejectedException; import io.vertx.core.http.WebSocket; import io.vertx.core.http.WebSocketClient; import io.vertx.core.http.WebSocketConnectOptions; @@ -65,6 +69,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.opentest4j.AssertionFailedError; /** * Boot-time and lifecycle contract of the public data-plane edge. The per-request serving behaviour @@ -238,9 +243,7 @@ void webSocketBranchReleasesThroughTheStashedGuard() throws Exception { HttpServer front = startFront(new RouteTable(List.of(webSocketRoute(upstream.actualPort()))), stashed); WebSocketClient client = vertx.createWebSocketClient(); try { - WebSocket socket = client.connect(new WebSocketConnectOptions() - .setHost("localhost").setPort(front.actualPort()).setURI("/w/room")) - .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + WebSocket socket = connectWs(client, front.actualPort()); AtomicBoolean guard = assertInstanceOf(AtomicBoolean.class, stashed.get(15, TimeUnit.SECONDS), "the WebSocket request stashes the same release guard"); assertFalse(guard.get(), @@ -260,6 +263,54 @@ void webSocketBranchReleasesThroughTheStashedGuard() throws Exception { } } + @Test + @DisplayName("bounds concurrent relays by the sub-budget and returns both permits at teardown") + void boundsConcurrentRelaysByTheSubBudget() throws Exception { + // Arrange — admission_cap 2 with a websocket_relay_cap of 1, so a single established relay + // exhausts the sub-budget while leaving one general permit for ordinary traffic + HttpServer upstream = vertx.createHttpServer() + .webSocketHandler(ws -> ws.textMessageHandler(ws::writeTextMessage)) + .listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + Router router = Router.router(vertx); + new GatewayEdgeRoute(new RouteTable(List.of(webSocketRoute(upstream.actualPort()))), gatewayConfig, + new SingletonInstance<>(tokenValidator), vertx, virtualThreadExecutor, + new EdgeHardeningOptions(new EdgeHardeningConfig(Optional.of(2), Optional.of(1))), + new SheriffMetrics(new SimpleMeterRegistry()), BffRuntime.inert()).registerRoutes(router); + HttpServer front = vertx.createHttpServer().requestHandler(router) + .listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + WebSocketClient wsClient = vertx.createWebSocketClient(); + HttpClient httpClient = vertx.createHttpClient(); + try { + WebSocket held = connectWs(wsClient, front.actualPort()); + + // Act + Assert — further upgrades are refused while the one relay slot is occupied + for (int attempt = 0; attempt < 5; attempt++) { + ExecutionException refused = assertThrows(ExecutionException.class, + () -> connectWs(wsClient, front.actualPort())); + assertEquals(503, + assertInstanceOf(UpgradeRejectedException.class, refused.getCause()).getStatus(), + "an upgrade beyond the relay sub-budget is refused 503"); + } + + // Assert — each refusal returned the general permit it had already taken; had it not, + // five refusals would have drained the two-permit pool and this would answer 503 + assertEquals(404, statusOf(httpClient, front.actualPort()), + "a refused upgrade releases the general admission permit it was holding"); + + // Act — tearing the relay down must return the sub-permit too + held.close().toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + + // Assert + connectWhenAdmitted(wsClient, front.actualPort()) + .close().toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + } finally { + httpClient.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + wsClient.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + front.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + upstream.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS); + } + } + private HttpServer startFront(RouteTable table, CompletableFuture stashed) throws Exception { Router router = Router.router(vertx); // Registered before the edge, which adds its catch-all last: ctx.next() runs handle() @@ -274,6 +325,37 @@ private HttpServer startFront(RouteTable table, CompletableFuture stashe } } + private WebSocket connectWs(WebSocketClient client, int port) throws Exception { + return client.connect(new WebSocketConnectOptions() + .setHost("localhost").setPort(port).setURI("/w/room")) + .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); + } + + /** + * Retries the upgrade until the relay sub-permit released at teardown becomes visible. The release + * lands on the Vert.x event loop after the socket close round-trips, so the first attempt can + * legitimately still see the exhausted budget. + */ + // NOSONAR java:S2925 - Thread.sleep is load-bearing: see awaitReleased. + @SuppressWarnings("java:S2925") + private WebSocket connectWhenAdmitted(WebSocketClient client, int port) throws Exception { + for (int attempt = 0; attempt < 200; attempt++) { + try { + return connectWs(client, port); + } catch (ExecutionException refused) { + Thread.sleep(25); + } + } + throw new AssertionFailedError( + "no upgrade was admitted after teardown — the relay sub-permit was never returned"); + } + + private static int statusOf(HttpClient client, int port) throws Exception { + return client.request(io.vertx.core.http.HttpMethod.GET, port, "localhost", "/unmatched") + .compose(HttpClientRequest::send) + .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS).statusCode(); + } + // NOSONAR java:S2925 - Thread.sleep is load-bearing: relay teardown is a real socket round-trip on // a live Vert.x event loop with no virtual clock to advance, so the guard is polled until it flips. @SuppressWarnings("java:S2925") diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducerTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducerTest.java index c73f162d..78e834d9 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducerTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/ConfigProducerTest.java @@ -28,10 +28,12 @@ import de.cuioss.sheriff.gateway.config.model.AssetConfig; +import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import de.cuioss.sheriff.gateway.config.model.GatewayConfig; import de.cuioss.sheriff.gateway.config.model.ResolvedAsset; import de.cuioss.sheriff.gateway.config.model.ResolvedUpstream; import de.cuioss.sheriff.gateway.config.model.RouteTable; +import de.cuioss.sheriff.gateway.edge.EdgeHardeningOptions; import de.cuioss.test.juli.LogAsserts; import de.cuioss.test.juli.TestLogLevel; import de.cuioss.test.juli.junit5.EnableTestLogger; @@ -102,6 +104,31 @@ class ConfigProducerTest { access: public """; + /** A gateway declaring the operator-facing admission budget with both caps set. */ + private static final String GATEWAY_WITH_EDGE_HARDENING = """ + version: 1 + metadata: + config_version: "2026-07-13" + edge_hardening: + admission_cap: 64 + websocket_relay_cap: 8 + """; + + /** + * A gateway whose WebSocket sub-budget exceeds the admission pool it draws from. The document is + * schema-valid — both members are integers at or above the schema minimum — so the violation is + * caught by {@code ConfigValidator} at boot, not by the schema, which is what makes it exercise + * the whole bind → validate chain for the new block. + */ + private static final String GATEWAY_WITH_INVERTED_CAPS = """ + version: 1 + metadata: + config_version: "2026-07-13" + edge_hardening: + admission_cap: 4 + websocket_relay_cap: 16 + """; + @TempDir Path configDir; @@ -137,6 +164,38 @@ void shouldProduceBeansFromValidConfig() throws Exception { assertTrue(routeTable.routes().isEmpty(), "a config with no endpoints yields an empty route table"); } + @Test + void shouldProduceTheConfiguredAdmissionBudget() throws Exception { + ConfigProducer producer = producerForGateway(GATEWAY_WITH_EDGE_HARDENING); + + EdgeHardeningOptions hardening = producer.edgeHardeningOptions(); + + assertEquals(64, hardening.admissionCap(), + "the declared edge_hardening.admission_cap reaches the edge"); + assertEquals(8, hardening.webSocketRelayCap(), + "the declared edge_hardening.websocket_relay_cap reaches the edge"); + } + + @Test + void shouldProduceTheDefaultAdmissionBudgetWhenTheBlockIsOmitted() throws Exception { + ConfigProducer producer = producerForValidConfig(); + + EdgeHardeningOptions hardening = producer.edgeHardeningOptions(); + + assertEquals(EdgeHardeningConfig.DEFAULT_ADMISSION_CAP, hardening.admissionCap(), + "an omitted edge_hardening block preserves the documented default admission cap"); + assertEquals(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP, hardening.webSocketRelayCap(), + "an omitted edge_hardening block preserves the documented default relay cap"); + } + + @Test + void shouldRefuseBootWhenTheRelayCapExceedsTheAdmissionCap() throws Exception { + ConfigProducer producer = producerForGateway(GATEWAY_WITH_INVERTED_CAPS); + + assertThrows(IllegalStateException.class, () -> producer.onStartup(null), + "a relay sub-budget larger than the admission pool aborts boot rather than serving"); + } + @Test void shouldLogConfigLoadedOnSuccess() throws Exception { ConfigProducer producer = producerForValidConfig(); diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.java index 4c37a2b2..483c22e2 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.java @@ -306,7 +306,7 @@ private Optional serverMode() { private GatewayConfig configWith(Optional metadata, Optional tokenValidation, Optional oidc) { return new GatewayConfig(1, metadata, Optional.empty(), Optional.empty(), Optional.empty(), - null, null, Optional.empty(), Optional.empty(), tokenValidation, oidc); + null, null, Optional.empty(), Optional.empty(), tokenValidation, oidc, Optional.empty()); } } From d1fd8063d0d2aa702bc2f02099cb8756dbcb9baa Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:32:44 +0200 Subject: [PATCH 03/11] test(it): prove the WebSocket relay returns its admission permits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven passing WebSocket tests coexisted with a permit leak on every established relay, because each performs at most one handshake and asserts only that one response — three leaked permits against a 2048-permit pool is invisible by construction. A per-request correctness assertion cannot observe cumulative resource exhaustion. Adds the assertion of the shape that can: N sequential relays, then a claim about the edge's remaining capacity. It needs a budget small enough to exhaust, and the admission budget is a property of the whole gateway process, so the caps live on a dedicated low-cap instance (api-sheriff-ws-admission, port 10447, admission_cap 8 / websocket_relay_cap 2) rather than on the primary instance that serves the rest of the suite. Ten sequential upgrades exceed both caps, so a leak of either permit fails the run; a following plain HTTP request proves the general permit came back too. The relay releases both permits before closing either leg, so awaiting the close frame makes each iteration race-free rather than timed. WsAdmissionActivationWiringTest is the guard against that regression quietly stopping: it is a no-Docker descriptor assertion that the overlay is mounted, the instance is defined, and both caps stay below the ten upgrades actually driven — because against the shipped defaults the exhaustion test passes whether or not the permits are ever returned, and no black-box assertion inside the IT can tell the difference. Also records the blind-spot analysis in the suite's own javadoc, and corrects the benchmark lane's stack-startup budget comment, which still claimed four gateway instances. --- .github/workflows/benchmark.yml | 15 +- integration-tests/docker-compose.yml | 75 +++++++ integration-tests/pom.xml | 9 + .../scripts/dump-keycloak-logs.sh | 13 +- .../scripts/start-integration-container.sh | 38 ++-- .../sheriff-config-ws-admission/gateway.yaml | 141 +++++++++++++ .../gateway/integration/WebSocketProxyIT.java | 93 +++++++++ .../WsAdmissionActivationWiringTest.java | 189 ++++++++++++++++++ 8 files changed, 546 insertions(+), 27 deletions(-) create mode 100644 integration-tests/src/main/docker/sheriff-config-ws-admission/gateway.yaml create mode 100644 integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WsAdmissionActivationWiringTest.java diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 7b862118..82b40f2c 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -43,11 +43,16 @@ jobs: # that: it is transfer-bound at reduced concurrency, so it does not finish # in the same wall time as a request-rate run. Re-derive this term when a # skip property is flipped to false. - # * stack startup ~90 s -- since #118 the lane boots FOUR native gateway instances - # (api-sheriff, api-sheriff-mtls, api-sheriff-cookie, api-sheriff-cookie-2) - # alongside Keycloak, go-httpbin, nginx-static, passthrough-backend, - # grpc-echo, toxiproxy, asset-origin and prometheus, because - # start-integration-container.sh runs a bare `up -d`. + # * stack startup ~100 s -- the lane boots FIVE native gateway instances (api-sheriff, + # api-sheriff-mtls, api-sheriff-cookie, api-sheriff-cookie-2 and + # api-sheriff-ws-admission, the last added for the WebSocket relay-permit + # exhaustion regression) alongside Keycloak, go-httpbin, nginx-static, + # passthrough-backend, grpc-echo, toxiproxy, asset-origin and prometheus, + # because start-integration-container.sh runs a bare `up -d`. Each added + # gateway instance costs roughly another 10 s of the readiness wait — they + # share one native image, so the term grows with instance count, not with + # image builds. Re-derive this term whenever an api-sheriff* service is + # added to or removed from integration-tests/docker-compose.yml. # * native compile the dominant and most variable term on a cold cache. # # 75 minutes holds with headroom for the cold native compile; it is kept unchanged because the diff --git a/integration-tests/docker-compose.yml b/integration-tests/docker-compose.yml index e8cb6ac6..1ef935b8 100644 --- a/integration-tests/docker-compose.yml +++ b/integration-tests/docker-compose.yml @@ -536,6 +536,81 @@ services: - api-sheriff restart: unless-stopped + # --- The low-admission-budget instance backing the relay-permit exhaustion regression ---------- + # The admission budget is a property of the whole gateway process, and the production-shaped + # defaults the other instances run with (2048 general permits, 512 relay permits) are far too + # large for a test to exhaust — proving the WebSocket relay returns its permits at teardown would + # need thousands of sequential upgrades. This instance reuses the SAME native image and overlays a + # gateway.yaml whose edge_hardening block shrinks the caps to single digits (admission_cap 8, + # websocket_relay_cap 2), turning that proof into a handful of connections. It is separate because + # an 8-request admission budget cannot serve the rest of the IT suite; published on 10447. + api-sheriff-ws-admission: + image: "api-sheriff:distroless" + # Same JVM-default-truststore trust as every other instance — see the primary api-sheriff + # service's command comment for the mechanism (token-sheriff-client#597). + command: + - -Djavax.net.ssl.trustStore=/app/certificates/localhost-truststore.p12 + - -Djavax.net.ssl.trustStorePassword=localhost-trust + - -Djavax.net.ssl.trustStoreType=PKCS12 + ports: + - "10447:8443" # External test port for the exhaustion regression (test.ws.admission.port) + - "19004:9000" # Management interface (health/metrics, HTTPS — single port, see below) + environment: + - QUARKUS_PROFILE=it + - LOG_FILE_PATH=/logs/quarkus-ws-admission.log + # Terminate directly on the public port (no passthrough front on this instance). + - QUARKUS_HTTP_SSL_CERTIFICATE_FILES=/app/certificates/localhost.crt + - QUARKUS_HTTP_SSL_CERTIFICATE_KEY_FILES=/app/certificates/localhost.key + # Management-interface certificate, kept in lockstep with every other instance — see the + # primary api-sheriff service's comment for the single-port mechanism. Without it this + # instance's management port 19004 stays plain HTTP while the others are HTTPS, and the + # host-side readiness wait in start-integration-container.sh (https + -k for all) would hang. + - QUARKUS_MANAGEMENT_SSL_CERTIFICATE_FILES=/app/certificates/localhost.crt + - QUARKUS_MANAGEMENT_SSL_CERTIFICATE_KEY_FILES=/app/certificates/localhost.key + - QUARKUS_TLS_DEFAULT_TRUST__STORE_P12_PATH=/app/certificates/localhost-truststore.p12 + - QUARKUS_TLS_DEFAULT_TRUST__STORE_P12_PASSWORD=localhost-trust + - SHERIFF_CONFIG_DIR=/app/sheriff-config + # Container-side value the bare ${OIDC_CLIENT_SECRET} reference in the overlaid gateway.yaml + # (oidc.client_secret) resolves to via EnvSecretResolver — kept in lockstep with the primary + # api-sheriff service, whose gateway.yaml has the same oidc block. + - OIDC_CLIENT_SECRET=integration-secret + depends_on: + keycloak: + condition: service_started + go-httpbin: + condition: service_started + asset-origin: + condition: service_started + grpc-echo: + condition: service_healthy + volumes: + - ./src/main/docker/certificates:/app/certificates:ro + # Shared config dir, then overlay ONLY gateway.yaml with the low-admission-budget variant. + - ./src/main/docker/sheriff-config:/app/sheriff-config:ro + - ./src/main/docker/sheriff-config-ws-admission/gateway.yaml:/app/sheriff-config/gateway.yaml:ro + - ./src/main/docker/assets:/app/assets:ro + - ${LOG_TARGET_DIR:-./target/quarkus-logs}:/logs:rw + security_opt: + - no-new-privileges:true + cap_drop: + - ALL + read_only: true + tmpfs: + - /tmp:rw,noexec,nosuid,size=100m + deploy: + resources: + limits: + memory: 512M + cpus: '4.0' + reservations: + memory: 256M + cpus: '1.0' + # No in-container healthcheck: the distroless image ships neither a shell nor curl. Readiness is + # gated host-side by start-integration-container.sh probing the published management port 19004. + networks: + - api-sheriff + restart: unless-stopped + prometheus: image: prom/prometheus:v3.6.0 ports: diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index a679ccef..883906a4 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -253,6 +253,15 @@ localhost-trust ${project.basedir}/src/main/docker/certificates/mtls-wrong.p12 wrong-trust + + 10447 org.jboss.logmanager.LogManager ${skipITs} diff --git a/integration-tests/scripts/dump-keycloak-logs.sh b/integration-tests/scripts/dump-keycloak-logs.sh index ebe566bd..5028be7e 100755 --- a/integration-tests/scripts/dump-keycloak-logs.sh +++ b/integration-tests/scripts/dump-keycloak-logs.sh @@ -43,16 +43,19 @@ echo "📝 Output file: $KEYCLOAK_LOG_FILE_PATH" # Never fail the build on a dump problem. # # The list MUST name every gateway instance docker-compose.yml starts, not just the primary and the -# mTLS peer: the Bff*Cookie*IT suites drive the two dedicated cookie-mode instances, and a CI-only -# failure on those instances previously produced NO uploaded log at all, forcing a local repro to -# see the gateway's own rejection reason. Keep this list in lockstep with the api-sheriff* services -# in integration-tests/docker-compose.yml. +# mTLS peer: the Bff*Cookie*IT suites drive the two dedicated cookie-mode instances and +# WebSocketProxyIT's relay-exhaustion regression drives the low-admission-budget instance, and a +# CI-only failure on those instances previously produced NO uploaded log at all, forcing a local +# repro to see the gateway's own rejection reason. An admission refusal in particular is a bare 503 +# on the wire whose reason exists only in the gateway's own log. Keep this list in lockstep with the +# api-sheriff* services in integration-tests/docker-compose.yml. FAILSAFE_DIR="${TARGET_ABS_PATH}/failsafe-reports" mkdir -p "$FAILSAFE_DIR" || true for app in integration-tests-api-sheriff-1 \ integration-tests-api-sheriff-mtls-1 \ integration-tests-api-sheriff-cookie-1 \ - integration-tests-api-sheriff-cookie-2-1; do + integration-tests-api-sheriff-cookie-2-1 \ + integration-tests-api-sheriff-ws-admission-1; do if docker ps -a --format "{{.Names}}" | grep -q "^${app}$"; then echo "📥 Dumping app logs: ${app} -> ${FAILSAFE_DIR}/${app}.log" docker logs "$app" > "${FAILSAFE_DIR}/${app}.log" 2>&1 || true diff --git a/integration-tests/scripts/start-integration-container.sh b/integration-tests/scripts/start-integration-container.sh index f47020e9..a476f9bd 100755 --- a/integration-tests/scripts/start-integration-container.sh +++ b/integration-tests/scripts/start-integration-container.sh @@ -205,32 +205,36 @@ for i in {1..30}; do sleep 1 done -# Wait for the two dedicated cookie-mode gateway instances (api-sheriff-cookie on 10445 / -# management 19002, and its key-sharing peer api-sheriff-cookie-2 on 10446 / management 19003). -# Both reuse the same native image and reach readiness offline (static-file JWKS). The Bff*Cookie*IT -# suites drive their TLS ports directly — and BffCookieStatelessnessIT drives BOTH in one test — so -# an unwaited instance is a race that surfaces as a connection refusal in the IT phase, not as a -# start-up failure here. Mirrors the mTLS block above exactly. -for cookie_instance in "api-sheriff-cookie:19002" "api-sheriff-cookie-2:19003"; do - COOKIE_SERVICE="${cookie_instance%%:*}" - COOKIE_MGMT_PORT="${cookie_instance##*:}" - echo "⏳ Waiting for the ${COOKIE_SERVICE} gateway instance to be ready..." +# Wait for the remaining dedicated gateway instances: the two cookie-mode instances +# (api-sheriff-cookie on 10445 / management 19002, and its key-sharing peer api-sheriff-cookie-2 on +# 10446 / management 19003) and the low-admission-budget instance (api-sheriff-ws-admission on +# 10447 / management 19004). All reuse the same native image and reach readiness offline +# (static-file JWKS). Their suites drive the TLS ports directly — the Bff*Cookie*IT suites, with +# BffCookieStatelessnessIT driving BOTH cookie instances in one test, and WebSocketProxyIT's relay +# exhaustion regression driving the low-cap instance — so an unwaited instance is a race that +# surfaces as a connection refusal in the IT phase, not as a start-up failure here. Mirrors the +# mTLS block above exactly; keep the list in lockstep with the api-sheriff* services in +# docker-compose.yml. +for gateway_instance in "api-sheriff-cookie:19002" "api-sheriff-cookie-2:19003" "api-sheriff-ws-admission:19004"; do + GATEWAY_SERVICE="${gateway_instance%%:*}" + GATEWAY_MGMT_PORT="${gateway_instance##*:}" + echo "⏳ Waiting for the ${GATEWAY_SERVICE} gateway instance to be ready..." for i in {1..30}; do - if curl -skf "https://localhost:${COOKIE_MGMT_PORT}/q/health/live" > /dev/null 2>&1; then - echo "✅ ${COOKIE_SERVICE} gateway instance is ready!" + if curl -skf "https://localhost:${GATEWAY_MGMT_PORT}/q/health/live" > /dev/null 2>&1; then + echo "✅ ${GATEWAY_SERVICE} gateway instance is ready!" break fi if [ $i -eq 30 ]; then - echo "❌ ${COOKIE_SERVICE} gateway instance failed to start within 30 seconds" + echo "❌ ${GATEWAY_SERVICE} gateway instance failed to start within 30 seconds" DIAG_DIR="target/failsafe-reports" mkdir -p "$DIAG_DIR" - echo "----- $COMPOSE_BASE logs ${COOKIE_SERVICE} -----" - $COMPOSE_BASE logs --no-color "${COOKIE_SERVICE}" 2>&1 | tee "$DIAG_DIR/${COOKIE_SERVICE}-app.log" - curl -sk "https://localhost:${COOKIE_MGMT_PORT}/q/health" 2>&1 | tee "$DIAG_DIR/${COOKIE_SERVICE}-health.json" + echo "----- $COMPOSE_BASE logs ${GATEWAY_SERVICE} -----" + $COMPOSE_BASE logs --no-color "${GATEWAY_SERVICE}" 2>&1 | tee "$DIAG_DIR/${GATEWAY_SERVICE}-app.log" + curl -sk "https://localhost:${GATEWAY_MGMT_PORT}/q/health" 2>&1 | tee "$DIAG_DIR/${GATEWAY_SERVICE}-health.json" echo "" exit 1 fi - echo "⏳ Waiting for ${COOKIE_SERVICE} gateway... (attempt $i/30)" + echo "⏳ Waiting for ${GATEWAY_SERVICE} gateway... (attempt $i/30)" sleep 1 done done diff --git a/integration-tests/src/main/docker/sheriff-config-ws-admission/gateway.yaml b/integration-tests/src/main/docker/sheriff-config-ws-admission/gateway.yaml new file mode 100644 index 00000000..9608a5d3 --- /dev/null +++ b/integration-tests/src/main/docker/sheriff-config-ws-admission/gateway.yaml @@ -0,0 +1,141 @@ +# yaml-language-server: $schema=../../../../../api-sheriff/src/main/resources/schema/gateway.schema.json +# Global gateway document for the DEDICATED low-admission-budget integration-test instance +# (api-sheriff-ws-admission in docker-compose.yml). It is overlay-mounted on top of the shared +# sheriff-config directory so it replaces ONLY gateway.yaml — the endpoints/ subdirectory and +# topology.properties are shared with the primary instance, so the /ws/echo relay route and the +# /proxy HTTP route resolve identically here. +# +# This instance exists because the admission budget is a property of the whole gateway process. The +# production defaults (2048 general permits, 512 relay permits) are far too large for a test to +# exhaust: proving the WebSocket relay returns its permits at teardown would need thousands of +# sequential upgrades. Shrinking the caps to single digits turns that proof into a handful of +# connections — but a gateway with an 8-request admission budget cannot serve the rest of the IT +# suite, so the small budget lives on this separate instance (published on 10447) while the primary +# instance (10443) keeps the production-shaped defaults. +# +# The anchors, token_validation and oidc blocks are a faithful copy of the primary gateway.yaml so +# the shared endpoints/ resolve identically and the offline it-static JWKS issuer reaches readiness +# without Keycloak. The intentional differences are exactly two: the edge_hardening block below, and +# the absence of tls.passthrough_sni (the SNI front never starts, so Quarkus terminates directly on +# the public port — matching the mTLS and cookie instances). +version: 1 +metadata: + config_version: "integration-test-ws-admission" +allowed_methods: ["GET", "POST", "PUT", "DELETE"] +tls: + alpn: ["h2", "http/1.1"] +# --- The point of this instance: a deliberately tiny admission budget --------------------------- +# websocket_relay_cap is a sub-budget acquired IN ADDITION TO the general admission permit, and a +# relay holds BOTH for the connection's whole lifetime. WsAdmissionExhaustion (WebSocketProxyIT) +# opens and closes more sequential upgrades than either cap allows: with the teardown release in +# place every permit is returned when the relay closes, so all of them succeed. Leak either permit +# and the run dies on the connection that first exceeds the leaked cap — the relay sub-budget after +# 2 upgrades, the general budget after 8 — which is precisely the regression this instance guards. +edge_hardening: + admission_cap: 8 + websocket_relay_cap: 2 +anchors: + api: + path_prefix: /proxy + type: proxy + access: public + bff: + path_prefix: /bff + type: proxy + access: public + bff-session: + path_prefix: /bff-session + type: bff + access: authenticated + auth: + require: session + secure: + path_prefix: /secure + type: proxy + access: authenticated + auth: + require: bearer + graphql: + path_prefix: /graphql + type: proxy + access: public + allowed_methods: ["POST"] + upload: + path_prefix: /upload + type: proxy + access: public + allowed_methods: ["POST"] + security_filter: + max_body_bytes: 67108864 + ws: + path_prefix: /ws + type: proxy + access: public + grpc: + path_prefix: /grpc + type: proxy + access: public + assets-public: + path_prefix: /assets + type: asset + access: public + allowed_methods: ["GET", "HEAD"] + assets-secure: + path_prefix: /secure-assets + type: asset + access: authenticated + allowed_methods: ["GET", "HEAD"] + auth: + require: bearer +token_validation: + issuers: + - name: it-static + issuer: https://api-sheriff.test/it + jwks: + source: file + file: /app/certificates/test-jwks.json + - name: benchmark-keycloak + issuer: https://keycloak:8443/realms/benchmark + jwks: + source: http + url: https://keycloak:8443/realms/benchmark/protocol/openid-connect/certs + allowed_egress_hosts: ["keycloak"] + tls_profile: benchmark-idp + - name: integration-keycloak + issuer: https://keycloak:8443/realms/integration + jwks: + source: http + url: https://keycloak:8443/realms/integration/protocol/openid-connect/certs + allowed_egress_hosts: ["keycloak"] + tls_profile: benchmark-idp +# --- Server-mode BFF confidential-client block --------------------------------------------------- +# Kept in lockstep with the primary gateway.yaml (the shared endpoints/ directory declares a +# require: session route, and ConfigValidator refuses a session floor with no oidc block). No login +# is ever driven against this instance; the URLs name this instance's own published port so the +# document is internally consistent rather than silently pointing at the primary gateway. +oidc: + issuer: https://keycloak:8443/realms/integration + client_id: integration-client + client_secret: ${OIDC_CLIENT_SECRET} + scopes: ["openid", "profile", "email"] + redirect_uri: https://localhost:10447/auth/callback + logout: + path: /auth/logout + post_logout_redirect_uri: https://localhost:10447/auth/logout/return + final_redirect: / + backchannel_path: /auth/backchannel + session: + mode: server + store: memory + ttl_seconds: 3600 + refresh: + enabled: true + leeway_seconds: 30 + csrf: + trusted_origins: ["https://localhost:10447"] + user_info: + path: /auth/userinfo + allowed_claims: ["sub", "preferred_username", "email", "groups"] + default_view: ["sub", "preferred_username"] + login: + path: /auth/login diff --git a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WebSocketProxyIT.java b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WebSocketProxyIT.java index cb565ea7..62758b2c 100644 --- a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WebSocketProxyIT.java +++ b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WebSocketProxyIT.java @@ -15,6 +15,7 @@ */ package de.cuioss.sheriff.gateway.integration; +import static io.restassured.RestAssured.given; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -57,6 +58,32 @@ * empty {@code allowed_origins} — cannot coexist with a bootable stack (it aborts boot fail-fast), so * it is proven by {@code verify-invalid-config-fails.sh} and the unit-level {@code ConfigValidatorTest} * rather than against the running edge here. + *

+ * Admission-permit exhaustion. One test in this suite deliberately drives a + * different instance: {@code api-sheriff-ws-admission} on {@code test.ws.admission.port} + * (10447), whose overlaid {@code gateway.yaml} shrinks the {@code edge_hardening} budget to single + * digits. The permit-release contract cannot be observed on the primary instance at all — its + * production-shaped caps (2048 general permits, 512 relay permits) would need thousands of sequential + * upgrades to exhaust, so a handful of connections passes there whether or not the relay ever returns + * its permits. The low-cap instance is what turns that silent pass into a real assertion. + *

+ * Why the pre-existing suite could not see the leak. Before that regression landed, + * this class held seven tests, all green, while the relay leaked its admission permit on every + * established connection. The blind spot was structural, not an oversight in any one test. Each of + * the seven performs at most one handshake and asserts only the status of that one + * request — and only three of them complete an upgrade at all + * ({@link #echoRoundTripThroughGateway()}, {@link #idleRelayReclaimedAfterTimeout()}, + * {@link #heartbeatedRelaySurvivesIdleTimeout()}). The other four are rejection paths that end the + * HTTP response, so the edge's own end handler releases their permit correctly; they could not leak + * even in principle. Three leaked permits against a 2048-permit pool is invisible by construction: no + * assertion in the suite reads the pool, and the pool never runs dry. + *

+ * The generalisable rule: a per-request correctness assertion cannot observe cumulative resource + * exhaustion. Detecting it needs an assertion of a different shape — one made after N + * operations, and made about the gateway rather than about any single response. That is + * exactly the shape of {@link #sequentialRelaysReturnTheirAdmissionPermitsAtTeardown()}: N sequential + * relays, then a claim about the edge's remaining capacity. Adding an eighth per-request test would + * have left the leak equally invisible. */ class WebSocketProxyIT extends BaseIntegrationTest { @@ -71,7 +98,17 @@ class WebSocketProxyIT extends BaseIntegrationTest { /** Idle-reclaim close code (WebSocket 1001 Going Away) the relay emits on idle expiry. */ private static final int CLOSE_GOING_AWAY = 1001; + /** + * Sequential upgrades driven against the low-cap instance. It exceeds BOTH caps that instance + * declares ({@code admission_cap: 8}, {@code websocket_relay_cap: 2}), so a leak of either permit + * fails the run: the relay sub-budget would run dry on the 3rd upgrade and the general budget on + * the 9th. With both permits returned at relay teardown, all ten succeed. + */ + private static final int LOW_CAP_SEQUENTIAL_UPGRADES = 10; + private static String wsBaseUri; + private static String lowCapWsBaseUri; + private static int lowCapHttpPort; private static HttpClient httpClient; @BeforeAll @@ -79,6 +116,12 @@ static void setUpWebSocketClient() throws Exception { String testPort = System.getProperty("test.https.port", "10443"); wsBaseUri = "wss://localhost:" + testPort; + // The dedicated low-admission-budget instance (api-sheriff-ws-admission), published on its + // own host port and mounting the sheriff-config-ws-admission gateway.yaml overlay. It shares + // the endpoints/ directory with the primary instance, so /ws/echo and /proxy resolve here too. + lowCapHttpPort = Integer.parseInt(System.getProperty("test.ws.admission.port", "10447")); + lowCapWsBaseUri = "wss://localhost:" + lowCapHttpPort; + // Trust-all TLS: the integration stack serves a self-signed localhost certificate, exactly // the case BaseIntegrationTest handles for REST Assured via useRelaxedHTTPSValidation(). The // JDK WebSocket client offers no equivalent one-liner, so a trust-all context is built here. @@ -174,6 +217,56 @@ void unmatchedWebSocketPathRejected() { "an unmatched WS path must be rejected 404 by deny-by-default route selection"); } + @Test + @DisplayName("more sequential relays than either admission cap allows all succeed, and HTTP still serves") + void sequentialRelaysReturnTheirAdmissionPermitsAtTeardown() throws Exception { + for (int upgrade = 1; upgrade <= LOW_CAP_SEQUENTIAL_UPGRADES; upgrade++) { + var listener = new RecordingListener(); + WebSocket socket = openLowCapRelay(listener, upgrade); + String payload = "sheriff-ws-admission-" + upgrade; + + socket.sendText(payload, true).get(HANDSHAKE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + String echoed = listener.firstMessage.get(HANDSHAKE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + socket.sendClose(WebSocket.NORMAL_CLOSURE, "done").get(HANDSHAKE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + // Awaiting the close frame is what makes the next iteration race-free rather than timed: + // the relay's single teardown funnel releases both permits BEFORE it closes either leg, + // so a close frame observed here proves the release has already happened. + listener.closed.get(HANDSHAKE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + + assertEquals(payload, echoed, + "relay " + upgrade + " must round-trip its frame — a permit-starved edge cannot echo"); + } + + // A leaked general permit starves ordinary HTTP long before it exhausts the pool entirely, so + // the surviving HTTP path is the second half of the proof: the ten relays above returned their + // general permits too, not just their relay sub-permits. + given() + .port(lowCapHttpPort) + .when() + .get("/proxy/get") + .then() + .statusCode(200); + } + + /** + * Opens one relay against the low-cap instance, translating a refused upgrade into an assertion + * failure that names which upgrade in the sequence was refused — the raw + * {@link ExecutionException} would otherwise report only a bare {@code 503} with no indication of + * whether the run died at the relay sub-budget or the general one. + */ + private static WebSocket openLowCapRelay(RecordingListener listener, int upgrade) throws Exception { + try { + return httpClient.newWebSocketBuilder() + .header("Origin", ALLOWED_ORIGIN) + .buildAsync(URI.create(lowCapWsBaseUri + "/ws/echo"), listener) + .get(HANDSHAKE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (ExecutionException refused) { + throw new AssertionError("upgrade " + upgrade + " of " + LOW_CAP_SEQUENTIAL_UPGRADES + + " was refused — the closed relays before it did not return their admission permits", + refused.getCause()); + } + } + /** * Opens a handshake expected to fail (non-101) and returns the underlying * {@link WebSocketHandshakeException} so the caller can assert the HTTP status. An {@code origin} diff --git a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WsAdmissionActivationWiringTest.java b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WsAdmissionActivationWiringTest.java new file mode 100644 index 00000000..749b08ad --- /dev/null +++ b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/WsAdmissionActivationWiringTest.java @@ -0,0 +1,189 @@ +/* + * Copyright © 2026 CUI-OpenSource-Software (info@cuioss.de) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package de.cuioss.sheriff.gateway.integration; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import org.yaml.snakeyaml.Yaml; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Fast, no-Docker surefire guard that the committed integration-test deployment descriptors + * actually activate the low admission budget the relay-permit exhaustion regression + * depends on — the exact wiring whose absence would let that regression pass while proving nothing. + *

+ * The failure mode this test exists for is silent, and it is the reason a descriptor assertion is + * warranted at all. {@code WebSocketProxyIT}'s exhaustion test opens ten sequential relays and + * asserts they all succeed. Against a gateway running the shipped defaults — 2048 general permits and + * 512 relay permits — ten relays succeed whether or not the relay ever returns a permit, + * because ten never approaches either cap. So an overlay that is never mounted, a compose service + * that is never defined, or an {@code edge_hardening} block whose caps drift upward all leave a green + * test that has stopped testing anything. No black-box assertion inside the IT itself can see that: + * the wire looks identical either way. + *

+ * It parses the committed descriptors only (YAML text) and asserts the activation is present — it + * starts no container and reaches no network. Modelled on {@code TlsEdgeActivationWiringTest} and + * {@code BffCookieActivationWiringTest}. + * + * @author API Sheriff Team + * @since 1.0 + */ +class WsAdmissionActivationWiringTest { + + /** The module base directory (surefire runs with the module root as the working directory). */ + private static final Path MODULE = Path.of(System.getProperty("user.dir")); + private static final Path DOCKER = MODULE.resolve("src/main/docker"); + private static final Path WS_ADMISSION_GATEWAY = DOCKER.resolve("sheriff-config-ws-admission/gateway.yaml"); + + private static final String WS_ADMISSION_SERVICE = "api-sheriff-ws-admission"; + + private static final String WS_ADMISSION_OVERLAY_MOUNT = + "./src/main/docker/sheriff-config-ws-admission/gateway.yaml:/app/sheriff-config/gateway.yaml:ro"; + + /** + * The published host port {@code WebSocketProxyIT} reaches the low-cap instance on, kept in + * lockstep with the {@code test.ws.admission.port} failsafe system property in {@code pom.xml}. + */ + private static final String WS_ADMISSION_PORT_PREFIX = "10447:"; + + /** + * The ceiling both caps must stay under. The exhaustion regression drives ten sequential + * upgrades, so any cap at ten or above is never reached and the regression silently stops + * regressing — this bound is what makes cap drift a build failure rather than a quiet no-op. + */ + private static final int SEQUENTIAL_UPGRADES = 10; + + @Test + @DisplayName("the ws-admission overlay declares an edge_hardening budget small enough to exhaust") + void overlayDeclaresAnExhaustibleAdmissionBudget() throws Exception { + // Arrange + Map hardening = edgeHardeningBlock(); + + // Act + Object admissionCap = hardening.get("admission_cap"); + Object relayCap = hardening.get("websocket_relay_cap"); + + // Assert — an absent member silently resolves to the shipped default (2048 / 512), which the + // regression can never reach, so both must be declared explicitly and both must stay below + // the number of upgrades the regression actually drives. + assertInstanceOf(Integer.class, admissionCap, "edge_hardening.admission_cap must be declared explicitly"); + assertInstanceOf(Integer.class, relayCap, "edge_hardening.websocket_relay_cap must be declared explicitly"); + assertTrue((Integer) admissionCap < SEQUENTIAL_UPGRADES, + "admission_cap must stay below the " + SEQUENTIAL_UPGRADES + + " sequential upgrades the regression drives, was: " + admissionCap); + assertTrue((Integer) relayCap < SEQUENTIAL_UPGRADES, + "websocket_relay_cap must stay below the " + SEQUENTIAL_UPGRADES + + " sequential upgrades the regression drives, was: " + relayCap); + } + + @Test + @DisplayName("the relay sub-budget stays within the general pool it draws from") + void theRelaySubBudgetStaysWithinTheGeneralPool() throws Exception { + // Arrange + Map hardening = edgeHardeningBlock(); + + // Act + int admissionCap = (Integer) hardening.get("admission_cap"); + int relayCap = (Integer) hardening.get("websocket_relay_cap"); + + // Assert — a relay permit is taken IN ADDITION to a general one, so a sub-budget larger than + // the pool can never bind. ConfigValidator refuses that at boot, which would turn a descriptor + // mistake into a stack that never reaches readiness rather than a legible test failure. + assertTrue(relayCap <= admissionCap, + "websocket_relay_cap (" + relayCap + ") must not exceed admission_cap (" + admissionCap + ")"); + } + + @Test + @DisplayName("the overlay terminates directly on the public port with no passthrough front") + void overlayDeclaresNoPassthroughFront() throws Exception { + // Arrange + Map doc = loadYaml(WS_ADMISSION_GATEWAY); + Object tls = doc.get("tls"); + assertInstanceOf(Map.class, tls, "the ws-admission gateway.yaml must declare a tls block"); + @SuppressWarnings("unchecked") + Map tlsMap = (Map) tls; + + // Assert — a declared passthrough_sni starts the SNI front listener on the public port, which + // would require the terminated listener to move aside (QUARKUS_HTTP_SSL_PORT) exactly as the + // primary instance does. This instance publishes no such override, so declaring one here + // would bind-conflict at boot. Same shape as the mTLS and cookie overlays. + assertNull(tlsMap.get("passthrough_sni"), + "the ws-admission overlay must not declare tls.passthrough_sni — it terminates on the public port"); + } + + @Test + @DisplayName("docker-compose defines the ws-admission instance mounting the low-cap overlay") + void composeDefinesTheWsAdmissionInstance() throws Exception { + // Arrange + Map services = composeServices(); + + // Act + Object wsAdmissionService = services.get(WS_ADMISSION_SERVICE); + + // Assert — without a dedicated instance the exhaustion regression would drive the primary + // gateway's production-shaped caps and pass without exhausting anything. + assertNotNull(wsAdmissionService, + "a dedicated " + WS_ADMISSION_SERVICE + " gateway instance must be defined"); + @SuppressWarnings("unchecked") + Map wsAdmission = (Map) wsAdmissionService; + + Object ports = wsAdmission.get("ports"); + assertInstanceOf(List.class, ports, "the ws-admission instance must publish ports"); + assertTrue(((List) ports).stream().map(String::valueOf).anyMatch(p -> p.startsWith(WS_ADMISSION_PORT_PREFIX)), + "the ws-admission instance must publish host port 10447 for test.ws.admission.port"); + + Object volumes = wsAdmission.get("volumes"); + assertInstanceOf(List.class, volumes, "the ws-admission instance must declare volumes"); + List mounts = ((List) volumes).stream().map(String::valueOf).toList(); + assertTrue(mounts.contains(WS_ADMISSION_OVERLAY_MOUNT), + "the ws-admission instance must overlay ONLY gateway.yaml with the low-cap variant, was: " + mounts); + } + + @SuppressWarnings("unchecked") + private static Map edgeHardeningBlock() throws IOException { + Map doc = loadYaml(WS_ADMISSION_GATEWAY); + Object hardening = doc.get("edge_hardening"); + assertNotNull(hardening, "the ws-admission gateway.yaml must declare an edge_hardening block"); + assertInstanceOf(Map.class, hardening, "the edge_hardening block must be a mapping"); + return (Map) hardening; + } + + @SuppressWarnings("unchecked") + private static Map composeServices() throws IOException { + Map doc = loadYaml(MODULE.resolve("docker-compose.yml")); + Object services = doc.get("services"); + assertInstanceOf(Map.class, services, "docker-compose.yml must declare services"); + return (Map) services; + } + + @SuppressWarnings("unchecked") + private static Map loadYaml(Path path) throws IOException { + try (InputStream in = Files.newInputStream(path)) { + return new Yaml().loadAs(in, Map.class); + } + } +} From 65f9451ac6c6d29a8bd899e539ca5979d68ba12e Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:37:32 +0200 Subject: [PATCH 04/11] feat(benchmarks): re-enable the websocketEcho goal now the permit leak is fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal was suppressed because a sustained WebSocket load run exhausted the edge admission pool and left the gateway 503-ing all traffic until restart — which would also have poisoned every goal sequenced after it. The relay now releases its permit from its own teardown funnel, so the aspect measures relay throughput instead of the exhaustion path. Flips skip.benchmark.websocket.echo to false. The property is kept rather than deleted so the goal stays individually suppressible without editing the execution, and the goal keeps its defense-in-depth position at the end of the profile. Re-derives every count and narrative the flip invalidates rather than only the property: the CI lane now expects ELEVEN of the twelve declared goals to produce a result, so websocketEcho joins the coverage step's expected list — without which the goal could vanish from a run and the job would still pass — and leaves the "Deliberately skipped" table, where only uploadLarge remains. The goal-execution budget term and the README's coverage and trend-series passages move with it. Every uploadLarge / skip.benchmark.upload.large row is left byte-unchanged; that goal is still blocked on its own gateway defect. --- .github/workflows/benchmark.yml | 40 +++++++++++++++--------------- benchmarks/README.adoc | 40 ++++++++++++++++-------------- benchmarks/pom.xml | 44 ++++++++++++++++++--------------- 3 files changed, 66 insertions(+), 58 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 82b40f2c..f07b69ed 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -27,22 +27,23 @@ jobs: # The -Pbenchmark profile DECLARES TWELVE k6 goals: the eight cross-gateway matrix aspects, the # two retained non-matrix health benchmarks (healthLiveCheck, gatewayHealth), and the two # API-Sheriff-only passthrough-relay executions (mapped relay throughput and the empty-mode - # no-regression run). TEN of them execute: uploadLarge and websocketEcho each carry a - # bound to a property that defaults to true in benchmarks/pom.xml, because each is blocked by an - # open gateway defect (see the "Deliberately skipped" table in the coverage step below). Maven - # therefore never runs them, so they consume no wall time and cannot abort the suite. Both are - # still ordered last in the profile as defense-in-depth, so that flipping a skip property to - # false cannot mask a goal that would otherwise produce a result. + # no-regression run). ELEVEN of them execute: only uploadLarge still carries a bound to a + # property that defaults to true in benchmarks/pom.xml, because it remains blocked by an open + # gateway defect (see the "Deliberately skipped" table in the coverage step below). Maven + # therefore never runs it, so it consumes no wall time and cannot abort the suite. It stays + # ordered last in the profile as defense-in-depth, so that flipping its skip property to false + # cannot mask a goal that would otherwise produce a result. # # Budget, from per-goal timings measured locally 2026-07-28 rather than estimated: - # * goal execution ~12 min -- sized on the TEN goals that actually execute, which is now the + # * goal execution ~13 min -- sized on the ELEVEN goals that actually execute, which is the # real basis: a skipped goal is not run at all, so it costs nothing but the # Maven bookkeeping for a skipped execution. - # 10 goals x (60s k6 window + ~10s compose/k6 start and summary write). - # Re-enabling a skipped goal costs another ~70s, and upload-50MB more than - # that: it is transfer-bound at reduced concurrency, so it does not finish - # in the same wall time as a request-rate run. Re-derive this term when a - # skip property is flipped to false. + # 11 goals x (60s k6 window + ~10s compose/k6 start and summary write). + # The websocketEcho term is the ~70s a re-enabled request-rate goal costs, + # now that its blocking permit leak is fixed. Re-enabling upload-50MB would + # cost more than that: it is transfer-bound at reduced concurrency, so it + # does not finish in the same wall time as a request-rate run. Re-derive + # this term when a skip property is flipped to false. # * stack startup ~100 s -- the lane boots FIVE native gateway instances (api-sheriff, # api-sheriff-mtls, api-sheriff-cookie, api-sheriff-cookie-2 and # api-sheriff-ws-admission, the last added for the WebSocket relay-permit @@ -118,10 +119,10 @@ jobs: # twelve goals: the eight cross-gateway matrix aspects (unauth, bearer, http2, # graphql, upload-1MB, upload-50MB, ws, grpc), the two retained non-matrix health # benchmarks (healthLiveCheck, gatewayHealth), and the two API-Sheriff-only - # passthrough-relay executions (mapped and empty). Ten of the twelve execute and are - # expected to produce a CI result; upload-50MB (uploadLarge) and ws (websocketEcho) are - # skipped by property against open gateway defects, so Maven does not run them and they - # cannot abort the suite. The on-demand APISIX comparison lane is deliberately NOT run + # passthrough-relay executions (mapped and empty). Eleven of the twelve execute and are + # expected to produce a CI result; only upload-50MB (uploadLarge) is still skipped by + # property against an open gateway defect, so Maven does not run it and it cannot abort + # the suite. The on-demand APISIX comparison lane is deliberately NOT run # here. Maven still fails fast on the goals that do run, so the coverage step below # remains what proves which goals actually produced a result. echo "Running k6 integration benchmarks with native Quarkus..." @@ -133,8 +134,8 @@ jobs: ls -la benchmarks/target/benchmark-results/ # Maven fails fast, so a goal that errors aborts every goal after it and a goal that is never - # reached leaves no trace in the job output — a suite that ran 3 of the 10 executing goals - # renders exactly like one that ran all 10. This step diffs the goals that MUST produce a + # reached leaves no trace in the job output — a suite that ran 3 of the 11 executing goals + # renders exactly like one that ran all 11. This step diffs the goals that MUST produce a # summary document against the documents k6 actually wrote, so a partial suite is visibly # partial and a goal that silently vanished fails the job. When Maven failed, it also states # in words whether the suite was truncated by that failure. Skipped goals are named with their @@ -150,7 +151,7 @@ jobs: # Goals expected to write a summary. A goal settled as deliberately skipped is NOT listed # here — it belongs in the skip table below, which is what keeps the two states distinct. - expected="healthLiveCheck gatewayHealth proxiedStatic passthroughRelay passthroughRelayEmpty bearerProxied http2 graphql uploadSmall grpcUnary" + expected="healthLiveCheck gatewayHealth proxiedStatic passthroughRelay passthroughRelayEmpty bearerProxied http2 graphql uploadSmall grpcUnary websocketEcho" missing=0 missing_names="" @@ -178,7 +179,6 @@ jobs: echo "| Benchmark | Skipped by | Reason |" echo "| --- | --- | --- |" echo "| \`uploadLarge\` | \`skip.benchmark.upload.large\` (defaults to \`true\`) | The gateway rejects the 50MB body with 413: the Quarkus HTTP body limit is not derived from the upload anchor's 64 MiB \`max_body_bytes\`, so the declared cap is unreachable. Needs a gateway-side fix; re-enable with \`-Dskip.benchmark.upload.large=false\`. |" - echo "| \`websocketEcho\` | \`skip.benchmark.websocket.echo\` (defaults to \`true\`) | Each WebSocket upgrade leaks an edge admission permit; once exhausted the gateway rejects all traffic with 503 until restarted, which would also poison every goal after it. Needs a gateway-side fix; re-enable with \`-Dskip.benchmark.websocket.echo=false\`. |" echo "| \`sessionMediated\` | not wired | Wired to no Maven goal on purpose — BFF session mediation belongs to PLAN-07A. |" } >> "$GITHUB_STEP_SUMMARY" diff --git a/benchmarks/README.adoc b/benchmarks/README.adoc index 5026f22a..6043c5b2 100644 --- a/benchmarks/README.adoc +++ b/benchmarks/README.adoc @@ -180,23 +180,25 @@ enforces this table, so a goal that silently stops running fails the job: before the security filter runs. Blocked on a gateway-side fix. |`websocketEcho` -|*skipped* -|Skipped by `skip.benchmark.websocket.echo`, which defaults to `true`. Each WebSocket upgrade leaks - an edge admission permit; once they are exhausted the gateway rejects *all* traffic with `503` - until restarted, which would also poison every goal sequenced after it. Blocked on a gateway-side - fix. +|runs +|Skipped until the edge admission permit leak was fixed: every WebSocket upgrade stranded a permit, + so a sustained load run exhausted the pool and the gateway rejected *all* traffic with `503` until + restarted, poisoning every goal sequenced after it. The relay now releases its permit from its own + teardown funnel and takes a bounded relay sub-budget on top, so the goal measures relay throughput + instead of the exhaustion path. Its trend series starts empty. `skip.benchmark.websocket.echo` is + retained (now `false`) so the goal stays individually suppressible. |`sessionMediated` |*unwired* |Backs no Maven goal on purpose — see below. |=== -*Skipped means not executed.* Each skipped goal carries a `` element in `benchmarks/pom.xml` -bound to its own property, declared there with the id of the finding that blocks it and defaulting -to `true` — so Maven does not run the goal, it consumes no wall time, and it cannot abort the suite. -The two states are kept distinct rather than collapsed: the CI coverage step gates on the ten goals -that execute and names each skipped goal, its skip property and its reason in the job summary. When -Maven fails, that step also states in words whether the failure truncated the suite. Silence is +*Skipped means not executed.* A skipped goal carries a `` element in `benchmarks/pom.xml` bound +to its own property, declared there with the id of the finding that blocks it and defaulting to +`true` — so Maven does not run the goal, it consumes no wall time, and it cannot abort the suite. +The two states are kept distinct rather than collapsed: the CI coverage step gates on the eleven +goals that execute and names each skipped goal, its skip property and its reason in the job summary. +When Maven fails, that step also states in words whether the failure truncated the suite. Silence is never the signal. Re-enable a goal once its gateway defect is fixed by flipping its property, which is also how you @@ -501,9 +503,10 @@ know: . *Post-processing*: `WrkBenchmarkConverter` / `WrkResultPostProcessor` are replaced by `K6BenchmarkConverter` / `K6ResultPostProcessor`. The downstream report pipeline (badges, 10-run history, trends, GitHub Pages) consumes an identically-shaped model and is unchanged. -. *Coverage*: the lane grew from three wired executions to twelve *declared* goals, ten of which - execute and are expected to produce a CI result — `uploadLarge` and `websocketEcho` are declared - but skipped by property against open gateway defects, so Maven does not run them at all. `bearer` +. *Coverage*: the lane grew from three wired executions to twelve *declared* goals, eleven of which + execute and are expected to produce a CI result — only `uploadLarge` is declared but skipped by + property against an open gateway defect, so Maven does not run it at all. `websocketEcho` was + skipped alongside it until its blocking edge admission-permit leak was fixed. `bearer` is genuinely new CI coverage — the wrk-era bearer runner existed on disk but was registered in no Maven execution. Being *wired* is not the same as *executing*, and *executing* is not the same as *producing a result*: until PLAN-25 the job died at the fourth goal and goals 5-12 had never run @@ -522,10 +525,11 @@ incomparable, the k6 series starts fresh rather than continuing the wrk series: (see the `latency_ms.stdev` limit above). * Every benchmark that writes a summary document for the first time begins with *no history* and needs ten runs before its trend series is meaningful. Under PLAN-25 that is most of the lane: - eight of the twelve declared goals had never executed in CI, and six of those eight are expected - to produce a result and so start their series empty at this commit. `uploadLarge` and - `websocketEcho` start *no* series at all — only a goal that actually writes a summary document - opens one, and both are skipped by property rather than executed. The workflow's end-of-job coverage step + eight of the twelve declared goals had never executed in CI, and seven of those eight are expected + to produce a result and so start their series empty — six at the PLAN-25 commit, and + `websocketEcho` once its blocking permit leak was fixed. `uploadLarge` starts *no* series at all — + only a goal that actually writes a summary document opens one, and it is skipped by property + rather than executed. The workflow's end-of-job coverage step (`.github/workflows/benchmark.yml`) is the record of which goals produced a result in a given run. A flat or short trend line on a name that *did* run is an absence of history, not a regression. diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index 003bfee3..5ed9b10b 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -28,11 +28,10 @@ true - + true - - true + + false false @@ -555,15 +558,15 @@ - + ${skip.benchmark.websocket.echo} docker From c52351d6ccb665762318056be1f01caa2b2db666 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:46:05 +0200 Subject: [PATCH 05/11] docs: document the edge_hardening admission budget across all three layers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A configuration key that exists only in the schema is a key operators cannot find, so the new block is documented at each of the three layers that answer a different question about it. doc/configuration.adoc is the reference: the gateway.yaml sample gains the block with both keys commented, and the "Edge hardening defaults" section is rewritten around the distinction the block introduces — the framing and drain bounds remain fixed in code, while the two admission budgets are now operator-owned, because their sizing depends on the deployment's traffic shape rather than on protocol safety. That section previously claimed the whole set was applied in code "not by gateway.yaml"; the claim is now false and is gone. A Field Reference entry and two Validation Rules entries (both caps >= 1, and the relay cap within the pool it draws from) complete the reference surface. doc/user/protocol-routes.adoc answers the operator's actual question — what to set it to. A relay holds its permit for the connection's lifetime rather than a round-trip, so relays accumulate against the pool HTTP draws from; size the sub-cap to peak concurrent relays plus reconnect-storm headroom, and read admission_cap minus websocket_relay_cap as the request concurrency guaranteed to survive a saturated relay pool. doc/development/protocol-processors.adoc records the permit lifecycle as a contract, because this is the one dispatch path where the edge's ordinary release site does not work: a completed upgrade takes the connection over, so ctx.addEndHandler never fires. Names all three release sites and the reason both permits must be returned together. --- doc/configuration.adoc | 89 +++++++++++++++++++++--- doc/development/protocol-processors.adoc | 36 ++++++++++ doc/user/protocol-routes.adoc | 33 +++++++++ 3 files changed, 150 insertions(+), 8 deletions(-) diff --git a/doc/configuration.adoc b/doc/configuration.adoc index a6c48e85..73fdfe0b 100644 --- a/doc/configuration.adoc +++ b/doc/configuration.adoc @@ -126,7 +126,7 @@ file-and-JSON-pointer error before any immutable value object is built: | `schema/gateway.schema.json` | The global `gateway.yaml` document -- `version` (the only required key), `metadata`, `tls`, `security_headers`, `security_defaults`, `allowed_methods`, `anchors`, `upstream_defaults`, - `forwarded`, `token_validation` and `oidc`. + `forwarded`, `edge_hardening`, `token_validation` and `oidc`. | `schema/endpoint.schema.json` | A single `endpoints/*.yaml` file -- the `endpoint` block (`id`, `enabled`, `base_url`, @@ -219,6 +219,18 @@ forwarded: # resolved by cui-http (de.cuioss.http.forwar # (ForwardedResolverConfig.trustAll) emit: x-forwarded # x-forwarded (default) | both (adds RFC 7239 Forwarded) +edge_hardening: # OPTIONAL admission budget for the public data-plane edge. + # Omit the block, or either key, to take the defaults shown. + admission_cap: 2048 # max concurrently in-flight requests across the whole edge; + # a request beyond it is refused 503. Minimum 1. + websocket_relay_cap: 512 # max concurrently ESTABLISHED WebSocket relays. A sub-budget + # acquired IN ADDITION TO the general permit above, never + # instead of it, because a relay holds its permit for the + # connection's whole lifetime rather than one round-trip. + # Minimum 1, and must not exceed admission_cap -- a larger + # value could never bind. See "Sizing the WebSocket relay + # budget" in the protocol-routes guide. + token_validation: # offline bearer validation (routes with auth.require: bearer) issuers: # one entry per accepted identity provider - name: main @@ -595,11 +607,20 @@ Deny-by-default extends to omitted configuration: [[_edge_hardening_defaults]] === Edge hardening (transport) defaults -The public data-plane HTTP listener is bounded by a set of fixed secure defaults applied in code -(`EdgeHardeningOptions`), not by `gateway.yaml`. They fail fast on abusive framing before a request -is admitted to the pipeline and bound the in-flight and shutdown-drain windows. These values are -recorded here doc-first — they are the contract the edge-hardening code implements, and any change -to a constant must be reflected in this table. +The public data-plane HTTP listener is bounded on two axes, and they are governed differently. + +The *transport bounds* — the framing limits and the drain window — are fixed secure defaults applied +in code (`EdgeHardeningOptions`) and are not operator-configurable. They fail fast on abusive framing +before a request is admitted to the pipeline. These values are recorded here doc-first: they are the +contract the edge-hardening code implements, and any change to a constant must be reflected in this +table. + +The *admission budgets* — how many requests, and how many WebSocket relays, may be in flight at once +— are operator-facing and configurable through the optional +link:#_edge_hardening[`edge_hardening`] block in `gateway.yaml`. Their sizing depends on the +deployment's traffic shape rather than on protocol safety, which is why they are the two bounds the +operator owns. Omitting the block, or either key, resolves to the defaults in the table below, so a +deployment that never writes it keeps the behaviour shown here. [cols="2,1,3"] |=== @@ -622,10 +643,21 @@ to a constant must be reflected in this table. | An idle connection is closed after this window, so a slow-loris / h2 abuse load cannot pin connection slots. -| Admission cap (in-flight requests) +| Admission cap (in-flight requests) + + [`edge_hardening.admission_cap`] | 2048 | Requests beyond the cap are rejected `503` *before* a virtual thread is dispatched, so a flood - cannot spawn unbounded virtual threads. + cannot spawn unbounded virtual threads. Operator-configurable; minimum 1. + +| WebSocket relay cap (established relays) + + [`edge_hardening.websocket_relay_cap`] +| 512 +| A sub-budget acquired *in addition to* the general admission permit, never instead of it. An + established relay holds both permits for the connection's whole lifetime rather than for one + round-trip, so without this second bound a modest number of long-lived relays would consume the + general pool and starve ordinary HTTP traffic. An upgrade beyond the cap is refused `503` and + releases the general permit on the way out. Operator-configurable; minimum 1, and must not exceed + `admission_cap`. | Graceful-drain timeout | 25 s @@ -1367,6 +1399,38 @@ upstream_defaults: not_modified: { enabled: false } ---- +[[_edge_hardening]] +=== `edge_hardening` + +The two operator-facing admission budgets for the public data-plane edge, declarable only at the +`gateway.yaml` (global) scope — the pools they bound are process-wide, so there is no per-endpoint or +per-route form. The block and both members are optional; each omission resolves to its default, so a +deployment that never writes the block keeps the shipped behaviour. The remaining edge bounds +(framing limits, idle reap, drain window) are *not* configurable — see +link:#_edge_hardening_defaults[Edge hardening defaults] for the full table. + +[cols="1,3"] +|=== +| Field | Meaning + +| `admission_cap` +| The maximum number of concurrently in-flight requests the edge admits, across every route + (default `2048`, minimum `1`). A request beyond the cap is refused `503` *before* a virtual thread + is dispatched, so a flood cannot spawn unbounded virtual threads. Size it to the concurrency the + deployment's upstreams can absorb: too high defers the backpressure to the upstream, too low + refuses traffic the gateway could have served. + +| `websocket_relay_cap` +| The maximum number of concurrently *established* WebSocket relays (default `512`, minimum `1`). + This is a *sub-budget acquired in addition to* the general permit, never instead of it, because an + established relay holds its permit for the connection's whole lifetime rather than for one + request/response round-trip. Without the second bound, a modest number of long-lived relays would + consume the general pool and starve ordinary HTTP traffic. An upgrade beyond the cap is refused + `503`, releasing the general permit on the way out. Must not exceed `admission_cap` — a larger + value could never bind, so it is refused at boot rather than silently ignored. For sizing guidance + see link:user/protocol-routes.adoc[Protocol routes]. +|=== + === `rate_limit` Reserved schema for per-node, in-memory token-bucket rate limiting @@ -1628,3 +1692,12 @@ topology leak RFC 7239 §8.2/§8.3 warn about more simply than the obfuscation t fails the boot. * `websocket.idle_timeout_seconds`, when declared, must be a *positive integer* (default `300` when omitted); it is valid only on a `protocol: websocket` route. +* The `edge_hardening` block is optional, and so is each of its members; an omission resolves to the + documented default (`admission_cap` `2048`, `websocket_relay_cap` `512`). When declared, each cap + must be at least `1` -- a cap of zero would refuse every request (or every relay) and is always a + misconfiguration rather than a deliberate posture. +* `edge_hardening.websocket_relay_cap` must not exceed `edge_hardening.admission_cap`. The relay + budget is a *sub-budget of* the admission pool -- a relay takes a permit from both -- so a larger + value could never bind and would silently disable the sub-cap the operator meant to impose. The + comparison uses the *effective* values, so it holds when one member is omitted and takes its + default. diff --git a/doc/development/protocol-processors.adoc b/doc/development/protocol-processors.adoc index 64d8e310..9cd7ee17 100644 --- a/doc/development/protocol-processors.adoc +++ b/doc/development/protocol-processors.adoc @@ -87,6 +87,42 @@ upstream, completes the client upgrade, relays frames opaquely in both direction established relay that idles past the route's `idle_timeout_seconds` (closing both legs with WebSocket code `1001`). No HTTP `ResponseStage` relay runs for a WebSocket route. +==== The admission-permit lifecycle + +The WebSocket arm is the one dispatch path where the edge's ordinary permit-release site does not +work, and getting this wrong is not a leak that shows up in a test — it shows up as a gateway that +`503`s everything after enough upgrades. + +Every request acquires an *admission permit* at the top of `handle()`, and the HTTP paths return it +from `ctx.addEndHandler`, which fires when the HTTP response ends. A *completed upgrade takes the +connection over*, so for a relayed WebSocket that handler never fires: the response never ends in the +HTTP sense. The end handler therefore cannot be the release site here, and the permit must be held +across the upgrade anyway — an established relay occupies edge capacity for its whole lifetime, so +releasing it at upgrade completion would under-count exactly the connections that cost the most. + +`dispatchWebSocket` hands `WebSocketRelayStage` its own release callback instead, and the relay owns +the release from that point. The contract, in full: + +* *Acquired* at admission in `handle()`, before the pipeline runs, and *held across the upgrade* for + the relay's whole lifetime. +* An upgrade additionally acquires the *relay sub-permit* (`edge_hardening.websocket_relay_cap`) + before the hand-off — taken *in addition to* the general permit, not instead of it, so long-lived + relays cannot squeeze HTTP out of the admission pool. An upgrade beyond that cap is refused `503` + and releases the general permit on the way out. +* *Released exactly once*, from whichever terminal path is reached first: `RelaySession.closeBoth` + — the single idempotent teardown funnel every established-relay terminal path converges on (client + close, upstream close, idle reclaim, relay error) — or the *client-upgrade-failure* branch, where + the upstream connected but the client upgrade did not complete, so no session exists to tear down. + `closeBoth` releases *before* it closes either leg, which is what lets an integration test treat an + observed close frame as proof the permit is already back. +* Both permits are returned *together*, each behind its own idempotence latch, so an upstream-dial + failure — which *does* end the HTTP response, and so does reach the end handler — cannot strand the + sub-permit while the general one is released by a different site. + +The double-release risk is real and is handled by construction rather than by convention: the release +routes through the same CAS-guarded callback the end handler and the executor-rejection rollback use, +so every path can call it and only the first call counts. + === gRPC -- forced-h2 dispatch and trailer relay `dispatchGrpc` streams the request opaquely to the forced-HTTP/2 upstream via `GrpcDispatchStage`, diff --git a/doc/user/protocol-routes.adoc b/doc/user/protocol-routes.adoc index e7c88ddb..63b3bc4d 100644 --- a/doc/user/protocol-routes.adoc +++ b/doc/user/protocol-routes.adoc @@ -67,6 +67,39 @@ Raise it for long-lived push channels that legitimately idle between messages; l reclaim connection slots more aggressively. For the full field contract see link:../configuration.adoc#_websocket[Configuration -- `websocket`]. +=== Sizing the WebSocket relay budget + +An established relay is not like a proxied request. A request holds one *admission permit* for a +single round-trip and returns it in milliseconds; a relay holds its permit for as long as the +connection lives — minutes, or the whole day for a push channel. Long-lived relays therefore +accumulate against the same pool that ordinary HTTP traffic draws from, and enough of them will +starve it: the gateway starts refusing `503` while doing very little work. + +`edge_hardening.websocket_relay_cap` bounds them separately. It is a *sub-budget acquired in +addition to* the general permit, never instead of it — a relay takes one permit from each pool — so +capping it leaves the remaining `admission_cap` headroom available to HTTP no matter how many +sockets are open. An upgrade beyond the cap is refused `503` at the handshake, before the upstream is +dialled; the general permit is released on the way out, so a refusal costs nothing. + +[source,yaml] +---- +# gateway.yaml -- global; the pools are process-wide, so there is no per-route form +edge_hardening: + admission_cap: 2048 # default; all in-flight requests, relays included + websocket_relay_cap: 512 # default; established relays only +---- + +Size `websocket_relay_cap` to the *peak concurrent relays* the deployment expects, plus headroom for +reconnect storms — after an upstream restart or a network blip, clients reconnect together, and the +old sockets may not have been reaped yet, so the momentary peak runs well above steady state. Then +check the remainder: `admission_cap - websocket_relay_cap` is the request concurrency guaranteed to +survive a fully saturated relay pool, and it is what HTTP traffic actually gets in the worst case. +The defaults reserve three quarters of the pool for HTTP on that reasoning. + +Two boot-time rules bound the pair: each cap must be at least `1`, and `websocket_relay_cap` must not +exceed `admission_cap` — a larger sub-budget could never bind, so it is refused rather than silently +ignored. Both keys are optional and resolve to the defaults shown above. + == Configuring a gRPC route A `protocol: grpc` route proxies gRPC calls -- each an HTTP/2 `POST` to a service/method path. From ad2bc76cd42a17edb8e3e9f19fc540595c8efc76 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:56:26 +0200 Subject: [PATCH 06/11] style(edge): apply the pre-commit formatter to the admission-budget sources Formatter and import-sort output the quality gate produced against the files the two preceding commits touched: import-group separation in EdgeHardeningOptions and EdgeHardeningTest, continuation-indent normalisation in the two WebSocket test helpers. No behavioural change; committed so the tree the gate produces and the tree in the branch agree. --- .../de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java | 1 + .../java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java | 1 + .../de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java | 2 +- .../de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java index 75e994b5..dd1bc2e7 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java @@ -17,6 +17,7 @@ import java.util.concurrent.TimeUnit; + import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import io.quarkus.vertx.http.HttpServerOptionsCustomizer; diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java index 3601b120..a7e474fe 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java @@ -21,6 +21,7 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; + import de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig; import io.vertx.core.http.HttpServerOptions; diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java index 65d746cb..4c439e03 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java @@ -327,7 +327,7 @@ private HttpServer startFront(RouteTable table, CompletableFuture stashe private WebSocket connectWs(WebSocketClient client, int port) throws Exception { return client.connect(new WebSocketConnectOptions() - .setHost("localhost").setPort(port).setURI("/w/room")) + .setHost("localhost").setPort(port).setURI("/w/room")) .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); } diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java index 11bea007..b375ebf8 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.java @@ -388,7 +388,7 @@ void doesNotReleaseOnUpstreamDialFailure() throws Exception { private WebSocket connectTo(int port) throws Exception { return wsClient.connect(new WebSocketConnectOptions() - .setHost("localhost").setPort(port).setURI("/relay")) + .setHost("localhost").setPort(port).setURI("/relay")) .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS); } } From eaa00c721911f40ee0a7931c0827560d8b5c9f4a Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:03:40 +0200 Subject: [PATCH 07/11] fix(test): use an unnamed catch pattern in GatewayEdgeRouteTest Sonar java:S7467 - the ExecutionException binding in connectWhenAdmitted is never read, so replace it with the Java 25 unnamed pattern. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UV4pXNUtk5CKceRLWxvapz --- .../de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java index 4c439e03..93f5238a 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.java @@ -342,7 +342,7 @@ private WebSocket connectWhenAdmitted(WebSocketClient client, int port) throws E for (int attempt = 0; attempt < 200; attempt++) { try { return connectWs(client, port); - } catch (ExecutionException refused) { + } catch (ExecutionException _) { Thread.sleep(25); } } From 83ee419db731e7327b377c24590b7495f275d7d6 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:01:34 +0200 Subject: [PATCH 08/11] fix(edge): derive the omitted websocket_relay_cap from the effective admission cap A partially declared edge_hardening block self-rejected at boot: with admission_cap lowered below 512 and websocket_relay_cap omitted, the relay cap resolved to the fixed DEFAULT_WEBSOCKET_RELAY_CAP (512) while ConfigValidator compares the resolved pair, so the block was refused with 'websocket_relay_cap 512 must not exceed admission_cap 64' - a configuration the class Javadoc documents as valid. effectiveWebsocketRelayCap() now resolves an absent member to Math.max(1, effectiveAdmissionCap() / 4) instead of the fixed constant. That preserves the documented three-quarters-for-HTTP reservation at every admission cap (a min-clamp against 512 would resolve relay == admission below 512, letting relays consume the whole general pool) and is byte-identical at the shipped default, since 2048 / 4 == 512. An explicitly declared relay cap is untouched, so an explicit cap above admission_cap is still refused at boot. Javadoc, doc/configuration.adoc and doc/user/protocol-routes.adoc updated to describe the derived default. EdgeHardeningTest.fallsBackPerOmittedMember asserted the old fixed-constant contract at admission_cap 4096 and now asserts the derived 1024; the four default-cap assertions are unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UV4pXNUtk5CKceRLWxvapz --- .../config/model/EdgeHardeningConfig.java | 27 +++++++++++++---- .../config/model/ConfigModelContractTest.java | 19 ++++++++++++ .../validation/ConfigValidatorTest.java | 16 ++++++++++ .../gateway/edge/EdgeHardeningTest.java | 30 +++++++++++++++---- doc/configuration.adoc | 29 +++++++++++------- doc/user/protocol-routes.adoc | 6 +++- 6 files changed, 104 insertions(+), 23 deletions(-) diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java index 7c6ec29d..39136eac 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/EdgeHardeningConfig.java @@ -32,9 +32,13 @@ * HTTP keeps the remaining headroom. A {@code websocket_relay_cap} larger than {@code admission_cap} * is meaningless — the sub-budget can never bind — and is refused at boot by the config validator. *

- * Both members are optional: an omitted block, or an omitted member, resolves to the + * Both members are optional and are independently omissible. An omitted block resolves to the * {@link #defaults() documented defaults}, so an operator who never writes the block keeps today's - * behaviour. Values are validated at boot (both {@code >= 1}); this record only carries them. + * behaviour. An omitted {@code websocket_relay_cap} resolves to a quarter of the effective + * {@code admission_cap} rather than to a fixed constant, so lowering {@code admission_cap} alone + * keeps the pair consistent — the relay sub-budget follows the pool it draws from instead of + * overshooting it and self-rejecting at boot. Values are validated at boot (both {@code >= 1}); + * this record only carries them. * * @param admissionCap the maximum number of concurrently in-flight requests the edge admits, * empty when the operator did not declare one @@ -50,10 +54,13 @@ public record EdgeHardeningConfig(Optional admissionCap, Optionalthat cap rather than reading this constant, so the ratio + * — not the literal value — is what the default preserves. */ public static final int DEFAULT_WEBSOCKET_RELAY_CAP = 512; @@ -80,10 +87,18 @@ public int effectiveAdmissionCap() { } /** - * @return the declared WebSocket-relay sub-budget, or {@link #DEFAULT_WEBSOCKET_RELAY_CAP} when - * the member is absent + * The implicit relay sub-budget is derived from the pool it draws from rather than fixed, so a + * partially declared block can never resolve to a relay cap above its own admission cap: with + * {@code admission_cap: 64} and no {@code websocket_relay_cap}, this yields {@code 16} instead of + * a self-rejecting {@code 512}. At the shipped {@link #DEFAULT_ADMISSION_CAP} the quarter is + * exactly {@link #DEFAULT_WEBSOCKET_RELAY_CAP}. An explicitly declared cap is returned untouched + * — an explicit relay cap above the admission cap remains a boot-time error. + * + * @return the declared WebSocket-relay sub-budget, or — when the member is absent — a quarter of + * the {@linkplain #effectiveAdmissionCap() effective admission cap}, never below + * {@code 1} */ public int effectiveWebsocketRelayCap() { - return websocketRelayCap.orElse(DEFAULT_WEBSOCKET_RELAY_CAP); + return websocketRelayCap.orElseGet(() -> Math.max(1, effectiveAdmissionCap() / 4)); } } diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java index 4e2df441..370aa4c0 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.java @@ -468,6 +468,25 @@ void edgeHardeningConfigNormalizesAbsentCaps() { assertEquals(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP, cfg.effectiveWebsocketRelayCap()); } + @Test + void edgeHardeningConfigDerivesAbsentRelayCapFromDeclaredAdmissionCap() { + EdgeHardeningConfig cfg = new EdgeHardeningConfig(Optional.of(64), Optional.empty()); + assertEquals(64, cfg.effectiveAdmissionCap()); + assertEquals(16, cfg.effectiveWebsocketRelayCap()); + } + + @Test + void edgeHardeningConfigFloorsDerivedRelayCapAtOne() { + EdgeHardeningConfig cfg = new EdgeHardeningConfig(Optional.of(1), Optional.empty()); + assertEquals(1, cfg.effectiveWebsocketRelayCap()); + } + + @Test + void edgeHardeningConfigKeepsExplicitRelayCapAboveAdmissionCap() { + EdgeHardeningConfig cfg = new EdgeHardeningConfig(Optional.of(4), Optional.of(16)); + assertEquals(16, cfg.effectiveWebsocketRelayCap()); + } + @Test void edgeHardeningConfigDefaultsCarryBothCaps() { EdgeHardeningConfig defaults = EdgeHardeningConfig.defaults(); diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java index d4e3d3b0..29b91aff 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.java @@ -16,6 +16,7 @@ package de.cuioss.sheriff.gateway.config.validation; import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -429,6 +430,21 @@ void shouldRejectInvertedCapPair() { assertHasError(errors, "/edge_hardening/websocket_relay_cap", "must not exceed admission_cap"); } + @Test + @DisplayName("Should accept a lowered admission_cap with websocket_relay_cap omitted") + void shouldAcceptLoweredAdmissionCapWithoutRelayCap() { + GatewayConfig gateway = validGateway() + .edgeHardening(Optional.of(new EdgeHardeningConfig(Optional.of(64), Optional.empty()))).build(); + EndpointConfig endpoint = endpoint("orders", "ORDERS", List.of(), route("r", HttpMethod.GET)); + + List errors = validator.validate(gateway, List.of(endpoint), topologyWith("ORDERS")); + + assertTrue(errors.isEmpty(), + "A partially declared edge_hardening block must not self-reject, but got: " + errors); + assertEquals(16, gateway.edgeHardening().orElseThrow().effectiveWebsocketRelayCap(), + "The implicit relay sub-budget stays a quarter of the effective admission cap"); + } + @Test @DisplayName("Should accept a well-formed edge_hardening block") void shouldAcceptValidEdgeHardeningBlock() { diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java index a7e474fe..b8fa545e 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningTest.java @@ -136,18 +136,38 @@ void carriesOperatorConfiguredCaps() { } @Test - @DisplayName("falls back per member, so a half-declared block keeps the default for the omitted cap") + @DisplayName("falls back per member, deriving an omitted relay cap from the declared admission cap") void fallsBackPerOmittedMember() { - // Arrange — only the admission cap is declared + // Arrange — only the admission cap is declared, and it is raised above the default EdgeHardeningConfig partial = new EdgeHardeningConfig(Optional.of(4096), Optional.empty()); // Act EdgeHardeningOptions hardening = new EdgeHardeningOptions(partial); - // Assert — the omitted member resolves to its documented default rather than to zero + // Assert — the omitted member resolves to a quarter of the effective admission cap, so the + // documented three-quarters-for-HTTP reservation scales with the pool instead of staying + // pinned to DEFAULT_WEBSOCKET_RELAY_CAP assertEquals(4096, hardening.admissionCap(), "The declared admission_cap is carried through"); - assertEquals(EdgeHardeningConfig.DEFAULT_WEBSOCKET_RELAY_CAP, hardening.webSocketRelayCap(), - "An omitted websocket_relay_cap resolves to the documented default"); + assertEquals(1024, hardening.webSocketRelayCap(), + "An omitted websocket_relay_cap resolves to a quarter of the effective admission_cap"); + } + + @Test + @DisplayName("keeps a lowered admission cap valid, deriving a relay sub-budget that stays inside it") + void derivesRelayCapBelowALoweredAdmissionCap() { + // Arrange — the operator lowers only the admission cap, well below the shipped relay default + EdgeHardeningConfig partial = new EdgeHardeningConfig(Optional.of(64), Optional.empty()); + + // Act + EdgeHardeningOptions hardening = new EdgeHardeningOptions(partial); + + // Assert — the derived sub-budget must never exceed the pool it draws from, otherwise the + // boot-time validator would refuse a block the configuration contract documents as valid + assertEquals(64, hardening.admissionCap(), "The declared admission_cap is carried through"); + assertEquals(16, hardening.webSocketRelayCap(), + "An omitted websocket_relay_cap resolves to a quarter of the lowered admission_cap"); + assertTrue(hardening.webSocketRelayCap() <= hardening.admissionCap(), + "The relay sub-budget must stay within the admission pool it draws from"); } @Test diff --git a/doc/configuration.adoc b/doc/configuration.adoc index 73fdfe0b..e7c6b073 100644 --- a/doc/configuration.adoc +++ b/doc/configuration.adoc @@ -228,8 +228,10 @@ edge_hardening: # OPTIONAL admission budget for the public da # instead of it, because a relay holds its permit for the # connection's whole lifetime rather than one round-trip. # Minimum 1, and must not exceed admission_cap -- a larger - # value could never bind. See "Sizing the WebSocket relay - # budget" in the protocol-routes guide. + # value could never bind. Omitting this key alone takes a + # QUARTER of the effective admission_cap (>= 1), so lowering + # admission_cap alone stays valid. See "Sizing the WebSocket + # relay budget" in the protocol-routes guide. token_validation: # offline bearer validation (routes with auth.require: bearer) issuers: # one entry per accepted identity provider @@ -651,13 +653,15 @@ deployment that never writes it keeps the behaviour shown here. | WebSocket relay cap (established relays) + [`edge_hardening.websocket_relay_cap`] -| 512 +| 512 + + (a quarter of the effective `admission_cap`) | A sub-budget acquired *in addition to* the general admission permit, never instead of it. An established relay holds both permits for the connection's whole lifetime rather than for one round-trip, so without this second bound a modest number of long-lived relays would consume the general pool and starve ordinary HTTP traffic. An upgrade beyond the cap is refused `503` and releases the general permit on the way out. Operator-configurable; minimum 1, and must not exceed - `admission_cap`. + `admission_cap`. Omitting it takes a quarter of the *effective* `admission_cap` rather than a fixed + `512`, so the three-quarters-for-HTTP reservation holds at every admission cap. | Graceful-drain timeout | 25 s @@ -1421,7 +1425,8 @@ link:#_edge_hardening_defaults[Edge hardening defaults] for the full table. refuses traffic the gateway could have served. | `websocket_relay_cap` -| The maximum number of concurrently *established* WebSocket relays (default `512`, minimum `1`). +| The maximum number of concurrently *established* WebSocket relays (minimum `1`; when omitted, a + quarter of the effective `admission_cap` — `512` at the default `admission_cap` of `2048`). This is a *sub-budget acquired in addition to* the general permit, never instead of it, because an established relay holds its permit for the connection's whole lifetime rather than for one request/response round-trip. Without the second bound, a modest number of long-lived relays would @@ -1692,12 +1697,14 @@ topology leak RFC 7239 §8.2/§8.3 warn about more simply than the obfuscation t fails the boot. * `websocket.idle_timeout_seconds`, when declared, must be a *positive integer* (default `300` when omitted); it is valid only on a `protocol: websocket` route. -* The `edge_hardening` block is optional, and so is each of its members; an omission resolves to the - documented default (`admission_cap` `2048`, `websocket_relay_cap` `512`). When declared, each cap - must be at least `1` -- a cap of zero would refuse every request (or every relay) and is always a - misconfiguration rather than a deliberate posture. +* The `edge_hardening` block is optional, and so is each of its members. An omitted `admission_cap` + resolves to `2048`; an omitted `websocket_relay_cap` resolves to a *quarter of the effective* + `admission_cap` (never below `1`), which is `512` at the default `admission_cap` -- so an operator + who lowers `admission_cap` alone gets a proportionally lowered relay sub-budget rather than a + boot-time rejection. When declared, each cap must be at least `1` -- a cap of zero would refuse + every request (or every relay) and is always a misconfiguration rather than a deliberate posture. * `edge_hardening.websocket_relay_cap` must not exceed `edge_hardening.admission_cap`. The relay budget is a *sub-budget of* the admission pool -- a relay takes a permit from both -- so a larger value could never bind and would silently disable the sub-cap the operator meant to impose. The - comparison uses the *effective* values, so it holds when one member is omitted and takes its - default. + comparison uses the *effective* values; because an omitted relay cap is derived from the admission + cap it can only be tripped by an *explicitly declared* relay cap. diff --git a/doc/user/protocol-routes.adoc b/doc/user/protocol-routes.adoc index 63b3bc4d..58c46682 100644 --- a/doc/user/protocol-routes.adoc +++ b/doc/user/protocol-routes.adoc @@ -98,7 +98,11 @@ The defaults reserve three quarters of the pool for HTTP on that reasoning. Two boot-time rules bound the pair: each cap must be at least `1`, and `websocket_relay_cap` must not exceed `admission_cap` — a larger sub-budget could never bind, so it is refused rather than silently -ignored. Both keys are optional and resolve to the defaults shown above. +ignored. Both keys are optional. An omitted `admission_cap` is `2048`; an omitted +`websocket_relay_cap` is a *quarter of the effective* `admission_cap` (never below `1`), which is the +`512` shown above at the default pool size. Lowering `admission_cap` on its own therefore lowers the +relay budget with it — the three-quarters-for-HTTP reservation is preserved instead of being turned +into a boot-time rejection. == Configuring a gRPC route From 18121d4c2ecd85ddf9a583aef35a58d83029a5f0 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:43:35 +0200 Subject: [PATCH 09/11] chore(architecture): record benchmark expected-list independence insight Promoted from a recurring finalize disposition: review-bot suggestions to derive the benchmark coverage expected= list from benchmarks/pom.xml were declined twice in this plan, because independence from the pom is the property the check relies on. Co-Authored-By: Claude --- .plan/project-architecture/benchmarks/enriched.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.plan/project-architecture/benchmarks/enriched.json b/.plan/project-architecture/benchmarks/enriched.json index 40351dc9..c296e449 100644 --- a/.plan/project-architecture/benchmarks/enriched.json +++ b/.plan/project-architecture/benchmarks/enriched.json @@ -1,6 +1,8 @@ { "best_practices": [], - "insights": [], + "insights": [ + "The benchmark coverage 'expected=' list in .github/workflows/benchmark.yml is an independent oracle, not a mirror of the run-k6-* executions in benchmarks/pom.xml. Review-bot suggestions to derive it from the pom are declined: deriving it would make a goal deleted from the pom vanish from both sides and the coverage step go green, losing the silent-deletion detection that is the step's main reason to exist. Skip state is also property-driven and CLI-overridable, so a naive derivation would demand a summary for a deliberately skipped goal. Maintain the list by hand when a goal is added or un-skipped." + ], "internal_dependencies": [], "key_dependencies": [ "de.cuioss:benchmarking-common", From ad6e74b2fda2a8e9b505d4038284af23c1bc5072 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:51:31 +0200 Subject: [PATCH 10/11] docs(adr): add ADR-0018 for the WebSocket admission budget Records why a WebSocket relay holds its general admission permit for the connection lifetime and additionally acquires a derived websocket_relay_cap sub-budget, and why the omitted relay cap derives proportionally from the admission cap rather than being clamped to a constant. Co-Authored-By: Claude --- ...d_are_bounded_by_a_derived_sub-budget.adoc | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc diff --git a/doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc b/doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc new file mode 100644 index 00000000..c0a5d419 --- /dev/null +++ b/doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc @@ -0,0 +1,187 @@ += ADR-0018: WebSocket relays hold their admission permit for the connection lifetime and are bounded by a derived sub-budget +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template +// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's +// relevance without reading the full file. List fields are comma-separated. +// summary: A WebSocket relay holds its general admission permit for the connection's entire lifetime and additionally acquires a dedicated websocket_relay_cap sub-budget, both operator-configurable through the edge_hardening block; an omitted relay cap derives as a quarter of the effective admission cap so a partially declared block can never resolve to a self-rejecting pair. +// tags: edge, admission-control, websocket, denial-of-service, resource-accounting, configuration-surface, defaulting +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +// Authoring discipline (see manage-adr SKILL.md → "Authoring Discipline"): +// ADRs are durable architectural statements, not incident write-ups. No PR numbers, +// commit SHAs, dates, or lesson IDs anywhere in the body. CLAUDE.md's "current state +// only" rule applies — describe the architecture, not the chronology. + +== Status + +Proposed + +== Context + +The edge admits work through a single counting semaphore: a request acquires one permit +before a virtual thread is dispatched, and a request that cannot acquire one is refused +`503` at the cheapest possible point. The permit is released from the request's completion +hook. That accounting is exact for the request/response shape the semaphore was designed +around — acquire on arrival, release on completion, occupancy tracks concurrency. + +A WebSocket upgrade breaks the shape's central assumption. The upgrade takes the +connection over: the HTTP exchange the permit was acquired for never completes in the +sense the completion hook understands, and the connection outlives it by an unbounded +interval — minutes, hours, or the process lifetime. Two structurally different failure +modes follow, and any admission design has to choose which side of them to land on: + +* If the permit is released when the upgrade succeeds, occupancy *under-counts*. An + established relay consumes a real connection, a real virtual thread, and real buffers, + yet is invisible to the admission counter. The gateway's own measure of how loaded it is + becomes systematically wrong, and the cap stops bounding the resource it exists to bound. +* If the permit is held for the connection's lifetime, occupancy is exact but the permit is + held for an *unbounded* duration. A modest population of long-lived relays then occupies + a large share of a pool sized for short request/response turns, and ordinary HTTP traffic + is starved out of a budget it was meant to share. + +There is no release point that avoids both. The architectural question this ADR settles is +therefore not *when* to release the permit but *what kind of thing a relay is* with respect +to admission — and, once that is answered, what second bound has to exist so that the +answer does not convert a correctness fix into a starvation vector. + +A second question rides along. A cap that has to be sized against a deployment's memory +limit, traffic mix, and — for relays specifically — connection lifetime distribution cannot +be a compile-time constant, because none of those are knowable at build time. + +== Decision + +*A WebSocket relay holds its general admission permit for the connection's entire lifetime, +and additionally acquires a dedicated relay sub-budget. Both budgets are operator-facing +configuration, and the relay budget's implicit value is derived from the general budget +rather than fixed.* + +Three mechanisms realise this. + +* *Lifetime-scoped release.* The permit acquired for the upgrade request is released when + the relay closes, not when the upgrade completes. Release travels through the same + single-shot CAS guard the HTTP path uses, so it happens exactly once regardless of which + teardown path runs, and it covers *both* terminal paths — the established-relay teardown + funnel and the client-upgrade-failure branch that never constructs a relay session at + all. The guard is threaded to the relay stage rather than duplicated, so there is one + release primitive in the edge, not two that can drift. + +* *A sub-budget acquired in addition, never instead.* `websocket_relay_cap` bounds how + many of the in-flight requests may be established relays. A relay therefore holds two + permits: the general one (which keeps occupancy exact) and the relay one (which keeps + relay pressure from consuming the general pool). An upgrade beyond the relay cap is + refused `503` on the same fail-closed terms as an over-cap request. Because the sub-budget + is strictly additive, a relay cap above the admission cap can never bind, and the config + validator refuses that pair at boot rather than letting a meaningless configuration run. + +* *Proportional, not constant, defaulting.* Both members of the `edge_hardening` block are + independently omissible. An omitted block resolves to the documented defaults. An omitted + `websocket_relay_cap` resolves to a quarter of the *effective* admission cap, floored at + one — the ratio is what the default preserves, not the literal. This is what makes a + partially declared block safe: declaring only a small `admission_cap` yields a + correspondingly small relay cap instead of an implicit constant that overshoots its own + pool and is then refused at boot by the very validator that is supposed to protect the + operator. At the shipped admission default the derived quarter is exactly the documented + relay default, so the derivation is behaviour-preserving where it overlaps a constant. + +An explicitly declared relay cap is honoured verbatim, including one above the admission +cap — which stays a boot-time error. Derivation fills a gap; it never overrides an operator. + +== Consequences + +=== Positive + +* Admission occupancy is exact for every shape of work the edge admits. The counter means + what its name says, so the cap bounds the resource it is meant to bound. +* WebSocket pressure and HTTP pressure are bounded independently. Relay saturation refuses + further upgrades while leaving the remaining general headroom to ordinary traffic, so a + WebSocket-heavy workload degrades the WebSocket lane rather than the whole gateway. +* The budget is sized where the knowledge lives — in the deployment — instead of at build + time, and an operator who never writes the block keeps the shipped behaviour. +* A partially declared block is always internally consistent by construction. There is no + configuration an operator can write that the documentation promises is valid and the boot + validator then refuses. + +=== Negative + +* A relay occupies a general permit for its whole life, so the general cap must be sized + with the deployment's relay population in mind; it is no longer purely a request-rate + bound. This is the honest cost of exact accounting and is the reason the cap became + configurable in the same decision. +* Two budgets exist where one did, and an upgrade acquires and releases two permits. The + release path must stay single-shot across both teardown routes; that invariant is carried + by one shared guard rather than by convention. +* The derived relay default couples the two members: changing `admission_cap` silently + changes the implicit relay cap. This is deliberate — the coupling is what keeps the pair + consistent — but it means the relay bound is not independently stable unless declared. + +=== Risks + +* A future change that releases the general permit at upgrade time to "free the pool" + would restore the under-counting failure mode with no compiler or test signal beyond the + admission-exhaustion regression coverage. The regression that pins this runs against the + deployed container with a deliberately low cap, so it exercises the real accounting rather + than a handler in isolation. +* A relay that leaks its close signal now leaks two permits instead of one. The single-shot + guard bounds double-release, not non-release; non-release remains bounded only by the + transport's own idle reaping. + +== Alternatives Considered + +*Release the admission permit when the upgrade succeeds.* The minimal shape: treat the +upgrade as the end of the HTTP exchange and hand the permit back immediately. Rejected +because it makes the counter lie in the direction that matters. Established relays consume +connections, threads and buffers while being invisible to admission, so the cap no longer +bounds concurrency and an operator reading the gauge is misled about how loaded the gateway +is. It converts an over-counting bug into an under-counting one and calls it a fix. + +*Hold the permit but add no second bound.* Keep accounting exact and let relays draw from +the general pool alone. Rejected because it trades a WebSocket-triggered accounting defect +for a WebSocket-triggered starvation defect: long-lived relays would occupy a pool sized for +short turns and refuse HTTP traffic that has nothing to do with them. Exactness without a +sub-budget is exactly the failure the sub-budget exists to prevent. + +*Clamp the implicit relay cap at `min(fixed_default, admission_cap)`.* An obvious way to +stop a partially declared block from resolving to a self-rejecting pair. Rejected because +the clamp collapses the two budgets into one at any admission cap below the fixed default: +the relay cap equals the admission cap, the sub-budget can never bind, and relays may again +consume the entire pool — reintroducing the starvation the sub-budget exists to prevent, +precisely in the small-deployment configurations that can least absorb it. A proportional +derivation preserves the *separation* the clamp destroys. + +*Move the cap accessors onto the route that consumes them.* The caps are read by the +route that opens the semaphores, so hosting them there would shorten the path from +configuration to use. Rejected because the caps do not travel alone: the reserved-body +ceiling derives its worst-case bound from the admission cap, and the drain timeout is +sized against the same in-flight population. Splitting the group would put a derived bound +in one owner and its input in another, and the derivation — the thing that must not drift — +would span a seam. The inbound-bound group stays under one owner precisely because its +members are defined in terms of each other. + +The unifying principle: an admission counter must count the resource it claims to count, +for the whole time that resource is held. Where holding a permit for its true lifetime makes +one class of work able to monopolise a shared budget, the answer is a second bound on that +class — not a shorter, dishonest lifetime. And where a bound's implicit value must stay +consistent with another bound, derive it from that bound rather than restating it as a +constant that can contradict it. + +== References + +* link:0008-data-plane-transport.adoc[ADR-0008] — the data-plane transport the edge admits + work onto; admission runs ahead of the transport hand-off. +* link:0015-websocket-origin-failclosed-allowlist.adoc[ADR-0015] — fail-closed Origin + admission for WebSocket upgrades. That decision governs *whether* an upgrade is allowed; + this one governs *what it costs* once allowed. +* link:0017-Accept-time_SNI_split_at_a_dedicated_front_listener_passthrough_relays_opaquely_at_L4_everything_else_terminates_internally.adoc[ADR-0017] + — listener topology; passthrough L4 relays bypass the terminating listener and therefore + this admission budget entirely. +* link:../configuration.adoc[configuration.adoc] — the `edge_hardening` block's + authoritative per-key reference and its boot-validation contract. +* `api-sheriff` edge admission (`de.cuioss.sheriff.gateway.edge.EdgeHardeningOptions`, + `de.cuioss.sheriff.gateway.config.model.EdgeHardeningConfig`) — the two caps, their + derivation, and the reserved-body ceiling that shares their owner. From 27866fa2f4f7223a411b099c163896c58fd644d5 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 13:45:50 +0200 Subject: [PATCH 11/11] docs(adr): renumber the WebSocket admission-budget ADR to 0020 PR #125 landed ADR-0018 and ADR-0019 while this branch was queued, so the number this ADR was authored under is taken. The rebase was conflict-free because the filenames differ, which is exactly why the collision had to be caught by inspection rather than by git. Co-Authored-By: Claude --- ...ction_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename doc/adr/{0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc => 0020-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc} (99%) diff --git a/doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc b/doc/adr/0020-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc similarity index 99% rename from doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc rename to doc/adr/0020-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc index c0a5d419..4ccf168e 100644 --- a/doc/adr/0018-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc +++ b/doc/adr/0020-WebSocket_relays_hold_their_admission_permit_for_the_connection_lifetime_and_are_bounded_by_a_derived_sub-budget.adoc @@ -1,4 +1,4 @@ -= ADR-0018: WebSocket relays hold their admission permit for the connection lifetime and are bounded by a derived sub-budget += ADR-0020: WebSocket relays hold their admission permit for the connection lifetime and are bounded by a derived sub-budget :toc: left :toclevels: 2 :sectnums: