Use InsecureChannelCredentials.create instead of Optional.

Author: rmehta19

s2a/src/main/java/io/grpc/s2a/S2AChannelCredentials.java

@@ -25,14 +25,14 @@
 import io.grpc.Channel;
 import io.grpc.ChannelCredentials;
 import io.grpc.ExperimentalApi;
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.internal.ObjectPool;
 import io.grpc.internal.SharedResourcePool;
 import io.grpc.netty.InternalNettyChannelCredentials;
 import io.grpc.netty.InternalProtocolNegotiator;
 import io.grpc.s2a.channel.S2AHandshakerServiceChannel;
 import io.grpc.s2a.handshaker.S2AIdentity;
 import io.grpc.s2a.handshaker.S2AProtocolNegotiatorFactory;
-import java.util.Optional;
 import javax.annotation.concurrent.NotThreadSafe;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
@@ -58,13 +58,13 @@ public static Builder newBuilder(String s2aAddress) {
   public static final class Builder {
     private final String s2aAddress;
     private ObjectPool<Channel> s2aChannelPool;
-    private Optional<ChannelCredentials> s2aChannelCredentials;
+    private ChannelCredentials s2aChannelCredentials;
     private @Nullable S2AIdentity localIdentity = null;
 
     Builder(String s2aAddress) {
       this.s2aAddress = s2aAddress;
       this.s2aChannelPool = null;
-      this.s2aChannelCredentials = Optional.empty();
+      this.s2aChannelCredentials = InsecureChannelCredentials.create();
     }
 
     /**
@@ -109,7 +109,7 @@ public Builder setLocalUid(String localUid) {
     /** Sets the credentials to be used when connecting to the S2A. */
     @CanIgnoreReturnValue
     public Builder setS2AChannelCredentials(ChannelCredentials s2aChannelCredentials) {
-      this.s2aChannelCredentials = Optional.of(s2aChannelCredentials);
+      this.s2aChannelCredentials = s2aChannelCredentials;
       return this;
     }
 

s2a/src/main/java/io/grpc/s2a/channel/S2AHandshakerServiceChannel.java

@@ -34,7 +34,6 @@
 import io.netty.channel.socket.nio.NioSocketChannel;
 import io.netty.util.concurrent.DefaultThreadFactory;
 import java.time.Duration;
-import java.util.Optional;
 import java.util.concurrent.ConcurrentMap;
 import javax.annotation.concurrent.ThreadSafe;
 
@@ -74,8 +73,9 @@ public final class S2AHandshakerServiceChannel {
    *     running at {@code s2aAddress}.
    */
   public static Resource<Channel> getChannelResource(
-      String s2aAddress, Optional<ChannelCredentials> s2aChannelCredentials) {
+      String s2aAddress, ChannelCredentials s2aChannelCredentials) {
     checkNotNull(s2aAddress);
+    checkNotNull(s2aChannelCredentials);
     return SHARED_RESOURCE_CHANNELS.computeIfAbsent(
         s2aAddress, channelResource -> new ChannelResource(s2aAddress, s2aChannelCredentials));
   }
@@ -87,9 +87,9 @@ public static Resource<Channel> getChannelResource(
    */
   private static class ChannelResource implements Resource<Channel> {
     private final String targetAddress;
-    private final Optional<ChannelCredentials> channelCredentials;
+    private final ChannelCredentials channelCredentials;
 
-    public ChannelResource(String targetAddress, Optional<ChannelCredentials> channelCredentials) {
+    public ChannelResource(String targetAddress, ChannelCredentials channelCredentials) {
       this.targetAddress = targetAddress;
       this.channelCredentials = channelCredentials;
     }
@@ -103,25 +103,12 @@ public ChannelResource(String targetAddress, Optional<ChannelCredentials> channe
     public Channel create() {
       EventLoopGroup eventLoopGroup =
           new NioEventLoopGroup(1, new DefaultThreadFactory("S2A channel pool", true));
-      ManagedChannel channel = null;
-      if (channelCredentials.isPresent()) {
-        // Create a secure channel.
-        channel =
-            NettyChannelBuilder.forTarget(targetAddress, channelCredentials.get())
-                .channelType(NioSocketChannel.class)
-                .directExecutor()
-                .eventLoopGroup(eventLoopGroup)
-                .build();
-      } else {
-        // Create a plaintext channel.
-        channel =
-            NettyChannelBuilder.forTarget(targetAddress)
-                .channelType(NioSocketChannel.class)
-                .directExecutor()
-                .eventLoopGroup(eventLoopGroup)
-                .usePlaintext()
-                .build();
-      }
+      ManagedChannel channel =
+          NettyChannelBuilder.forTarget(targetAddress, channelCredentials)
+              .channelType(NioSocketChannel.class)
+              .directExecutor()
+              .eventLoopGroup(eventLoopGroup)
+              .build();
       return EventLoopHoldingChannel.create(channel, eventLoopGroup);
     }
 

s2a/src/test/java/io/grpc/s2a/channel/S2AHandshakerServiceChannelTest.java

@@ -28,6 +28,7 @@
 import io.grpc.Channel;
 import io.grpc.ChannelCredentials;
 import io.grpc.ClientCall;
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.ManagedChannel;
 import io.grpc.MethodDescriptor;
 import io.grpc.Server;
@@ -48,7 +49,6 @@
 import io.netty.channel.EventLoopGroup;
 import java.io.File;
 import java.time.Duration;
-import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import org.junit.Before;
 import org.junit.ClassRule;
@@ -82,7 +82,7 @@ public void getChannelResource_success() {
     Resource<Channel> resource =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     assertThat(resource.toString()).isEqualTo("grpc-s2a-channel");
   }
 
@@ -104,11 +104,11 @@ public void getChannelResource_twoEqualChannels() {
     Resource<Channel> resource =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     Resource<Channel> resourceTwo =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     assertThat(resource).isEqualTo(resourceTwo);
   }
 
@@ -133,10 +133,10 @@ public void getChannelResource_twoDistinctChannels() {
     Resource<Channel> resource =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     Resource<Channel> resourceTwo =
         S2AHandshakerServiceChannel.getChannelResource(
-            "localhost:" + Utils.pickUnusedPort(), /* s2aChannelCredentials= */ Optional.empty());
+            "localhost:" + Utils.pickUnusedPort(), InsecureChannelCredentials.create());
     assertThat(resourceTwo).isNotEqualTo(resource);
   }
 
@@ -161,7 +161,7 @@ public void close_success() {
     Resource<Channel> resource =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     Channel channel = resource.create();
     resource.close(channel);
     StatusRuntimeException expected =
@@ -199,7 +199,7 @@ public void newCall_performSimpleRpcSuccess() {
     Resource<Channel> resource =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     Channel channel = resource.create();
     assertThat(channel).isInstanceOf(EventLoopHoldingChannel.class);
     assertThat(
@@ -268,7 +268,7 @@ public void create_succeedsAfterCloseIsCalledOnce() throws Exception {
     Resource<Channel> resource =
         S2AHandshakerServiceChannel.getChannelResource(
             "localhost:" + plaintextServer.getPort(),
-            /* s2aChannelCredentials= */ Optional.empty());
+            InsecureChannelCredentials.create());
     Channel channelOne = resource.create();
     resource.close(channelOne);
 
@@ -320,15 +320,14 @@ private static Server createPlaintextServer() {
         ServerBuilder.forPort(Utils.pickUnusedPort()).addService(service).build());
   }
 
-  private static Optional<ChannelCredentials> getTlsChannelCredentials() throws Exception {
+  private static ChannelCredentials getTlsChannelCredentials() throws Exception {
     File clientCert = new File("src/test/resources/client_cert.pem");
     File clientKey = new File("src/test/resources/client_key.pem");
     File rootCert = new File("src/test/resources/root_cert.pem");
-    return Optional.of(
-        TlsChannelCredentials.newBuilder()
+    return TlsChannelCredentials.newBuilder()
             .keyManager(clientCert, clientKey)
             .trustManager(rootCert)
-            .build());
+            .build();
   }
 
   private static class SimpleServiceImpl extends SimpleServiceGrpc.SimpleServiceImplBase {

s2a/src/test/java/io/grpc/s2a/handshaker/S2AProtocolNegotiatorFactoryTest.java

@@ -115,7 +115,7 @@ public void createProtocolNegotiator_nullArgument() throws Exception {
         S2AGrpcChannelPool.create(
             SharedResourcePool.forResource(
                 S2AHandshakerServiceChannel.getChannelResource(
-                    "localhost:8080", /* s2aChannelCredentials= */ Optional.empty())));
+                    "localhost:8080", InsecureChannelCredentials.create())));
 
     NullPointerTester tester =
         new NullPointerTester()

s2a/src/test/java/io/grpc/s2a/handshaker/S2AStubTest.java

@@ -21,13 +21,13 @@
 import static org.junit.Assert.assertThrows;
 
 import com.google.common.truth.Expect;
+import io.grpc.InsecureChannelCredentials;
 import io.grpc.internal.SharedResourcePool;
 import io.grpc.s2a.channel.S2AChannelPool;
 import io.grpc.s2a.channel.S2AGrpcChannelPool;
 import io.grpc.s2a.channel.S2AHandshakerServiceChannel;
 import io.grpc.stub.StreamObserver;
 import java.io.IOException;
-import java.util.Optional;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -55,7 +55,7 @@ public void send_receiveOkStatus() throws Exception {
         S2AGrpcChannelPool.create(
             SharedResourcePool.forResource(
                 S2AHandshakerServiceChannel.getChannelResource(
-                    S2A_ADDRESS, /* s2aChannelCredentials= */ Optional.empty())));
+                    S2A_ADDRESS, InsecureChannelCredentials.create())));
     S2AServiceGrpc.S2AServiceStub serviceStub = S2AServiceGrpc.newStub(channelPool.getChannel());
     S2AStub newStub = S2AStub.newInstance(serviceStub);