[AKS] Fix snapshot not resolved according to the subscriptions field in the --snapshot-id option

src/azure-cli/azure/cli/command_modules/acs/_client_factory.py

@@ -9,9 +9,14 @@
 from azure.mgmt.msi import ManagedServiceIdentityClient
 from knack.util import CLIError
 
+# Note: cf_xxx, as the client_factory option value of a command group at command declaration, it should ignore
+# parameters other than cli_ctx; get_xxx_client is used as the client of other services in the command implementation,
+# and usually accepts subscription_id as a parameter to reconfigure the subscription when sending the request
 
-def get_container_service_client(cli_ctx, **_):
-    return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_CONTAINERSERVICE)
+
+# container service clients
+def get_container_service_client(cli_ctx, subscription_id=None):
+    return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_CONTAINERSERVICE, subscription_id=subscription_id)
 
 
 def cf_container_services(cli_ctx, *_):
@@ -30,21 +35,26 @@ def cf_snapshots(cli_ctx, *_):
     return get_container_service_client(cli_ctx).snapshots
 
 
-def cf_compute_service(cli_ctx, *_):
+def get_snapshots_client(cli_ctx, subscription_id=None):
+    return get_container_service_client(cli_ctx, subscription_id=subscription_id).snapshots
+
+
+# dependent clients
+def get_compute_client(cli_ctx, *_):
     return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_COMPUTE)
 
 
-def cf_resource_groups(cli_ctx, subscription_id=None):
+def get_resource_groups_client(cli_ctx, subscription_id=None):
     return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_RESOURCE_RESOURCES,
                                    subscription_id=subscription_id).resource_groups
 
 
-def cf_resources(cli_ctx, subscription_id=None):
+def get_resources_client(cli_ctx, subscription_id=None):
     return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_RESOURCE_RESOURCES,
                                    subscription_id=subscription_id).resources
 
 
-def cf_container_registry_service(cli_ctx, subscription_id=None):
+def get_container_registry_client(cli_ctx, subscription_id=None):
     return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_CONTAINERREGISTRY,
                                    subscription_id=subscription_id)
 
@@ -62,7 +72,7 @@ def get_auth_management_client(cli_ctx, scope=None, **_):
     return get_mgmt_service_client(cli_ctx, ResourceType.MGMT_AUTHORIZATION, subscription_id=subscription_id)
 
 
-def cf_graph_client(cli_ctx):
+def get_graph_client(cli_ctx):
     from azure.cli.command_modules.role import graph_client_factory
     return graph_client_factory(cli_ctx)
 

src/azure-cli/azure/cli/command_modules/acs/_completers.py

@@ -16,7 +16,7 @@ def get_k8s_upgrades_completion_list(cmd, prefix, namespace, **kwargs):  # pylin
 
 
 def get_k8s_upgrades(cli_ctx, resource_group, name):
-    from ._client_factory import cf_managed_clusters
+    from azure.cli.command_modules.acs._client_factory import cf_managed_clusters
 
     results = cf_managed_clusters(cli_ctx).get_upgrade_profile(resource_group, name).as_dict()
     return results['control_plane_profile']['upgrades']
@@ -31,7 +31,7 @@ def get_k8s_versions_completion_list(cmd, prefix, namespace, **kwargs):  # pylin
 
 def get_k8s_versions(cli_ctx, location):
     """Return a list of Kubernetes versions available for a new cluster."""
-    from ._client_factory import cf_container_services
+    from azure.cli.command_modules.acs._client_factory import cf_container_services
     from jmespath import search
 
     results = cf_container_services(cli_ctx).list_orchestrators(location, resource_type='managedClusters').as_dict()
@@ -50,9 +50,9 @@ def get_vm_size_completion_list(cmd, prefix, namespace, **kwargs):  # pylint: di
 
 
 def get_vm_sizes(cli_ctx, location):
-    from ._client_factory import cf_compute_service
+    from azure.cli.command_modules.acs._client_factory import get_compute_client
 
