Refactor xDS bootstrapper to improve role separation between io.grpc.xds/io.grpc (grpc specific) and io.grpc.xds.client (generic) packages.

- Remove call credentials from BootstrapperImpl.ServerInfo.
- Store call credentials in implSpecificConfig (Object) wrapped in a Map.
- Move JwtTokenFileCallCredentials to io.grpc.xds package.
- Remove ChannelCredentials fallback from GrpcXdsTransportFactory.

Author: kannanjgithub

xds/build.gradle

@@ -49,7 +49,6 @@ dependencies {
             project(':grpc-core'),
             project(':grpc-util'),
             project(':grpc-services'),
-            project(':grpc-auth'),
             project(path: ':grpc-alts', configuration: 'shadow'),
             libraries.guava,
             libraries.gson,

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

@@ -21,6 +21,8 @@
 import com.google.errorprone.annotations.concurrent.GuardedBy;
 import io.grpc.CallCredentials;
 import io.grpc.ChannelCredentials;
+import io.grpc.CompositeCallCredentials;
+import io.grpc.internal.GrpcUtil;
 import io.grpc.internal.JsonUtil;
 import io.grpc.xds.client.AllowedGrpcServices;
 import io.grpc.xds.client.AllowedGrpcServices.AllowedGrpcService;
@@ -30,12 +32,17 @@
 import io.grpc.xds.client.XdsInitializationException;
 import io.grpc.xds.client.XdsLogger;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import javax.annotation.Nullable;
 
 class GrpcBootstrapperImpl extends BootstrapperImpl {
+  @VisibleForTesting
+  public static boolean enableXdsBootstrapCallCreds = GrpcUtil.getFlag(
+      "GRPC_EXPERIMENTAL_XDS_BOOTSTRAP_CALL_CREDS", false);
+
   private static final String BOOTSTRAP_PATH_SYS_ENV_VAR = "GRPC_XDS_BOOTSTRAP";
   private static final String BOOTSTRAP_PATH_SYS_PROPERTY = "io.grpc.xds.bootstrap";
   private static final String BOOTSTRAP_CONFIG_SYS_ENV_VAR = "GRPC_XDS_BOOTSTRAP_CONFIG";
@@ -104,7 +111,62 @@ protected String getJsonContent() throws XdsInitializationException, IOException
   protected Object getImplSpecificConfig(Map<String, ?> serverConfig, String serverUri)
       throws XdsInitializationException {
     ConfiguredChannelCredentials configuredChannel = getChannelCredentials(serverConfig, serverUri);
-    return configuredChannel != null ? configuredChannel.channelCredentials() : null;
+    ChannelCredentials channelCredentials = configuredChannel != null
+        ? configuredChannel.channelCredentials() : null;
+
+    CallCredentials callCredentials = null;
+    List<?> rawCallCreds = JsonUtil.getList(serverConfig, "call_creds");
+    if (enableXdsBootstrapCallCreds && rawCallCreds != null) {
+      List<Map<String, ?>> callCredsList = JsonUtil.checkObjectList(rawCallCreds);
+      callCredentials = parseCallCredentials(callCredsList, serverUri);
+    }
+
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
+    if (channelCredentials != null) {
+      builder.put("grpc.channel_credentials", channelCredentials);
+    }
+    if (callCredentials != null) {
+      builder.put("grpc.call_credentials", callCredentials);
+    }
+    return builder.buildOrThrow();
+  }
+
+  @Nullable
+  private CallCredentials parseCallCredentials(List<Map<String, ?>> jsonList, String serverUri)
+      throws XdsInitializationException {
+    List<CallCredentials> parsedCreds = new ArrayList<>();
+    for (Map<String, ?> credJson : jsonList) {
+      String type = JsonUtil.getString(credJson, "type");
+      if (type == null) {
+        throw new XdsInitializationException(
+            "Invalid bootstrap: server " + serverUri + " with 'call_creds' type unspecified");
+      }
+      if ("jwt_token_file".equals(type)) {
+        Map<String, ?> config = JsonUtil.getObject(credJson, "config");
+        if (config == null) {
+          throw new XdsInitializationException(
+              "Invalid bootstrap: server " + serverUri + " with 'jwt_token_file' config missing");
+        }
+        String jwtTokenFile = JsonUtil.getString(config, "jwt_token_file");
+        if (jwtTokenFile == null || jwtTokenFile.isEmpty()) {
+          throw new XdsInitializationException(
+              "Invalid bootstrap: server " + serverUri
+                  + " with 'jwt_token_file' jwt_token_file missing or empty");
+        }
+        parsedCreds.add(new JwtTokenFileCallCredentials(jwtTokenFile));
+      } else {
+        logger.log(XdsLogger.XdsLogLevel.INFO,
+            "Skipping unsupported call credential type: {0}", type);
+      }
+    }
+    if (parsedCreds.isEmpty()) {
+      return null;
+    }
+    CallCredentials combined = parsedCreds.get(0);
+    for (int i = 1; i < parsedCreds.size(); i++) {
+      combined = new CompositeCallCredentials(combined, parsedCreds.get(i));
+    }
+    return combined;
   }
 
   @GuardedBy("GrpcBootstrapperImpl.class")
@@ -194,8 +256,8 @@ protected Optional<Object> parseImplSpecificObject(
       Optional<CallCredentials> callCredentials = Optional.empty();
       List<?> rawCallCredsList = JsonUtil.getList(serviceConfig, "call_creds");
       if (rawCallCredsList != null && !rawCallCredsList.isEmpty()) {
-        callCredentials =
-            parseCallCredentials(JsonUtil.checkObjectList(rawCallCredsList), targetUri);
+        callCredentials = Optional.ofNullable(
+            parseCallCredentials(JsonUtil.checkObjectList(rawCallCredsList), targetUri));
       }
 
       AllowedGrpcService.Builder b = AllowedGrpcService.builder()
@@ -208,16 +270,7 @@ protected Optional<Object> parseImplSpecificObject(
     return Optional.of(customConfig);
   }
 
-  @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();
-  }
+
 
   private static final class JsonChannelCredsConfig implements ChannelCredsConfig {
     private final String type;

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

@@ -34,6 +34,7 @@
 import io.grpc.Status;
 import io.grpc.xds.client.Bootstrapper;
 import io.grpc.xds.client.XdsTransportFactory;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 final class GrpcXdsTransportFactory implements XdsTransportFactory {
@@ -80,19 +81,26 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo,
                             CallCredentials callCredentials,
                             ChannelConfigurator channelConfigurator) {
       String target = serverInfo.target();
-      ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig();
+      Object implConfig = serverInfo.implSpecificConfig();
+      ChannelCredentials channelCredentials = null;
+      CallCredentials serverCallCredentials = null;
+      if (implConfig instanceof Map) {
+        Map<?, ?> configMap = (Map<?, ?>) implConfig;
+        channelCredentials = (ChannelCredentials) configMap.get("grpc.channel_credentials");
+        serverCallCredentials = (CallCredentials) configMap.get("grpc.call_credentials");
+      }
       ManagedChannelBuilder<?> channelBuilder = Grpc.newChannelBuilder(target, channelCredentials)
           .keepAliveTime(5, TimeUnit.MINUTES);
       if (channelConfigurator != null) {
         channelConfigurator.configureChannelBuilder(channelBuilder);
         channelBuilder.childChannelConfigurator(channelConfigurator);
       }
       this.channel = channelBuilder.build();
-      if (callCredentials != null && serverInfo.callCredentials() != null) {
+      if (callCredentials != null && serverCallCredentials != null) {
         this.callCredentials = new CompositeCallCredentials(
-            callCredentials, serverInfo.callCredentials());
-      } else if (serverInfo.callCredentials() != null) {
-        this.callCredentials = serverInfo.callCredentials();
+            callCredentials, serverCallCredentials);
+      } else if (serverCallCredentials != null) {
+        this.callCredentials = serverCallCredentials;
       } else {
         this.callCredentials = callCredentials;
       }

…/client/JwtTokenFileCallCredentials.java …rpc/xds/JwtTokenFileCallCredentials.javaxds/src/main/java/io/grpc/xds/client/JwtTokenFileCallCredentials.java renamed to xds/src/main/java/io/grpc/xds/JwtTokenFileCallCredentials.java xds/src/main/java/io/grpc/xds/client/JwtTokenFileCallCredentials.java renamed to xds/src/main/java/io/grpc/xds/JwtTokenFileCallCredentials.java

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.grpc.xds.client;
+package io.grpc.xds;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 

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

@@ -22,7 +22,6 @@
 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.Internal;
 import io.grpc.xds.client.EnvoyProtoData.Node;
 import java.util.List;
@@ -69,23 +68,20 @@ public abstract static class ServerInfo {
 
     public abstract boolean failOnDataErrors();
 
-    @Nullable public abstract CallCredentials callCredentials();
-
     @VisibleForTesting
     public static ServerInfo create(String target, @Nullable Object implSpecificConfig) {
       return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig,
-          false, false, false, false, null);
+          false, false, false, false);
     }
 
     @VisibleForTesting
     public static ServerInfo create(
         String target, Object implSpecificConfig,
         boolean ignoreResourceDeletion, boolean isTrustedXdsServer,
-        boolean resourceTimerIsTransientError, boolean failOnDataErrors,
-        @Nullable CallCredentials callCredentials) {
+        boolean resourceTimerIsTransientError, boolean failOnDataErrors) {
       return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig,
           ignoreResourceDeletion, isTrustedXdsServer,
-          resourceTimerIsTransientError, failOnDataErrors, callCredentials);
+          resourceTimerIsTransientError, failOnDataErrors);
     }
   }
 

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

