diff --git a/.projenrc.ts b/.projenrc.ts index 4f0ea9a54..4a13e8a5c 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -1456,6 +1456,7 @@ const integRunner = configureProject( '@types/yargs', 'constructs@^10', '@aws-cdk/integ-tests-alpha@2.184.1-alpha.0', + 'fast-check@^3.23.2', ], allowPrivateDeps: true, tsconfig: { diff --git a/packages/@aws-cdk/integ-runner/.projen/deps.json b/packages/@aws-cdk/integ-runner/.projen/deps.json index f48ff2aa7..848180b63 100644 --- a/packages/@aws-cdk/integ-runner/.projen/deps.json +++ b/packages/@aws-cdk/integ-runner/.projen/deps.json @@ -94,6 +94,11 @@ "version": "^9", "type": "build" }, + { + "name": "fast-check", + "version": "^3.23.2", + "type": "build" + }, { "name": "jest", "type": "build" diff --git a/packages/@aws-cdk/integ-runner/lib/cli.ts b/packages/@aws-cdk/integ-runner/lib/cli.ts index 68202a79c..b3e5f4761 100644 --- a/packages/@aws-cdk/integ-runner/lib/cli.ts +++ b/packages/@aws-cdk/integ-runner/lib/cli.ts @@ -8,7 +8,7 @@ import type { IntegTest, IntegTestInfo } from './runner/integration-tests'; import { IntegrationTests } from './runner/integration-tests'; import { processUnstableFeatures, availableFeaturesDescription } from './unstable-features'; import type { IntegRunnerMetrics, IntegTestWorkerConfig, DestructiveChange } from './workers'; -import { runSnapshotTests, runIntegrationTests } from './workers'; +import { runSnapshotTests, runIntegrationTests, printEnvironmentsSummary } from './workers'; import { watchIntegrationTest } from './workers/integ-watch-worker'; // https://github.com/yargs/yargs/issues/1929 @@ -184,7 +184,7 @@ async function run(options: ReturnType) { // run integration tests if `--update-on-failed` OR `--force` is used if (options.runUpdateOnFailed || options.force) { - const { success, metrics } = await runIntegrationTests({ + const { success, metrics, testEnvironments } = await runIntegrationTests({ pool, tests: testsToRun, regions: options.testRegions, @@ -197,6 +197,9 @@ async function run(options: ReturnType) { }); testsSucceeded = success; + // Print summary of removed environments due to bootstrap errors + printEnvironmentsSummary(testEnvironments); + if (options.clean === false) { logger.warning('Not cleaning up stacks since "--no-clean" was used'); } diff --git a/packages/@aws-cdk/integ-runner/lib/workers/common.ts b/packages/@aws-cdk/integ-runner/lib/workers/common.ts index 3424d4405..42e148ded 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/common.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/common.ts @@ -2,6 +2,7 @@ import { format } from 'util'; import type { ResourceImpact } from '@aws-cdk/cloudformation-diff'; import * as chalk from 'chalk'; import * as logger from '../logger'; +import type { TestEnvironment, EnvironmentSummary } from './environment-pool'; import type { IntegTestInfo } from '../runner/integration-tests'; /** @@ -118,6 +119,11 @@ export interface IntegBatchResponse { * list represents metrics from a single worker (account + region). */ readonly metrics: IntegRunnerMetrics[]; + + /** + * Summary of the environments involed in the test run. + */ + readonly testEnvironments: EnvironmentSummary; } /** @@ -220,6 +226,11 @@ export enum DiagnosticReason { * The assertion failed */ ASSERTION_FAILED = 'ASSERTION_FAILED', + + /** + * The environment is not bootstrapped - test can be retried in a different environment + */ + NOT_BOOTSTRAPPED = 'NOT_BOOTSTRAPPED', } /** @@ -235,7 +246,7 @@ export interface Diagnostic { /** * The name of the stack */ - readonly stackName: string; + readonly stackName?: string; /** * The diagnostic message @@ -261,6 +272,12 @@ export interface Diagnostic { * Relevant config options that were used for the integ test */ readonly config?: Record; + + /** + * The environment where the diagnostic occurred. + * Used for NOT_BOOTSTRAPPED diagnostics to track which environment failed. + */ + readonly environment?: TestEnvironment; } export function printSummary(total: number, failed: number): void { @@ -281,35 +298,45 @@ export function formatAssertionResults(results: AssertionResults): string { .join('\n '); } +/** + * Formats a status keyword with 2 spaces prefix and right-padded to 12 characters total. + */ +function formatStatus(keyword: string): string { + return ` ${keyword}`.padEnd(12); +} + /** * Print out the results from tests */ export function printResults(diagnostic: Diagnostic): void { switch (diagnostic.reason) { case DiagnosticReason.SNAPSHOT_SUCCESS: - logger.success(' UNCHANGED %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); + logger.success('%s %s %s', formatStatus('UNCHANGED'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); break; case DiagnosticReason.TEST_SUCCESS: - logger.success(' SUCCESS %s %s\n ', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); + logger.success('%s %s %s\n ', formatStatus('SUCCESS'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); break; case DiagnosticReason.NO_SNAPSHOT: - logger.error(' NEW %s %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); + logger.error('%s %s %s', formatStatus('NEW'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`)); break; case DiagnosticReason.SNAPSHOT_FAILED: - logger.error(' CHANGED %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); + logger.error('%s %s %s\n%s', formatStatus('CHANGED'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); break; case DiagnosticReason.TEST_WARNING: - logger.warning(' WARN %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); + logger.warning('%s %s %s\n%s', formatStatus('WARN'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); break; case DiagnosticReason.SNAPSHOT_ERROR: case DiagnosticReason.TEST_ERROR: - logger.error(' ERROR %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); + logger.error('%s %s %s\n%s', formatStatus('ERROR'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); break; case DiagnosticReason.TEST_FAILED: - logger.error(' FAILED %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); + logger.error('%s %s %s\n%s', formatStatus('FAILED'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); break; case DiagnosticReason.ASSERTION_FAILED: - logger.error(' ASSERT %s %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); + logger.error('%s %s %s\n%s', formatStatus('ASSERT'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); + break; + case DiagnosticReason.NOT_BOOTSTRAPPED: + logger.warning('%s %s %s\n%s', formatStatus('ENV'), diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), indentLines(diagnostic.message, 6)); break; } for (const addl of diagnostic.additionalMessages ?? []) { @@ -344,3 +371,24 @@ export function formatError(error: any): string { return `${name}: ${message}`; } + +/** + * Prints a summary of environments that were removed due to bootstrap errors + */ +export function printEnvironmentsSummary(summary: EnvironmentSummary): void { + if (summary.removed.length === 0) { + return; + } + + logger.warning('\n%s', chalk.bold('Environments removed due to bootstrap errors:')); + + for (const env of summary.removed) { + const profileStr = env.profile ? `${env.profile}/` : ''; + const accountStr = env.account ? `aws://${env.account}/${env.region}` : env.region; + + logger.warning(' • %s%s', profileStr, env.region); + logger.warning(' Run: %s', chalk.blue(`cdk bootstrap ${accountStr}`)); + } + + logger.warning(''); +} diff --git a/packages/@aws-cdk/integ-runner/lib/workers/environment-pool.ts b/packages/@aws-cdk/integ-runner/lib/workers/environment-pool.ts new file mode 100644 index 000000000..6a038820b --- /dev/null +++ b/packages/@aws-cdk/integ-runner/lib/workers/environment-pool.ts @@ -0,0 +1,101 @@ +/** + * Identifies a specific profile+region combination (an "environment" for test execution) + */ +export interface TestEnvironment { + readonly profile?: string; + readonly region: string; + readonly account?: string; +} + +export interface EnvironmentSummary { + /** + * Enviornments that got removed from the pool during the test run.s + */ + readonly removed: RemovedEnvironment[]; +} + +/** + * Information about why an environment was removed + */ +export interface RemovedEnvironment extends TestEnvironment { + readonly reason: string; + readonly removedAt: Date; +} + +/** + * Manages a pool of test environments for integration test workers. + * + * This class serves as a centralized pool for test environments, handling: + * - Tracking which environments are available vs removed + * - Recording removal reasons for reporting + * + * Future extensions could include: + * - Load balancing across environments + * - Rate limiting per environment + * - Environment health scoring + * - Automatic environment recovery + */ +export class EnvironmentPool { + private readonly availableEnvironments: Set; + private readonly removedEnvironments: Map = new Map(); + + constructor(environments: TestEnvironment[]) { + this.availableEnvironments = new Set(environments.map(e => this.makeKey(e))); + } + + /** + * Creates a unique key for a profile+region combination + */ + private makeKey(env: TestEnvironment): string { + return `${env.profile ?? 'default'}:${env.region}`; + } + + /** + * Parses a key back into a TestEnvironment + */ + private parseKey(key: string): TestEnvironment { + const [profile, region] = key.split(':'); + return { + profile: profile === 'default' ? undefined : profile, + region, + }; + } + + /** + * Marks an environment as removed (unavailable for future tests) + */ + public removeEnvironment(env: TestEnvironment, reason: string): void { + const key = this.makeKey(env); + if (this.availableEnvironments.has(key)) { + this.availableEnvironments.delete(key); + this.removedEnvironments.set(key, { + ...env, + reason, + removedAt: new Date(), + }); + } + } + + /** + * Checks if an environment is still available + */ + public isAvailable(env: TestEnvironment): boolean { + return this.availableEnvironments.has(this.makeKey(env)); + } + + /** + * Gets all available environments + */ + public getAvailableEnvironments(): TestEnvironment[] { + return Array.from(this.availableEnvironments).map(key => this.parseKey(key)); + } + + /** + * Gets all removed environments with their removal info + */ + public summary(): EnvironmentSummary { + return { + removed: Array.from(this.removedEnvironments.values()), + }; + } +} diff --git a/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts b/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts index 0761f7f5e..b15f816f2 100644 --- a/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts +++ b/packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts @@ -1,3 +1,4 @@ +import { ToolkitError } from '@aws-cdk/toolkit-lib'; import * as workerpool from 'workerpool'; import { IntegSnapshotRunner, IntegTestRunner } from '../../runner'; import type { IntegTestInfo } from '../../runner/integration-tests'; @@ -58,23 +59,41 @@ export async function integTestWorker(request: IntegTestBatchRequest): Promise { +export async function runIntegrationTests(options: IntegTestRunOptions): Promise { logger.highlight('\nRunning integration tests for failed tests...\n'); logger.print( 'Running in parallel across %sregions: %s', @@ -59,6 +82,7 @@ export async function runIntegrationTests(options: IntegTestRunOptions): Promise return { success: responses.failedTests.length === 0, metrics: responses.metrics, + testEnvironments: responses.testEnvironments, }; } @@ -113,20 +137,74 @@ export async function runIntegrationTestsInParallel( options: IntegTestRunOptions, ): Promise { const queue = options.tests; - const results: IntegBatchResponse = { + const results: Omit = { metrics: [], failedTests: [], }; const accountWorkers: AccountWorker[] = getAccountWorkers(options.regions, options.profiles); + // Create EnvironmentPool from initial environments + const initialEnvironments: TestEnvironment[] = accountWorkers.map(w => ({ + profile: w.profile, + region: w.region, + })); + const environmentPool = new EnvironmentPool(initialEnvironments); + + // Track retryable failures that need to be re-queued + const retryQueue: IntegTestWorkerConfig[] = []; + async function runTest(worker: AccountWorker): Promise { const start = Date.now(); const tests: { [testName: string]: number } = {}; + const workerEnv: TestEnvironment = { profile: worker.profile, region: worker.region }; + do { - const test = queue.pop(); + // Check if this worker's environment is still available + if (!environmentPool.isAvailable(workerEnv)) { + // Environment was removed due to bootstrap error, stop this worker + break; + } + + // Try to get a test from the main queue first, then from retry queue + let test = queue.pop(); + if (!test && retryQueue.length > 0) { + test = retryQueue.pop(); + } if (!test) break; + const testStart = Date.now(); logger.highlight(`Running test ${test.fileName} in ${worker.profile ? worker.profile + '/' : ''}${worker.region}`); + + // Create a message handler that processes diagnostics and handles NOT_BOOTSTRAPPED + const handleWorkerMessage = (diagnostic: Diagnostic) => { + if (diagnostic.reason === DiagnosticReason.NOT_BOOTSTRAPPED) { + // Handle bootstrap error - remove environment and potentially retry test + if (diagnostic.environment && environmentPool.isAvailable(diagnostic.environment)) { + environmentPool.removeEnvironment(diagnostic.environment, diagnostic.message); + emitEnvironmentRemovedWarning(diagnostic.environment, diagnostic.message); + } + + const availableEnvs = environmentPool.getAvailableEnvironments(); + if (availableEnvs.length > 0) { + retryQueue.push({ + fileName: test.fileName, + discoveryRoot: test.discoveryRoot, + }); + emitTestRetryInfo(test.fileName); + } else { + // No valid environments remain - add to failed tests + results.failedTests.push({ + fileName: test.fileName, + discoveryRoot: test.discoveryRoot, + }); + logger.print(chalk.red(` No valid environments remaining for test ${test.fileName}`)); + } + } + + // Handle regular diagnostic messages + printResults(diagnostic); + }; + const response: IntegTestInfo[][] = await options.pool.exec('integTestWorker', [{ watch: options.watch, region: worker.region, @@ -137,12 +215,13 @@ export async function runIntegrationTestsInParallel( verbosity: options.verbosity, updateWorkflow: options.updateWorkflow, }], { - on: printResults, + on: handleWorkerMessage, }); results.failedTests.push(...flatten(response)); tests[test.fileName] = (Date.now() - testStart) / 1000; - } while (queue.length > 0); + } while (queue.length > 0 || retryQueue.length > 0); + const metrics: IntegRunnerMetrics = { region: worker.region, profile: worker.profile, @@ -158,5 +237,29 @@ export async function runIntegrationTestsInParallel( // Workers are their own concurrency limits // eslint-disable-next-line @cdklabs/promiseall-no-unbounded-parallelism await Promise.all(workers); - return results; + + return { + ...results, + // Return environments summary in ressults for summary reporting + testEnvironments: environmentPool.summary(), + }; +} + +/** + * Emits a warning when an environment is removed due to a bootstrap error + */ +function emitEnvironmentRemovedWarning(environment: TestEnvironment, reason: string): void { + const profileStr = environment.profile ? `${environment.profile}/` : ''; + const accountStr = environment.account ? `aws://${environment.account}/${environment.region}` : environment.region; + + logger.warning(`\n⚠️ Environment ${profileStr}${environment.region} removed due to bootstrap error`); + logger.warning(` Reason: ${reason}`); + logger.warning(` Run: ${chalk.blue(`cdk bootstrap ${accountStr}`)}\n`); +} + +/** + * Emits an info message when a test is being retried in a different environment + */ +function emitTestRetryInfo(testFileName: string): void { + logger.print(` ℹ️ Test ${chalk.cyan(testFileName)} will be retried in a different environment`); } diff --git a/packages/@aws-cdk/integ-runner/package.json b/packages/@aws-cdk/integ-runner/package.json index a2f35d552..ca852fbc0 100644 --- a/packages/@aws-cdk/integ-runner/package.json +++ b/packages/@aws-cdk/integ-runner/package.json @@ -55,6 +55,7 @@ "eslint-plugin-jest": "^29.12.1", "eslint-plugin-jsdoc": "^62.4.1", "eslint-plugin-prettier": "^5.5.4", + "fast-check": "^3.23.2", "jest": "^29.7.0", "jest-junit": "^16", "license-checker": "^25.0.1", diff --git a/packages/@aws-cdk/integ-runner/test/workers/environment-pool.test.ts b/packages/@aws-cdk/integ-runner/test/workers/environment-pool.test.ts new file mode 100644 index 000000000..4853bee9a --- /dev/null +++ b/packages/@aws-cdk/integ-runner/test/workers/environment-pool.test.ts @@ -0,0 +1,478 @@ +import * as fc from 'fast-check'; +import type { TestEnvironment } from '../../lib/workers/environment-pool'; +import { EnvironmentPool } from '../../lib/workers/environment-pool'; + +describe('EnvironmentPool', () => { + describe('constructor', () => { + test('initializes with provided environments', () => { + // GIVEN + const environments: TestEnvironment[] = [ + { region: 'us-east-1' }, + { region: 'us-west-2', profile: 'dev' }, + ]; + + // WHEN + const pool = new EnvironmentPool(environments); + + // THEN + expect(pool.isAvailable({ region: 'us-east-1' })).toBe(true); + expect(pool.isAvailable({ region: 'us-west-2', profile: 'dev' })).toBe(true); + expect(pool.getAvailableEnvironments()).toHaveLength(2); + }); + + test('initializes with empty environments list', () => { + // WHEN + const pool = new EnvironmentPool([]); + + // THEN + expect(pool.getAvailableEnvironments()).toHaveLength(0); + expect(pool.summary().removed).toHaveLength(0); + }); + + test('handles duplicate environments', () => { + // GIVEN - same environment specified twice + const environments: TestEnvironment[] = [ + { region: 'us-east-1' }, + { region: 'us-east-1' }, + ]; + + // WHEN + const pool = new EnvironmentPool(environments); + + // THEN - should deduplicate + expect(pool.getAvailableEnvironments()).toHaveLength(1); + }); + }); + + describe('removeEnvironment', () => { + test('removes environment correctly', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1' }, + { region: 'us-west-2' }, + ]); + + // WHEN + pool.removeEnvironment({ region: 'us-east-1' }, 'Not bootstrapped'); + + // THEN + expect(pool.isAvailable({ region: 'us-east-1' })).toBe(false); + expect(pool.isAvailable({ region: 'us-west-2' })).toBe(true); + expect(pool.getAvailableEnvironments()).toHaveLength(1); + }); + + test('records removal reason and account', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1', account: '123456789012' }]); + + // WHEN + pool.removeEnvironment({ region: 'us-east-1', account: '123456789012' }, 'Bootstrap stack not found'); + + // THEN + const { removed } = pool.summary(); + expect(removed).toHaveLength(1); + expect(removed[0].region).toBe('us-east-1'); + expect(removed[0].reason).toBe('Bootstrap stack not found'); + expect(removed[0].account).toBe('123456789012'); + expect(removed[0].removedAt).toBeInstanceOf(Date); + }); + + test('does not add to removed list if environment was not available', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + + // WHEN - try to remove an environment that doesn't exist + pool.removeEnvironment({ region: 'eu-west-1' }, 'Not bootstrapped'); + + // THEN + expect(pool.summary().removed).toHaveLength(0); + }); + + test('removing same environment twice has no effect', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + pool.removeEnvironment({ region: 'us-east-1' }, 'First removal'); + + // WHEN + pool.removeEnvironment({ region: 'us-east-1' }, 'Second removal'); + + // THEN - should still only have one removal record + expect(pool.summary().removed).toHaveLength(1); + expect(pool.summary().removed[0].reason).toBe('First removal'); + }); + }); + + describe('isAvailable', () => { + test('returns true for available environment', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + + // THEN + expect(pool.isAvailable({ region: 'us-east-1' })).toBe(true); + }); + + test('returns false for removed environment', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + pool.removeEnvironment({ region: 'us-east-1' }, 'Not bootstrapped'); + + // THEN + expect(pool.isAvailable({ region: 'us-east-1' })).toBe(false); + }); + + test('returns false for environment never added', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + + // THEN + expect(pool.isAvailable({ region: 'eu-west-1' })).toBe(false); + }); + + test('treats undefined profile as default profile', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + + // THEN - both should refer to the same environment + expect(pool.isAvailable({ region: 'us-east-1' })).toBe(true); + expect(pool.isAvailable({ region: 'us-east-1', profile: undefined })).toBe(true); + }); + }); + + describe('profile+region combinations', () => { + test('tracks profile+region combinations independently', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1', profile: 'profile1' }, + { region: 'us-east-1', profile: 'profile2' }, + { region: 'us-east-1' }, // default profile + ]); + + // THEN - all three should be available + expect(pool.isAvailable({ region: 'us-east-1', profile: 'profile1' })).toBe(true); + expect(pool.isAvailable({ region: 'us-east-1', profile: 'profile2' })).toBe(true); + expect(pool.isAvailable({ region: 'us-east-1' })).toBe(true); + expect(pool.getAvailableEnvironments()).toHaveLength(3); + }); + + test('removing one profile does not affect other profiles in same region', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1', profile: 'profile1' }, + { region: 'us-east-1', profile: 'profile2' }, + ]); + + // WHEN + pool.removeEnvironment({ region: 'us-east-1', profile: 'profile1' }, 'Not bootstrapped'); + + // THEN + expect(pool.isAvailable({ region: 'us-east-1', profile: 'profile1' })).toBe(false); + expect(pool.isAvailable({ region: 'us-east-1', profile: 'profile2' })).toBe(true); + }); + + test('same region with different profiles are distinct', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1', profile: 'dev' }, + { region: 'us-east-1', profile: 'prod' }, + ]); + + // WHEN - remove dev profile + pool.removeEnvironment({ region: 'us-east-1', profile: 'dev' }, 'Not bootstrapped'); + + // THEN + expect(pool.getAvailableEnvironments()).toEqual([ + { region: 'us-east-1', profile: 'prod' }, + ]); + expect(pool.summary().removed).toHaveLength(1); + expect(pool.summary().removed[0].profile).toBe('dev'); + }); + }); + + describe('getAvailableEnvironments', () => { + test('returns all available environments', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1' }, + { region: 'us-west-2', profile: 'dev' }, + ]); + + // THEN + const available = pool.getAvailableEnvironments(); + expect(available).toHaveLength(2); + expect(available).toContainEqual({ region: 'us-east-1', profile: undefined }); + expect(available).toContainEqual({ region: 'us-west-2', profile: 'dev' }); + }); + + test('excludes removed environments', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1' }, + { region: 'us-west-2' }, + ]); + pool.removeEnvironment({ region: 'us-east-1' }, 'Not bootstrapped'); + + // THEN + const available = pool.getAvailableEnvironments(); + expect(available).toHaveLength(1); + expect(available[0].region).toBe('us-west-2'); + }); + }); + + describe('getRemovedEnvironments', () => { + test('returns empty array when no environments removed', () => { + // GIVEN + const pool = new EnvironmentPool([{ region: 'us-east-1' }]); + + // THEN + expect(pool.summary()).toEqual({ removed: [] }); + }); + + test('returns all removed environments with info', () => { + // GIVEN + const pool = new EnvironmentPool([ + { region: 'us-east-1', account: '111111111111' }, + { region: 'us-west-2', profile: 'dev', account: '222222222222' }, + ]); + + // WHEN + pool.removeEnvironment({ region: 'us-east-1', account: '111111111111' }, 'Reason 1'); + pool.removeEnvironment({ region: 'us-west-2', profile: 'dev', account: '222222222222' }, 'Reason 2'); + + // THEN + const { removed } = pool.summary(); + expect(removed).toHaveLength(2); + + const env1 = removed.find(e => e.region === 'us-east-1'); + expect(env1).toBeDefined(); + expect(env1!.reason).toBe('Reason 1'); + expect(env1!.account).toBe('111111111111'); + + const env2 = removed.find(e => e.region === 'us-west-2'); + expect(env2).toBeDefined(); + expect(env2!.profile).toBe('dev'); + expect(env2!.reason).toBe('Reason 2'); + expect(env2!.account).toBe('222222222222'); + }); + }); +}); + +/** + * Property-Based Tests for EnvironmentPool + * + * These tests verify universal properties that should hold across all valid inputs. + */ +describe('EnvironmentPool Property-Based Tests', () => { + // Arbitrary generators for test data + const regionArb = fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '1', '2', '3'), { minLength: 1, maxLength: 15 }); + const profileArb = fc.option(fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '1', '2', '3'), { minLength: 1, maxLength: 10 }), { nil: undefined }); + const accountArb = fc.option(fc.stringMatching(/^[0-9]{12}$/), { nil: undefined }); + const reasonArb = fc.string({ minLength: 1, maxLength: 100 }); + + const testEnvironmentArb: fc.Arbitrary = fc.record({ + region: regionArb, + profile: profileArb, + account: accountArb, + }); + + /** + * Property 5: Environment Removal Tracking + * + * *For any* environment marked as removed via `EnvironmentPool.removeEnvironment()`, + * subsequent calls to `isAvailable()` with the same profile+region combination should + * return `false`, and `getRemovedEnvironments()` should include that environment. + * + * **Validates: Requirements 3.1** + */ + describe('Property 5: Environment Removal Tracking', () => { + test('removed environments are no longer available and appear in getRemovedEnvironments', () => { + fc.assert( + fc.property( + fc.array(testEnvironmentArb, { minLength: 1, maxLength: 20 }), + reasonArb, + (environments, reason) => { + // GIVEN - a pool with some environments + const pool = new EnvironmentPool(environments); + + // Pick a random environment to remove (first one for simplicity) + const envToRemove = environments[0]; + + // Verify it's initially available + const wasAvailable = pool.isAvailable(envToRemove); + + // WHEN - remove the environment + pool.removeEnvironment(envToRemove, reason); + + // THEN + // 1. isAvailable should return false for the removed environment + expect(pool.isAvailable(envToRemove)).toBe(false); + + // 2. If it was available before, it should now be in getRemovedEnvironments + if (wasAvailable) { + const { removed } = pool.summary(); + const found = removed.find( + r => r.region === envToRemove.region && r.profile === envToRemove.profile, + ); + expect(found).toBeDefined(); + expect(found!.reason).toBe(reason); + if (envToRemove.account !== undefined) { + expect(found!.account).toBe(envToRemove.account); + } + } + }, + ), + { numRuns: 100 }, + ); + }); + + test('multiple removals maintain consistency', () => { + fc.assert( + fc.property( + fc.array(testEnvironmentArb, { minLength: 2, maxLength: 10 }), + fc.array(fc.nat({ max: 9 }), { minLength: 1, maxLength: 5 }), + reasonArb, + (environments, indicesToRemove, reason) => { + // GIVEN + const pool = new EnvironmentPool(environments); + const removedSet = new Set(); + + // WHEN - remove multiple environments + for (const idx of indicesToRemove) { + const envIdx = idx % environments.length; + const env = environments[envIdx]; + const key = `${env.profile ?? 'default'}:${env.region}`; + + if (pool.isAvailable(env)) { + removedSet.add(key); + } + pool.removeEnvironment(env, reason); + } + + // THEN - all removed environments should not be available + for (const env of environments) { + const key = `${env.profile ?? 'default'}:${env.region}`; + if (removedSet.has(key)) { + expect(pool.isAvailable(env)).toBe(false); + } + } + + // And getRemovedEnvironments should contain all removed ones + const { removed } = pool.summary(); + for (const removedEnv of removed) { + const key = `${removedEnv.profile ?? 'default'}:${removedEnv.region}`; + expect(removedSet.has(key)).toBe(true); + } + }, + ), + { numRuns: 100 }, + ); + }); + }); + + /** + * Property 6: Profile-Region Independence + * + * *For any* two distinct profile+region combinations (e.g., profile1/us-east-1 and + * profile2/us-east-1), removing one environment should not affect the `isAvailable()` + * result for the other. + * + * **Validates: Requirements 3.3** + */ + describe('Property 6: Profile-Region Independence', () => { + test('removing one profile+region does not affect other combinations', () => { + fc.assert( + fc.property( + testEnvironmentArb, + testEnvironmentArb, + reasonArb, + (env1, env2, reason) => { + // Skip if environments are the same + const key1 = `${env1.profile ?? 'default'}:${env1.region}`; + const key2 = `${env2.profile ?? 'default'}:${env2.region}`; + fc.pre(key1 !== key2); + + // GIVEN - a pool with both environments + const pool = new EnvironmentPool([env1, env2]); + + // Both should be available initially + expect(pool.isAvailable(env1)).toBe(true); + expect(pool.isAvailable(env2)).toBe(true); + + // WHEN - remove env1 + pool.removeEnvironment(env1, reason); + + // THEN - env2 should still be available + expect(pool.isAvailable(env1)).toBe(false); + expect(pool.isAvailable(env2)).toBe(true); + }, + ), + { numRuns: 100 }, + ); + }); + + test('same region with different profiles are independent', () => { + fc.assert( + fc.property( + regionArb, + // Use alphanumeric profiles without colons to match realistic AWS profile names + fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '_', '1', '2', '3'), { minLength: 1, maxLength: 10 }), + fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '_', '1', '2', '3'), { minLength: 1, maxLength: 10 }), + reasonArb, + (region, profile1, profile2, reason) => { + // Ensure profiles are different + fc.pre(profile1 !== profile2); + + const env1: TestEnvironment = { region, profile: profile1 }; + const env2: TestEnvironment = { region, profile: profile2 }; + + // GIVEN - same region, different profiles + const pool = new EnvironmentPool([env1, env2]); + + // WHEN - remove one + pool.removeEnvironment(env1, reason); + + // THEN - the other should still be available + expect(pool.isAvailable(env1)).toBe(false); + expect(pool.isAvailable(env2)).toBe(true); + + // And available environments should only contain env2 + const available = pool.getAvailableEnvironments(); + expect(available).toHaveLength(1); + expect(available[0].region).toBe(region); + expect(available[0].profile).toBe(profile2); + }, + ), + { numRuns: 100 }, + ); + }); + + test('default profile is independent from named profiles', () => { + fc.assert( + fc.property( + regionArb, + // Use alphanumeric profiles without colons to match realistic AWS profile names + fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '_', '1', '2', '3'), { minLength: 1, maxLength: 10 }), + reasonArb, + (region, namedProfile, reason) => { + const envDefault: TestEnvironment = { region }; + const envNamed: TestEnvironment = { region, profile: namedProfile }; + + // GIVEN - same region, one default profile, one named profile + const pool = new EnvironmentPool([envDefault, envNamed]); + + // Both should be available + expect(pool.isAvailable(envDefault)).toBe(true); + expect(pool.isAvailable(envNamed)).toBe(true); + + // WHEN - remove the default profile environment + pool.removeEnvironment(envDefault, reason); + + // THEN - named profile should still be available + expect(pool.isAvailable(envDefault)).toBe(false); + expect(pool.isAvailable(envNamed)).toBe(true); + }, + ), + { numRuns: 100 }, + ); + }); + }); +}); diff --git a/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts b/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts index dc73c2264..75d201856 100644 --- a/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts +++ b/packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts @@ -1,5 +1,6 @@ import * as builtinFs from 'fs'; import * as path from 'path'; +import { BootstrapError } from '@aws-cdk/toolkit-lib'; import * as fs from 'fs-extra'; import * as workerpool from 'workerpool'; import { IntegTestRunner } from '../../lib/runner'; @@ -301,6 +302,206 @@ describe('integTestWorker', () => { }), ); }); + + describe('bootstrap error handling', () => { + /** + * Tests for bootstrap error detection in extract_worker + * Validates: Requirements 2.1, 2.2, 3.1 + */ + + test('bootstrap error during test case execution emits NOT_BOOTSTRAPPED diagnostic', async () => { + // GIVEN + mockActualTests.mockResolvedValue({ + 'test-case-1': { stacks: ['Stack1'] }, + }); + const bootstrapError = new BootstrapError('Bootstrap stack not found', { + account: '123456789012', + region: 'us-east-1', + }); + mockRunIntegTestCase.mockRejectedValue(bootstrapError); + + // WHEN + const results = await integTestWorker({ + tests: [{ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }], + region: 'us-east-1', + profile: 'test-profile', + }); + + // THEN - test should NOT be in failedTests (it's retryable via diagnostic) + expect(results).toEqual([]); + // NOT_BOOTSTRAPPED diagnostic should be emitted with environment + expect(workerpool.workerEmit).toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'NOT_BOOTSTRAPPED', + environment: { + profile: 'test-profile', + region: 'us-east-1', + account: '123456789012', + }, + }), + ); + }); + + test('bootstrap error emits diagnostic with environment info for removal', async () => { + // GIVEN + mockActualTests.mockResolvedValue({ + 'test-case-1': { stacks: ['Stack1'] }, + }); + const bootstrapError = new BootstrapError('CDKToolkit stack not found', { + account: '123456789012', + region: 'eu-west-1', + }); + mockRunIntegTestCase.mockRejectedValue(bootstrapError); + + // WHEN + await integTestWorker({ + tests: [{ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }], + region: 'eu-west-1', + profile: 'prod-profile', + }); + + // THEN - NOT_BOOTSTRAPPED diagnostic should contain environment for removal + expect(workerpool.workerEmit).toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'NOT_BOOTSTRAPPED', + environment: { + profile: 'prod-profile', + region: 'eu-west-1', + account: '123456789012', + }, + }), + ); + }); + + test('bootstrap error emits NOT_BOOTSTRAPPED diagnostic with error message', async () => { + // GIVEN + mockActualTests.mockResolvedValue({ + 'test-case-1': { stacks: ['Stack1'] }, + }); + const bootstrapError = new BootstrapError('Bootstrap version insufficient', { + account: '123456789012', + region: 'ap-southeast-1', + }); + mockRunIntegTestCase.mockRejectedValue(bootstrapError); + + // WHEN + await integTestWorker({ + tests: [{ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }], + region: 'ap-southeast-1', + }); + + // THEN - diagnostic should be NOT_BOOTSTRAPPED with error message + expect(workerpool.workerEmit).toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'NOT_BOOTSTRAPPED', + message: expect.stringContaining('Bootstrap version insufficient'), + }), + ); + }); + + test('non-bootstrap error is handled normally and added to failedTests', async () => { + // GIVEN + mockActualTests.mockResolvedValue({ + 'test-case-1': { stacks: ['Stack1'] }, + }); + mockRunIntegTestCase.mockRejectedValue(new Error('Deployment failed: resource limit exceeded')); + + // WHEN + const results = await integTestWorker({ + tests: [{ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }], + region: 'us-west-2', + }); + + // THEN - test should be in failedTests + expect(results).toEqual([{ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }]); + // Should emit TEST_FAILED, not NOT_BOOTSTRAPPED + expect(workerpool.workerEmit).toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'TEST_FAILED', + }), + ); + expect(workerpool.workerEmit).not.toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'NOT_BOOTSTRAPPED', + }), + ); + }); + + test('non-bootstrap error at test level is handled normally', async () => { + // GIVEN - non-bootstrap error thrown during actualTests() call + mockActualTests.mockRejectedValue( + new Error('xxxxx.integ-test2 is a new test. Please use the IntegTest construct'), + ); + + // WHEN + const results = await integTestWorker({ + tests: [{ + fileName: 'test/test-data/xxxxx.integ-test2.js', + discoveryRoot: 'test/test-data', + }], + region: 'us-east-1', + }); + + // THEN - test should be in failedTests + expect(results).toEqual([{ + fileName: 'test/test-data/xxxxx.integ-test2.js', + discoveryRoot: 'test/test-data', + }]); + // Should emit TEST_ERROR, not NOT_BOOTSTRAPPED + expect(workerpool.workerEmit).toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'TEST_ERROR', + }), + ); + }); + + test('bootstrap error without profile sets profile to undefined in environment', async () => { + // GIVEN + mockActualTests.mockResolvedValue({ + 'test-case-1': { stacks: ['Stack1'] }, + }); + const bootstrapError = new BootstrapError('Not bootstrapped', { + account: '123456789012', + region: 'sa-east-1', + }); + mockRunIntegTestCase.mockRejectedValue(bootstrapError); + + // WHEN - no profile specified + await integTestWorker({ + tests: [{ + fileName: 'test/test-data/xxxxx.test-with-snapshot.js', + discoveryRoot: 'test/test-data', + }], + region: 'sa-east-1', + }); + + // THEN - environment should have undefined profile + expect(workerpool.workerEmit).toHaveBeenCalledWith( + expect.objectContaining({ + reason: 'NOT_BOOTSTRAPPED', + environment: expect.objectContaining({ + profile: undefined, + region: 'sa-east-1', + }), + }), + ); + }); + }); }); describe('parallel worker', () => { @@ -365,6 +566,9 @@ describe('parallel worker', () => { }, }, ]), + testEnvironments: { + removed: [], + }, }); }); @@ -459,6 +663,9 @@ describe('parallel worker', () => { }, }, ]), + testEnvironments: { + removed: [], + }, }); }); @@ -512,6 +719,9 @@ describe('parallel worker', () => { }, }, ]), + testEnvironments: { + removed: [], + }, }); }); @@ -559,6 +769,9 @@ describe('parallel worker', () => { }, }, ]), + testEnvironments: { + removed: [], + }, }); }); @@ -612,6 +825,417 @@ describe('parallel worker', () => { }, }, ]), + testEnvironments: { + removed: [], + }, }); }); }); + +describe('parallel worker retry logic', () => { + /** + * Tests for retry logic in runIntegrationTestsInParallel + * Validates: Requirements 3.2, 4.1, 4.2, 4.3 + * + * The new approach uses NOT_BOOTSTRAPPED diagnostics emitted via workerEmit + * instead of returning retryableFailures/environmentRemovals in the response. + * The message handler in integ-test-worker.ts processes these diagnostics. + */ + + let mockPool: workerpool.WorkerPool; + let execMock: jest.Mock; + + beforeEach(() => { + execMock = jest.fn(); + mockPool = { + exec: execMock, + terminate: jest.fn(), + } as unknown as workerpool.WorkerPool; + }); + + /** + * Helper to create a mock pool.exec that emits NOT_BOOTSTRAPPED diagnostics + * via the 'on' callback (simulating workerEmit behavior) + */ + function createMockExecWithDiagnostics( + shouldFailInRegion: (region: string, profile?: string) => boolean, + testsRunInRegion?: Record, + ) { + return (_method: string, args: any[], options?: { on?: (msg: any) => void }) => { + const request = args[0]; + const region = request.region; + const profile = request.profile; + const testName = request.tests[0].fileName; + + // Track which tests run in which region + if (testsRunInRegion) { + if (!testsRunInRegion[region]) { + testsRunInRegion[region] = []; + } + testsRunInRegion[region].push(testName); + } + + if (shouldFailInRegion(region, profile)) { + // Emit NOT_BOOTSTRAPPED diagnostic via the 'on' callback + if (options?.on) { + options.on({ + reason: 'NOT_BOOTSTRAPPED', + testName: `${testName} (${profile ? profile + '/' : ''}${region})`, + message: 'Bootstrap stack not found', + environment: { + profile, + region, + account: '123456789012', + }, + testInfo: { + fileName: testName, + discoveryRoot: request.tests[0].discoveryRoot, + }, + }); + } + // Return empty failures since the test is retryable + return Promise.resolve([]); + } + + // Success case - emit TEST_SUCCESS diagnostic + if (options?.on) { + options.on({ + reason: 'TEST_SUCCESS', + testName: `${testName} (${profile ? profile + '/' : ''}${region})`, + message: 'Test passed', + }); + } + return Promise.resolve([]); + }; + } + + test('tests are re-queued when bootstrap error occurs and other environments available', async () => { + /** + * Validates: Requirements 3.2, 4.1, 4.2 + * When a bootstrap error occurs and other valid regions remain, + * the test should be re-queued for execution in a valid region. + */ + + // GIVEN - us-east-1 fails with bootstrap error, us-east-2 succeeds + const testsRunInRegion: Record = {}; + + execMock.mockImplementation( + createMockExecWithDiagnostics( + (region) => region === 'us-east-1', + testsRunInRegion, + ), + ); + + // WHEN - run with multiple tests so us-east-2 worker stays active + const results = await runIntegrationTestsInParallel({ + pool: mockPool, + tests: [ + { fileName: 'test1.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test2.js', discoveryRoot: 'test/test-data' }, + ], + regions: ['us-east-1', 'us-east-2'], + }); + + // THEN - all tests should succeed (failed test was retried in us-east-2) + expect(results.failedTests).toEqual([]); + + // us-east-1 should only have run one test (then stopped) + expect(testsRunInRegion['us-east-1']?.length).toBe(1); + + // us-east-2 should have run multiple tests (including the retried one) + expect(testsRunInRegion['us-east-2']?.length).toBeGreaterThanOrEqual(1); + }); + + test('tests are not re-queued when all environments have been removed', async () => { + /** + * Validates: Requirements 4.3 + * If all regions have been removed due to bootstrap errors, + * the test should NOT be retried and should be reported as failed. + */ + + // GIVEN - single region that fails with bootstrap error + execMock.mockImplementation( + createMockExecWithDiagnostics(() => true), // All regions fail + ); + + // WHEN - run with only one region that will fail + const results = await runIntegrationTestsInParallel({ + pool: mockPool, + tests: [{ + fileName: 'test.js', + discoveryRoot: 'test/test-data', + }], + regions: ['us-east-1'], + }); + + // THEN - test should be in failedTests since no valid environments remain + expect(results.failedTests).toEqual([{ + fileName: 'test.js', + discoveryRoot: 'test/test-data', + }]); + }); + + test('removed environments are skipped for new tests', async () => { + /** + * Validates: Requirements 3.2 + * When a region is removed, the worker for that environment should stop + * and not schedule any new tests for that region. + */ + + // GIVEN - track which regions tests are run in + const regionsUsed: string[] = []; + + execMock.mockImplementation((_method: string, args: any[], options?: { on?: (msg: any) => void }) => { + const request = args[0]; + regionsUsed.push(request.region); + + if (request.region === 'us-east-1') { + // First region fails with bootstrap error on first test + if (options?.on) { + options.on({ + reason: 'NOT_BOOTSTRAPPED', + testName: `${request.tests[0].fileName} (${request.region})`, + message: 'Bootstrap stack not found', + environment: { + profile: request.profile, + region: request.region, + account: '123456789012', + }, + testInfo: { + fileName: request.tests[0].fileName, + discoveryRoot: request.tests[0].discoveryRoot, + }, + }); + } + return Promise.resolve([]); + } + + // us-east-2 succeeds + if (options?.on) { + options.on({ + reason: 'TEST_SUCCESS', + testName: `${request.tests[0].fileName} (${request.region})`, + message: 'Test passed', + }); + } + return Promise.resolve([]); + }); + + // WHEN - run with multiple tests + const results = await runIntegrationTestsInParallel({ + pool: mockPool, + tests: [ + { fileName: 'test1.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test2.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test3.js', discoveryRoot: 'test/test-data' }, + ], + regions: ['us-east-1', 'us-east-2'], + }); + + // THEN - us-east-1 should only be used once (then removed) + const usEast1Count = regionsUsed.filter(r => r === 'us-east-1').length; + expect(usEast1Count).toBe(1); + + // All tests should eventually succeed (retried in us-east-2) + expect(results.failedTests).toEqual([]); + }); + + test('environment removal is tracked in results for summary reporting', async () => { + /** + * Validates: Requirements 3.1 + * When an environment is removed, it should be tracked in the results + * for summary reporting at the end of the test run. + */ + + // GIVEN - region fails with bootstrap error + execMock.mockImplementation((_method: string, args: any[], options?: { on?: (msg: any) => void }) => { + const request = args[0]; + + if (request.region === 'us-east-1') { + if (options?.on) { + options.on({ + reason: 'NOT_BOOTSTRAPPED', + testName: `${request.tests[0].fileName} (${request.region})`, + message: 'Bootstrap stack not found', + environment: { + profile: request.profile, + region: request.region, + account: '123456789012', + }, + testInfo: { + fileName: request.tests[0].fileName, + discoveryRoot: request.tests[0].discoveryRoot, + }, + }); + } + return Promise.resolve([]); + } + + if (options?.on) { + options.on({ + reason: 'TEST_SUCCESS', + testName: `${request.tests[0].fileName} (${request.region})`, + message: 'Test passed', + }); + } + return Promise.resolve([]); + }); + + // WHEN - run with multiple tests so us-east-2 stays active + const results = await runIntegrationTestsInParallel({ + pool: mockPool, + tests: [ + { fileName: 'test1.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test2.js', discoveryRoot: 'test/test-data' }, + ], + regions: ['us-east-1', 'us-east-2'], + }); + + // THEN - removed environments should be tracked in results + expect(results.testEnvironments.removed).toBeDefined(); + expect(results.testEnvironments.removed).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + region: 'us-east-1', + reason: 'Bootstrap stack not found', + }), + ]), + ); + }); + + test('duplicate environment removal requests are handled correctly', async () => { + /** + * Validates: Requirements 3.1 + * When multiple tests fail in the same environment with bootstrap errors, + * the environment should only be removed once. + */ + + // GIVEN - multiple tests fail in the same region + let usEast1CallCount = 0; + execMock.mockImplementation((_method: string, args: any[], options?: { on?: (msg: any) => void }) => { + const request = args[0]; + + if (request.region === 'us-east-1') { + usEast1CallCount++; + // Emit NOT_BOOTSTRAPPED diagnostic + if (options?.on) { + options.on({ + reason: 'NOT_BOOTSTRAPPED', + testName: `${request.tests[0].fileName} (${request.region})`, + message: 'Bootstrap stack not found', + environment: { + profile: request.profile, + region: request.region, + account: '123456789012', + }, + testInfo: { + fileName: request.tests[0].fileName, + discoveryRoot: request.tests[0].discoveryRoot, + }, + }); + } + return Promise.resolve([]); + } + + if (options?.on) { + options.on({ + reason: 'TEST_SUCCESS', + testName: `${request.tests[0].fileName} (${request.region})`, + message: 'Test passed', + }); + } + return Promise.resolve([]); + }); + + // WHEN - run with multiple tests + const results = await runIntegrationTestsInParallel({ + pool: mockPool, + tests: [ + { fileName: 'test1.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test2.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test3.js', discoveryRoot: 'test/test-data' }, + ], + regions: ['us-east-1', 'us-east-2'], + }); + + // THEN - us-east-1 should only be called once (worker stops after removal) + expect(usEast1CallCount).toBe(1); + + // Only one removal entry for us-east-1 + const removedEnvs = results.testEnvironments.removed || []; + const usEast1Removals = removedEnvs.filter((e: any) => e.region === 'us-east-1'); + expect(usEast1Removals.length).toBe(1); + + // All tests should succeed (retried in us-east-2) + expect(results.failedTests).toEqual([]); + }); + + test('profile-specific environment removal only affects that profile', async () => { + /** + * Validates: Requirements 3.3 + * When multiple profiles are configured, removing a region for one profile + * should not affect the same region for other profiles. + */ + + // GIVEN - track calls by profile+region + const callsByEnv: Record = {}; + + execMock.mockImplementation((_method: string, args: any[], options?: { on?: (msg: any) => void }) => { + const request = args[0]; + const envKey = `${request.profile || 'default'}/${request.region}`; + callsByEnv[envKey] = (callsByEnv[envKey] || 0) + 1; + + // Only profile1/us-east-1 fails with bootstrap error + if (request.profile === 'profile1' && request.region === 'us-east-1') { + if (options?.on) { + options.on({ + reason: 'NOT_BOOTSTRAPPED', + testName: `${request.tests[0].fileName} (${request.profile}/${request.region})`, + message: 'Bootstrap stack not found', + environment: { + profile: request.profile, + region: request.region, + account: '111111111111', + }, + testInfo: { + fileName: request.tests[0].fileName, + discoveryRoot: request.tests[0].discoveryRoot, + }, + }); + } + return Promise.resolve([]); + } + + if (options?.on) { + options.on({ + reason: 'TEST_SUCCESS', + testName: `${request.tests[0].fileName} (${request.profile}/${request.region})`, + message: 'Test passed', + }); + } + return Promise.resolve([]); + }); + + // WHEN - run with two profiles + const results = await runIntegrationTestsInParallel({ + pool: mockPool, + tests: [ + { fileName: 'test1.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test2.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test3.js', discoveryRoot: 'test/test-data' }, + { fileName: 'test4.js', discoveryRoot: 'test/test-data' }, + ], + profiles: ['profile1', 'profile2'], + regions: ['us-east-1'], + }); + + // THEN - profile1/us-east-1 should only be called once (then removed) + expect(callsByEnv['profile1/us-east-1']).toBe(1); + + // profile2/us-east-1 should still be available and used for multiple tests + expect(callsByEnv['profile2/us-east-1']).toBeGreaterThan(1); + + // All tests should succeed + expect(results.failedTests).toEqual([]); + }); +}); diff --git a/packages/@aws-cdk/integ-runner/test/workers/notifications.test.ts b/packages/@aws-cdk/integ-runner/test/workers/notifications.test.ts new file mode 100644 index 000000000..b60006c5e --- /dev/null +++ b/packages/@aws-cdk/integ-runner/test/workers/notifications.test.ts @@ -0,0 +1,634 @@ +import * as fc from 'fast-check'; +import * as logger from '../../lib/logger'; +import { printEnvironmentsSummary } from '../../lib/workers/common'; +import type { RemovedEnvironment, TestEnvironment } from '../../lib/workers/environment-pool'; + +// Mock the logger module +jest.mock('../../lib/logger', () => ({ + print: jest.fn(), + error: jest.fn(), + warning: jest.fn(), + highlight: jest.fn(), + trace: jest.fn(), +})); + +// We need to test the emitEnvironmentRemovedWarning function which is not exported +// So we'll test it indirectly through the integration tests or extract it for testing +// For now, we'll focus on printRemovedEnvironmentsSummary which is exported + +describe('printRemovedEnvironmentsSummary', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('unit tests', () => { + test('does not print anything when no environments were removed', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = []; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - no logger calls should be made + expect(logger.warning).not.toHaveBeenCalled(); + expect(logger.print).not.toHaveBeenCalled(); + }); + + test('prints summary header when environments were removed', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'us-east-1', + reason: 'Bootstrap stack not found', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should print header + expect(logger.warning).toHaveBeenCalledWith( + '\n%s', + expect.stringContaining('Environments removed due to bootstrap errors'), + ); + }); + + test('prints region name for each removed environment', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'us-east-1', + reason: 'Not bootstrapped', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should include region name + expect(logger.warning).toHaveBeenCalledWith( + ' • %s%s', + '', + 'us-east-1', + ); + }); + + test('prints profile prefix when profile is provided', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'us-west-2', + profile: 'dev-profile', + reason: 'Not bootstrapped', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should include profile prefix + expect(logger.warning).toHaveBeenCalledWith( + ' • %s%s', + 'dev-profile/', + 'us-west-2', + ); + }); + + test('prints bootstrap command with account when account is provided', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'eu-west-1', + account: '123456789012', + reason: 'Not bootstrapped', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should include bootstrap command with account + expect(logger.warning).toHaveBeenCalledWith( + ' Run: %s', + expect.stringContaining('cdk bootstrap aws://123456789012/eu-west-1'), + ); + }); + + test('prints bootstrap command with region only when account is not provided', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'ap-southeast-1', + reason: 'Not bootstrapped', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should include bootstrap command with region only + expect(logger.warning).toHaveBeenCalledWith( + ' Run: %s', + expect.stringContaining('cdk bootstrap ap-southeast-1'), + ); + }); + + test('prints entry for each removed environment', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'us-east-1', + reason: 'Not bootstrapped', + removedAt: new Date(), + }, + { + region: 'us-west-2', + profile: 'prod', + account: '123456789012', + reason: 'Version insufficient', + removedAt: new Date(), + }, + { + region: 'eu-central-1', + account: '234567890123', + reason: 'SSM parameter not found', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should print entry for each environment + // Header + 3 environments * 2 lines each + trailing newline = 8 calls + expect(logger.warning).toHaveBeenCalledTimes(8); + + // Verify each region is mentioned + const allCalls = (logger.warning as jest.Mock).mock.calls; + const allCallsStr = JSON.stringify(allCalls); + expect(allCallsStr).toContain('us-east-1'); + expect(allCallsStr).toContain('us-west-2'); + expect(allCallsStr).toContain('eu-central-1'); + }); + + test('prints trailing newline after summary', () => { + // GIVEN + const removedEnvironments: RemovedEnvironment[] = [ + { + region: 'us-east-1', + reason: 'Not bootstrapped', + removedAt: new Date(), + }, + ]; + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - last call should be empty string (trailing newline) + const lastCall = (logger.warning as jest.Mock).mock.calls.slice(-1)[0]; + expect(lastCall).toEqual(['']); + }); + }); +}); + +/** + * Property-Based Tests for Notification Functions + * + * These tests verify universal properties that should hold across all valid inputs. + */ +describe('Notification Property-Based Tests', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + // Arbitrary generators for test data + const awsAccountArb = fc.stringMatching(/^[0-9]{12}$/); + const awsRegionArb = fc.constantFrom( + 'us-east-1', 'us-east-2', 'us-west-1', 'us-west-2', + 'eu-west-1', 'eu-west-2', 'eu-west-3', 'eu-central-1', + 'ap-northeast-1', 'ap-northeast-2', 'ap-southeast-1', 'ap-southeast-2', + 'sa-east-1', 'ca-central-1', 'me-south-1', 'af-south-1', + ); + const profileArb = fc.option( + fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '_', '1', '2', '3'), { minLength: 1, maxLength: 20 }), + { nil: undefined }, + ); + const reasonArb = fc.string({ minLength: 1, maxLength: 200 }); + + const RemovedEnvironmentArb: fc.Arbitrary = fc.record({ + region: awsRegionArb, + profile: profileArb, + account: fc.option(awsAccountArb, { nil: undefined }), + reason: reasonArb, + removedAt: fc.date(), + }); + + /** + * Property 8: Summary Content Completeness + * + * *For any* non-empty list of removed regions, the printed summary should contain + * an entry for each removed region, and each entry should include the region name, + * profile (if applicable), and a bootstrap command. + * + * **Validates: Requirements 6.2, 6.3** + */ + describe('Property 8: Summary Content Completeness', () => { + test('summary contains entry for each removed region', () => { + fc.assert( + fc.property( + fc.array(RemovedEnvironmentArb, { minLength: 1, maxLength: 10 }), + (removedEnvironments) => { + // Clear mocks before each property test iteration + jest.clearAllMocks(); + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - logger.warning should have been called + expect(logger.warning).toHaveBeenCalled(); + + // Collect all warning calls + const allCalls = (logger.warning as jest.Mock).mock.calls; + const allCallsStr = JSON.stringify(allCalls); + + // Each region should appear in the output + for (const env of removedEnvironments) { + expect(allCallsStr).toContain(env.region); + } + }, + ), + { numRuns: 100 }, + ); + }); + + test('each entry includes profile when provided', () => { + fc.assert( + fc.property( + fc.array(RemovedEnvironmentArb, { minLength: 1, maxLength: 10 }), + (removedEnvironments) => { + // Clear mocks before each property test iteration + jest.clearAllMocks(); + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - for each environment with a profile, the profile should appear + const allCalls = (logger.warning as jest.Mock).mock.calls; + const allCallsStr = JSON.stringify(allCalls); + + for (const env of removedEnvironments) { + if (env.profile) { + // Profile should appear with trailing slash (profile/) + expect(allCallsStr).toContain(`${env.profile}/`); + } + } + }, + ), + { numRuns: 100 }, + ); + }); + + test('each entry includes bootstrap command', () => { + fc.assert( + fc.property( + fc.array(RemovedEnvironmentArb, { minLength: 1, maxLength: 10 }), + (removedEnvironments) => { + // Clear mocks before each property test iteration + jest.clearAllMocks(); + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - bootstrap command should appear for each environment + const allCalls = (logger.warning as jest.Mock).mock.calls; + const allCallsStr = JSON.stringify(allCalls); + + // Should contain 'cdk bootstrap' command + expect(allCallsStr).toContain('cdk bootstrap'); + + // For each environment, the bootstrap target should be present + for (const env of removedEnvironments) { + if (env.account) { + // Should have aws://account/region format + expect(allCallsStr).toContain(`aws://${env.account}/${env.region}`); + } else { + // Should have just the region + expect(allCallsStr).toContain(env.region); + } + } + }, + ), + { numRuns: 100 }, + ); + }); + + test('summary is not printed when list is empty', () => { + fc.assert( + fc.property( + fc.constant([]), + (removedEnvironments: RemovedEnvironment[]) => { + // Clear mocks beforRemovedEnvironmentteration + jest.clearAllMocks(); + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - no output should be produced + expect(logger.warning).not.toHaveBeenCalled(); + }, + ), + { numRuns: 100 }, + ); + }); + + test('number of environment entries matches number of removed environments', () => { + fc.assert( + fc.property( + fc.array(RemovedEnvironmentArb, { minLength: 1, maxLength: 10 }), + (removedEnvironments) => { + // Clear mocks before each property test iteration + jest.clearAllMocks(); + + // WHEN + printEnvironmentsSummary({ removed: removedEnvironments }); + + // THEN - should have correct number of calls + // Format: 1 header + (2 lines per env) + 1 trailing newline + const expectedCalls = 1 + (removedEnvironments.length * 2) + 1; + expect(logger.warning).toHaveBeenCalledTimes(expectedCalls); + }, + ), + { numRuns: 100 }, + ); + }); + }); +}); + +/** + * Tests for emitEnvironmentRemovedWarning + * + * Since emitEnvironmentRemovedWarning is a private function in integ-test-worker.ts, + * we test it indirectly through integration tests or by extracting the logic. + * + * For Property 7, we create a test helper that mimics the warning emission logic. + */ +describe('Warning Message Content Tests', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + /** + * Helper interface for testing NOT_BOOTSTRAPPED diagnostic handling + */ + interface NotBootstrappedDiagnosticInput { + environment: TestEnvironment; + reason: string; + } + + // Helper function that mimics emitEnvironmentRemovedWarning logic for testing + function emitWarningForTesting(input: NotBootstrappedDiagnosticInput): string[] { + const profileStr = input.environment.profile ? `${input.environment.profile}/` : ''; + const accountStr = input.environment.account ? `aws://${input.environment.account}/${input.environment.region}` : input.environment.region; + + return [ + `⚠️ Environment ${profileStr}${input.environment.region} removed due to bootstrap error`, + ` Reason: ${input.reason}`, + ` Run: cdk bootstrap ${accountStr}`, + ]; + } + + describe('unit tests for warning message content', () => { + test('warning contains region name', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'us-east-1' }, + reason: 'Not bootstrapped', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN + expect(messages[0]).toContain('us-east-1'); + }); + + test('warning contains profile when provided', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'us-west-2', profile: 'dev-profile' }, + reason: 'Not bootstrapped', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN + expect(messages[0]).toContain('dev-profile/'); + expect(messages[0]).toContain('us-west-2'); + }); + + test('warning does not contain profile prefix when profile is not provided', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'eu-west-1' }, + reason: 'Not bootstrapped', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN - should not have double slash or undefined + expect(messages[0]).not.toContain('undefined'); + expect(messages[0]).toContain('Environment eu-west-1'); + }); + + test('warning contains cdk bootstrap command', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'ap-southeast-1' }, + reason: 'Not bootstrapped', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN + expect(messages[2]).toContain('cdk bootstrap'); + }); + + test('warning contains account in bootstrap command when provided', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'sa-east-1', account: '123456789012' }, + reason: 'Not bootstrapped', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN + expect(messages[2]).toContain('cdk bootstrap aws://123456789012/sa-east-1'); + }); + + test('warning contains region only in bootstrap command when account not provided', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'ca-central-1' }, + reason: 'Not bootstrapped', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN + expect(messages[2]).toContain('cdk bootstrap ca-central-1'); + expect(messages[2]).not.toContain('aws://'); + }); + + test('warning contains reason', () => { + // GIVEN + const input: NotBootstrappedDiagnosticInput = { + environment: { region: 'us-east-1' }, + reason: 'Bootstrap stack version is insufficient', + }; + + // WHEN + const messages = emitWarningForTesting(input); + + // THEN + expect(messages[1]).toContain('Bootstrap stack version is insufficient'); + }); + }); + + /** + * Property 7: Warning Message Content + * + * *For any* removed region info containing a region name and optional profile, + * the emitted warning message should contain the region name, the profile (if provided), + * and a `cdk bootstrap` command. + * + * **Validates: Requirements 5.2, 5.3** + */ + describe('Property 7: Warning Message Content', () => { + // Arbitrary generators + const awsAccountArb = fc.stringMatching(/^[0-9]{12}$/); + const awsRegionArb = fc.constantFrom( + 'us-east-1', 'us-east-2', 'us-west-1', 'us-west-2', + 'eu-west-1', 'eu-west-2', 'eu-west-3', 'eu-central-1', + 'ap-northeast-1', 'ap-northeast-2', 'ap-southeast-1', 'ap-southeast-2', + 'sa-east-1', 'ca-central-1', 'me-south-1', 'af-south-1', + ); + const profileArb = fc.option( + fc.stringOf(fc.constantFrom('a', 'b', 'c', 'd', 'e', '-', '_', '1', '2', '3'), { minLength: 1, maxLength: 20 }), + { nil: undefined }, + ); + const reasonArb = fc.string({ minLength: 1, maxLength: 200 }); + + const notBootstrappedInputArb: fc.Arbitrary = fc.record({ + environment: fc.record({ + region: awsRegionArb, + profile: profileArb, + account: fc.option(awsAccountArb, { nil: undefined }), + }), + reason: reasonArb, + }); + + test('warning message contains region name for any removal request', () => { + fc.assert( + fc.property( + notBootstrappedInputArb, + (input) => { + // WHEN + const messages = emitWarningForTesting(input); + const allMessages = messages.join('\n'); + + // THEN - region should appear in the warning + expect(allMessages).toContain(input.environment.region); + }, + ), + { numRuns: 100 }, + ); + }); + + test('warning message contains profile when provided', () => { + fc.assert( + fc.property( + notBootstrappedInputArb, + (input) => { + // WHEN + const messages = emitWarningForTesting(input); + const allMessages = messages.join('\n'); + + // THEN - if profile is provided, it should appear with trailing slash + if (input.environment.profile) { + expect(allMessages).toContain(`${input.environment.profile}/`); + } + }, + ), + { numRuns: 100 }, + ); + }); + + test('warning message contains cdk bootstrap command for any removal request', () => { + fc.assert( + fc.property( + notBootstrappedInputArb, + (input) => { + // WHEN + const messages = emitWarningForTesting(input); + const allMessages = messages.join('\n'); + + // THEN - should contain cdk bootstrap command + expect(allMessages).toContain('cdk bootstrap'); + }, + ), + { numRuns: 100 }, + ); + }); + + test('bootstrap command contains correct target for any removal request', () => { + fc.assert( + fc.property( + notBootstrappedInputArb, + (input) => { + // WHEN + const messages = emitWarningForTesting(input); + const bootstrapLine = messages[2]; + + // THEN - bootstrap target should be correct + if (input.environment.account) { + expect(bootstrapLine).toContain(`aws://${input.environment.account}/${input.environment.region}`); + } else { + expect(bootstrapLine).toContain(input.environment.region); + expect(bootstrapLine).not.toContain('aws://'); + } + }, + ), + { numRuns: 100 }, + ); + }); + + test('warning message does not contain undefined or null strings', () => { + fc.assert( + fc.property( + notBootstrappedInputArb, + (input) => { + // WHEN + const messages = emitWarningForTesting(input); + const allMessages = messages.join('\n'); + + // THEN - should not contain literal 'undefined' or 'null' + expect(allMessages).not.toContain('undefined'); + expect(allMessages).not.toContain('null'); + }, + ), + { numRuns: 100 }, + ); + }); + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/assets.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/assets.ts index dc1d6b85a..97f767d4e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/assets.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/assets.ts @@ -3,7 +3,7 @@ import * as cxapi from '@aws-cdk/cloud-assembly-api'; import * as cxschema from '@aws-cdk/cloud-assembly-schema'; import * as chalk from 'chalk'; import type { AssetManifestBuilder } from './asset-manifest-builder'; -import { ToolkitError } from '../../toolkit/toolkit-error'; +import { BootstrapError, ToolkitError } from '../../toolkit/toolkit-error'; import type { EnvironmentResources } from '../environment'; import type { IoHelper } from '../io/private'; import type { ToolkitInfo } from '../toolkit-info'; @@ -30,8 +30,12 @@ export async function addMetadataAssetsToManifest( const toolkitInfo = await envResources.lookupToolkit(); if (!toolkitInfo.found) { + const environment = stack.environment!; // eslint-disable-next-line @stylistic/max-len - throw new ToolkitError(`This stack uses assets, so the toolkit stack must be deployed to the environment (Run "${chalk.blue('cdk bootstrap ' + stack.environment!.name)}")`); + throw new BootstrapError( + `This stack uses assets, so the toolkit stack must be deployed to the environment (Run "${chalk.blue('cdk bootstrap ' + environment.name)}")`, + { account: environment.account, region: environment.region }, + ); } const params: Record = {}; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/checks.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/checks.ts index e1199d9cb..d23d5464e 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/checks.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/checks.ts @@ -1,4 +1,4 @@ -import { ToolkitError } from '../../toolkit/toolkit-error'; +import { BootstrapError, ToolkitError } from '../../toolkit/toolkit-error'; import type { SDK } from '../aws-auth/private'; import type { IoHelper } from '../io/private'; @@ -45,42 +45,42 @@ interface BootstrapStackInfo { } export async function getBootstrapStackInfo(sdk: SDK, stackName: string): Promise { - try { - const cfn = sdk.cloudFormation(); - const stackResponse = await cfn.describeStacks({ StackName: stackName }); + const cfn = sdk.cloudFormation(); + const stackResponse = await cfn.describeStacks({ StackName: stackName }); - if (!stackResponse.Stacks || stackResponse.Stacks.length === 0) { - throw new ToolkitError(`Toolkit stack ${stackName} not found`); - } + if (!stackResponse.Stacks || stackResponse.Stacks.length === 0) { + const account = await sdk.currentAccount(); + throw new BootstrapError( + `Toolkit stack ${stackName} not found. Please run 'cdk bootstrap'.`, + { account: account.accountId, region: sdk.currentRegion }, + ); + } - const stack = stackResponse.Stacks[0]; - const versionOutput = stack.Outputs?.find(output => output.OutputKey === 'BootstrapVersion'); + const stack = stackResponse.Stacks[0]; + const versionOutput = stack.Outputs?.find(output => output.OutputKey === 'BootstrapVersion'); - if (!versionOutput?.OutputValue) { - throw new ToolkitError(`Unable to find BootstrapVersion output in the toolkit stack ${stackName}`); - } + if (!versionOutput?.OutputValue) { + throw new ToolkitError(`Unable to find BootstrapVersion output in the toolkit stack ${stackName}`); + } - const bootstrapVersion = parseInt(versionOutput.OutputValue); - if (isNaN(bootstrapVersion)) { - throw new ToolkitError(`Invalid BootstrapVersion value: ${versionOutput.OutputValue}`); - } + const bootstrapVersion = parseInt(versionOutput.OutputValue); + if (isNaN(bootstrapVersion)) { + throw new ToolkitError(`Invalid BootstrapVersion value: ${versionOutput.OutputValue}`); + } - // try to get bucketname from the logical resource id. If there is no - // bucketname, or the value doesn't look like an S3 bucket name, we assume - // the bucket doesn't exist (this is for the case where a template customizer did - // not dare to remove the Output, but put a dummy value there like '' or '-' or '***'). - // - // We would have preferred to look at the stack resources here, but - // unfortunately the deploy role doesn't have permissions call DescribeStackResources. - const bucketName = stack.Outputs?.find(output => output.OutputKey === 'BucketName')?.OutputValue; - // Must begin and end with letter or number. - const hasStagingBucket = !!(bucketName && bucketName.match(/^[a-z0-9]/) && bucketName.match(/[a-z0-9]$/)); + // try to get bucketname from the logical resource id. If there is no + // bucketname, or the value doesn't look like an S3 bucket name, we assume + // the bucket doesn't exist (this is for the case where a template customizer did + // not dare to remove the Output, but put a dummy value there like '' or '-' or '***'). + // + // We would have preferred to look at the stack resources here, but + // unfortunately the deploy role doesn't have permissions call DescribeStackResources. + const bucketName = stack.Outputs?.find(output => output.OutputKey === 'BucketName')?.OutputValue; + // Must begin and end with letter or number. + const hasStagingBucket = !!(bucketName && bucketName.match(/^[a-z0-9]/) && bucketName.match(/[a-z0-9]$/)); - return { - hasStagingBucket, - bootstrapVersion, - }; - } catch (e) { - throw new ToolkitError(`Error retrieving toolkit stack info: ${e}`); - } + return { + hasStagingBucket, + bootstrapVersion, + }; } diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/environment/environment-resources.ts b/packages/@aws-cdk/toolkit-lib/lib/api/environment/environment-resources.ts index 083d70ffb..97657dbe6 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/environment/environment-resources.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/environment/environment-resources.ts @@ -1,5 +1,5 @@ import type { Environment } from '@aws-cdk/cloud-assembly-api'; -import { ToolkitError } from '../../toolkit/toolkit-error'; +import { BootstrapError, ToolkitError } from '../../toolkit/toolkit-error'; import { formatErrorMessage } from '../../util'; import type { SDK } from '../aws-auth/private'; import type { IoHelper } from '../io/private'; @@ -123,8 +123,9 @@ export class EnvironmentResources { notices.addBootstrappedEnvironment({ bootstrapStackVersion: version, environment }); } if (defExpectedVersion > version) { - throw new ToolkitError( + throw new BootstrapError( `This CDK deployment requires bootstrap stack version '${expectedVersion}', found '${version}'. Please run 'cdk bootstrap'.`, + { account: environment.account, region: environment.region }, ); } } @@ -160,8 +161,9 @@ export class EnvironmentResources { return asNumber; } catch (e: any) { if (e.name === 'ParameterNotFound') { - throw new ToolkitError( + throw new BootstrapError( `SSM parameter ${parameterName} not found. Has the environment been bootstrapped? Please run \'cdk bootstrap\' (see https://docs.aws.amazon.com/cdk/latest/guide/bootstrapping.html)`, + { account: this.environment.account, region: this.environment.region }, ); } throw e; diff --git a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit-error.ts b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit-error.ts index 7fc77cef8..ef49127b5 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit-error.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit-error.ts @@ -5,6 +5,7 @@ const AUTHENTICATION_ERROR_SYMBOL = Symbol.for('@aws-cdk/toolkit-lib.Authenticat const ASSEMBLY_ERROR_SYMBOL = Symbol.for('@aws-cdk/toolkit-lib.AssemblyError'); const CONTEXT_PROVIDER_ERROR_SYMBOL = Symbol.for('@aws-cdk/toolkit-lib.ContextProviderError'); const NO_RESULTS_FOUND_ERROR_SYMBOL = Symbol.for('@aws-cdk/toolkit-lib.NoResultsFoundError'); +const BOOTSTRAP_ERROR_SYMBOL = Symbol.for('@aws-cdk/toolkit-lib.BootstrapError'); /** * Represents a general toolkit error in the AWS CDK Toolkit. @@ -38,6 +39,13 @@ export class ToolkitError extends Error { return ToolkitError.isToolkitError(x) && CONTEXT_PROVIDER_ERROR_SYMBOL in x; } + /** + * Determines if a given error is an instance of BootstrapError. + */ + public static isBootstrapError(x: any): x is BootstrapError { + return ToolkitError.isToolkitError(x) && BOOTSTRAP_ERROR_SYMBOL in x; + } + /** * A ToolkitError with an original error as cause */ @@ -168,3 +176,45 @@ export class NoResultsFoundError extends ContextProviderError { Object.defineProperty(this, NO_RESULTS_FOUND_ERROR_SYMBOL, { value: true }); } } + +/** + * Information about the AWS environment where a bootstrap error occurred + */ +export interface BootstrapEnvironment { + /** + * The AWS account ID + */ + readonly account: string; + + /** + * The AWS region + */ + readonly region: string; +} + +/** + * Represents a bootstrap-related error in the AWS CDK Toolkit. + * + * This error is thrown when: + * - The CDK toolkit stack is not found in a region + * - The bootstrap stack version is insufficient + * - The SSM parameter for bootstrap version is not found + */ +export class BootstrapError extends ToolkitError { + /** + * The AWS environment (account/region) where the bootstrap error occurred + */ + public readonly environment: BootstrapEnvironment; + + /** + * Denotes the source of the error as user (they need to bootstrap) + */ + public readonly source = 'user'; + + constructor(message: string, environment: BootstrapEnvironment, cause?: unknown) { + super(message, 'bootstrap', cause); + Object.setPrototypeOf(this, BootstrapError.prototype); + Object.defineProperty(this, BOOTSTRAP_ERROR_SYMBOL, { value: true }); + this.environment = environment; + } +} diff --git a/packages/@aws-cdk/toolkit-lib/test/api/deployments/assets.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/deployments/assets.test.ts index 3d666ce3c..276641f1c 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/deployments/assets.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/deployments/assets.test.ts @@ -3,6 +3,8 @@ import type { AssetMetadataEntry } from '@aws-cdk/cloud-assembly-schema'; import { addMetadataAssetsToManifest, AssetManifestBuilder } from '../../../lib/api/deployments'; import type { EnvironmentResources } from '../../../lib/api/environment'; import { EnvironmentResourcesRegistry } from '../../../lib/api/environment'; +import { ToolkitInfo } from '../../../lib/api/toolkit-info'; +import { ToolkitError } from '../../../lib/toolkit/toolkit-error'; import { MockSdk } from '../../_helpers/mock-sdk'; import { MockToolkitInfo } from '../_helpers/mock-toolkitinfo'; import { TestIoHost } from '../../_helpers/test-io-host'; @@ -257,3 +259,38 @@ function mockFn any>(fn: F): jest.Mock } return fn; } + +describe('BootstrapError throwing', () => { + test('throws BootstrapError with correct environment info when toolkit not found', async () => { + // GIVEN - toolkit not found + toolkitMock.dispose(); + ToolkitInfo.lookup = jest.fn().mockResolvedValue(ToolkitInfo.bootstrapStackNotFoundInfo('CDKToolkit')); + + const stack = stackWithAssets([ + { + sourceHash: 'source-hash', + path: __filename, + id: 'SomeStackSomeResource4567', + packaging: 'file', + s3BucketParameter: 'BucketParameter', + s3KeyParameter: 'KeyParameter', + artifactHashParameter: 'ArtifactHashParameter', + }, + ]); + + // WHEN/THEN + await expect(addMetadataAssetsToManifest(ioHelper, stack, assets, envResources)).rejects.toThrow( + /toolkit stack must be deployed/, + ); + + try { + await addMetadataAssetsToManifest(ioHelper, stack, assets, envResources); + } catch (error: any) { + expect(ToolkitError.isBootstrapError(error)).toBe(true); + expect(error.environment).toEqual({ + account: '123456789012', + region: 'here', + }); + } + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/test/api/deployments/checks.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/deployments/checks.test.ts index 1ddf30a81..5e64ab8d5 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/deployments/checks.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/deployments/checks.test.ts @@ -1,5 +1,6 @@ import { DescribeStacksCommand, StackStatus } from '@aws-sdk/client-cloudformation'; import { determineAllowCrossAccountAssetPublishing, getBootstrapStackInfo } from '../../../lib/api/deployments'; +import { ToolkitError } from '../../../lib/toolkit/toolkit-error'; import { mockCloudFormationClient, MockSdk } from '../../_helpers/mock-sdk'; import { TestIoHost } from '../../_helpers/test-io-host'; @@ -116,14 +117,24 @@ describe('getBootstrapStackInfo', () => { }); }); - it('should throw error when stack is not found', async () => { + it('should throw BootstrapError when stack is not found', async () => { const mockSDK = new MockSdk(); mockCloudFormationClient.on(DescribeStacksCommand).resolves({ Stacks: [], }); - await expect(getBootstrapStackInfo(mockSDK, 'CDKToolkit')).rejects.toThrow('Toolkit stack CDKToolkit not found'); + await expect(getBootstrapStackInfo(mockSDK, 'CDKToolkit')).rejects.toThrow("Toolkit stack CDKToolkit not found. Please run 'cdk bootstrap'."); + + try { + await getBootstrapStackInfo(mockSDK, 'CDKToolkit'); + } catch (error: any) { + expect(ToolkitError.isBootstrapError(error)).toBe(true); + expect(error.environment).toEqual({ + account: '123456789012', + region: 'bermuda-triangle-1337', + }); + } }); it('should throw error when BootstrapVersion output is missing', async () => { diff --git a/packages/@aws-cdk/toolkit-lib/test/api/environment/environment-resources.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/environment/environment-resources.test.ts index 05b9e3d05..9424fe924 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/environment/environment-resources.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/environment/environment-resources.test.ts @@ -5,6 +5,7 @@ import { EnvironmentResourcesRegistry } from '../../../lib/api/environment'; import { Notices } from '../../../lib/api/notices'; import { CachedDataSource } from '../../../lib/api/notices/cached-data-source'; import { NoticesFilter } from '../../../lib/api/notices/filter'; +import { ToolkitError } from '../../../lib/toolkit/toolkit-error'; import { MockSdk, mockBootstrapStack, mockSSMClient } from '../../_helpers/mock-sdk'; import { TestIoHost } from '../../_helpers/test-io-host'; import { MockToolkitInfo } from '../_helpers/mock-toolkitinfo'; @@ -173,3 +174,82 @@ describe('validate version without bootstrap stack', () => { }); }); }); + +describe('BootstrapError throwing', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + test('throws BootstrapError with correct environment info for version mismatch', async () => { + // GIVEN - bootstrap stack with version 5, but we require version 10 + mockToolkitInfo( + ToolkitInfo.fromStack( + mockBootstrapStack({ + Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '5' }], + }), + ), + ); + + // WHEN/THEN + await expect(envResources().validateVersion(10, undefined)).rejects.toThrow( + /requires bootstrap stack version/, + ); + + try { + await envResources().validateVersion(10, undefined); + } catch (error: any) { + expect(ToolkitError.isBootstrapError(error)).toBe(true); + expect(error.environment).toEqual({ + account: '11111111', + region: 'us-nowhere', + }); + } + }); + + test('throws BootstrapError with correct environment info for missing SSM parameter', async () => { + // GIVEN - no bootstrap stack, SSM parameter not found + mockToolkitInfo(ToolkitInfo.bootstrapStackNotFoundInfo('TestBootstrapStack')); + + const ssmError = new Error('Parameter not found'); + ssmError.name = 'ParameterNotFound'; + mockSSMClient.on(GetParameterCommand).rejects(ssmError); + + // WHEN/THEN + await expect(envResources().validateVersion(8, '/cdk-bootstrap/version')).rejects.toThrow( + /Has the environment been bootstrapped/, + ); + + try { + await envResources().validateVersion(8, '/cdk-bootstrap/version'); + } catch (error: any) { + expect(ToolkitError.isBootstrapError(error)).toBe(true); + expect(error.environment).toEqual({ + account: '11111111', + region: 'us-nowhere', + }); + } + }); + + test('throws BootstrapError with correct environment info when SSM version is insufficient', async () => { + // GIVEN - SSM parameter returns version 5, but we require version 10 + mockToolkitInfo(ToolkitInfo.bootstrapStackNotFoundInfo('TestBootstrapStack')); + mockSSMClient.on(GetParameterCommand).resolves({ + Parameter: { Value: '5' }, + }); + + // WHEN/THEN + await expect(envResources().validateVersion(10, '/cdk-bootstrap/version')).rejects.toThrow( + /requires bootstrap stack version/, + ); + + try { + await envResources().validateVersion(10, '/cdk-bootstrap/version'); + } catch (error: any) { + expect(ToolkitError.isBootstrapError(error)).toBe(true); + expect(error.environment).toEqual({ + account: '11111111', + region: 'us-nowhere', + }); + } + }); +}); diff --git a/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit-error.test.ts b/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit-error.test.ts index f6c220a9d..30897e27f 100644 --- a/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit-error.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/toolkit/toolkit-error.test.ts @@ -1,4 +1,4 @@ -import { AssemblyError, AuthenticationError, ContextProviderError, NoResultsFoundError, ToolkitError } from '../../lib/toolkit/toolkit-error'; +import { AssemblyError, AuthenticationError, BootstrapError, ContextProviderError, NoResultsFoundError, ToolkitError } from '../../lib/toolkit/toolkit-error'; describe('toolkit error', () => { let toolkitError = new ToolkitError('Test toolkit error'); @@ -8,6 +8,8 @@ describe('toolkit error', () => { let assemblyError = AssemblyError.withStacks('Test authentication error', []); let assemblyCauseError = AssemblyError.withCause('Test authentication error', new Error('other error')); let noResultsError = new NoResultsFoundError('Test no results error'); + let bootstrapError = new BootstrapError('Test bootstrap error', { account: '123456789012', region: 'us-east-1' }); + let bootstrapErrorWithCause = new BootstrapError('Test bootstrap error with cause', { account: '123456789012', region: 'eu-west-1' }, new Error('underlying cause')); test('types are correctly assigned', async () => { expect(toolkitError.type).toBe('toolkit'); @@ -16,6 +18,8 @@ describe('toolkit error', () => { expect(assemblyCauseError.type).toBe('assembly'); expect(contextProviderError.type).toBe('context-provider'); expect(noResultsError.type).toBe('context-provider'); + expect(bootstrapError.type).toBe('bootstrap'); + expect(bootstrapErrorWithCause.type).toBe('bootstrap'); }); test('isToolkitError works', () => { @@ -26,6 +30,8 @@ describe('toolkit error', () => { expect(ToolkitError.isToolkitError(assemblyError)).toBe(true); expect(ToolkitError.isToolkitError(assemblyCauseError)).toBe(true); expect(ToolkitError.isToolkitError(contextProviderError)).toBe(true); + expect(ToolkitError.isToolkitError(bootstrapError)).toBe(true); + expect(ToolkitError.isToolkitError(bootstrapErrorWithCause)).toBe(true); }); test('ToolkitError.withCause', () => { @@ -79,4 +85,72 @@ describe('toolkit error', () => { expect(ToolkitError.isAssemblyError(noResultsError)).toBe(false); expect(ToolkitError.isAuthenticationError(noResultsError)).toBe(false); }); + + describe('BootstrapError', () => { + test('constructor creates error with correct properties', () => { + const error = new BootstrapError('Bootstrap stack not found', { account: '123456789012', region: 'ap-southeast-1' }); + + expect(error.message).toBe('Bootstrap stack not found'); + expect(error.type).toBe('bootstrap'); + expect(error.source).toBe('user'); + expect(error.environment).toEqual({ account: '123456789012', region: 'ap-southeast-1' }); + expect(error.name).toBe('BootstrapError'); + }); + + test('constructor with cause preserves cause', () => { + const cause = new Error('underlying error'); + const error = new BootstrapError('Bootstrap failed', { account: '123456789012', region: 'us-west-2' }, cause); + + expect(error.cause).toBe(cause); + expect((error.cause as Error).message).toBe('underlying error'); + }); + + test('environment property contains account and region', () => { + expect(bootstrapError.environment.account).toBe('123456789012'); + expect(bootstrapError.environment.region).toBe('us-east-1'); + + expect(bootstrapErrorWithCause.environment.account).toBe('123456789012'); + expect(bootstrapErrorWithCause.environment.region).toBe('eu-west-1'); + }); + + test('inherits from ToolkitError (instanceof)', () => { + expect(bootstrapError instanceof ToolkitError).toBe(true); + expect(bootstrapError instanceof Error).toBe(true); + expect(bootstrapErrorWithCause instanceof ToolkitError).toBe(true); + }); + + test('isBootstrapError type guard returns true for BootstrapError', () => { + expect(ToolkitError.isBootstrapError(bootstrapError)).toBe(true); + expect(ToolkitError.isBootstrapError(bootstrapErrorWithCause)).toBe(true); + }); + + test('isBootstrapError returns false for other ToolkitError types', () => { + expect(ToolkitError.isBootstrapError(toolkitError)).toBe(false); + expect(ToolkitError.isBootstrapError(authError)).toBe(false); + expect(ToolkitError.isBootstrapError(assemblyError)).toBe(false); + expect(ToolkitError.isBootstrapError(assemblyCauseError)).toBe(false); + expect(ToolkitError.isBootstrapError(contextProviderError)).toBe(false); + expect(ToolkitError.isBootstrapError(noResultsError)).toBe(false); + }); + + test('isBootstrapError returns false for non-ToolkitError types', () => { + expect(ToolkitError.isBootstrapError(new Error('plain error'))).toBe(false); + expect(ToolkitError.isBootstrapError(null)).toBe(false); + expect(ToolkitError.isBootstrapError(undefined)).toBe(false); + expect(ToolkitError.isBootstrapError('string error')).toBe(false); + expect(ToolkitError.isBootstrapError(42)).toBe(false); + expect(ToolkitError.isBootstrapError({ message: 'fake error' })).toBe(false); + }); + + test('isToolkitError returns true for BootstrapError', () => { + expect(ToolkitError.isToolkitError(bootstrapError)).toBe(true); + expect(ToolkitError.isToolkitError(bootstrapErrorWithCause)).toBe(true); + }); + + test('other type guards return false for BootstrapError', () => { + expect(ToolkitError.isAuthenticationError(bootstrapError)).toBe(false); + expect(ToolkitError.isAssemblyError(bootstrapError)).toBe(false); + expect(ToolkitError.isContextProviderError(bootstrapError)).toBe(false); + }); + }); });