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.

Sign up for GitHub

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

[KeyVault] Common folder #8866

Merged
merged 26 commits into from
Jun 17, 2020
Merged

[KeyVault] Common folder #8866

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7c46f37
Update challengeBasedAuthenticationPolicy.ts
sadasant Apr 27, 2020
ec0f23a
Common folder for the challenge based auth
sadasant Apr 29, 2020
8b82b38
deleting this from certificates again
sadasant Apr 29, 2020
8824a8c
wip
sadasant Apr 29, 2020
e1f9ea0
fixes from https://github.com/ljian3377/azure-sdk-for-js/commit/0ce33…
sadasant May 12, 2020
960ac88
other changes that make sense after rebasing
sadasant May 12, 2020
2efcad1
making sure keyvault-common is up to date
sadasant May 12, 2020
bce9304
same changes on keys and secrets
sadasant May 12, 2020
ba15401
formatting
sadasant May 12, 2020
996faf4
this .gitignore is not needed
sadasant May 12, 2020
a3b1ba7
bringing the hotfix to this PR and making sure it passes CI
sadasant May 21, 2020
daf036b
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant May 21, 2020
e873fcf
new pnpm-lock after merging master
sadasant May 21, 2020
29de300
package.json and eslintrc updates
sadasant May 21, 2020
d737798
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant May 27, 2020
925bae1
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant May 29, 2020
91007b4
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant Jun 1, 2020
bf263c6
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant Jun 2, 2020
4e8265d
fixed end of file on keyvault-common/src/index.ts
sadasant Jun 2, 2020
963fae8
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant Jun 2, 2020
aae90ff
some of the feedback
sadasant Jun 13, 2020
f423dbe
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant Jun 13, 2020
a56bd99
standard tsconf on keyvault-common
sadasant Jun 13, 2020
fea30d9
Merge remote-tracking branch 'Azure/master' into keyvault/common-folder
sadasant Jun 17, 2020
4de173b
tsconfig.json cleanup
sadasant Jun 17, 2020
4ae09d5
removed unnecessary scripts from keyvault-common
sadasant Jun 17, 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
81 changes: 48 additions & 33 deletions common/config/rush/pnpm-lock.yaml
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions rush.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@
"projectFolder": "sdk/identity/identity",
"versionPolicyName": "client"
},
{
"packageName": "@azure/keyvault-common",
"projectFolder": "sdk/keyvault/keyvault-common",
"versionPolicyName": "utility"
},
{
"packageName": "@azure/keyvault-certificates",
"projectFolder": "sdk/keyvault/keyvault-certificates",
Expand Down
2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-certificates/api-extractor.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"mainEntryPointFilePath": "types/src/index.d.ts",
"mainEntryPointFilePath": "types/keyvault-certificates/src/index.d.ts",
"docModel": {
"enabled": false
},
Expand Down
9 changes: 5 additions & 4 deletions sdk/keyvault/keyvault-certificates/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@
"url": "https://github.com/Azure/azure-sdk-for-js/issues"
},
"main": "./dist/index.js",
"module": "dist-esm/src/index.js",
"module": "dist-esm/keyvault-certificates/src/index.js",
HarshaNalluru marked this conversation as resolved.
Show resolved Hide resolved
"types": "./types/keyvault-certificates.d.ts",
"engines": {
"node": ">=8.0.0"
},
"files": [
"types/keyvault-certificates.d.ts",
"dist/",
"dist-esm/src",
"dist-esm/keyvault-certificates/src",
"dist-esm/keyvault-common/src",
Copy link
Member

Choose a reason for hiding this comment

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

👍

"README.md",
"LICENSE"
],
Expand All @@ -52,8 +53,8 @@
"extract-api": "tsc -p . && api-extractor run --local",
"format": "prettier --write --config ../../.prettierrc.json \"src/**/*.ts\" \"samples/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "karma start --single-run",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 180000 --full-trace dist-esm/test/*.test.js",
"integration-test:node:no-timeout": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --no-timeouts --full-trace dist-esm/test/*.test.js",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 180000 --full-trace dist-esm/keyvault-certificates/test/*.test.js",
"integration-test:node:no-timeout": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --no-timeouts --full-trace dist-esm/keyvault-certificates/test/*.test.js",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json api-extractor.json src test --ext .ts -f html -o keyvault-certificates-lintReport.html",
Expand Down
8 changes: 4 additions & 4 deletions sdk/keyvault/keyvault-certificates/rollup.base.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function nodeConfig(test = false) {
const externalNodeBuiltins = ["crypto", "fs", "os", "url", "assert"];
const additionalExternals = ["keytar"];
const baseConfig = {
input: "dist-esm/src/index.js",
input: "dist-esm/keyvault-certificates/src/index.js",
external: depNames.concat(externalNodeBuiltins, additionalExternals),
output: {
file: "dist/index.js",
Expand All @@ -59,7 +59,7 @@ export function nodeConfig(test = false) {

if (test) {
// entry point is every test file
baseConfig.input = ["dist-esm/test/*.test.js"];
baseConfig.input = ["dist-esm/keyvault-certificates/test/*.test.js"];
baseConfig.plugins.unshift(
multiEntry({ exports: false }),
json() // This allows us to import/require the package.json file, to get the version and test it against the user agent.
Expand All @@ -85,7 +85,7 @@ export function nodeConfig(test = false) {

export function browserConfig(test = false) {
const baseConfig = {
input: "dist-esm/src/index.js",
input: "dist-esm/keyvault-certificates/src/index.js",
output: {
file: "dist-browser/azure-keyvault-certificates.js",
banner: banner,
Expand Down Expand Up @@ -135,7 +135,7 @@ export function browserConfig(test = false) {
baseConfig.external = ["fs", "fs-extra", "child_process", "path", "crypto", "constants"];
if (test) {
baseConfig.external.push("os");
baseConfig.input = ["dist-esm/test/*.test.js"];
baseConfig.input = ["dist-esm/keyvault-certificates/test/*.test.js"];
baseConfig.plugins.unshift(
multiEntry({ exports: false }),
json() // This allows us to import/require the package.json file, to get the version and test it against the user agent.
Expand Down
8 changes: 5 additions & 3 deletions sdk/keyvault/keyvault-certificates/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ import { SDK_VERSION } from "./core/utils/constants";
import { parseKeyvaultIdentifier as parseKeyvaultEntityIdentifier } from "./core/utils";
import "@azure/core-paging";
import { PageSettings, PagedAsyncIterableIterator } from "@azure/core-paging";
import { challengeBasedAuthenticationPolicy } from "./core/challengeBasedAuthenticationPolicy";

import { challengeBasedAuthenticationPolicy } from "../../keyvault-common/src";
import { CreateCertificatePoller } from "./lro/create/poller";
import { CertificateOperationPoller } from "./lro/operation/poller";
import { DeleteCertificatePoller } from "./lro/delete/poller";
Expand Down Expand Up @@ -232,7 +231,10 @@ export {
/**
* Deprecated KeyVault copy of core-lro's PollerLike.
*/
export type KVPollerLike<TState extends PollOperationState<TResult>, TResult> = PollerLike<TState, TResult>;
export type KVPollerLike<TState extends PollOperationState<TResult>, TResult> = PollerLike<
TState,
TResult
>;
sadasant marked this conversation as resolved.
Show resolved Hide resolved

