fix: remove call credentials from call options if DirectPath (#3670) · googleapis/sdk-platform-java@5ede29c · GitHub | Latest TMZ Celebrity News & Gossip | Watch TMZ Live
Skip to content

Commit 5ede29c

Browse files
rocksporerockspore
authored
fix: remove call credentials from call options if DirectPath (#3670)
This PR eliminates the issue where call credentials get attached twice to a RPC in DirectPath cases. Particularly, when user credentials get used, the problem causes the RPC to fail due to the duplication of the quota project ID (internal-only context: [b/364288002](http://b/364288002)). The approach is to strip the credentials in the callOptions returned by the `GrpcCallContext` if the `TransportChannel` is DirectPath. The side effect is that users won't be able to configure call credentials via the `ApiCallContext` if DirectPath is used. We think this is acceptable because: 1. Users can still configure the credentials via `TransportChannelProvider.withCredentials()`. At a higher level, this is done by configuring the `CredentialsProvider` in the `StubSettings` or the `ServiceOptions`. 2. As of now, DirectPath has its own special authentication flow, in which the service account attached to the GCE VM or GKE Pod will be used. Although in some special cases, the call credentials will be used to authenticate the client's identity, the peculiar nature of DirectPath should justify us limiting the flexibility of how call credentials can be configured in this case. ## Tested DirectPath using Spanner Headers sent ``` [:authority: spanner.googleapis.com, :path: /google.spanner.v1.Spanner/BatchCreateSessions, :method: POST, :scheme: https, content-type: application/grpc, te: trailers, user-agent: spanner-java/6.86.0 grpc-java-netty/1.69.0, ..., grpc-accept-encoding: gzip, authorization: Bearer ya29.****, ..., authorization: Bearer 1234, grpc-timeout: 56962080u] ``` Bearer token is sent twice (first `ya29.***` value is valid and second `1234` is invalid). The second one was attached by customizing the ApiCallContext to send an invalid CallCredentials as part of the CallOptions. The call still succeeded as the first Bearer token in the Metadata is used.
1 parent 3857e3f commit 5ede29c

File tree

2 files changed

+101
-19
lines changed

2 files changed

+101
-19
lines changed

‎gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallContext.java

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public final class GrpcCallContext implements ApiCallContext {
9696
private final ImmutableMap<String, List<String>> extraHeaders;
9797
private final ApiCallContextOptions options;
9898
private final EndpointContext endpointContext;
99+
private final boolean isDirectPath;
99100

100101
/** Returns an empty instance with a null channel and default {@link CallOptions}. */
101102
public static GrpcCallContext createDefault() {
@@ -111,7 +112,8 @@ public static GrpcCallContext createDefault() {
111112
ApiCallContextOptions.getDefaultOptions(),
112113
null,
113114
null,
114-
null);
115+
null,
116+
false);
115117
}
116118

117119
/** Returns an instance with the given channel and {@link CallOptions}. */
@@ -128,7 +130,8 @@ public static GrpcCallContext of(Channel channel, CallOptions callOptions) {
128130
ApiCallContextOptions.getDefaultOptions(),
129131
null,
130132
null,
131-
null);
133+
null,
134+
false);
132135
}
133136

134137
private GrpcCallContext(
@@ -143,10 +146,14 @@ private GrpcCallContext(
143146
ApiCallContextOptions options,
144147
@Nullable RetrySettings retrySettings,
145148
@Nullable Set<StatusCode.Code> retryableCodes,
146-
@Nullable EndpointContext endpointContext) {
149+
@Nullable EndpointContext endpointContext,
150+
boolean isDirectPath) {
147151
this.channel = channel;
148152
this.credentials = credentials;
149-
this.callOptions = Preconditions.checkNotNull(callOptions);
153+
Preconditions.checkNotNull(callOptions);
154+
// CallCredentials is stripped from CallOptions because CallCredentials are attached
155+
// to ChannelCredentials in DirectPath flows. Adding it again would duplicate the headers.
156+
this.callOptions = isDirectPath ? callOptions.withCallCredentials(null) : callOptions;
150157
this.timeout = timeout;
151158
this.streamWaitTimeout = streamWaitTimeout;
152159
this.streamIdleTimeout = streamIdleTimeout;
@@ -159,6 +166,7 @@ private GrpcCallContext(
159166
// a valid EndpointContext with user configurations after the client has been initialized.
160167
this.endpointContext =
161168
endpointContext == null ? EndpointContext.getDefaultInstance() : endpointContext;
169+
this.isDirectPath = isDirectPath;
162170
}
163171

164172
/**
@@ -199,7 +207,8 @@ public GrpcCallContext withCredentials(Credentials newCredentials) {
199207
options,
200208
retrySettings,
201209
retryableCodes,
202-
endpointContext);
210+
endpointContext,
211+
isDirectPath);
203212
}
204213

205214
@Override
@@ -210,7 +219,20 @@ public GrpcCallContext withTransportChannel(TransportChannel inputChannel) {
210219
"Expected GrpcTransportChannel, got " + inputChannel.getClass().getName());
211220
}
212221
GrpcTransportChannel transportChannel = (GrpcTransportChannel) inputChannel;
213-
return withChannel(transportChannel.getChannel());
222+
return new GrpcCallContext(
223+
transportChannel.getChannel(),
224+
credentials,
225+
callOptions,
226+
timeout,
227+
streamWaitTimeout,
228+
streamIdleTimeout,
229+
channelAffinity,
230+
extraHeaders,
231+
options,
232+
retrySettings,
233+
retryableCodes,
234+
endpointContext,
235+
transportChannel.isDirectPath());
214236
}
215237

216238
@Override
@@ -228,7 +250,8 @@ public GrpcCallContext withEndpointContext(EndpointContext endpointContext) {
228250
options,
229251
retrySettings,
230252
retryableCodes,
231-
endpointContext);
253+
endpointContext,
254+
isDirectPath);
232255
}
233256

234257
/** This method is obsolete. Use {@link #withTimeoutDuration(java.time.Duration)} instead. */
@@ -262,7 +285,8 @@ public GrpcCallContext withTimeoutDuration(@Nullable java.time.Duration timeout)
262285
options,
263286
retrySettings,
264287
retryableCodes,
265-
endpointContext);
288+
endpointContext,
289+
isDirectPath);
266290
}
267291

268292
/** This method is obsolete. Use {@link #getTimeoutDuration()} instead. */
@@ -310,7 +334,8 @@ public GrpcCallContext withStreamWaitTimeoutDuration(
310334
options,
311335
retrySettings,
312336
retryableCodes,
313-
endpointContext);
337+
endpointContext,
338+
isDirectPath);
314339
}
315340

