oscerd opened a new pull request, #26524:
URL: https://github.com/apache/camel/pull/26524
## What
Gives `evaluationMode=rest` — the **default** — a transport this component
controls: bounded timeouts, one HTTP client instead of one per message, and
`sslContextParameters`.
Closes **CAMEL-24784** (the bug) and **CAMEL-24785** (the TLS option). They
are one PR because there is exactly one seam for both: the SDK's default
transport is hardcoded, so the `SSLContext` has nowhere to go until the
transport is ours. Splitting would leave the second PR as ~20 lines that cannot
be reviewed on their own. Happy to split if you would rather.
## Why
`camel-opa` built its client with `new OPAClient(serverUrl)` / `new
OPAClient(serverUrl, headers)`. Neither passes an `HTTPClient`, so the SDK
falls back to `SpeakeasyHTTPClient`, whose entire implementation is:
```java
public HttpResponse<InputStream> send(HttpRequest request) throws ... {
HttpClient client = HttpClient.newHttpClient();
...
return client.send(request, HttpResponse.BodyHandlers.ofInputStream());
}
```
Verified against the `com.styra:opa:2.1.1` bytecode.
**1. Nothing bounds the call.** `HttpClient.newHttpClient()` sets no connect
timeout, and nothing in the SDK sets `HttpRequest.Builder.timeout` — checked
across every class in the artifact. Both JDK defaults are *wait indefinitely*.
So an OPA server that accepts the connection and then goes quiet parks the
routing thread for ever. For a component whose contract is **fail closed** this
is the worst shape available: it never reaches a decision, so it never denies —
it simply stops. `failOpen` does not rescue it either, because that branch sits
downstream of a call that never returns. A *refused* connection fails fast, so
this only appears against a host that accepts and stalls — exactly the case
that does not reproduce locally.
`evaluationMode=wasm` already had a bounded `borrowTimeout` from
CAMEL-24741. The gap was on the default mode.
**2. A new `HttpClient` per message.** On the Java 17 baseline `HttpClient`
is not `AutoCloseable`, so each one holds its selector thread and executor
until collected. `OpaProducerHealthCheck` already shares one client for
precisely this reason — and the probe fires once per health poll, while this
fires once per **message** through an `OpaSecurityPolicy`.
**3. No way to configure TLS.** No `sslContextParameters`, so an HTTPS OPA
server behind a private CA needed the certificate in the JVM-wide truststore,
and mutual TLS to the decision point was impossible. That last one matters for
the pairing these components were designed around: SPIFFE establishes *who*,
OPA decides *whether* — but a SPIFFE X.509-SVID could not be used to
authenticate to OPA, despite CAMEL-24571 adding `SpiffeSSLContextParameters`
for this exact shape.
## How
`OPAClient` has a constructor the component never used — `OPAClient(String,
HTTPClient)`. `OpaHttpClient` holds **one** `java.net.http.HttpClient` per
evaluator, built with `connectionTimeout` and the resolved `SSLContext`, and
re-issues each request carrying `requestTimeout`.
`HttpRequest.newBuilder(HttpRequest, BiPredicate)` makes the rebuild possible
without reconstructing the request by hand.
One wrinkle worth flagging: the SDK has no constructor taking a transport
**and** headers, so the bearer token moved into `OpaHttpClient`, which sets
`Authorization` on the rebuilt request. The header filter drops any incoming
`Authorization` first, because `header()` appends rather than overwrites.
`OpaBearerTokenIT` — which runs a real OPA with `--authentication=token` — is
the guard on that, and passes.
New options: `connectionTimeout` (10s), `requestTimeout` (30s),
`sslContextParameters`, plus `useGlobalSslContextParameters` on the component.
A timeout is an evaluation failure rather than a deny, so it fails closed or
proceeds under `failOpen` like anything else.
## Testing
`OpaRestTransportTest` points the component at a listener that completes the
TCP handshake and then says nothing — deliberately a bare `ServerSocket` rather
than an `HttpServer`, because the case being reproduced is a peer that is
reachable but mute.
The numbers are the argument:
| | with the fix | reverted to the SDK transport |
|---|---|---|
| `failsClosedWhenTheServerAcceptsAndThenSaysNothing` | passes in ~0.9s |
**hangs**, killed by `@Timeout(60)` |
| `proceedsOnTheSameStallWhenFailOpenIsSet` | passes in ~0.8s | **hangs**,
killed by `@Timeout(60)` |
78 tests in the module, 11 ITs including `OpaBearerTokenIT` against a real
token-authenticating OPA. Full reactor build green.
## Scope
`main` only. `camel-opa` is new and unreleased in 4.23.0, so nothing
released changes and no advisory is warranted. Additive: with no
`sslContextParameters` the TLS behaviour is what it was, and the timeouts
replace "never" with a bound.
_Claude Code on behalf of @oscerd_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
