Skip to content

Commit 21c3318

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Policy nested rule fix
PiperOrigin-RevId: 907239115
1 parent f566450 commit 21c3318

24 files changed

Lines changed: 893 additions & 381 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,19 +247,19 @@ java_library(
247247
visibility = ["//visibility:private"],
248248
deps = [
249249
":compiled_rule",
250-
"//:auto_value",
251250
"//bundle:cel",
252251
"//common:cel_ast",
253252
"//common:compiler_common",
254253
"//common:mutable_ast",
254+
"//common:mutable_source",
255255
"//common:operator",
256256
"//common/ast",
257+
"//common/ast:mutable_expr",
257258
"//common/formats:value_string",
258259
"//common/navigation:mutable_navigation",
259260
"//extensions:optional_library",
260261
"//optimizer:ast_optimizer",
261262
"//optimizer:mutable_ast",
262-
"@maven//:com_google_errorprone_error_prone_annotations",
263263
"@maven//:com_google_guava_guava",
264264
],
265265
)

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

Lines changed: 205 additions & 79 deletions
Large diffs are not rendered by default.

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,30 @@ public void evaluateYamlPolicy_withCanonicalTestData(
214214
// Read the policy source
215215
String policySource = testData.yamlPolicy.readPolicyYamlContent();
216216
CelPolicy policy = POLICY_PARSER.parse(policySource);
217-
CelAbstractSyntaxTree expectedOutputAst = cel.compile(testData.testCase.getOutput()).getAst();
217+
Object outputObj = testData.testCase.getOutput();
218+
String exprToCompile;
219+
if (outputObj instanceof String) {
220+
exprToCompile = (String) outputObj;
221+
} else if (outputObj instanceof Map) {
222+
@SuppressWarnings("unchecked") // Test only
223+
Map<String, Object> outputMap = (Map<String, Object>) outputObj;
224+
if (outputMap.containsKey("value")) {
225+
Object value = outputMap.get("value");
226+
if (value instanceof String) {
227+
String escapedValue = ((String) value).replace("\"", "\\\"");
228+
exprToCompile = "\"" + escapedValue + "\""; // Quote string literals
229+
} else {
230+
exprToCompile = String.valueOf(value);
231+
}
232+
} else if (outputMap.containsKey("expr")) {
233+
exprToCompile = (String) outputMap.get("expr");
234+
} else {
235+
throw new IllegalArgumentException("Invalid output format: " + outputObj);
236+
}
237+
} else {
238+
throw new IllegalArgumentException("Invalid output format: " + outputObj);
239+
}
240+
CelAbstractSyntaxTree expectedOutputAst = cel.compile(exprToCompile).getAst();
218241
Object expectedOutput = cel.createProgram(expectedOutputAst).eval();
219242

220243
// Act
@@ -266,8 +289,8 @@ public void evaluateYamlPolicy_nestedRuleProducesOptionalOutput() throws Excepti
266289
CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
267290
Optional<Object> evalResult = (Optional<Object>) cel.createProgram(compiledPolicyAst).eval();
268291

269-
// Result is Optional<Optional<Object>>
270-
assertThat(evalResult).hasValue(Optional.of(true));
292+
// Result is Optional<Object> containing true
293+
assertThat(evalResult).hasValue(true);
271294
}
272295

