feat(google-auth): make _CLOUD_RESOURCE_MANAGER URL universe-domain-aware#16546
feat(google-auth): make _CLOUD_RESOURCE_MANAGER URL universe-domain-aware#16546baha-zrelli wants to merge 1 commit intogoogleapis:mainfrom
Conversation
…ware
Replace hardcoded googleapis.com in _CLOUD_RESOURCE_MANAGER with a
{universe_domain} placeholder, resolved at credential construction time
via self._cloud_resource_manager_url. This mirrors the existing pattern
used for _DEFAULT_TOKEN_URL.
Add tests verifying the URL is correctly built for both the default
(googleapis.com) and custom universe domains, including an end-to-end
test through get_project_id.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the external_account module to support custom universe domains for the Cloud Resource Manager URL by parameterizing the base URL and initializing it within the class. Unit tests were added to verify the URL construction for both default and custom domains. A review comment suggests using .format() for URL construction to improve readability and robustness.
| if project_number and scopes: | ||
| headers = {} | ||
| url = _CLOUD_RESOURCE_MANAGER + project_number | ||
| url = self._cloud_resource_manager_url + project_number |
There was a problem hiding this comment.
While the existing code used string concatenation, using an f-string or .format() is generally preferred for URL construction to ensure that the project_number is correctly handled even if it is passed as an integer. This also improves readability.
| url = self._cloud_resource_manager_url + project_number | |
| url = "{}{}".format(self._cloud_resource_manager_url, project_number) |
Replace hardcoded googleapis.com in _CLOUD_RESOURCE_MANAGER with a {universe_domain} placeholder, resolved at credential construction time via self._cloud_resource_manager_url. This mirrors the existing pattern used for _DEFAULT_TOKEN_URL.
Add tests verifying the URL is correctly built for both the default (googleapis.com) and custom universe domains, including an end-to-end test through get_project_id.
Fixes #16545