From 0963f3151ddf33417eb4db21034e6cbb6a19aa4b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 26 Apr 2022 08:59:08 -0700 Subject: [PATCH] auth: Add support for Retryable interface Retryable was added in google-auth-library 1.5.3 to make clear the situations that deserve a retry of the RPC. Bump to that version and swap away from the imprecise IOException heuristic. go/auth-correct-retry Fixes #6808 --- .../GoogleAuthLibraryCallCredentials.java | 6 +-- .../GoogleAuthLibraryCallCredentialsTest.java | 40 ++++++++++++++++++- build.gradle | 2 +- gcp-observability/build.gradle | 7 ++-- .../integration/AbstractInteropTest.java | 6 +-- repositories.bzl | 4 +- 6 files changed, 50 insertions(+), 15 deletions(-) diff --git a/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java b/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java index 4b95a6c7f4d..98910fd97e5 100644 --- a/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java +++ b/auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java @@ -20,6 +20,7 @@ import com.google.auth.Credentials; import com.google.auth.RequestMetadataCallback; +import com.google.auth.Retryable; import com.google.common.annotations.VisibleForTesting; import com.google.common.io.BaseEncoding; import io.grpc.Metadata; @@ -27,7 +28,6 @@ import io.grpc.SecurityLevel; import io.grpc.Status; import io.grpc.StatusException; -import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.net.URI; @@ -133,8 +133,8 @@ public void onSuccess(Map> metadata) { @Override public void onFailure(Throwable e) { - if (e instanceof IOException) { - // Since it's an I/O failure, let the call be retried with UNAVAILABLE. + if (e instanceof Retryable && ((Retryable) e).isRetryable()) { + // Let the call be retried with UNAVAILABLE. applier.fail(Status.UNAVAILABLE .withDescription("Credentials failed to obtain metadata") .withCause(e)); diff --git a/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java b/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java index ee5713bfd27..2a2a7b6e478 100644 --- a/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java +++ b/auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java @@ -30,6 +30,7 @@ import com.google.auth.Credentials; import com.google.auth.RequestMetadataCallback; +import com.google.auth.Retryable; import com.google.auth.http.HttpTransportFactory; import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.GoogleCredentials; @@ -191,8 +192,9 @@ public void invalidBase64() throws Exception { } @Test - public void credentialsFailsWithIoException() throws Exception { - Exception exception = new IOException("Broken"); + public void credentialsFailsWithRetryableRetryableException() throws Exception { + boolean retryable = true; + Exception exception = new RetryableException(retryable); when(credentials.getRequestMetadata(eq(expectedUri))).thenThrow(exception); GoogleAuthLibraryCallCredentials callCredentials = @@ -206,6 +208,23 @@ public void credentialsFailsWithIoException() throws Exception { assertEquals(exception, status.getCause()); } + @Test + public void credentialsFailsWithUnretryableRetryableException() throws Exception { + boolean retryable = false; + Exception exception = new RetryableException(retryable); + when(credentials.getRequestMetadata(eq(expectedUri))).thenThrow(exception); + + GoogleAuthLibraryCallCredentials callCredentials = + new GoogleAuthLibraryCallCredentials(credentials); + callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier); + + verify(credentials).getRequestMetadata(eq(expectedUri)); + verify(applier).fail(statusCaptor.capture()); + Status status = statusCaptor.getValue(); + assertEquals(Status.Code.UNAUTHENTICATED, status.getCode()); + assertEquals(exception, status.getCause()); + } + @Test public void credentialsFailsWithRuntimeException() throws Exception { Exception exception = new RuntimeException("Broken"); @@ -453,4 +472,21 @@ public Attributes getTransportAttrs() { return Attributes.EMPTY; } } + + private static class RetryableException extends IOException implements Retryable { + private final boolean retryable; + + public RetryableException(boolean retryable) { + super("Broken"); + this.retryable = retryable; + } + + @Override public boolean isRetryable() { + return retryable; + } + + @Override public int getRetryCount() { + return 0; + } + } } diff --git a/build.gradle b/build.gradle index f982f600bbe..9d8f8725576 100644 --- a/build.gradle +++ b/build.gradle @@ -57,7 +57,7 @@ subprojects { nettyVersion = '4.1.72.Final' guavaVersion = '31.0.1-android' - googleauthVersion = '1.4.0' + googleauthVersion = '1.5.3' protobufVersion = '3.19.2' protocVersion = protobufVersion opencensusVersion = '0.28.0' diff --git a/gcp-observability/build.gradle b/gcp-observability/build.gradle index 5e81dd438d4..1d1fae3a2d3 100644 --- a/gcp-observability/build.gradle +++ b/gcp-observability/build.gradle @@ -28,16 +28,15 @@ dependencies { implementation project(':grpc-protobuf'), project(':grpc-stub'), project(':grpc-alts'), + libraries.google_auth_credentials, libraries.google_auth_oauth2_http, libraries.autovalue_annotation, libraries.perfmark, ('com.google.guava:guava:31.0.1-jre'), ('com.google.errorprone:error_prone_annotations:2.11.0'), - ('com.google.auth:google-auth-library-credentials:1.4.0'), ('org.checkerframework:checker-qual:3.20.0'), - ('com.google.auto.value:auto-value-annotations:1.9'), - ('com.google.http-client:google-http-client:1.41.0'), - ('com.google.http-client:google-http-client-gson:1.41.0'), + ('com.google.http-client:google-http-client:1.41.3'), + ('com.google.http-client:google-http-client-gson:1.41.3'), ('com.google.api.grpc:proto-google-common-protos:2.7.1'), ("com.google.cloud:google-cloud-logging:${cloudLoggingVersion}") diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java index 8e107add3e3..7f6f5a6e90e 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java @@ -100,7 +100,6 @@ import io.opencensus.trace.Span; import io.opencensus.trace.SpanContext; import io.opencensus.trace.Tracing; -import io.opencensus.trace.unsafe.ContextUtils; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -1547,6 +1546,7 @@ public void customMetadata() throws Exception { Collections.singleton(streamingRequest), Collections.singleton(goldenStreamingResponse)); } + @SuppressWarnings("deprecation") @Test(timeout = 10000) public void censusContextsPropagated() { Assume.assumeTrue("Skip the test because server is not in the same process.", server != null); @@ -1561,7 +1561,7 @@ public void censusContextsPropagated() { .emptyBuilder() .putLocal(StatsTestUtils.EXTRA_TAG, TagValue.create("extra value")) .build()); - ctx = ContextUtils.withValue(ctx, clientParentSpan); + ctx = io.opencensus.trace.unsafe.ContextUtils.withValue(ctx, clientParentSpan); Context origCtx = ctx.attach(); try { blockingStub.unaryCall(SimpleRequest.getDefaultInstance()); @@ -1581,7 +1581,7 @@ public void censusContextsPropagated() { } assertTrue("tag not found", tagFound); - Span span = ContextUtils.getValue(serverCtx); + Span span = io.opencensus.trace.unsafe.ContextUtils.getValue(serverCtx); assertNotNull(span); SpanContext spanContext = span.getContext(); assertEquals(clientParentSpan.getContext().getTraceId(), spanContext.getTraceId()); diff --git a/repositories.bzl b/repositories.bzl index 3d8f86c7d69..13ebaea9844 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -12,8 +12,8 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") IO_GRPC_GRPC_JAVA_ARTIFACTS = [ "com.google.android:annotations:4.1.1.4", "com.google.api.grpc:proto-google-common-protos:2.0.1", - "com.google.auth:google-auth-library-credentials:0.22.0", - "com.google.auth:google-auth-library-oauth2-http:0.22.0", + "com.google.auth:google-auth-library-credentials:1.5.3", + "com.google.auth:google-auth-library-oauth2-http:1.5.3", "com.google.code.findbugs:jsr305:3.0.2", "com.google.code.gson:gson:2.8.9", "com.google.auto.value:auto-value:1.7.4",