273296
@Test

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ final class PolicyTestHelper {
4040
enum TestYamlPolicy {
4141
NESTED_RULE(
4242
"nested_rule",
43-
true,
43+
false,
4444
"cel.@block([resource.origin, @index0 in [\"us\", \"uk\", \"es\"], {\"banned\": true}],"
4545
+ " ((@index0 in {\"us\": false, \"ru\": false, \"ir\": false} && !@index1) ?"
46-
+ " optional.of(@index2) : optional.none()).or(optional.of(@index1 ? {\"banned\":"
47-
+ " false} : @index2)))"),
46+
+ " optional.of(@index2) : optional.none()).orValue(@index1 ? {\"banned\":"
47+
+ " false} : @index2))"),
4848
NESTED_RULE2(
4949
"nested_rule2",
5050
false,
@@ -61,6 +61,22 @@ enum TestYamlPolicy {
6161
+ " false, \"ru\": false, \"ir\": false} && @index1) ? {\"banned\":"
6262
+ " \"restricted_region\"} : {\"banned\": \"bad_actor\"}) : (@index1 ?"
6363
+ " optional.of({\"banned\": \"unconfigured_region\"}) : optional.none()))"),
64+
NESTED_RULE4("nested_rule4", false, "(x > 0) ? true : false"),
65+
NESTED_RULE5(
66+
"nested_rule5",
67+
true,
68+
"cel.@block([optional.of(true), optional.none()], (x > 0) ? ((x > 2) ? @index0 : @index1) :"
69+
+ " ((x > 1) ? ((x >= 2) ? @index0 : @index1) : optional.of(false)))"),
70+
NESTED_RULE6(
71+
"nested_rule6",
72+
false,
73+
"cel.@block([optional.of(true), optional.none()], ((x > 2) ? @index0 : @index1).orValue(((x"
74+
+ " > 3) ? @index0 : @index1).orValue(false)))"),
75+
NESTED_RULE7(
76+
"nested_rule7",
77+
true,
78+
"cel.@block([optional.of(true), optional.none()], ((x > 2) ? @index0 : @index1).or(((x > 3)"
79+
+ " ? @index0 : @index1).or((x > 1) ? optional.of(false) : @index1)))"),
6480
REQUIRED_LABELS(
6581
"required_labels",
6682
true,
@@ -198,7 +214,7 @@ public List<PolicyTestCase> getTests() {
198214
public static final class PolicyTestCase {
199215
private String name;
200216
private Map<String, PolicyTestInput> input;
201-
private String output;
217+
private Object output;
202218

203219
public void setName(String name) {
204220
this.name = name;
@@ -208,7 +224,7 @@ public void setInput(Map<String, PolicyTestInput> input) {
208224
this.input = input;
209225
}
210226

211-
public void setOutput(String output) {
227+
public void setOutput(Object output) {
212228
this.output = output;
213229
}
214230

@@ -220,7 +236,7 @@ public Map<String, PolicyTestInput> getInput() {
220236
return input;
221237
}
222238

223-
public String getOutput() {
239+
public Object getOutput() {
224240
return output;
225241
}
226242

‎testing/src/test/resources/policy/k8s/tests.yaml‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,19 @@
1414

1515
description: K8s admission control tests
1616
section:
17-
- name: "invalid"
18-
tests:
19-
- name: "restricted_container"
20-
input:
21-
resource.namespace:
22-
value: "dev.cel"
23-
resource.labels:
24-
value:
25-
environment: "staging"
26-
resource.containers:
27-
value:
28-
- staging.dev.cel.container1
29-
- staging.dev.cel.container2
30-
- preprod.dev.cel.container3
31-
output: "'only staging containers are allowed in namespace dev.cel'"
17+
- name: "invalid"
18+
tests:
19+
- name: "restricted_container"
20+
input:
21+
resource.namespace:
22+
value: "dev.cel"
23+
resource.labels:
24+
value:
25+
environment: "staging"
26+
resource.containers:
27+
value:
28+
- staging.dev.cel.container1
29+
- staging.dev.cel.container2
30+
- preprod.dev.cel.container3
31+
output:
32+
value: "only staging containers are allowed in namespace dev.cel"

‎testing/src/test/resources/policy/limits/tests.yaml‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,29 @@
1414

1515
description: Limits related tests
1616
section:
17-
- name: "now_after_hours"
18-
tests:
19-
- name: "7pm"
20-
input:
21-
now:
22-
expr: "timestamp('2024-07-30T00:30:00Z')"
23-
output: "'hello, me'"
24-
- name: "8pm"
25-
input:
26-
now:
27-
expr: "timestamp('2024-07-30T20:30:00Z')"
28-
output: "'goodbye, me!'"
29-
- name: "9pm"
30-
input:
31-
now:
32-
expr: "timestamp('2024-07-30T21:30:00Z')"
33-
output: "'goodbye, me!!'"
34-
- name: "11pm"
35-
input:
36-
now:
37-
expr: "timestamp('2024-07-30T23:30:00Z')"
38-
output: "'goodbye, me!!!'"
17+
- name: "now_after_hours"
18+
tests:
19+
- name: "7pm"
20+
input:
21+
now:
22+
expr: "timestamp('2024-07-30T00:30:00Z')"
23+
output:
24+
value: "hello, me"
25+
- name: "8pm"
26+
input:
27+
now:
28+
expr: "timestamp('2024-07-30T20:30:00Z')"
29+
output:
30+
value: "goodbye, me!"
31+
- name: "9pm"
32+
input:
33+
now:
34+
expr: "timestamp('2024-07-30T21:30:00Z')"
35+
output:
36+
value: "goodbye, me!!"
37+
- name: "11pm"
38+
input:
39+
now:
40+
expr: "timestamp('2024-07-30T23:30:00Z')"
41+
output:
42+
value: "goodbye, me!!!"

‎testing/src/test/resources/policy/nested_rule/tests.yaml‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,26 @@ description: Nested rule conformance tests
1616
section:
1717
- name: "banned"
1818
tests:
19-
- name: "restricted_origin"
20-
input:
21-
resource:
22-
value:
23-
origin: "ir"
24-
output: "{'banned': true}"
25-
- name: "by_default"
26-
input:
27-
resource:
28-
value:
29-
origin: "de"
30-
output: "{'banned': true}"
19+
- name: "restricted_origin"
20+
input:
21+
resource:
22+
value:
23+
origin: "ir"
24+
output:
25+
expr: "{'banned': true}"
26+
- name: "by_default"
27+
input:
28+
resource:
29+
value:
30+
origin: "de"
31+
output:
32+
expr: "{'banned': true}"
3133
- name: "permitted"
3234
tests:
33-
- name: "valid_origin"
34-
input:
35-
resource:
36-
value:
37-
origin: "uk"
38-
output: "{'banned': false}"
35+
- name: "valid_origin"
36+
input:
37+
resource:
38+
value:
39+
origin: "uk"
40+
output:
41+
expr: "{'banned': false}"

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

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,35 +14,39 @@
1414

1515
description: Nested rule conformance tests
1616
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: "{}"
17+
- name: "banned"
18+
tests:
19+
- name: "restricted_origin"
20+
input:
21+
resource:
22+
value:
23+
user: "bad-user"
24+
origin: "ir"
25+
output:
26+
expr: "{'banned': 'restricted_region'}"
27+
- name: "by_default"
28+
input:
29+
resource:
30+
value:
31+
user: "bad-user"
32+
origin: "de"
33+
output:
34+
expr: "{'banned': 'bad_actor'}"
35+
- name: "unconfigured_region"
36+
input:
37+
resource:
38+
value:
39+
user: "good-user"
40+
origin: "de"
41+
output:
42+
expr: "{'banned': 'unconfigured_region'}"
43+
- name: "permitted"
44+
tests:
45+
- name: "valid_origin"
46+
input:
47+
resource:
48+
value:
49+
user: "good-user"
50+
origin: "uk"
51+
output:
52+
expr: "{}"

0 commit comments

Comments
 (0)