Skip to content

Commit fb716e5

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Add extension versioning check during planning stage
PiperOrigin-RevId: 998697665
1 parent 32ba216 commit fb716e5

11 files changed

Lines changed: 227 additions & 55 deletions

File tree

‎bundle/src/test/java/dev/cel/bundle/CelImplTest.java‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,17 +2119,11 @@ public void eval_withJsonFieldName_runtimeOptionDisabled_throws() throws Excepti
21192119
CelAbstractSyntaxTree ast = celCompiler.compile("file.int64CamelCaseJsonName").getAst();
21202120

21212121
CelEvaluationException e =
2122-
assertThrows(
2123-
CelEvaluationException.class,
2124-
() ->
2125-
celRuntime
2126-
.createProgram(ast)
2127-
.eval(ImmutableMap.of("file", SingleFile.getDefaultInstance())));
2122+
assertThrows(CelEvaluationException.class, () -> celRuntime.createProgram(ast));
2123+
21282124
assertThat(e)
21292125
.hasMessageThat()
2130-
.contains(
2131-
"field 'int64CamelCaseJsonName' is not declared in message"
2132-
+ " 'dev.cel.testing.testdata.SingleFile");
2126+
.contains("json_name extension requires CelOptions.enableJsonFieldNames(true)");
21332127
}
21342128

21352129
@Test

‎common/src/main/java/dev/cel/common/CelSource.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,19 +367,19 @@ public CelSource build() {
367367
public abstract static class Extension {
368368

369369
/** Identifier for the extension. Example: constant_folding */
370-
abstract String id();
370+
public abstract String id();
371371

372372
/**
373373
* Version info. May be skipped if it isn't meaningful for the extension. (for example
374374
* constant_folding might always be v0.0).
375375
*/
376-
abstract Version version();
376+
public abstract Version version();
377377

378378
/**
379379
* If set, the listed components must understand the extension for the expression to evaluate
380380
* correctly.
381381
*/
382-
abstract ImmutableList<Component> affectedComponents();
382+
public abstract ImmutableList<Component> affectedComponents();
383383

384384
/** Version of the extension */
385385
@AutoValue
@@ -390,18 +390,20 @@ public abstract static class Version {
390390
* Major version changes indicate different required support level from the required
391391
* components.
392392
*/
393-
abstract long major();
393+
public abstract long major();
394394

395395
/**
396396
* Minor version changes must not change the observed behavior from existing implementations,
397397
* but may be provided informational.
398398
*/
399-
abstract long minor();
399+
public abstract long minor();
400400

401401
/** Create a new instance of Version with the provided major and minor values. */
402402
public static Version of(long major, long minor) {
403403
return new AutoValue_CelSource_Extension_Version(major, minor);
404404
}
405+
406+
Version() {}
405407
}
406408

407409
/** CEL component specifier. */
@@ -427,5 +429,7 @@ public static Extension create(String id, Version version, Iterable<Component> c
427429
public static Extension create(String id, Version version, Component... components) {
428430
return create(id, version, Arrays.asList(components));
429431
}
432+
433+
Extension() {}
430434
}
431435
}

‎conformance/src/test/java/dev/cel/conformance/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ java_library(
1717
deps = [
1818
"//:java_truth",
1919
"//checker:checker_builder",
20+
"//common:cel_ast",
21+
"//common:cel_source",
2022
"//common:compiler_common",
2123
"//common:container",
2224
"//common:options",

‎conformance/src/test/java/dev/cel/conformance/ConformanceTest.java‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,13 @@
3131
import com.google.protobuf.TypeRegistry;
3232
import dev.cel.checker.CelChecker;
3333
import dev.cel.checker.CelCheckerBuilder;
34+
import dev.cel.common.CelAbstractSyntaxTree;
3435
import dev.cel.common.CelContainer;
3536
import dev.cel.common.CelIssue;
3637
import dev.cel.common.CelOptions;
38+
import dev.cel.common.CelSource.Extension;
39+
import dev.cel.common.CelSource.Extension.Component;
40+
import dev.cel.common.CelSource.Extension.Version;
3741
import dev.cel.common.CelValidationResult;
3842
import dev.cel.common.CelVarDecl;
3943
import dev.cel.common.ast.CelBlock;
@@ -227,7 +231,19 @@ public void evaluate() throws Throwable {
227231
response = getChecker(test).check(response.getAst());
228232
}
229233
assertThat(response.getErrors()).isEmpty();
230-
Type resultType = CelProtoTypes.celTypeToType(response.getAst().getResultType());
234+
CelAbstractSyntaxTree ast = response.getAst();
235+
if (ast.getExpr().callOrDefault().function().equals(CelBlock.FUNCTION_NAME)) {
236+
ast =
237+
CelAbstractSyntaxTree.newCheckedAst(
238+
ast.getExpr(),
239+
ast.getSource().toBuilder()
240+
.addAllExtensions(
241+
Extension.create("cel_block", Version.of(1, 1), Component.COMPONENT_RUNTIME))
242+
.build(),
243+
ast.getReferenceMap(),
244+
ast.getTypeMap());
245+
}
246+
Type resultType = CelProtoTypes.celTypeToType(ast.getResultType());
231247

232248
if (test.getCheckOnly()) {
233249
assertThat(test.hasTypedResult()).isTrue();
@@ -244,8 +260,8 @@ public void evaluate() throws Throwable {
244260
ExprValue result = null;
245261
CelEvaluationException error = null;
246262
try {
247-
Program program = runtime.createProgram(response.getAst());
248-
result = toExprValue(program.eval(getBindings(test)), response.getAst().getResultType());
263+
Program program = runtime.createProgram(ast);
264+
result = toExprValue(program.eval(getBindings(test)), ast.getResultType());
249265
} catch (CelEvaluationException e) {
250266
error = e;
251267
}

‎optimizer/src/test/java/dev/cel/optimizer/optimizers/SubexpressionOptimizerTest.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -827,10 +827,17 @@ private CelAbstractSyntaxTree compileUsingInternalFunctions(String expression, b
827827
indexExpr.ident().setName(internalIdentName);
828828
});
829829

830+
CelAbstractSyntaxTree parsedAst =
831+
CelAbstractSyntaxTree.newParsedAst(
832+
mutableAst.toParsedAst().getExpr(),
833+
astToModify.getSource().toBuilder()
834+
.addAllExtensions(
835+
Extension.create("cel_block", Version.of(1L, 1L), Component.COMPONENT_RUNTIME))
836+
.build());
830837
if (parsedOnly) {
831-
return mutableAst.toParsedAst();
838+
return parsedAst;
832839
}
833-
return celForEvaluatingBlock.check(mutableAst.toParsedAst()).getAst();
840+
return celForEvaluatingBlock.check(parsedAst).getAst();
834841
}
835842

836843
private CelAbstractSyntaxTree compileUsingInternalFunctions(String expression)

‎runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ java_library(
4646
":planner_helpers",
4747
"//:auto_value",
4848
"//common:cel_ast",
49+
"//common:cel_source",
4950
"//common:container",
5051
"//common:operator",
5152
"//common:options",
@@ -555,16 +556,13 @@ java_library(
555556
":planned_interpretable",
556557
":planner_helpers",
557558
"//common/ast",
558-
"//common/types",
559559
"//common/types:cel_types",
560-
"//common/types:type_providers",
561560
"//common/values",
562561
"//common/values:cel_byte_string",
563562
"//common/values:optimized_select_traversal",
564563
"//common/values:select_field",
565564
"@maven//:com_google_errorprone_error_prone_annotations",
566565
"@maven//:com_google_guava_guava",
567-
"@maven//:org_jspecify_jspecify",
568566
],
569567
)
570568

@@ -720,6 +718,7 @@ cel_android_library(
720718
":planner_helpers_android",
721719
"//:auto_value",
722720
"//common:cel_ast_android",
721+
"//common:cel_source_android",
723722
"//common:container_android",
724723
"//common:operator_android",
725724
"//common:options",
@@ -1232,14 +1231,11 @@ cel_android_library(
12321231
":planner_helpers_android",
12331232
"//common/ast:ast_android",
12341233
"//common/types:cel_types_android",
1235-
"//common/types:type_providers_android",
1236-
"//common/types:types_android",
12371234
"//common/values:cel_byte_string",
12381235
"//common/values:optimized_select_traversal_android",
12391236
"//common/values:select_field_android",
12401237
"//common/values:values_android",
12411238
"@maven//:com_google_errorprone_error_prone_annotations",
1242-
"@maven//:org_jspecify_jspecify",
12431239
"@maven_android//:com_google_guava_guava",
12441240
],
12451241
)

‎runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package dev.cel.runtime.planner;
1616

17+
import static com.google.common.base.Preconditions.checkArgument;
1718
import static com.google.common.base.Preconditions.checkNotNull;
1819

1920
import com.google.auto.value.AutoValue;
@@ -28,6 +29,9 @@
2829
import dev.cel.common.CelAbstractSyntaxTree;
2930
import dev.cel.common.CelContainer;
3031
import dev.cel.common.CelOptions;
32+
import dev.cel.common.CelSource.Extension;
33+
import dev.cel.common.CelSource.Extension.Component;
34+
import dev.cel.common.CelSource.Extension.Version;
3135
import dev.cel.common.Operator;
3236
import dev.cel.common.annotations.Internal;
3337
import dev.cel.common.ast.CelBlock;
@@ -60,6 +64,7 @@
6064
import dev.cel.runtime.RuntimeEquality;
6165
import java.util.Arrays;
6266
import java.util.HashMap;
67+
import java.util.Map;
6368
import java.util.NoSuchElementException;
6469
import java.util.Optional;
6570
import org.jspecify.annotations.Nullable;
@@ -72,6 +77,15 @@
7277
@Internal
7378
public final class ProgramPlanner {
7479
private static final String MAP_INSERT_FUNCTION = "cel.@mapInsert";
80+
private static final String CEL_BLOCK_EXTENSION = "cel_block";
81+
private static final String JSON_NAME_EXTENSION = "json_name";
82+
private static final String SELECT_OPTIMIZATION_EXTENSION = "select_optimization";
83+
84+
private static final ImmutableMap<String, Version> SUPPORTED_RUNTIME_EXTENSIONS =
85+
ImmutableMap.of(
86+
CEL_BLOCK_EXTENSION, Version.of(1, 1),
87+
JSON_NAME_EXTENSION, Version.of(1, 1),
88+
SELECT_OPTIMIZATION_EXTENSION, Version.of(1, 0));
7589

7690
private final CelTypeProvider typeProvider;
7791
private final CelValueProvider valueProvider;
@@ -102,7 +116,7 @@ public PlannedProgram plan(CelAbstractSyntaxTree ast) throws CelEvaluationExcept
102116
ErrorMetadata errorMetadata =
103117
ErrorMetadata.create(ast.getSource().getPositionsMap(), ast.getSource().getDescription());
104118
try {
105-
PlannerContext ctx = PlannerContext.create(ast);
119+
PlannerContext ctx = PlannerContext.create(ast, options);
106120
plannedInterpretable =
107121
CelBlock.extract(ast)
108122
.map(celBlock -> planBlock(celBlock, ctx))
@@ -248,6 +262,11 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) {
248262
// map key lookup on Map, and SelectableValue fallback).
249263
if (functionName.equals(OptimizedSelectPlanner.CEL_ATTRIBUTE_FUNCTION_NAME)
250264
|| functionName.equals(OptimizedSelectPlanner.CEL_HAS_FIELD_FUNCTION_NAME)) {
265+
checkArgument(
266+
ctx.runtimeExtensions.contains(SELECT_OPTIMIZATION_EXTENSION),
267+
"%s requires the %s AST extension",
268+
functionName,
269+
SELECT_OPTIMIZATION_EXTENSION);
251270
return optimizedSelectPlanner.plan(expr, functionName, operandExpr -> plan(operandExpr, ctx));
252271
}
253272

@@ -382,6 +401,11 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) {
382401
}
383402

384403
private PlannedInterpretable planBlock(CelBlock celBlock, PlannerContext ctx) {
404+
checkArgument(
405+
ctx.runtimeExtensions.contains(CEL_BLOCK_EXTENSION),
406+
"%s requires the %s AST extension",
407+
CelBlock.FUNCTION_NAME,
408+
CEL_BLOCK_EXTENSION);
385409
ImmutableList<CelExpr> indices = celBlock.indices();
386410

387411
PlannedInterpretable[] slotExprs = new PlannedInterpretable[indices.size()];
@@ -716,23 +740,20 @@ private static Builder newBuilder() {
716740
}
717741
}
718742

719-
static final class PlannerContext {
743+
private static final class PlannerContext {
720744
private final CelAbstractSyntaxTree ast;
721-
private final HashMap<String, Integer> localVars = new HashMap<>();
722-
723-
CelAbstractSyntaxTree ast() {
724-
return ast;
725-
}
745+
private final ImmutableSet<String> runtimeExtensions;
746+
private final Map<String, Integer> localVars;
726747

727-
ImmutableMap<Long, CelReference> referenceMap() {
748+
private ImmutableMap<Long, CelReference> referenceMap() {
728749
return ast.getReferenceMap();
729750
}
730751

731-
ImmutableMap<Long, CelType> typeMap() {
752+
private ImmutableMap<Long, CelType> typeMap() {
732753
return ast.getTypeMap();
733754
}
734755

735-
boolean isChecked() {
756+
private boolean isChecked() {
736757
return ast.isChecked();
737758
}
738759

@@ -766,12 +787,35 @@ private boolean isLocalVar(String name) {
766787
return localVars.containsKey(name);
767788
}
768789

769-
static PlannerContext create(CelAbstractSyntaxTree ast) {
770-
return new PlannerContext(ast);
790+
private static PlannerContext create(CelAbstractSyntaxTree ast, CelOptions options) {
791+
ImmutableSet.Builder<String> runtimeExtensions = ImmutableSet.builder();
792+
for (Extension extension : ast.getSource().getExtensions()) {
793+
if (!extension.affectedComponents().contains(Component.COMPONENT_RUNTIME)) {
794+
continue;
795+
}
796+
Version version = extension.version();
797+
Version supportedVersion = SUPPORTED_RUNTIME_EXTENSIONS.get(extension.id());
798+
checkArgument(
799+
supportedVersion != null
800+
&& version.major() == supportedVersion.major()
801+
&& version.minor() <= supportedVersion.minor(),
802+
"unsupported CEL extension: %s@%s.%s",
803+
extension.id(),
804+
version.major(),
805+
version.minor());
806+
runtimeExtensions.add(extension.id());
807+
}
808+
ImmutableSet<String> extensions = runtimeExtensions.build();
809+
checkArgument(
810+
!extensions.contains(JSON_NAME_EXTENSION) || options.enableJsonFieldNames(),
811+
"json_name extension requires CelOptions.enableJsonFieldNames(true)");
812+
return new PlannerContext(ast, extensions);
771813
}
772814

773-
private PlannerContext(CelAbstractSyntaxTree ast) {
815+
private PlannerContext(CelAbstractSyntaxTree ast, ImmutableSet<String> runtimeExtensions) {
774816
this.ast = checkNotNull(ast);
817+
this.runtimeExtensions = runtimeExtensions;
818+
this.localVars = new HashMap<>();
775819
}
776820
}
777821

‎runtime/src/test/java/dev/cel/runtime/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ java_library(
4040
"//common:container",
4141
"//common:error_codes",
4242
"//common:options",
43+
"//common:proto_ast",
4344
"//common:proto_v1alpha1_ast",
4445
"//common/ast",
4546
"//common/ast:cel_block",
@@ -111,6 +112,7 @@ java_library(
111112
"//testing/protos:single_file_java_proto",
112113
"//testing/protos:test_all_types_cel_java_proto2",
113114
"//testing/protos:test_all_types_cel_java_proto3",
115+
"@cel_spec//proto/cel/expr:checked_java_proto",
114116
"@cel_spec//proto/cel/expr/conformance/proto2:test_all_types_java_proto",
115117
"@cel_spec//proto/cel/expr/conformance/proto3:test_all_types_java_proto",
116118
"@com_google_googleapis//google/api/expr/v1alpha1:expr_java_proto",

‎runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import static java.nio.charset.StandardCharsets.UTF_8;
1919
import static org.junit.Assert.assertThrows;
2020

21-
import com.google.api.expr.v1alpha1.CheckedExpr;
21+
import dev.cel.expr.CheckedExpr;
2222
import com.google.common.collect.ImmutableList;
2323
import com.google.common.collect.ImmutableMap;
2424
import com.google.common.collect.ImmutableSet;
@@ -40,7 +40,7 @@
4040
import dev.cel.common.CelFunctionDecl;
4141
import dev.cel.common.CelOptions;
4242
import dev.cel.common.CelOverloadDecl;
43-
import dev.cel.common.CelProtoV1Alpha1AbstractSyntaxTree;
43+
import dev.cel.common.CelProtoAbstractSyntaxTree;
4444
import dev.cel.common.ast.CelBlock;
4545
import dev.cel.common.internal.ProtoTimeUtils;
4646
import dev.cel.common.types.ListType;
@@ -1879,11 +1879,11 @@ public void checkedExprWireRoundTrip_preservesOptimizedSelectsUnderVersionSkew()
18791879
.getAst();
18801880
CelAbstractSyntaxTree optimizedAst = serverOptimizer.optimize(ast);
18811881
byte[] serializedCheckedExpr =
1882-
CelProtoV1Alpha1AbstractSyntaxTree.fromCelAst(optimizedAst).toCheckedExpr().toByteArray();
1882+
CelProtoAbstractSyntaxTree.fromCelAst(optimizedAst).toCheckedExpr().toByteArray();
18831883
CheckedExpr deserializedCheckedExpr =
18841884
CheckedExpr.parseFrom(serializedCheckedExpr, ExtensionRegistryLite.getEmptyRegistry());
18851885
CelAbstractSyntaxTree deserializedAst =
1886-
CelProtoV1Alpha1AbstractSyntaxTree.fromCheckedExpr(deserializedCheckedExpr).getAst();
1886+
CelProtoAbstractSyntaxTree.fromCheckedExpr(deserializedCheckedExpr).getAst();
18871887

18881888
Object result =
18891889
clientRuntime

0 commit comments

Comments
 (0)