316341
/**
@@ -344,7 +369,8 @@ public GrpcCallContext withStreamIdleTimeoutDuration(
344369
options,
345370
retrySettings,
346371
retryableCodes,
347-
endpointContext);
372+
endpointContext,
373+
isDirectPath);
348374
}
349375

350376
@BetaApi("The surface for channel affinity is not stable yet and may change in the future.")
@@ -361,7 +387,8 @@ public GrpcCallContext withChannelAffinity(@Nullable Integer affinity) {
361387
options,
362388
retrySettings,
363389
retryableCodes,
364-
endpointContext);
390+
endpointContext,
391+
isDirectPath);
365392
}
366393

367394
@BetaApi("The surface for extra headers is not stable yet and may change in the future.")
@@ -382,7 +409,8 @@ public GrpcCallContext withExtraHeaders(Map<String, List<String>> extraHeaders)
382409
options,
383410
retrySettings,
384411
retryableCodes,
385-
endpointContext);
412+
endpointContext,
413+
isDirectPath);
386414
}
387415

388416
@Override
@@ -404,7 +432,8 @@ public GrpcCallContext withRetrySettings(RetrySettings retrySettings) {
404432
options,
405433
retrySettings,
406434
retryableCodes,
407-
endpointContext);
435+
endpointContext,
436+
isDirectPath);
408437
}
409438

410439
@Override
@@ -426,7 +455,8 @@ public GrpcCallContext withRetryableCodes(Set<StatusCode.Code> retryableCodes) {
426455
options,
427456
retrySettings,
428457
retryableCodes,
429-
endpointContext);
458+
endpointContext,
459+
isDirectPath);
430460
}
431461

432462
@Override
@@ -456,6 +486,8 @@ public ApiCallContext merge(ApiCallContext inputCallContext) {
456486
newDeadline = callOptions.getDeadline();
457487
}
458488

489+
boolean newIsDirectPath = grpcCallContext.isDirectPath;
490+
459491
CallCredentials newCallCredentials = grpcCallContext.callOptions.getCredentials();
460492
if (newCallCredentials == null) {
461493
newCallCredentials = callOptions.getCredentials();
@@ -525,7 +557,8 @@ public ApiCallContext merge(ApiCallContext inputCallContext) {
525557
newOptions,
526558
newRetrySettings,
527559
newRetryableCodes,
528-
endpointContext);
560+
endpointContext,
561+
newIsDirectPath);
529562
}
530563

531564
/** The {@link Channel} set on this context. */
@@ -588,7 +621,11 @@ public Map<String, List<String>> getExtraHeaders() {
588621
return extraHeaders;
589622
}
590623

