oscerd commented on PR #26524: URL: https://github.com/apache/camel/pull/26524#issuecomment-5710668789
Thanks — and sorry for the broken build; that one is on the branch, not on you finding it. All six addressed. Notes where the outcome differed from the suggestion or where I checked something rather than taking it on trust. **1. Compile error — fixed.** Reproduced locally before touching anything (`createClient ... cannot be applied`, `constructor OpaRestEvaluator ... cannot be applied`, both in `OpaSecurityPolicy`). `68b0ec2f` changed `OpaRestEvaluator`'s signatures and updated `OpaEndpoint` but not the policy. It now uses `createTransport` + `createClient(String, OpaHttpClient)` + the transport-carrying constructor, like the endpoint. **2. Probe TLS — plumbed rather than documented.** You are right that this is the case the PR exists to enable, and a readiness check that fails a handshake the decision call passes is not a gap worth documenting — it is an application that never comes up. `OpaHealthProbe` now keeps one `HttpClient` per distinct `SSLContext` (`ConcurrentHashMap`, never evicted — bounded in practice, since the key comes from an endpoint's `sslContextParameters` and a deployment has one or two, not one per exchange). Both checks carry the context; the endpoint exposes the one it resolved so the producer's check probes the same way. Worth saying that this is the same shape as the trailing-slash bug you found on #26436 — a healthy server reported DOWN — which is twice now that the probe has diverged from the thing it is probing. **3. Empty bearer token — fixed,** and thank you, that was a regression I introduced by moving the header off `OPAClient(String, Map)`. `ObjectHelper.isNotEmpty` now, and `sendsNoAuthorizationAtAllWhenTheTokenIsEmpty` pins it: it goes red with the guard reverted, asserting the header is absent rather than `Bearer `. **4. FQCN — fixed,** imported and the class is no longer package-private by accident. **5. Lifecycle — taken.** `route.getCamelContext().addService(evaluator)` in `beforeWrap`, so the policy's transport closes on shutdown. `addService` throws checked and `beforeWrap` cannot, so it is wrapped in `RuntimeCamelException` — same treatment as `createSslContext` above it. **6. Tests — both added.** `reachesAnHttpsServerWhenSslContextParametersTrustIt` against an HTTPS listener with a throwaway self-signed certificate, and `failsClosedAgainstTheSameServerWithoutTheTrustMaterial` — same listener, same policy, only the trust material missing, so a pass there would mean the option was never consulted. The fixture is generated with `keytool` and its provenance recorded in `README-tls-fixture.md`, having been bitten on this PR's predecessor by a committed binary that had drifted from its source. `connectionTimeout` has no test: triggering a connect timeout deterministically needs a blackholed address, which is environment-dependent and would be flaky in CI. The request timeout is the half that matters and it is covered. 82 tests in the module, 11 ITs, full reactor green. _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]
