[Key Vault Admin] Updates to the upsertRoleDefinition method and options (Azure#15342)

Updates to the upsertRoleDefinition method and options.

This PR makes only the first parameter of `upsertRoleDefinition` required and moves all of the others to the options bag. Also adds relevant comments.

Fixes Azure#15182

Author: sadasant

sdk/keyvault/keyvault-admin/package.json

@@ -96,6 +96,8 @@
     "@azure/core-paging": "^1.1.1",
     "@azure/core-tracing": "1.0.0-preview.11",
     "@azure/logger": "^1.0.0",
+    "@types/uuid": "^8.0.0",
+    "uuid": "^8.3.0",
     "tslib": "^2.0.0"
   },
   "devDependencies": {
@@ -116,7 +118,6 @@
     "@types/sinon": "^9.0.4",
     "@types/mocha": "^7.0.2",
     "@types/node": "^8.0.0",
-    "@types/uuid": "^8.0.0",
     "assert": "^1.4.1",
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
@@ -137,7 +138,6 @@
     "sinon": "^9.0.2",
     "source-map-support": "^0.5.9",
     "typescript": "~4.2.0",
-    "uuid": "^8.3.0",
     "typedoc": "0.15.2"
   }
 }

sdk/keyvault/keyvault-admin/recordings/node/keyvaultaccesscontrolclient_role_definitions/recording_can_create_update_and_delete_a_role_definition_happy_path.js

@@ -45,7 +45,7 @@ export class KeyVaultAccessControlClient {
     getRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: GetRoleDefinitionOptions): Promise<KeyVaultRoleDefinition>;
     listRoleAssignments(roleScope: KeyVaultRoleScope, options?: ListRoleAssignmentsOptions): PagedAsyncIterableIterator<KeyVaultRoleAssignment>;
     listRoleDefinitions(roleScope: KeyVaultRoleScope, options?: ListRoleDefinitionsOptions): PagedAsyncIterableIterator<KeyVaultRoleDefinition>;
-    upsertRoleDefinition(roleScope: KeyVaultRoleScope, name: string, permissions: KeyVaultPermission[], description?: string, options?: UpsertRoleDefinitionOptions): Promise<KeyVaultRoleDefinition>;
+    upsertRoleDefinition(roleScope: KeyVaultRoleScope, options?: UpsertRoleDefinitionOptions): Promise<KeyVaultRoleDefinition>;
     readonly vaultUrl: string;
 }
 
@@ -184,6 +184,11 @@ export type SUPPORTED_API_VERSIONS = "7.2";
 
 // @public
 export interface UpsertRoleDefinitionOptions extends coreHttp.OperationOptions {
+    assignableScopes?: KeyVaultRoleScope[];
+    description?: string;
+    permissions?: KeyVaultPermission[];
+    roleDefinitionName?: string;
+    roleName?: string;
 }
 
 

sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md

@@ -40,12 +40,12 @@ export async function main(): Promise<void> {
       ]
     }
   ];
-  let roleDefinition = await client.upsertRoleDefinition(
-    globalScope,
+  let roleDefinition = await client.upsertRoleDefinition(globalScope, {
     roleDefinitionName,
+    roleName: "Backup Manager",
     permissions,
-    "Allow backup actions"
-  );
+    description: "Allow backup actions"
+  });
   console.log(roleDefinition);
 
   // This sample uses a custom role but you may assign one of the many built-in roles.

sdk/keyvault/keyvault-admin/samples-dev/accessControlHelloWorld.ts

@@ -42,9 +42,12 @@ async function main() {
   ];
   let roleDefinition = await client.upsertRoleDefinition(
     globalScope,
-    roleDefinitionName,
-    permissions,
-    "Allow backup actions"
+    {
+      roleDefinitionName,
+      roleName: "Backup Manager",
+      permissions,
+      description: "Allow backup actions"
+    }
   );
   console.log(roleDefinition);
 

sdk/keyvault/keyvault-admin/samples/v4/javascript/accessControlHelloWorld.js

@@ -42,9 +42,12 @@ export async function main(): Promise<void> {
   ];
   let roleDefinition = await client.upsertRoleDefinition(
     globalScope,
-    roleDefinitionName,
-    permissions,
-    "Allow backup actions"
+    {
+      roleDefinitionName,
+      roleName: "Backup Manager",
+      permissions,
+      description: "Allow backup actions"
+    }
   );
   console.log(roleDefinition);
 

sdk/keyvault/keyvault-admin/samples/v4/typescript/src/accessControlHelloWorld.ts

