oscerd commented on code in PR #26679:
URL: https://github.com/apache/camel/pull/26679#discussion_r4069832923
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java:
##########
@@ -125,6 +126,98 @@ public boolean evaluate(Exchange exchange) throws
OpaPolicyEvaluationException {
*/
protected abstract Object evaluateDecision(Map<String, Object> input)
throws Exception;
+ /**
+ * Evaluates one input document per element in a single batch. The map is
keyed so a result can be matched back to
+ * its element; each value carries either the decision or the failure that
stopped it being reached. Only the REST
+ * evaluator implements this - wasm evaluates in-process, where batching
saves nothing - so the default refuses.
+ */
+ protected Map<String, BatchElement> evaluateBatchDecisions(Map<String,
Map<String, Object>> inputs) throws Exception {
+ throw new UnsupportedOperationException("batch evaluation is only
supported with evaluationMode=rest");
+ }
+
+ /**
+ * Authorizes a list in one call and records the per-element verdicts in
{@link OpaConstants#BATCH_DECISION}, a
+ * {@code List<Boolean>} parallel to the input.
+ * <p/>
+ * Fail-closed is per element: an element whose evaluation could not be
reached is denied (or allowed under
+ * {@code failOpen}) while the others decide normally. Only a batch call
that fails as a whole - the server could
+ * not be reached at all - denies (or, under {@code failOpen}, allows)
every element.
+ */
+ public List<Boolean> evaluateBatch(Exchange exchange, List<?> elements)
throws OpaPolicyEvaluationException {
+ clearDecisionHeaders(exchange);
Review Comment:
Fixed in 8bc7722. `evaluateBatch` now short-circuits an empty `List` before
the engine call and returns a deterministic empty `CamelOpaBatchDecision` (with
`CamelOpaPolicyPath` set), so an empty body can no longer be turned into a
fail-closed error by the SDK's undefined empty-batch behaviour. Verified by
`reportsAnEmptyVerdictListForAnEmptyBatch`, which also asserts via Mockito
`verify(never())` that `client.evaluateBatch` is not called.
_Claude Code on behalf of oscerd_
##########
components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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 com.styra.opa.OPAClient;
+import com.styra.opa.OPAResult;
+import org.apache.camel.BindToRegistry;
+import org.apache.camel.Exchange;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Batch evaluation authorizes a List body in one call and reports a
per-element verdict list. Fail-closed is per
+ * element: an element that could not be evaluated is denied (or allowed under
failOpen) while its neighbours decide
+ * normally (CAMEL-24740).
+ */
+public class OpaBatchEvaluationTest extends CamelTestSupport {
+
+ private static final String PATH = "authz/allow";
+
+ @BindToRegistry("opaClient")
+ private final OPAClient client = mock(OPAClient.class);
+
+ private OPAResult failed() {
+ // a batch element whose evaluation could not be reached: the server
returned a result carrying an error
+ OPAResult result = mock(OPAResult.class);
+ when(result.success()).thenReturn(false);
+ return result;
+ }
+
+ private void stubBatch(Map<String, OPAResult> results) throws Exception {
+ when(client.evaluateBatch(eq(PATH), anyMap())).thenReturn(results);
+ }
+
+ @Test
+ void reportsAVerdictParallelToEachElement() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", new OPAResult(Boolean.FALSE));
+ results.put("2", new OPAResult(Boolean.TRUE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody(List.of("alice", "mallory",
"carol")));
+
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, false, true);
+ }
+
+ @Test
+ void deniesAFailedElementButLetsItsNeighboursDecide() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", failed());
+ results.put("2", new OPAResult(Boolean.TRUE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody(List.of("a", "b", "c")));
+
+ // the middle element is denied because it could not be evaluated, not
because the whole batch failed
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, false, true);
+ }
+
+ @Test
+ void allowsAFailedElementUnderFailOpen() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", failed());
+ results.put("2", new OPAResult(Boolean.FALSE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true&failOpen=true",
+ e -> e.getMessage().setBody(List.of("a", "b", "c")));
+
+ // failOpen turns the unreachable element into an allow; the genuine
deny at index 2 is unaffected
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, true, false);
+ }
+
+ @Test
+ void requiresAListBody() {
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody("not a list"));
+
+
assertThat(out.getException()).isInstanceOf(IllegalArgumentException.class);
+ assertThat(out.getException().getMessage()).contains("List");
+ }
+
+ @Test
+ void rejectsBatchInWasmModeAtStartup() {
+ assertThatThrownBy(() -> context.getEndpoint(
+ "opa:" + PATH +
"?evaluationMode=wasm&policyBundle=classpath:authz.wasm&batch=true").start())
+ .isInstanceOf(Exception.class)
+ .hasMessageContaining("batch");
+ }
+}
Review Comment:
Added in 8bc7722: `reportsAnEmptyVerdictListForAnEmptyBatch`. It asserts the
empty batch yields an empty `CamelOpaBatchDecision`, that `CamelOpaPolicyPath`
is still set, and — via Mockito `verify(never())` — that `client.evaluateBatch`
is never called. The short-circuit in `evaluateBatch` makes the SDK's
empty-batch behaviour moot. (Confirmed the assertion fails against the un-fixed
code.)
_Claude Code on behalf of oscerd_
##########
components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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 com.styra.opa.OPAClient;
+import com.styra.opa.OPAResult;
+import org.apache.camel.BindToRegistry;
+import org.apache.camel.Exchange;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Batch evaluation authorizes a List body in one call and reports a
per-element verdict list. Fail-closed is per
+ * element: an element that could not be evaluated is denied (or allowed under
failOpen) while its neighbours decide
+ * normally (CAMEL-24740).
+ */
+public class OpaBatchEvaluationTest extends CamelTestSupport {
+
+ private static final String PATH = "authz/allow";
+
+ @BindToRegistry("opaClient")
+ private final OPAClient client = mock(OPAClient.class);
+
+ private OPAResult failed() {
+ // a batch element whose evaluation could not be reached: the server
returned a result carrying an error
+ OPAResult result = mock(OPAResult.class);
+ when(result.success()).thenReturn(false);
+ return result;
+ }
+
+ private void stubBatch(Map<String, OPAResult> results) throws Exception {
+ when(client.evaluateBatch(eq(PATH), anyMap())).thenReturn(results);
+ }
+
+ @Test
+ void reportsAVerdictParallelToEachElement() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", new OPAResult(Boolean.FALSE));
+ results.put("2", new OPAResult(Boolean.TRUE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody(List.of("alice", "mallory",
"carol")));
+
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, false, true);
+ }
+
+ @Test
+ void deniesAFailedElementButLetsItsNeighboursDecide() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", failed());
+ results.put("2", new OPAResult(Boolean.TRUE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody(List.of("a", "b", "c")));
+
+ // the middle element is denied because it could not be evaluated, not
because the whole batch failed
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, false, true);
+ }
+
+ @Test
+ void allowsAFailedElementUnderFailOpen() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", failed());
+ results.put("2", new OPAResult(Boolean.FALSE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true&failOpen=true",
+ e -> e.getMessage().setBody(List.of("a", "b", "c")));
+
+ // failOpen turns the unreachable element into an allow; the genuine
deny at index 2 is unaffected
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, true, false);
+ }
+
+ @Test
+ void requiresAListBody() {
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody("not a list"));
+
+
assertThat(out.getException()).isInstanceOf(IllegalArgumentException.class);
+ assertThat(out.getException().getMessage()).contains("List");
+ }
+
+ @Test
+ void rejectsBatchInWasmModeAtStartup() {
+ assertThatThrownBy(() -> context.getEndpoint(
+ "opa:" + PATH +
"?evaluationMode=wasm&policyBundle=classpath:authz.wasm&batch=true").start())
+ .isInstanceOf(Exception.class)
+ .hasMessageContaining("batch");
+ }
+}
Review Comment:
Added in 8bc7722, both whole-batch failure paths:
- `allowsEveryElementUnderFailOpenWhenTheWholeBatchFails` — stubs
`client.evaluateBatch` to throw `OPAException`, `failOpen=true`, asserts every
verdict is `true`.
- `failsClosedWhenTheWholeBatchCannotBeEvaluated` — same stub with
`failOpen=false`, asserts an `OpaPolicyEvaluationException` and no
`CamelOpaBatchDecision` left on the exchange.
_Claude Code on behalf of oscerd_
##########
components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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 com.styra.opa.OPAClient;
+import com.styra.opa.OPAResult;
+import org.apache.camel.BindToRegistry;
+import org.apache.camel.Exchange;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Batch evaluation authorizes a List body in one call and reports a
per-element verdict list. Fail-closed is per
+ * element: an element that could not be evaluated is denied (or allowed under
failOpen) while its neighbours decide
+ * normally (CAMEL-24740).
+ */
+public class OpaBatchEvaluationTest extends CamelTestSupport {
+
+ private static final String PATH = "authz/allow";
+
+ @BindToRegistry("opaClient")
+ private final OPAClient client = mock(OPAClient.class);
+
+ private OPAResult failed() {
+ // a batch element whose evaluation could not be reached: the server
returned a result carrying an error
+ OPAResult result = mock(OPAResult.class);
+ when(result.success()).thenReturn(false);
+ return result;
+ }
+
+ private void stubBatch(Map<String, OPAResult> results) throws Exception {
+ when(client.evaluateBatch(eq(PATH), anyMap())).thenReturn(results);
+ }
+
+ @Test
+ void reportsAVerdictParallelToEachElement() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", new OPAResult(Boolean.FALSE));
+ results.put("2", new OPAResult(Boolean.TRUE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody(List.of("alice", "mallory",
"carol")));
+
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, false, true);
+ }
+
+ @Test
+ void deniesAFailedElementButLetsItsNeighboursDecide() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", failed());
+ results.put("2", new OPAResult(Boolean.TRUE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody(List.of("a", "b", "c")));
+
+ // the middle element is denied because it could not be evaluated, not
because the whole batch failed
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, false, true);
+ }
+
+ @Test
+ void allowsAFailedElementUnderFailOpen() throws Exception {
+ Map<String, OPAResult> results = new LinkedHashMap<>();
+ results.put("0", new OPAResult(Boolean.TRUE));
+ results.put("1", failed());
+ results.put("2", new OPAResult(Boolean.FALSE));
+ stubBatch(results);
+
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true&failOpen=true",
+ e -> e.getMessage().setBody(List.of("a", "b", "c")));
+
+ // failOpen turns the unreachable element into an allow; the genuine
deny at index 2 is unaffected
+ assertThat(out.getException()).isNull();
+ assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION,
List.class))
+ .containsExactly(true, true, false);
+ }
+
+ @Test
+ void requiresAListBody() {
+ Exchange out = template.request("opa:" + PATH +
"?opaClient=#opaClient&batch=true",
+ e -> e.getMessage().setBody("not a list"));
+
+
assertThat(out.getException()).isInstanceOf(IllegalArgumentException.class);
+ assertThat(out.getException().getMessage()).contains("List");
+ }
+
+ @Test
+ void rejectsBatchInWasmModeAtStartup() {
+ assertThatThrownBy(() -> context.getEndpoint(
+ "opa:" + PATH +
"?evaluationMode=wasm&policyBundle=classpath:authz.wasm&batch=true").start())
+ .isInstanceOf(Exception.class)
+ .hasMessageContaining("batch");
+ }
+}
Review Comment:
All three are in 8bc7722:
1. Whole-batch failure — `failsClosedWhenTheWholeBatchCannotBeEvaluated`
(fail-closed, throws `OpaPolicyEvaluationException`, no decision header) and
`allowsEveryElementUnderFailOpenWhenTheWholeBatchFails` (`failOpen=true`, all
allowed).
2. Empty-list body — `reportsAnEmptyVerdictListForAnEmptyBatch` (empty
verdict list, `client.evaluateBatch` never called).
3. `CamelOpaPolicyPath` in batch mode — asserted in
`reportsAVerdictParallelToEachElement`.
The two fix-guarding assertions (empty short-circuit, policy-path header)
were verified to fail against the un-fixed code before shipping.
_Claude Code on behalf of oscerd_
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java:
##########
@@ -125,6 +126,98 @@ public boolean evaluate(Exchange exchange) throws
OpaPolicyEvaluationException {
*/
protected abstract Object evaluateDecision(Map<String, Object> input)
throws Exception;
+ /**
+ * Evaluates one input document per element in a single batch. The map is
keyed so a result can be matched back to
+ * its element; each value carries either the decision or the failure that
stopped it being reached. Only the REST
+ * evaluator implements this - wasm evaluates in-process, where batching
saves nothing - so the default refuses.
+ */
+ protected Map<String, BatchElement> evaluateBatchDecisions(Map<String,
Map<String, Object>> inputs) throws Exception {
+ throw new UnsupportedOperationException("batch evaluation is only
supported with evaluationMode=rest");
+ }
+
+ /**
+ * Authorizes a list in one call and records the per-element verdicts in
{@link OpaConstants#BATCH_DECISION}, a
+ * {@code List<Boolean>} parallel to the input.
+ * <p/>
+ * Fail-closed is per element: an element whose evaluation could not be
reached is denied (or allowed under
+ * {@code failOpen}) while the others decide normally. Only a batch call
that fails as a whole - the server could
+ * not be reached at all - denies (or, under {@code failOpen}, allows)
every element.
+ */
+ public List<Boolean> evaluateBatch(Exchange exchange, List<?> elements)
throws OpaPolicyEvaluationException {
+ clearDecisionHeaders(exchange);
+ Map<String, Map<String, Object>> inputs = new LinkedHashMap<>();
+ for (int i = 0; i < elements.size(); i++) {
+ inputs.put(Integer.toString(i), buildInput(exchange,
elements.get(i), true));
+ }
+
+ List<Boolean> verdicts = new ArrayList<>(elements.size());
+ try {
+ Map<String, BatchElement> results = evaluateBatchDecisions(inputs);
+ for (int i = 0; i < elements.size(); i++) {
+ verdicts.add(verdictFor(i, results != null ?
results.get(Integer.toString(i)) : null));
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new OpaPolicyEvaluationException(
+ "Interrupted while evaluating policy " + getPolicyPath() +
" in batch", exchange, e);
+ } catch (Exception e) {
+ // the batch call itself failed, so nothing was decided; fail
closed for every element unless failOpen
+ if (!failOpen) {
+ throw new OpaPolicyEvaluationException(
+ "Failed to evaluate policy " + getPolicyPath() + " in
batch", exchange, e);
+ }
+ LOG.warn("Batch policy {} could not be evaluated, allowing all {}
elements because failOpen is enabled."
+ + " Reason: {}",
+ getPolicyPath(), elements.size(), e.getMessage());
+ for (int i = 0; i < elements.size(); i++) {
+ verdicts.add(Boolean.TRUE);
+ }
+ }
+
+ exchange.getMessage().setHeader(OpaConstants.BATCH_DECISION, verdicts);
+ return verdicts;
Review Comment:
Fixed in 8bc7722. `evaluateBatch` now sets `CamelOpaPolicyPath` alongside
`CamelOpaBatchDecision` (via a small `setBatchDecisionHeaders` helper) on every
path that produces a verdict list — normal, per-element failure, whole-batch
`failOpen`, and the empty short-circuit — giving batch mode header parity with
single evaluation. A fail-closed whole-batch failure deliberately leaves it
unset, mirroring `evaluate`. Covered by an assertion in
`reportsAVerdictParallelToEachElement`.
_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]
