From f2747391b6daadd6a12a2dcb54714cce237eb1ac Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 20 Apr 2023 11:50:38 -0400 Subject: [PATCH] fix: update grpc x-goog-user-project handling gracefulness (#1983) When an instance of credentials that hasRequestMetadata() but can't refresh an IllegalStateException can be thrown. Add new tests to force failure and update handling to be graceful of this. --- .../cloud/storage/GrpcStorageOptions.java | 28 ++++-- .../com/google/cloud/storage/TestUtils.java | 20 +++++ .../storage/it/ITStorageOptionsTest.java | 85 +++++++++++++++++++ 3 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageOptionsTest.java diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index d1b0f5802..639f60e3a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -164,17 +164,27 @@ private Tuple> resolveSettingsAndOpts() throw } else { boolean foundQuotaProject = false; if (credentials.hasRequestMetadata()) { - Map> requestMetadata = credentials.getRequestMetadata(uri); - for (Entry> e : requestMetadata.entrySet()) { - String key = e.getKey(); - if ("x-goog-user-project".equals(key.trim().toLowerCase(Locale.ENGLISH))) { - List value = e.getValue(); - if (!value.isEmpty()) { - foundQuotaProject = true; - defaultOpts = Opts.from(UnifiedOpts.userProject(value.get(0))); - break; + try { + Map> requestMetadata = credentials.getRequestMetadata(uri); + for (Entry> e : requestMetadata.entrySet()) { + String key = e.getKey(); + if ("x-goog-user-project".equals(key.trim().toLowerCase(Locale.ENGLISH))) { + List value = e.getValue(); + if (!value.isEmpty()) { + foundQuotaProject = true; + defaultOpts = Opts.from(UnifiedOpts.userProject(value.get(0))); + break; + } } } + } catch (IllegalStateException e) { + // This happens when an instance of OAuth2Credentials attempts to refresh its + // access token during our attempt at getting request metadata. + // This is most easily reproduced by OAuth2Credentials.create(null); + // see com.google.auth.oauth2.OAuth2Credentials.refreshAccessToken + if (!e.getMessage().startsWith("OAuth2Credentials")) { + throw e; + } } } if (foundQuotaProject) { diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/TestUtils.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/TestUtils.java index 0ce0b4c31..875a3b0d8 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/TestUtils.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/TestUtils.java @@ -35,6 +35,7 @@ import com.google.cloud.storage.Retrying.RetryingDependencies; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.io.Files; import com.google.protobuf.Any; import com.google.protobuf.ByteString; import com.google.rpc.DebugInfo; @@ -45,10 +46,12 @@ import io.grpc.netty.shaded.io.netty.buffer.ByteBufUtil; import io.grpc.netty.shaded.io.netty.buffer.Unpooled; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.nio.Buffer; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -264,4 +267,21 @@ public static void assertAll(ThrowingRunnable... trs) throws Exception { map.put(k2, v2); return Collections.unmodifiableMap(map); } + + // copied with minor modification from + // com.google.api.gax.grpc.InstantiatingGrpcChannelProvider#isOnComputeEngine + public static boolean isOnComputeEngine() { + String osName = System.getProperty("os.name"); + if ("Linux".equals(osName)) { + try { + String result = + Files.asCharSource(new File("/sys/class/dmi/id/product_name"), StandardCharsets.UTF_8) + .readFirstLine(); + return result != null && (result.contains("Google") || result.contains("Compute Engine")); + } catch (IOException ignored) { + return false; + } + } + return false; + } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageOptionsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageOptionsTest.java new file mode 100644 index 000000000..85ee5f642 --- /dev/null +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITStorageOptionsTest.java @@ -0,0 +1,85 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage.it; + +import static org.junit.Assume.assumeTrue; + +import com.google.auth.Credentials; +import com.google.auth.oauth2.GoogleCredentials; +import com.google.auth.oauth2.OAuth2Credentials; +import com.google.cloud.NoCredentials; +import com.google.cloud.storage.Storage; +import com.google.cloud.storage.StorageOptions; +import com.google.cloud.storage.TestUtils; +import com.google.cloud.storage.it.ITStorageOptionsTest.CredentialsParameters; +import com.google.cloud.storage.it.runner.StorageITRunner; +import com.google.cloud.storage.it.runner.annotations.Backend; +import com.google.cloud.storage.it.runner.annotations.Parameterized; +import com.google.cloud.storage.it.runner.annotations.Parameterized.Parameter; +import com.google.cloud.storage.it.runner.annotations.Parameterized.ParametersProvider; +import com.google.cloud.storage.it.runner.annotations.SingleBackend; +import com.google.common.collect.ImmutableList; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(StorageITRunner.class) +@SingleBackend(Backend.PROD) +@Parameterized(CredentialsParameters.class) +public final class ITStorageOptionsTest { + + public static final class CredentialsParameters implements ParametersProvider { + + @Override + public ImmutableList parameters() { + return ImmutableList.of( + NoCredentials.getInstance(), + GoogleCredentials.create(/* accessToken= */ null), + OAuth2Credentials.create(null)); + } + } + + @Parameter public Credentials credentials; + + @Test + public void clientShouldConstructCleanly_http() throws Exception { + StorageOptions options = StorageOptions.http().setCredentials(credentials).build(); + doTest(options); + } + + @Test + public void clientShouldConstructCleanly_grpc() throws Exception { + StorageOptions options = + StorageOptions.grpc().setCredentials(credentials).setAttemptDirectPath(false).build(); + doTest(options); + } + + @Test + @Ignore("waiting on conformation from the backend team if this should even be possible") + public void clientShouldConstructCleanly_directPath() throws Exception { + assumeTrue( + "Unable to determine environment can access directPath", TestUtils.isOnComputeEngine()); + StorageOptions options = + StorageOptions.grpc().setCredentials(credentials).setAttemptDirectPath(true).build(); + doTest(options); + } + + private static void doTest(StorageOptions options) throws Exception { + //noinspection EmptyTryBlock + try (Storage ignore = options.getService()) {} + } +}