oscerd commented on code in PR #26165:
URL: https://github.com/apache/camel/pull/26165#discussion_r3949941955


##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaEndpoint.java:
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.camel.component.opa;
+
+import com.styra.opa.OPAClient;
+import org.apache.camel.Category;
+import org.apache.camel.Component;
+import org.apache.camel.Consumer;
+import org.apache.camel.Processor;
+import org.apache.camel.Producer;
+import org.apache.camel.spi.Metadata;
+import org.apache.camel.spi.UriEndpoint;
+import org.apache.camel.spi.UriParam;
+import org.apache.camel.spi.UriPath;
+import org.apache.camel.support.DefaultEndpoint;
+
+/**
+ * Evaluate Open Policy Agent (Rego) policies against an Exchange and record 
the allow/deny decision on it.
+ */
+@UriEndpoint(firstVersion = "4.23.0", scheme = "opa", title = "OPA",
+             syntax = "opa:policyPath", producerOnly = true, category = { 
Category.SECURITY },
+             headersClass = OpaConstants.class)
+public class OpaEndpoint extends DefaultEndpoint {
+
+    @UriPath(description = "Path of the Rego rule head to evaluate, relative 
to the OPA data document. For a rule"
+                           + " named allow in a policy declaring package 
authz.orders, this is authz/orders/allow."
+                           + " The path is taken from the endpoint only: it is 
deliberately not overridable by a"
+                           + " message header, so that an inbound message 
cannot select which policy judges it.")
+    @Metadata(required = true)
+    private String policyPath;
+
+    @UriParam
+    private OpaConfiguration configuration;
+
+    private OPAClient opaClient;
+    private volatile OpaPolicyEvaluator evaluator;

Review Comment:
   Good catch on the ordering, and thanks for checking the lifecycle rather 
than just flagging the shape.
   
   Leaving it as-is, for two reasons. Camel's shutdown stops the routes and 
drains their producers before it stops the endpoints, so `process()` and 
`doStop()` do not overlap — and `camel-spiffe` nulls its `WorkloadApiClient` in 
exactly the same place, so this at least stays consistent with the neighbouring 
component.
   
   The nulling is also load-bearing rather than tidiness: `OPAClient` holds an 
`OpaApiClient` -> `OPAHTTPClient` -> `java.net.http.HttpClient`, which is not 
closeable on 17 and whose selector thread only goes away once the client is 
unreachable. Dropping the reference is what lets that happen when a stopped 
endpoint is retained in the registry. If I removed the assignments to close the 
theoretical window, a racing exchange would get a stale-but-working evaluator 
(fine in itself) at the cost of pinning that thread.
   
   Worth revisiting if the shutdown ordering ever changes, as you say — happy 
to add a null-guard in the producer if you would rather have belt and braces.
   
   _Claude Code on behalf of @oscerd_



##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.camel.component.opa;
+
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+
+import com.styra.opa.OPAClient;
+import org.apache.camel.Exchange;
+import org.apache.camel.util.ObjectHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Evaluates a Rego policy hosted by an OPA server against an {@link Exchange} 
and records the decision on it.
+ * <p/>
+ * Shared by the {@code opa:} producer and by {@code OpaSecurityPolicy} so 
that both build the same input document and
+ * read the verdict the same way.
+ */
+public class OpaPolicyEvaluator {
+
+    private static final Logger LOG = 
LoggerFactory.getLogger(OpaPolicyEvaluator.class);
+
+    private static final String ALL_HEADERS = "*";
+
+    private final OPAClient client;
+    private final String policyPath;
+    private final String allowKey;
+    private final Set<String> includedHeaders;
+    private final boolean includeBody;
+    private final boolean failOpen;
+
+    public OpaPolicyEvaluator(OPAClient client, String policyPath, String 
allowKey, String includeHeaders,
+                              boolean includeBody, boolean failOpen) {
+        this.client = ObjectHelper.notNull(client, "client");
+        this.policyPath = ObjectHelper.notNull(policyPath, "policyPath");
+        this.allowKey = ObjectHelper.isNotEmpty(allowKey) ? allowKey : "allow";
+        this.includedHeaders = parseIncludedHeaders(includeHeaders);
+        this.includeBody = includeBody;
+        this.failOpen = failOpen;
+    }
+
+    /**
+     * Creates a client for an OPA server, optionally authenticating with a 
bearer token.
+     *
+     * @param serverUrl   base URL of the OPA server, without the /v1/data 
suffix
+     * @param bearerToken token for OPA API authentication, or null when OPA 
does not require one
+     */
+    public static OPAClient createClient(String serverUrl, String bearerToken) 
{
+        if (ObjectHelper.isNotEmpty(bearerToken)) {
+            return new OPAClient(serverUrl, Map.of("Authorization", "Bearer " 
+ bearerToken));
+        }
+        return new OPAClient(serverUrl);
+    }
+
+    /**
+     * Evaluates the policy for the given exchange and sets the decision 
headers on it.
+     *
+     * @param  exchange                     the exchange to build the OPA 
input document from
+     * @return                              true when the policy allows the 
exchange to proceed
+     * @throws OpaPolicyEvaluationException when the policy could not be 
evaluated and {@code failOpen} is false
+     */
+    public boolean evaluate(Exchange exchange) throws 
OpaPolicyEvaluationException {
+        Object decision;
+        try {
+            decision = client.evaluate(policyPath, buildInput(exchange), 
Object.class);
+        } catch (Exception e) {
+            // any failure to reach a verdict is handled the same way, whether 
it comes from the OPA server
+            // (OPAException) or from building and serializing the input 
document; fail-closed must not depend
+            // on which layer gave up
+            if (failOpen) {
+                LOG.warn("Policy {} could not be evaluated, allowing the 
exchange to proceed because failOpen is"
+                         + " enabled. Reason: {}",
+                        policyPath, e.getMessage());
+                setDecisionHeaders(exchange, null, true);
+                return true;
+            }
+            throw new OpaPolicyEvaluationException(
+                    "Failed to evaluate policy " + policyPath + " at the OPA 
server", exchange, e);
+        }
+
+        boolean allowed = isAllowed(decision);
+        setDecisionHeaders(exchange, decision, allowed);
+        return allowed;
+    }
+
+    /**
+     * Builds the {@code input} document handed to OPA.
+     */
+    protected Map<String, Object> buildInput(Exchange exchange) {
+        Map<String, Object> input = new LinkedHashMap<>();
+        Map<String, Object> headers = new LinkedHashMap<>();
+        for (Map.Entry<String, Object> entry : 
exchange.getMessage().getHeaders().entrySet()) {
+            String name = entry.getKey();
+            // never feed our own decision headers back in: a policy must not 
be able to read a verdict

Review Comment:
   Agreed, the javadoc was overstating the contract. Fixed in 35f7c3e.
   
   `toJsonSafe` now says explicitly that the conversion is shallow — a `Map` or 
`List` is passed through as-is, so anything non-JSON-native nested inside it is 
left to the SDK's serializer, and a policy reading nested structures should not 
assume the same string conversion applies at depth. Added the same note to 
`opa-component.adoc`, since that is where a policy author is more likely to 
look.
   
   Left the behaviour alone: deep-converting would mean walking arbitrary 
structures on every exchange, and for the common case you describe (a `Map` 
body, which is exactly what a JSON payload converts to) passing it through is 
what makes the policy able to read it as a structure at all.
   
   _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