Skip to content

Retry GCE environment check for Application Default Credentials#110

Merged
tcoffee-google merged 4 commits intogoogleapis:masterfrom
tcoffee-google:retry_gce_check
Jun 30, 2017
Merged

Retry GCE environment check for Application Default Credentials#110
tcoffee-google merged 4 commits intogoogleapis:masterfrom
tcoffee-google:retry_gce_check

Conversation

@tcoffee-google
Copy link
Copy Markdown
Contributor

@tcoffee-google tcoffee-google commented Jun 26, 2017

Adapt fix from Apiary client libraries. Currently untested.

Addresses #109.

@tcoffee-google
WIP: Retry GCE environment check for Application Default Credentials
8eabb98 
Adapt fix from Apiary client libraries. Currently untested.

Addresses googleapis#109.
lukecwik
lukecwik reviewed Jun 27, 2017
View reviewed changes
Copy link
Copy Markdown

@lukecwik lukecwik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the changes done in the Apiary OAuth code and also addresses some other minor differences like the NO_GCE_CHECK and GCE_METADATA_HOST which aren't strictly required to fix the issue but I believe are a good idea to include in this change.

Comment thread oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java Outdated
@@ -61,6 +63,22 @@ public class ComputeEngineCredentials extends GoogleCredentials {
"http://metadata/computeMetadata/v1/instance/service-accounts/default/token";

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java Outdated
@@ -61,6 +63,22 @@ public class ComputeEngineCredentials extends GoogleCredentials {
"http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
static final String METADATA_SERVER_URL = "http://metadata.google.internal";

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
return true;
/** Return whether code is running on Google Compute Engine. */
static boolean runningOnComputeEngine(
HttpTransportFactory transportFactory, DefaultCredentialsProvider provider) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tcoffee-google tcoffee-google changed the title WIP: Retry GCE environment check for Application Default Credentials Retry GCE environment check for Application Default Credentials Jun 30, 2017
lukecwik
lukecwik suggested changes Jun 30, 2017
View reviewed changes
Comment thread oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java Outdated
}

public static String getTokenServerEncodedUrl() {
return getMetadataServerUrl() + "/computeMetadata/v1/instance/service-accounts/default/token";

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread oauth2_http/javatests/com/google/auth/oauth2/DefaultCredentialsProviderTest.java
assertEquals(transportFactory.transport.getRequestCount(), 0);
}

@Test

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@lukecwik
Copy link
Copy Markdown

lukecwik commented Jun 30, 2017

LGTM

@lukecwik
Copy link
Copy Markdown

lukecwik commented Jun 30, 2017

When is the next scheduled release so that Apache Beam can incorporate these fixes?

@tcoffee-google
Copy link
Copy Markdown
Contributor Author

tcoffee-google commented Jun 30, 2017

This is the only update since the last release, but I wouldn't mind doing another. @lesv do you have any objection?

@tcoffee-google tcoffee-google merged commit 05a343a into googleapis:master Jun 30, 2017
@tcoffee-google tcoffee-google deleted the retry_gce_check branch June 30, 2017 19:41
@garrettjonesgoogle garrettjonesgoogle mentioned this pull request Jul 18, 2017
Closed
@droazen droazen mentioned this pull request Sep 21, 2017
Closed
This was referenced Jun 19, 2018
Closed
Closed
@lbergelson lbergelson mentioned this pull request Oct 24, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anthmgoogle anthmgoogle Awaiting requested review from anthmgoogle

@garrettjonesgoogle garrettjonesgoogle Awaiting requested review from garrettjonesgoogle

2 more reviewers

@lukecwik lukecwik lukecwik requested changes

@lesv lesv lesv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tcoffee-google @lukecwik @lesv