gnodet-bot commented on code in PR #26679:
URL: https://github.com/apache/camel/pull/26679#discussion_r4069897420
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaPolicyEvaluator.java:
##########
@@ -125,6 +126,106 @@ 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);
+ // 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));
Review Comment:
💡 **`includeBody=false` silently overridden in batch mode**
`buildInput` is called with a hardcoded `true` for `withBody`, which means
the batch path always sends each element as the `body` field — regardless of
the user's `includeBody=false` endpoint configuration. The intent is correct
(the element *is* the body in batch mode), but users who have set
`includeBody=false` to keep the OPA input compact will be surprised that batch
sends a body anyway.
Two things are needed:
1. The `setBatch()` Javadoc (and `@UriParam` description) should state that
`includeBody` is ignored in batch mode — each element is always sent as the
`body`.
2. A test that configures `includeBody=false` and verifies that batch
evaluation still includes each element in the `body` slot of the input (or
alternatively, explicitly documents that `includeBody` is irrelevant in batch
mode).
Without either, an operator who reads the `includeBody` description
(`"Whether to send the message body to OPA as part of the input document.
Disabled by default"`) and then enables batch will have no indication that
their `includeBody=false` setting is ignored.
##########
components/camel-opa/src/main/java/org/apache/camel/component/opa/OpaConfiguration.java:
##########
@@ -281,6 +284,23 @@ public void setFailOpen(boolean failOpen) {
this.failOpen = failOpen;
}
+ public boolean isBatch() {
+ return batch;
+ }
+
+ /**
+ * Authorize a whole collection in one call. When enabled the producer
expects a {@code List} body, evaluates one
+ * input document per element - each element as the {@code body}, sharing
the exchange's headers and properties -
+ * and returns the per-element verdicts in the {@code
CamelOpaBatchDecision} header, a {@code List<Boolean>}
+ * parallel to the input. An element whose evaluation could not be reached
is denied, unless {@code failOpen} is
+ * set; the batch is never allowed or denied as a whole because one
element failed. Only for
+ * {@code evaluationMode=rest}: it saves the per-element HTTP round-trip
via OPA's batch API, which has no meaning
+ * for in-process {@code wasm}.
+ */
+ public void setBatch(boolean batch) {
Review Comment:
📝 **Javadoc: mention `includeBody` interaction**
The `setBatch()` Javadoc thoroughly covers the contract (per-element
fail-closed, REST-only, empty list behaviour) but says nothing about how
`includeBody` interacts with batch mode. Since `evaluateBatch` always calls
`buildInput(exchange, element, true)` — ignoring the `includeBody` field — the
Javadoc should say so explicitly. Suggested addition at the end of the
description:
```suggestion
* Authorize a whole collection in one call. When enabled the producer
expects a {@code List} body, evaluates one
* input document per element - each element as the {@code body},
sharing the exchange's headers and properties -
* and returns the per-element verdicts in the {@code
CamelOpaBatchDecision} header, a {@code List<Boolean>}
* parallel to the input. An element whose evaluation could not be
reached is denied, unless {@code failOpen} is
* set; the batch is never allowed or denied as a whole because one
element failed. Only for
* {@code evaluationMode=rest}: it saves the per-element HTTP round-trip
via OPA's batch API, which has no meaning
* for in-process {@code wasm}. The {@code includeBody} option has no
effect in batch mode: each element is always
* sent as the {@code body} field of its input document.
```
--
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]