@@ -19,8 +19,6 @@
 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.CompositeCallCredentials;
 import io.grpc.Internal;
 import io.grpc.InternalLogId;
 import io.grpc.internal.GrpcUtil;
@@ -33,7 +31,6 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -68,9 +65,6 @@ public abstract class BootstrapperImpl extends Bootstrapper {
   @VisibleForTesting
   static boolean enableXdsFallback = GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_FALLBACK, true);
 
-  @VisibleForTesting
-  public static boolean enableXdsBootstrapCallCreds = GrpcUtil.getFlag(
-      "GRPC_EXPERIMENTAL_XDS_BOOTSTRAP_CALL_CREDS", false);
 
   @VisibleForTesting
   public static boolean xdsDataErrorHandlingEnabled
@@ -290,58 +284,15 @@ private List<ServerInfo> parseServerInfos(List<?> rawServerConfigs, XdsLogger lo
         failOnDataErrors = xdsDataErrorHandlingEnabled
             && serverFeatures.contains(SERVER_FEATURE_FAIL_ON_DATA_ERRORS);
       }
-      CallCredentials callCredentials = null;
-      List<?> rawCallCreds = JsonUtil.getList(serverConfig, "call_creds");
-      if (enableXdsBootstrapCallCreds && rawCallCreds != null) {
-        List<Map<String, ?>> callCredsList = JsonUtil.checkObjectList(rawCallCreds);
-        callCredentials = parseCallCredentials(callCredsList, serverUri);
-      }
       servers.add(
           ServerInfo.create(serverUri, implSpecificConfig, ignoreResourceDeletion,
               serverFeatures != null
                   && serverFeatures.contains(SERVER_FEATURE_TRUSTED_XDS_SERVER),
-              resourceTimerIsTransientError, failOnDataErrors, callCredentials));
+              resourceTimerIsTransientError, failOnDataErrors));
     }
     return servers.build();
   }
 
