This is an automated email from the ASF dual-hosted git repository. oscerd pushed a commit to branch fix/CAMEL-24740 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 8bc772264f1b56ce054d9e3a8abd5d0a6d3ccf42 Author: Andrea Cosentino <[email protected]> AuthorDate: Tue Sep 22 10:37:07 2026 +0200 CAMEL-24740: camel-opa - short-circuit an empty batch, set the policy-path header, and cover whole-batch failure Addresses review feedback on batch evaluation: - an empty List body is answered without calling the OPA SDK, whose behaviour on an empty batch is undefined, so the verdict list is a deterministic empty list rather than a possible fail-closed error. - CamelOpaPolicyPath is now set in batch mode too, so observability tooling reads the same header it does after a single evaluation. - added tests for the whole-batch failure path, both fail-closed and under failOpen, and for the empty-batch short-circuit. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Andrea Cosentino <[email protected]> --- .../camel/component/opa/OpaPolicyEvaluator.java | 21 ++++++++- .../component/opa/OpaBatchEvaluationTest.java | 50 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java b/components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java index c0285a8b6a8d..690c450b43a5 100644 --- a/components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java +++ b/components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java @@ -145,6 +145,14 @@ public abstract class OpaPolicyEvaluator { */ public List<Boolean> evaluateBatch(Exchange exchange, List<?> elements) throws OpaPolicyEvaluationException { clearDecisionHeaders(exchange); + // an empty list has nothing to authorize. Short-circuit before the engine call: an empty batch is a case the + // OPA SDK does not define, and letting it reach the server could turn "nothing to decide" into a whole-batch + // failure - and so, fail-closed, into an error thrown for an empty list. Answer it deterministically instead. + if (elements.isEmpty()) { + List<Boolean> verdicts = List.of(); + setBatchDecisionHeaders(exchange, verdicts); + return verdicts; + } 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)); @@ -174,7 +182,7 @@ public abstract class OpaPolicyEvaluator { } } - exchange.getMessage().setHeader(OpaConstants.BATCH_DECISION, verdicts); + setBatchDecisionHeaders(exchange, verdicts); return verdicts; } @@ -344,6 +352,17 @@ public abstract class OpaPolicyEvaluator { exchange.getMessage().setHeader(OpaConstants.POLICY_PATH, policyPath); } + /** + * Records the batch outcome on the exchange: the per-element verdict list, and the policy path, so that tooling + * reading {@link OpaConstants#POLICY_PATH} sees the same value in batch mode as it does after a single evaluation. + * As in {@link #evaluate}, a fail-closed batch failure never reaches here, leaving the exchange carrying no + * verdict. + */ + private void setBatchDecisionHeaders(Exchange exchange, List<Boolean> verdicts) { + exchange.getMessage().setHeader(OpaConstants.BATCH_DECISION, verdicts); + exchange.getMessage().setHeader(OpaConstants.POLICY_PATH, policyPath); + } + private static boolean isIncluded(Set<String> filter, String name) { return filter == null || filter.contains(name); } diff --git a/components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java b/components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java index e20273f7c528..ab5e51c91135 100644 --- a/components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java +++ b/components/camel-opa/src/test/java/org/apache/camel/component/opa/OpaBatchEvaluationTest.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Map; import com.styra.opa.OPAClient; +import com.styra.opa.OPAException; import com.styra.opa.OPAResult; import org.apache.camel.BindToRegistry; import org.apache.camel.Exchange; @@ -30,8 +31,11 @@ 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.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** @@ -71,6 +75,9 @@ public class OpaBatchEvaluationTest extends CamelTestSupport { assertThat(out.getException()).isNull(); assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION, List.class)) .containsExactly(true, false, true); + // batch mode must set the policy-path header too, so observability tooling reads the same + // CamelOpaPolicyPath as it does after a single evaluation + assertThat(out.getMessage().getHeader(OpaConstants.POLICY_PATH, String.class)).isEqualTo(PATH); } @Test @@ -123,4 +130,47 @@ public class OpaBatchEvaluationTest extends CamelTestSupport { .isInstanceOf(Exception.class) .hasMessageContaining("batch"); } + + @Test + void reportsAnEmptyVerdictListForAnEmptyBatch() throws Exception { + // an empty list is answered without calling the SDK: an empty batch input is undefined there, so the + // short-circuit makes it a deterministic empty verdict list. The policy-path header is still set. + Exchange out = template.request("opa:" + PATH + "?opaClient=#opaClient&batch=true", + e -> e.getMessage().setBody(List.of())); + + assertThat(out.getException()).isNull(); + assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION, List.class)).isEmpty(); + assertThat(out.getMessage().getHeader(OpaConstants.POLICY_PATH, String.class)).isEqualTo(PATH); + verify(client, never()).evaluateBatch(anyString(), anyMap()); + } + + @Test + void failsClosedWhenTheWholeBatchCannotBeEvaluated() throws Exception { + // the batch call itself fails - the server could not be reached at all - so nothing was decided. With + // failOpen off, every element is denied by failing the exchange, not by returning a verdict list. + when(client.evaluateBatch(eq(PATH), anyMap())).thenThrow(new OPAException("connection refused")); + + Exchange out = template.request("opa:" + PATH + "?opaClient=#opaClient&batch=true", + e -> e.getMessage().setBody(List.of("a", "b"))); + + assertThat(out.getException()) + .isInstanceOf(OpaPolicyEvaluationException.class) + .hasMessageContaining("in batch"); + // fail-closed leaves no verdict on the exchange, mirroring the single-evaluation failure path + assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION)).isNull(); + } + + @Test + void allowsEveryElementUnderFailOpenWhenTheWholeBatchFails() throws Exception { + // failOpen turns a whole-batch failure into an allow for every element, parallel to the single-evaluation + // failOpen path + when(client.evaluateBatch(eq(PATH), anyMap())).thenThrow(new OPAException("connection refused")); + + Exchange out = template.request("opa:" + PATH + "?opaClient=#opaClient&batch=true&failOpen=true", + e -> e.getMessage().setBody(List.of("a", "b", "c"))); + + assertThat(out.getException()).isNull(); + assertThat(out.getMessage().getHeader(OpaConstants.BATCH_DECISION, List.class)) + .containsExactly(true, true, true); + } }
