[Key Vault] Reorder set_role_definition parameters (Azure#18743)

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

@@ -1,6 +1,13 @@
 # Release History
 
 ## 4.0.0b4 (Unreleased)
+### Added
+- `KeyVaultAccessControlClient.set_role_definition` accepts an optional 
+  `assignable_scopes` keyword-only argument
+
+### Breaking Changes
+- Changed parameter order in `KeyVaultAccessControlClient.set_role_definition`.
+  `permissions` is now an optional keyword-only argument
 
 
 ## 4.0.0b3 (2021-02-09)

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

@@ -12,11 +12,10 @@
 
 if TYPE_CHECKING:
     # pylint:disable=ungrouped-imports
-    from typing import Any, Iterable, Union
+    from typing import Any, Optional, Union
     from uuid import UUID
     from azure.core.paging import ItemPaged
     from ._enums import KeyVaultRoleScope
-    from ._models import KeyVaultPermission
 
 
 class KeyVaultAccessControlClient(KeyVaultClientBase):
@@ -41,7 +40,7 @@ def create_role_assignment(self, role_scope, role_definition_id, principal_id, *
         :param str principal_id: Azure Active Directory object ID of the principal which will be assigned the role. The
             principal can be a user, service principal, or security group.
         :keyword role_assignment_name: a name for the role assignment. Must be a UUID.
-        :type role_assignment_name: str or uuid.UUID
+        :paramtype role_assignment_name: str or uuid.UUID
         :rtype: KeyVaultRoleAssignment
         """
         role_assignment_name = kwargs.pop("role_assignment_name", None) or uuid4()
@@ -113,48 +112,51 @@ def list_role_assignments(self, role_scope, **kwargs):
         )
 
     @distributed_trace
-    def set_role_definition(self, role_scope, permissions, **kwargs):
-        # type: (Union[str, KeyVaultRoleScope], Iterable[KeyVaultPermission], **Any) -> KeyVaultRoleDefinition
+    def set_role_definition(self, role_scope, role_definition_name=None, **kwargs):
+        # type: (Union[str, KeyVaultRoleScope], Optional[Union[str, UUID]], **Any) -> KeyVaultRoleDefinition
         """Creates or updates a custom role definition.
 
         :param role_scope: scope of the role definition. :class:`KeyVaultRoleScope` defines common broad scopes.
             Specify a narrower scope as a string. Managed HSM only supports '/', or KeyVaultRoleScope.GLOBAL.
         :type role_scope: str or KeyVaultRoleScope
-        :param permissions: the role definition's permissions. An empty list results in a role definition with no action
-            permissions.
-        :type permissions: Iterable[KeyVaultPermission]
-        :keyword str role_name: the role's name. If unspecified when creating or updating a role definition, the role
-            name will be set to an empty string.
-        :keyword role_definition_name: the role definition's name. Must be a UUID.
+        :param role_definition_name: the unique role definition name. Unless a UUID is provided, a new role definition
+            will be created with a generated unique name. Providing the unique name of an existing role definition will
+            update that role definition.
         :type role_definition_name: str or uuid.UUID
+        :keyword str role_name: the role's display name. If unspecified when creating or updating a role definition, the
+            role name will be set to an empty string.
         :keyword str description: a description of the role definition. If unspecified when creating or updating a role
             definition, the description will be set to an empty string.
+        :keyword permissions: the role definition's permissions. If unspecified when creating or updating a role
+            definition, the role definition will have no action permissions.
+        :paramtype permissions: Iterable[KeyVaultPermission]
+        :keyword assignable_scopes: the scopes for which the role definition can be assigned.
+        :paramtype assignable_scopes: Iterable[str] or Iterable[KeyVaultRoleScope]
         :returns: The created or updated role definition
         :rtype: KeyVaultRoleDefinition
         """
-        role_definition_name = kwargs.pop("role_definition_name", None) or uuid4()
-
         permissions = [
             self._client.role_definitions.models.Permission(
                 actions=p.allowed_actions,
                 not_actions=p.denied_actions,
                 data_actions=p.allowed_data_actions,
                 not_data_actions=p.denied_data_actions,
             )
-            for p in permissions
+            for p in kwargs.pop("permissions", None) or []
         ]
 
         properties = self._client.role_definitions.models.RoleDefinitionProperties(
             role_name=kwargs.pop("role_name", None),
             description=kwargs.pop("description", None),
-            permissions=permissions
+            permissions=permissions,
+            assignable_scopes=kwargs.pop("assignable_scopes", None),
         )
         parameters = self._client.role_definitions.models.RoleDefinitionCreateParameters(properties=properties)
 
         definition = self._client.role_definitions.create_or_update(
             vault_base_url=self._vault_url,
             scope=role_scope,
-            role_definition_name=str(role_definition_name),
+            role_definition_name=str(role_definition_name or uuid4()),
             parameters=parameters,
             **kwargs
         )

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

@@ -13,11 +13,10 @@
 
 if TYPE_CHECKING:
     # pylint:disable=ungrouped-imports
-    from typing import Any, Iterable, Union
+    from typing import Any, Optional, Union
     from uuid import UUID
     from azure.core.async_paging import AsyncItemPaged
     from .._enums import KeyVaultRoleScope
-    from .._models import KeyVaultPermission
 
 
 class KeyVaultAccessControlClient(AsyncKeyVaultClientBase):
@@ -43,7 +42,7 @@ async def create_role_assignment(
         :param str principal_id: Azure Active Directory object ID of the principal which will be assigned the role. The
             principal can be a user, service principal, or security group.
         :keyword role_assignment_name: a name for the role assignment. Must be a UUID.
-        :type role_assignment_name: str or uuid.UUID
+        :paramtype role_assignment_name: str or uuid.UUID
         :rtype: KeyVaultRoleAssignment
         """
         role_assignment_name = kwargs.pop("role_assignment_name", None) or uuid4()
@@ -119,48 +118,54 @@ def list_role_assignments(
 
     @distributed_trace_async
     async def set_role_definition(
-        self, role_scope: "Union[str, KeyVaultRoleScope]", permissions: "Iterable[KeyVaultPermission]", **kwargs: "Any"
+        self,
+        role_scope: "Union[str, KeyVaultRoleScope]",
+        role_definition_name: "Optional[Union[str, UUID]]" = None,
+        **kwargs: "Any"
     ) -> "KeyVaultRoleDefinition":
         """Creates or updates a custom role definition.
 
         :param role_scope: scope of the role definition. :class:`KeyVaultRoleScope` defines common broad scopes.
             Specify a narrower scope as a string. Managed HSM only supports '/', or KeyVaultRoleScope.GLOBAL.
         :type role_scope: str or KeyVaultRoleScope
-        :param permissions: the role definition's permissions. An empty list results in a role definition with no action
-            permissions.
-        :type permissions: Iterable[KeyVaultPermission]
-        :keyword str role_name: the role's name. If unspecified when creating or updating a role definition, the role
-            name will be set to an empty string.
-        :keyword role_definition_name: the role definition's name. Must be a UUID.
+        :param role_definition_name: the unique role definition name. Unless a UUID is provided, a new role definition
+            will be created with a generated unique name. Providing the unique name of an existing role definition will
+            update that role definition.
         :type role_definition_name: str or uuid.UUID
+        :keyword str role_name: the role's display name. If unspecified when creating or updating a role definition, the
+            role name will be set to an empty string.
         :keyword str description: a description of the role definition. If unspecified when creating or updating a role
             definition, the description will be set to an empty string.
+        :keyword permissions: the role definition's permissions. If unspecified when creating or updating a role
+            definition, the role definition will have no action permissions.
+        :paramtype permissions: Iterable[KeyVaultPermission]
+        :keyword assignable_scopes: the scopes for which the role definition can be assigned.
+        :paramtype assignable_scopes: Iterable[str] or Iterable[KeyVaultRoleScope]
         :returns: The created or updated role definition
         :rtype: KeyVaultRoleDefinition
         """
-        role_definition_name = kwargs.pop("role_definition_name", None) or uuid4()
-
         permissions = [
             self._client.role_definitions.models.Permission(
                 actions=p.allowed_actions,
                 not_actions=p.denied_actions,
                 data_actions=p.allowed_data_actions,
                 not_data_actions=p.denied_data_actions,
             )
-            for p in permissions
+            for p in kwargs.pop("permissions", None) or []
         ]
 
         properties = self._client.role_definitions.models.RoleDefinitionProperties(
             role_name=kwargs.pop("role_name", None),
             description=kwargs.pop("description", None),
-            permissions=permissions
+            permissions=permissions,
+            assignable_scopes=kwargs.pop("assignable_scopes", None),
         )
         parameters = self._client.role_definitions.models.RoleDefinitionCreateParameters(properties=properties)
 
         definition = await self._client.role_definitions.create_or_update(
             vault_base_url=self._vault_url,
             scope=role_scope,
-            role_definition_name=str(role_definition_name),
+            role_definition_name=str(role_definition_name or uuid4()),
             parameters=parameters,
             **kwargs
         )

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

@@ -69,30 +69,32 @@ def test_role_definitions(self):
         permissions = [KeyVaultPermission(allowed_data_actions=[KeyVaultDataAction.READ_HSM_KEY])]
         created_definition = client.set_role_definition(
             role_scope=scope,
-            permissions=permissions,
-            role_name=role_name,
             role_definition_name=definition_name,
-            description="test"
+            role_name=role_name,
+            description="test",
+            permissions=permissions
         )
         assert "/" in created_definition.assignable_scopes
         assert created_definition.role_name == role_name
         assert created_definition.name == definition_name
         assert created_definition.description == "test"
         assert len(created_definition.permissions) == 1
         assert created_definition.permissions[0].allowed_data_actions == [KeyVaultDataAction.READ_HSM_KEY]
+        assert created_definition.assignable_scopes == [KeyVaultRoleScope.GLOBAL]
 
         # update custom role definition
         permissions = [
             KeyVaultPermission(allowed_data_actions=[], denied_data_actions=[KeyVaultDataAction.READ_HSM_KEY])
         ]
         updated_definition = client.set_role_definition(
-            role_scope=scope, permissions=permissions, role_definition_name=definition_name
+            role_scope=scope, role_definition_name=definition_name, permissions=permissions
         )
         assert updated_definition.role_name == ""
         assert updated_definition.description == ""
         assert len(updated_definition.permissions) == 1
         assert len(updated_definition.permissions[0].allowed_data_actions) == 0
         assert updated_definition.permissions[0].denied_data_actions == [KeyVaultDataAction.READ_HSM_KEY]
+        assert updated_definition.assignable_scopes == [KeyVaultRoleScope.GLOBAL]
 
         # assert that the created role definition isn't duplicated
         matching_definitions = [d for d in client.list_role_definitions(scope) if d.id == updated_definition.id]

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

@@ -79,30 +79,32 @@ async def test_role_definitions(self):
         permissions = [KeyVaultPermission(allowed_data_actions=[KeyVaultDataAction.READ_HSM_KEY])]
         created_definition = await client.set_role_definition(
             role_scope=scope,
-            permissions=permissions,
-            role_name=role_name,
             role_definition_name=definition_name,
-            description="test"
+            role_name=role_name,
+            description="test",
+            permissions=permissions
         )
         assert "/" in created_definition.assignable_scopes
         assert created_definition.role_name == role_name
         assert created_definition.name == definition_name
         assert created_definition.description == "test"
         assert len(created_definition.permissions) == 1
         assert created_definition.permissions[0].allowed_data_actions == [KeyVaultDataAction.READ_HSM_KEY]
+        assert created_definition.assignable_scopes == [KeyVaultRoleScope.GLOBAL]
 
         # update custom role definition
         permissions = [
             KeyVaultPermission(allowed_data_actions=[], denied_data_actions=[KeyVaultDataAction.READ_HSM_KEY])
         ]
         updated_definition = await client.set_role_definition(
-            role_scope=scope, permissions=permissions, role_definition_name=definition_name
+            role_scope=scope, role_definition_name=definition_name, permissions=permissions
         )
         assert updated_definition.role_name == ""
         assert updated_definition.description == ""
         assert len(updated_definition.permissions) == 1
         assert len(updated_definition.permissions[0].allowed_data_actions) == 0
         assert updated_definition.permissions[0].denied_data_actions == [KeyVaultDataAction.READ_HSM_KEY]
+        assert updated_definition.assignable_scopes == [KeyVaultRoleScope.GLOBAL]
 
         # assert that the created role definition isn't duplicated
         matching_definitions = []