-
Notifications
You must be signed in to change notification settings - Fork 3.3k
{Cloud} az cloud register/update: Use ARM new API version 2022-09-01 for endpoint discovery
#30682
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
Conversation
️✔️AzureCLI-FullTest
|
|
Hi @evelyn-ys, |
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
az cloud register/update: Use ARM new API version 2022-09-01 for endpoint discoveryaz cloud register/update: Use ARM new API version 2022-09-01 for endpoint discovery
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
src/azure-cli/azure/cli/command_modules/cloud/custom.py:37
- [nitpick] The function name
_populate_from_metadata_endpointis misleading as it now returns a value instead of populating the cloud object. Consider renaming it to_get_metadata_endpointsor similar.
def _populate_from_metadata_endpoint(arm_endpoint, session=None):
src/azure-cli/azure/cli/command_modules/cloud/custom.py:37
- The refactored function
_populate_from_metadata_endpointshould have corresponding tests to ensure it behaves correctly after the changes.
def _populate_from_metadata_endpoint(arm_endpoint, session=None):
| METADATA_ENDPOINT_SUFFIX = '/metadata/endpoints?api-version=2015-01-01' | ||
| if not arm_endpoint or all([cloud.endpoints.has_endpoint_set(n) for n in endpoints_in_metadata]): # pylint: disable=use-a-generator | ||
| return | ||
| def _populate_from_metadata_endpoint(arm_endpoint, session=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to change the function signature by removing cloud?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because now we generate a Cloud from none. While calling this func, there's no cloud yet so I have nothing to pass in
Related command
az cloud register/updateDescription
Close #25815, #28716
Upgrade ARM endpoint API from
/metadata/endpoints?api-version=2015-01-01to/metadata/endpoints?api-version=2022-09-01for cloud registration.Now data plane endpoints auto discovery is supported
But this will cause some output changes:
galleryendpointactiveDirectoryendpoint changed fromhttps://login.microsoftonline.com/tohttps://login.microsoftonline.commanagementendpoint explicitly set tohttps://management.core.windows.net/Testing Guide
Register cloud
az cloud register -n myCloud --endpoint-resource-manager https://management.azure.com/Check cloud
az cloud showBefore
After
History Notes
[Cloud] BREAKING CHANGE:
az cloud register/update: Nogalleryendpoint returned if use endpoint discovery with--endpoint-resource-manager[Cloud]
az cloud register/update: Support data plane endpoints auto discovery with--endpoint-resource-managerThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.