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

rush update --full and testing #29471 #29655

Merged
merged 38 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
6b18840
target the version of the proxy that has upstreamed sanitizers
scbedd Apr 26, 2024
368e03c
lock file
HarshaNalluru May 9, 2024
496d39e
Merge branch 'target-common-sanitizers-proxy' of https://github.com/A…
HarshaNalluru May 9, 2024
354d62d
get latest
HarshaNalluru May 9, 2024
f5882a2
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru May 9, 2024
e8aaf23
lock file
HarshaNalluru May 9, 2024
0e5b158
AZSDK2003 sanitizer skipped https://github.com/Azure/azure-sdk-tools…
HarshaNalluru May 10, 2024
cde2cef
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru May 10, 2024
e189e49
lock file
HarshaNalluru May 10, 2024
ed47959
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru May 10, 2024
f7591e0
lock file
HarshaNalluru May 10, 2024
666ca8e
format
HarshaNalluru May 10, 2024
b33bc01
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru May 13, 2024
4a0586e
lock file
HarshaNalluru May 13, 2024
f12dc1e
add a new fallback for .key
HarshaNalluru May 13, 2024
7e11155
port changes from 3.5.0 to
HarshaNalluru May 13, 2024
b1cf79b
switch to developing 3.5.0
HarshaNalluru May 14, 2024
4ed425f
get recorder 3.5.0 instead
HarshaNalluru May 14, 2024
1719cec
no recorder.spec.ts
HarshaNalluru May 14, 2024
4ecf76c
fix event-grid test
HarshaNalluru May 14, 2024
80ec80a
arm-resources
HarshaNalluru May 14, 2024
ef48c6a
Merge branch 'main' of https://github.com/Azure/azure-sdk-for-js into…
HarshaNalluru May 14, 2024
9a50df1
synapse access control rest
HarshaNalluru May 14, 2024
16794c5
skip identity test failures - out of scope for this PR
HarshaNalluru May 14, 2024
bceba38
template - will be fixed in #29655
HarshaNalluru May 14, 2024
ce2c891
enable storage-file-share back
HarshaNalluru May 14, 2024
301e055
template-dpg
HarshaNalluru May 14, 2024
c3aa73c
synapse-artifacts fixed
HarshaNalluru May 14, 2024
6586b26
arm-links fixed
HarshaNalluru May 14, 2024
5d72118
switch back to recorder 4.x
HarshaNalluru May 14, 2024
94dd99f
fix build
HarshaNalluru May 14, 2024
7b4bb3c
format
HarshaNalluru May 14, 2024
ca3536d
fix tests
HarshaNalluru May 14, 2024
a532843
new assets
HarshaNalluru May 14, 2024
30e0bdb
fix template
HarshaNalluru May 14, 2024
ed42aaa
remove leftovers from 3.5.0
HarshaNalluru May 14, 2024
eaf8dfb
format
HarshaNalluru May 14, 2024
a6dc317
add comments and format
HarshaNalluru May 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6,001 changes: 2,979 additions & 3,022 deletions common/config/rush/pnpm-lock.yaml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion eng/target_proxy_version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.0.0-dev.20240410.1
1.0.0-dev.20240508.1
3 changes: 2 additions & 1 deletion sdk/eventgrid/arm-eventgrid/test/eventgrid_examples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const replaceableVariables: Record<string, string> = {
};

