Skip to content
Merged
5 changes: 0 additions & 5 deletions src/azure-cli/azure/cli/command_modules/keyvault/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ class CLIJsonWebKeyCurveName(str, Enum):
with self.argument_context('keyvault key restore', arg_group='Restoring keys from storage account') as c:
c.argument('token', options_list=['--storage-container-SAS-token', '-t'],
help='The SAS token pointing to an Azure Blob storage container')
c.argument('backup_folder', help='Name of the blob container which contains the backup')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we need to sync with SDK team to understand why the breaking change. this IS a breaking change to CLI users. thanks for the email.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how about we keep this param in CLI for a while with deprecation message, explain it will be auto retrieved from storage-resource-uri in future, the param itself just do nothing.

@laiapat laiapat Feb 23, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I sent back an email a little while ago, but to make sure I state it here: there don't need to be any breaking changes in the CLI, if you don't think it makes sense to introduce any. The SDK now accepts a backup_storage_url that's a concatenation of what used to be provided as backup_storage_uri and folder_name, but this full URL can be generated by the CLI. This is the PR where the change was made on the SDK's side, for reference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @yungezz @mccoyp, we don't need command interface breaking change now.

c.argument('key_name', options_list=['--name', '-n'],
help='Name of the key. (Only for restoring from storage account)')

Expand Down Expand Up @@ -518,10 +517,6 @@ class CLIJsonWebKeyCurveName(str, Enum):
c.argument('token', options_list=['--storage-container-SAS-token', '-t'], required=True,
help='The SAS token pointing to an Azure Blob storage container')

with self.argument_context('keyvault restore start') as c:
c.argument('folder_to_restore', options_list=['--backup-folder'],
help='Name of the blob container which contains the backup')

with self.argument_context('keyvault restore start', arg_group='Storage Id') as c:
c.extra('storage_resource_uri', required=False,
help='Azure Blob storage container Uri. If specified all other \'Storage Id\' '
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ def load_command_table(self, _):
with self.command_group('keyvault backup', data_backup_entity.command_type,
is_preview=True) as g:
g.keyvault_custom('start', 'full_backup',
doc_string_source=data_backup_entity.operations_docs_tmpl.format('begin_full_backup'))
doc_string_source=data_backup_entity.operations_docs_tmpl.format('begin_backup'))

with self.command_group('keyvault restore', data_backup_entity.command_type,
is_preview=True) as g:
g.keyvault_custom('start', 'full_restore',
doc_string_source=data_backup_entity.operations_docs_tmpl.format('begin_full_restore'))
doc_string_source=data_backup_entity.operations_docs_tmpl.format('begin_restore'))

