Skip to content

Commit f274739

Browse files
authored
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.
1 parent 86fbc4a commit f274739

File tree

3 files changed

+124
-9
lines changed

3 files changed

+124
-9
lines changed

‎google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,17 +164,27 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw
164164
} else {
165165
boolean foundQuotaProject = false;
166166
if (credentials.hasRequestMetadata()) {
167-
Map<String, List<String>> requestMetadata = credentials.getRequestMetadata(uri);
168-
for (Entry<String, List<String>> e : requestMetadata.entrySet()) {
169-
String key = e.getKey();
170-
if ("x-goog-user-project".equals(key.trim().toLowerCase(Locale.ENGLISH))) {
171-
List<String> value = e.getValue();
172-
if (!value.isEmpty()) {
173-
foundQuotaProject = true;
174-
defaultOpts = Opts.from(UnifiedOpts.userProject(value.get(0)));
175-
break;
167+
try {
168+
Map<String, List<String>> requestMetadata = credentials.getRequestMetadata(uri);
169+
for (Entry<String, List<String>> e : requestMetadata.entrySet()) {
170+
String key = e.getKey();
171+
if ("x-goog-user-project".equals(key.trim().toLowerCase(Locale.ENGLISH))) {
172+
List<String> value = e.getValue();
173+
if (!value.isEmpty()) {
174+
foundQuotaProject = true;
175+
defaultOpts = Opts.from(UnifiedOpts.userProject(value.get(0)));
176+
break;
177+
}
176178
}
177179
}
180+
} catch (IllegalStateException e) {
181+
// This happens when an instance of OAuth2Credentials attempts to refresh its
182+
// access token during our attempt at getting request metadata.
183+
// This is most easily reproduced by OAuth2Credentials.create(null);
184+
// see com.google.auth.oauth2.OAuth2Credentials.refreshAccessToken
185+
if (!e.getMessage().startsWith("OAuth2Credentials")) {
186+
throw e;
187+
}
178188
}
179189
}
180190
if (foundQuotaProject) {

‎google-cloud-storage/src/test/java/com/google/cloud/storage/TestUtils.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.google.cloud.storage.Retrying.RetryingDependencies;
3636
import com.google.common.collect.ImmutableList;
3737
import com.google.common.collect.ImmutableSet;
38+
import com.google.common.io.Files;
3839
import com.google.protobuf.Any;
3940
import com.google.protobuf.ByteString;
4041
import com.google.rpc.DebugInfo;
@@ -45,10 +46,12 @@
4546
import io.grpc.netty.shaded.io.netty.buffer.ByteBufUtil;
4647
import io.grpc.netty.shaded.io.netty.buffer.Unpooled;
4748
import java.io.ByteArrayOutputStream;
49+
import java.io.File;
4850
import java.io.IOException;
4951
import java.io.OutputStream;
5052
import java.nio.Buffer;
5153
import java.nio.ByteBuffer;
54+
import java.nio.charset.StandardCharsets;
5255
import java.util.Arrays;
5356
import java.util.Collections;
5457
import java.util.HashMap;
@@ -264,4 +267,21 @@ public static void assertAll(ThrowingRunnable... trs) throws Exception {
264267
map.put(k2, v2);
265268
return Collections.unmodifiableMap(map);
266269
}
270+
271+
// copied with minor modification from
272+
// com.google.api.gax.grpc.InstantiatingGrpcChannelProvider#isOnComputeEngine
273+
public static boolean isOnComputeEngine() {
274+
String osName = System.getProperty("os.name");
275+
if ("Linux".equals(osName)) {
276+
try {
277+
String result =
278+
Files.asCharSource(new File("/sys/class/dmi/id/product_name"), StandardCharsets.UTF_8)
279+
.readFirstLine();
280+
return result != null && (result.contains("Google") || result.contains("Compute Engine"));
281+
} catch (IOException ignored) {
282+
return false;
283+
}
284+
}
285+
return false;
286+
}
267287
}
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Copyright 2023 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.storage.it;
18+
19+
import static org.junit.Assume.assumeTrue;
20+
21+
import com.google.auth.Credentials;
22+
import com.google.auth.oauth2.GoogleCredentials;
23+
import com.google.auth.oauth2.OAuth2Credentials;
24+
import com.google.cloud.NoCredentials;
25+
import com.google.cloud.storage.Storage;
26+
import com.google.cloud.storage.StorageOptions;
27+
import com.google.cloud.storage.TestUtils;
28+
import com.google.cloud.storage.it.ITStorageOptionsTest.CredentialsParameters;
29+
import com.google.cloud.storage.it.runner.StorageITRunner;
30+
import com.google.cloud.storage.it.runner.annotations.Backend;
31+
import com.google.cloud.storage.it.runner.annotations.Parameterized;
32+
import com.google.cloud.storage.it.runner.annotations.Parameterized.Parameter;
33+
import com.google.cloud.storage.it.runner.annotations.Parameterized.ParametersProvider;
34+
import com.google.cloud.storage.it.runner.annotations.SingleBackend;
35+
import com.google.common.collect.ImmutableList;
36+
import org.junit.Ignore;
37+
import org.junit.Test;
38+
import org.junit.runner.RunWith;
39+
40+
@RunWith(StorageITRunner.class)
41+
@SingleBackend(Backend.PROD)
42+
@Parameterized(CredentialsParameters.class)
43+
public final class ITStorageOptionsTest {
44+
45+
public static final class CredentialsParameters implements ParametersProvider {
46+
47+
@Override
48+
public ImmutableList<?> parameters() {
49+
return ImmutableList.of(
50+
NoCredentials.getInstance(),
51+
GoogleCredentials.create(/* accessToken= */ null),
52+
OAuth2Credentials.create(null));
53+
}
54+
}
55+
56+
@Parameter public Credentials credentials;
57+
58+
@Test
59+
public void clientShouldConstructCleanly_http() throws Exception {
60+
StorageOptions options = StorageOptions.http().setCredentials(credentials).build();
61+
doTest(options);
62+
}
63+
64+
@Test
65+
public void clientShouldConstructCleanly_grpc() throws Exception {
66+
StorageOptions options =
67+
StorageOptions.grpc().setCredentials(credentials).setAttemptDirectPath(false).build();
68+
doTest(options);
69+
}
70+
71+
@Test
72+
@Ignore("waiting on conformation from the backend team if this should even be possible")
73+
public void clientShouldConstructCleanly_directPath() throws Exception {
74+
assumeTrue(
75+
"Unable to determine environment can access directPath", TestUtils.isOnComputeEngine());
76+
StorageOptions options =
77+
StorageOptions.grpc().setCredentials(credentials).setAttemptDirectPath(true).build();
78+
doTest(options);
79+
}
80+
81+
private static void doTest(StorageOptions options) throws Exception {
82+
//noinspection EmptyTryBlock
83+
try (Storage ignore = options.getService()) {}
84+
}
85+
}

0 commit comments

Comments
 (0)