xds: add support for custom, per-target credentials on the transport. #11919

AshZhang · 2025-02-21T21:05:15Z

Add support for custom, per-target (per-RPC) credentials on the xDS transport.

This allows clients to use different credentials when requesting xDS resources for different targets that may require different authorization tokens to access.

To use a custom credential on the transport during resolution, the application is responsible for associating a CallCredentials object with a target in XdsTransportCallCredentialsProvider. The application then creates a channel to the target as usual, and the xDS client library takes care of looking up the correct CallCredentials object to use when resolving the target.

A custom per-target credential can only be set once for the channel / xDS transport, so it must be added to XdsTransportCallCredentialsProvider before creating the channel for the target. If called when there is already an existing channel for the target, XdsTransportCallCredentialsProvider.setTransportCallCredentials() will ignore the CallCredentials object that was newly passed in.

…PoolProvider.

…transport does nothing

AshZhang · 2025-02-24T17:23:33Z

CC: @anicr7 @ejona86

ejona86 · 2025-02-24T18:32:10Z

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

@@ -65,14 +77,19 @@ public GrpcXdsTransport(ManagedChannel channel) {
      this.channel = checkNotNull(channel, "channel");
    }

+    void setCallCredentials(CallCredentials callCredentials) {


Why not pass this in the constructor?

Replaced the setter with a CallCredentials parameter in the constructors. I originally used a setter here to avoid impacting GrpcXdsTransport constructor callsites (e.g. across tests), but I have updated those now.

FWIW, you could have left the old constructors in place and them just becoming something like this(serverInfo, null). If there's only a few callsites, it is nice to update the old callsites with the new parameters. But if there are a lot, then leaving the old constructors in place is easy and simple.

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

ejona86 · 2025-02-24T18:38:24Z

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

+    if (SharedXdsClientPoolProvider.getDefaultProvider().get(target) != null) {
+      return false;
+    }
+    xdsTransportCallCredentials.put(target, creds);


So once registered this will exist for the life of the process. Are there memory usage concerns there?

Updated RefCountedXdsClientObjectPool to remove the (target, CallCredentials) entry when the xDS client is cleaned up at refcount 0.

Is that what we're doing in the other languages? That actually sounds racy for this configuration, as a channel could be shutting down when you configuring the CallCreds.

ejona86 · 2025-02-24T18:40:46Z

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

+   * <p>A custom {@code CallCredentials} can only be set once on the xDS transport; in other words,
+   * it is not possible to change the custom transport {@code CallCredentials} for an existing xDS
+   * client. If {@code setTransportCallCredentials} is called when there is already an existing xDS
+   * client for the target, then this does nothing and returns false.


The false return is not reliable. So we should weaken the language here a bit to say it "attempts" or "tries" to return false, or say it is "best effort".

Are the callers actually going to use the return value?

Done, reworded the javadoc.

The return value is informational for callers to know whether their custom transport per-RPC credential is actually being used. A false return value gives clients the choice to handle it in any way - e.g. print a warning log, fail, or ignore.

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

…grpc#11724) Per-rpc verification of authority specified via call options or set by the LB API against peer cert's subject names.

…olProvider, and remove old creds when xDS client is deleted

ejona86 · 2025-02-25T21:26:46Z

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

  }

  @VisibleForTesting
  static class GrpcXdsTransport implements XdsTransport {

    private final ManagedChannel channel;
+    private CallCredentials callCredentials;


ejona86 · 2025-02-25T21:26:50Z

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

@@ -65,14 +77,19 @@ public GrpcXdsTransport(ManagedChannel channel) {
      this.channel = checkNotNull(channel, "channel");
    }

+    void setCallCredentials(CallCredentials callCredentials) {


FWIW, you could have left the old constructors in place and them just becoming something like this(serverInfo, null). If there's only a few callsites, it is nice to update the old callsites with the new parameters. But if there are a lot, then leaving the old constructors in place is easy and simple.

ejona86 · 2025-02-25T21:27:39Z

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


  @Override
  public XdsTransport create(Bootstrapper.ServerInfo serverInfo) {
-    return new GrpcXdsTransport(serverInfo);
+    GrpcXdsTransport transport = new GrpcXdsTransport(serverInfo, callCredentials);


nit: return directly without the local variable since you aren't calling set() any more?

ejona86 · 2025-02-25T21:29:21Z

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

@@ -59,6 +59,8 @@ final class SharedXdsClientPoolProvider implements XdsClientPoolFactory {
  private final Object lock = new Object();
  private final AtomicReference<Map<String, ?>> bootstrapOverride = new AtomicReference<>();
  private final Map<String, ObjectPool<XdsClient>> targetToXdsClientMap = new ConcurrentHashMap<>();
+  private static final Map<String, CallCredentials> transportCallCredentials =


This class is already a singleton/has a default instance. Make this non-static.

ejona86 · 2025-02-25T21:31:28Z

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

+    if (SharedXdsClientPoolProvider.getDefaultProvider().get(target) != null) {
+      return false;
+    }
+    xdsTransportCallCredentials.put(target, creds);


Is that what we're doing in the other languages? That actually sounds racy for this configuration, as a channel could be shutting down when you configuring the CallCreds.

ejona86 · 2025-02-25T21:37:49Z

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

@@ -35,34 +36,45 @@
 final class GrpcXdsTransportFactory implements XdsTransportFactory {

  static final GrpcXdsTransportFactory DEFAULT_XDS_TRANSPORT_FACTORY =


Move this into the tests that use it? It isn't used any more in production. (I probably wouldn't make it a CONSTANT like it is here, at it is no longer the DEFAULT and such. I'd probably just create a new instance when needed.)

AshZhang · 2025-03-08T00:30:48Z

Closing this as we are going to use NameResolver.Args instead.

AshZhang added 5 commits February 21, 2025 20:26

xds: Add provider for custom per-target xDS transport call credentials.

497fca5

xds: add custom CallCredentials support to GrpcXdsTransportFactory.

f3dbd10

xds: support custom xDS transport call credentials in SharedXdsClient…

aaf42f0

…PoolProvider.

Fix style

e7e308d

Test coverage: setting custom credentials for an existing xDS client/…

aa9c884

…transport does nothing

AshZhang changed the title ~~Add support for custom, per-target credentials on the xDS transport.~~ xds: add support for custom, per-target credentials on the transport. Feb 21, 2025

Reduce visibility of GrpcXdsTransport method

e941e01

AshZhang marked this pull request as ready for review February 24, 2025 17:18

ejona86 reviewed Feb 24, 2025

View reviewed changes

kannanjgithub and others added 6 commits February 24, 2025 23:11

netty: Per-rpc authority verification against peer cert subject names (…

72b3d55

…grpc#11724) Per-rpc verification of authority specified via call options or set by the LB API against peer cert's subject names.

Move CallCredentials to GrpcXdsTransport constructors

e6e2b1d

Move custom transport CallCredentials directly into SharedXdsClientPo…

518326e

…olProvider, and remove old creds when xDS client is deleted

Merge branch 'grpc:master' into transport_call_creds

26643ef

Fix GrpcXdsTransport constructor callsite

33796af

Clarify javadoc

a671484

ejona86 reviewed Feb 25, 2025

View reviewed changes

AshZhang closed this Mar 8, 2025

AshZhang deleted the transport_call_creds branch March 8, 2025 00:37

AshZhang mentioned this pull request Mar 11, 2025

xds: add support for custom per-target credentials on the transport. #11951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xds: add support for custom, per-target credentials on the transport. #11919

xds: add support for custom, per-target credentials on the transport. #11919

AshZhang commented Feb 21, 2025

AshZhang commented Feb 24, 2025 •

edited

Loading

ejona86 Feb 24, 2025

AshZhang Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 24, 2025

AshZhang Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 24, 2025

AshZhang Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 25, 2025

ejona86 Feb 25, 2025

AshZhang commented Mar 8, 2025

		@@ -35,34 +36,45 @@
		final class GrpcXdsTransportFactory implements XdsTransportFactory {

		static final GrpcXdsTransportFactory DEFAULT_XDS_TRANSPORT_FACTORY =

xds: add support for custom, per-target credentials on the transport. #11919

xds: add support for custom, per-target credentials on the transport. #11919

Conversation

AshZhang commented Feb 21, 2025

AshZhang commented Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AshZhang commented Mar 8, 2025

AshZhang commented Feb 24, 2025 •

edited

Loading