Skip to content
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

[Key Vault Admin] Convenience layer - KeyVaultAccessControlClient #10815

Merged
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
7c3cfc5
constructor and some of the surrounding things
sadasant Aug 24, 2020
28b4eb6
createRoleAssignment
sadasant Aug 24, 2020
eb9cbe0
createRoleAssignment documentation
sadasant Aug 24, 2020
a5524f5
deleteRoleAssignment
sadasant Aug 24, 2020
167935d
getRoleAssignment
sadasant Aug 24, 2020
44e9eb3
listRoleAssignments
sadasant Aug 24, 2020
3790bd1
listRoleDefinitions
sadasant Aug 24, 2020
2188a87
api-extractor
sadasant Aug 24, 2020
60f4bd3
formatting
sadasant Aug 24, 2020
ac414c9
deleted the filter option on the list operations
sadasant Aug 24, 2020
9e5505a
fixing the build
sadasant Aug 24, 2020
57fd102
lint fixes
sadasant Aug 24, 2020
0ce383e
forgot to api extract the last commit
sadasant Aug 24, 2020
7f179ef
bad reference to a parameter
sadasant Aug 25, 2020
8919a93
Resolves https://github.com/Azure/azure-sdk-for-js/pull/10815#discuss…
sadasant Aug 25, 2020
ee1d71e
Addressing: https://github.com/Azure/azure-sdk-for-js/pull/10815#disc…
sadasant Aug 25, 2020
b7d3bf5
Feedback fixes
sadasant Aug 25, 2020
26e9583
role to roleScope, and RoleDefinitionPermission to KeyVaultPermission
sadasant Aug 26, 2020
a10a592
KeyVaultAccessControlClient documentation and removing lint from the …
sadasant Aug 26, 2020
09148fd
cleaned up bad reference to KeyClient
sadasant Aug 26, 2020
1b0f642
Addressing https://github.com/Azure/azure-sdk-for-js/pull/10815#discu…
sadasant Aug 27, 2020
4513d72
Merge remote-tracking branch 'Azure/master' into keyvault-admin/10799…
sadasant Aug 27, 2020
6ab4ae1
fixed bad references
sadasant Aug 27, 2020
7c845ca
Update sdk/keyvault/keyvault-admin/src/accessControlClient.ts
sadasant Sep 1, 2020
9030441
Apply suggestions from code review
sadasant Sep 2, 2020
18d7911
Fixes after updating from remote
sadasant Sep 2, 2020
74b7bfc
Addressing https://github.com/Azure/azure-sdk-for-js/pull/10815/files…
sadasant Sep 2, 2020
ca12be4
tsdoc cleanup
sadasant Sep 2, 2020
e306ff4
linting
sadasant Sep 2, 2020
d61f0b6
Merge remote-tracking branch 'Azure/master' into keyvault-admin/10799…
sadasant Sep 2, 2020
64a5d1d
fixes after updating the generated files
sadasant Sep 2, 2020
1386365
enforcing most of the properties, after discussing with .Net
sadasant Sep 2, 2020
590a835
Addressing https://github.com/Azure/azure-sdk-for-js/pull/10815#discu…
sadasant Sep 2, 2020
23431cc
no more operationOptionsToRequestOptionsBase
sadasant Sep 4, 2020
66bcdd4
Key Vault
sadasant Sep 4, 2020
8484177
Merge remote-tracking branch 'Azure/master' into keyvault-admin/10799…
sadasant Sep 4, 2020
24380a7
only 7.2-preview service version
sadasant Sep 4, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions sdk/keyvault/keyvault-admin/api-extractor.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"mainEntryPointFilePath": "types/keyvault-admin/src/index.d.ts",
"docModel": {
"enabled": true
},
"apiReport": {
"enabled": true,
"reportFolder": "./review"
},
"dtsRollup": {
"enabled": true,
"untrimmedFilePath": "",
"publicTrimmedFilePath": "./types/keyvault-admin.d.ts"
},
"messages": {
"tsdocMessageReporting": {
"default": {
"logLevel": "none"
}
},
"extractorMessageReporting": {
"ae-missing-release-tag": {
"logLevel": "none"
},
"ae-unresolved-link": {
"logLevel": "none"
}
}
}
}
14 changes: 9 additions & 5 deletions sdk/keyvault/keyvault-admin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"url": "https://github.com/Azure/azure-sdk-for-js/issues"
},
"main": "./dist/index.js",
"module": "dist-esm/src/index.js",
"module": "dist-esm/keyvault-admin/src/index.js",
"types": "./types/keyvault-admin.d.ts",
"engine": {
"node": ">=8.0.0"
Expand All @@ -29,10 +29,11 @@
"node": ">=8.0.0"
},
"files": [
"types/",
"types/keyvault-admin.d.ts",
"dist/",
"dist-browser/",
"dist-esm/src",
"dist-esm/keyvault-admin/src",
"dist-esm/keyvault-common/src",
"README.md",
"LICENSE"
],
Expand All @@ -52,13 +53,13 @@
"execute:js-samples": "echo skipped",
"execute:ts-samples": "echo skipped",
"execute:samples": "npm run build:samples && npm run execute:js-samples && npm run execute:ts-samples",
"extract-api": "echo skipped",
"extract-api": "tsc -p . && api-extractor run --local",
"format": "prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "echo skipped",
"integration-test:node": "echo skipped",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"lint:fix": "eslint package.json src --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json src --ext .ts -f html -o keyvault-admin-lintReport.html",
"lint": "eslint package.json api-extractor.json src --ext .ts",
"lint:terminal": "eslint package.json src test --ext .ts",
"pack": "npm pack 2>&1",
"prebuild": "npm run clean",
Expand All @@ -74,12 +75,15 @@
"dependencies": {
"@azure/core-http": "^1.1.6",
"@azure/core-lro": "^1.0.2",
"@azure/core-paging": "^1.1.1",
"@azure/core-tracing": "1.0.0-preview.9",
"@azure/logger": "^1.0.0",
"@opentelemetry/api": "^0.10.2",
"tslib": "^2.0.0"
},
"devDependencies": {
"@azure/eslint-plugin-azure-sdk": "^3.0.0",
"@microsoft/api-extractor": "7.7.11",
"@rollup/plugin-commonjs": "11.0.2",
"@rollup/plugin-json": "^4.0.0",
"@rollup/plugin-multi-entry": "^3.0.0",
Expand Down
113 changes: 113 additions & 0 deletions sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
## API Report File for "@azure/keyvault-admin"

> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).

