xds: Implement channel caching utility for GrpcService channels for ext_authz and ext_proc by sauravzg · Pull Request #12690 · grpc/grpc-java

sauravzg · 2026-03-12T19:26:03Z

Child PR of #12492

Split this from some other PR way down the chain and modified it to be not ext authz specific.

This is currently a thread safe trivial implementation of caching based on (target, channel_creds)
This is intended to be instantiated per Filter and not be used as some sort of a shared channel pool
On a config change that requires creating new channels, this may perform 2 blocking operations before returning
Creation of the channel and this happens within a lock
shutdown of old channel if present

This should probably be okay, since this happens in the control path changes but has the potential of hammering dataplane throughtput temporarily during updates due to channel creation within the lock.

This commit introduces configuration objects for the external authorization (ExtAuthz) filter and the gRPC service it uses. These classes provide a structured, immutable representation of the configuration defined in the xDS protobuf messages. The main new classes are: - `ExtAuthzConfig`: Represents the configuration for the `ExtAuthz` filter, including settings for the gRPC service, header mutation rules, and other filter behaviors. - `GrpcServiceConfig`: Represents the configuration for a gRPC service, including the target URI, credentials, and other settings. - `HeaderMutationRulesConfig`: Represents the configuration for header mutation rules. This commit also includes parsers to create these configuration objects from the corresponding protobuf messages, as well as unit tests for the new classes.

… the updated requirements

kannanjgithub

Minor changes requested.

xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParser.java

xds/src/main/java/io/grpc/xds/internal/grpcservice/CachedChannelManager.java

… address nits.

… and add test coverage

… bug Makes `allowedGrpcServices` to be a non-optional struct instead of an `Optional<Map<str,AllowedService>>` since it's essentially an immuatable hash map, making it preferable to use an empty instance instead of null. Change a small bug where we continued instead of return when parsing bootstrap credentials.

Copilot

Pull request overview

Adds internal xDS utilities to parse Envoy GrpcService/ext_authz configuration into immutable Java config objects, introduces bootstrap parsing for allowed_grpc_services, and provides a simple per-filter ManagedChannel caching utility keyed by (target, channel_creds).

Changes:

Add GrpcServiceConfig model + GrpcServiceConfigParser (channel creds, access-token call creds, initial metadata, timeout) and related header validation/value utilities.
Add ext_authz config model/parser plus shared header-mutation rules model/parser and matcher parsing for fractional percents.
Add AllowedGrpcServices bootstrap parsing + CachedChannelManager for per-filter channel reuse.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutationRulesParser.java	Parses Envoy `HeaderMutationRules` into an internal config.
xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutationRulesConfig.java	Immutable config representation for header mutation rules.
xds/src/main/java/io/grpc/xds/internal/grpcservice/HeaderValueValidationUtils.java	Validates header key/value constraints for initial metadata.
xds/src/main/java/io/grpc/xds/internal/grpcservice/HeaderValue.java	Immutable internal header key/value representation (string vs raw bytes).
xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceXdsContextProvider.java	Provides per-target context for trusted/untrusted control plane decisions.
xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceXdsContext.java	AutoValue context object used during `GrpcService` parsing.
xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceParseException.java	Parse exception for GrpcService config parsing.
xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParser.java	Parses Envoy `GrpcService`/`GoogleGrpc`, validates metadata/timeout, builds creds.
xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfig.java	Immutable `GrpcService` config model.
xds/src/main/java/io/grpc/xds/internal/grpcservice/ConfiguredChannelCredentials.java	Bundles `ChannelCredentials` with comparable credential config.
xds/src/main/java/io/grpc/xds/internal/grpcservice/ChannelCredsConfig.java	Interface for credential config identity/equality.
xds/src/main/java/io/grpc/xds/internal/grpcservice/CachedChannelManager.java	Per-filter cache for `ManagedChannel` keyed by target + creds config.
xds/src/main/java/io/grpc/xds/internal/grpcservice/AllowedGrpcServices.java	Container for parsed `allowed_grpc_services` entries.
xds/src/main/java/io/grpc/xds/internal/grpcservice/AllowedGrpcService.java	Single allowed service entry (override creds/call creds).
xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzParseException.java	Parse exception for ext_authz parsing.
xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzConfigParser.java	Parses Envoy `ExtAuthz` into an internal config model.
xds/src/main/java/io/grpc/xds/internal/extauthz/ExtAuthzConfig.java	Immutable ext_authz config model.
xds/src/main/java/io/grpc/xds/internal/MatcherParser.java	Adds parsing for Envoy `FractionalPercent` to internal matcher type.
xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java	Wires bootstrap parsing hook for `allowed_grpc_services`.
xds/src/main/java/io/grpc/xds/client/Bootstrapper.java	Adds `allowedGrpcServices()` field to bootstrap info (opaque type).
xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java	Implements `allowed_grpc_services` parsing and wraps channel creds with config identity.
xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutationRulesParserTest.java	Tests parsing of header mutation rules and invalid regex behavior.
xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutationRulesConfigTest.java	Tests builder defaults and field setting for header mutation config.
xds/src/test/java/io/grpc/xds/internal/grpcservice/HeaderValueValidationUtilsTest.java	Tests header ignore/validation behavior (key + value/raw length rules).
xds/src/test/java/io/grpc/xds/internal/grpcservice/HeaderValueTest.java	Tests `HeaderValue` creation for string vs raw values.
xds/src/test/java/io/grpc/xds/internal/grpcservice/GrpcServiceXdsContextTestUtil.java	Provides dummy `GrpcServiceXdsContextProvider` for parser tests.
xds/src/test/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParserTest.java	Tests GrpcService parsing, creds selection, untrusted override behavior, timeout, call-creds security gating.
xds/src/test/java/io/grpc/xds/internal/grpcservice/CachedChannelManagerTest.java	Tests channel caching and shutdown behavior on config changes/close.
xds/src/test/java/io/grpc/xds/internal/extauthz/ExtAuthzConfigParserTest.java	Tests ext_authz parsing including header mutation rules and matchers.
xds/src/test/java/io/grpc/xds/internal/MatcherParserTest.java	Tests parsing of string matchers and fractional percent denominators.
xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java	Adds tests for parsing bootstrap `allowed_grpc_services`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T17:09:13Z

xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParser.java

+      if (HeaderValueValidationUtils.shouldIgnore(headerValue)) {
+        throw new GrpcServiceParseException("Invalid initial metadata header: " + key);
+      }
+      initialMetadata.add(headerValue);


HeaderValueValidationUtils.shouldIgnore(...) (and its Javadoc) indicates headers should be ignored/skipped, but the parser currently throws on shouldIgnore(headerValue). This is easy to misread/maintain and may not match the intended behavior. Either rename the helper to reflect “invalid/unsupported header” semantics, or change the parser to skip ignored headers instead of failing the whole config.

Suggested change

if (HeaderValueValidationUtils.shouldIgnore(headerValue)) {

throw new GrpcServiceParseException("Invalid initial metadata header: " + key);

}

initialMetadata.add(headerValue);

if (!HeaderValueValidationUtils.shouldIgnore(headerValue)) {

initialMetadata.add(headerValue);

}

Copilot · 2026-03-24T17:09:14Z

xds/src/main/java/io/grpc/xds/internal/grpcservice/GrpcServiceConfigParser.java

+    if (grpcServiceProto.hasTimeout()) {
+      com.google.protobuf.Duration timeout = grpcServiceProto.getTimeout();
+      if (timeout.getSeconds() < 0 || timeout.getNanos() < 0
+          || (timeout.getSeconds() == 0 && timeout.getNanos() == 0)) {
+        throw new GrpcServiceParseException("Timeout must be strictly positive");
+      }
+      builder.timeout(Duration.ofSeconds(timeout.getSeconds(), timeout.getNanos()));
+    }


Timeout validation only checks for negative/zero values, but does not validate protobuf Duration invariants (e.g., nanos should be in [0, 999,999,999]). Passing an unnormalized duration can lead to unexpected normalization by Duration.ofSeconds(...) or runtime exceptions on extreme values. Consider explicitly validating the nanos upper bound (and catching ArithmeticException/DateTimeException from Duration.ofSeconds) and failing with a GrpcServiceParseException.

Copilot · 2026-03-24T17:09:14Z

xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutationRulesParser.java

