Skip to content

fix(oauth2_http): Avoid retrying on 403 Forbidden during GCE metadata ping#13707

Closed
macastelaz wants to merge 4 commits into
googleapis:mainfrom
macastelaz:fix-403-retry
Closed

fix(oauth2_http): Avoid retrying on 403 Forbidden during GCE metadata ping#13707
macastelaz wants to merge 4 commits into
googleapis:mainfrom
macastelaz:fix-403-retry

Conversation

@macastelaz

Copy link
Copy Markdown

Don't retry GCE Metadata 403 errors which are generally non-retriable errors and only adds startup noise/latency by retrying 3 times.

Fixes #13649

@macastelaz macastelaz requested review from a team as code owners July 8, 2026 19:24

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates ComputeEngineCredentials to immediately return false when pinging the Compute Engine metadata server returns a 403 Forbidden response, avoiding unnecessary retries. It also adds a corresponding unit test. The reviewer suggested improving the unit test by asserting that only a single request is made, ensuring that no retries actually occur on a 403 response.

Comment on lines +1185 to +1187
boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider);
assertFalse(isOnGce);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The test isOnGce_forbidden_doesNotRetry asserts that isOnGce returns false, but it does not verify that the request was not retried. If a regression is introduced where the client still retries on a 403 response, the test would still pass because isOnGce would eventually return false after exhausting the retries. To ensure that we actually avoid retrying, assert that the request count is exactly 1.

Suggested change
boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider);
assertFalse(isOnGce);
}
boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider);
assertFalse(isOnGce);
assertEquals(1, transportFactory.transport.getRequestCount());
}

@macastelaz macastelaz closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(auth): ComputeEngineCredentials.isOnGce retries on non-transient 403 Forbidden responses

1 participant