Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle 404 Status Code from MDS Identity Token calls #1636

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lqiu96
Copy link
Contributor

@lqiu96 lqiu96 commented Jan 28, 2025

Fixes #1409

This changes makes the ID Token MDS call match the behavior of the three other MDS calls (Universe Domain, Access Token, and Service Account).

This will check for the additional error scenario (404 Not Found) in addition to the 503 (Unavailable). Perhaps other non 2xx status codes should be checked? For this PR, limit to just those two unless issues are raised.

Added tests for 404 + 503 for ID token. A larger refactoring effort outside of this PR can be made to add tests for the other three MDS calls (#1637).

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
68.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@lqiu96 lqiu96 requested a review from zhumin8 January 28, 2025 23:18
@lqiu96 lqiu96 marked this pull request as ready for review January 28, 2025 23:18
@lqiu96 lqiu96 requested review from a team as code owners January 28, 2025 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ComputeEngineCredentials does not handle error response from the metadata server correctly
1 participant