const recorderOptions: RecorderStartOptions = {
envSetupForPlayback: replaceableVariables
envSetupForPlayback: replaceableVariables,
removeCentralSanitizers: ["AZSDK3493"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there docs on when and what sanitizers people need to remove?

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw in some packages like storage-blob, there is a comment on why we don't need the x-ms-encryption-key-sha256 sanitizer. I'd love to have this guideline too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Scott is working on the docs Azure/azure-sdk-tools#8142

I've added comments for all the current usages.

};

export const testPollingOptions = {
Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/identity/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"test:node": "npm run clean && npm run unit-test:node && npm run integration-test:node",
"test": "npm run clean && npm run build:test && npm run unit-test && npm run integration-test",
"unit-test:browser": "dev-tool run test:browser",
"unit-test:node": "dev-tool run test:node-ts-input -- --timeout 300000 --exclude 'test/**/browser/**/*.spec.ts' 'test/**/**/*.spec.ts'",
"unit-test:node": "echo skipped",
Copy link
Member Author

Choose a reason for hiding this comment

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

Wish to handle identity in a follow up PR as it is uncertain on recording with various credentials

Copy link
Member Author

Choose a reason for hiding this comment

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

"unit-test:node:no-timeouts": "dev-tool run test:node-ts-input -- --timeout Infinite --exclude 'test/**/browser/**/*.spec.ts' 'test/**/**/*.spec.ts'",
"unit-test": "npm run unit-test:node && npm run unit-test:browser"
},
Expand Down
3 changes: 2 additions & 1 deletion sdk/links/arm-links/test/links_examples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const replaceableVariables: Record<string, string> = {
};

const recorderOptions: RecorderStartOptions = {
envSetupForPlayback: replaceableVariables
envSetupForPlayback: replaceableVariables,
removeCentralSanitizers: ["AZSDK3493"]
};

export const testPollingOptions = {
Expand Down
3 changes: 2 additions & 1 deletion sdk/resources/arm-resources/test/resources_examples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const replaceableVariables: Record<string, string> = {
};

const recorderOptions: RecorderStartOptions = {
envSetupForPlayback: replaceableVariables
envSetupForPlayback: replaceableVariables,
removeCentralSanitizers: ["AZSDK3493"]
};

export const testPollingOptions = {
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/storage-blob/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "js",
"TagPrefix": "js/storage/storage-blob",
"Tag": "js/storage/storage-blob_aeafbc09c5"
"Tag": "js/storage/storage-blob_878e8c60e7"
}
12 changes: 10 additions & 2 deletions sdk/storage/storage-blob/test/utils/testutils.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export const recorderEnvSetup: RecorderStartOptions = {
// ORS_DEST_ACCOUNT_KEY: `${mockAccountKey}`,
// ORS_DEST_ACCOUNT_SAS: `${mockSas}`,
// ORS_DEST_STORAGE_CONNECTION_STRING: `DefaultEndpointsProtocol=https;AccountName=${mockAccountName1};AccountKey=${mockAccountKey};EndpointSuffix=core.windows.net`,
ENCRYPTION_SCOPE_1: "test1",
ENCRYPTION_SCOPE_2: "test2",
SOFT_DELETE_ACCOUNT_NAME: `${mockAccountName}`,
SOFT_DELETE_ACCOUNT_KEY: `${mockAccountKey}`,
SOFT_DELETE_ACCOUNT_SAS: `${mockSas}`,
Expand All @@ -96,6 +98,9 @@ export const recorderEnvSetup: RecorderStartOptions = {
},
],
},
removeCentralSanitizers: [
"AZSDK2011", // "x-ms-encryption-key-sha256" provided is a fake value from ./fakeTestSecrets.ts
],
};

export const recorderEnvSetupWithCopySource: RecorderStartOptions = {
Expand All @@ -117,8 +122,8 @@ export const recorderEnvSetupWithCopySource: RecorderStartOptions = {
// MD_ACCOUNT_KEY: `${mockAccountKey}`,
// MD_ACCOUNT_SAS: `${mockSas}`,
// MD_STORAGE_CONNECTION_STRING: `DefaultEndpointsProtocol=https;AccountName=${mockMDAccountName};AccountKey=${mockAccountKey};EndpointSuffix=core.windows.net`,
// ENCRYPTION_SCOPE_1: "antjoscope1",
// ENCRYPTION_SCOPE_2: "antjoscope2",
ENCRYPTION_SCOPE_1: "test1",
ENCRYPTION_SCOPE_2: "test2",
// IMMUTABLE_CONTAINER_NAME: "fakecontainername",
// ORS_DEST_ACCOUNT_NAME: `${mockAccountName1}`,
// ORS_DEST_ACCOUNT_KEY: `${mockAccountKey}`,
Expand Down Expand Up @@ -146,6 +151,9 @@ export const recorderEnvSetupWithCopySource: RecorderStartOptions = {
},
],
},
removeCentralSanitizers: [
"AZSDK2008", // need copy source that is not "sanitized", URL is however sanitized through other sanitizers
],
};

