Validate required fields for policy variables

PiperOrigin-RevId: 649215393

Author: l46kok

policy/src/main/java/dev/cel/policy/CelPolicy.java

@@ -217,20 +217,28 @@ public abstract static class Variable {
 
     /** Builder for {@link Variable}. */
     @AutoValue.Builder
-    public abstract static class Builder {
+    public abstract static class Builder implements RequiredFieldsChecker {
+
+      abstract Optional<ValueString> name();
+
+      abstract Optional<ValueString> expression();
 
       public abstract Builder setName(ValueString name);
 
       public abstract Builder setExpression(ValueString expression);
 
+      @Override
+      public ImmutableList<RequiredField> requiredFields() {
+        return ImmutableList.of(
+            RequiredField.of("name", this::name), RequiredField.of("expression", this::expression));
+      }
+
       public abstract Variable build();
     }
 
     /** Creates a new builder to construct a {@link Variable} instance. */
     public static Builder newBuilder() {
-      return new AutoValue_CelPolicy_Variable.Builder()
-          .setName(ValueString.newBuilder().build())
-          .setExpression(ValueString.newBuilder().build());
+      return new AutoValue_CelPolicy_Variable.Builder();
     }
   }
 }

policy/src/main/java/dev/cel/policy/CelPolicyYamlParser.java

@@ -38,13 +38,11 @@
 final class CelPolicyYamlParser implements CelPolicyParser {
 
   // Sentinel values for parsing errors
+  private static final ValueString ERROR_VALUE = ValueString.newBuilder().setValue(ERROR).build();
   private static final Match ERROR_MATCH =
-      Match.newBuilder()
-          .setCondition(ValueString.newBuilder().setValue(ERROR).build())
-          .setResult(Result.ofOutput(ValueString.newBuilder().setValue(ERROR).build()))
-          .build();
+      Match.newBuilder().setCondition(ERROR_VALUE).setResult(Result.ofOutput(ERROR_VALUE)).build();
   private static final Variable ERROR_VARIABLE =
-      Variable.newBuilder().setName(ValueString.newBuilder().setValue(ERROR).build()).build();
+      Variable.newBuilder().setExpression(ERROR_VALUE).setName(ERROR_VALUE).build();
 
   private final TagVisitor<Node> tagVisitor;
 
@@ -272,6 +270,10 @@ public CelPolicy.Variable parseVariable(
         }
       }
 
+      if (!assertRequiredFields(ctx, id, builder.getMissingRequiredFieldNames())) {
+        return ERROR_VARIABLE;
+      }
+
       return builder.build();
     }
 

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

@@ -134,10 +134,28 @@ private enum PolicyParseErrorTestCase {
         "inputs:\n" + "  - name: a\n" + "  - name: b",
         "ERROR: <input>:1:1: Unsupported policy tag: inputs\n" + " | inputs:\n" + " | ^"),
     UNSUPPORTED_VARIABLE_TAG(
-        "rule:\n" + "  variables:\n" + "    - name: 'true'\n" + "      alt_name: 'bool_true'",
-        "ERROR: <input>:4:7: Unsupported variable tag: alt_name\n"
+        "rule:\n" //
+            + "  variables:\n" //
+            + "    - name: 'hello'\n" //
+            + "      expression: 'true'\n" //
+            + "      alt_name: 'bool_true'",
+        "ERROR: <input>:5:7: Unsupported variable tag: alt_name\n"
             + " |       alt_name: 'bool_true'\n"
             + " | ......^"),
+    MISSING_VARIABLE_NAME(
+        "rule:\n" //
+            + "  variables:\n" //
+            + "    - expression: 'true'", //
+        "ERROR: <input>:3:7: Missing required attribute(s): name\n"
+            + " |     - expression: 'true'\n"
+            + " | ......^"),
+    MISSING_VARIABLE_EXPRESSION(
+        "rule:\n" //
+            + "  variables:\n" //
+            + "    - name: 'hello'", //
+        "ERROR: <input>:3:7: Missing required attribute(s): expression\n"
+            + " |     - name: 'hello'\n"
+            + " | ......^"),
     UNSUPPORTED_MATCH_TAG(
         "rule:\n"
             + "  match:\n"