davsclaus commented on code in PR #26436:
URL: https://github.com/apache/camel/pull/26436#discussion_r4011954162
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/security/OpaSecurityPolicy.java:
##########
@@ -67,6 +70,7 @@ public OpaSecurityPolicy(String serverUrl, String policyPath)
{
@Override
public void beforeWrap(Route route, NamedNode definition) {
+ registerHealthCheck(route);
Review Comment:
+1 to registering after `notEmpty(policyPath, …)` so a misconfigured policy
doesn't leave an orphan `…/null` check in the registry.
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/security/OpaSecurityPolicy.java:
##########
@@ -77,6 +81,24 @@ public void beforeWrap(Route route, NamedNode definition) {
}
}
+ /**
+ * Registers a readiness check for the OPA server, once per policy however
many routes it wraps.
+ * <p/>
+ * Skipped when an {@code opaClient} was injected: that client can point
anywhere and this policy has no way to ask
+ * it where, so probing the configured URL would report on a server it may
never talk to.
+ */
+ private void registerHealthCheck(Route route) {
+ if (healthCheck != null || opaClient != null ||
ObjectHelper.isEmpty(serverUrl)) {
+ return;
+ }
+ HealthCheckRegistry registry =
HealthCheckRegistry.get(route.getCamelContext());
+ if (registry == null) {
+ return;
+ }
+ healthCheck = new OpaSecurityPolicyHealthCheck(serverUrl, bearerToken,
policyPath);
Review Comment:
Non-blocking: nothing unregisters this on route stop/removal.
`OpaSecurityProcessor` is a `Service`, so a ref-count here (unregister when the
last wrapped processor stops) would keep route reload clean. Bounded by the id
dedup, so fine to defer.
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/security/OpaSecurityPolicy.java:
##########
@@ -77,6 +81,24 @@ public void beforeWrap(Route route, NamedNode definition) {
}
}
+ /**
+ * Registers a readiness check for the OPA server, once per policy however
many routes it wraps.
+ * <p/>
+ * Skipped when an {@code opaClient} was injected: that client can point
anywhere and this policy has no way to ask
+ * it where, so probing the configured URL would report on a server it may
never talk to.
+ */
+ private void registerHealthCheck(Route route) {
Review Comment:
The producer check is opt-in (`healthCheckProducerEnabled`), this one is
always on and will flip readiness to DOWN when OPA is down. Defensible given
the policy hard-fails traffic, but consider a `healthCheckEnabled` property
(default true) and a doc line on how to disable it, so operators aren't
reaching for `camel.health.exclude-pattern`.
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaProducerHealthCheck.java:
##########
@@ -16,16 +16,10 @@
*/
package org.apache.camel.component.opa;
-import java.net.URI;
-import java.net.http.HttpClient;
-import java.net.http.HttpRequest;
-import java.net.http.HttpResponse;
-import java.time.Duration;
import java.util.Map;
import org.apache.camel.health.HealthCheckResultBuilder;
import org.apache.camel.impl.health.AbstractHealthCheck;
-import org.apache.camel.util.ObjectHelper;
import org.apache.camel.util.URISupport;
Review Comment:
Note the earlier suggestion to drop this import would break the build —
`URISupport.sanitizeUri(id)` is still used in the constructor below. Keep it.
--
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]
