Skip to content

[REST] Fix #14152: az rest: Accept ARM URLs without subscription ID#14370

Merged
jiasli merged 5 commits intoAzure:devfrom
jiasli:rest-sub
Jul 15, 2020
Merged

[REST] Fix #14152: az rest: Accept ARM URLs without subscription ID#14370
jiasli merged 5 commits intoAzure:devfrom
jiasli:rest-sub

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented Jul 15, 2020

Description
Fix #14152: No subscription ID specified in the URL

#13782 introduced a bug that az rest fails for ARM URLs without subscription ID, like /subscriptions, /tenants:

if not match:
raise CLIError('No subscription ID specified in the URL')

This PR makes az rest accept such URLs and retrieve token for the current selected tenant instead.

The debug log is also improved:

> az rest -u /subscriptions?api-version=2020-01-01 --debug

DEBUG: No subscription ID specified in the URL https://management.azure.com/subscriptions?api-version=2020-01-01
DEBUG: Retrieving token for resource https://management.core.windows.net/

> az rest -u /subscriptions/{subscriptionId}/resourcegroups/rg1?api-version=2019-10-01 --debug

DEBUG: Found subscription ID 0b1f6471-1bf0-4dda-aec3-cb9272f09590 in the URL https://management.azure.com/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourcegroups/rg1?api-version=2019-10-01
DEBUG: Retrieving token for resource https://management.core.windows.net/, subscription 0b1f6471-1bf0-4dda-aec3-cb9272f09590

Testing Guide

az rest -u /subscriptions?api-version=2020-01-01
az rest -u /tenants?api-version=2020-01-01
az rest -u '/subscriptions/{subscriptionId}/resourcegroups/rg1?api-version=2019-10-01'

Comment on lines +814 to +815
logger.debug('No subscription ID specified in the URL %s', url)
return None
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stop raising an exception but return None instead.

Comment on lines +24 to +32
# Test ARM Subscriptions - List
# https://docs.microsoft.com/en-us/rest/api/resources/subscriptions/list
self.cmd('az rest -u /subscriptions?api-version=2020-01-01',
checks=[self.exists("value")])

# Test ARM Tenants - List
# https://docs.microsoft.com/en-us/rest/api/resources/tenants/list
self.cmd('az rest -u /tenants?api-version=2020-01-01',
checks=[self.exists("value")])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scenario tests are added for APIs without subscription ID.

Comment on lines +304 to +324
# Test ARM Subscriptions - List
# https://docs.microsoft.com/en-us/rest/api/resources/subscriptions/list
# /subscriptions?api-version=2020-01-01
send_raw_request(cli_ctx, 'GET', '/subscriptions?api-version=2020-01-01', body=test_body,
generated_client_request_id_name=None)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, test_arm_endpoint.rstrip('/') + '/subscriptions?api-version=2020-01-01')
self.assertDictEqual(dict(request.headers), expected_header_with_auth)

# Test ARM Tenants - List
# https://docs.microsoft.com/en-us/rest/api/resources/tenants/list
# /tenants?api-version=2020-01-01
send_raw_request(cli_ctx, 'GET', '/tenants?api-version=2020-01-01', body=test_body,
generated_client_request_id_name=None)

get_raw_token_mock.assert_called_with(mock.ANY, test_arm_active_directory_resource_id)
request = send_mock.call_args.args[1]
self.assertEqual(request.url, test_arm_endpoint.rstrip('/') + '/tenants?api-version=2020-01-01')
self.assertDictEqual(dict(request.headers), expected_header_with_auth)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests are added for APIs without subscription ID.

# Azure Rest API doc
from azure.cli.core._profile import Profile
profile = Profile()
profile = Profile(cli_ctx=cli_ctx)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass the existing cli_ctx to Profile. Otherwise, Profile will create a new AzCli which is unnecessary.

self.cli_ctx = cli_ctx or get_default_cli()

jiasli and others added 2 commits July 15, 2020 13:35
Copy link
Member

@jsntcy jsntcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@jsntcy
Copy link
Member

jsntcy commented Jul 15, 2020

@jiasli Please fix style check error.

Copy link
Contributor

@zhoxing-ms zhoxing-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jiasli jiasli merged commit 935b46b into Azure:dev Jul 15, 2020
@jiasli jiasli deleted the rest-sub branch July 15, 2020 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No subscription ID specified in the URL

5 participants