Skip to content

Commit 537c16c

Browse files
macastelazmacastelaz
authored
fix(oauth2_http): Avoid retrying on 4xx errors during GCE metadata ping (#13715)
Don't retry GCE Metadata 4xx errors which are generally non-retriable errors and only adds startup noise/latency by retrying 3 times. Fixes #13649
1 parent ebde3bb commit 537c16c

105 files changed

Lines changed: 681 additions & 605 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,10 @@ void setProjectId(String projectId) {
648648
private static boolean pingComputeEngineMetadata(
649649
HttpTransportFactory transportFactory, DefaultCredentialsProvider provider) {
650650
GenericUrl tokenUrl = new GenericUrl(getMetadataServerUrl(provider));
651+
// pingComputeEngineMetadata is executed heavily during startup (within isOnGce()) on non-GCE
652+
// environments. We use a strict 500ms timeout and manual 3-try loop (instead of
653+
// ExponentialBackOff and HttpRequest.setUnsuccessfulResponseHandler) to fail fast and avoid
654+
// significantly delaying application startup for workflows running on local setups.
651655
for (int i = 1; i <= MAX_COMPUTE_PING_TRIES; ++i) {
652656
try {
653657
HttpRequest request =
@@ -673,6 +677,12 @@ private static boolean pingComputeEngineMetadata(
673677
} catch (SocketTimeoutException expected) {
674678
// Ignore logging timeouts which is the expected failure mode in non GCE environments.
675679
} catch (IOException e) {
680+
if (e instanceof HttpResponseException) {
681+
int statusCode = ((HttpResponseException) e).getStatusCode();
682+
if (statusCode >= 400 && statusCode < 500) {
683+
return false;
684+
}
685+
}
676686
LOGGER.log(
677687
Level.FINE,
678688
"Encountered an unexpected exception when checking"

‎google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/ComputeEngineCredentialsTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,13 @@
5858
import com.google.auth.TestUtils;
5959
import com.google.auth.http.HttpTransportFactory;
6060
import com.google.auth.oauth2.DefaultCredentialsProviderTest.MockRequestCountingTransportFactory;
61+
import java.io.ByteArrayInputStream;
62+
import java.io.File;
63+
import java.io.FileNotFoundException;
6164
import java.io.IOException;
65+
import java.io.InputStream;
6266
import java.net.URI;
67+
import java.nio.charset.StandardCharsets;
6368
import java.util.ArrayDeque;
6469
import java.util.Arrays;
6570
import java.util.Collection;
@@ -1177,6 +1182,60 @@ void getProjectId_explicitSet_noMDsCall() {
11771182
assertEquals(0, transportFactory.transport.getRequestCount());
11781183
}
11791184

1185+
@Test
1186+
void isOnGce_clientError_doesNotRetry_returnsFalseOnUnknownOs() {
1187+
MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
1188+
transportFactory.transport.setStatusCode(HttpStatusCodes.STATUS_CODE_NOT_FOUND);
1189+
DefaultCredentialsProvider provider =
1190+
new DefaultCredentialsProvider() {
1191+
@Override
1192+
String getEnv(String name) {
1193+
if (DefaultCredentialsProvider.NO_GCE_CHECK_ENV_VAR.equals(name)) {
1194+
return "false";
1195+
}
1196+
return super.getEnv(name);
1197+
}
1198+
1199+
@Override
1200+
String getOsName() {
1201+
return "Unknown";
1202+
}
1203+
};
1204+
boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider);
1205+
assertFalse(isOnGce);
1206+
assertEquals(1, transportFactory.transport.getRequestCount());
1207+
}
1208+
1209+
@Test
1210+
void isOnGce_clientError_doesNotRetry_returnsTrueOnLinuxGce() {
1211+
MockMetadataServerTransportFactory transportFactory = new MockMetadataServerTransportFactory();
1212+
transportFactory.transport.setStatusCode(HttpStatusCodes.STATUS_CODE_FORBIDDEN);
1213+
DefaultCredentialsProvider provider =
1214+
new DefaultCredentialsProvider() {
1215+
@Override
1216+
String getEnv(String name) {
1217+
if (DefaultCredentialsProvider.NO_GCE_CHECK_ENV_VAR.equals(name)) {
1218+
return "false";
1219+
}
1220+
return super.getEnv(name);
1221+
}
1222+
1223+
@Override
1224+
String getOsName() {
1225+
return "linux";
1226+
}
1227+
1228+
@Override
1229+
InputStream readStream(File file) throws FileNotFoundException {
1230+
return new ByteArrayInputStream(
1231+
"Google Compute Engine".getBytes(StandardCharsets.UTF_8));
1232+
}
1233+
};
1234+
boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider);
1235+
assertTrue(isOnGce);
1236+
assertEquals(1, transportFactory.transport.getRequestCount());
1237+
}
1238+
11801239
static class MockMetadataServerTransportFactory implements HttpTransportFactory {
11811240

11821241
MockMetadataServerTransport transport =

‎google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/MockMetadataServerTransport.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ public class MockMetadataServerTransport extends MockHttpTransport {
7171

7272
private boolean emptyContent;
7373
private MockLowLevelHttpRequest request;
74+
private int requestCount = 0;
75+
76+
public int getRequestCount() {
77+
return requestCount;
78+
}
7479

7580
public MockMetadataServerTransport() {}
7681

@@ -125,6 +130,7 @@ public MockLowLevelHttpRequest getRequest() {
125130

126131
@Override
127132
public LowLevelHttpRequest buildRequest(String method, String url) throws IOException {
133+
requestCount++;
128134
if (url.startsWith(ComputeEngineCredentials.getTokenServerEncodedUrl())) {
129135
this.request = getMockRequestForTokenEndpoint(url);
130136
return this.request;

‎java-backstory/proto-google-cloud-backstory/src/main/java/com/google/backstory/AnalyticsMetadata.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ private AnalyticsMetadata() {
5656
}
5757

5858
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
59-
return com.google.backstory.Udm.internal_static_google_backstory_AnalyticsMetadata_descriptor;
59+
return com.google.backstory.UdmProto.internal_static_google_backstory_AnalyticsMetadata_descriptor;
6060
}
6161

6262
@java.lang.Override
6363
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
6464
internalGetFieldAccessorTable() {
65-
return com.google.backstory.Udm
65+
return com.google.backstory.UdmProto
6666
.internal_static_google_backstory_AnalyticsMetadata_fieldAccessorTable
6767
.ensureFieldAccessorsInitialized(
6868
com.google.backstory.AnalyticsMetadata.class,
@@ -295,13 +295,13 @@ public static final class Builder extends com.google.protobuf.GeneratedMessage.B
295295
// @@protoc_insertion_point(builder_implements:google.backstory.AnalyticsMetadata)
296296
com.google.backstory.AnalyticsMetadataOrBuilder {
297297
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
298-
return com.google.backstory.Udm.internal_static_google_backstory_AnalyticsMetadata_descriptor;
298+
return com.google.backstory.UdmProto.internal_static_google_backstory_AnalyticsMetadata_descriptor;
299299
}
300300

301301
@java.lang.Override
302302
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
303303
internalGetFieldAccessorTable() {
304-
return com.google.backstory.Udm
304+
return com.google.backstory.UdmProto
305305
.internal_static_google_backstory_AnalyticsMetadata_fieldAccessorTable
306306
.ensureFieldAccessorsInitialized(
307307
com.google.backstory.AnalyticsMetadata.class,
@@ -325,7 +325,7 @@ public Builder clear() {
325325

326326
@java.lang.Override
327327
public com.google.protobuf.Descriptors.Descriptor getDescriptorForType() {
328-
return com.google.backstory.Udm.internal_static_google_backstory_AnalyticsMetadata_descriptor;
328+
return com.google.backstory.UdmProto.internal_static_google_backstory_AnalyticsMetadata_descriptor;
329329
}
330330

331331
@java.lang.Override

‎java-backstory/proto-google-cloud-backstory/src/main/java/com/google/backstory/AppCompatMetadata.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ private AppCompatMetadata() {
5656
}
5757

5858
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
59-
return com.google.backstory.Udm.internal_static_google_backstory_AppCompatMetadata_descriptor;
59+
return com.google.backstory.UdmProto.internal_static_google_backstory_AppCompatMetadata_descriptor;
6060
}
6161

6262
@java.lang.Override
6363
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
6464
internalGetFieldAccessorTable() {
65-
return com.google.backstory.Udm
65+
return com.google.backstory.UdmProto
6666
.internal_static_google_backstory_AppCompatMetadata_fieldAccessorTable
6767
.ensureFieldAccessorsInitialized(
6868
com.google.backstory.AppCompatMetadata.class,
@@ -354,13 +354,13 @@ public static final class Builder extends com.google.protobuf.GeneratedMessage.B
354354
// @@protoc_insertion_point(builder_implements:google.backstory.AppCompatMetadata)
355355
com.google.backstory.AppCompatMetadataOrBuilder {
356356
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
357-
return com.google.backstory.Udm.internal_static_google_backstory_AppCompatMetadata_descriptor;
357+
return com.google.backstory.UdmProto.internal_static_google_backstory_AppCompatMetadata_descriptor;
358358
}
359359

360360
@java.lang.Override
361361
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
362362
internalGetFieldAccessorTable() {
363-
return com.google.backstory.Udm
363+
return com.google.backstory.UdmProto
364364
.internal_static_google_backstory_AppCompatMetadata_fieldAccessorTable
365365
.ensureFieldAccessorsInitialized(
366366
com.google.backstory.AppCompatMetadata.class,
@@ -386,7 +386,7 @@ public Builder clear() {
386386

387387
@java.lang.Override
388388
public com.google.protobuf.Descriptors.Descriptor getDescriptorForType() {
389-
return com.google.backstory.Udm.internal_static_google_backstory_AppCompatMetadata_descriptor;
389+
return com.google.backstory.UdmProto.internal_static_google_backstory_AppCompatMetadata_descriptor;
390390
}
391391

392392
@java.lang.Override

‎java-backstory/proto-google-cloud-backstory/src/main/java/com/google/backstory/Artifact.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,13 @@ private Artifact() {
6363
}
6464

6565
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
66-
return com.google.backstory.Udm.internal_static_google_backstory_Artifact_descriptor;
66+
return com.google.backstory.UdmProto.internal_static_google_backstory_Artifact_descriptor;
6767
}
6868

6969
@java.lang.Override
7070
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
7171
internalGetFieldAccessorTable() {
72-
return com.google.backstory.Udm.internal_static_google_backstory_Artifact_fieldAccessorTable
72+
return com.google.backstory.UdmProto.internal_static_google_backstory_Artifact_fieldAccessorTable
7373
.ensureFieldAccessorsInitialized(
7474
com.google.backstory.Artifact.class, com.google.backstory.Artifact.Builder.class);
7575
}
@@ -1454,13 +1454,13 @@ public static final class Builder extends com.google.protobuf.GeneratedMessage.B
14541454
// @@protoc_insertion_point(builder_implements:google.backstory.Artifact)
14551455
com.google.backstory.ArtifactOrBuilder {
14561456
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
1457-
return com.google.backstory.Udm.internal_static_google_backstory_Artifact_descriptor;
1457+
return com.google.backstory.UdmProto.internal_static_google_backstory_Artifact_descriptor;
14581458
}
14591459

14601460
@java.lang.Override
14611461
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
14621462
internalGetFieldAccessorTable() {
1463-
return com.google.backstory.Udm.internal_static_google_backstory_Artifact_fieldAccessorTable
1463+
return com.google.backstory.UdmProto.internal_static_google_backstory_Artifact_fieldAccessorTable
14641464
.ensureFieldAccessorsInitialized(
14651465
com.google.backstory.Artifact.class, com.google.backstory.Artifact.Builder.class);
14661466
}
@@ -1560,7 +1560,7 @@ public Builder clear() {
15601560

15611561
@java.lang.Override
15621562
public com.google.protobuf.Descriptors.Descriptor getDescriptorForType() {
1563-
return com.google.backstory.Udm.internal_static_google_backstory_Artifact_descriptor;
1563+
return com.google.backstory.UdmProto.internal_static_google_backstory_Artifact_descriptor;
15641564
}
15651565

15661566
@java.lang.Override

‎java-backstory/proto-google-cloud-backstory/src/main/java/com/google/backstory/ArtifactClient.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,13 @@ private ArtifactClient() {
5757
}
5858

5959
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
60-
return com.google.backstory.Udm.internal_static_google_backstory_ArtifactClient_descriptor;
60+
return com.google.backstory.UdmProto.internal_static_google_backstory_ArtifactClient_descriptor;
6161
}
6262

6363
@java.lang.Override
6464
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
6565
internalGetFieldAccessorTable() {
66-
return com.google.backstory.Udm
66+
return com.google.backstory.UdmProto
6767
.internal_static_google_backstory_ArtifactClient_fieldAccessorTable
6868
.ensureFieldAccessorsInitialized(
6969
com.google.backstory.ArtifactClient.class,
@@ -401,13 +401,13 @@ public static final class Builder extends com.google.protobuf.GeneratedMessage.B
401401
// @@protoc_insertion_point(builder_implements:google.backstory.ArtifactClient)
402402
com.google.backstory.ArtifactClientOrBuilder {
403403
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
404-
return com.google.backstory.Udm.internal_static_google_backstory_ArtifactClient_descriptor;
404+
return com.google.backstory.UdmProto.internal_static_google_backstory_ArtifactClient_descriptor;
405405
}
406406

407407
@java.lang.Override
408408
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
409409
internalGetFieldAccessorTable() {
410-
return com.google.backstory.Udm
410+
return com.google.backstory.UdmProto
411411
.internal_static_google_backstory_ArtifactClient_fieldAccessorTable
412412
.ensureFieldAccessorsInitialized(
413413
com.google.backstory.ArtifactClient.class,
@@ -432,7 +432,7 @@ public Builder clear() {
432432

433433
@java.lang.Override
434434
public com.google.protobuf.Descriptors.Descriptor getDescriptorForType() {
435-
return com.google.backstory.Udm.internal_static_google_backstory_ArtifactClient_descriptor;
435+
return com.google.backstory.UdmProto.internal_static_google_backstory_ArtifactClient_descriptor;
436436
}
437437

438438
@java.lang.Override

‎java-backstory/proto-google-cloud-backstory/src/main/java/com/google/backstory/Asset.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,13 @@ private Asset() {
7070
}
7171

7272
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
73-
return com.google.backstory.Udm.internal_static_google_backstory_Asset_descriptor;
73+
return com.google.backstory.UdmProto.internal_static_google_backstory_Asset_descriptor;
7474
}
7575

7676
@java.lang.Override
7777
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
7878
internalGetFieldAccessorTable() {
79-
return com.google.backstory.Udm.internal_static_google_backstory_Asset_fieldAccessorTable
79+
return com.google.backstory.UdmProto.internal_static_google_backstory_Asset_fieldAccessorTable
8080
.ensureFieldAccessorsInitialized(
8181
com.google.backstory.Asset.class, com.google.backstory.Asset.Builder.class);
8282
}
@@ -2493,13 +2493,13 @@ public static final class Builder extends com.google.protobuf.GeneratedMessage.B
24932493
// @@protoc_insertion_point(builder_implements:google.backstory.Asset)
24942494
com.google.backstory.AssetOrBuilder {
24952495
public static final com.google.protobuf.Descriptors.Descriptor getDescriptor() {
2496-
return com.google.backstory.Udm.internal_static_google_backstory_Asset_descriptor;
2496+
return com.google.backstory.UdmProto.internal_static_google_backstory_Asset_descriptor;
24972497
}
24982498

24992499
@java.lang.Override
25002500
protected com.google.protobuf.GeneratedMessage.FieldAccessorTable
25012501
internalGetFieldAccessorTable() {
2502-
return com.google.backstory.Udm.internal_static_google_backstory_Asset_fieldAccessorTable
2502+
return com.google.backstory.UdmProto.internal_static_google_backstory_Asset_fieldAccessorTable
25032503
.ensureFieldAccessorsInitialized(
25042504
com.google.backstory.Asset.class, com.google.backstory.Asset.Builder.class);
25052505
}
@@ -2630,7 +2630,7 @@ public Builder clear() {
26302630

26312631
@java.lang.Override
26322632
public com.google.protobuf.Descriptors.Descriptor getDescriptorForType() {
2633-
return com.google.backstory.Udm.internal_static_google_backstory_Asset_descriptor;
2633+
return com.google.backstory.UdmProto.internal_static_google_backstory_Asset_descriptor;
26342634
}
26352635

26362636
@java.lang.Override

0 commit comments

Comments
 (0)