Skip to content

[KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output of az keyvault backup/restore, az keyvault key restore#17011

Merged
houk-ms merged 12 commits intoAzure:devfrom
houk-ms:kv-role-definition
Feb 25, 2021

Conversation

@houk-ms
Copy link
Contributor

@houk-ms houk-ms commented Feb 20, 2021

Description

This PR bumps the azure-keyvault-administration version to 4.0.0b3, so that we can support keyvault role definition feature later.

The SDK azure-keyvault-administration==4.0.0b3 brings many breaking changes, see more details on pypi. Among these breaking changes, some of them only resulting in CLI internal changes, but others make AzureCLI introduce breaking changes too.

In summary, the breaking changes for end users are:

  • az keyvault backup start : azure_storage_blob_container_uri property changes to be folder_url in command output
  • az keyvault backup start, az keyvault restore start, az keyvault key restore: id property changes to be job_id in command output

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


This checklist is used to make sure that common guidelines for a pull request are followed.

@houk-ms houk-ms added the KeyVault az keyvault label Feb 20, 2021
@houk-ms
Copy link
Contributor Author

houk-ms commented Feb 20, 2021

azure-keyvault-administration==4.0.0b3 depends on msrest>=0.6.21. This PR depends on msrest bump PR.

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
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
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.

Copy link
Member

@mccoyp mccoyp Feb 23, 2021

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
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.

@houk-ms
Copy link
Contributor Author

houk-ms commented Feb 24, 2021

Need run live test for full_restore and selective_key_restore

@houk-ms houk-ms changed the title [KeyVault] BREAKING CHANGE: Remove redundant --backup-folder parameter from az keyvault restore start and az keyvault key restore start [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command az keyvault backup start, az keyvault restore start, az keyvault key restore Feb 24, 2021
@houk-ms houk-ms changed the title [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command az keyvault backup start, az keyvault restore start, az keyvault key restore [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output az keyvault backup start, az keyvault restore start, az keyvault key restore Feb 24, 2021
@houk-ms houk-ms changed the title [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output az keyvault backup start, az keyvault restore start, az keyvault key restore [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output of az keyvault backup start, az keyvault restore start, az keyvault key restore Feb 24, 2021
@houk-ms houk-ms changed the title [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output of az keyvault backup start, az keyvault restore start, az keyvault key restore [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output of az keyvault backup/restore start, az keyvault key restore Feb 24, 2021
@houk-ms houk-ms changed the title [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output of az keyvault backup/restore start, az keyvault key restore [KeyVault] BREAKING CHANGE: id changes to be jobId, azureStorageBlobContainerUri changes to be folderUrl in command output of az keyvault backup/restore, az keyvault key restore Feb 24, 2021
@houk-ms
Copy link
Contributor Author

houk-ms commented Feb 24, 2021

Locally runned live test on full_restore and selective_key_restore related scenario tests.

@houk-ms houk-ms merged commit 0a42656 into Azure:dev Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

KeyVault az keyvault

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants