From 7a876e957ddd1ace8501cfeff6718f47868442c1 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Mon, 13 Jul 2020 10:47:28 +0800 Subject: [PATCH 1/3] Delay importing MSIAuthenticationWrapper and requests --- src/azure-cli-core/azure/cli/core/__init__.py | 4 +--- src/azure-cli-core/azure/cli/core/_profile.py | 4 ++-- src/azure-cli-core/azure/cli/core/extension/_index.py | 2 +- src/azure-cli-core/azure/cli/core/extension/operations.py | 3 ++- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index c28f6727ce0..9636e33d430 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -255,9 +255,6 @@ def _update_command_table_from_extensions(ext_suppressions, extension_modname=No Otherwise, the list will be extended using ALWAYS_LOADED_EXTENSIONS. If the extensions in the list are not installed, it will be skipped. """ - - from azure.cli.core.extension.operations import check_version_compatibility - def _handle_extension_suppressions(extensions): filtered_extensions = [] for ext in extensions: @@ -299,6 +296,7 @@ def _filter_modname(extensions): for ext in allowed_extensions: try: + from azure.cli.core.extension.operations import check_version_compatibility check_version_compatibility(ext.get_metadata()) except CLIError as ex: # issue warning and skip loading extensions that aren't compatible with the CLI core diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 1700a65b2f8..7a957e09ec2 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -21,8 +21,6 @@ is_windows, is_wsl from azure.cli.core.cloud import get_active_cloud, set_cloud_subscription -from .adal_authentication import MSIAuthenticationWrapper - from knack.log import get_logger from knack.util import CLIError @@ -309,6 +307,7 @@ def find_subscriptions_in_vm_with_msi(self, identity_id=None, allow_no_subscript import jwt from requests import HTTPError from msrestazure.tools import is_valid_resource_id + from azure.cli.core.adal_authentication import MSIAuthenticationWrapper resource = self.cli_ctx.cloud.endpoints.active_directory_resource_id if identity_id: @@ -773,6 +772,7 @@ def valid_msi_account_types(): @staticmethod def msi_auth_factory(cli_account_name, identity, resource): + from azure.cli.core.adal_authentication import MSIAuthenticationWrapper if cli_account_name == MsiAccountTypes.system_assigned: return MSIAuthenticationWrapper(resource=resource) if cli_account_name == MsiAccountTypes.user_assigned_client_id: diff --git a/src/azure-cli-core/azure/cli/core/extension/_index.py b/src/azure-cli-core/azure/cli/core/extension/_index.py index 9a3b72c344d..bea5d310e2f 100644 --- a/src/azure-cli-core/azure/cli/core/extension/_index.py +++ b/src/azure-cli-core/azure/cli/core/extension/_index.py @@ -2,7 +2,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -import requests from knack.log import get_logger from knack.util import CLIError @@ -22,6 +21,7 @@ # pylint: disable=inconsistent-return-statements def get_index(index_url=None): + import requests from azure.cli.core.util import should_disable_connection_verify index_url = index_url or DEFAULT_INDEX_URL diff --git a/src/azure-cli-core/azure/cli/core/extension/operations.py b/src/azure-cli-core/azure/cli/core/extension/operations.py index 8a929e24d63..8aacfc970e8 100644 --- a/src/azure-cli-core/azure/cli/core/extension/operations.py +++ b/src/azure-cli-core/azure/cli/core/extension/operations.py @@ -15,7 +15,6 @@ from subprocess import check_output, STDOUT, CalledProcessError from six.moves.urllib.parse import urlparse # pylint: disable=import-error -import requests from pkg_resources import parse_version from azure.cli.core import CommandIndex @@ -64,6 +63,7 @@ def _run_pip(pip_exec_args, extension_path=None): def _whl_download_from_url(url_parse_result, ext_file): + import requests from azure.cli.core.util import should_disable_connection_verify url = url_parse_result.geturl() r = requests.get(url, stream=True, verify=(not should_disable_connection_verify())) @@ -106,6 +106,7 @@ def _add_whl_ext(cmd, source, ext_sha256=None, pip_extra_index_urls=None, pip_pr tmp_dir = tempfile.mkdtemp() ext_file = os.path.join(tmp_dir, whl_filename) logger.debug('Downloading %s to %s', source, ext_file) + import requests try: cmd.cli_ctx.get_progress_controller().add(message='Downloading') _whl_download_from_url(url_parse_result, ext_file) From 0e6082e0c4f0c04feeca2ed0c49af34944b90b81 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Mon, 13 Jul 2020 11:00:03 +0800 Subject: [PATCH 2/3] Add comments --- src/azure-cli-core/azure/cli/core/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/azure-cli-core/azure/cli/core/__init__.py b/src/azure-cli-core/azure/cli/core/__init__.py index 9636e33d430..a317ae70dea 100644 --- a/src/azure-cli-core/azure/cli/core/__init__.py +++ b/src/azure-cli-core/azure/cli/core/__init__.py @@ -296,6 +296,8 @@ def _filter_modname(extensions): for ext in allowed_extensions: try: + # Import in the `for` loop because `allowed_extensions` can be []. In such case we + # don't need to import `check_version_compatibility` at all. from azure.cli.core.extension.operations import check_version_compatibility check_version_compatibility(ext.get_metadata()) except CLIError as ex: From e8efdce2d79d977850d3c964f0b3aecd44515be9 Mon Sep 17 00:00:00 2001 From: Jiashuo Li Date: Mon, 13 Jul 2020 11:28:54 +0800 Subject: [PATCH 3/3] Fix tests --- src/azure-cli-core/azure/cli/core/_profile.py | 1 + .../azure/cli/core/tests/test_profile.py | 16 ++++++++-------- .../cli/core/tests/test_profile_v2016_06_01.py | 14 +++++++------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 7a957e09ec2..be698a3cad3 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -387,6 +387,7 @@ def find_subscriptions_in_cloud_console(self): return deepcopy(consolidated) def _get_token_from_cloud_shell(self, resource): # pylint: disable=no-self-use + from azure.cli.core.adal_authentication import MSIAuthenticationWrapper auth = MSIAuthenticationWrapper(resource=resource) auth.set_token() token_entry = auth.token diff --git a/src/azure-cli-core/azure/cli/core/tests/test_profile.py b/src/azure-cli-core/azure/cli/core/tests/test_profile.py index 2909a05e870..7022fca706d 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_profile.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_profile.py @@ -645,7 +645,7 @@ def test_get_login_credentials_aux_tenants(self, mock_get_token, mock_read_cred_ aux_tenants=[test_tenant_id2]) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_system_assigned(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -676,7 +676,7 @@ def test_get_login_credentials_msi_system_assigned(self, mock_msi_auth, mock_rea self.assertTrue(cred.token_read_count) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_user_assigned_with_client_id(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -707,7 +707,7 @@ def test_get_login_credentials_msi_user_assigned_with_client_id(self, mock_msi_a self.assertTrue(cred.client_id, test_client_id) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_user_assigned_with_object_id(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -738,7 +738,7 @@ def test_get_login_credentials_msi_user_assigned_with_object_id(self, mock_msi_a self.assertTrue(cred.object_id, test_object_id) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_user_assigned_with_res_id(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -849,7 +849,7 @@ def test_get_raw_token_for_sp(self, mock_get_token, mock_read_cred_file): self.assertEqual(tenant, self.tenant_id) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_raw_token_msi_system_assigned(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -884,7 +884,7 @@ def test_get_raw_token_msi_system_assigned(self, mock_msi_auth, mock_read_cred_f @mock.patch('azure.cli.core._profile.in_cloud_console', autospec=True) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_raw_token_in_cloud_console(self, mock_msi_auth, mock_read_cred_file, mock_in_cloud_console): mock_read_cred_file.return_value = [] mock_in_cloud_console.return_value = True @@ -1037,7 +1037,7 @@ def test_find_subscriptions_thru_username_non_password(self, mock_auth_context): # assert self.assertEqual([], subs) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) @mock.patch('azure.cli.core.profiles._shared.get_client_class', autospec=True) @mock.patch('azure.cli.core._profile._get_cloud_console_token_endpoint', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder', autospec=True) @@ -1186,7 +1186,7 @@ def __init__(self, *args, **kwargs): self.assertEqual(s['id'], self.id1.split('/')[-1]) self.assertEqual(s['tenantId'], '54826b22-38d6-4fb2-bad9-b7b93a3e9c5a') - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) @mock.patch('azure.cli.core.profiles._shared.get_client_class', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder', autospec=True) def test_find_subscriptions_in_vm_with_msi_user_assigned_with_object_id(self, mock_subscription_finder, mock_get_client_class, diff --git a/src/azure-cli-core/azure/cli/core/tests/test_profile_v2016_06_01.py b/src/azure-cli-core/azure/cli/core/tests/test_profile_v2016_06_01.py index 5f84985d241..d4cf59042cd 100644 --- a/src/azure-cli-core/azure/cli/core/tests/test_profile_v2016_06_01.py +++ b/src/azure-cli-core/azure/cli/core/tests/test_profile_v2016_06_01.py @@ -571,7 +571,7 @@ def test_get_login_credentials_aux_subscriptions(self, mock_get_token, mock_read self.assertEqual(mock_get_token.call_count, 2) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_system_assigned(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -602,7 +602,7 @@ def test_get_login_credentials_msi_system_assigned(self, mock_msi_auth, mock_rea self.assertTrue(cred.token_read_count) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_user_assigned_with_client_id(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -633,7 +633,7 @@ def test_get_login_credentials_msi_user_assigned_with_client_id(self, mock_msi_a self.assertTrue(cred.client_id, test_client_id) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_user_assigned_with_object_id(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -664,7 +664,7 @@ def test_get_login_credentials_msi_user_assigned_with_object_id(self, mock_msi_a self.assertTrue(cred.object_id, test_object_id) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_login_credentials_msi_user_assigned_with_res_id(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -753,7 +753,7 @@ def test_get_raw_token_for_sp(self, mock_get_token, mock_read_cred_file): self.assertEqual(tenant, self.tenant_id) @mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) def test_get_raw_token_msi_system_assigned(self, mock_msi_auth, mock_read_cred_file): mock_read_cred_file.return_value = [] @@ -899,7 +899,7 @@ def test_find_subscriptions_thru_username_non_password(self, mock_auth_context): # assert self.assertEqual([], subs) - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) @mock.patch('azure.cli.core.profiles._shared.get_client_class', autospec=True) @mock.patch('azure.cli.core._profile._get_cloud_console_token_endpoint', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder', autospec=True) @@ -1048,7 +1048,7 @@ def __init__(self, *args, **kwargs): self.assertEqual(s['id'], self.id1.split('/')[-1]) self.assertEqual(s['tenantId'], '54826b22-38d6-4fb2-bad9-b7b93a3e9c5a') - @mock.patch('azure.cli.core._profile.MSIAuthenticationWrapper', autospec=True) + @mock.patch('azure.cli.core.adal_authentication.MSIAuthenticationWrapper', autospec=True) @mock.patch('azure.cli.core.profiles._shared.get_client_class', autospec=True) @mock.patch('azure.cli.core._profile.SubscriptionFinder', autospec=True) def test_find_subscriptions_in_vm_with_msi_user_assigned_with_object_id(self, mock_subscription_finder, mock_get_client_class,