591-
/** Returns a new instance with the channel set to the given channel. */
624+
/**
625+
* This method is obsolete. Use {@link #withTransportChannel()} instead. Returns a new instance
626+
* with the channel set to the given channel.
627+
*/
628+
@ObsoleteApi("Use withTransportChannel() instead")
592629
public GrpcCallContext withChannel(Channel newChannel) {
593630
return new GrpcCallContext(
594631
newChannel,
@@ -602,7 +639,8 @@ public GrpcCallContext withChannel(Channel newChannel) {
602639
options,
603640
retrySettings,
604641
retryableCodes,
605-
endpointContext);
642+
endpointContext,
643+
isDirectPath);
606644
}
607645

608646
/** Returns a new instance with the call options set to the given call options. */
@@ -619,7 +657,8 @@ public GrpcCallContext withCallOptions(CallOptions newCallOptions) {
619657
options,
620658
retrySettings,
621659
retryableCodes,
622-
endpointContext);
660+
endpointContext,
661+
isDirectPath);
623662
}
624663

625664
public GrpcCallContext withRequestParamsDynamicHeaderOption(String requestParams) {
@@ -663,7 +702,8 @@ public <T> GrpcCallContext withOption(Key<T> key, T value) {
663702
newOptions,
664703
retrySettings,
665704
retryableCodes,
666-
endpointContext);
705+
endpointContext,
706+
isDirectPath);
667707
}
668708

669709
/** {@inheritDoc} */

‎gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcCallContextTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,14 @@
4242
import com.google.api.gax.rpc.testing.FakeTransportChannel;
4343
import com.google.api.gax.tracing.ApiTracer;
4444
import com.google.auth.Credentials;
45+
import com.google.auth.oauth2.GoogleCredentials;
4546
import com.google.common.collect.ImmutableMap;
4647
import com.google.common.truth.Truth;
48+
import io.grpc.CallCredentials;
4749
import io.grpc.CallOptions;
4850
import io.grpc.ManagedChannel;
4951
import io.grpc.Metadata.Key;
52+
import io.grpc.auth.MoreCallCredentials;
5053
import java.io.IOException;
5154
import java.util.ArrayList;
5255
import java.util.Collections;
@@ -100,6 +103,26 @@ void testWithTransportChannelWrongType() {
100103
}
101104
}
102105

106+
@Test
107+
void testWithTransportChannelIsDirectPath() {
108+
ManagedChannel channel = Mockito.mock(ManagedChannel.class);
109+
Credentials credentials = Mockito.mock(GoogleCredentials.class);
110+
GrpcCallContext context = GrpcCallContext.createDefault().withCredentials(credentials);
111+
assertNotNull(context.getCallOptions().getCredentials());
112+
context =
113+
context.withTransportChannel(
114+
GrpcTransportChannel.newBuilder()
115+
.setDirectPath(true)
116+
.setManagedChannel(channel)
117+
.build());
118+
assertNull(context.getCallOptions().getCredentials());
119+
120+
// Call credentials from the call options will be stripped.
121+
context.withCallOptions(
122+
CallOptions.DEFAULT.withCallCredentials(MoreCallCredentials.from(credentials)));
123+
assertNull(context.getCallOptions().getCredentials());
124+
}
125+
103126
@Test
104127
void testMergeWrongType() {
105128
try {
@@ -320,6 +343,25 @@ void testMergeWithCustomCallOptions() {
320343
.isEqualTo(ctx2.getCallOptions().getOption(key));
321344
}
322345

346+
@Test
347+
void testMergeWithIsDirectPath() {
348+
ManagedChannel channel = Mockito.mock(ManagedChannel.class);
349+
CallCredentials callCredentials = Mockito.mock(CallCredentials.class);
350+
GrpcCallContext ctx1 =
351+
GrpcCallContext.createDefault()
352+
.withCallOptions(CallOptions.DEFAULT.withCallCredentials(callCredentials));
353+
GrpcCallContext ctx2 =
354+
GrpcCallContext.createDefault()
355+
.withTransportChannel(
356+
GrpcTransportChannel.newBuilder()
357+
.setDirectPath(true)
358+
.setManagedChannel(channel)
359+
.build());
360+
361+
GrpcCallContext merged = (GrpcCallContext) ctx1.merge(ctx2);
362+
assertNull(merged.getCallOptions().getCredentials());
363+
}
364+
323365
@Test
324366
void testWithExtraHeaders() {
325367
Map<String, List<String>> extraHeaders =

0 commit comments

Comments
 (0)

TMZ Celebrity News – Breaking Stories, Videos & Gossip

Looking for the latest TMZ celebrity news? You've come to the right place. From shocking Hollywood scandals to exclusive videos, TMZ delivers it all in real time.

Whether it’s a red carpet slip-up, a viral paparazzi moment, or a legal drama involving your favorite stars, TMZ news is always first to break the story. Stay in the loop with daily updates, insider tips, and jaw-dropping photos.

🎥 Watch TMZ Live

TMZ Live brings you daily celebrity news and interviews straight from the TMZ newsroom. Don’t miss a beat—watch now and see what’s trending in Hollywood.