+import io.grpc.xds.internal.extauthz.ExtAuthzParseException;
+
+/**
+ * Parser for {@link io.envoyproxy.envoy.config.common.mutation_rules.v3.HeaderMutationRules}.
+ */
+public final class HeaderMutationRulesParser {
+
+  private HeaderMutationRulesParser() {}
+
+  public static HeaderMutationRulesConfig parse(HeaderMutationRules proto)
+      throws ExtAuthzParseException {
+    HeaderMutationRulesConfig.Builder builder = HeaderMutationRulesConfig.builder();


HeaderMutationRulesParser is placed in a generic headermutations package, but it throws/depends on ExtAuthzParseException, which couples it to the ext_authz filter and makes reuse (e.g., ext_proc) awkward. Consider introducing a filter-agnostic parse exception (e.g., HeaderMutationRulesParseException) or throwing IllegalArgumentException and letting the caller wrap into filter-specific exceptions.

Copilot · 2026-03-24T17:09:14Z

xds/src/main/java/io/grpc/xds/GrpcBootstrapperImpl.java

+  @SuppressWarnings("unused")
+  private static Optional<CallCredentials> parseCallCredentials(List<Map<String, ?>> jsonList,
+                                                          String targetUri)
+      throws XdsInitializationException {
+    // TODO(sauravzg): Currently no xDS call credentials providers are implemented (no
+    // XdsCallCredentialsRegistry).
+    // As per A102/A97, we should just ignore unsupported call credentials types
+    // without throwing an exception.
+    return Optional.empty();
+  }


@SuppressWarnings("unused") on parseCallCredentials(...) is misleading because the method is actually used by parseAllowedGrpcServices(...). Consider removing the suppression to avoid masking real unused warnings in the future.

Copilot · 2026-03-24T17:09:15Z

xds/src/main/java/io/grpc/xds/internal/grpcservice/CachedChannelManager.java

+    ChannelHolder holder = channelHolder.get();
+    if (holder != null) {
+      holder.channel().shutdown();
+    }


close() shuts down the cached channel but does not clear channelHolder. If getChannel() is called after close(), it can return the already-shutdown channel on the fast path because the key still matches. Consider clearing the reference (e.g., getAndSet(null)) and/or tracking a closed state to prevent reuse; also synchronize with the same lock used by getChannel() to avoid races with concurrent updates.

Suggested change

ChannelHolder holder = channelHolder.get();

if (holder != null) {

holder.channel().shutdown();

}

ManagedChannel channelToShutdown = null;

synchronized (lock) {

ChannelHolder holder = channelHolder.getAndSet(null);

if (holder != null) {

channelToShutdown = holder.channel();

}

}

if (channelToShutdown != null) {

channelToShutdown.shutdown();

}

kannanjgithub · 2026-03-24T17:17:14Z

The review by Co-pilot seems to have all valid points. Please fix them.

sauravzg added 3 commits February 2, 2026 05:09

Fixup: Address comments from grpc#12492

5654c64

Fixup: 12492 Split HeaderValueValidationUtils to GrpcService to match…

5ed6993

… the updated requirements

kannanjgithub requested changes Mar 13, 2026

View reviewed changes

sauravzg mentioned this pull request Mar 15, 2026

xds: Add header mutations library #12494

Open

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Mar 15, 2026

Fixup: grpc#12690: Add VisibleForTesting

31d62ad

Fixup: 12492 Remove artifact from old channelcreds implementation and…

35f3956

… address nits.

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Mar 15, 2026

Fixup: grpc#12690: Add VisibleForTesting

1e202f0

sauravzg force-pushed the feat/cached-channel branch from 31d62ad to 1e202f0 Compare March 15, 2026 21:44

kannanjgithub requested a review from AgraVator March 17, 2026 06:02

Fixup 12492: Use builder instead of newBuilder

1d4e48b

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Mar 17, 2026

Fixup: grpc#12690: Add VisibleForTesting

0e12e12

sauravzg force-pushed the feat/cached-channel branch from 1e202f0 to 0bb06f3 Compare March 17, 2026 07:02

Fixup: 12492 Fix callcredentials to not apply instead of erroring out…

c1b95f1

… and add test coverage

sauravzg added a commit to sauravzg/grpc-java that referenced this pull request Mar 17, 2026

Fixup: grpc#12690: Add VisibleForTesting

caa0c91

sauravzg force-pushed the feat/cached-channel branch from 0bb06f3 to a748980 Compare March 17, 2026 08:40

sauravzg added 4 commits March 23, 2026 16:46

feat(xds): Add CachedChannelManager for caching channel instances

04ee79b

Fixup: grpc#12690: Add VisibleForTesting

869bdd4

Fixup: 12690 Use builder in unit tests

75f7130

sauravzg force-pushed the feat/cached-channel branch from a748980 to 75f7130 Compare March 23, 2026 18:02

kannanjgithub requested a review from Copilot March 24, 2026 17:03

Copilot started reviewing on behalf of kannanjgithub March 24, 2026 17:04 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xds: Implement channel caching utility for GrpcService channels for ext_authz and ext_proc#12690

xds: Implement channel caching utility for GrpcService channels for ext_authz and ext_proc#12690
sauravzg wants to merge 10 commits intogrpc:masterfrom
sauravzg:feat/cached-channel

sauravzg commented Mar 12, 2026

Uh oh!

kannanjgithub left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

kannanjgithub commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    ChannelHolder holder = channelHolder.get();
-    if (holder != null) {
-      holder.channel().shutdown();
-    }
+    ManagedChannel channelToShutdown = null;
+    synchronized (lock) {
+      ChannelHolder holder = channelHolder.getAndSet(null);
+      if (holder != null) {
+        channelToShutdown = holder.channel();
+      }
+    }
+    if (channelToShutdown != null) {
+      channelToShutdown.shutdown();
+    }

Conversation

sauravzg commented Mar 12, 2026

Uh oh!

kannanjgithub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

kannanjgithub commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants