From 0842b1f131c9d784551fbd96c7f2ba417bcb8a3b Mon Sep 17 00:00:00 2001 From: Vsevolod Kukol Date: Wed, 30 Sep 2026 18:51:14 +0200 Subject: [PATCH] Protect active E2E resources from scheduled cleanup (#2595) * Protect active E2E test resources from cleanup * Validate E2E cleanup duration * Test E2E resource naming --- package.json | 2 +- test/fx.ts | 11 ++--- utils/cleanupDBs.js | 78 ++++++++++++++++++++++++---------- utils/cleanupDBs.test.js | 37 ++++++++++++++++ utils/testResourceName.js | 16 +++++++ utils/testResourceName.test.js | 52 +++++++++++++++++++++++ 6 files changed, 164 insertions(+), 32 deletions(-) create mode 100644 utils/cleanupDBs.test.js create mode 100644 utils/testResourceName.js create mode 100644 utils/testResourceName.test.js diff --git a/package.json b/package.json index 610b13075..584f27af1 100644 --- a/package.json +++ b/package.json @@ -218,7 +218,7 @@ "pack:prod": "webpack --mode production", "pack:fast": "webpack --mode development --progress", "copyToConsumers": "node copyToConsumers", - "test": "rimraf coverage && jest", + "test": "rimraf coverage && jest && node --test utils/*.test.js", "test:debug": "jest --runInBand", "test:e2e": "jest -c ./jest.config.playwright.js --detectOpenHandles", "test:file": "jest --coverage=false", diff --git a/test/fx.ts b/test/fx.ts index 337964c56..d8b16e76d 100644 --- a/test/fx.ts +++ b/test/fx.ts @@ -1,6 +1,7 @@ import { DefaultAzureCredential } from "@azure/identity"; import { Frame, Locator, Page, expect } from "@playwright/test"; -import crypto, { webcrypto } from "crypto"; +import { webcrypto } from "crypto"; +import { generateUniqueName as generateUniqueResourceName } from "../utils/testResourceName"; import { TestContainerContext } from "./testData"; // The @azure/cosmos client signs requests with globalThis.crypto (Web Crypto API). @@ -19,13 +20,7 @@ export interface TestNameOptions { } export function generateUniqueName(baseName: string, options?: TestNameOptions): string { - const length = options?.length ?? 1; - const timestamp = options?.timestampped === undefined ? true : options.timestampped; - const prefixed = options?.prefixed === undefined ? true : options.prefixed; - - const prefix = prefixed ? "t_" : ""; - const suffix = timestamp ? `_${Date.now()}` : ""; - return `${prefix}${baseName}${crypto.randomBytes(length).toString("hex")}${suffix}`; + return generateUniqueResourceName(baseName, options, process.env); } export function getAzureCLICredentials(): DefaultAzureCredential { diff --git a/utils/cleanupDBs.js b/utils/cleanupDBs.js index 233743c0f..1bdb1ef69 100644 --- a/utils/cleanupDBs.js +++ b/utils/cleanupDBs.js @@ -5,7 +5,26 @@ const ms = require("ms"); const subscriptionId = process.env["AZURE_SUBSCRIPTION_ID"]; const resourceGroupName = process.env["E2ETESTS_RESOURCEGROUP_NAME"]; -const thirtyMinutesAgo = new Date(Date.now() - 1000 * 60 * 30).getTime(); +function parseCleanupMinimumAge(configuredAge) { + let parsedAge; + try { + parsedAge = ms(configuredAge === undefined ? "6h" : configuredAge); + } catch { + parsedAge = undefined; + } + + if (!Number.isFinite(parsedAge) || parsedAge <= 0) { + throw new Error("E2E_CLEANUP_MINIMUM_AGE must be a positive duration"); + } + + return parsedAge; +} +const cleanupMinimumAge = parseCleanupMinimumAge(process.env["E2E_CLEANUP_MINIMUM_AGE"]); +const cleanupThreshold = Date.now() - cleanupMinimumAge; + +function shouldDeleteResource(name, timestamp, threshold = cleanupThreshold) { + return Boolean(name?.startsWith("t_") && timestamp && timestamp < threshold); +} function friendlyTime(date) { try { @@ -29,22 +48,27 @@ async function main() { for await (const database of mongoDatabases) { // Unfortunately Mongo does not provide a timestamp in ARM. There is no way to tell how old the DB is other thn encoding it in the ID :( const timestamp = Number(database.name.split("_").pop()); - if (timestamp && timestamp < thirtyMinutesAgo) { - await client.mongoDBResources.beginDeleteMongoDBDatabaseAndWait(resourceGroupName, account.name, database.name); + if (shouldDeleteResource(database.name, timestamp)) { + await client.mongoDBResources.beginDeleteMongoDBDatabaseAndWait( + resourceGroupName, + account.name, + database.name, + ); console.log(`DELETED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); } else { console.log(`SKIPPED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); } } } else if (account.capabilities.find((c) => c.name === "EnableCassandra")) { - const cassandraDatabases = client.cassandraResources.listCassandraKeyspaces( - resourceGroupName, - account.name, - ); + const cassandraDatabases = client.cassandraResources.listCassandraKeyspaces(resourceGroupName, account.name); for await (const database of cassandraDatabases) { const timestamp = Number(database.resource.ts) * 1000; - if (timestamp && timestamp < thirtyMinutesAgo) { - await client.cassandraResources.beginDeleteCassandraKeyspaceAndWait(resourceGroupName, account.name, database.name); + if (shouldDeleteResource(database.name, timestamp)) { + await client.cassandraResources.beginDeleteCassandraKeyspaceAndWait( + resourceGroupName, + account.name, + database.name, + ); console.log(`DELETED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); } else { console.log(`SKIPPED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); @@ -54,7 +78,7 @@ async function main() { const tablesDatabase = client.tableResources.listTables(resourceGroupName, account.name); for await (const database of tablesDatabase) { const timestamp = Number(database.resource.ts) * 1000; - if (timestamp && timestamp < thirtyMinutesAgo) { + if (shouldDeleteResource(database.name, timestamp)) { await client.tableResources.beginDeleteTableAndWait(resourceGroupName, account.name, database.name); console.log(`DELETED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); } else { @@ -65,8 +89,12 @@ async function main() { const graphDatabases = client.gremlinResources.listGremlinDatabases(resourceGroupName, account.name); for await (const database of graphDatabases) { const timestamp = Number(database.resource.ts) * 1000; - if (timestamp && timestamp < thirtyMinutesAgo) { - await client.gremlinResources.beginDeleteGremlinDatabaseAndWait(resourceGroupName, account.name, database.name); + if (shouldDeleteResource(database.name, timestamp)) { + await client.gremlinResources.beginDeleteGremlinDatabaseAndWait( + resourceGroupName, + account.name, + database.name, + ); console.log(`DELETED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); } else { console.log(`SKIPPED: ${account.name} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); @@ -92,7 +120,7 @@ async function deleteWithRetry(client, database, accountName) { while (attempt < maxRetries) { try { const timestamp = Number(database.resource.ts) * 1000; - if (timestamp && timestamp < thirtyMinutesAgo) { + if (shouldDeleteResource(database.name, timestamp)) { await client.sqlResources.beginDeleteSqlDatabaseAndWait(resourceGroupName, accountName, database.name); console.log(`DELETED: ${accountName} | ${database.name} | Age: ${friendlyTime(Date.now() - timestamp)}`); } else { @@ -118,15 +146,19 @@ async function deleteWithRetry(client, database, accountName) { // Helper function to delay the retry attempts function delay(ms) { - return new Promise(resolve => setTimeout(resolve, ms)); + return new Promise((resolve) => setTimeout(resolve, ms)); } -main() - .then(() => { - console.log("Completed"); - process.exit(0); - }) - .catch((err) => { - console.error(err); - process.exit(1); - }); \ No newline at end of file +if (require.main === module) { + main() + .then(() => { + console.log("Completed"); + process.exit(0); + }) + .catch((err) => { + console.error(err); + process.exit(1); + }); +} + +module.exports = { parseCleanupMinimumAge, shouldDeleteResource }; diff --git a/utils/cleanupDBs.test.js b/utils/cleanupDBs.test.js new file mode 100644 index 000000000..e0dc4c4d8 --- /dev/null +++ b/utils/cleanupDBs.test.js @@ -0,0 +1,37 @@ +const assert = require("node:assert/strict"); +const test = require("node:test"); +const { parseCleanupMinimumAge, shouldDeleteResource } = require("./cleanupDBs"); + +const cleanupThreshold = Date.now(); + +test("uses a six-hour cleanup age when no value is configured", () => { + assert.equal(parseCleanupMinimumAge(undefined), 6 * 60 * 60 * 1000); +}); + +test("accepts a finite positive cleanup age", () => { + assert.equal(parseCleanupMinimumAge("12h"), 12 * 60 * 60 * 1000); +}); + +test("rejects unsafe cleanup ages", () => { + for (const configuredAge of ["", "invalid", "0ms", "-1h", "Infinity"]) { + assert.throws(() => parseCleanupMinimumAge(configuredAge), /E2E_CLEANUP_MINIMUM_AGE must be a positive duration/); + } +}); + +test("deletes owned test resources older than the threshold", () => { + assert.equal(shouldDeleteResource("t_12345_1_dbab_1000", cleanupThreshold - 1, cleanupThreshold), true); +}); + +test("keeps owned test resources at or newer than the threshold", () => { + assert.equal(shouldDeleteResource("t_12345_1_dbab_1000", cleanupThreshold, cleanupThreshold), false); + assert.equal(shouldDeleteResource("t_12345_1_dbab_1000", cleanupThreshold + 1, cleanupThreshold), false); +}); + +test("keeps resources that are not marked as test-owned", () => { + assert.equal(shouldDeleteResource("seeded-database", cleanupThreshold - 1, cleanupThreshold), false); +}); + +test("keeps resources without a valid name or timestamp", () => { + assert.equal(shouldDeleteResource(undefined, cleanupThreshold - 1, cleanupThreshold), false); + assert.equal(shouldDeleteResource("t_12345_1_dbab_1000", undefined, cleanupThreshold), false); +}); diff --git a/utils/testResourceName.js b/utils/testResourceName.js new file mode 100644 index 000000000..ccfa03a84 --- /dev/null +++ b/utils/testResourceName.js @@ -0,0 +1,16 @@ +const crypto = require("crypto"); + +function generateUniqueName(baseName, options, environment = process.env) { + const length = options?.length ?? 1; + const timestamp = options?.timestampped === undefined ? true : options.timestampped; + const prefixed = options?.prefixed === undefined ? true : options.prefixed; + + const runId = environment.GITHUB_RUN_ID; + const runAttempt = environment.GITHUB_RUN_ATTEMPT ?? "1"; + const runPrefix = runId ? `${runId}_${runAttempt}_` : ""; + const prefix = prefixed ? `t_${runPrefix}` : ""; + const suffix = timestamp ? `_${Date.now()}` : ""; + return `${prefix}${baseName}${crypto.randomBytes(length).toString("hex")}${suffix}`; +} + +module.exports = { generateUniqueName }; diff --git a/utils/testResourceName.test.js b/utils/testResourceName.test.js new file mode 100644 index 000000000..f72871617 --- /dev/null +++ b/utils/testResourceName.test.js @@ -0,0 +1,52 @@ +const assert = require("node:assert/strict"); +const test = require("node:test"); +const { generateUniqueName } = require("./testResourceName"); + +const noRandomSuffix = { length: 0 }; + +test("includes the workflow run and attempt in CI resource names", () => { + const name = generateUniqueName("db", noRandomSuffix, { + GITHUB_RUN_ID: "12345", + GITHUB_RUN_ATTEMPT: "2", + }); + + assert.match(name, /^t_12345_2_db_\d+$/); +}); + +test("defaults a missing workflow attempt to one", () => { + const name = generateUniqueName("db", noRandomSuffix, { GITHUB_RUN_ID: "12345" }); + + assert.match(name, /^t_12345_1_db_\d+$/); +}); + +test("preserves the local resource name format outside GitHub Actions", () => { + const name = generateUniqueName("db", noRandomSuffix, {}); + + assert.match(name, /^t_db_\d+$/); +}); + +test("omits the test and workflow prefixes when requested", () => { + const name = generateUniqueName( + "db", + { ...noRandomSuffix, prefixed: false }, + { + GITHUB_RUN_ID: "12345", + GITHUB_RUN_ATTEMPT: "2", + }, + ); + + assert.match(name, /^db_\d+$/); +}); + +test("omits the timestamp when requested", () => { + const name = generateUniqueName( + "db", + { ...noRandomSuffix, timestampped: false }, + { + GITHUB_RUN_ID: "12345", + GITHUB_RUN_ATTEMPT: "2", + }, + ); + + assert.equal(name, "t_12345_2_db"); +});