Policy nested rule fix

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

@@ -247,19 +247,19 @@ java_library(
     visibility = ["//visibility:private"],
     deps = [
         ":compiled_rule",
-        "//:auto_value",
         "//bundle:cel",
         "//common:cel_ast",
         "//common:compiler_common",
         "//common:mutable_ast",
+        "//common:mutable_source",
         "//common:operator",
         "//common/ast",
+        "//common/ast:mutable_expr",
         "//common/formats:value_string",
         "//common/navigation:mutable_navigation",
         "//extensions:optional_library",
         "//optimizer:ast_optimizer",
         "//optimizer:mutable_ast",
-        "@maven//:com_google_errorprone_error_prone_annotations",
         "@maven//:com_google_guava_guava",
     ],
 )

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

@@ -214,7 +214,30 @@ public void evaluateYamlPolicy_withCanonicalTestData(
     // Read the policy source
     String policySource = testData.yamlPolicy.readPolicyYamlContent();
     CelPolicy policy = POLICY_PARSER.parse(policySource);
-    CelAbstractSyntaxTree expectedOutputAst = cel.compile(testData.testCase.getOutput()).getAst();
+    Object outputObj = testData.testCase.getOutput();
+    String exprToCompile;
+    if (outputObj instanceof String) {
+      exprToCompile = (String) outputObj;
+    } else if (outputObj instanceof Map) {
+      @SuppressWarnings("unchecked") // Test only
+      Map<String, Object> outputMap = (Map<String, Object>) outputObj;
+      if (outputMap.containsKey("value")) {
+        Object value = outputMap.get("value");
+        if (value instanceof String) {
+          String escapedValue = ((String) value).replace("\"", "\\\"");
+          exprToCompile = "\"" + escapedValue + "\""; // Quote string literals
+        } else {
+          exprToCompile = String.valueOf(value);
+        }
+      } else if (outputMap.containsKey("expr")) {
+        exprToCompile = (String) outputMap.get("expr");
+      } else {
+        throw new IllegalArgumentException("Invalid output format: " + outputObj);
+      }
+    } else {
+      throw new IllegalArgumentException("Invalid output format: " + outputObj);
+    }
+    CelAbstractSyntaxTree expectedOutputAst = cel.compile(exprToCompile).getAst();
     Object expectedOutput = cel.createProgram(expectedOutputAst).eval();
 
     // Act
@@ -266,8 +289,8 @@ public void evaluateYamlPolicy_nestedRuleProducesOptionalOutput() throws Excepti
         CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
     Optional<Object> evalResult = (Optional<Object>) cel.createProgram(compiledPolicyAst).eval();
 
-    // Result is Optional<Optional<Object>>
-    assertThat(evalResult).hasValue(Optional.of(true));
+    // Result is Optional<Object> containing true
+    assertThat(evalResult).hasValue(true);
   }
 
   @Test

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