function toCoreAttributes(properties: CertificateProperties): CoreCertificateAttributes {
return {
Expand Down
5 changes: 1 addition & 4 deletions sdk/keyvault/keyvault-certificates/test/challengeBasedAuthenticationPolicy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ import { CertificateClient } from "../src";
import { env, Recorder } from "@azure/test-utils-recorder";
import { authenticate } from "./utils/testAuthentication";
import TestClient from "./utils/testClient";
import {
AuthenticationChallengeCache,
AuthenticationChallenge
} from "../src/core/challengeBasedAuthenticationPolicy";
import { AuthenticationChallengeCache, AuthenticationChallenge } from "../../keyvault-common/src";
import { testPollerProperties } from "./utils/recorderUtils";

describe("Challenge based authentication tests", () => {
Expand Down
2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-certificates/test/list.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe("Certificates client - list certificates in various ways", () => {
try {
await testClient.flushCertificate(certificate.name!);
} catch (e) {
// Nothing to do here
sadasant marked this conversation as resolved.
Show resolved Hide resolved
// Nothing to do here
}
}
for await (const certificate of client.listDeletedCertificates({ includePending: true })) {
Expand Down
4 changes: 2 additions & 2 deletions sdk/keyvault/keyvault-certificates/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@
"resolveJsonModule": true
},
"compileOnSave": true,
"exclude": ["node_modules", "./samples/**/*.ts"],
"include": ["./src/**/*.ts", "./test/**/*.ts"]
"exclude": ["node_modules", "../keyvault-common/node_modules", "./samples/**/*.ts"],
"include": ["./src/**/*.ts", "./test/**/*.ts", "../keyvault-common/**/*.ts"]
sadasant marked this conversation as resolved.
Show resolved Hide resolved
}
4 changes: 4 additions & 0 deletions sdk/keyvault/keyvault-common/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
test.csr
test.crt
ca.srl
pkcs12.p12
4 changes: 4 additions & 0 deletions sdk/keyvault/keyvault-common/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

export * from "./src";
sadasant marked this conversation as resolved.
Show resolved Hide resolved
55 changes: 55 additions & 0 deletions sdk/keyvault/keyvault-common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{
"name": "@azure/keyvault-common",
"sideEffects": false,
"private": true,
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this library need to ship? If so why it is marked private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a library, it's a shared folder that gets built and shipped through the other packages: keyvault-keys, keyvault-secrets and keyvault-certificates.

The package.json is there so that VSCode can pick up the dependencies.

"author": "Microsoft Corporation",
"version": "4.1.0-preview.2",
sadasant marked this conversation as resolved.
Show resolved Hide resolved
"license": "MIT",
"description": "Isomorphic client library for Azure KeyVault's certificates.",
sadasant marked this conversation as resolved.
Show resolved Hide resolved
"repository": "github:Azure/azure-sdk-for-js",
"main": "./src/index.ts",
"module": "dist-esm/index.js",
"types": "./types/index.d.ts",
"engines": {
"node": ">=8.0.0"
},
"scripts": {
"audit": "node ../../../keyvault-common/scripts/rush-audit.js && rimraf node_modules package-lock.json && npm i --package-lock-only 2>&1 && npm audit",
Copy link
Member

Choose a reason for hiding this comment

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

feels weird that it should go three levels up to come back down to itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't think of how to make this better at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

am I missing something? or can this line not be node ./scripts/rush-audit.js instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh this is a mistake, it should be common/ not keyvault-common/ Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(this is not part of keyvault-common)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll skip this script for now

"build:minify": "uglifyjs -c -m --comments --source-map \"content='./dist/index.js.map'\" -o ./dist/index.min.js ./dist/index.js 2>&1",
"build:samples": "echo skipped",
"build:es6": "tsc -p tsconfig.json",
"build:nodebrowser": "echo skipped",
"build:test": "echo skipped",
"build": "npm run extract-api && npm run build:es6 && npm run build:nodebrowser",
"check-format": "prettier --list-different --config ../../.prettierrc.json \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"clean": "rimraf dist-esm dist-test typings *.tgz *.log samples/typescript/dist",
"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",
"format": "prettier --write --config ../../.prettierrc.json \"src/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "echo skipped",
"integration-test:node": "echo skipped",
"integration-test:node:no-timeout": "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 src --ext .ts -f html -o search-lintReport.html || exit 0",
"lint:terminal": "eslint src --ext .ts",
"pack": "npm pack 2>&1",
"prebuild": "npm run clean",
"test:browser": "npm run clean && npm run build:test && npm run unit-test:browser",
"test:node": "npm run clean && npm run build:test && npm run unit-test:node",
"test": "npm run clean && npm run build:test && npm run unit-test",
"unit-test:browser": "echo skipped",
"unit-test:node": "echo skipped",
"unit-test:node:no-timeout": "echo skipped",
"unit-test": "npm run unit-test:node && npm run unit-test:browser"
},
"dependencies": {
"@azure/core-http": "^1.1.1",
"tslib": "^1.10.0"
sadasant marked this conversation as resolved.
Show resolved Hide resolved
},
"devDependencies": {
"typescript": "~3.8.3"
}
}
0 ...ore/challengeBasedAuthenticationPolicy.ts → ...src/challengeBasedAuthenticationPolicy.ts
View file
Open in desktop
File renamed without changes.
4 changes: 4 additions & 0 deletions sdk/keyvault/keyvault-common/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

export * from "./challengeBasedAuthenticationPolicy";
27 changes: 27 additions & 0 deletions sdk/keyvault/keyvault-common/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

nit: this should extend the common one rather than creating more debt ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woah!! Since when??? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be good now!

"compilerOptions": {
"alwaysStrict": true,
"noImplicitAny": true,
"preserveConstEnums": true,
"sourceMap": true,
"inlineSources": true,
"newLine": "LF",
"target": "es5",
"moduleResolution": "node",
"noUnusedLocals": true,
"noUnusedParameters": true,
"strict": true,
"module": "esNext",
"outDir": "./dist-esm",
"declaration": true,
"declarationMap": true,
"importHelpers": true,
"declarationDir": "./types",
"lib": ["dom", "es5", "es6", "es7", "esnext"],
"esModuleInterop": true,
"resolveJsonModule": true
},
"compileOnSave": true,
"exclude": ["node_modules"],
"include": ["./src/**/*.ts"]
}
2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-keys/api-extractor.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"mainEntryPointFilePath": "types/src/index.d.ts",
"mainEntryPointFilePath": "types/keyvault-keys/src/index.d.ts",
"docModel": {
"enabled": false
},
Expand Down
9 changes: 5 additions & 4 deletions sdk/keyvault/keyvault-keys/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@
"url": "https://github.com/Azure/azure-sdk-for-js/issues"
},
"main": "./dist/index.js",
"module": "dist-esm/src/index.js",
"module": "dist-esm/keyvault-keys/src/index.js",
"types": "./types/keyvault-keys.d.ts",
"engines": {
"node": ">=8.0.0"
},
"files": [
"types/keyvault-keys.d.ts",
"dist/",
"dist-esm/src",
"dist-esm/keyvault-keys/src",
"dist-esm/keyvault-common/src",
"README.md",
"LICENSE"
],
Expand All @@ -52,8 +53,8 @@
"extract-api": "tsc -p . && api-extractor run --local",
"format": "prettier --write --config ../../.prettierrc.json \"src/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "karma start --single-run",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 180000 --full-trace dist-esm/test/*.test.js",
"integration-test:node:no-timeout": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --no-timeouts --full-trace dist-esm/test/*.test.js",
"integration-test:node": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 180000 --full-trace dist-esm/keyvault-keys/test/*.test.js",
"integration-test:node:no-timeout": "nyc mocha -r esm --require source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --no-timeouts --full-trace dist-esm/keyvault-keys/test/*.test.js",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json api-extractor.json src test --ext .ts -f html -o keyvault-keys-lintReport.html",
Expand Down
8 changes: 4 additions & 4 deletions sdk/keyvault/keyvault-keys/rollup.base.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function nodeConfig(test = false) {
const externalNodeBuiltins = ["crypto", "fs", "os", "url", "assert"];
const additionalExternals = ["keytar"];
const baseConfig = {
input: "dist-esm/src/index.js",
input: "dist-esm/keyvault-keys/src/index.js",
external: depNames.concat(externalNodeBuiltins, additionalExternals),
output: {
file: "dist/index.js",
Expand All @@ -59,7 +59,7 @@ export function nodeConfig(test = false) {

if (test) {
// entry point is every test file
baseConfig.input = ["dist-esm/test/*.test.js"];
baseConfig.input = ["dist-esm/keyvault-keys/test/*.test.js"];
baseConfig.plugins.unshift(
multiEntry({ exports: false }),
json() // This allows us to import/require the package.json file, to get the version and test it against the user agent.
Expand All @@ -85,7 +85,7 @@ export function nodeConfig(test = false) {

export function browserConfig(test = false) {
const baseConfig = {
input: "dist-esm/src/index.js",
input: "dist-esm/keyvault-keys/src/index.js",
output: {
file: "dist-browser/azure-keyvault-keys.js",
banner: banner,
Expand Down Expand Up @@ -132,7 +132,7 @@ export function browserConfig(test = false) {

baseConfig.external = ["fs-extra", "path", "crypto", "constants"];
if (test) {
baseConfig.input = ["dist-esm/test/*.test.js"];
baseConfig.input = ["dist-esm/keyvault-keys/test/*.test.js"];
baseConfig.plugins.unshift(
multiEntry({ exports: false }),
json() // This allows us to import/require the package.json file, to get the version and test it against the user agent.
Expand Down
Loading