Skip to content
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ IO_GRPC_GRPC_JAVA_ARTIFACTS = [
"com.google.re2j:re2j:1.8",
"com.google.s2a.proto.v2:s2a-proto:0.1.3",
"com.google.truth:truth:1.4.5",
"dev.cel:runtime:0.11.1",
"dev.cel:compiler:0.11.1",
"dev.cel:protobuf:0.11.1",
"org.antlr:antlr4-runtime:4.9.1",
"com.squareup.okhttp:okhttp:2.7.5",
"com.squareup.okio:okio:2.10.0", # 3.0+ needs swapping to -jvm; need work to avoid flag-day
"io.netty:netty-buffer:4.1.130.Final",
Expand Down
3 changes: 3 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ checkstyle = "com.puppycrawl.tools:checkstyle:10.26.1"
# checkstyle 10.0+ requires Java 11+
# See https://checkstyle.sourceforge.io/releasenotes_old_8-35_10-26.html#Release_10.0
# checkForUpdates: checkstylejava8:9.+
cel-runtime = "dev.cel:runtime:0.11.1"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you plan on using both the compilation and runtime packages, you could consider just taking a dependency on dev.cel:cel instead, which includes both. It will likely make your dependency management much easier.

On a separate note -- let me know if you'd like us to cut a new release.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We purposefully don't want a dependency on the cel parser. Also, it seems dev.cel:cel is busted, in that it re-defines dev.cel.runtime instead of depending on the other artifact. That will cause hard-to-debug duplicate classes in the class path.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For artifact issue -- it's tracked, we can look into prioritizing it: (b/328302799).

The PR here is referencing CelCompiler, so I think you'll need the compiler dependency (unless if it shouldn't have been included here).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is pretty serious, as it prevents dependency management from working properly and it is not something regular people are able to debug. It's the sort of thing where everything looks fine, but then you make an unrelated change and somehow suddenly breaks CEL. So the concern is in support costs.

Really, I'm wary about merging this with the broken cel jars; it's best not to have them in our dependency tree. Even with the code disabled they can still harm users. Maybe it is best to make them compileOnly until things are fixed, which also means parts of testing and feature stabilization would be delayed until it is fixed.

The PR is only using CelCompiler in tests. It will need to get reorganized to avoid the dependency for production.

@shivaspeaks, you may want to glance at go/grpc-cel-integration. It was supposed to have been superseded by the gRFC, but I'm seeing the gRFC doesn't make a point of calling out some of the goals (like not depending on the CEL compiler). It's longer than you probably want to read in full, but you may just skim it to find the "interesting" parts.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into this next week. I was actually under the impression this wasn't a hard blocker. Thanks for surfacing it up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an observation @ejona86! I was trying what the doc said:
"
Production code: no need to depend on the full dev.cel:cel package.

Testing code: it's acceptable to use unstable CEL methods and depend on dev.cel:cel / dev.cel:compiler in testing code.
"

When ChannelAndServerBuilderTest runs, it uses Guava's ClassPath to scan all io.grpc.* classes. When the JVM Verifier loads CelMatcher or PredicateEvaluator, it traverses the method signatures and fields, eventually hitting CodePointStream. Because CodePointStream implements CharStream, the JVM tries to load CharStream. Since dev.cel:runtime didn't bring ANTLR transitively, and I removed dev.cel:compiler (which does bring ANTLR) from implementation, the JVM throws NoClassDefFoundError. I am being forced to add compileOnly(libraries.cel.compiler) or else we can add implementation 'org.antlr:antlr4-runtime'

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged in the fix for this: google/cel-java#954. Runtime pulling in the antlr dependency is unintended, which is also fixed with this PR. I'll cut a new release sometime next week.

In the interim, I've pushed a snapshot version if you'd like to locally verify the fix (and let me know if you encounter any issues): https://central.sonatype.com/repository/maven-snapshots/dev/cel/runtime/0.12.0-SNAPSHOT/runtime-0.12.0-SNAPSHOT.pom

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes its working with the snapshot version.