-    return cf_compute_service(cli_ctx).virtual_machine_sizes.list(location)
+    return get_compute_client(cli_ctx).virtual_machine_sizes.list(location)
 
 
 def _get_location(cli_ctx, namespace):
@@ -71,11 +71,11 @@ def _get_location(cli_ctx, namespace):
 
 
 def _get_location_from_resource_group(cli_ctx, resource_group_name):
-    from ._client_factory import cf_resource_groups
+    from azure.cli.command_modules.acs._client_factory import get_resource_groups_client
     from msrestazure.azure_exceptions import CloudError
 
     try:
-        rg = cf_resource_groups(cli_ctx).get(resource_group_name)
+        rg = get_resource_groups_client(cli_ctx).get(resource_group_name)
         return rg.location
     except CloudError as err:
         # Print a warning if the user hit [TAB] but the `--resource-group` argument was incorrect.

src/azure-cli/azure/cli/command_modules/acs/_graph.py

@@ -3,15 +3,15 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-from azure.cli.command_modules.acs._client_factory import cf_graph_client
+from azure.cli.command_modules.acs._client_factory import get_graph_client
 from azure.cli.core.azclierror import AzCLIError, RequiredArgumentMissingError
 from knack.log import get_logger
 
 logger = get_logger(__name__)
 
 
 def resolve_object_id(cli_ctx, assignee):
-    client = cf_graph_client(cli_ctx)
+    client = get_graph_client(cli_ctx)
     result = None
     if assignee is None:
         raise AzCLIError('Inputted parameter "assignee" is None.')

src/azure-cli/azure/cli/command_modules/acs/_helpers.py

@@ -6,7 +6,7 @@
 import re
 from typing import Any, List, TypeVar
 
