diff --git a/optimizer/src/main/java/dev/cel/optimizer/AstMutator.java b/optimizer/src/main/java/dev/cel/optimizer/AstMutator.java index 59f842e29..0f428f75a 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/AstMutator.java +++ b/optimizer/src/main/java/dev/cel/optimizer/AstMutator.java @@ -664,8 +664,8 @@ private CelMutableSource mangleIdentsInMacroSource( return newSource; } - private static CelMutableSource combine( - CelMutableSource celSource1, CelMutableSource celSource2) { + /** Combines two {@link CelMutableSource} instances into a single new instance. */ + public static CelMutableSource combine(CelMutableSource celSource1, CelMutableSource celSource2) { return CelMutableSource.newInstance() .setDescription( Strings.isNullOrEmpty(celSource1.getDescription()) @@ -677,6 +677,7 @@ private static CelMutableSource combine( .addAllMacroCalls(celSource2.getMacroCalls()); } + /** * Stabilizes the incoming AST by ensuring that all of expr IDs are consistently renumbered * (monotonically increased) from the starting seed ID. If the AST contains any macro calls, its diff --git a/policy/src/main/java/dev/cel/policy/BUILD.bazel b/policy/src/main/java/dev/cel/policy/BUILD.bazel index e0d6af461..79c7a1f05 100644 --- a/policy/src/main/java/dev/cel/policy/BUILD.bazel +++ b/policy/src/main/java/dev/cel/policy/BUILD.bazel @@ -179,6 +179,7 @@ java_library( name = "compiled_rule", srcs = ["CelCompiledRule.java"], deps = [ + ":policy", "//:auto_value", "//bundle:cel", "//common:cel_ast", @@ -246,6 +247,7 @@ java_library( srcs = ["RuleComposer.java"], deps = [ ":compiled_rule", + ":policy", "//bundle:cel", "//common:cel_ast", "//common:compiler_common", @@ -256,11 +258,13 @@ java_library( "//common/ast:mutable_expr", "//common/formats:value_string", "//common/navigation:mutable_navigation", + "//common/types", "//common/types:cel_types", "//common/types:type_providers", "//extensions:optional_library", "//optimizer:ast_optimizer", "//optimizer:mutable_ast", "@maven//:com_google_guava_guava", + "@maven//:org_jspecify_jspecify", ], ) diff --git a/policy/src/main/java/dev/cel/policy/CelCompiledRule.java b/policy/src/main/java/dev/cel/policy/CelCompiledRule.java index af40bd74f..ccacfd628 100644 --- a/policy/src/main/java/dev/cel/policy/CelCompiledRule.java +++ b/policy/src/main/java/dev/cel/policy/CelCompiledRule.java @@ -23,6 +23,7 @@ import dev.cel.common.ast.CelConstant; import dev.cel.common.ast.CelExpr; import dev.cel.common.formats.ValueString; +import dev.cel.policy.CelPolicy.EvaluationSemantic; import java.util.Optional; /** @@ -43,11 +44,20 @@ public abstract class CelCompiledRule { public abstract Cel cel(); + public abstract EvaluationSemantic semantic(); + /** * HasOptionalOutput returns whether the rule returns a concrete or optional value. The rule may * return an optional value if all match expressions under the rule are conditional. */ public boolean hasOptionalOutput() { + // AGGREGATE rules always return a concrete list (falling back to an empty list rather than + // optional.none()), meaning they are never optional structurally. This also prevents dead + // code evasion inside parent FIRST_MATCH rules. + if (semantic() == EvaluationSemantic.AGGREGATE) { + return false; + } + boolean isOptionalOutput = false; for (CelCompiledMatch match : matches()) { if (match.result().kind().equals(CelCompiledMatch.Result.Kind.RULE) @@ -157,7 +167,8 @@ static CelCompiledRule create( Optional ruleId, ImmutableList variables, ImmutableList matches, - Cel cel) { - return new AutoValue_CelCompiledRule(sourceId, ruleId, variables, matches, cel); + Cel cel, + CelPolicy.EvaluationSemantic semantic) { + return new AutoValue_CelCompiledRule(sourceId, ruleId, variables, matches, cel, semantic); } } diff --git a/policy/src/main/java/dev/cel/policy/CelPolicy.java b/policy/src/main/java/dev/cel/policy/CelPolicy.java index 6756481df..2df6f4e5f 100644 --- a/policy/src/main/java/dev/cel/policy/CelPolicy.java +++ b/policy/src/main/java/dev/cel/policy/CelPolicy.java @@ -40,6 +40,12 @@ @AutoValue public abstract class CelPolicy { + /** Evaluation semantic for a rule. */ + public enum EvaluationSemantic { + FIRST_MATCH, + AGGREGATE + } + public abstract ValueString name(); public abstract Optional description(); @@ -176,12 +182,15 @@ public abstract static class Rule { public abstract ImmutableSet matches(); + public abstract EvaluationSemantic semantic(); + /** Builder for {@link Rule}. */ public static Builder newBuilder(long id) { return new AutoValue_CelPolicy_Rule.Builder() .setId(id) .setVariables(ImmutableSet.of()) - .setMatches(ImmutableSet.of()); + .setMatches(ImmutableSet.of()) + .setSemantic(EvaluationSemantic.FIRST_MATCH); } /** Creates a new builder to construct a {@link Rule} instance. */ @@ -228,6 +237,8 @@ public Builder addMatches(Iterable matches) { abstract Rule.Builder setMatches(ImmutableSet matches); + public abstract Rule.Builder setSemantic(EvaluationSemantic semantic); + public abstract Rule build(); } } diff --git a/policy/src/main/java/dev/cel/policy/CelPolicyCompilerImpl.java b/policy/src/main/java/dev/cel/policy/CelPolicyCompilerImpl.java index 37a79f98a..588e8942d 100644 --- a/policy/src/main/java/dev/cel/policy/CelPolicyCompilerImpl.java +++ b/policy/src/main/java/dev/cel/policy/CelPolicyCompilerImpl.java @@ -44,6 +44,7 @@ import dev.cel.policy.CelCompiledRule.CelCompiledMatch.Result; import dev.cel.policy.CelCompiledRule.CelCompiledMatch.Result.Kind; import dev.cel.policy.CelCompiledRule.CelCompiledVariable; +import dev.cel.policy.CelPolicy.EvaluationSemantic; import dev.cel.policy.CelPolicy.Import; import dev.cel.policy.CelPolicy.Match; import dev.cel.policy.CelPolicy.Variable; @@ -240,7 +241,12 @@ private CelCompiledRule compileRuleImpl( CelCompiledRule compiledRule = CelCompiledRule.create( - rule.id(), rule.ruleId(), variableBuilder.build(), matchBuilder.build(), ruleCel); + rule.id(), + rule.ruleId(), + variableBuilder.build(), + matchBuilder.build(), + ruleCel, + rule.semantic()); // Validate that all branches in the policy are reachable checkUnreachableCode(compiledRule, compilerContext); @@ -255,6 +261,12 @@ private void checkUnreachableCode(CelCompiledRule compiledRule, CompilerContext CelCompiledMatch compiledMatch = compiledMatches.get(i); boolean isTriviallyTrue = compiledMatch.isConditionTriviallyTrue(); + // Flag literally false conditions as dead code regardless of semantic + if (isConditionLiterallyFalse(compiledMatch.condition())) { + compilerContext.addIssue( + compiledMatch.sourceId(), CelIssue.formatError(1, 0, "Condition is always false")); + } + // If the match is a single output or a nested rule that always returns a value, it is // exhaustive. If the condition is trivially true, then all subsequent branches are // unreachable. @@ -263,7 +275,9 @@ private void checkUnreachableCode(CelCompiledRule compiledRule, CompilerContext && (compiledMatch.result().kind().equals(Kind.OUTPUT) || !compiledMatch.result().rule().hasOptionalOutput()); - if (isExhaustive && i != matchCount - 1) { + if (compiledRule.semantic() == EvaluationSemantic.FIRST_MATCH + && isExhaustive + && i != matchCount - 1) { if (compiledMatch.result().kind().equals(Kind.OUTPUT)) { compilerContext.addIssue( compiledMatch.sourceId(), @@ -277,6 +291,12 @@ private void checkUnreachableCode(CelCompiledRule compiledRule, CompilerContext } } + private static boolean isConditionLiterallyFalse(CelAbstractSyntaxTree condition) { + CelExpr celExpr = condition.getExpr(); + return celExpr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE) + && !celExpr.constant().booleanValue(); + } + private static CelAbstractSyntaxTree newErrorAst() { return CelAbstractSyntaxTree.newParsedAst( CelExpr.ofConstant(0, CelConstant.ofValue("*error*")), CelSource.newBuilder().build()); diff --git a/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java b/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java index 408c86247..09702e77c 100644 --- a/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java +++ b/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java @@ -28,6 +28,7 @@ import dev.cel.common.formats.YamlHelper.YamlNodeType; import dev.cel.common.formats.YamlParserContextImpl; import dev.cel.common.internal.CelCodePointArray; +import dev.cel.policy.CelPolicy.EvaluationSemantic; import dev.cel.policy.CelPolicy.Import; import dev.cel.policy.CelPolicy.Invariant; import dev.cel.policy.CelPolicy.Match; @@ -271,6 +272,8 @@ public CelPolicy.Rule parseRule( return ruleBuilder.build(); } + boolean hasMatch = false; + boolean hasAggregate = false; for (NodeTuple nodeTuple : ((MappingNode) node).getValue()) { Node key = nodeTuple.getKeyNode(); long tagId = ctx.collectMetadata(key); @@ -290,8 +293,24 @@ public CelPolicy.Rule parseRule( ruleBuilder.addVariables(parseVariables(ctx, policyBuilder, value)); break; case "match": - ruleBuilder.addMatches(parseMatches(ctx, policyBuilder, value)); + if (hasAggregate) { + ctx.reportError(tagId, "Only one of 'match' or 'aggregate' may be set in a rule"); + } + hasMatch = true; + ruleBuilder + .addMatches(parseMatches(ctx, policyBuilder, value, false)) + .setSemantic(EvaluationSemantic.FIRST_MATCH); + break; + case "aggregate": + if (hasMatch) { + ctx.reportError(tagId, "Only one of 'match' or 'aggregate' may be set in a rule"); + } + hasAggregate = true; + ruleBuilder + .addMatches(parseMatches(ctx, policyBuilder, value, true)) + .setSemantic(EvaluationSemantic.AGGREGATE); break; + default: tagVisitor.visitRuleTag(ctx, tagId, fieldName, value, policyBuilder, ruleBuilder); break; @@ -301,7 +320,10 @@ public CelPolicy.Rule parseRule( } private ImmutableSet parseMatches( - PolicyParserContext ctx, CelPolicy.Builder policyBuilder, Node node) { + PolicyParserContext ctx, + CelPolicy.Builder policyBuilder, + Node node, + boolean isAggregate) { long valueId = ctx.collectMetadata(node); ImmutableSet.Builder matchesBuilder = ImmutableSet.builder(); if (!assertYamlType(ctx, valueId, node, YamlNodeType.LIST)) { @@ -310,7 +332,7 @@ private ImmutableSet parseMatches( SequenceNode matchListNode = (SequenceNode) node; for (Node elementNode : matchListNode.getValue()) { - matchesBuilder.add(parseMatch(ctx, policyBuilder, elementNode)); + matchesBuilder.add(parseMatchInternal(ctx, policyBuilder, elementNode, isAggregate)); } return matchesBuilder.build(); @@ -319,6 +341,14 @@ private ImmutableSet parseMatches( @Override public CelPolicy.Match parseMatch( PolicyParserContext ctx, CelPolicy.Builder policyBuilder, Node node) { + return parseMatchInternal(ctx, policyBuilder, node, false); + } + + private CelPolicy.Match parseMatchInternal( + PolicyParserContext ctx, + CelPolicy.Builder policyBuilder, + Node node, + boolean isAggregate) { long nodeId = ctx.collectMetadata(node); if (!assertYamlType(ctx, nodeId, node, YamlNodeType.MAP)) { return ERROR_MATCH; @@ -339,6 +369,20 @@ public CelPolicy.Match parseMatch( matchBuilder.setCondition(ctx.newSourceString(value)); break; case "output": + if (isAggregate) { + ctx.reportError(tagId, "Rule aggregate requires 'emit' tag instead of 'output'"); + } + matchBuilder + .result() + .filter(result -> result.kind().equals(Match.Result.Kind.RULE)) + .ifPresent( + result -> ctx.reportError(tagId, "Only the rule or the output may be set")); + matchBuilder.setResult(Match.Result.ofOutput(ctx.newSourceString(value))); + break; + case "emit": + if (!isAggregate) { + ctx.reportError(tagId, "Rule match requires 'output' tag instead of 'emit'"); + } matchBuilder .result() .filter(result -> result.kind().equals(Match.Result.Kind.RULE)) diff --git a/policy/src/main/java/dev/cel/policy/RuleComposer.java b/policy/src/main/java/dev/cel/policy/RuleComposer.java index 73d31a4ee..bf667cb93 100644 --- a/policy/src/main/java/dev/cel/policy/RuleComposer.java +++ b/policy/src/main/java/dev/cel/policy/RuleComposer.java @@ -30,11 +30,14 @@ import dev.cel.common.ast.CelConstant; import dev.cel.common.ast.CelExpr.ExprKind.Kind; import dev.cel.common.ast.CelMutableExpr; +import dev.cel.common.ast.CelMutableExpr.CelMutableList; import dev.cel.common.formats.ValueString; import dev.cel.common.navigation.CelNavigableMutableAst; import dev.cel.common.navigation.CelNavigableMutableExpr; import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypes; +import dev.cel.common.types.ListType; +import dev.cel.common.types.OptionalType; import dev.cel.extensions.CelOptionalLibrary.Function; import dev.cel.optimizer.AstMutator; import dev.cel.optimizer.CelAstOptimizer; @@ -45,6 +48,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.jspecify.annotations.Nullable; /** Package-private class for composing various rules into a single expression using optimizer. */ final class RuleComposer implements CelAstOptimizer { @@ -54,11 +58,11 @@ final class RuleComposer implements CelAstOptimizer { @Override public OptimizationResult optimize(CelAbstractSyntaxTree ast, Cel cel) { - Step result = optimizeRule(cel, compiledRule); + Step result = optimizeRule(cel, compiledRule, /* asList= */ false); return OptimizationResult.create(result.expr.toParsedAst()); } - private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) { + private Step optimizeRule(Cel cel, CelCompiledRule compiledRule, boolean asList) { cel = cel.toCelBuilder() .addVarDeclarations( @@ -67,15 +71,10 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) { .collect(toImmutableList())) .build(); - Step output = null; - // If the rule has an optional output, the last result in the ternary should return - // `optional.none`. This output is implicit and created here to reflect the desired - // last possible output of this type of rule. - if (compiledRule.hasOptionalOutput()) { - output = - Step.newUnconditionalOptionalStep( - newTrueLiteral(), astMutator.newGlobalCall(Function.OPTIONAL_NONE.getFunction())); - } + boolean isAggregate = compiledRule.semantic() == CelPolicy.EvaluationSemantic.AGGREGATE; + boolean returnList = isAggregate || asList; + + Step output = createBaseStep(returnList, compiledRule.hasOptionalOutput()); long lastOutputId = 0; // The expected output type of the rule, used to verify that all branches agree on the type. @@ -85,59 +84,55 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) { boolean isTriviallyTrue = match.isConditionTriviallyTrue(); CelMutableAst condAst = CelMutableAst.fromCelAst(conditionAst); - long currentSourceId = lastOutputId; + Step currentStep; + long currentSourceId; + String validationMessage; switch (match.result().kind()) { case OUTPUT: + OutputValue matchOutput = match.result().output(); // If the match has an output, then it is considered a non-optional output since // it is explicitly stated. If the rule itself is optional, then the base case value // of output being optional.none() will convert the non-optional value to an optional // one. - OutputValue matchOutput = match.result().output(); - Step step = + CelMutableAst matchOutputAst = CelMutableAst.fromCelAst(matchOutput.ast()); + currentStep = Step.newNonOptionalStep( - !isTriviallyTrue, condAst, CelMutableAst.fromCelAst(matchOutput.ast())); - currentSourceId = matchOutput.sourceId(); + !isTriviallyTrue, condAst, returnList ? newList(matchOutputAst) : matchOutputAst); - output = combine(astMutator, step, output); - - String outputFailureMessage = - String.format( - "incompatible output types: block has output type %s, but previous outputs have" - + " type %s", - lastOutputType == null ? "" : CelTypes.format(lastOutputType), - CelTypes.format(matchOutput.ast().getResultType())); - lastOutputType = - assertComposedAstIsValid( - cel, output.expr, outputFailureMessage, currentSourceId, lastOutputId) - .getResultType(); + currentSourceId = matchOutput.sourceId(); + validationMessage = + incompatibleOutputTypesMessage( + lastOutputType, + matchOutput.ast().getResultType(), + returnList, + compiledRule.hasOptionalOutput()); break; case RULE: + CelCompiledRule matchNestedRule = match.result().rule(); // If the match has a nested rule, then compute the rule and whether it has // an optional return value. - CelCompiledRule matchNestedRule = match.result().rule(); - Step nestedRule = optimizeRule(cel, matchNestedRule); - Step ruleStep = - new Step( - matchNestedRule.hasOptionalOutput(), !isTriviallyTrue, condAst, nestedRule.expr); + Step nestedRule = optimizeRule(cel, matchNestedRule, returnList); + currentStep = new Step(nestedRule.isOptional, !isTriviallyTrue, condAst, nestedRule.expr); currentSourceId = getFirstOutputSourceId(matchNestedRule); - - output = combine(astMutator, ruleStep, output); - - lastOutputType = - assertComposedAstIsValid( - cel, - output.expr, - String.format( - "failed composing the subrule '%s' due to incompatible output types.", - matchNestedRule.ruleId().map(ValueString::value).orElse("")), - currentSourceId, - lastOutputId) - .getResultType(); + validationMessage = + String.format( + "failed composing the subrule '%s' due to incompatible output types.", + matchNestedRule.ruleId().map(ValueString::value).orElse("")); break; + default: + throw new IllegalStateException("Unknown match kind"); } + output = + isAggregate + ? combineAggregate(astMutator, currentStep, output) + : combine(astMutator, currentStep, output); + lastOutputType = + assertComposedAstIsValid( + cel, output.expr, validationMessage, currentSourceId, lastOutputId) + .getResultType(); lastOutputId = currentSourceId; } @@ -151,6 +146,24 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) { : Step.newUnconditionalNonOptionalStep(newTrueLiteral(), resultExpr); } + private @Nullable Step createBaseStep(boolean returnList, boolean hasOptionalOutput) { + if (returnList) { + // If the rule is evaluated as a list (AGGREGATE), the base case is an empty list. + return Step.newUnconditionalNonOptionalStep(newTrueLiteral(), newList()); + } + + if (hasOptionalOutput) { + // If the rule has an optional output, the last result in the ternary should return + // `optional.none`. This output is implicit and created here to reflect the desired + // last possible output of this type of rule. + return Step.newUnconditionalOptionalStep( + newTrueLiteral(), astMutator.newGlobalCall(Function.OPTIONAL_NONE.getFunction())); + } + + // Exhaustive non-optional rules start with no base case. + return null; + } + static RuleComposer newInstance( CelCompiledRule compiledRule, String variablePrefix, int iterationLimit) { return new RuleComposer(compiledRule, variablePrefix, iterationLimit); @@ -250,6 +263,76 @@ private Step combineWhenCurrentIsNonOptional( } } + private Step combineAggregate(AstMutator astMutator, Step currentStep, Step accumulatedStep) { + CelMutableAst trueCondition = newTrueLiteral(); + // We assume currentStep.expr evaluates to a list due to contextual list generation. + CelMutableAst currentListPart = currentStep.expr; + // Stitch: currentStep.cond ? currentListPart : [] + // If the condition is false, we contribute an empty list to the accumulation, + // effectively dropping the result of this branch if it didn't match. + CelMutableAst conditionalListPart; + if (currentStep.isConditional) { + conditionalListPart = + astMutator.newGlobalCall( + Operator.CONDITIONAL.getFunction(), currentStep.cond, currentListPart, newList()); + + } else { + conditionalListPart = currentListPart; + } + + CelMutableAst concatenated = + astMutator.newGlobalCall( + Operator.ADD.getFunction(), conditionalListPart, accumulatedStep.expr); + + return Step.newUnconditionalNonOptionalStep(trueCondition, concatenated); + } + + /** + * Strips the structural type wrapper injected by the RuleComposer (e.g., optionals for + * FIRST_MATCH, lists for AGGREGATE) so that type mismatch errors display the raw underlying types + * authored by the user. + */ + private static @Nullable CelType unwrapComposerWrapper( + @Nullable CelType type, boolean returnList, boolean hasOptionalOutput) { + if (type == null) { + return null; + } + + if (returnList && type instanceof ListType) { + return ((ListType) type).elemType(); + } + + if (!returnList && hasOptionalOutput && type instanceof OptionalType) { + return type.parameters().get(0); + } + + return type; + } + + private static String incompatibleOutputTypesMessage( + @Nullable CelType lastOutputType, + CelType matchOutputType, + boolean returnList, + boolean hasOptionalOutput) { + CelType unwrappedLastOutputType = + unwrapComposerWrapper(lastOutputType, returnList, hasOptionalOutput); + return String.format( + "incompatible output types: block has output type %s, but previous outputs have" + + " type %s", + unwrappedLastOutputType == null ? "unknown type" : CelTypes.format(unwrappedLastOutputType), + CelTypes.format(matchOutputType)); + } + + private static CelMutableAst newList(CelMutableAst... elements) { + List exprs = new ArrayList<>(); + CelMutableSource combinedSource = CelMutableSource.newInstance(); + for (CelMutableAst element : elements) { + exprs.add(element.expr()); + combinedSource = AstMutator.combine(combinedSource, element.source()); + } + return CelMutableAst.of(CelMutableExpr.ofList(CelMutableList.create(exprs)), combinedSource); + } + private static boolean isOptionalNone(CelMutableAst ast) { CelMutableExpr expr = ast.expr(); return expr.getKind().equals(Kind.CALL) diff --git a/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java b/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java index 73950069f..b4194b5ae 100644 --- a/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java +++ b/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java @@ -34,6 +34,8 @@ import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelOptions; import dev.cel.common.formats.ValueString; +import dev.cel.common.types.ListType; +import dev.cel.common.types.MapType; import dev.cel.common.types.OptionalType; import dev.cel.common.types.SimpleType; import dev.cel.expr.conformance.proto3.TestAllTypes; @@ -137,6 +139,100 @@ public void compileYamlPolicy_nestedRuleOptionalFallbackDivergence() throws Exce assertThat(ast.getResultType()).isEqualTo(OptionalType.create(SimpleType.BOOL)); } + @Test + public void evalYamlPolicy_aggregate() throws Exception { + String policySource = + "name: \"aggregate_policy\"\n" + + "rule:\n" + + " aggregate:\n" + + " - condition: 'true'\n" + + " emit: '\"PII\"'\n" + + " - condition: 'true'\n" + + " emit: '\"CONFIDENTIAL\"'\n"; + Cel cel = newCel(); + CelPolicy policy = POLICY_PARSER.parse(policySource); + + CelAbstractSyntaxTree ast = + CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy); + + Object evalResult = cel.createProgram(ast).eval(); + assertThat(evalResult).isEqualTo(ImmutableList.of("PII", "CONFIDENTIAL")); + } + + @Test + public void evaluateYamlPolicy_aggregate_cseApplied() throws Exception { + String policySource = + "name: \"cse_policy\"\n" + + "rule:\n" + + " aggregate:\n" + + " - condition: \"size(resource.payload) > 5\"\n" + + " emit: '\"CSE1\"'\n" + + " - condition: \"size(resource.payload) > 5\"\n" + + " emit: '\"CSE2\"'\n" + + " - condition: 'true'\n" + + " emit: '\"ALWAYS\"'\n"; + Cel cel = + newCel() + .toCelBuilder() + .addVar("resource", MapType.create(SimpleType.STRING, ListType.create(SimpleType.INT))) + .build(); + CelPolicy policy = POLICY_PARSER.parse(policySource); + + CelAbstractSyntaxTree ast = + CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy); + + String unparsed = CelUnparserFactory.newUnparser().unparse(ast); + assertThat(unparsed) + .isEqualTo( + "cel.@block(" + + "[size(resource.payload) > 5], " + + "(@index0 ? [\"CSE1\"] : []) " + + "+ ((@index0 ? [\"CSE2\"] : []) + [\"ALWAYS\"]))"); + + // Evaluate under true condition (size of payload is 6 > 5) + ImmutableMap inputTrue = + ImmutableMap.of("resource", ImmutableMap.of("payload", ImmutableList.of(1, 2, 3, 4, 5, 6))); + Object evalResultTrue = cel.createProgram(ast).eval(inputTrue); + assertThat(evalResultTrue).isEqualTo(ImmutableList.of("CSE1", "CSE2", "ALWAYS")); + // Evaluate under false condition (size of payload is 3 <= 5) + ImmutableMap inputFalse = + ImmutableMap.of("resource", ImmutableMap.of("payload", ImmutableList.of(1, 2, 3))); + Object evalResultFalse = cel.createProgram(ast).eval(inputFalse); + assertThat(evalResultFalse).isEqualTo(ImmutableList.of("ALWAYS")); + } + + @Test + public void compileYamlPolicy_aggregate_macrosPreserved() throws Exception { + String policySource = + "name: aggregate_macros_preserved\n" + + "rule:\n" + + " aggregate:\n" + + " - condition: \"cond\"\n" + + " rule:\n" + + " match:\n" + + " - condition: \"true\"\n" + + " output: \"payload.filter(x, x > 10).exists(y, y % 2 == 0)\"\n" + + " - condition: \"true\"\n" + + " emit: \"payload.all(x, x > 0)\"\n"; + Cel cel = + newCel() + .toCelBuilder() + .addVar("cond", SimpleType.BOOL) + .addVar("payload", ListType.create(SimpleType.INT)) + .build(); + + CelPolicy policy = POLICY_PARSER.parse(policySource); + + CelAbstractSyntaxTree ast = + CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy); + + String unparsed = CelUnparserFactory.newUnparser().unparse(ast); + assertThat(unparsed) + .isEqualTo( + "(cond ? [payload.filter(x, x > 10, x).exists(y, y % 2 == 0)] : []) " + + "+ ([payload.all(x, x > 0)] + [])"); + } + @Test public void compileYamlPolicy_containsCompilationError_throws( @TestParameter TestErrorYamlPolicy testCase) throws Exception { @@ -398,7 +494,8 @@ public void compose_ruleWithNoOutputs_throws() throws Exception { Optional.of(ValueString.of(2L, "empty_rule")), ImmutableList.of(), ImmutableList.of(), - cel); + cel, + CelPolicy.EvaluationSemantic.FIRST_MATCH); RuleComposer composer = RuleComposer.newInstance(emptyRule, "variables.", 1000); CelAbstractSyntaxTree ast = cel.compile("true").getAst(); @@ -549,6 +646,10 @@ private enum TestErrorYamlPolicy { INCOMPATIBLE_OUTPUTS("incompatible_outputs"), SYNTAX("syntax"), UNDECLARED_REFERENCE("undeclared_reference"); + // TODO: Re-enable once cel-policy OSS dependency is updated with aggregate + // testdata. + // AGGREGATE_ERRORS("aggregate_errors"), + // AGGREGATE_LIST_ERRORS("aggregate_list_errors"); private final String name; private final String policyFilePath; diff --git a/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java b/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java index aaa30518a..dfb483981 100644 --- a/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java +++ b/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java @@ -328,6 +328,36 @@ private enum PolicyParseErrorTestCase { + " [tag:yaml.org,2002:str !txt]\n" + " | illegal: yaml-type\n" + " | ..^"), + BOTH_MATCH_AND_AGGREGATE_SET( + "name: test\n" + + "rule:\n" + + " match:\n" + + " - output: 'true'\n" + + " aggregate:\n" + + " - emit: 'true'\n", + "ERROR: :5:3: Only one of 'match' or 'aggregate' may be set in a rule\n" + + " | aggregate:\n" + + " | ..^"), + BOTH_AGGREGATE_AND_MATCH_SET( + "name: test\n" + + "rule:\n" + + " aggregate:\n" + + " - emit: 'true'\n" + + " match:\n" + + " - output: 'true'\n", + "ERROR: :5:3: Only one of 'match' or 'aggregate' may be set in a rule\n" + + " | match:\n" + + " | ..^"), + AGGREGATE_RULE_USES_OUTPUT( + "name: test\n" + "rule:\n" + " aggregate:\n" + " - output: 'true'\n", + "ERROR: :4:7: Rule aggregate requires 'emit' tag instead of 'output'\n" + + " | - output: 'true'\n" + + " | ......^"), + MATCH_RULE_USES_EMIT( + "name: test\n" + "rule:\n" + " match:\n" + " - emit: 'true'\n", + "ERROR: :4:7: Rule match requires 'output' tag instead of 'emit'\n" + + " | - emit: 'true'\n" + + " | ......^"), ILLEGAL_YAML_TYPE_ON_RULE_VALUE( "rule: illegal", "ERROR: :1:7: Got yaml node type tag:yaml.org,2002:str, wanted type(s)" diff --git a/testing/src/test/resources/policy/aggregate_errors/expected_errors.baseline b/testing/src/test/resources/policy/aggregate_errors/expected_errors.baseline new file mode 100644 index 000000000..538728881 --- /dev/null +++ b/testing/src/test/resources/policy/aggregate_errors/expected_errors.baseline @@ -0,0 +1,6 @@ +ERROR: aggregate_errors/policy.yaml:22:22: incompatible output types: block has output type int, but previous outputs have type optional_type(string) + | output: "optional.of('USER_PII')" + | .....................^ +ERROR: aggregate_errors/policy.yaml:24:22: incompatible output types: block has output type int, but previous outputs have type optional_type(string) + | output: "403" + | .....................^ \ No newline at end of file diff --git a/testing/src/test/resources/policy/aggregate_list_errors/expected_errors.baseline b/testing/src/test/resources/policy/aggregate_list_errors/expected_errors.baseline new file mode 100644 index 000000000..69afbc7e7 --- /dev/null +++ b/testing/src/test/resources/policy/aggregate_list_errors/expected_errors.baseline @@ -0,0 +1,6 @@ +ERROR: aggregate_list_errors/policy.yaml:22:22: incompatible output types: block has output type int, but previous outputs have type list(string) + | output: "['tag1', 'tag2']" + | .....................^ +ERROR: aggregate_list_errors/policy.yaml:24:22: incompatible output types: block has output type int, but previous outputs have type list(string) + | output: "403" + | .....................^ \ No newline at end of file diff --git a/testing/src/test/resources/policy/incompatible_outputs/expected_errors.baseline b/testing/src/test/resources/policy/incompatible_outputs/expected_errors.baseline index be370847f..7510dc02d 100644 --- a/testing/src/test/resources/policy/incompatible_outputs/expected_errors.baseline +++ b/testing/src/test/resources/policy/incompatible_outputs/expected_errors.baseline @@ -1,6 +1,7 @@ -ERROR: incompatible_outputs/policy.yaml:19:16: incompatible output types: block has output type optional_type(string), but previous outputs have type bool +ERROR: incompatible_outputs/policy.yaml:19:16: incompatible output types: block has output type string, but previous outputs have type bool | output: "true" | ...............^ -ERROR: incompatible_outputs/policy.yaml:21:16: incompatible output types: block has output type optional_type(string), but previous outputs have type bool +ERROR: incompatible_outputs/policy.yaml:21:16: incompatible output types: block has output type string, but previous outputs have type bool | output: "'false'" | ...............^ + diff --git a/testing/src/test/resources/policy/unreachable/expected_errors.baseline b/testing/src/test/resources/policy/unreachable/expected_errors.baseline index 768f0eeb1..875e00392 100644 --- a/testing/src/test/resources/policy/unreachable/expected_errors.baseline +++ b/testing/src/test/resources/policy/unreachable/expected_errors.baseline @@ -1,3 +1,6 @@ +ERROR: unreachable/policy.yaml:38:9: Condition is always false + | - condition: "false" + | ........^ ERROR: unreachable/policy.yaml:36:9: Match creates unreachable outputs | - output: | | ........^