@@ -40,11 +40,11 @@ final class PolicyTestHelper {
   enum TestYamlPolicy {
     NESTED_RULE(
         "nested_rule",
-        true,
+        false,
         "cel.@block([resource.origin, @index0 in [\"us\", \"uk\", \"es\"], {\"banned\": true}],"
             + " ((@index0 in {\"us\": false, \"ru\": false, \"ir\": false} && !@index1) ?"
-            + " optional.of(@index2) : optional.none()).or(optional.of(@index1 ? {\"banned\":"
-            + " false} : @index2)))"),
+            + " optional.of(@index2) : optional.none()).orValue(@index1 ? {\"banned\":"
+            + " false} : @index2))"),
     NESTED_RULE2(
         "nested_rule2",
         false,
@@ -61,6 +61,22 @@ enum TestYamlPolicy {
             + " false, \"ru\": false, \"ir\": false} && @index1) ? {\"banned\":"
             + " \"restricted_region\"} : {\"banned\": \"bad_actor\"}) : (@index1 ?"
             + " optional.of({\"banned\": \"unconfigured_region\"}) : optional.none()))"),
+    NESTED_RULE4("nested_rule4", false, "(x > 0) ? true : false"),
+    NESTED_RULE5(
+        "nested_rule5",
+        true,
+        "cel.@block([optional.of(true), optional.none()], (x > 0) ? ((x > 2) ? @index0 : @index1) :"
+            + " ((x > 1) ? ((x >= 2) ? @index0 : @index1) : optional.of(false)))"),
+    NESTED_RULE6(
+        "nested_rule6",
+        false,
+        "cel.@block([optional.of(true), optional.none()], ((x > 2) ? @index0 : @index1).orValue(((x"
+            + " > 3) ? @index0 : @index1).orValue(false)))"),
+    NESTED_RULE7(
+        "nested_rule7",
+        true,
+        "cel.@block([optional.of(true), optional.none()], ((x > 2) ? @index0 : @index1).or(((x > 3)"
+            + " ? @index0 : @index1).or((x > 1) ? optional.of(false) : @index1)))"),
     REQUIRED_LABELS(
         "required_labels",
         true,
@@ -198,7 +214,7 @@ public List<PolicyTestCase> getTests() {
       public static final class PolicyTestCase {
         private String name;
         private Map<String, PolicyTestInput> input;
-        private String output;
+        private Object output;
 
         public void setName(String name) {
           this.name = name;
@@ -208,7 +224,7 @@ public void setInput(Map<String, PolicyTestInput> input) {
           this.input = input;
         }
 
-        public void setOutput(String output) {
+        public void setOutput(Object output) {
           this.output = output;
         }
 
@@ -220,7 +236,7 @@ public Map<String, PolicyTestInput> getInput() {
           return input;
         }
 
-        public String getOutput() {
+        public Object getOutput() {
           return output;
         }
 

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

@@ -14,18 +14,19 @@
 
 description: K8s admission control tests
 section:
-- name: "invalid"
-  tests:
-  - name: "restricted_container"
-    input:
-      resource.namespace:
-        value: "dev.cel"
-      resource.labels:
-        value:
-          environment: "staging"
-      resource.containers:
-        value:
-        - staging.dev.cel.container1
-        - staging.dev.cel.container2
-        - preprod.dev.cel.container3
-    output: "'only staging containers are allowed in namespace dev.cel'"
+  - name: "invalid"
+    tests:
+      - name: "restricted_container"
+        input:
+          resource.namespace:
+            value: "dev.cel"
+          resource.labels:
+            value:
+              environment: "staging"
+          resource.containers:
+            value:
+              - staging.dev.cel.container1
+              - staging.dev.cel.container2
+              - preprod.dev.cel.container3
+        output:
+          value: "only staging containers are allowed in namespace dev.cel"

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

@@ -14,25 +14,29 @@
 
 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!!!'"
+  - name: "now_after_hours"
+    tests:
+      - name: "7pm"
+        input:
+          now:
+            expr: "timestamp('2024-07-30T00:30:00Z')"
+        output:
+          value: "hello, me"
+      - name: "8pm"
+        input:
+          now:
+            expr: "timestamp('2024-07-30T20:30:00Z')"
+        output:
+          value: "goodbye, me!"
+      - name: "9pm"
+        input:
+          now:
+            expr: "timestamp('2024-07-30T21:30:00Z')"
+        output:
+          value: "goodbye, me!!"
+      - name: "11pm"
+        input:
+          now:
+            expr: "timestamp('2024-07-30T23:30:00Z')"
+        output:
+          value: "goodbye, me!!!"

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

@@ -16,23 +16,26 @@ description: Nested rule conformance tests
 section:
   - name: "banned"
     tests:
-    - name: "restricted_origin"
-      input:
-        resource:
-          value:
-            origin: "ir"
-      output: "{'banned': true}"
-    - name: "by_default"
-      input:
-        resource:
-          value:
-            origin: "de"
-      output: "{'banned': true}"
+      - name: "restricted_origin"
+        input:
+          resource:
+            value:
+              origin: "ir"
+        output:
+          expr: "{'banned': true}"
+      - name: "by_default"
+        input:
+          resource:
+            value:
+              origin: "de"
+        output:
+          expr: "{'banned': true}"
   - name: "permitted"
     tests:
-    - name: "valid_origin"
-      input:
-        resource:
-          value:
-            origin: "uk"
-      output: "{'banned': false}"
+      - name: "valid_origin"
+        input:
+          resource:
+            value:
+              origin: "uk"
+        output:
+          expr: "{'banned': false}"

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

@@ -14,35 +14,39 @@
 
 description: Nested rule conformance tests
 section:
-- name: "banned"
-  tests:
-  - name: "restricted_origin"
-    input:
-      resource:
-        value:
-          user: "bad-user"
-          origin: "ir"
-    output: "{'banned': 'restricted_region'}"
-  - name: "by_default"
-    input:
-      resource:
-        value:
-          user: "bad-user"
-          origin: "de"
-    output: "{'banned': 'bad_actor'}"
-  - name: "unconfigured_region"
-    input:
-      resource:
-        value:
-          user: "good-user"
-          origin: "de"
-    output: "{'banned': 'unconfigured_region'}"
-- name: "permitted"
-  tests:
-  - name: "valid_origin"
-    input:
-      resource:
-        value:
-          user: "good-user"
-          origin: "uk"
-    output: "{}"
+  - name: "banned"
+    tests:
+      - name: "restricted_origin"
+        input:
+          resource:
+            value:
+              user: "bad-user"
+              origin: "ir"
+        output:
+          expr: "{'banned': 'restricted_region'}"
+      - name: "by_default"
+        input:
+          resource:
+            value:
+              user: "bad-user"
+              origin: "de"
+        output:
+          expr: "{'banned': 'bad_actor'}"
+      - name: "unconfigured_region"
+        input:
+          resource:
+            value:
+              user: "good-user"
+              origin: "de"
+        output:
+          expr: "{'banned': 'unconfigured_region'}"
+  - name: "permitted"
+    tests:
+      - name: "valid_origin"
+        input:
+          resource:
+            value:
+              user: "good-user"
+              origin: "uk"
+        output:
+          expr: "{}"