feat(google-auth): make _CLOUD_RESOURCE_MANAGER URL universe-domain-aware by baha-zrelli · Pull Request #16546 · googleapis/google-cloud-python
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
…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.
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 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 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters