alts: make both GoogleDefaultChannelCredentials and ComputeEngineChan…

…nelCredentials choose ALTS for backends given by xDS TD (#7999) GoogleDefaultChannelCredentials and ComputeEngineChannelCredentials are literally the same thing for DirectPath, both of them should behave the same for choosing the protocol negotiator for talking to backends given by Traffic Director.
grpc · Mar 20, 2021 · b2e4757 · b2e4757
1 parent ccd43b6
commit b2e4757
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 33 deletions.
diff --git a/alts/src/main/java/io/grpc/alts/ComputeEngineChannelCredentials.java b/alts/src/main/java/io/grpc/alts/ComputeEngineChannelCredentials.java
@@ -69,7 +69,6 @@ private static InternalProtocolNegotiator.ClientFactory createClientFactory() {
     return new GoogleDefaultProtocolNegotiatorFactory(
         /* targetServiceAccounts= */ ImmutableList.<String>of(),
         SharedResourcePool.forResource(HandshakerServiceChannel.SHARED_HANDSHAKER_CHANNEL),
-        sslContext,
-        null);
+        sslContext);
   }
 }
diff --git a/alts/src/main/java/io/grpc/alts/GoogleDefaultChannelCredentials.java b/alts/src/main/java/io/grpc/alts/GoogleDefaultChannelCredentials.java
@@ -18,7 +18,6 @@
 
 import com.google.auth.oauth2.GoogleCredentials;
 import com.google.common.collect.ImmutableList;
-import io.grpc.Attributes;
 import io.grpc.CallCredentials;
 import io.grpc.ChannelCredentials;
 import io.grpc.CompositeChannelCredentials;
@@ -32,8 +31,6 @@
 import io.grpc.netty.InternalProtocolNegotiator;
 import io.netty.handler.ssl.SslContext;
 import java.io.IOException;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import javax.net.ssl.SSLException;
 
 /**
@@ -42,8 +39,6 @@
  */
 @ExperimentalApi("https://github.com/grpc/grpc-java/issues/7479")
 public final class GoogleDefaultChannelCredentials {
-  private static Logger logger = Logger.getLogger(GoogleDefaultChannelCredentials.class.getName());
-
   private GoogleDefaultChannelCredentials() {}
 
   /**
@@ -66,33 +61,16 @@ public static ChannelCredentials create() {
     return CompositeChannelCredentials.create(nettyCredentials, callCredentials);
   }
 
-  @SuppressWarnings("unchecked")
   private static InternalProtocolNegotiator.ClientFactory createClientFactory() {
     SslContext sslContext;
     try {
       sslContext = GrpcSslContexts.forClient().build();
     } catch (SSLException e) {
       throw new RuntimeException(e);
     }
-    Attributes.Key<String> clusterNameAttrKey = null;
-    try {
-      Class<?> klass = Class.forName("io.grpc.xds.InternalXdsAttributes");
-      clusterNameAttrKey =
-          (Attributes.Key<String>) klass.getField("ATTR_CLUSTER_NAME").get(null);
-    } catch (ClassNotFoundException e) {
-      logger.log(Level.FINE,
-          "Unable to load xDS endpoint cluster name key, this may be expected", e);
-    } catch (NoSuchFieldException e) {
-      logger.log(Level.FINE,
-          "Unable to load xDS endpoint cluster name key, this may be expected", e);
-    } catch (IllegalAccessException e) {
-      logger.log(Level.FINE,
-          "Unable to load xDS endpoint cluster name key, this may be expected", e);
-    }
     return new GoogleDefaultProtocolNegotiatorFactory(
         /* targetServiceAccounts= */ ImmutableList.<String>of(),
         SharedResourcePool.forResource(HandshakerServiceChannel.SHARED_HANDSHAKER_CHANNEL),
-        sslContext,
-        clusterNameAttrKey);
+        sslContext);
   }
 }
diff --git a/alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java b/alts/src/main/java/io/grpc/alts/internal/AltsProtocolNegotiator.java
@@ -41,6 +41,7 @@
 import io.netty.util.AsciiString;
 import java.security.GeneralSecurityException;
 import java.util.List;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
@@ -194,11 +195,12 @@ public void close() {
    */
   public static final class GoogleDefaultProtocolNegotiatorFactory
       implements InternalProtocolNegotiator.ClientFactory {
+    @VisibleForTesting
+    @Nullable
+    static Attributes.Key<String> clusterNameAttrKey = loadClusterNameAttrKey();
     private final ImmutableList<String> targetServiceAccounts;
     private final ObjectPool<Channel> handshakerChannelPool;
     private final SslContext sslContext;
-    @Nullable
-    private final Attributes.Key<String> clusterNameAttrKey;
 
     /**
      * Creates Negotiator Factory, which will either use the targetServiceAccounts and
@@ -207,12 +209,10 @@ public static final class GoogleDefaultProtocolNegotiatorFactory
     public GoogleDefaultProtocolNegotiatorFactory(
         List<String> targetServiceAccounts,
         ObjectPool<Channel> handshakerChannelPool,
-        SslContext sslContext,
-        @Nullable Attributes.Key<String> clusterNameAttrKey) {
+        SslContext sslContext) {
       this.targetServiceAccounts = ImmutableList.copyOf(targetServiceAccounts);
       this.handshakerChannelPool = checkNotNull(handshakerChannelPool, "handshakerChannelPool");
       this.sslContext = checkNotNull(sslContext, "sslContext");
-      this.clusterNameAttrKey = clusterNameAttrKey;
     }
 
     @Override
@@ -228,6 +228,26 @@ public ProtocolNegotiator newNegotiator() {
     public int getDefaultPort() {
       return 443;
     }
+
+    @SuppressWarnings("unchecked")
+    @Nullable
+    private static Attributes.Key<String> loadClusterNameAttrKey() {
+      Attributes.Key<String> key = null;
+      try {
+        Class<?> klass = Class.forName("io.grpc.xds.InternalXdsAttributes");
+        key = (Attributes.Key<String>) klass.getField("ATTR_CLUSTER_NAME").get(null);
+      } catch (ClassNotFoundException e) {
+        logger.log(Level.FINE,
+            "Unable to load xDS endpoint cluster name key, this may be expected", e);
+      } catch (NoSuchFieldException e) {
+        logger.log(Level.FINE,
+            "Unable to load xDS endpoint cluster name key, this may be expected", e);
+      } catch (IllegalAccessException e) {
+        logger.log(Level.FINE,
+            "Unable to load xDS endpoint cluster name key, this may be expected", e);
+      }
+      return key;
+    }
   }
 
   private static final class GoogleDefaultProtocolNegotiator implements ProtocolNegotiator {

diff --git a/alts/src/test/java/io/grpc/alts/internal/GoogleDefaultProtocolNegotiatorTest.java b/alts/src/test/java/io/grpc/alts/internal/GoogleDefaultProtocolNegotiatorTest.java
@@ -56,6 +56,7 @@ public final class GoogleDefaultProtocolNegotiatorTest {
   @RunWith(JUnit4.class)
   public abstract static class HandlerSelectionTest {
     private ProtocolNegotiator googleProtocolNegotiator;
+    private Attributes.Key<String> originalClusterNameAttrKey;
     private final ObjectPool<Channel> handshakerChannelPool = new ObjectPool<Channel>() {
 
       @Override
@@ -73,18 +74,22 @@ public Channel returnObject(Object object) {
     @Before
     public void setUp() throws Exception {
       SslContext sslContext = GrpcSslContexts.forClient().build();
-
+      originalClusterNameAttrKey =
+          AltsProtocolNegotiator.GoogleDefaultProtocolNegotiatorFactory.clusterNameAttrKey;
+      AltsProtocolNegotiator.GoogleDefaultProtocolNegotiatorFactory.clusterNameAttrKey =
+          getClusterNameAttrKey();
       googleProtocolNegotiator = new AltsProtocolNegotiator.GoogleDefaultProtocolNegotiatorFactory(
           ImmutableList.<String>of(),
           handshakerChannelPool,
-          sslContext,
-          getClusterNameAttrKey())
+          sslContext)
           .newNegotiator();
     }
 
     @After
     public void tearDown() {
       googleProtocolNegotiator.close();
+      AltsProtocolNegotiator.GoogleDefaultProtocolNegotiatorFactory.clusterNameAttrKey =
+          originalClusterNameAttrKey;
     }
 
     @Nullable