diff --git a/policy/src/main/java/dev/cel/policy/CelPolicyParserBuilder.java b/policy/src/main/java/dev/cel/policy/CelPolicyParserBuilder.java index ba34a1a60..266264780 100644 --- a/policy/src/main/java/dev/cel/policy/CelPolicyParserBuilder.java +++ b/policy/src/main/java/dev/cel/policy/CelPolicyParserBuilder.java @@ -29,6 +29,30 @@ public interface CelPolicyParserBuilder { @CanIgnoreReturnValue CelPolicyParserBuilder addTagVisitor(TagVisitor tagVisitor); + /** + * Configures the parser to allow for key-value pairs to declare a variable name and expression. + * + *

For example: + * + *

{@code
+   * variables:
+   * - foo: bar
+   * - baz: qux
+   * }
+ * + *

This is in contrast to the default behavior, which requires the following syntax: + * + *

{@code
+   * variables:
+   * - name: foo
+   *   expression: bar
+   * - name: baz
+   *   expression: qux
+   * }
+ */ + @CanIgnoreReturnValue + CelPolicyParserBuilder enableSimpleVariables(boolean enable); + /** Builds a new instance of {@link CelPolicyParser}. */ @CheckReturnValue CelPolicyParser build(); diff --git a/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java b/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java index b7255c93b..43595c4ab 100644 --- a/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java +++ b/policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java @@ -49,6 +49,7 @@ final class CelPolicyYamlParser implements CelPolicyParser { Variable.newBuilder().setExpression(ERROR_VALUE).setName(ERROR_VALUE).build(); private final TagVisitor tagVisitor; + private final boolean enableSimpleVariables; @Override public CelPolicy parse(String policySource) throws CelPolicyValidationException { @@ -58,13 +59,15 @@ public CelPolicy parse(String policySource) throws CelPolicyValidationException @Override public CelPolicy parse(String policySource, String description) throws CelPolicyValidationException { - ParserImpl parser = new ParserImpl(tagVisitor, policySource, description); + ParserImpl parser = + new ParserImpl(tagVisitor, enableSimpleVariables, policySource, description); return parser.parseYaml(); } private static class ParserImpl implements PolicyParserContext { private final TagVisitor tagVisitor; + private final boolean enableSimpleVariables; private final CelPolicySource policySource; private final ParserContext ctx; @@ -336,9 +339,45 @@ public CelPolicy.Variable parseVariable( if (!assertYamlType(ctx, id, node, YamlNodeType.MAP)) { return ERROR_VARIABLE; } + MappingNode variableMap = (MappingNode) node; Variable.Builder builder = Variable.newBuilder(); + if (enableSimpleVariables) { + return parseVariableInline(ctx, id, variableMap, builder); + } + return parseVariableObject(ctx, policyBuilder, id, variableMap, builder); + } + + private Variable parseVariableInline( + PolicyParserContext ctx, long id, MappingNode variableMap, Variable.Builder builder) { + int iterations = 0; + for (NodeTuple nodeTuple : variableMap.getValue()) { + Node keyNode = nodeTuple.getKeyNode(); + long keyId = ctx.collectMetadata(keyNode); + builder + .setName(ctx.newValueString(keyNode)) + .setExpression(ctx.newValueString(nodeTuple.getValueNode())); + iterations++; + + if (iterations > 1) { + ctx.reportError(keyId, "Only one variable may be defined inline"); + } + } + + if (!assertRequiredFields(ctx, id, builder.getMissingRequiredFieldNames())) { + return ERROR_VARIABLE; + } + + return builder.build(); + } + + private Variable parseVariableObject( + PolicyParserContext ctx, + CelPolicy.Builder policyBuilder, + long id, + MappingNode variableMap, + Variable.Builder builder) { for (NodeTuple nodeTuple : variableMap.getValue()) { Node keyNode = nodeTuple.getKeyNode(); long keyId = ctx.collectMetadata(keyNode); @@ -370,8 +409,13 @@ public CelPolicy.Variable parseVariable( return builder.build(); } - private ParserImpl(TagVisitor tagVisitor, String source, String description) { + private ParserImpl( + TagVisitor tagVisitor, + boolean enableSimpleVariables, + String source, + String description) { this.tagVisitor = tagVisitor; + this.enableSimpleVariables = enableSimpleVariables; this.policySource = CelPolicySource.newBuilder(CelCodePointArray.fromString(source)) .setDescription(description) @@ -413,9 +457,11 @@ public ValueString newValueString(Node node) { static final class Builder implements CelPolicyParserBuilder { private TagVisitor tagVisitor; + private boolean enableSimpleVariables; private Builder() { this.tagVisitor = new TagVisitor() {}; + this.enableSimpleVariables = false; } @Override @@ -424,17 +470,24 @@ public CelPolicyParserBuilder addTagVisitor(TagVisitor tagVisitor) { return this; } + @Override + public CelPolicyParserBuilder enableSimpleVariables(boolean enable) { + this.enableSimpleVariables = enable; + return this; + } + @Override public CelPolicyParser build() { - return new CelPolicyYamlParser(tagVisitor); + return new CelPolicyYamlParser(tagVisitor, enableSimpleVariables); } } - static Builder newBuilder() { - return new Builder(); + static CelPolicyParserBuilder newBuilder() { + return new Builder().enableSimpleVariables(false); } - private CelPolicyYamlParser(TagVisitor tagVisitor) { + private CelPolicyYamlParser(TagVisitor tagVisitor, boolean enableSimpleVariables) { this.tagVisitor = checkNotNull(tagVisitor); + this.enableSimpleVariables = enableSimpleVariables; } } diff --git a/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java b/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java index fa0da8a9a..336a392ff 100644 --- a/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java +++ b/policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java @@ -302,6 +302,29 @@ public void evaluateYamlPolicy_lateBoundFunction() throws Exception { assertThat(evalResult).isEqualTo("foo" + exampleValue); } + @Test + public void evaluateYamlPolicy_withSimpleVariable() throws Exception { + Cel cel = newCel(); + String policySource = + "name: shorthand_variables_policy\n" + + "rule:\n" + + " variables:\n" + + " - first: 'true'\n" + + " - second: 'false'\n" + + " match:\n" + + " - output: 'variables.first && variables.second'"; + CelPolicyParser parser = + CelPolicyParserFactory.newYamlParserBuilder().enableSimpleVariables(true).build(); + CelPolicy policy = parser.parse(policySource); + + CelAbstractSyntaxTree compiledPolicyAst = + CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy); + + boolean evalResult = (boolean) cel.createProgram(compiledPolicyAst).eval(); + + assertThat(evalResult).isFalse(); + } + private static final class EvaluablePolicyTestData { private final TestYamlPolicy yamlPolicy; private final PolicyTestCase testCase; diff --git a/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java b/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java index 803d99202..f8327c255 100644 --- a/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java +++ b/policy/src/test/java/dev/cel/policy/CelPolicyYamlParserTest.java @@ -148,6 +148,30 @@ public void parseYamlPolicy_withImports() throws Exception { .inOrder(); } + @Test + public void parseYamlPolicy_withSimpleVariable_multipleInlinedVariables() { + String policySource = + "name: shorthand_variables_policy\n" + + "rule:\n" + + " variables:\n" + + " - first: 'true'\n" + + " second: 'false'\n" + + " match:\n" + + " - condition: 'variables.my_var'\n" + + " output: 'true'\n"; + CelPolicyParser parser = + CelPolicyParserFactory.newYamlParserBuilder().enableSimpleVariables(true).build(); + + CelPolicyValidationException e = + assertThrows(CelPolicyValidationException.class, () -> parser.parse(policySource)); + assertThat(e) + .hasMessageThat() + .contains( + "ERROR: :5:7: Only one variable may be defined inline\n" + + " | second: 'false'\n" + + " | ......^"); + } + @Test public void parseYamlPolicy_errors(@TestParameter PolicyParseErrorTestCase testCase) { CelPolicyValidationException e =