From b836def6123ec2f5d8993b3d0f72fd71371a0389 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Fri, 1 Dec 2023 18:14:58 +0000 Subject: [PATCH 1/5] fix: DirectPath non-default SA requires creds --- .../google/api/gax/grpc/InstantiatingGrpcChannelProvider.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 4e7a654f7e..d98195baad 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -300,6 +300,10 @@ private void logDirectPathMisconfig() { } private boolean isNonDefaultServiceAccountAllowed() { + // DirectPath non-default SA requires a credential. + if (needsCredentials()) { + return false; + } if (allowNonDefaultServiceAccount != null && allowNonDefaultServiceAccount) { return true; } From 26cb5847ddc01e8b2da25272296c2b75e0d3e03c Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Mon, 4 Dec 2023 18:09:52 +0000 Subject: [PATCH 2/5] fix: DirectPath non-default SA requires creds --- .../api/gax/grpc/InstantiatingGrpcChannelProvider.java | 8 ++++---- .../gax/grpc/InstantiatingGrpcChannelProviderTest.java | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index d98195baad..dc5e3a66fa 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -282,7 +282,7 @@ private void logDirectPathMisconfig() { + " attemptDirectPathXds option."); } else { // Case 2: credential is not correctly set - if (!isNonDefaultServiceAccountAllowed()) { + if (!isCredentialDirectPathCompatible()) { LOG.log( Level.WARNING, "DirectPath is misconfigured. Please make sure the credential is an instance of " @@ -299,8 +299,8 @@ private void logDirectPathMisconfig() { } } - private boolean isNonDefaultServiceAccountAllowed() { - // DirectPath non-default SA requires a credential. + private boolean isCredentialDirectPathCompatible() { + // DirectPath requires a credential. if (needsCredentials()) { return false; } @@ -360,7 +360,7 @@ private ManagedChannel createSingleChannel() throws IOException { // Check DirectPath traffic. boolean useDirectPathXds = false; - if (isDirectPathEnabled() && isNonDefaultServiceAccountAllowed() && isOnComputeEngine()) { + if (isDirectPathEnabled() && isCredentialDirectPathCompatible() && isOnComputeEngine()) { CallCredentials callCreds = MoreCallCredentials.from(credentials); ChannelCredentials channelCreds = GoogleDefaultChannelCredentials.newBuilder().callCredentials(callCreds).build(); diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index edd7a73768..ac60ea8b3c 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -284,6 +284,14 @@ public void testDirectPathXdsDisableByDefault() throws IOException { assertThat(provider.isDirectPathXdsEnabled()).isFalse(); } + @Test + public void testDirectPathDisallowNullCredentials() throws IOException { + InstantiatingGrpcChannelProvider provider = + InstantiatingGrpcChannelProvider.newBuilder().build(); + + assertThat(provider.isCredentialDirectPathCompatible()).isFalse(); + } + @Test public void testDirectPathXdsEnabled() throws IOException { InstantiatingGrpcChannelProvider provider = From 15db7e82f2437581590ff4014d454625a22634f7 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Mon, 4 Dec 2023 18:29:33 +0000 Subject: [PATCH 3/5] fix: DirectPath non-default SA requires creds --- .../google/api/gax/grpc/InstantiatingGrpcChannelProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index dc5e3a66fa..f6cd0a065f 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -299,7 +299,8 @@ private void logDirectPathMisconfig() { } } - private boolean isCredentialDirectPathCompatible() { + @VisibleForTesting + boolean isCredentialDirectPathCompatible() { // DirectPath requires a credential. if (needsCredentials()) { return false; From 33d5b823ff4b29afaca19fe419de8543ce41c3fb Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Wed, 13 Dec 2023 18:41:12 +0000 Subject: [PATCH 4/5] fix: DirectPath non-default SA requires creds --- .../grpc/InstantiatingGrpcChannelProvider.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 9afc9243bd..4ccc8d832c 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -301,11 +301,8 @@ private void logDirectPathMisconfig() { @VisibleForTesting boolean isCredentialDirectPathCompatible() { - // DirectPath requires a credential. - if (needsCredentials()) { - return false; - } if (allowNonDefaultServiceAccount != null && allowNonDefaultServiceAccount) { + // If non-default service account is allowed, a null call credential is OK. return true; } return credentials instanceof ComputeEngineCredentials; @@ -362,9 +359,15 @@ private ManagedChannel createSingleChannel() throws IOException { // Check DirectPath traffic. boolean useDirectPathXds = false; if (isDirectPathEnabled() && isCredentialDirectPathCompatible() && isOnComputeEngine()) { - CallCredentials callCreds = MoreCallCredentials.from(credentials); - ChannelCredentials channelCreds = - GoogleDefaultChannelCredentials.newBuilder().callCredentials(callCreds).build(); + ChannelCredentials channelCreds; + if (credentials == null) { + // GoogleDefaultChannelCredentials will use application default call credential. + channelCreds = GoogleDefaultChannelCredentials.newBuilder().build(); + } else { + CallCredentials callCreds = MoreCallCredentials.from(credentials); + channelCreds = + GoogleDefaultChannelCredentials.newBuilder().callCredentials(callCreds).build(); + } useDirectPathXds = isDirectPathXdsEnabled(); if (useDirectPathXds) { // google-c2p: CloudToProd(C2P) Directpath. This scheme is defined in From 5c0418b9fd943c963918e5366d4870274d0f2b49 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Thu, 14 Dec 2023 19:24:28 +0000 Subject: [PATCH 5/5] fix: DirectPath non-default SA requires creds --- .../grpc/InstantiatingGrpcChannelProvider.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 4ccc8d832c..35a068426c 100644 --- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -301,8 +301,11 @@ private void logDirectPathMisconfig() { @VisibleForTesting boolean isCredentialDirectPathCompatible() { + // DirectPath requires a call credential during gRPC channel construction. + if (needsCredentials()) { + return false; + } if (allowNonDefaultServiceAccount != null && allowNonDefaultServiceAccount) { - // If non-default service account is allowed, a null call credential is OK. return true; } return credentials instanceof ComputeEngineCredentials; @@ -359,15 +362,9 @@ private ManagedChannel createSingleChannel() throws IOException { // Check DirectPath traffic. boolean useDirectPathXds = false; if (isDirectPathEnabled() && isCredentialDirectPathCompatible() && isOnComputeEngine()) { - ChannelCredentials channelCreds; - if (credentials == null) { - // GoogleDefaultChannelCredentials will use application default call credential. - channelCreds = GoogleDefaultChannelCredentials.newBuilder().build(); - } else { - CallCredentials callCreds = MoreCallCredentials.from(credentials); - channelCreds = - GoogleDefaultChannelCredentials.newBuilder().callCredentials(callCreds).build(); - } + CallCredentials callCreds = MoreCallCredentials.from(credentials); + ChannelCredentials channelCreds = + GoogleDefaultChannelCredentials.newBuilder().callCredentials(callCreds).build(); useDirectPathXds = isDirectPathXdsEnabled(); if (useDirectPathXds) { // google-c2p: CloudToProd(C2P) Directpath. This scheme is defined in