with self.command_group('keyvault security-domain', private_data_entity.command_type,
is_preview=True) as g:
Expand Down
54 changes: 39 additions & 15 deletions src/azure-cli/azure/cli/command_modules/keyvault/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -1091,9 +1091,9 @@ def backup_key(client, file_path, vault_base_url=None,
def restore_key(cmd, client, file_path=None, vault_base_url=None, hsm_name=None,
identifier=None, storage_resource_uri=None, # pylint: disable=unused-argument
storage_account_name=None, blob_container_name=None,
token=None, backup_folder=None, key_name=None, no_wait=False):
token=None, key_name=None, no_wait=False):
if file_path:
if any([storage_account_name, blob_container_name, token, backup_folder]):
if any([storage_account_name, blob_container_name, token]):
raise MutuallyExclusiveArgumentError('Do not use --file/-f with any of --storage-account-name/'
'--blob-container-name/--storage-container-SAS-token/--backup-folder')
if key_name:
Expand All @@ -1102,7 +1102,7 @@ def restore_key(cmd, client, file_path=None, vault_base_url=None, hsm_name=None,
if no_wait:
raise MutuallyExclusiveArgumentError('Please do not specify --no-wait when using --file/-f')

if not file_path and not any([storage_account_name, blob_container_name, token, backup_folder]):
if not file_path and not any([storage_account_name, blob_container_name, token]):
raise RequiredArgumentMissingError('Please specify --file/-f or --storage-account-name & '
'--blob-container-name & --storage-container-SAS-token & --backup-folder')

Expand All @@ -1115,8 +1115,6 @@ def restore_key(cmd, client, file_path=None, vault_base_url=None, hsm_name=None,

if not token:
raise RequiredArgumentMissingError('Please specify --storage-container-SAS-token/-t')
if not backup_folder:
raise RequiredArgumentMissingError('Please specify --backup-folder')

storage_account_parameters_check(storage_resource_uri, storage_account_name, blob_container_name)
if not storage_resource_uri:
Expand All @@ -1127,9 +1125,8 @@ def restore_key(cmd, client, file_path=None, vault_base_url=None, hsm_name=None,
ResourceType.DATA_KEYVAULT_ADMINISTRATION_BACKUP)(cmd.cli_ctx, {'hsm_name': hsm_name})
return sdk_no_wait(
no_wait, backup_client.begin_selective_restore,
blob_storage_uri=storage_resource_uri,
folder_url=storage_resource_uri,
sas_token=token,
folder_name=backup_folder,
key_name=key_name
)

Expand Down Expand Up @@ -1819,14 +1816,14 @@ def _resolve_role_id(client, role, scope):
else:
all_roles = list_role_definitions(client, scope=scope)
for _role in all_roles:
if getattr(_role, 'role_name', None) == role:
role_id = _role.id
if _role.get('roleName', None) == role:
role_id = _role['id']
break
return role_id


def _get_role_dics(role_defs):
return {i.id: getattr(i, 'role_name', None) for i in role_defs}
return {i['id']: i.get('roleName', None) for i in role_defs}


def _get_principal_dics(cli_ctx, role_assignments):
Expand All @@ -1849,7 +1846,7 @@ def _get_principal_dics(cli_ctx, role_assignments):

def _reconstruct_role_assignment(role_dics, principal_dics, role_assignment):
ret = {
'id': role_assignment.assignment_id,
'id': role_assignment.role_assignment_id,
'name': role_assignment.name,
'scope': role_assignment.scope,
'type': role_assignment.type
Expand Down Expand Up @@ -2007,12 +2004,39 @@ def list_role_assignments(cmd, client, scope=None, assignee=None, role=None, ass
return ret


def _reconstruct_role_definition(role_definition):
ret_permissions = []
permissions = role_definition.permissions
for permission in permissions:
ret_permissions.append({
'actions': permission.allowed_actions,
'notActions': permission.denied_actions,
'dataActions': permission.allowed_data_actions,
'notDataActions': permission.denied_data_actions
})

ret = {
'assignableScopes': role_definition.assignable_scopes,
'description': role_definition.description,
'id': role_definition.id,
'name': role_definition.name,
'permissions': ret_permissions,
'roleName': role_definition.role_name,
'roleType': role_definition.role_type,
'type': role_definition.type,
}

return ret


def list_role_definitions(client, scope=None, hsm_name=None): # pylint: disable=unused-argument
""" List role definitions. """
query_scope = scope
if query_scope is None:
query_scope = ''
return client.list_role_definitions(role_scope=query_scope)
role_definitions = client.list_role_definitions(role_scope=query_scope)
return [_reconstruct_role_definition(role) for role in role_definitions]
# return client.list_role_definitions(role_scope=query_scope)
# endregion


Expand Down Expand Up @@ -2042,16 +2066,16 @@ def full_backup(cmd, client, token, storage_resource_uri=None, storage_account_n
if not storage_resource_uri:
storage_resource_uri = construct_storage_uri(
cmd.cli_ctx.cloud.suffixes.storage_endpoint, storage_account_name, blob_container_name)
return client.begin_full_backup(storage_resource_uri, token)
return client.begin_backup(storage_resource_uri, token)


def full_restore(cmd, client, token, folder_to_restore, storage_resource_uri=None, storage_account_name=None,
def full_restore(cmd, client, token, storage_resource_uri=None, storage_account_name=None,
blob_container_name=None, hsm_name=None): # pylint: disable=unused-argument
storage_account_parameters_check(storage_resource_uri, storage_account_name, blob_container_name)
if not storage_resource_uri:
storage_resource_uri = construct_storage_uri(
cmd.cli_ctx.cloud.suffixes.storage_endpoint, storage_account_name, blob_container_name)
return client.begin_full_restore(storage_resource_uri, token, folder_to_restore)
return client.begin_restore(storage_resource_uri, token)
# endregion


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,11 @@ def test_keyvault_hsm_selective_key_restore(self):
checks=[
self.check('status', 'Succeeded'),
self.exists('startTime'),
self.exists('id'),
self.exists('azureStorageBlobContainerUri')
self.exists('jobId'),
self.exists('folderUrl')
]).get_output_in_json()

self.kwargs['backup_folder'] = backup_data['azureStorageBlobContainerUri'].split('/')[-1]
self.kwargs['backup_folder'] = backup_data['folderUrl'].split('/')[-1]

self.cmd('az keyvault key list --hsm-name {hsm_name}', checks=self.check('length(@)', 1))
self.cmd('az keyvault key delete -n {key_name} --hsm-name {hsm_name}')
Expand All @@ -445,7 +445,6 @@ def test_keyvault_hsm_selective_key_restore(self):
self.cmd('az keyvault key restore --hsm-name {hsm_name} --blob-container-name {blob} '
'--storage-account-name {storage_account} '
'--storage-container-SAS-token "{sas}" '
'--backup-folder "{backup_folder}" '
'--name {key_name}', checks=self.check('status', 'Succeeded'))

self.cmd('az keyvault key list --hsm-name {hsm_name}', checks=[
Expand Down Expand Up @@ -481,8 +480,8 @@ def test_keyvault_hsm_full_backup_restore(self):
checks=[
self.check('status', 'Succeeded'),
self.exists('startTime'),
self.exists('id'),
self.exists('azureStorageBlobContainerUri')
self.exists('jobId'),
self.exists('folderUrl')
])

backup_data = self.cmd('az keyvault backup start --hsm-name {hsm_name} --blob-container-name {blob} '
Expand All @@ -491,19 +490,18 @@ def test_keyvault_hsm_full_backup_restore(self):
checks=[
self.check('status', 'Succeeded'),
self.exists('startTime'),
self.exists('id'),
self.exists('azureStorageBlobContainerUri')
self.exists('jobId'),
self.exists('folderUrl')
]).get_output_in_json()

self.kwargs['backup_folder'] = backup_data['azureStorageBlobContainerUri'].split('/')[-1]
self.kwargs['backup_folder'] = backup_data['folderUrl'].split('/')[-1]
self.cmd('az keyvault restore start --hsm-name {hsm_name} --blob-container-name {blob} '
'--storage-account-name {storage_account} '
'--storage-container-SAS-token "{sas}" '
'--backup-folder "{backup_folder}"',
'--storage-container-SAS-token "{sas}" ',
checks=[
self.check('status', 'Succeeded'),
self.exists('startTime'),
self.exists('id')
self.exists('jobId')
])


Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/requirements.py3.Darwin.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ azure-datalake-store==0.0.49
azure-functions-devops-build==0.0.22
azure-graphrbac==0.60.0
azure-keyvault==1.1.0
azure-keyvault-administration==4.0.0b1
azure-keyvault-administration==4.0.0b3
azure-mgmt-advisor==2.0.1
azure-mgmt-apimanagement==0.2.0
azure-mgmt-appconfiguration==1.0.1
Expand Down Expand Up @@ -106,7 +106,7 @@ jsmin==2.2.2
knack==0.8.0rc2
MarkupSafe==1.1.1
mock==4.0.2
msrest==0.6.18
msrest==0.6.21
msrestazure==0.6.3
oauthlib==3.0.1
paramiko==2.6.0
Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/requirements.py3.Linux.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ azure-datalake-store==0.0.49
azure-functions-devops-build==0.0.22
azure-graphrbac==0.60.0
azure-keyvault==1.1.0
azure-keyvault-administration==4.0.0b1
azure-keyvault-administration==4.0.0b3
azure-mgmt-advisor==2.0.1
azure-mgmt-apimanagement==0.2.0
azure-mgmt-appconfiguration==1.0.1
Expand Down Expand Up @@ -106,7 +106,7 @@ jsmin==2.2.2
knack==0.8.0rc2
MarkupSafe==1.1.1
mock==4.0.2
msrest==0.6.18
msrest==0.6.21
msrestazure==0.6.3
oauthlib==3.0.1
paramiko==2.6.0
Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/requirements.py3.windows.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ azure-datalake-store==0.0.49
azure-functions-devops-build==0.0.22
azure-graphrbac==0.60.0
azure-keyvault==1.1.0
azure-keyvault-administration==4.0.0b1
azure-keyvault-administration==4.0.0b3
azure-mgmt-advisor==2.0.1
azure-mgmt-apimanagement==0.2.0
azure-mgmt-appconfiguration==1.0.1
Expand Down Expand Up @@ -105,7 +105,7 @@ jsmin==2.2.2
knack==0.8.0rc2
MarkupSafe==1.1.1
mock==4.0.2
msrest==0.6.18
msrest==0.6.21
msrestazure==0.6.3
oauthlib==3.0.1
paramiko==2.6.0
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
'azure-functions-devops-build~=0.0.22',
'azure-graphrbac~=0.60.0',
'azure-keyvault~=1.1.0',
'azure-keyvault-administration==4.0.0b1',
'azure-keyvault-administration==4.0.0b3',
'azure-mgmt-advisor>=2.0.1,<3.0.0',
'azure-mgmt-apimanagement~=0.2.0',
'azure-mgmt-applicationinsights~=0.1.1',
Expand Down