```ts

import * as coreHttp from '@azure/core-http';
import { PagedAsyncIterableIterator } from '@azure/core-paging';
import { TokenCredential } from '@azure/core-http';

// @public
export interface AccessControlClientOptions extends coreHttp.PipelineOptions {
serviceVersion?: SUPPORTED_API_VERSIONS;
}

// @public
export interface CreateRoleAssignmentOptions extends coreHttp.OperationOptions {
}

// @public
export interface DeleteRoleAssignmentOptions extends coreHttp.OperationOptions {
}

// @public
export interface GetRoleAssignmentOptions extends coreHttp.OperationOptions {
}

// @public
export class KeyVaultAccessControlClient {
constructor(vaultUrl: string, credential: TokenCredential, pipelineOptions?: AccessControlClientOptions);
createRoleAssignment(roleScope: RoleAssignmentScope, name: string, roleDefinitionId: string, principalId: string, options?: CreateRoleAssignmentOptions): Promise<KeyVaultRoleAssignment>;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in python we have it as role_assignment_name

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for every name parameter for role assignments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.Net is using name. I don't mind, but I'd rather reach to an agreement with @heaths , @christothes

deleteRoleAssignment(roleScope: RoleAssignmentScope, name: string, options?: DeleteRoleAssignmentOptions): Promise<KeyVaultRoleAssignment>;
sadasant marked this conversation as resolved.
Show resolved Hide resolved
getRoleAssignment(roleScope: RoleAssignmentScope, name: string, options?: GetRoleAssignmentOptions): Promise<KeyVaultRoleAssignment>;
listRoleAssignments(roleScope: RoleAssignmentScope, options?: ListRoleAssignmentsOptions): PagedAsyncIterableIterator<KeyVaultRoleAssignment>;
listRoleDefinitions(roleScope: RoleAssignmentScope, options?: ListRoleDefinitionsOptions): PagedAsyncIterableIterator<KeyVaultRoleDefinition>;
readonly vaultUrl: string;
}

// @public
export interface KeyVaultPermission {
sadasant marked this conversation as resolved.
Show resolved Hide resolved
actions?: string[];
dataActions?: string[];
notActions?: string[];
notDataActions?: string[];
}

// @public
export interface KeyVaultRoleAssignment {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just RoleAssignment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I do this change, let's argue about prefixes here: #10815 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heaths I'm assuming KeyVaultRoleAssignment is favored after that conversation I mentioned ^

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python has KeyVaultRoleAssignment

readonly id?: string;
readonly name?: string;
properties?: KeyVaultRoleAssignmentPropertiesWithScope;
readonly type?: string;
}

// @public
export interface KeyVaultRoleAssignmentProperties {
principalId: string;
roleDefinitionId: string;
}

// @public
export interface KeyVaultRoleAssignmentPropertiesWithScope {
principalId?: string;
roleDefinitionId?: string;
scope?: RoleAssignmentScope;
}

// @public
export interface KeyVaultRoleDefinition {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just RoleDefinition

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I do this change, let's argue about prefixes here: #10815 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heaths I'm assuming KeyVaultRoleDefinition is favored after that conversation I mentioned ^

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python has KeyVaultRoleDefinition

assignableScopes?: string[];
description?: string;
readonly id?: string;
readonly name?: string;
permissions?: KeyVaultPermission[];
roleName?: string;
roleType?: string;
readonly type?: string;
sadasant marked this conversation as resolved.
Show resolved Hide resolved
}

// @public
export const LATEST_API_VERSION = "7.1";

// @public
export interface ListRoleAssignmentsOptions extends coreHttp.OperationOptions {
}

// @public
export interface ListRoleAssignmentsPageSettings {
continuationToken?: string;
}

// @public
export interface ListRoleDefinitionsOptions extends coreHttp.OperationOptions {
}

// @public
export interface ListRoleDefinitionsPageSettings {
continuationToken?: string;
}

// @public
export type RoleAssignmentScope = "/" | "/keys" | string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels like a weird type, though I'm not sure what would be better

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string part is because it accepts UUIDs. I wonder if we could have some UUID validator as a type? That would be generally helpful in our clients.


// @public
export const SDK_VERSION: string;

// @public
export type SUPPORTED_API_VERSIONS = "7.0" | "7.1" | "7.2-preview";
sadasant marked this conversation as resolved.
Show resolved Hide resolved


// (No @packageDocumentation comment for this package)

```
4 changes: 2 additions & 2 deletions sdk/keyvault/keyvault-admin/rollup.base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function nodeConfig(test = false) {
const externalNodeBuiltins = [];
const additionalExternals = [];
const baseConfig = {
input: "dist-esm/src/index.js",
input: "dist-esm/keyvault-admin/src/index.js",
external: depNames.concat(externalNodeBuiltins, additionalExternals),
output: {
file: "dist/index.js",
Expand Down Expand Up @@ -83,7 +83,7 @@ export function nodeConfig(test = false) {

export function browserConfig(test = false) {
const baseConfig = {
input: "dist-esm/src/index.js",
input: "dist-esm/keyvault-admin/src/index.js",
output: {
file: "dist-browser/azure-keyvault-admin.js",
banner: banner,
Expand Down
Loading