Postponed ChannelCredentials instantiation.

Lifetime of `ChannelCredentials` is now bounded to `GrpcXdsTransport`

Author: Zgoda91

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

@@ -17,8 +17,7 @@
 package io.grpc.xds;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableMap;
-import io.grpc.ChannelCredentials;
+import com.google.common.collect.ImmutableSet;
 import io.grpc.internal.JsonUtil;
 import io.grpc.xds.client.BootstrapperImpl;
 import io.grpc.xds.client.XdsInitializationException;
@@ -90,30 +89,30 @@ protected String getJsonContent() throws XdsInitializationException, IOException
   }
 
   @Override
-  protected Object getImplSpecificConfig(Map<String, ?> serverConfig, String serverUri)
+  protected Map<String, ?> getImplSpecificConfig(Map<String, ?> serverConfig, String serverUri)
       throws XdsInitializationException {
-    return getChannelCredentials(serverConfig, serverUri);
+    return getChannelCredentialsConfig(serverConfig, serverUri);
   }
 
-  private static ChannelCredentials getChannelCredentials(Map<String, ?> serverConfig,
-                                                          String serverUri)
+  private static Map<String, ?> getChannelCredentialsConfig(Map<String, ?> serverConfig,
+                                                            String serverUri)
       throws XdsInitializationException {
     List<?> rawChannelCredsList = JsonUtil.getList(serverConfig, "channel_creds");
     if (rawChannelCredsList == null || rawChannelCredsList.isEmpty()) {
       throw new XdsInitializationException(
           "Invalid bootstrap: server " + serverUri + " 'channel_creds' required");
     }
-    ChannelCredentials channelCredentials =
+    Map<String, ?> channelCredentialsConfig =
         parseChannelCredentials(JsonUtil.checkObjectList(rawChannelCredsList), serverUri);
-    if (channelCredentials == null) {
+    if (channelCredentialsConfig == null) {
       throw new XdsInitializationException(
           "Server " + serverUri + ": no supported channel credentials found");
     }
-    return channelCredentials;
+    return channelCredentialsConfig;
   }
 
   @Nullable
-  private static ChannelCredentials parseChannelCredentials(List<Map<String, ?>> jsonList,
+  private static Map<String, ?> parseChannelCredentials(List<Map<String, ?>> jsonList,
                                                             String serverUri)
       throws XdsInitializationException {
     for (Map<String, ?> channelCreds : jsonList) {
@@ -122,15 +121,10 @@ private static ChannelCredentials parseChannelCredentials(List<Map<String, ?>> j
         throw new XdsInitializationException(
             "Invalid bootstrap: server " + serverUri + " with 'channel_creds' type unspecified");
       }
-      XdsCredentialsProvider provider =  XdsCredentialsRegistry.getDefaultRegistry()
-          .getProvider(type);
-      if (provider != null) {
-        Map<String, ?> config = JsonUtil.getObject(channelCreds, "config");
-        if (config == null) {
-          config = ImmutableMap.of();
-        }
-
-        return provider.newChannelCredentials(config);
+      ImmutableSet<String> supportedNames =  XdsCredentialsRegistry.getDefaultRegistry()
+          .getSupportedCredentialNames();
+      if (supportedNames.contains(type)) {
+        return channelCreds;
       }
     }
     return null;

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

@@ -19,6 +19,8 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import io.grpc.CallCredentials;
 import io.grpc.CallOptions;
 import io.grpc.ChannelCredentials;
@@ -28,9 +30,15 @@
 import io.grpc.ManagedChannel;
 import io.grpc.Metadata;
 import io.grpc.MethodDescriptor;
+import io.grpc.ResourceAllocatingChannelCredentials;
 import io.grpc.Status;
+import io.grpc.internal.GrpcUtil;
+import io.grpc.internal.JsonUtil;
 import io.grpc.xds.client.Bootstrapper;
+import io.grpc.xds.client.XdsInitializationException;
 import io.grpc.xds.client.XdsTransportFactory;
+import java.io.Closeable;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 final class GrpcXdsTransportFactory implements XdsTransportFactory {
@@ -42,7 +50,7 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory {
   }
 
   @Override
-  public XdsTransport create(Bootstrapper.ServerInfo serverInfo) {
+  public XdsTransport create(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException {
     return new GrpcXdsTransport(serverInfo, callCredentials);
   }
 
@@ -56,8 +64,9 @@ static class GrpcXdsTransport implements XdsTransport {
 
     private final ManagedChannel channel;
     private final CallCredentials callCredentials;
+    private final ImmutableList<Closeable> resources;
 
-    public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo) {
+    public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException {
       this(serverInfo, null);
     }
 
@@ -66,9 +75,27 @@ public GrpcXdsTransport(ManagedChannel channel) {
       this(channel, null);
     }
 
-    public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials) {
+    public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials)
+        throws XdsInitializationException {
       String target = serverInfo.target();
-      ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig();
+      Map<String, ?> implSpecificConfig = serverInfo.implSpecificConfig();
+      String type = JsonUtil.getString(implSpecificConfig, "type");
+      XdsCredentialsProvider provider =  XdsCredentialsRegistry.getDefaultRegistry()
+          .getProvider(type);
+      Map<String, ?> config = JsonUtil.getObject(implSpecificConfig, "config");
+      if (config == null) {
+        config = ImmutableMap.of();
+      }
+      ChannelCredentials channelCredentials = provider.newChannelCredentials(config);
+      if (channelCredentials == null) {
+        throw new XdsInitializationException(
+            "Cannot create channel credentials of type " + type + " for target " + target);
+      }
+      // if {@code ChannelCredentials} instance has allocated resource of any type, save them to be
+      // released once the channel is shutdown
+      this.resources = (channelCredentials instanceof ResourceAllocatingChannelCredentials)
+          ? ((ResourceAllocatingChannelCredentials) channelCredentials).getAllocatedResources()
+          : ImmutableList.<Closeable>of();
       this.channel = Grpc.newChannelBuilder(target, channelCredentials)
           .keepAliveTime(5, TimeUnit.MINUTES)
           .build();
@@ -79,6 +106,7 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call
     public GrpcXdsTransport(ManagedChannel channel, CallCredentials callCredentials) {
       this.channel = checkNotNull(channel, "channel");
       this.callCredentials = callCredentials;
+      this.resources = ImmutableList.<Closeable>of();
     }
 
     @Override
@@ -99,6 +127,9 @@ public <ReqT, RespT> StreamingCall<ReqT, RespT> createStreamingCall(
     @Override
     public void shutdown() {
       channel.shutdown();
+      for (Closeable resource : resources) {
+        GrpcUtil.closeQuietly(resource);
+      }
     }
 
     private class XdsStreamingCall<ReqT, RespT> implements

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

@@ -21,6 +21,7 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.annotations.concurrent.GuardedBy;
 import io.grpc.InternalServiceProviders;
 import java.util.ArrayList;
@@ -147,6 +148,14 @@ public synchronized XdsCredentialsProvider getProvider(String name) {
     return effectiveProviders.get(checkNotNull(name, "name"));
   }
 
+  /**
+   * Returns list of registered xds credential names. Each provider declares its name via
+   * {@link XdsCredentialsProvider#getName}.
+   */
+  public synchronized ImmutableSet<String> getSupportedCredentialNames() {
+    return effectiveProviders.keySet();
+  }
+
   @VisibleForTesting
   static List<Class<?>> getHardCodedClasses() {
     // Class.forName(String) is used to remove the need for ProGuard configuration. Note that

xds/src/main/java/io/grpc/xds/client/Bootstrapper.java

@@ -57,7 +57,7 @@ public BootstrapInfo bootstrap(Map<String, ?> rawData) throws XdsInitializationE
   public abstract static class ServerInfo {
     public abstract String target();
 
-    public abstract Object implSpecificConfig();
+    public abstract Map<String, ?> implSpecificConfig();
 
     public abstract boolean ignoreResourceDeletion();
 
@@ -66,14 +66,14 @@ public abstract static class ServerInfo {
     public abstract boolean resourceTimerIsTransientError();
 
     @VisibleForTesting
-    public static ServerInfo create(String target, @Nullable Object implSpecificConfig) {
+    public static ServerInfo create(String target, @Nullable Map<String, ?> implSpecificConfig) {
       return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig,
           false, false, false);
     }
 
     @VisibleForTesting
     public static ServerInfo create(
-        String target, Object implSpecificConfig, boolean ignoreResourceDeletion,
+        String target, Map<String, ?> implSpecificConfig, boolean ignoreResourceDeletion,
         boolean isTrustedXdsServer, boolean resourceTimerIsTransientError) {
       return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig,
           ignoreResourceDeletion, isTrustedXdsServer, resourceTimerIsTransientError);

xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java

@@ -76,8 +76,8 @@ protected BootstrapperImpl() {
 
   protected abstract String getJsonContent() throws IOException, XdsInitializationException;
 
-  protected abstract Object getImplSpecificConfig(Map<String, ?> serverConfig, String serverUri)
-      throws XdsInitializationException;
+  protected abstract Map<String, ?> getImplSpecificConfig(
+      Map<String, ?> serverConfig, String serverUri) throws XdsInitializationException;
 
 
   /**
@@ -253,7 +253,7 @@ private List<ServerInfo> parseServerInfos(List<?> rawServerConfigs, XdsLogger lo
       }
       logger.log(XdsLogLevel.INFO, "xDS server URI: {0}", serverUri);
 
-      Object implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri);
+      Map<String, ?> implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri);
 
       boolean resourceTimerIsTransientError = false;
       boolean ignoreResourceDeletion = false;

xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java

@@ -25,7 +25,7 @@
  */
 @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10823")
 public interface XdsTransportFactory {
-  XdsTransport create(Bootstrapper.ServerInfo serverInfo);
+  XdsTransport create(Bootstrapper.ServerInfo serverInfo) throws XdsInitializationException;
 
   /**
    * Represents transport for xDS communication (e.g., a gRPC channel).

xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java

@@ -34,7 +34,6 @@
 import io.grpc.ConnectivityState;
 import io.grpc.ConnectivityStateInfo;
 import io.grpc.EquivalentAddressGroup;
-import io.grpc.InsecureChannelCredentials;
 import io.grpc.LoadBalancer;
 import io.grpc.LoadBalancer.CreateSubchannelArgs;
 import io.grpc.LoadBalancer.FixedResultPicker;
@@ -116,7 +115,7 @@ public class ClusterImplLoadBalancerTest {
   private static final String CLUSTER = "cluster-foo.googleapis.com";
   private static final String EDS_SERVICE_NAME = "service.googleapis.com";
   private static final ServerInfo LRS_SERVER_INFO =
-      ServerInfo.create("api.google.com", InsecureChannelCredentials.create());
+      ServerInfo.create("api.google.com", ImmutableMap.of("type", "insecure"));
   private static final Metadata.Key<OrcaLoadReport> ORCA_ENDPOINT_LOAD_METRICS_KEY =
       Metadata.Key.of(
           "endpoint-load-metrics-bin",

xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java

@@ -38,7 +38,6 @@
 import io.grpc.ConnectivityState;
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.HttpConnectProxiedSocketAddress;
-import io.grpc.InsecureChannelCredentials;
 import io.grpc.LoadBalancer;
 import io.grpc.LoadBalancer.Helper;
 import io.grpc.LoadBalancer.PickResult;
@@ -126,7 +125,7 @@ public class ClusterResolverLoadBalancerTest {
   private static final String EDS_SERVICE_NAME2 = "backend-service-bar.googleapis.com";
   private static final String DNS_HOST_NAME = "dns-service.googleapis.com";
   private static final ServerInfo LRS_SERVER_INFO =
-      ServerInfo.create("lrs.googleapis.com", InsecureChannelCredentials.create());
+      ServerInfo.create("lrs.googleapis.com", ImmutableMap.of("type", "insecure"));
   private final Locality locality1 =
       Locality.create("test-region-1", "test-zone-1", "test-subzone-1");
   private final Locality locality2 =

xds/src/test/java/io/grpc/xds/CsdsServiceTest.java

@@ -38,7 +38,6 @@
 import io.envoyproxy.envoy.service.status.v3.ClientStatusResponse;
 import io.envoyproxy.envoy.type.matcher.v3.NodeMatcher;
 import io.grpc.Deadline;
-import io.grpc.InsecureChannelCredentials;
 import io.grpc.MetricRecorder;
 import io.grpc.Status;
 import io.grpc.Status.Code;
@@ -79,7 +78,7 @@ public class CsdsServiceTest {
       EnvoyProtoData.Node.newBuilder().setId(NODE_ID).build();
   private static final BootstrapInfo BOOTSTRAP_INFO = BootstrapInfo.builder()
       .servers(ImmutableList.of(
-          ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create())))
+          ServerInfo.create(SERVER_URI, ImmutableMap.of("type", "insecure"))))
       .node(BOOTSTRAP_NODE)
       .build();
   private static final FakeXdsClient XDS_CLIENT_NO_RESOURCES = new FakeXdsClient();

xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java

@@ -24,8 +24,6 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
-import io.grpc.InsecureChannelCredentials;
-import io.grpc.TlsChannelCredentials;
 import io.grpc.internal.GrpcUtil;
 import io.grpc.internal.GrpcUtil.GrpcBuildVersion;
 import io.grpc.xds.client.Bootstrapper;
@@ -115,7 +113,7 @@ public void parseBootstrap_singleXdsServer() throws XdsInitializationException {
     assertThat(info.servers()).hasSize(1);
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
     assertThat(info.node()).isEqualTo(
         getNodeBuilder()
             .setId("ENVOY_NODE_ID")
@@ -169,11 +167,11 @@ public void parseBootstrap_multipleXdsServers() throws XdsInitializationExceptio
     assertThat(serverInfoList.get(0).target())
         .isEqualTo("trafficdirector-foo.googleapis.com:443");
     assertThat(serverInfoList.get(0).implSpecificConfig())
-        .isInstanceOf(TlsChannelCredentials.class);
+        .isEqualTo(ImmutableMap.of("type", "tls"));
     assertThat(serverInfoList.get(1).target())
         .isEqualTo("trafficdirector-bar.googleapis.com:443");
     assertThat(serverInfoList.get(1).implSpecificConfig())
-        .isInstanceOf(InsecureChannelCredentials.class);
+        .isEqualTo(ImmutableMap.of("type", "insecure"));
     assertThat(info.node()).isEqualTo(
         getNodeBuilder()
             .setId("ENVOY_NODE_ID")
@@ -217,7 +215,7 @@ public void parseBootstrap_IgnoreIrrelevantFields() throws XdsInitializationExce
     assertThat(info.servers()).hasSize(1);
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));;
     assertThat(info.node()).isEqualTo(
         getNodeBuilder()
             .setId("ENVOY_NODE_ID")
@@ -288,7 +286,7 @@ public void parseBootstrap_useFirstSupportedChannelCredentials()
     assertThat(info.servers()).hasSize(1);
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));;
     assertThat(info.node()).isEqualTo(getNodeBuilder().build());
   }
 
@@ -583,7 +581,7 @@ public void useV2ProtocolByDefault() throws XdsInitializationException {
     BootstrapInfo info = bootstrapper.bootstrap();
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
     assertThat(serverInfo.ignoreResourceDeletion()).isFalse();
   }
 
@@ -605,7 +603,7 @@ public void useV3ProtocolIfV3FeaturePresent() throws XdsInitializationException
     BootstrapInfo info = bootstrapper.bootstrap();
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
     assertThat(serverInfo.ignoreResourceDeletion()).isFalse();
   }
 
@@ -627,7 +625,7 @@ public void serverFeatureIgnoreResourceDeletion() throws XdsInitializationExcept
     BootstrapInfo info = bootstrapper.bootstrap();
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
     // Only ignore_resource_deletion feature enabled: confirm it's on, and xds_v3 is off.
     assertThat(serverInfo.ignoreResourceDeletion()).isTrue();
   }
@@ -650,7 +648,7 @@ public void serverFeatureTrustedXdsServer() throws XdsInitializationException {
     BootstrapInfo info = bootstrapper.bootstrap();
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
     assertThat(serverInfo.isTrustedXdsServer()).isTrue();
   }
 
@@ -672,7 +670,7 @@ public void serverFeatureIgnoreResourceDeletion_xdsV3() throws XdsInitialization
     BootstrapInfo info = bootstrapper.bootstrap();
     ServerInfo serverInfo = Iterables.getOnlyElement(info.servers());
     assertThat(serverInfo.target()).isEqualTo(SERVER_URI);
-    assertThat(serverInfo.implSpecificConfig()).isInstanceOf(InsecureChannelCredentials.class);
+    assertThat(serverInfo.implSpecificConfig()).isEqualTo(ImmutableMap.of("type", "insecure"));
     // ignore_resource_deletion features enabled: confirm both are on.
     assertThat(serverInfo.ignoreResourceDeletion()).isTrue();
   }