@@ -30,7 +30,6 @@ import {
   GetRoleAssignmentOptions,
   ListRoleDefinitionsPageSettings,
   ListRoleAssignmentsPageSettings,
-  KeyVaultPermission,
   GetRoleDefinitionOptions,
   UpsertRoleDefinitionOptions,
   DeleteRoleDefinitionOptions
@@ -39,6 +38,7 @@ import {
 import { SDK_VERSION, LATEST_API_VERSION } from "./constants";
 import { mappings } from "./mappings";
 import { logger } from "./log";
+import { v4 as v4uuid } from "uuid";
 
 const withTrace = createTraceFunction("Azure.KeyVault.Admin.KeyVaultAccessControlClient");
 
@@ -453,22 +453,19 @@ export class KeyVaultAccessControlClient {
    */
   public upsertRoleDefinition(
     roleScope: KeyVaultRoleScope,
-    name: string,
-    permissions: KeyVaultPermission[],
-    description?: string,
     options: UpsertRoleDefinitionOptions = {}
   ): Promise<KeyVaultRoleDefinition> {
     return withTrace("upsertRoleDefinition", options, async (updatedOptions) => {
       const response = await this.client.roleDefinitions.createOrUpdate(
         this.vaultUrl,
         roleScope,
-        name,
+        options.roleDefinitionName || v4uuid(),
         {
           properties: {
-            description,
-            permissions,
+            description: options.description,
+            permissions: options.permissions,
             assignableScopes: [roleScope],
-            roleName: name,
+            roleName: options.roleName,
             roleType: "CustomRole"
           }
         },

sdk/keyvault/keyvault-admin/src/accessControlClient.ts

@@ -192,7 +192,28 @@ export interface GetRoleDefinitionOptions extends coreHttp.OperationOptions {}
 /**
  * An interface representing optional parameters passed to {@link upsertRoleDefinition}.
  */
-export interface UpsertRoleDefinitionOptions extends coreHttp.OperationOptions {}
+export interface UpsertRoleDefinitionOptions extends coreHttp.OperationOptions {
+  /**
+   * UUID used as the name of the role definition to create. If it's not provided, a new UUID will be generated.
+   */
+  roleDefinitionName?: string;
+  /**
+   * Friendly display name for the role definition.
+   */
+  roleName?: string;
+  /**
+   * Long-form description of the role definition.
+   */
+  description?: string;
+  /**
+   * List of Key Vault permissions
+   */
+  permissions?: KeyVaultPermission[];
+  /**
+   * List of assignable Key Vault role scopes
+   */
+  assignableScopes?: KeyVaultRoleScope[];
+}
 
 /**
  * An interface representing optional parameters passed to {@link deleteRoleDefinition}.

sdk/keyvault/keyvault-admin/src/accessControlModels.ts

@@ -91,13 +91,14 @@ describe("KeyVaultAccessControlClient", () => {
 
     it("can create, update, and delete a role definition (happy path)", async function() {
       const name = generateFakeUUID();
+      const roleName = "custom role definition name";
       const description = "custom role description";
-      let roleDefinition: KeyVaultRoleDefinition = await client.upsertRoleDefinition(
-        globalScope,
-        name,
+      let roleDefinition: KeyVaultRoleDefinition = await client.upsertRoleDefinition(globalScope, {
+        roleDefinitionName: name,
+        roleName,
         permissions,
         description
-      );
+      });
 
       assert.equal(roleDefinition.name, name);
       assert.equal(roleDefinition.description, description);
@@ -115,12 +116,12 @@ describe("KeyVaultAccessControlClient", () => {
         notDataActions: ["Microsoft.KeyVault/managedHsm/keys/encrypt/action"]
       });
 
-      roleDefinition = await client.upsertRoleDefinition(
-        globalScope,
-        name,
+      roleDefinition = await client.upsertRoleDefinition(globalScope, {
+        roleDefinitionName: name,
+        roleName,
         permissions,
         description
-      );
+      });
 
       assert.equal(roleDefinition.id, id);
       assert.deepEqual(roleDefinition.permissions, permissions);
@@ -138,7 +139,13 @@ describe("KeyVaultAccessControlClient", () => {
 
     describe("upsertRoleDefinition", function() {
       it("errors when name is not a valid guid", async function() {
-        await assert.isRejected(client.upsertRoleDefinition(globalScope, "foo", []));
+        await assert.isRejected(
+          client.upsertRoleDefinition(globalScope, {
+            roleDefinitionName: "foo unique value",
+            roleName: "foo role definition name",
+            permissions: []
+          })
+        );
       });
 
       it("errors when updating a built-in role definition", async function() {
@@ -155,7 +162,11 @@ describe("KeyVaultAccessControlClient", () => {
         }
 
         await assert.isRejected(
-          client.upsertRoleDefinition(globalScope, builtInDefinition.name, permissions)
+          client.upsertRoleDefinition(globalScope, {
+            roleDefinitionName: builtInDefinition.name,
+            roleName: builtInDefinition.roleName,
+            permissions
+          })
         );
       });
     });