fix(oauth2_http): Avoid retrying on 403 Forbidden during GCE metadata ping#13707
fix(oauth2_http): Avoid retrying on 403 Forbidden during GCE metadata ping#13707macastelaz wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
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.
| boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider); | ||
| assertFalse(isOnGce); | ||
| } |
There was a problem hiding this comment.
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.
| boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider); | |
| assertFalse(isOnGce); | |
| } | |
| boolean isOnGce = ComputeEngineCredentials.isOnGce(transportFactory, provider); | |
| assertFalse(isOnGce); | |
| assertEquals(1, transportFactory.transport.getRequestCount()); | |
| } |
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