oscerd commented on PR #26670:
URL: https://github.com/apache/camel/pull/26670#issuecomment-5810048548

   Fixed in `0b922966`, and you were right about the direction.
   
   I checked both sides before touching anything:
   
   | | `opaClient` | `bearerToken` | `serverUrl` |
   |---|---|---|---|
   | `OpaSecurityPolicy.warnIgnoredServerOptions` | warns | warns | warns |
   | `OpaEndpoint.warnAboutIgnoredServerOptions` (before) | **silent** | warns 
| warns |
   
   And `OpaEndpoint`'s wasm branch does go straight to `createWasmEvaluator()` 
without reading `configuration.getOpaClient()` — only the `rest` branch reads 
it, at line 82 — so an injected client was being dropped there with no signal 
at all. The endpoint now warns with the same wording, and its javadoc says it 
is kept in step with the policy, since the two configure the same evaluators 
and an option warned about by one and dropped by the other is a trap for anyone 
moving a policy path between the producer and `.policy(...)`.
   
   **On where it belongs:** you suggested it might be #26669's file rather than 
this PR's. #26669 is merged, so this is where it can land.
   
   **On testing it, which I want to be straight about rather than quietly 
skip:** there is no test. The change is a log line, and neither of the two 
warnings it joins is covered today on either class. Asserting on it needs a 
log-capturing appender, and `LogCaptureAppender` is not shared — `camel-netty` 
and `camel-netty-http` each carry their own copy, so covering this one line 
means adding a third to `camel-opa`. That felt disproportionate for a WARN, but 
if you would rather have the coverage, the version worth writing pins all three 
warnings on both classes, which is the thing that actually decays. Happy to do 
that here or as a follow-up, your call.
   
   `camel-opa` unit tests green with the change.
   
   **Merge-order:** this file references `classpath:authz.wasm` at lines 47 and 
160, which #26677 deletes. I have deliberately not pre-rebased either PR — 
whichever lands second gets the rebase then, rather than me guessing an order. 
Cross-referenced on #26677.
   
   _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]

Reply via email to