Skip to content

Commit 9bee5b7

Browse files
l46kokcopybara-github
authored andcommitted
Consolidate emit to output for aggregate policies
PiperOrigin-RevId: 961178627
1 parent 0cdb3b6 commit 9bee5b7

3 files changed

Lines changed: 15 additions & 50 deletions

File tree

policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ public CelPolicy.Rule parseRule(
298298
}
299299
hasMatch = true;
300300
ruleBuilder
301-
.addMatches(parseMatches(ctx, policyBuilder, value, false))
301+
.addMatches(parseMatches(ctx, policyBuilder, value))
302302
.setSemantic(EvaluationSemantic.FIRST_MATCH);
303303
break;
304304
case "aggregate":
@@ -307,7 +307,7 @@ public CelPolicy.Rule parseRule(
307307
}
308308
hasAggregate = true;
309309
ruleBuilder
310-
.addMatches(parseMatches(ctx, policyBuilder, value, true))
310+
.addMatches(parseMatches(ctx, policyBuilder, value))
311311
.setSemantic(EvaluationSemantic.AGGREGATE);
312312
break;
313313

@@ -320,10 +320,7 @@ public CelPolicy.Rule parseRule(
320320
}
321321

322322
private ImmutableSet<CelPolicy.Match> parseMatches(
323-
PolicyParserContext<Node> ctx,
324-
CelPolicy.Builder policyBuilder,
325-
Node node,
326-
boolean isAggregate) {
323+
PolicyParserContext<Node> ctx, CelPolicy.Builder policyBuilder, Node node) {
327324
long valueId = ctx.collectMetadata(node);
328325
ImmutableSet.Builder<CelPolicy.Match> matchesBuilder = ImmutableSet.builder();
329326
if (!assertYamlType(ctx, valueId, node, YamlNodeType.LIST)) {
@@ -332,7 +329,7 @@ private ImmutableSet<CelPolicy.Match> parseMatches(
332329

333330
SequenceNode matchListNode = (SequenceNode) node;
334331
for (Node elementNode : matchListNode.getValue()) {
335-
matchesBuilder.add(parseMatchInternal(ctx, policyBuilder, elementNode, isAggregate));
332+
matchesBuilder.add(parseMatch(ctx, policyBuilder, elementNode));
336333
}
337334

338335
return matchesBuilder.build();
@@ -341,14 +338,6 @@ private ImmutableSet<CelPolicy.Match> parseMatches(
341338
@Override
342339
public CelPolicy.Match parseMatch(
343340
PolicyParserContext<Node> ctx, CelPolicy.Builder policyBuilder, Node node) {
344-
return parseMatchInternal(ctx, policyBuilder, node, false);
345-
}
346-
347-
private CelPolicy.Match parseMatchInternal(
348-
PolicyParserContext<Node> ctx,
349-
CelPolicy.Builder policyBuilder,
350-
Node node,
351-
boolean isAggregate) {
352341
long nodeId = ctx.collectMetadata(node);
353342
if (!assertYamlType(ctx, nodeId, node, YamlNodeType.MAP)) {
354343
return ERROR_MATCH;
@@ -369,20 +358,6 @@ private CelPolicy.Match parseMatchInternal(
369358
matchBuilder.setCondition(ctx.newSourceString(value));
370359
break;
371360
case "output":
372-
if (isAggregate) {
373-
ctx.reportError(tagId, "Rule aggregate requires 'emit' tag instead of 'output'");
374-
}
375-
matchBuilder
376-
.result()
377-
.filter(result -> result.kind().equals(Match.Result.Kind.RULE))
378-
.ifPresent(
379-
result -> ctx.reportError(tagId, "Only the rule or the output may be set"));
380-
matchBuilder.setResult(Match.Result.ofOutput(ctx.newSourceString(value)));
381-
break;
382-
case "emit":
383-
if (!isAggregate) {
384-
ctx.reportError(tagId, "Rule match requires 'output' tag instead of 'emit'");
385-
}
386361
matchBuilder
387362
.result()
388363
.filter(result -> result.kind().equals(Match.Result.Kind.RULE))

policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ public void evalYamlPolicy_aggregate() throws Exception {
146146
+ "rule:\n"
147147
+ " aggregate:\n"
148148
+ " - condition: 'true'\n"
149-
+ " emit: '\"PII\"'\n"
149+
+ " output: '\"PII\"'\n"
150150
+ " - condition: 'true'\n"
151-
+ " emit: '\"CONFIDENTIAL\"'\n";
151+
+ " output: '\"CONFIDENTIAL\"'\n";
152152
Cel cel = newCel();
153153
CelPolicy policy = POLICY_PARSER.parse(policySource);
154154

@@ -166,11 +166,11 @@ public void evaluateYamlPolicy_aggregate_cseApplied() throws Exception {
166166
+ "rule:\n"
167167
+ " aggregate:\n"
168168
+ " - condition: \"size(resource.payload) > 5\"\n"
169-
+ " emit: '\"CSE1\"'\n"
169+
+ " output: '\"CSE1\"'\n"
170170
+ " - condition: \"size(resource.payload) > 5\"\n"
171-
+ " emit: '\"CSE2\"'\n"
171+
+ " output: '\"CSE2\"'\n"
172172
+ " - condition: 'true'\n"
173-
+ " emit: '\"ALWAYS\"'\n";
173+
+ " output: '\"ALWAYS\"'\n";
174174
Cel cel =
175175
newCel()
176176
.toCelBuilder()
@@ -213,7 +213,7 @@ public void compileYamlPolicy_aggregate_macrosPreserved() throws Exception {
213213
+ " - condition: \"true\"\n"
214214
+ " output: \"payload.filter(x, x > 10).exists(y, y % 2 == 0)\"\n"
215215
+ " - condition: \"true\"\n"
216-
+ " emit: \"payload.all(x, x > 0)\"\n";
216+
+ " output: \"payload.all(x, x > 0)\"\n";
217217
Cel cel =
218218
newCel()
219219
.toCelBuilder()
@@ -243,7 +243,7 @@ public void compileYamlPolicy_nestedAggregate_throws() throws Exception {
243243
+ " rule:\n"
244244
+ " aggregate:\n"
245245
+ " - condition: 'true'\n"
246-
+ " emit: \"'foo'\"\n";
246+
+ " output: \"'foo'\"\n";
247247
CelPolicy policy = POLICY_PARSER.parse(policySource);
248248

249249
CelPolicyValidationException e =
@@ -269,7 +269,7 @@ public void compileYamlPolicy_nestedAggregate_withInterveningMatch_throws() thro
269269
+ " rule:\n"
270270
+ " aggregate:\n"
271271
+ " - condition: 'true'\n"
272-
+ " emit: \"'foo'\"\n";
272+
+ " output: \"'foo'\"\n";
273273
CelPolicy policy = POLICY_PARSER.parse(policySource);
274274

275275
CelPolicyValidationException e =
@@ -292,7 +292,7 @@ public void compileYamlPolicy_aggregateUnderMatch_success() throws Exception {
292292
+ " rule:\n"
293293
+ " aggregate:\n"
294294
+ " - condition: 'true'\n"
295-
+ " emit: \"'foo'\"\n";
295+
+ " output: \"'foo'\"\n";
296296
CelPolicy policy = POLICY_PARSER.parse(policySource);
297297

298298
CelAbstractSyntaxTree ast =

policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -334,30 +334,20 @@ private enum PolicyParseErrorTestCase {
334334
+ " match:\n"
335335
+ " - output: 'true'\n"
336336
+ " aggregate:\n"
337-
+ " - emit: 'true'\n",
337+
+ " - output: 'true'\n",
338338
"ERROR: <input>:5:3: Only one of 'match' or 'aggregate' may be set in a rule\n"
339339
+ " | aggregate:\n"
340340
+ " | ..^"),
341341
BOTH_AGGREGATE_AND_MATCH_SET(
342342
"name: test\n"
343343
+ "rule:\n"
344344
+ " aggregate:\n"
345-
+ " - emit: 'true'\n"
345+
+ " - output: 'true'\n"
346346
+ " match:\n"
347347
+ " - output: 'true'\n",
348348
"ERROR: <input>:5:3: Only one of 'match' or 'aggregate' may be set in a rule\n"
349349
+ " | match:\n"
350350
+ " | ..^"),
351-
AGGREGATE_RULE_USES_OUTPUT(
352-
"name: test\n" + "rule:\n" + " aggregate:\n" + " - output: 'true'\n",
353-
"ERROR: <input>:4:7: Rule aggregate requires 'emit' tag instead of 'output'\n"
354-
+ " | - output: 'true'\n"
355-
+ " | ......^"),
356-
MATCH_RULE_USES_EMIT(
357-
"name: test\n" + "rule:\n" + " match:\n" + " - emit: 'true'\n",
358-
"ERROR: <input>:4:7: Rule match requires 'output' tag instead of 'emit'\n"
359-
+ " | - emit: 'true'\n"
360-
+ " | ......^"),
361351
ILLEGAL_YAML_TYPE_ON_RULE_VALUE(
362352
"rule: illegal",
363353
"ERROR: <input>:1:7: Got yaml node type tag:yaml.org,2002:str, wanted type(s)"

0 commit comments

Comments
 (0)