I assume there is no periodic release schedule for cel-java, like every 6 weeks for grpc (https://github.com/grpc/grpc-java/milestones or like go/grpc-oss-release-dates)? Is it done as and when required a new release for cel?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part, we cut a release whenever as needed. At a minimum, we'll cut one once per quarter.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.12.0 has been cut. Release notes here: https://github.com/google/cel-java/releases/tag/v0.12.0

cel-protobuf = "dev.cel:protobuf:0.11.1"
cel-compiler = "dev.cel:compiler:0.11.1"
checkstylejava8 = "com.puppycrawl.tools:checkstyle:9.3"
commons-math3 = "org.apache.commons:commons-math3:3.6.1"
conscrypt = "org.conscrypt:conscrypt-openjdk-uber:2.5.2"
Expand Down
3 changes: 2 additions & 1 deletion interop-testing/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ dependencies {
project(':grpc-inprocess'),
project(':grpc-core'),
libraries.mockito.core,
libraries.okhttp
libraries.okhttp,
libraries.cel.compiler

signature (libraries.signature.java) {
artifact {
Expand Down
4 changes: 4 additions & 0 deletions repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ IO_GRPC_GRPC_JAVA_ARTIFACTS = [
"com.google.re2j:re2j:1.8",
"com.google.s2a.proto.v2:s2a-proto:0.1.3",
"com.google.truth:truth:1.4.5",
"dev.cel:runtime:0.11.1",
"dev.cel:compiler:0.11.1",
"dev.cel:protobuf:0.11.1",
"org.antlr:antlr4-runtime:4.9.1",
"com.squareup.okhttp:okhttp:2.7.5",
"com.squareup.okio:okio:2.10.0", # 3.0+ needs swapping to -jvm; need work to avoid flag-day
"io.netty:netty-buffer:4.1.130.Final",
Expand Down
5 changes: 5 additions & 0 deletions xds/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ java_library(
artifact("com.google.errorprone:error_prone_annotations"),
artifact("com.google.guava:guava"),
artifact("com.google.re2j:re2j"),
artifact("dev.cel:runtime"),
artifact("dev.cel:protobuf"),
artifact("org.antlr:antlr4-runtime"),
artifact("io.netty:netty-buffer"),
artifact("io.netty:netty-codec"),
artifact("io.netty:netty-common"),
Expand Down Expand Up @@ -97,6 +100,8 @@ JAR_JAR_RULES = [
"rule com.google.api.expr.** io.grpc.xds.shaded.com.google.api.expr.@1",
"rule com.google.security.** io.grpc.xds.shaded.com.google.security.@1",
"rule dev.cel.expr.** io.grpc.xds.shaded.dev.cel.expr.@1",
"rule dev.cel.** io.grpc.xds.shaded.dev.cel.@1",
"rule cel.** io.grpc.xds.shaded.cel.@1",
"rule envoy.annotations.** io.grpc.xds.shaded.envoy.annotations.@1",
"rule io.envoyproxy.** io.grpc.xds.shaded.io.envoyproxy.@1",
"rule udpa.annotations.** io.grpc.xds.shaded.udpa.annotations.@1",
Expand Down
12 changes: 12 additions & 0 deletions xds/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,19 @@ dependencies {
libraries.re2j,
libraries.auto.value.annotations,
libraries.protobuf.java.util
implementation(libraries.cel.runtime) {
exclude group: 'com.google.protobuf', module: 'protobuf-java'
}
implementation(libraries.cel.protobuf) {
exclude group: 'com.google.protobuf', module: 'protobuf-java'
}
implementation "org.antlr:antlr4-runtime:4.9.1"
def nettyDependency = implementation project(':grpc-netty')

testImplementation project(':grpc-api')
testImplementation project(':grpc-rls')
testImplementation project(':grpc-inprocess')
testImplementation libraries.cel.compiler
testImplementation testFixtures(project(':grpc-core')),
testFixtures(project(':grpc-api')),
testFixtures(project(':grpc-util'))
Expand Down Expand Up @@ -175,13 +183,15 @@ tasks.named("javadoc").configure {
exclude 'io/grpc/xds/XdsNameResolverProvider.java'
exclude 'io/grpc/xds/internal/**'
exclude 'io/grpc/xds/Internal*'
exclude 'dev/cel/**'
}

def prefixName = 'io.grpc.xds'
tasks.named("shadowJar").configure {
archiveClassifier = null
dependencies {
include(project(':grpc-xds'))
include(dependency('dev.cel:.*'))
}
// Relocated packages commonly need exclusions in jacocoTestReport and javadoc
// Keep in sync with BUILD.bazel's JAR_JAR_RULES
Expand All @@ -198,6 +208,8 @@ tasks.named("shadowJar").configure {
// TODO: missing java_package option in .proto
relocate 'udpa.annotations', "${prefixName}.shaded.udpa.annotations"
relocate 'xds.annotations', "${prefixName}.shaded.xds.annotations"
relocate 'dev.cel', "${prefixName}.shaded.dev.cel"
relocate 'cel', "${prefixName}.shaded.cel"
exclude "**/*.proto"
}

Expand Down
34 changes: 33 additions & 1 deletion xds/src/main/java/io/grpc/xds/internal/MatcherParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,43 @@ public static Matchers.StringMatcher parseStringMatcher(
return Matchers.StringMatcher.forSafeRegEx(
Pattern.compile(proto.getSafeRegex().getRegex()));
case CONTAINS:
return Matchers.StringMatcher.forContains(proto.getContains());
return Matchers.StringMatcher.forContains(proto.getContains(), proto.getIgnoreCase());
case MATCHPATTERN_NOT_SET:
default:
throw new IllegalArgumentException(
"Unknown StringMatcher match pattern: " + proto.getMatchPatternCase());
}
}

/** Translate StringMatcher xDS proto to internal StringMatcher. */
public static Matchers.StringMatcher parseStringMatcher(
com.github.xds.type.matcher.v3.StringMatcher proto) {
switch (proto.getMatchPatternCase()) {
case EXACT:
return Matchers.StringMatcher.forExact(proto.getExact(), proto.getIgnoreCase());
case PREFIX:
return Matchers.StringMatcher.forPrefix(
checkNonEmpty(proto.getPrefix(), "prefix"), proto.getIgnoreCase());
case SUFFIX:
return Matchers.StringMatcher.forSuffix(
checkNonEmpty(proto.getSuffix(), "suffix"), proto.getIgnoreCase());
case CONTAINS:
return Matchers.StringMatcher.forContains(
checkNonEmpty(proto.getContains(), "contains"), proto.getIgnoreCase());
case SAFE_REGEX:
String regex = checkNonEmpty(proto.getSafeRegex().getRegex(), "regex");
return Matchers.StringMatcher.forSafeRegEx(Pattern.compile(regex));
default:
throw new IllegalArgumentException(
"Unknown StringMatcher match pattern: " + proto.getMatchPatternCase());
}
}

private static String checkNonEmpty(String value, String name) {
if (value.isEmpty()) {
throw new IllegalArgumentException("StringMatcher " + name
+ " (match_pattern) must be non-empty");
}
return value;
}
}
13 changes: 10 additions & 3 deletions xds/src/main/java/io/grpc/xds/internal/Matchers.java
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,15 @@ public static StringMatcher forSafeRegEx(Pattern regEx) {
}

/** The input string should contain this substring. */
public static StringMatcher forContains(String contains) {
public static StringMatcher forContains(String contains, boolean ignoreCase) {
checkNotNull(contains, "contains");
return StringMatcher.create(null, null, null, null, contains,
false/* doesn't matter */);
ignoreCase);
}

/** The input string should contain this substring. */
public static StringMatcher forContains(String contains) {
return forContains(contains, false);
}

/** Returns the matching result for this string. */
Expand All @@ -281,7 +286,9 @@ public boolean matches(String args) {
? args.toLowerCase(Locale.ROOT).endsWith(suffix().toLowerCase(Locale.ROOT))
: args.endsWith(suffix());
} else if (contains() != null) {
return args.contains(contains());
return ignoreCase()
? args.toLowerCase(Locale.ROOT).contains(contains().toLowerCase(Locale.ROOT))
: args.contains(contains());
}
return regEx().matches(args);
}
Expand Down
72 changes: 72 additions & 0 deletions xds/src/main/java/io/grpc/xds/internal/matcher/CelCommon.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright 2026 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.grpc.xds.internal.matcher;

import dev.cel.common.CelAbstractSyntaxTree;
import dev.cel.common.CelOptions;
import dev.cel.runtime.CelRuntime;
import dev.cel.runtime.CelRuntimeFactory;

/**
* Shared utilities for CEL-based matchers and extractors.
*/
final class CelCommon {
private static final CelOptions CEL_OPTIONS = CelOptions.newBuilder()
.enableComprehension(false)
.maxRegexProgramSize(100)
.build();


private static final dev.cel.runtime.CelStandardFunctions FUNCTIONS =
dev.cel.runtime.CelStandardFunctions.newBuilder()
.filterFunctions((func, over) -> {
if (func == dev.cel.runtime.CelStandardFunctions.StandardFunction.STRING) {
return false;
}
if (func == dev.cel.runtime.CelStandardFunctions.StandardFunction.ADD) {
return !over.equals(
(Object) dev.cel.runtime.standard.AddOperator.AddOverload.ADD_STRING)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we needing to cast to Object? If you're working around ErrorProne, then something seems broken and casting to Object is just hiding it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it was an errorProne workaround.
over is statically known as a StandardOverload interface, but ADD_STRING is an AddOverload enum. ErrorProne threw an [EqualsIncompatibleType] build error so we need to cast the ADD_STRING enum specifically to (Object) before passing it into equals.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ADD_STRING is a CelStandardOverload, which isn't a StandardOverload. It seems ErrorProne is correct; these will never be equal.

You should comment cases like this. If this were a bug in ErrorProne we'd probably want to file a bug with ErrorProne, but even if not, the comment helps us know when it is no longer necessary. (We don't have to guess why it was done, so we can confirm if that is no longer the case.)

It looks like that has been adjusted on cel-java master, as the function is now passed CelStandardOverload.

@sergiitk, the interface change looks like a breaking API change. That's surprising since I thought we were using stable APIs. Is this PR using an API that it shouldn't be, was this a known change that just hadn't been released yet, or should we be concerned about the actual stability of the API?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a relatively new API that didn't exist when API stabilization doc was made (go/cel-api-stabilization). It's stable now (I'll update the doc when I get a chance). Reason why I suggested this API is that it follows the recommended pattern for how environment subsetting is supposed to be done across all CEL stacks.

As for EP -- I'm surprised it's actually flagging this (internally, it doesn't). AddOperator enum implements CelStandardOverload, so they should equal. My only guess is that CelStandardOverload is a functional interface, so theoretically it's possible to provide a lambda that's not equals comparable. Though we control the standard overload implementations, so in practice this won't happen. Checking for reference identity should likely get around the issue over == AddOverload.ADD_STRING

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, this is the new setup flow. I didn't look where this FUNCTIONS constant was used. That's fine; I'm less surprised that changed. (And yes, it's good to add it to the doc.)

I'm surprised it's actually flagging this (internally, it doesn't)

I don't think it'd get flagged on master either. It only looked to be a problem because we're still using 0.11.1; in 0.11.1 I agree with ErrorProne that the equals() will always return false.

&& !over.equals(
(Object) dev.cel.runtime.standard.AddOperator.AddOverload.ADD_LIST);
}
return true;
})
.build();

static final CelRuntime RUNTIME = CelRuntimeFactory.standardCelRuntimeBuilder()
.setStandardEnvironmentEnabled(false)
.setStandardFunctions(FUNCTIONS)
.setOptions(CEL_OPTIONS)
.build();

private CelCommon() {}

static void checkAllowedVariables(CelAbstractSyntaxTree ast) {
for (java.util.Map.Entry<Long, dev.cel.common.ast.CelReference> entry :
ast.getReferenceMap().entrySet()) {
dev.cel.common.ast.CelReference ref = entry.getValue();
// If overload_id is empty, it's a variable reference or type name.
// We only support "request".
if (!ref.value().isPresent() && ref.overloadIds().isEmpty()) {
if (!"request".equals(ref.name())) {
throw new IllegalArgumentException(
"CEL expression references unknown variable: " + ref.name());
}
}
}
}
}
73 changes: 73 additions & 0 deletions xds/src/main/java/io/grpc/xds/internal/matcher/CelMatcher.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright 2026 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.grpc.xds.internal.matcher;

import dev.cel.common.CelAbstractSyntaxTree;
import dev.cel.common.types.SimpleType;
import dev.cel.runtime.CelEvaluationException;
import dev.cel.runtime.CelRuntime;

/**
* Executes compiled CEL expressions.
*/
public final class CelMatcher {
private final CelRuntime.Program program;

private CelMatcher(CelRuntime.Program program) {
this.program = program;
}

/**
* Compiles the AST into a CelMatcher.
* Throws an Exception if validation or evaluation fails during compilation setup.
*/
public static CelMatcher compile(CelAbstractSyntaxTree ast)
throws Exception {
// CelEvaluationException -> inside cel-runtime -> Allowed in production signatures
// CelValidationException -> inside cel-compiler -> Forbidden in production signatures
if (ast.getResultType() != SimpleType.BOOL) {
throw new IllegalArgumentException(
"CEL expression must evaluate to boolean, got: " + ast.getResultType());
}
CelCommon.checkAllowedVariables(ast);
CelRuntime.Program program = CelCommon.RUNTIME.createProgram(ast);
return new CelMatcher(program);
}

/**
* Evaluates the CEL expression against the input activation.
*/
public boolean match(Object input) throws CelEvaluationException {
Object result;
if (input instanceof dev.cel.runtime.CelVariableResolver) {
result = program.eval((dev.cel.runtime.CelVariableResolver) input);
} else if (input instanceof java.util.Map) {
@SuppressWarnings("unchecked")
java.util.Map<String, ?> mapInput = (java.util.Map<String, ?>) input;
result = program.eval(mapInput);
} else {
throw new CelEvaluationException(
"Unsupported input type for CEL evaluation: " + input.getClass().getName());
}

if (result instanceof Boolean) {
return (Boolean) result;
}
throw new CelEvaluationException(
"CEL expression must evaluate to boolean, got: " + result.getClass().getName());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright 2026 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.grpc.xds.internal.matcher;

import dev.cel.common.CelAbstractSyntaxTree;
import dev.cel.common.types.SimpleType;
import dev.cel.runtime.CelEvaluationException;
import dev.cel.runtime.CelRuntime;

/**
* Executes compiled CEL expressions that extract a string.
*/
public final class CelStringExtractor {
private final CelRuntime.Program program;

private CelStringExtractor(CelRuntime.Program program) {
this.program = program;
}

/**
* Compiles the AST into a CelStringExtractor.
* Throws an Exception if validation or evaluation fails during compilation setup.
*/
public static CelStringExtractor compile(CelAbstractSyntaxTree ast)
throws Exception {
if (ast.getResultType() != SimpleType.STRING && ast.getResultType() != SimpleType.DYN) {
throw new IllegalArgumentException(
"CEL expression must evaluate to string, got: " + ast.getResultType());
}
CelCommon.checkAllowedVariables(ast);
CelRuntime.Program program = CelCommon.RUNTIME.createProgram(ast);
return new CelStringExtractor(program);
}

/**
* Evaluates the CEL expression against the input activation and returns the string result.
* Returns null if the result is not a string.
*/
public String extract(Object input) throws CelEvaluationException {
Object result;
if (input instanceof dev.cel.runtime.CelVariableResolver) {
result = program.eval((dev.cel.runtime.CelVariableResolver) input);
} else if (input instanceof java.util.Map) {
@SuppressWarnings("unchecked")
java.util.Map<String, ?> mapInput = (java.util.Map<String, ?>) input;
result = program.eval(mapInput);
} else {
throw new CelEvaluationException(
"Unsupported input type for CEL evaluation: " + input.getClass().getName());
}

if (result instanceof String) {
return (String) result;
}
return null;
}
}
Loading