-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[KeyVault] Add --no-wait for command az keyvault security-domain download and --target-operation for command az keyvault security-domain wait
#17263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
435a1ad
f0be844
0b9d92c
5b775d0
22fffd0
c1205ff
000d07e
96553de
f9ad8d5
c17cd75
2fce0ba
8a042c1
0d9c337
d6e55fa
f186f43
15cd31b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2204,14 +2204,21 @@ def get_x5c_as_pem(): | |
| raise ex | ||
|
|
||
|
|
||
| def _wait_security_domain_operation(client, hsm_name, identifier=None): # pylint: disable=unused-argument | ||
| def _wait_security_domain_operation(client, hsm_name, target_operation='upload', identifier=None): # pylint: disable=unused-argument | ||
| retries = 0 | ||
| max_retries = 30 | ||
| wait_second = 5 | ||
| while retries < max_retries: | ||
| try: | ||
| ret = client.upload_pending(vault_base_url=hsm_name) | ||
| if ret and getattr(ret, 'status', None) in ['Succeeded', 'Failed']: | ||
| ret = None | ||
| if target_operation == 'upload': | ||
| ret = client.upload_pending(vault_base_url=hsm_name) | ||
| elif target_operation == 'download': | ||
| ret = client.download_pending(vault_base_url=hsm_name) | ||
|
|
||
| # v7.2-preview and v7.2 will change the upload operation from Sync to Async | ||
| # due to service defects, it returns 'Succeeded' before the change and 'Success' after the change | ||
| if ret and getattr(ret, 'status', None) in ['Succeeded', 'Success', 'Failed']: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. align on case before string comparasion, to avoid service change |
||
| return ret | ||
| except: # pylint: disable=bare-except | ||
| pass | ||
|
|
@@ -2356,14 +2363,14 @@ def security_domain_upload(cmd, client, hsm_name, sd_file, sd_exchange_key, sd_w | |
| if no_wait: | ||
| return retval | ||
|
|
||
| new_retval = _wait_security_domain_operation(client, hsm_name) | ||
| new_retval = _wait_security_domain_operation(client, hsm_name, 'upload') | ||
| if new_retval: | ||
| return new_retval | ||
| return retval | ||
|
|
||
|
|
||
| def security_domain_download(cmd, client, hsm_name, sd_wrapping_keys, security_domain_file, sd_quorum, | ||
| identifier=None, vault_base_url=None): # pylint: disable=unused-argument | ||
| identifier=None, vault_base_url=None, no_wait=False): # pylint: disable=unused-argument | ||
| if os.path.exists(security_domain_file): | ||
| raise CLIError("File named '{}' already exists.".format(security_domain_file)) | ||
|
|
||
|
|
@@ -2406,15 +2413,28 @@ def security_domain_download(cmd, client, hsm_name, sd_wrapping_keys, security_d | |
|
|
||
| certificates.append(sd_jwk) | ||
|
|
||
| # save security-domain backup value to local file | ||
| def _save_to_local_file(file_path, security_domain): | ||
| try: | ||
| with open(file_path, 'w') as f: | ||
| f.write(security_domain.value) | ||
| except Exception as ex: # pylint: disable=bare-except | ||
| if os.path.isfile(file_path): | ||
| os.remove(file_path) | ||
| from azure.cli.core.azclierror import FileOperationError | ||
| raise FileOperationError(str(ex)) | ||
|
|
||
| ret = client.download( | ||
| vault_base_url=hsm_name or vault_base_url, | ||
| certificates=CertificateSet(certificates=certificates, required=sd_quorum) | ||
| ) | ||
|
|
||
| try: | ||
| with open(security_domain_file, 'w') as f: | ||
| f.write(ret.value) | ||
| except: # pylint: disable=bare-except | ||
| if os.path.isfile(security_domain_file): | ||
| os.remove(security_domain_file) | ||
| if not no_wait: | ||
| polling_ret = _wait_security_domain_operation(client, hsm_name, 'download') | ||
| # Due to service defect, status could be 'Success' or 'Succeeded' when it succeeded | ||
| if polling_ret and getattr(polling_ret, 'status', None) != 'Failed': | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pls lower/upper case then compare on string
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer we rely on service in avoid of risks in possible type convertion. |
||
| _save_to_local_file(security_domain_file, ret) | ||
| return polling_ret | ||
|
|
||
| _save_to_local_file(security_domain_file, ret) | ||
| # endregion | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| # pylint: skip-file | ||
| # flake8: noqa | ||
| import json | ||
| import time | ||
|
|
||
| from msrest.service_client import SDKClient | ||
| from msrest import Serializer, Deserializer | ||
|
|
@@ -114,12 +115,16 @@ def download(self, vault_base_url, certificates, custom_headers=None, raw=False, | |
| response = self._client.send( | ||
| request, header_parameters, body_content, stream=False, **operation_config) | ||
|
|
||
| if response.status_code not in [200]: | ||
| # v7.2-preview and v7.2 will introduce a breaking change to make the operation change from Sync to Async | ||
| # 200: for compatability of response before the change (Sync Operation) | ||
| # 202: for the support of new response after the change (Async Operation) | ||
| if response.status_code not in [200, 202]: | ||
| raise models.KeyVaultErrorException(self._deserialize, response) | ||
|
|
||
| deserialized = None | ||
|
|
||
| if response.status_code == 200: | ||
| # for both old response and new response | ||
| if response.status_code in [200, 202]: | ||
| deserialized = self._deserialize('SecurityDomainObject', response) | ||
|
|
||
| if raw: | ||
|
|
@@ -129,6 +134,58 @@ def download(self, vault_base_url, certificates, custom_headers=None, raw=False, | |
| return deserialized | ||
| download.metadata = {'url': '/securitydomain/download'} | ||
|
|
||
| def download_pending(self, vault_base_url, custom_headers=None, raw=False, **operation_config): | ||
| """Get Security domain upload operation status. | ||
| :param vault_base_url: The vault name, for example https://myvault.vault.azure.net. | ||
| :type vault_base_url: str | ||
| :keyword callable cls: A custom type or function that will be passed the direct response | ||
| :return: SecurityDomainOperationStatus, or the result of cls(response) | ||
| :rtype: ~key_vault_client.models.SecurityDomainOperationStatus | ||
| :raises: ~azure.core.exceptions.HttpResponseError | ||
| """ | ||
|
|
||
| # Construct URL | ||
| url = self.upload_pending.metadata['url'] | ||
| path_format_arguments = { | ||
| 'vaultBaseUrl': self._serialize.url("vault_base_url", vault_base_url, 'str', skip_quote=True) | ||
| } | ||
| url = self._client.format_url(url, **path_format_arguments) | ||
|
|
||
| # Construct parameters | ||
| query_parameters = {} | ||
| query_parameters['api-version'] = self._serialize.query("self.api_version", self.api_version, 'str') | ||
|
|
||
| # Construct headers | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [thumb up]! |
||
| header_parameters = {} | ||
| header_parameters['Content-Type'] = 'application/json; charset=utf-8' | ||
| if self.config.generate_client_request_id: | ||
| header_parameters['x-ms-client-request-id'] = str(uuid.uuid1()) | ||
| if custom_headers: | ||
| header_parameters.update(custom_headers) | ||
| if self.config.accept_language is not None: | ||
| header_parameters['accept-language'] = self._serialize.header("self.config.accept_language", | ||
| self.config.accept_language, 'str') | ||
|
|
||
| # Construct and send request | ||
| request = self._client.get(url, query_parameters) | ||
| response = self._client.send( | ||
| request, header_parameters, stream=False, **operation_config) | ||
|
|
||
| if response.status_code not in [200]: | ||
| raise models.KeyVaultErrorException(self._deserialize, response) | ||
|
|
||
| deserialized = None | ||
|
|
||
| if response.status_code == 200: | ||
| deserialized = self._deserialize('SecurityDomainOperationStatus', response) | ||
|
|
||
| if raw: | ||
| client_raw_response = ClientRawResponse(deserialized, response) | ||
| return client_raw_response | ||
|
|
||
| return deserialized | ||
| download_pending.metadata = {'url': '/securitydomain/download/pending'} | ||
|
|
||
| def transfer_key(self, vault_base_url, custom_headers=None, raw=False, **operation_config): | ||
| """Retrieve security domain transfer key. | ||
| :param vault_base_url: The vault name, for example https://myvault.vault.azure.net. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final command will be ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--target_operationdefaults touploadin avoid of breaking change. so either specifying--target-operationor not would work forupload.for
download, we have to specify--target-operation download