diff --git a/src/azure-cli/HISTORY.rst b/src/azure-cli/HISTORY.rst index f091f159e11..523e674b63f 100644 --- a/src/azure-cli/HISTORY.rst +++ b/src/azure-cli/HISTORY.rst @@ -121,6 +121,7 @@ Release History **Compute** +* `az vm boot-diagnostics get-boot-log`: Fix `TypeError: 'method' object is not subscriptable` with azure-mgmt-storage 25.0.0 (#33727) * `az vm create/update/show`: Support scheduled events profile via new parameters `--scheduled-events-api-version` and `--enable-all-instance-down` (#33451) * `az vmss create/update/show`: Support scheduled events profile via new parameters `--scheduled-events-api-version` and `--enable-all-instance-down` (#33451) * `az availability-set create/show`: Support scheduled events profile via new parameters `--scheduled-events-api-version` and `--enable-all-instance-down` (#33451) diff --git a/src/azure-cli/azure/cli/command_modules/batchai/custom.py b/src/azure-cli/azure/cli/command_modules/batchai/custom.py index b5c256d1618..2ee4ab4ca8f 100644 --- a/src/azure-cli/azure/cli/command_modules/batchai/custom.py +++ b/src/azure-cli/azure/cli/command_modules/batchai/custom.py @@ -160,14 +160,24 @@ def _get_storage_account_key(cli_ctx, account_name, account_key): raise CLIError('Cannot find "{0}" storage account.'.format(account_name)) resource_group = parse_resource_id(account[0])['resource_group'] keys_list_result = storage_client.storage_accounts.list_keys(resource_group, account_name) - if not keys_list_result or not keys_list_result.keys: + account_keys = _get_account_keys_list(keys_list_result) + if not account_keys: raise CLIError('Cannot find a key for "{0}" storage account.'.format(account_name)) - key_value = None - try: - key_value = keys_list_result.keys_property[0].value - except AttributeError: - key_value = keys_list_result.keys[0].value - return key_value + return account_keys[0].value + + +def _get_account_keys_list(keys_list_result): + """Returns the list of storage account keys from a list_keys result. + + In azure-mgmt-storage 25.0.0 the `keys` attribute was renamed to `keys_property` because the model + now inherits from MutableMapping, where `keys` is a method. + """ + if not keys_list_result: + return None + account_keys = getattr(keys_list_result, 'keys_property', None) + if account_keys is None: + account_keys = keys_list_result.keys + return account_keys def _get_effective_storage_account_name_and_key(cli_ctx, account_name, account_key): diff --git a/src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py b/src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py index ae8cdfe0032..ed4f3f632f0 100644 --- a/src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py +++ b/src/azure-cli/azure/cli/command_modules/batchai/tests/latest/test_batchai_custom.py @@ -57,7 +57,7 @@ def _get_mock_storage_accounts_and_keys(accounts_and_keys): '/subscriptions/000/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/{0}'.format(a), a, Endpoints('https://{0}.file.core.windows.net/'.format(a))) for a in accounts_and_keys.keys()]) - Keys = collections.namedtuple('Keys', 'keys') + Keys = collections.namedtuple('Keys', 'keys_property') Key = collections.namedtuple('Key', 'value') mock_storage_client.storage_accounts.list_keys = MagicMock( side_effect=lambda _, account: Keys([Key(accounts_and_keys.get(account, None))])) diff --git a/src/azure-cli/azure/cli/command_modules/vm/custom.py b/src/azure-cli/azure/cli/command_modules/vm/custom.py index b1b55aab17d..a009f813caa 100644 --- a/src/azure-cli/azure/cli/command_modules/vm/custom.py +++ b/src/azure-cli/azure/cli/command_modules/vm/custom.py @@ -201,10 +201,23 @@ def _get_disk_lun_by_aaz(data_disks): return len(existing_luns) +def _get_account_keys_list(keys_list_result): + """Returns the list of storage account keys from a list_keys result. + + In azure-mgmt-storage 25.0.0 the `keys` attribute was renamed to `keys_property` because the model + now inherits from MutableMapping, where `keys` is a method. + """ + account_keys = getattr(keys_list_result, 'keys_property', None) + if account_keys is None: + account_keys = keys_list_result.keys + return account_keys + + def _get_private_config(cli_ctx, resource_group_name, storage_account): storage_mgmt_client = _get_storage_management_client(cli_ctx) # pylint: disable=no-member - keys = storage_mgmt_client.storage_accounts.list_keys(resource_group_name, storage_account).keys + keys = _get_account_keys_list( + storage_mgmt_client.storage_accounts.list_keys(resource_group_name, storage_account)) private_config = { 'storageAccountName': storage_account, @@ -2300,7 +2313,7 @@ def get_boot_log(cmd, resource_group_name, vm_name): # Get account key keys = storage_mgmt_client.storage_accounts.list_keys(rg, storage_account.name) - blob_client = BlobClient.from_blob_url(blob_url=blob_uri, credential=keys.keys[0].value) + blob_client = BlobClient.from_blob_url(blob_url=blob_uri, credential=_get_account_keys_list(keys)[0].value) # our streamwriter not seekable, so no parallel. downloader = blob_client.download_blob(max_concurrency=1) diff --git a/src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py b/src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py index 0291e22490b..b8cc9928b30 100644 --- a/src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py +++ b/src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_custom_vm_commands.py @@ -195,6 +195,112 @@ class ErrorToExitCommandEarly(Exception): except ErrorToExitCommandEarly: get_sdk_mock.assert_called_with(cli_ctx_mock, ResourceType.DATA_STORAGE_BLOB, '_blob_client#BlobClient') + @mock.patch('azure.cli.command_modules.vm.custom._get_storage_management_client') + @mock.patch('azure.cli.command_modules.vm.custom.get_instance_view') + @mock.patch('azure.cli.core.profiles.get_sdk', autospec=True) + def test_vm_boot_log_uses_keys_property(self, get_sdk_mock, get_instance_view_mock, + get_storage_management_client_mock): + """Verify get_boot_log uses keys_property (not .keys) for azure-mgmt-storage >= 25.0.0.""" + blob_uri = 'https://mystorage.blob.core.windows.net/bootdiagnostics/vm1/serial.log' + + # VM with instanceView providing a blob URI (custom/non-managed storage path) + get_instance_view_mock.return_value = { + 'instanceView': { + 'bootDiagnostics': { + 'serialConsoleLogBlobUri': blob_uri + } + } + } + + # Storage account that matches the blob URI + storage_account_mock = mock.MagicMock() + storage_account_mock.primary_endpoints.blob = 'https://mystorage.blob.core.windows.net/' + storage_account_mock.id = '/subscriptions/sub1/resourceGroups/rg1/providers/Microsoft.Storage/storageAccounts/mystorage' + storage_account_mock.name = 'mystorage' + + storage_mgmt_mock = mock.MagicMock() + storage_mgmt_mock.storage_accounts.list.return_value = [storage_account_mock] + + # Simulate azure-mgmt-storage >= 25.0.0: keys_property holds the list of keys + key_mock = mock.MagicMock() + key_mock.value = 'fakeaccountkey==' + keys_result_mock = mock.MagicMock(spec=[]) + keys_result_mock.keys_property = [key_mock] + storage_mgmt_mock.storage_accounts.list_keys.return_value = keys_result_mock + + get_storage_management_client_mock.return_value = storage_mgmt_mock + + # BlobClient mock + blob_client_cls_mock = mock.MagicMock() + blob_client_mock = mock.MagicMock() + blob_client_cls_mock.from_blob_url.return_value = blob_client_mock + downloader_mock = mock.MagicMock() + blob_client_mock.download_blob.return_value = downloader_mock + get_sdk_mock.return_value = blob_client_cls_mock + + cmd_mock = mock.MagicMock() + cli_ctx_mock = mock.MagicMock() + cmd_mock.cli_ctx = cli_ctx_mock + + get_boot_log(cmd_mock, 'rg1', 'vm1') + + # Verify from_blob_url was called with credential from keys_property[0].value + blob_client_cls_mock.from_blob_url.assert_called_once_with( + blob_url=blob_uri, credential='fakeaccountkey==' + ) + blob_client_mock.download_blob.assert_called_once_with(max_concurrency=1) + self.assertEqual(downloader_mock.readinto.call_count, 1) + + @mock.patch('azure.cli.command_modules.vm.custom._get_storage_management_client') + @mock.patch('azure.cli.command_modules.vm.custom.get_instance_view') + @mock.patch('azure.cli.core.profiles.get_sdk', autospec=True) + def test_vm_boot_log_falls_back_to_keys(self, get_sdk_mock, get_instance_view_mock, + get_storage_management_client_mock): + """Verify get_boot_log still works with azure-mgmt-storage < 25.0.0, which only exposes .keys.""" + blob_uri = 'https://mystorage.blob.core.windows.net/bootdiagnostics/vm1/serial.log' + + get_instance_view_mock.return_value = { + 'instanceView': { + 'bootDiagnostics': { + 'serialConsoleLogBlobUri': blob_uri + } + } + } + + storage_account_mock = mock.MagicMock() + storage_account_mock.primary_endpoints.blob = 'https://mystorage.blob.core.windows.net/' + storage_account_mock.id = '/subscriptions/sub1/resourceGroups/rg1/providers/Microsoft.Storage/storageAccounts/mystorage' + storage_account_mock.name = 'mystorage' + + storage_mgmt_mock = mock.MagicMock() + storage_mgmt_mock.storage_accounts.list.return_value = [storage_account_mock] + + # Simulate azure-mgmt-storage < 25.0.0: only `keys` is available + key_mock = mock.MagicMock() + key_mock.value = 'legacykey==' + keys_result_mock = mock.MagicMock(spec=['keys']) + keys_result_mock.keys = [key_mock] + storage_mgmt_mock.storage_accounts.list_keys.return_value = keys_result_mock + get_storage_management_client_mock.return_value = storage_mgmt_mock + + blob_client_cls_mock = mock.MagicMock() + blob_client_mock = mock.MagicMock() + blob_client_cls_mock.from_blob_url.return_value = blob_client_mock + downloader_mock = mock.MagicMock() + blob_client_mock.download_blob.return_value = downloader_mock + get_sdk_mock.return_value = blob_client_cls_mock + + cmd_mock = mock.MagicMock() + cmd_mock.cli_ctx = mock.MagicMock() + + get_boot_log(cmd_mock, 'rg1', 'vm1') + + blob_client_cls_mock.from_blob_url.assert_called_once_with( + blob_url=blob_uri, credential='legacykey==' + ) + blob_client_mock.download_blob.assert_called_once_with(max_concurrency=1) + self.assertEqual(downloader_mock.readinto.call_count, 1) + class FakedVM: # pylint: disable=too-few-public-methods def __init__(self, nics=None, disks=None, os_disk=None):