Skip to content

Commit 247946e

Browse files
chore: allow non-default service account for DP (#2635)
* chore: allow non-default service account for DP * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: only try DP with credentials * fix: add option for disabling direct path * fix: disable direct path for test --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent 1a2676d commit 247946e

File tree

5 files changed

+39
-17
lines changed

5 files changed

+39
-17
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.cloud.spanner;
1818

1919
import com.google.api.core.ApiFunction;
20+
import com.google.api.core.BetaApi;
2021
import com.google.api.core.InternalApi;
2122
import com.google.api.gax.core.ExecutorProvider;
2223
import com.google.api.gax.grpc.GrpcCallContext;
@@ -135,6 +136,7 @@ public class SpannerOptions extends ServiceOptions<Spanner, SpannerOptions> {
135136
private final CloseableExecutorProvider asyncExecutorProvider;
136137
private final String compressorName;
137138
private final boolean leaderAwareRoutingEnabled;
139+
private final boolean attemptDirectPath;
138140

139141
/** Interface that can be used to provide {@link CallCredentials} to {@link SpannerOptions}. */
140142
public interface CallCredentialsProvider {
@@ -624,6 +626,7 @@ private SpannerOptions(Builder builder) {
624626
asyncExecutorProvider = builder.asyncExecutorProvider;
625627
compressorName = builder.compressorName;
626628
leaderAwareRoutingEnabled = builder.leaderAwareRoutingEnabled;
629+
attemptDirectPath = builder.attemptDirectPath;
627630
}
628631

629632
/**
@@ -725,6 +728,7 @@ public static class Builder
725728
private String compressorName;
726729
private String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST");
727730
private boolean leaderAwareRoutingEnabled = true;
731+
private boolean attemptDirectPath = true;
728732

729733
private static String createCustomClientLibToken(String token) {
730734
return token + " " + ServiceOptions.getGoogApiClientLibName();
@@ -784,6 +788,7 @@ private Builder() {
784788
this.channelProvider = options.channelProvider;
785789
this.channelConfigurator = options.channelConfigurator;
786790
this.interceptorProvider = options.interceptorProvider;
791+
this.attemptDirectPath = options.attemptDirectPath;
787792
}
788793

789794
@Override
@@ -1220,6 +1225,12 @@ public Builder disableLeaderAwareRouting() {
12201225
return this;
12211226
}
12221227

1228+
@BetaApi
1229+
public Builder disableDirectPath() {
1230+
this.attemptDirectPath = false;
1231+
return this;
1232+
}
1233+
12231234
@SuppressWarnings("rawtypes")
12241235
@Override
12251236
public SpannerOptions build() {
@@ -1360,6 +1371,11 @@ public boolean isLeaderAwareRoutingEnabled() {
13601371
return leaderAwareRoutingEnabled;
13611372
}
13621373

1374+
@BetaApi
1375+
public boolean isAttemptDirectPath() {
1376+
return attemptDirectPath;
1377+
}
1378+
13631379
/** Returns the default query options to use for the specific database. */
13641380
public QueryOptions getDefaultQueryOptions(DatabaseId databaseId) {
13651381
// Use the specific query options for the database if any have been specified. These have

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import com.google.api.gax.rpc.UnavailableException;
5454
import com.google.api.gax.rpc.WatchdogProvider;
5555
import com.google.api.pathtemplate.PathTemplate;
56+
import com.google.cloud.NoCredentials;
5657
import com.google.cloud.RetryHelper;
5758
import com.google.cloud.RetryHelper.RetryHelperException;
5859
import com.google.cloud.grpc.GcpManagedChannelBuilder;
@@ -339,9 +340,16 @@ public GapicSpannerRpc(final SpannerOptions options) {
339340
// This sets the response compressor (Server -> Client).
340341
.withEncoding(compressorName))
341342
.setHeaderProvider(headerProviderWithUserAgent)
343+
.setAllowNonDefaultServiceAccount(true)
342344
// Attempts direct access to spanner service over gRPC to improve throughput,
343345
// whether the attempt is allowed is totally controlled by service owner.
344-
.setAttemptDirectPath(true);
346+
// We'll only attempt DirectPath if we are using real credentials.
347+
// NoCredentials is used for plain text connections, for example when connecting to
348+
// the emulator.
349+
.setAttemptDirectPath(
350+
options.isAttemptDirectPath()
351+
&& !Objects.equals(
352+
options.getScopedCredentials(), NoCredentials.getInstance()));
345353

346354
// If it is enabled in options uses the channel pool provided by the gRPC-GCP extension.
347355
maybeEnableGrpcGcpExtension(defaultChannelProviderBuilder, options);

google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/CredentialsProviderTest.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,10 @@ public void testCredentialsProvider() throws Throwable {
8181
"cloudspanner://localhost:%d/projects/proj/instances/inst/databases/db?credentialsProvider=%s",
8282
getPort(), TestCredentialsProvider.class.getName()))
8383
.setConfigurator(
84-
spannerOptions ->
85-
spannerOptions.setChannelConfigurator(
86-
ManagedChannelBuilder::usePlaintext))
84+
spannerOptions -> {
85+
spannerOptions.setChannelConfigurator(ManagedChannelBuilder::usePlaintext);
86+
spannerOptions.disableDirectPath();
87+
})
8788
.build();
8889

8990
try (Connection connection = options.getConnection()) {
@@ -122,9 +123,10 @@ public void testCredentialsProvider() throws Throwable {
122123
"cloudspanner://localhost:%d/projects/proj/instances/inst/databases/db?credentialsProvider=%s",
123124
getPort(), TestCredentialsProvider.class.getName()))
124125
.setConfigurator(
125-
spannerOptions ->
126-
spannerOptions.setChannelConfigurator(
127-
ManagedChannelBuilder::usePlaintext))
126+
spannerOptions -> {
127+
spannerOptions.setChannelConfigurator(ManagedChannelBuilder::usePlaintext);
128+
spannerOptions.disableDirectPath();
129+
})
128130
.build();
129131
try (Connection connection = options.getConnection()) {
130132
assertEquals(

google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.google.spanner.v1.TypeCode;
6464
import io.grpc.Context;
6565
import io.grpc.Contexts;
66+
import io.grpc.ManagedChannelBuilder;
6667
import io.grpc.Metadata;
6768
import io.grpc.Metadata.Key;
6869
import io.grpc.MethodDescriptor;
@@ -641,11 +642,8 @@ private SpannerOptions createSpannerOptions() {
641642
return SpannerOptions.newBuilder()
642643
.setProjectId("[PROJECT]")
643644
// Set a custom channel configurator to allow http instead of https.
644-
.setChannelConfigurator(
645-
input -> {
646-
input.usePlaintext();
647-
return input;
648-
})
645+
.setChannelConfigurator(ManagedChannelBuilder::usePlaintext)
646+
.disableDirectPath()
649647
.setHost("http://" + endpoint)
650648
// Set static credentials that will return the static OAuth test token.
651649
.setCredentials(STATIC_CREDENTIALS)

google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GfeLatencyTest.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.spanner.v1.StructType;
3535
import com.google.spanner.v1.TypeCode;
3636
import io.grpc.ForwardingServerCall;
37+
import io.grpc.ManagedChannelBuilder;
3738
import io.grpc.Metadata;
3839
import io.grpc.Server;
3940
import io.grpc.ServerCall;
@@ -311,11 +312,8 @@ private static SpannerOptions createSpannerOptions(InetSocketAddress address, Se
311312
return SpannerOptions.newBuilder()
312313
.setProjectId("[PROJECT]")
313314
// Set a custom channel configurator to allow http instead of https.
314-
.setChannelConfigurator(
315-
input -> {
316-
input.usePlaintext();
317-
return input;
318-
})
315+
.setChannelConfigurator(ManagedChannelBuilder::usePlaintext)
316+
.disableDirectPath()
319317
.setHost("http://" + endpoint)
320318
// Set static credentials that will return the static OAuth test token.
321319
.setCredentials(STATIC_CREDENTIALS)

0 commit comments

Comments
 (0)