netty: Support SocketAddress with ChannelCredentials (grpc#8194)

cfredri4 · web-flow · commit c8cd4cb26008 · 2021-05-24T09:49:20.000-07:00
This adds support for creating a Netty Channel with SocketAddress and ChannelCredentials.

This aligns with NettyServerBuilder.forAddress(SocketAddress address, ServerCredentials creds).
diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java
@@ -283,9 +283,25 @@ static String makeTargetStringForDirectAddress(SocketAddress address) {
   public ManagedChannelImplBuilder(SocketAddress directServerAddress, String authority,
       ClientTransportFactoryBuilder clientTransportFactoryBuilder,
       @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) {
+      this(directServerAddress, authority, null, null, clientTransportFactoryBuilder,
+          channelBuilderDefaultPortProvider);
+  }
+
+  /**
+   * Creates a new managed channel builder with the given server address, authority string of the
+   * channel. Transport implementors must provide client transport factory builder, and may set
+   * custom channel default port provider.
+   * 
+   * @param channelCreds The ChannelCredentials provided by the user. These may be used when
+   *     creating derivative channels.
+   */
+  public ManagedChannelImplBuilder(SocketAddress directServerAddress, String authority,
+      @Nullable ChannelCredentials channelCreds, @Nullable CallCredentials callCreds,
+      ClientTransportFactoryBuilder clientTransportFactoryBuilder,
+      @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) {
     this.target = makeTargetStringForDirectAddress(directServerAddress);
-    this.channelCredentials = null;
-    this.callCredentials = null;
+    this.channelCredentials = channelCreds;
+    this.callCredentials = callCreds;
     this.clientTransportFactoryBuilder = Preconditions
         .checkNotNull(clientTransportFactoryBuilder, "clientTransportFactoryBuilder");
     this.directServerAddress = directServerAddress;
diff --git a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java
@@ -127,6 +127,23 @@ public static NettyChannelBuilder forAddress(SocketAddress serverAddress) {
     return new NettyChannelBuilder(serverAddress);
   }
 
+  /**
+   * Creates a new builder with the given server address. This factory method is primarily intended
+   * for using Netty Channel types other than SocketChannel.
+   * {@link #forAddress(String, int, ChannelCredentials)} should generally be preferred over this
+   * method, since that API permits delaying DNS lookups and noticing changes to DNS. If an
+   * unresolved InetSocketAddress is passed in, then it will remain unresolved.
+   */
+  @CheckReturnValue
+  public static NettyChannelBuilder forAddress(SocketAddress serverAddress,
+      ChannelCredentials creds) {
+    FromChannelCredentialsResult result = ProtocolNegotiators.from(creds);
+    if (result.error != null) {
+      throw new IllegalArgumentException(result.error);
+    }
+    return new NettyChannelBuilder(serverAddress, creds, result.callCredentials, result.negotiator);
+  }
+
   /**
    * Creates a new builder with the given host and port.
    */
@@ -207,6 +224,18 @@ public int getDefaultPort() {
     this.freezeProtocolNegotiatorFactory = false;
   }
 
+  NettyChannelBuilder(
+      SocketAddress address, ChannelCredentials channelCreds, CallCredentials callCreds,
+      ProtocolNegotiator.ClientFactory negotiator) {
+    managedChannelImplBuilder = new ManagedChannelImplBuilder(address,
+        getAuthorityFromAddress(address),
+        channelCreds, callCreds,
+        new NettyChannelTransportFactoryBuilder(),
+        new NettyChannelDefaultPortProvider());
+    this.protocolNegotiatorFactory = checkNotNull(negotiator, "negotiator");
+    this.freezeProtocolNegotiatorFactory = true;
+  }
+
   @Internal
   @Override
   protected ManagedChannelBuilder<?> delegate() {
diff --git a/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java b/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java
@@ -23,6 +23,7 @@
 import static org.mockito.Mockito.mock;
 
 import io.grpc.ChannelCredentials;
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.ManagedChannel;
 import io.grpc.internal.ClientTransportFactory;
 import io.grpc.internal.ClientTransportFactory.SwapChannelCredentialsResult;
@@ -153,6 +154,28 @@ public void failIfSslContextIsNotClient() {
     builder.sslContext(sslContext);
   }
 
+  @Test
+  public void failNegotiationTypeWithChannelCredentials_target() {
+    NettyChannelBuilder builder = NettyChannelBuilder.forTarget(
+        "fakeTarget", InsecureChannelCredentials.create());
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Cannot change security when using ChannelCredentials");
+
+    builder.negotiationType(NegotiationType.TLS);
+  }
+
+  @Test
+  public void failNegotiationTypeWithChannelCredentials_socketAddress() {
+    NettyChannelBuilder builder = NettyChannelBuilder.forAddress(
+        new SocketAddress(){}, InsecureChannelCredentials.create());
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Cannot change security when using ChannelCredentials");
+
+    builder.negotiationType(NegotiationType.TLS);
+  }
+
   @Test
   public void createProtocolNegotiatorByType_plaintext() {
     ProtocolNegotiator negotiator = NettyChannelBuilder.createProtocolNegotiatorByType(
