Align azure-keyvault-administration API with other SDKs (#15717)

Author: chlowell

sdk/keyvault/azure-keyvault-administration/CHANGELOG.md

@@ -2,17 +2,22 @@
 
 ## 4.0.0b3 (Unreleased)
 ### Breaking Changes
-- Removed `folder_name` parameter from `KeyVaultBackupClient.begin_full_restore`
-  and `.begin_selective_restore`
-- Renamed `BackupOperation.azure_storage_blob_container_uri` to `.blob_storage_url`
-- Renamed `blob_storage_uri` parameters of `KeyVaultBackupClient` methods to
-  `blob_storage_url`
+- Renamed `KeyVaultBackupClient.begin_full_backup()` to `.begin_backup()`
+- Renamed `KeyVaultBackupClient.begin_full_restore()` to `.begin_restore()`
+- Renamed `BackupOperation.azure_storage_blob_container_uri` to `.folder_url`
+- Renamed `id` property of `BackupOperation`, `RestoreOperation`, and
+ `SelectiveKeyRestoreOperation` to `job_id`
+- Renamed `blob_storage_uri` parameters of `KeyVaultBackupClient.begin_restore()`
+  and `.begin_selective_restore()` to `folder_url`
+- Removed redundant `folder_name` parameter from
+  `KeyVaultBackupClient.begin_restore()` and `.begin_selective_restore()` (the
+  `folder_url` parameter contains the folder name)
 - Renamed `KeyVaultPermission` attributes:
   - `actions` -> `allowed_actions`
   - `data_actions` -> `allowed_data_actions`
   - `not_actions` -> `denied_actions`
   - `not_data_actions` -> `denied_data_actions`
-
+- Renamed `KeyVaultRoleAssignment.assignment_id` to `.role_assignment_id`
 
 ## 4.0.0b2 (2020-10-06)
 ### Added

sdk/keyvault/azure-keyvault-administration/README.md

@@ -234,9 +234,9 @@ sas_token = "<your-sas-token>"  # replace with a sas token to your storage accou
 
 # performing a full key backup is a long-running operation. Calling result() on the poller will wait
 # until the backup is completed, then return an object representing the backup operation.
-backup_operation = client.begin_full_backup(blob_storage_url, sas_token).result()
+backup_operation = client.begin_backup(blob_storage_url, sas_token).result()
 
-print(backup_operation.blob_storage_url)
+print(backup_operation.folder_url)
 print(backup_operation.status)
 print(backup_operation.job_id)
 ```
@@ -263,7 +263,7 @@ blob_url = "<your-blob-url>"
 
 # performing a full key restore is a long-running operation. Calling `result()` on the poller will wait
 # until the restore is completed, then return an object representing the restore operation.
-restore_operation = client.begin_full_restore(blob_url, sas_token).result()
+restore_operation = client.begin_restore(blob_url, sas_token).result()
 
 print(restore_operation.status)
 print(restore_operation.job_id)

sdk/keyvault/azure-keyvault-administration/azure/keyvault/administration/_backup_client.py

@@ -7,7 +7,7 @@
 from azure.core.polling.base_polling import LROBasePolling
 
 from ._models import BackupOperation, RestoreOperation, SelectiveKeyRestoreOperation
-from ._internal import KeyVaultClientBase, parse_blob_storage_url
+from ._internal import KeyVaultClientBase, parse_folder_url
 from ._internal.polling import KeyVaultBackupClientPolling
 
 if TYPE_CHECKING:
@@ -25,7 +25,7 @@ class KeyVaultBackupClient(KeyVaultClientBase):
     """
 
     # pylint:disable=protected-access
-    def begin_full_backup(self, blob_storage_url, sas_token, **kwargs):
+    def begin_backup(self, blob_storage_url, sas_token, **kwargs):
         # type: (str, str, **Any) -> LROPoller[BackupOperation]
         """Begin a full backup of the Key Vault.
 
@@ -47,18 +47,18 @@ def begin_full_backup(self, blob_storage_url, sas_token, **kwargs):
             **kwargs
         )
 
-    def begin_full_restore(self, blob_storage_url, sas_token, **kwargs):
+    def begin_restore(self, folder_url, sas_token, **kwargs):
         # type: (str, str, **Any) -> LROPoller[RestoreOperation]
         """Restore a full backup of a Key Vault.
 
-        :param str blob_storage_url: URL of the blob holding the backup. This would be the `blob_storage_url` of a
-            :class:`BackupOperation` returned by :func:`begin_full_backup` or :func:`get_backup_status`, for example
+        :param str folder_url: URL of the blob holding the backup. This would be the `folder_url` of a
+            :class:`BackupOperation` returned by :func:`begin_backup` or :func:`get_backup_status`, for example
             https://<account>.blob.core.windows.net/backup/mhsm-account-2020090117323313
         :param str sas_token: a Shared Access Signature (SAS) token authorizing access to the blob storage resource
         :rtype: ~azure.core.polling.LROPoller[RestoreOperation]
         """
         polling_interval = kwargs.pop("_polling_interval", 5)
-        container_url, folder_name = parse_blob_storage_url(blob_storage_url)
+        container_url, folder_name = parse_folder_url(folder_url)
         sas_parameter = self._models.SASTokenParameter(storage_resource_uri=container_url, token=sas_token)
         restore_details = self._models.RestoreOperationParameters(
             sas_token_parameters=sas_parameter, folder_to_restore=folder_name
@@ -72,20 +72,20 @@ def begin_full_restore(self, blob_storage_url, sas_token, **kwargs):
             **kwargs
         )
 
-    def begin_selective_restore(self, blob_storage_url, sas_token, key_name, **kwargs):
+    def begin_selective_restore(self, folder_url, sas_token, key_name, **kwargs):
         # type: (str, str, str, **Any) -> LROPoller[SelectiveKeyRestoreOperation]
         """Restore a single key from a full Key Vault backup.
 
-        :param str blob_storage_url: URL for the blob storage resource, including the path to the blob holding the
-            backup. This would be the `blob_storage_url` of a :class:`BackupOperation` returned by
-            :func:`begin_full_backup` or :func:`get_backup_status`, for example
+        :param str folder_url: URL for the blob storage resource, including the path to the blob holding the
+            backup. This would be the `folder_url` of a :class:`BackupOperation` returned by
+            :func:`begin_backup` or :func:`get_backup_status`, for example
             https://<account>.blob.core.windows.net/backup/mhsm-account-2020090117323313
         :param str sas_token: a Shared Access Signature (SAS) token authorizing access to the blob storage resource
         :param str key_name: name of the key to restore from the backup
         :rtype: ~azure.core.polling.LROPoller[RestoreOperation]
         """
         polling_interval = kwargs.pop("_polling_interval", 5)
-        container_url, folder_name = parse_blob_storage_url(blob_storage_url)
+        container_url, folder_name = parse_folder_url(folder_url)
         sas_parameter = self._models.SASTokenParameter(storage_resource_uri=container_url, token=sas_token)
         restore_details = self._models.SelectiveKeyRestoreOperationParameters(
             sas_token_parameters=sas_parameter, folder=folder_name

sdk/keyvault/azure-keyvault-administration/azure/keyvault/administration/_internal/__init__.py

@@ -4,11 +4,7 @@
 # ------------------------------------
 from collections import namedtuple
 
-try:
-    import urllib.parse as parse
-except ImportError:
-    # pylint:disable=import-error
-    import urlparse as parse  # type: ignore
+from six.moves.urllib_parse import urlparse
 
 from .challenge_auth_policy import ChallengeAuthPolicy, ChallengeAuthPolicyBase
 from .client_base import KeyVaultClientBase
@@ -29,7 +25,7 @@
 
 def parse_vault_id(url):
     try:
-        parsed_uri = parse.urlparse(url)
+        parsed_uri = urlparse(url)
     except Exception:  # pylint: disable=broad-except
         raise ValueError("'{}' is not not a valid url".format(url))
     if not (parsed_uri.scheme and parsed_uri.hostname):
@@ -51,7 +47,7 @@ def parse_vault_id(url):
 BackupLocation = namedtuple("BackupLocation", ["container_url", "folder_name"])
 
 
-def parse_blob_storage_url(blob_storage_url):
+def parse_folder_url(folder_url):
     # type: (str) -> BackupLocation
     """Parse the blob container URL and folder name from a backup's blob storage URL.
 
@@ -60,12 +56,22 @@ def parse_blob_storage_url(blob_storage_url):
     """
 
     try:
-        folder_name = blob_storage_url.rstrip("/").split("/")[-1]
-        container_url = blob_storage_url[: blob_storage_url.rindex(folder_name) - 1]
+        parsed = urlparse(folder_url)
+
+        # the first segment of the path is the container name
+        stripped_path = parsed.path.strip("/")
+        container = stripped_path.split("/")[0]
+
+        # the rest of the path is the folder name
+        folder_name = stripped_path[len(container) + 1 :]
+
+        # this intentionally discards any SAS token in the URL--methods require the SAS token as a separate parameter
+        container_url = "{}://{}/{}".format(parsed.scheme, parsed.netloc, container)
+
         return BackupLocation(container_url, folder_name)
     except:  # pylint:disable=broad-except
         raise ValueError(
-            '"blob_storage_url" should be the URL of a blob holding a Key Vault backup, for example '
+            '"folder_url" should be the URL of a blob holding a Key Vault backup, for example '
             '"https://<account>.blob.core.windows.net/backup/mhsm-account-2020090117323313"'
         )
 

sdk/keyvault/azure-keyvault-administration/azure/keyvault/administration/_models.py

@@ -51,20 +51,20 @@ class KeyVaultRoleAssignment(object):
 
     def __init__(self, **kwargs):
         # type: (**Any) -> None
-        self._assignment_id = kwargs.get("assignment_id")
+        self._role_assignment_id = kwargs.get("role_assignment_id")
         self._name = kwargs.get("name")
         self._properties = kwargs.get("properties")
         self._type = kwargs.get("assignment_type")
 
     def __repr__(self):
         # type: () -> str
-        return "KeyVaultRoleAssignment<{}>".format(self._assignment_id)
+        return "KeyVaultRoleAssignment<{}>".format(self._role_assignment_id)
 
     @property
-    def assignment_id(self):
+    def role_assignment_id(self):
         # type: () -> str
         """unique identifier for this assignment"""
-        return self._assignment_id
+        return self._role_assignment_id
 
     @property
     def name(self):
@@ -102,7 +102,7 @@ def type(self):
     @classmethod
     def _from_generated(cls, role_assignment):
         return cls(
-            assignment_id=role_assignment.id,
+            role_assignment_id=role_assignment.id,
             name=role_assignment.name,
             assignment_type=role_assignment.type,
             properties=KeyVaultRoleAssignmentProperties._from_generated(role_assignment.properties),
@@ -183,7 +183,7 @@ def __init__(self, **kwargs):
         self.error = kwargs.get("error", None)
         self.start_time = kwargs.get("start_time", None)
         self.end_time = kwargs.get("end_time", None)
-        self.id = kwargs.get("job_id", None)
+        self.job_id = kwargs.get("job_id", None)
 
     @classmethod
     def _wrap_generated(cls, response, deserialized_operation, response_headers):  # pylint:disable=unused-argument
@@ -200,11 +200,11 @@ class BackupOperation(_Operation):
     :ivar datetime.datetime start_time: UTC start time of the operation
     :ivar datetime.datetime end_time: UTC end time of the operation
     :ivar str job_id: identifier for the operation
-    :ivar str blob_storage_url: URL of the Azure blob storage container which contains the backup
+    :ivar str folder_url: URL of the Azure blob storage container which contains the backup
     """
 
     def __init__(self, **kwargs):
-        self.blob_storage_url = kwargs.pop("azure_storage_blob_container_uri", None)
+        self.folder_url = kwargs.pop("azure_storage_blob_container_uri", None)
         super(BackupOperation, self).__init__(**kwargs)
 
 

sdk/keyvault/azure-keyvault-administration/azure/keyvault/administration/aio/_backup_client.py

@@ -6,7 +6,7 @@
 
 from azure.core.polling.async_base_polling import AsyncLROBasePolling
 
-from .._internal import AsyncKeyVaultClientBase, parse_blob_storage_url
+from .._internal import AsyncKeyVaultClientBase, parse_folder_url
 from .._internal.polling import KeyVaultBackupClientPolling
 from .._models import BackupOperation, RestoreOperation, SelectiveKeyRestoreOperation
 
@@ -25,7 +25,7 @@ class KeyVaultBackupClient(AsyncKeyVaultClientBase):
     """
 
     # pylint:disable=protected-access
-    async def begin_full_backup(
+    async def begin_backup(
         self, blob_storage_url: str, sas_token: str, **kwargs: "Any"
     ) -> "AsyncLROPoller[BackupOperation]":
         """Begin a full backup of the Key Vault.
@@ -50,20 +50,20 @@ async def begin_full_backup(
             **kwargs
         )
 
-    async def begin_full_restore(
-        self, blob_storage_url: str, sas_token: str, **kwargs: "Any"
+    async def begin_restore(
+        self, folder_url: str, sas_token: str, **kwargs: "Any"
     ) -> "AsyncLROPoller[RestoreOperation]":
         """Restore a full backup of a Key Vault.
 
-        :param str blob_storage_url: URL for the blob storage resource, including the path to the blob holding the
-            backup. This would be the `blob_storage_url` of a :class:`BackupOperation` returned by
-            :func:`begin_full_backup` or :func:`get_backup_status`, for example
+        :param str folder_url: URL for the blob storage resource, including the path to the blob holding the
+            backup. This would be the `folder_url` of a :class:`BackupOperation` returned by
+            :func:`begin_backup` or :func:`get_backup_status`, for example
             https://<account>.blob.core.windows.net/backup/mhsm-account-2020090117323313
         :param str sas_token: a Shared Access Signature (SAS) token authorizing access to the blob storage resource
         :rtype: ~azure.core.polling.AsyncLROPoller[RestoreOperation]
         """
         polling_interval = kwargs.pop("_polling_interval", 5)
-        container_url, folder_name = parse_blob_storage_url(blob_storage_url)
+        container_url, folder_name = parse_folder_url(folder_url)
         sas_parameter = self._models.SASTokenParameter(storage_resource_uri=container_url, token=sas_token)
         restore_details = self._models.RestoreOperationParameters(
             sas_token_parameters=sas_parameter, folder_to_restore=folder_name
@@ -80,20 +80,20 @@ async def begin_full_restore(
         )
 
     async def begin_selective_restore(
-        self, blob_storage_url: str, sas_token: str, key_name: str, **kwargs: "Any"
+        self, folder_url: str, sas_token: str, key_name: str, **kwargs: "Any"
     ) -> "AsyncLROPoller[SelectiveKeyRestoreOperation]":
         """Restore a single key from a full Key Vault backup.
 
-        :param str blob_storage_url: URL for the blob storage resource, including the path to the blob holding the
-            backup. This would be the `blob_storage_url` of a :class:`BackupOperation` returned by
-            :func:`begin_full_backup` or :func:`get_backup_status`, for example
+        :param str folder_url: URL for the blob storage resource, including the path to the blob holding the
+            backup. This would be the `folder_url` of a :class:`BackupOperation` returned by
+            :func:`begin_backup` or :func:`get_backup_status`, for example
             https://<account>.blob.core.windows.net/backup/mhsm-account-2020090117323313
         :param str sas_token: a Shared Access Signature (SAS) token authorizing access to the blob storage resource
         :param str key_name: name of the key to restore from the backup
         :rtype: ~azure.core.polling.AsyncLROPoller[RestoreOperation]
         """
         polling_interval = kwargs.pop("_polling_interval", 5)
-        container_url, folder_name = parse_blob_storage_url(blob_storage_url)
+        container_url, folder_name = parse_folder_url(folder_url)
         sas_parameter = self._models.SASTokenParameter(storage_resource_uri=container_url, token=sas_token)
         restore_details = self._models.SelectiveKeyRestoreOperationParameters(
             sas_token_parameters=sas_parameter, folder=folder_name

sdk/keyvault/azure-keyvault-administration/tests/test_access_control.py

@@ -81,15 +81,17 @@ def test_role_assignment(self, client):
 
         # new assignment should be in the list of all assignments
         matching_assignments = [
-            a for a in client.list_role_assignments(scope) if a.assignment_id == created.assignment_id
+            a for a in client.list_role_assignments(scope) if a.role_assignment_id == created.role_assignment_id
         ]
         assert len(matching_assignments) == 1
 
         # delete the assignment
         deleted = client.delete_role_assignment(scope, created.name)
         assert deleted.name == created.name
-        assert deleted.assignment_id == created.assignment_id
+        assert deleted.role_assignment_id == created.role_assignment_id
         assert deleted.scope == scope
         assert deleted.role_definition_id == created.role_definition_id
 
-        assert not any(a for a in client.list_role_assignments(scope) if a.assignment_id == created.assignment_id)
+        assert not any(
+            a for a in client.list_role_assignments(scope) if a.role_assignment_id == created.role_assignment_id
+        )

sdk/keyvault/azure-keyvault-administration/tests/test_access_control_async.py

@@ -87,16 +87,18 @@ async def test_role_assignment(self, client):
         # new assignment should be in the list of all assignments
         matching_assignments = []
         async for assignment in client.list_role_assignments(scope):
-            if assignment.assignment_id == created.assignment_id:
+            if assignment.role_assignment_id == created.role_assignment_id:
                 matching_assignments.append(assignment)
         assert len(matching_assignments) == 1
 
         # delete the assignment
         deleted = await client.delete_role_assignment(scope, created.name)
         assert deleted.name == created.name
-        assert deleted.assignment_id == created.assignment_id
+        assert deleted.role_assignment_id == created.role_assignment_id
         assert deleted.scope == scope
         assert deleted.role_definition_id == created.role_definition_id
 
         async for assignment in client.list_role_assignments(scope):
-            assert assignment.assignment_id != created.assignment_id, "the role assignment should have been deleted"
+            assert (
+                assignment.role_assignment_id != created.role_assignment_id
+            ), "the role assignment should have been deleted"