-from azure.cli.command_modules.acs._client_factory import cf_snapshots, get_msi_client
+from azure.cli.command_modules.acs._client_factory import get_snapshots_client, get_msi_client
 from azure.cli.core.azclierror import (
     AzureInternalError,
     AzureResponseError,
@@ -143,14 +143,15 @@ def get_snapshot_by_snapshot_id(cli_ctx, snapshot_id):
     snapshot_id = snapshot_id.lower()
     match = _re_snapshot_resource_id.search(snapshot_id)
     if match:
+        subscription_id = match.group(1)
         resource_group_name = match.group(2)
         snapshot_name = match.group(3)
-        return get_snapshot(cli_ctx, resource_group_name, snapshot_name)
+        return get_snapshot(cli_ctx, subscription_id, resource_group_name, snapshot_name)
     raise InvalidArgumentValueError("Cannot parse snapshot name from provided resource id '{}'.".format(snapshot_id))
 
 
-def get_snapshot(cli_ctx, resource_group_name, snapshot_name):
-    snapshot_client = cf_snapshots(cli_ctx)
+def get_snapshot(cli_ctx, subscription_id, resource_group_name, snapshot_name):
+    snapshot_client = get_snapshots_client(cli_ctx, subscription_id=subscription_id)
     try:
         snapshot = snapshot_client.get(resource_group_name, snapshot_name)
     # track 2 sdk raise exception from azure.core.exceptions

src/azure-cli/azure/cli/command_modules/acs/_resourcegroup.py

@@ -5,11 +5,11 @@
 
 from azure.cli.core.azclierror import AzCLIError
 
-from ._client_factory import cf_resource_groups
+from azure.cli.command_modules.acs._client_factory import get_resource_groups_client
 
 
 def get_rg_location(ctx, resource_group_name, subscription_id=None):
-    groups = cf_resource_groups(ctx, subscription_id=subscription_id)
+    groups = get_resource_groups_client(ctx, subscription_id=subscription_id)
     # Just do the get, we don't need the result, it will error out if the group doesn't exist.
     rg = groups.get(resource_group_name)
     if rg is None:

src/azure-cli/azure/cli/command_modules/acs/_roleassignments.py

@@ -8,7 +8,7 @@
 import uuid
 
 from azure.cli.command_modules.acs._client_factory import (
-    cf_container_registry_service,
+    get_container_registry_client,
     get_auth_management_client,
     get_resource_by_name,
 )
@@ -317,7 +317,7 @@ def ensure_aks_acr(cmd, assignee, acr_name_or_id, subscription_id, detach=False,
     if is_valid_resource_id(acr_name_or_id):
         try:
             parsed_registry = parse_resource_id(acr_name_or_id)
-            acr_client = cf_container_registry_service(cmd.cli_ctx, subscription_id=parsed_registry["subscription"])
+            acr_client = get_container_registry_client(cmd.cli_ctx, subscription_id=parsed_registry["subscription"])
             registry = acr_client.registries.get(parsed_registry["resource_group"], parsed_registry["name"])
         except (CloudError, HttpResponseError) as ex:
             raise AzCLIError(ex.message)

src/azure-cli/azure/cli/command_modules/acs/addonconfiguration.py

@@ -5,7 +5,7 @@
 import datetime
 import json
 
-from azure.cli.command_modules.acs._client_factory import cf_resource_groups, cf_resources
+from azure.cli.command_modules.acs._client_factory import get_resource_groups_client, get_resources_client
 from azure.cli.command_modules.acs._consts import (
     CONST_INGRESS_APPGW_ADDON_NAME,
     CONST_INGRESS_APPGW_APPLICATION_GATEWAY_ID,
@@ -186,8 +186,8 @@ def ensure_default_log_analytics_workspace_for_monitoring(
             default_workspace_name,
         )
     )
-    resource_groups = cf_resource_groups(cmd.cli_ctx, subscription_id)
-    resources = cf_resources(cmd.cli_ctx, subscription_id)
+    resource_groups = get_resource_groups_client(cmd.cli_ctx, subscription_id)
+    resources = get_resources_client(cmd.cli_ctx, subscription_id)
 
     # check if default RG exists
     if resource_groups.check_existence(default_workspace_resource_group):
@@ -309,7 +309,7 @@ def ensure_container_insights_for_monitoring(
 
     # region of workspace can be different from region of RG so find the location of the workspace_resource_id
     if not remove_monitoring:
-        resources = cf_resources(cmd.cli_ctx, subscription_id)
+        resources = get_resources_client(cmd.cli_ctx, subscription_id)
         try:
             resource = resources.get_by_id(
                 workspace_resource_id, "2015-11-01-preview"
@@ -561,7 +561,7 @@ def _is_container_insights_solution_exists(cmd, workspace_resource_id):
         "subscription"], parsed["resource_group"], parsed["name"]
     solution_resource_id = "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.OperationsManagement/solutions/ContainerInsights({2})".format(
         subscription_id, resource_group, workspace_name)
-    resources = cf_resources(cmd.cli_ctx, subscription_id)
+    resources = get_resources_client(cmd.cli_ctx, subscription_id)
     for retry_count in range(0, _MAX_RETRY_TIMES):
         try:
             resources.get_by_id(solution_resource_id, '2015-11-01-preview')

src/azure-cli/azure/cli/command_modules/acs/custom.py

@@ -37,7 +37,7 @@
 from azure.cli.command_modules.acs import acs_client, proxy
 from azure.cli.command_modules.acs._client_factory import (
     cf_agent_pools,
-    cf_container_registry_service,
+    get_container_registry_client,
     cf_container_services,
     get_auth_management_client,
     get_graph_rbac_management_client,
@@ -3440,7 +3440,7 @@ def _ensure_aks_acr(cmd,
     if is_valid_resource_id(acr_name_or_id):
         try:
             parsed_registry = parse_resource_id(acr_name_or_id)
-            acr_client = cf_container_registry_service(
+            acr_client = get_container_registry_client(
                 cmd.cli_ctx, subscription_id=parsed_registry['subscription'])
             registry = acr_client.registries.get(
                 parsed_registry['resource_group'], parsed_registry['name'])

src/azure-cli/azure/cli/command_modules/acs/proxy.py

@@ -39,7 +39,7 @@ def _get_proxy_instance():
     if os_platform == 'Darwin':
         return MacProxy()
     if os_platform == 'Windows':
-        from .win_proxy import WinProxy
+        from azure.cli.command_modules.acs.win_proxy import WinProxy
         return WinProxy()
     if os_platform == 'Linux':
         return LinuxProxy()

src/azure-cli/azure/cli/command_modules/acs/tests/hybrid_2020_09_01/test_aks_commands.py

@@ -15,7 +15,7 @@
 from azure.cli.testsdk.checkers import (
     StringContainCheck, StringContainCheckIgnoreCase)
 from azure.cli.command_modules.acs._format import version_to_tuple
-from .recording_processors import KeyReplacer
+from azure.cli.command_modules.acs.tests.hybrid_2020_09_01.recording_processors import KeyReplacer
 
 # flake8: noqa
 

src/azure-cli/azure/cli/command_modules/acs/tests/hybrid_2020_09_01/test_custom.py

@@ -597,9 +597,9 @@ def test_merge_credentials_already_present(self):
         self.assertEqual(merged['current-context'], obj2['current-context'])
 
     @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus')
-    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups', autospec=True)
-    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.cf_resources', autospec=True)
-    def test_update_addons(self, rg_def, cf_resource_groups, cf_resources):
+    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client', autospec=True)
+    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resources_client', autospec=True)
+    def test_update_addons(self, rg_def, get_resource_groups_client, get_resources_client):
         # http_application_routing enabled
         instance = mock.MagicMock()
         instance.addon_profiles = None

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py

@@ -585,9 +585,9 @@ def test_merge_credentials_already_present(self):
         self.assertEqual(merged['current-context'], obj2['current-context'])
 
     @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_rg_location', return_value='eastus')
-    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups', autospec=True)
-    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.cf_resources', autospec=True)
-    def test_update_addons(self, rg_def, cf_resource_groups, cf_resources):
+    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client', autospec=True)
+    @mock.patch('azure.cli.command_modules.acs.addonconfiguration.get_resources_client', autospec=True)
+    def test_update_addons(self, rg_def, get_resource_groups_client, get_resources_client):
         # http_application_routing enabled
         instance = mock.MagicMock()
         instance.addon_profiles = None

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_helpers.py

@@ -199,29 +199,29 @@ def test_get_snapshot_by_snapshot_id(self):
                 "/subscriptions/test_sub/resourcegroups/test_rg/providers/microsoft.containerservice/snapshots/test_snapshot",
             )
             self.assertEqual(snapshot, mock_snapshot)
-            mock_get_snapshot.assert_called_once_with("mock_cli_ctx", "test_rg", "test_snapshot")
+            mock_get_snapshot.assert_called_once_with("mock_cli_ctx", "test_sub", "test_rg", "test_snapshot")
 
     def test_get_snapshot(self):
         mock_snapshot = Mock()
         mock_snapshot_operations = Mock(get=Mock(return_value=mock_snapshot))
-        with patch("azure.cli.command_modules.acs._helpers.cf_snapshots", return_value=mock_snapshot_operations):
-            snapshot = get_snapshot("mock_cli_ctx", "mock_rg", "mock_snapshot_name")
+        with patch("azure.cli.command_modules.acs._helpers.get_snapshots_client", return_value=mock_snapshot_operations):
+            snapshot = get_snapshot("mock_cli_ctx", "test_sub", "mock_rg", "mock_snapshot_name")
             self.assertEqual(snapshot, mock_snapshot)
 
         mock_snapshot_operations_2 = Mock(get=Mock(side_effect=AzureError("mock snapshot was not found")))
         with patch(
-            "azure.cli.command_modules.acs._helpers.cf_snapshots", return_value=mock_snapshot_operations_2
+            "azure.cli.command_modules.acs._helpers.get_snapshots_client", return_value=mock_snapshot_operations_2
         ), self.assertRaises(ResourceNotFoundError):
-            get_snapshot("mock_cli_ctx", "mock_rg", "mock_snapshot_name")
+            get_snapshot("mock_cli_ctx", "test_sub", "mock_rg", "mock_snapshot_name")
 
         http_response_error = HttpResponseError()
         http_response_error.status_code = 400
         http_response_error.message = "test_error_msg"
         mock_snapshot_operations_3 = Mock(get=Mock(side_effect=http_response_error))
         with patch(
-            "azure.cli.command_modules.acs._helpers.cf_snapshots", return_value=mock_snapshot_operations_3
+            "azure.cli.command_modules.acs._helpers.get_snapshots_client", return_value=mock_snapshot_operations_3
         ), self.assertRaises(BadRequestError):
-            get_snapshot("mock_cli_ctx", "mock_rg", "mock_snapshot_name")
+            get_snapshot("mock_cli_ctx", "test_sub", "mock_rg", "mock_snapshot_name")
 
 
 class GetUserAssignedIdentityTestCase(unittest.TestCase):

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py

@@ -1999,23 +1999,23 @@ def test_get_workspace_resource_id(self):
             DecoratorMode.CREATE,
         )
         ctx_3.set_intermediate("subscription_id", "test_subscription_id")
-        cf_resource_groups = Mock(check_existence=Mock(return_value=False))
+        get_resource_groups_client = Mock(check_existence=Mock(return_value=False))
         result = Mock(id="test_workspace_resource_id")
         async_poller = Mock(result=Mock(return_value=result), done=Mock(return_value=True))
-        cf_resources = Mock(begin_create_or_update_by_id=Mock(return_value=async_poller))
+        get_resources_client = Mock(begin_create_or_update_by_id=Mock(return_value=async_poller))
         with patch(
             "azure.cli.command_modules.acs.addonconfiguration.get_rg_location",
             return_value="test_location",
         ), patch(
-            "azure.cli.command_modules.acs.addonconfiguration.cf_resource_groups",
-            return_value=cf_resource_groups,
+            "azure.cli.command_modules.acs.addonconfiguration.get_resource_groups_client",
+            return_value=get_resource_groups_client,
         ), patch(
-            "azure.cli.command_modules.acs.addonconfiguration.cf_resources",
-            return_value=cf_resources,
+            "azure.cli.command_modules.acs.addonconfiguration.get_resources_client",
+            return_value=get_resources_client,
         ):
             self.assertEqual(ctx_3.get_workspace_resource_id(), "/test_workspace_resource_id")
-        cf_resource_groups.check_existence.assert_called_once_with("DefaultResourceGroup-EUS")
-        cf_resource_groups.create_or_update.assert_called_once_with("DefaultResourceGroup-EUS", {"location": "eastus"})
+        get_resource_groups_client.check_existence.assert_called_once_with("DefaultResourceGroup-EUS")
+        get_resource_groups_client.create_or_update.assert_called_once_with("DefaultResourceGroup-EUS", {"location": "eastus"})
         default_workspace_resource_id = (
             "/subscriptions/{0}/resourceGroups/{1}/providers/Microsoft.OperationalInsights/workspaces/{2}".format(
                 "test_subscription_id",
@@ -2024,7 +2024,7 @@ def test_get_workspace_resource_id(self):
             )
         )
         # the return values are func_name, args and kwargs
-        _, args, _ = cf_resources.begin_create_or_update_by_id.mock_calls[0]
+        _, args, _ = get_resources_client.begin_create_or_update_by_id.mock_calls[0]
         # not interested in mocking generic_resource, so we only check the first two args
         self.assertEqual(args[:2], (default_workspace_resource_id, "2015-11-01-preview"))
 

src/azure-cli/azure/cli/command_modules/acs/win_proxy.py

@@ -6,7 +6,7 @@
 from ctypes import POINTER, Structure, Union, byref, c_ulong, create_unicode_buffer, sizeof, windll
 from ctypes.wintypes import BOOL, DWORD, FILETIME, LPVOID, WCHAR
 
-from .proxy import Proxy
+from azure.cli.command_modules.acs.proxy import Proxy
 
 LPWSTR = POINTER(WCHAR)
 HINTERNET = LPVOID