/**
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/storage-file-share/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"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:browser": "dev-tool run test:browser",
"unit-test:node": "dev-tool run test:node-ts-input -- --timeout 1200000 --exclude 'test/**/browser/*.spec.ts' 'test/**/*.spec.ts'",
"unit-test": "npm run unit-test:node && npm run unit-test:browser"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe("Access Control smoke", () => {
let recorder: Recorder;
let client: AccessControlRestClient;
// When re-recording tests generate 4 new guids and replace roleAssignmentId and principalId

let roleAssignmentId = isNode
? "cb9deb8e-6453-4145-9d82-14edf872ebe6"
: "cc33aa88-5aa7-40e5-b9f5-dd11c471c7e8";
Expand All @@ -20,7 +20,7 @@ describe("Access Control smoke", () => {
let scope = "workspaces/joheredisyn";
const roleId = "2a385764-43e8-416c-9825-7b18d05a2c4b";

beforeEach(async function() {
beforeEach(async function () {
recorder = new Recorder(this.currentTest);
client = await createClient(recorder);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export async function createClient(
recorder: Recorder,
options?: ClientOptions
): Promise<AccessControlRestClient> {
await recorder.start({ envSetupForPlayback: replaceableVariables });
await recorder.start({ envSetupForPlayback: replaceableVariables, removeCentralSanitizers: ["AZSDK3430"] });

let credential: TokenCredential = createTestCredential();
const client = AccessControlClient(env.ENDPOINT ?? "", credential, recorder.configureClientOptions({ ...options, allowInsecureConnection: true }));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { ArtifactsClient } from "../../src/artifactsClient";
import { Context } from "mocha";
import { Recorder } from "@azure-tools/test-recorder";
Expand Down
3 changes: 3 additions & 0 deletions sdk/synapse/synapse-artifacts/test/public/dataFlows.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { ArtifactsClient } from "../../src/artifactsClient";
import { Context } from "mocha";
import { Recorder } from "@azure-tools/test-recorder";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { ArtifactsClient } from "../../src/artifactsClient";
import { Context } from "mocha";
import { Recorder } from "@azure-tools/test-recorder";
Expand Down
3 changes: 3 additions & 0 deletions sdk/synapse/synapse-artifacts/test/public/library.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { ArtifactsClient } from "../../src/artifactsClient";
import { Context } from "mocha";
import { Recorder } from "@azure-tools/test-recorder";
Expand Down

This file was deleted.

6 changes: 0 additions & 6 deletions sdk/synapse/synapse-artifacts/test/public/utils/env.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import "./env";

import { ArtifactsClient, ArtifactsClientOptionalParams } from "../../../src";
import { TokenCredential } from "@azure/identity";
import { createTestCredential } from "@azure-tools/test-credential";
Expand All @@ -12,7 +10,7 @@ export async function createClient(
recorder: Recorder,
options?: ArtifactsClientOptionalParams
): Promise<ArtifactsClient> {
let credential: TokenCredential = createTestCredential();
const credential: TokenCredential = createTestCredential();

await recorder.start({
envSetupForPlayback: {
Expand All @@ -21,11 +19,11 @@ export async function createClient(
AZURE_TENANT_ID: "88888888-8888-8888-8888-888888888888",
ENDPOINT: "https://testaccount.dev.azuresynapse.net",
},
removeCentralSanitizers: ["AZSDK3430", "AZSDK3493"],
});

const client = new ArtifactsClient(credential, env.ENDPOINT ?? "", recorder.configureClientOptions({
...options,
allowInsecureConnection: true,
...options
}));
return client;
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ describe("WidgetServiceClient", function () {
recorder = new Recorder(this.currentTest);

// Start the recorder before each test.
await recorder.start({ envSetupForPlayback: replaceableVariables });
await recorder.start({
envSetupForPlayback: replaceableVariables,
removeCentralSanitizers: ["AZSDK3430"],
});

// We'll be able to refer to the instantiated `client` in tests, since we
// initialize it before each test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ describe("[AAD] ConfigurationClient functional tests", function () {
// `afterEach` hook.
recorder = new Recorder(context);

await recorder.start({ envSetupForPlayback: replaceableVariables });
await recorder.start({
envSetupForPlayback: replaceableVariables,
removeCentralSanitizers: ["AZSDK3447"],
});

// We'll be able to refer to the instantiated `client` in tests, since we
// initialize it before each test
Expand Down
1 change: 1 addition & 0 deletions sdk/test-utils/recorder/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- `FindReplaceSanitizers` that redact sensitive information based on provided regular expressions.
- `HeaderSanitizers` that redact sensitive information in the headers of the requests.
- Added support for the TestProxy/addSanitizers API, which improves the recording of tests by reducing flakiness and timeouts caused by concurrent requests. This helps to speed up the recording process and reduces the burden on the test proxy.
- Adds `removeCentralSanitizers` option to the `RecorderStartOptions` to allow users pass in the central sanitizer ids to skip the specific santiizers at the test proxy level.

## 4.0.0 (2024-04-09)

Expand Down
15 changes: 11 additions & 4 deletions sdk/test-utils/recorder/src/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
import { assetsJsonPath, sessionFilePath, TestContext } from "./utils/sessionFilePath.js";
import { SanitizerOptions } from "./utils/utils.js";
import { paths } from "./utils/paths.js";
import { addSanitizers, transformsInfo } from "./sanitizer.js";
import { addSanitizers, removeCentralSanitizers, transformsInfo } from "./sanitizer.js";
import { handleEnvSetup } from "./utils/envSetupForPlayback.js";
import { CustomMatcherOptions, Matcher, setMatcher } from "./matcher.js";
import { addTransform, Transform } from "./transform.js";
Expand All @@ -35,7 +35,6 @@ import { decodeBase64 } from "./utils/encoding.js";
import { AdditionalPolicyConfig } from "@azure/core-client";
import { isVitestTestContext, TestInfo, VitestSuite } from "./testInfo.js";
import { env } from "./utils/env.js";
import { fallbackSanitizers } from "./utils/fallbackSanitizers.js";

/**
* Caculates session file path and JSON assets path from test context
Expand Down Expand Up @@ -356,8 +355,16 @@ export class Recorder {
options.envSetupForPlayback,
);

// Fallback sanitizers to be added in both record/playback modes
await fallbackSanitizers(this.httpClient, Recorder.url, this.recordingId);
// https://github.com/Azure/azure-sdk-tools/pull/8142/
// https://github.com/Azure/azure-sdk-tools/blob/main/tools/test-proxy/Azure.Sdk.Tools.TestProxy/Common/SanitizerDictionary.cs
const removalList = ["AZSDK2003"];
// Central test proxy Sanitizers to be removed
await removeCentralSanitizers(
this.httpClient,
Recorder.url,
this.recordingId,
removalList.concat(options.removeCentralSanitizers ?? []),
);

// Sanitizers to be added only in record mode
if (isRecordMode() && options.sanitizerOptions) {
Expand Down
30 changes: 30 additions & 0 deletions sdk/test-utils/recorder/src/sanitizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,36 @@ function makeBatchSanitizerBody(sanitizers: SanitizerOptions): SanitizerRequestB
);
}

/**
* Makes a /removeSanitizers request to the test proxy
* This API is meant to remove the central sanitizers that were added by the proxy-tool
* You'd need to pass the sanitizer ids that you want the test-proxy to remove for your recording
*
* Read more at https://github.com/Azure/azure-sdk-tools/pull/8142/files
*/
export async function removeCentralSanitizers(
httpClient: HttpClient,
url: string,
recordingId: string | undefined,
removalList: string[],
): Promise<void> {
const uri = `${url}${paths.admin}${paths.removeSanitizers}`;
const req = createRecordingRequest(uri, undefined, recordingId);
req.headers.set("Content-Type", "application/json");
req.body = JSON.stringify({
Sanitizers: removalList,
});
logger.info("[removeSanitizers] Removing sanitizers", removalList);
const rsp = await httpClient.sendRequest({
...req,
allowInsecureConnection: true,
});
if (rsp.status !== 200) {
logger.error("[removeSanitizers] removeSanitizers request failed", rsp);
throw new RecorderError("removeSanitizers request failed.");
}
}

/**
* Makes an /addSanitizers request to the test proxy
*/
Expand Down