-  @Nullable
-  private CallCredentials parseCallCredentials(List<Map<String, ?>> jsonList, String serverUri)
-      throws XdsInitializationException {
-    List<CallCredentials> parsedCreds = new ArrayList<>();
-    for (Map<String, ?> credJson : jsonList) {
-      String type = JsonUtil.getString(credJson, "type");
-      if (type == null) {
-        throw new XdsInitializationException(
-            "Invalid bootstrap: server " + serverUri + " with 'call_creds' type unspecified");
-      }
-      if ("jwt_token_file".equals(type)) {
-        Map<String, ?> config = JsonUtil.getObject(credJson, "config");
-        if (config == null) {
-          throw new XdsInitializationException(
-              "Invalid bootstrap: server " + serverUri + " with 'jwt_token_file' config missing");
-        }
-        String jwtTokenFile = JsonUtil.getString(config, "jwt_token_file");
-        if (jwtTokenFile == null || jwtTokenFile.isEmpty()) {
-          throw new XdsInitializationException(
-              "Invalid bootstrap: server " + serverUri
-                  + " with 'jwt_token_file' jwt_token_file missing or empty");
-        }
-        parsedCreds.add(new JwtTokenFileCallCredentials(jwtTokenFile));
-      } else {
-        logger.log(XdsLogLevel.INFO, "Skipping unsupported call credential type: {0}", type);
-      }
-    }
-    if (parsedCreds.isEmpty()) {
-      return null;
-    }
-    CallCredentials combined = parsedCreds.get(0);
-    for (int i = 1; i < parsedCreds.size(); i++) {
-      combined = new CompositeCallCredentials(combined, parsedCreds.get(i));
-    }
-    return combined;
-  }
-
   @VisibleForTesting
   public void setFileReader(FileReader reader) {
     this.reader = reader;

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

@@ -65,7 +65,7 @@ private static BootstrapInfo dummyBootstrapInfo() {
 
   private static ServerInfo dummyServerInfo() {
     return ServerInfo.create(
-        "test_target", Collections.emptyMap(), false, true, false, false, null);
+        "test_target", Collections.emptyMap(), false, true, false, false);
   }
 
   private ExtAuthz.Builder extAuthzBuilder;

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

@@ -246,7 +246,7 @@ public void setUp() throws Exception {
 
     serverInfo =
         Bootstrapper.ServerInfo.create(
-            "test_target", Collections.emptyMap(), false, true, false, false, null);
+            "test_target", Collections.emptyMap(), false, true, false, false);
 
     filterContext = Filter.FilterConfigParseContext.builder()
         .bootstrapInfo(bootstrapInfo)

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

@@ -77,7 +77,7 @@ public void setUp() throws Exception {
 
     serverInfo =
         Bootstrapper.ServerInfo.create(
-            "test_target", Collections.emptyMap(), false, true, false, false, null);
+            "test_target", Collections.emptyMap(), false, true, false, false);
 
     filterContext = Filter.FilterConfigParseContext.builder()
         .bootstrapInfo(bootstrapInfo)

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

@@ -114,7 +114,7 @@ private static Filter.FilterConfigParseContext getFilterContext() {
             .node(Node.newBuilder().build())
             .build())
         .serverInfo(ServerInfo.create(
-            "test_target", Collections.emptyMap(), false, true, false, false, null))
+            "test_target", Collections.emptyMap(), false, true, false, false))
         .build();
   }
 }