Skip to content

Commit

Permalink
Fix conditionally nested rules to include its condition
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 657714654
  • Loading branch information
l46kok authored and copybara-github committed Jul 30, 2024
1 parent 77d7853 commit f78c6c4
Show file tree
Hide file tree
Showing 9 changed files with 202 additions and 9 deletions.
4 changes: 3 additions & 1 deletion policy/src/main/java/dev/cel/policy/CelCompiledRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ public enum Kind {
*/
@AutoValue
public abstract static class OutputValue {
public abstract long id();

/** Source metadata identifier associated with the output. */
public abstract long sourceId();

public abstract CelAbstractSyntaxTree ast();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static dev.cel.policy.YamlHelper.newInteger;
import static dev.cel.policy.YamlHelper.newString;
import static dev.cel.policy.YamlHelper.parseYamlSource;
import static dev.cel.policy.YamlHelper.validateYamlType;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -258,7 +259,18 @@ private static ExtensionConfig parseExtension(ParserContext<Node> ctx, Node node
builder.setName(newString(ctx, valueNode));
break;
case "version":
builder.setVersion(newInteger(ctx, valueNode));
if (validateYamlType(valueNode, YamlNodeType.INTEGER)) {
builder.setVersion(newInteger(ctx, valueNode));
break;
} else if (validateYamlType(valueNode, YamlNodeType.STRING, YamlNodeType.TEXT)) {
String versionStr = newString(ctx, valueNode);
if (versionStr.equals("latest")) {
builder.setVersion(Integer.MAX_VALUE);
break;
}
// Fall-through
}
ctx.reportError(keyId, String.format("Unsupported version tag: %s", keyName));
break;
default:
ctx.reportError(keyId, String.format("Unsupported extension tag: %s", keyName));
Expand Down
21 changes: 17 additions & 4 deletions policy/src/main/java/dev/cel/policy/RuleComposer.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private RuleOptimizationResult optimizeRule(Cel cel, CelCompiledRule compiledRul
if (isTriviallyTrue) {
matchAst = outAst;
isOptionalResult = false;
lastOutputId = matchOutput.id();
lastOutputId = matchOutput.sourceId();
continue;
}
if (isOptionalResult) {
Expand All @@ -99,8 +99,12 @@ private RuleOptimizationResult optimizeRule(Cel cel, CelCompiledRule compiledRul
outAst,
matchAst);
assertComposedAstIsValid(
cel, matchAst, "conflicting output types found.", matchOutput.id(), lastOutputId);
lastOutputId = matchOutput.id();
cel,
matchAst,
"conflicting output types found.",
matchOutput.sourceId(),
lastOutputId);
lastOutputId = matchOutput.sourceId();
continue;
case RULE:
CelCompiledRule matchNestedRule = match.result().rule();
Expand All @@ -117,7 +121,16 @@ private RuleOptimizationResult optimizeRule(Cel cel, CelCompiledRule compiledRul
if (!isOptionalResult && !nestedRule.isOptionalResult()) {
throw new IllegalArgumentException("Subrule early terminates policy");
}
matchAst = astMutator.newMemberCall(nestedRuleAst, Function.OR.getFunction(), matchAst);
if (isTriviallyTrue) {
matchAst = astMutator.newMemberCall(nestedRuleAst, Function.OR.getFunction(), matchAst);
} else {
matchAst =
astMutator.newGlobalCall(
Operator.CONDITIONAL.getFunction(),
CelMutableAst.fromCelAst(conditionAst),
nestedRuleAst,
matchAst);
}
assertComposedAstIsValid(
cel,
matchAst,
Expand Down
13 changes: 11 additions & 2 deletions policy/src/main/java/dev/cel/policy/YamlHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,23 @@ static boolean assertRequiredFields(
return false;
}

static boolean assertYamlType(
ParserContext<Node> ctx, long id, Node node, YamlNodeType... expectedNodeTypes) {
static boolean validateYamlType(Node node, YamlNodeType... expectedNodeTypes) {
String nodeTag = node.getTag().getValue();
for (YamlNodeType expectedNodeType : expectedNodeTypes) {
if (expectedNodeType.tag().equals(nodeTag)) {
return true;
}
}
return false;
}

static boolean assertYamlType(
ParserContext<Node> ctx, long id, Node node, YamlNodeType... expectedNodeTypes) {
if (validateYamlType(node, expectedNodeTypes)) {
return true;
}
String nodeTag = node.getTag().getValue();

ctx.reportError(
id,
String.format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,42 @@ public void config_setExtensions() throws Exception {
assertThat(policyConfig.extend(CEL_WITH_MESSAGE_TYPES, CelOptions.DEFAULT)).isNotNull();
}

@Test
public void config_setExtensionVersionToLatest() throws Exception {
String yamlConfig =
"extensions:\n" //
+ " - name: 'bindings'\n" //
+ " version: latest";

CelPolicyConfig policyConfig = POLICY_CONFIG_PARSER.parse(yamlConfig);

assertThat(policyConfig)
.isEqualTo(
CelPolicyConfig.newBuilder()
.setConfigSource(policyConfig.configSource())
.setExtensions(ImmutableSet.of(ExtensionConfig.of("bindings", Integer.MAX_VALUE)))
.build());
assertThat(policyConfig.extend(CEL_WITH_MESSAGE_TYPES, CelOptions.DEFAULT)).isNotNull();
}

@Test
public void config_setExtensionVersionToInvalidValue() throws Exception {
String yamlConfig =
"extensions:\n" //
+ " - name: 'bindings'\n" //
+ " version: invalid";

CelPolicyValidationException e =
assertThrows(
CelPolicyValidationException.class, () -> POLICY_CONFIG_PARSER.parse(yamlConfig));
assertThat(e)
.hasMessageThat()
.contains(
"ERROR: <input>:3:5: Unsupported version tag: version\n"
+ " | version: invalid\n"
+ " | ....^");
}

@Test
public void config_setFunctions() throws Exception {
String yamlConfig =
Expand Down
13 changes: 12 additions & 1 deletion policy/src/test/java/dev/cel/policy/PolicyTestHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,18 @@ enum TestYamlPolicy {
"pb",
true,
"(spec.single_int32 > 10) ? optional.of(\"invalid spec, got single_int32=\" +"
+ " string(spec.single_int32) + \", wanted <= 10\") : optional.none()");
+ " string(spec.single_int32) + \", wanted <= 10\") : optional.none()"),
LIMITS(
"limits",
true,
"cel.bind(variables.greeting, \"hello\", cel.bind(variables.farewell, \"goodbye\","
+ " cel.bind(variables.person, \"me\", cel.bind(variables.message_fmt, \"%s, %s\","
+ " (now.getHours() >= 20) ? cel.bind(variables.message, variables.farewell + \", \" +"
+ " variables.person, (now.getHours() < 21) ? optional.of(variables.message + \"!\") :"
+ " ((now.getHours() < 22) ? optional.of(variables.message + \"!!\") : ((now.getHours()"
+ " < 24) ? optional.of(variables.message + \"!!!\") : optional.none()))) :"
+ " optional.of(variables.greeting + \", \" + variables.person)))))");

private final String name;
private final boolean producesOptionalResult;
private final String unparsed;
Expand Down
22 changes: 22 additions & 0 deletions policy/src/test/resources/limits/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: "limits"
extensions:
- name: "strings"
version: latest
variables:
- name: "now"
type:
type_name: "google.protobuf.Timestamp"
50 changes: 50 additions & 0 deletions policy/src/test/resources/limits/policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: "limits"
rule:
variables:
- name: "greeting"
expression: "'hello'"
- name: "farewell"
expression: "'goodbye'"
- name: "person"
expression: "'me'"
- name: "message_fmt"
expression: "'%s, %s'"
match:
- condition: |
now.getHours() >= 20
rule:
id: "farewells"
variables:
- name: "message"
expression: >
variables.farewell + ', ' + variables.person
# TODO: replace when string.format is available
# variables.message_fmt.format([variables.farewell,
# variables.person])
match:
- condition: >
now.getHours() < 21
output: variables.message + "!"
- condition: >
now.getHours() < 22
output: variables.message + "!!"
- condition: >
now.getHours() < 24
output: variables.message + "!!!"
- output: >
variables.greeting + ', ' + variables.person
# variables.message_fmt.format([variables.greeting, variables.person]) TODO: replace when string.format is available
38 changes: 38 additions & 0 deletions policy/src/test/resources/limits/tests.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

description: Limits related tests
section:
- name: "now_after_hours"
tests:
- name: "7pm"
input:
now:
expr: "timestamp('2024-07-30T00:30:00Z')"
output: "'hello, me'"
- name: "8pm"
input:
now:
expr: "timestamp('2024-07-30T20:30:00Z')"
output: "'goodbye, me!'"
- name: "9pm"
input:
now:
expr: "timestamp('2024-07-30T21:30:00Z')"
output: "'goodbye, me!!'"
- name: "11pm"
input:
now:
expr: "timestamp('2024-07-30T23:30:00Z')"
output: "'goodbye, me!!!'"

0 comments on commit f78c6c4

Please sign in to comment.