Skip to content

Commit 803aa2e

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Improved support for nested rules
PiperOrigin-RevId: 662601763
1 parent 83fe5a3 commit 803aa2e

File tree

10 files changed

+288
-11
lines changed

10 files changed

+288
-11
lines changed

‎policy/src/main/java/dev/cel/policy/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ java_library(
227227
"//bundle:cel",
228228
"//common",
229229
"//common:compiler_common",
230+
"//common/ast",
230231
"@maven//:com_google_errorprone_error_prone_annotations",
231232
"@maven//:com_google_guava_guava",
232233
],
@@ -304,7 +305,6 @@ java_library(
304305
"//common",
305306
"//common:compiler_common",
306307
"//common:mutable_ast",
307-
"//common/ast",
308308
"//extensions:optional_library",
309309
"//optimizer:ast_optimizer",
310310
"//optimizer:mutable_ast",

‎policy/src/main/java/dev/cel/policy/CelCompiledRule.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import dev.cel.bundle.Cel;
2121
import dev.cel.common.CelAbstractSyntaxTree;
2222
import dev.cel.common.CelVarDecl;
23+
import dev.cel.common.ast.CelConstant;
24+
import dev.cel.common.ast.CelExpr;
2325
import java.util.Optional;
2426

2527
/**
@@ -36,6 +38,28 @@ public abstract class CelCompiledRule {
3638

3739
public abstract Cel cel();
3840

41+
/**
42+
* HasOptionalOutput returns whether the rule returns a concrete or optional value. The rule may
43+
* return an optional value if all match expressions under the rule are conditional.
44+
*/
45+
public boolean hasOptionalOutput() {
46+
boolean isOptionalOutput = false;
47+
for (CelCompiledMatch match : matches()) {
48+
if (match.result().kind().equals(CelCompiledMatch.Result.Kind.RULE)
49+
&& match.result().rule().hasOptionalOutput()) {
50+
return true;
51+
}
52+
53+
if (match.isConditionLiteral()) {
54+
return false;
55+
}
56+
57+
isOptionalOutput = true;
58+
}
59+
60+
return isOptionalOutput;
61+
}
62+
3963
/**
4064
* A compiled policy variable (ex: variables.foo). Note that this is not the same thing as the
4165
* variables declared in the config.
@@ -63,6 +87,12 @@ public abstract static class CelCompiledMatch {
6387

6488
public abstract Result result();
6589

90+
public boolean isConditionLiteral() {
91+
CelExpr celExpr = condition().getExpr();
92+
return celExpr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)
93+
&& celExpr.constant().booleanValue();
94+
}
95+
6696
/** Encapsulates the result of this match when condition is met. (either an output or a rule) */
6797
@AutoOneOf(CelCompiledMatch.Result.Kind.class)
6898
public abstract static class Result {

‎policy/src/main/java/dev/cel/policy/RuleComposer.java

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import dev.cel.common.CelAbstractSyntaxTree;
2525
import dev.cel.common.CelMutableAst;
2626
import dev.cel.common.CelValidationException;
27-
import dev.cel.common.ast.CelConstant.Kind;
2827
import dev.cel.extensions.CelOptionalLibrary.Function;
2928
import dev.cel.optimizer.AstMutator;
3029
import dev.cel.optimizer.CelAstOptimizer;
@@ -75,10 +74,14 @@ private RuleOptimizationResult optimizeRule(Cel cel, CelCompiledRule compiledRul
7574
long lastOutputId = 0;
7675
for (CelCompiledMatch match : Lists.reverse(compiledRule.matches())) {
7776
CelAbstractSyntaxTree conditionAst = match.condition();
78-
boolean isTriviallyTrue =
79-
conditionAst.getExpr().constantOrDefault().getKind().equals(Kind.BOOLEAN_VALUE)
80-
&& conditionAst.getExpr().constant().booleanValue();
77+
// If the condition is trivially true, none of the matches in the rule causes the result
78+
// to become optional, and the rule is not the last match, then this will introduce
79+
// unreachable outputs or rules.
80+
boolean isTriviallyTrue = match.isConditionLiteral();
81+
8182
switch (match.result().kind()) {
83+
// For the match's output, determine whether the output should be wrapped
84+
// into an optional value, a conditional, or both.
8285
case OUTPUT:
8386
OutputValue matchOutput = match.result().output();
8487
CelMutableAst outAst = CelMutableAst.fromCelAst(matchOutput.ast());
@@ -107,21 +110,25 @@ private RuleOptimizationResult optimizeRule(Cel cel, CelCompiledRule compiledRul
107110
lastOutputId = matchOutput.sourceId();
108111
continue;
109112
case RULE:
113+
// If the match has a nested rule, then compute the rule and whether it has
114+
// an optional return value.
110115
CelCompiledRule matchNestedRule = match.result().rule();
111116
RuleOptimizationResult nestedRule = optimizeRule(cel, matchNestedRule);
117+
boolean nestedHasOptional = matchNestedRule.hasOptionalOutput();
112118
CelMutableAst nestedRuleAst = nestedRule.ast();
113-
if (isOptionalResult && !nestedRule.isOptionalResult()) {
119+
if (isOptionalResult && !nestedHasOptional) {
114120
nestedRuleAst =
115121
astMutator.newGlobalCall(Function.OPTIONAL_OF.getFunction(), nestedRuleAst);
116122
}
117-
if (!isOptionalResult && nestedRule.isOptionalResult()) {
123+
if (!isOptionalResult && nestedHasOptional) {
118124
matchAst = astMutator.newGlobalCall(Function.OPTIONAL_OF.getFunction(), matchAst);
119125
isOptionalResult = true;
120126
}
121-
if (!isOptionalResult && !nestedRule.isOptionalResult()) {
122-
throw new IllegalArgumentException("Subrule early terminates policy");
123-
}
124-
if (isTriviallyTrue) {
127+
// If either the nested rule or current condition output are optional then
128+
// use optional.or() to specify the combination of the first and second results
129+
// Note, the argument order is reversed due to the traversal of matches in
130+
// reverse order.
131+
if (isOptionalResult && isTriviallyTrue) {
125132
matchAst = astMutator.newMemberCall(nestedRuleAst, Function.OR.getFunction(), matchAst);
126133
} else {
127134
matchAst =
@@ -131,6 +138,7 @@ private RuleOptimizationResult optimizeRule(Cel cel, CelCompiledRule compiledRul
131138
nestedRuleAst,
132139
matchAst);
133140
}
141+
134142
assertComposedAstIsValid(
135143
cel,
136144
matchAst,

‎policy/src/test/java/dev/cel/policy/PolicyTestHelper.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,26 @@ enum TestYamlPolicy {
4848
+ "? optional.of({\"banned\": true}) : optional.none()).or("
4949
+ "optional.of((resource.origin in variables.permitted_regions)"
5050
+ " ? {\"banned\": false} : {\"banned\": true})))"),
51+
NESTED_RULE2(
52+
"nested_rule2",
53+
false,
54+
"cel.bind(variables.permitted_regions, [\"us\", \"uk\", \"es\"],"
55+
+ " resource.?user.orValue(\"\").startsWith(\"bad\") ?"
56+
+ " cel.bind(variables.banned_regions, {\"us\": false, \"ru\": false, \"ir\": false},"
57+
+ " (resource.origin in variables.banned_regions && !(resource.origin in"
58+
+ " variables.permitted_regions)) ? {\"banned\": \"restricted_region\"} : {\"banned\":"
59+
+ " \"bad_actor\"}) : (!(resource.origin in variables.permitted_regions) ? {\"banned\":"
60+
+ " \"unconfigured_region\"} : {}))"),
61+
NESTED_RULE3(
62+
"nested_rule3",
63+
true,
64+
"cel.bind(variables.permitted_regions, [\"us\", \"uk\", \"es\"],"
65+
+ " resource.?user.orValue(\"\").startsWith(\"bad\") ?"
66+
+ " optional.of(cel.bind(variables.banned_regions, {\"us\": false, \"ru\": false,"
67+
+ " \"ir\": false}, (resource.origin in variables.banned_regions && !(resource.origin"
68+
+ " in variables.permitted_regions)) ? {\"banned\": \"restricted_region\"} :"
69+
+ " {\"banned\": \"bad_actor\"})) : (!(resource.origin in variables.permitted_regions)"
70+
+ " ? optional.of({\"banned\": \"unconfigured_region\"}) : optional.none()))"),
5171
REQUIRED_LABELS(
5272
"required_labels",
5373
true,

‎policy/src/test/resources/nested_rule2/config.yaml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Copyright 2024 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
name: "nested_rule2"
16+
variables:
17+
- name: "resource"
18+
type:
19+
type_name: "map"
20+
params:
21+
- type_name: "string"
22+
- type_name: "dyn"

‎policy/src/test/resources/nested_rule2/policy.yaml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Copyright 2024 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
name: nested_rule2
16+
rule:
17+
variables:
18+
- name: "permitted_regions"
19+
expression: "['us', 'uk', 'es']"
20+
match:
21+
- condition: resource.?user.orValue("").startsWith("bad")
22+
rule:
23+
id: "banned regions"
24+
description: >
25+
determine whether the resource origin is in the banned
26+
list. If the region is also in the permitted list, the
27+
ban has no effect.
28+
variables:
29+
- name: "banned_regions"
30+
expression: "{'us': false, 'ru': false, 'ir': false}"
31+
match:
32+
- condition: |
33+
resource.origin in variables.banned_regions &&
34+
!(resource.origin in variables.permitted_regions)
35+
output: "{'banned': 'restricted_region'}"
36+
explanation: "'resource is in the banned region ' + resource.origin"
37+
- output: "{'banned': 'bad_actor'}"
38+
- condition: "!(resource.origin in variables.permitted_regions)"
39+
output: "{'banned': 'unconfigured_region'}"
40+
- output: "{}"

‎policy/src/test/resources/nested_rule2/tests.yaml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Copyright 2024 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
description: Nested rule conformance tests
16+
section:
17+
- name: "banned"
18+
tests:
19+
- name: "restricted_origin"
20+
input:
21+
resource:
22+
value:
23+
user: "bad-user"
24+
origin: "ir"
25+
output: "{'banned': 'restricted_region'}"
26+
- name: "by_default"
27+
input:
28+
resource:
29+
value:
30+
user: "bad-user"
31+
origin: "de"
32+
output: "{'banned': 'bad_actor'}"
33+
- name: "unconfigured_region"
34+
input:
35+
resource:
36+
value:
37+
user: "good-user"
38+
origin: "de"
39+
output: "{'banned': 'unconfigured_region'}"
40+
- name: "permitted"
41+
tests:
42+
- name: "valid_origin"
43+
input:
44+
resource:
45+
value:
46+
user: "good-user"
47+
origin: "uk"
48+
output: "{}"

‎policy/src/test/resources/nested_rule3/config.yaml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Copyright 2024 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
name: "nested_rule3"
16+
variables:
17+
- name: "resource"
18+
type:
19+
type_name: "map"
20+
params:
21+
- type_name: "string"
22+
- type_name: "dyn"

‎policy/src/test/resources/nested_rule3/policy.yaml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Copyright 2024 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
name: nested_rule3
16+
rule:
17+
variables:
18+
- name: "permitted_regions"
19+
expression: "['us', 'uk', 'es']"
20+
match:
21+
- condition: resource.?user.orValue("").startsWith("bad")
22+
rule:
23+
id: "banned regions"
24+
description: >
25+
determine whether the resource origin is in the banned
26+
list. If the region is also in the permitted list, the
27+
ban has no effect.
28+
variables:
29+
- name: "banned_regions"
30+
expression: "{'us': false, 'ru': false, 'ir': false}"
31+
match:
32+
- condition: |
33+
resource.origin in variables.banned_regions &&
34+
!(resource.origin in variables.permitted_regions)
35+
output: "{'banned': 'restricted_region'}"
36+
explanation: "'resource is in the banned region ' + resource.origin"
37+
- output: "{'banned': 'bad_actor'}"
38+
- condition: "!(resource.origin in variables.permitted_regions)"
39+
output: "{'banned': 'unconfigured_region'}"

‎policy/src/test/resources/nested_rule3/tests.yaml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Copyright 2024 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
description: Nested rule conformance tests
16+
section:
17+
- name: "banned"
18+
tests:
19+
- name: "restricted_origin"
20+
input:
21+
resource:
22+
value:
23+
user: "bad-user"
24+
origin: "ir"
25+
output: "{'banned': 'restricted_region'}"
26+
- name: "by_default"
27+
input:
28+
resource:
29+
value:
30+
user: "bad-user"
31+
origin: "de"
32+
output: "{'banned': 'bad_actor'}"
33+
- name: "unconfigured_region"
34+
input:
35+
resource:
36+
value:
37+
user: "good-user"
38+
origin: "de"
39+
output: "{'banned': 'unconfigured_region'}"
40+
- name: "permitted"
41+
tests:
42+
- name: "valid_origin"
43+
input:
44+
resource:
45+
value:
46+
user: "good-user"
47+
origin: "uk"
48+
output: "optional.none()"

0 commit comments

Comments
 (0)