diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 8e56480e9f4c5..7cf0ccc6b42f1 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -17,7 +17,7 @@ import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { ResourceImporter } from './import'; -import { data, debug, error, highlight, print, success, warning } from './logging'; +import { data, debug, error, highlight, print, success, warning, withCorkedLogging } from './logging'; import { deserializeStructure, serializeStructure } from './serialize'; import { Configuration, PROJECT_CONFIG } from './settings'; import { numberFromBool, partition } from './util'; @@ -238,23 +238,24 @@ export class CdkToolkit { if (requireApproval !== RequireApproval.Never) { const currentTemplate = await this.props.deployments.readCurrentTemplate(stack); if (printSecurityDiff(currentTemplate, stack, requireApproval)) { - - // only talk to user if STDIN is a terminal (otherwise, fail) - if (!process.stdin.isTTY) { - throw new Error( - '"--require-approval" is enabled and stack includes security-sensitive updates, ' + - 'but terminal (TTY) is not attached so we are unable to get a confirmation from the user'); - } - - // only talk to user if concurrency is 1 (otherwise, fail) - if (concurrency > 1) { - throw new Error( - '"--require-approval" is enabled and stack includes security-sensitive updates, ' + - 'but concurrency is greater than 1 so we are unable to get a confirmation from the user'); - } - - const confirmed = await promptly.confirm('Do you wish to deploy these changes (y/n)?'); - if (!confirmed) { throw new Error('Aborted by user'); } + await withCorkedLogging(async () => { + // only talk to user if STDIN is a terminal (otherwise, fail) + if (!process.stdin.isTTY) { + throw new Error( + '"--require-approval" is enabled and stack includes security-sensitive updates, ' + + 'but terminal (TTY) is not attached so we are unable to get a confirmation from the user'); + } + + // only talk to user if concurrency is 1 (otherwise, fail) + if (concurrency > 1) { + throw new Error( + '"--require-approval" is enabled and stack includes security-sensitive updates, ' + + 'but concurrency is greater than 1 so we are unable to get a confirmation from the user'); + } + + const confirmed = await promptly.confirm('Do you wish to deploy these changes (y/n)?'); + if (!confirmed) { throw new Error('Aborted by user'); } + }); } } diff --git a/packages/aws-cdk/lib/logging.ts b/packages/aws-cdk/lib/logging.ts index ceade56caed6d..50f739f185216 100644 --- a/packages/aws-cdk/lib/logging.ts +++ b/packages/aws-cdk/lib/logging.ts @@ -7,6 +7,34 @@ const { stdout, stderr } = process; type WritableFactory = () => Writable; +export async function withCorkedLogging(block: () => Promise): Promise { + corkLogging(); + try { + return await block(); + } finally { + uncorkLogging(); + } +} + +let CORK_COUNTER = 0; +const logBuffer: [Writable, string][] = []; + +function corked() { + return CORK_COUNTER !== 0; +} + +function corkLogging() { + CORK_COUNTER += 1; +} + +function uncorkLogging() { + CORK_COUNTER -= 1; + if (!corked()) { + logBuffer.forEach(([stream, str]) => stream.write(str + '\n')); + logBuffer.splice(0); + } +} + const logger = (stream: Writable | WritableFactory, styles?: StyleFn[], timestamp?: boolean) => (fmt: string, ...args: unknown[]) => { const ts = timestamp ? `[${formatTime(new Date())}] ` : ''; @@ -15,11 +43,19 @@ const logger = (stream: Writable | WritableFactory, styles?: StyleFn[], timestam str = styles.reduce((a, style) => style(a), str); } - const realStream = typeof stream === 'function' ? stream() : stream; + + // Logger is currently corked, so we store the message to be printed + // later when we are uncorked. + if (corked()) { + logBuffer.push([realStream, str]); + return; + } + realStream.write(str + '\n'); }; + function formatTime(d: Date) { return `${lpad(d.getHours(), 2)}:${lpad(d.getMinutes(), 2)}:${lpad(d.getSeconds(), 2)}`; diff --git a/packages/cdk-assets/lib/publishing.ts b/packages/cdk-assets/lib/publishing.ts index c35a0254e55ed..9e38308cd7e66 100644 --- a/packages/cdk-assets/lib/publishing.ts +++ b/packages/cdk-assets/lib/publishing.ts @@ -82,9 +82,6 @@ export class AssetPublishing implements IPublishProgress { private readonly publishInParallel: boolean; private readonly buildAssets: boolean; private readonly publishAssets: boolean; - private readonly startMessagePrefix: string; - private readonly successMessagePrefix: string; - private readonly errorMessagePrefix: string; private readonly handlerCache = new Map(); constructor(private readonly manifest: AssetManifest, private readonly options: AssetPublishingOptions) { @@ -94,34 +91,6 @@ export class AssetPublishing implements IPublishProgress { this.buildAssets = options.buildAssets ?? true; this.publishAssets = options.publishAssets ?? true; - const getMessages = () => { - if (this.buildAssets && this.publishAssets) { - return { - startMessagePrefix: 'Building and publishing', - successMessagePrefix: 'Built and published', - errorMessagePrefix: 'Error building and publishing', - }; - } else if (this.buildAssets) { - return { - startMessagePrefix: 'Building', - successMessagePrefix: 'Built', - errorMessagePrefix: 'Error building', - }; - } else { - return { - startMessagePrefix: 'Publishing', - successMessagePrefix: 'Published', - errorMessagePrefix: 'Error publishing', - }; - } - }; - - const messages = getMessages(); - - this.startMessagePrefix = messages.startMessagePrefix; - this.successMessagePrefix = messages.successMessagePrefix; - this.errorMessagePrefix = messages.errorMessagePrefix; - const self = this; this.handlerHost = { aws: this.options.aws, @@ -146,7 +115,7 @@ export class AssetPublishing implements IPublishProgress { } if ((this.options.throwOnError ?? true) && this.failures.length > 0) { - throw new Error(`${this.errorMessagePrefix}: ${this.failures.map(e => e.error.message)}`); + throw new Error(`Error publishing: ${this.failures.map(e => e.error.message)}`); } } @@ -155,7 +124,7 @@ export class AssetPublishing implements IPublishProgress { */ public async buildEntry(asset: IManifestEntry) { try { - if (this.progressEvent(EventType.START, `${this.startMessagePrefix} ${asset.id}`)) { return false; } + if (this.progressEvent(EventType.START, `Building ${asset.id}`)) { return false; } const handler = this.assetHandler(asset); await handler.build(); @@ -163,6 +132,9 @@ export class AssetPublishing implements IPublishProgress { if (this.aborted) { throw new Error('Aborted'); } + + this.completedOperations++; + if (this.progressEvent(EventType.SUCCESS, `Built ${asset.id}`)) { return false; } } catch (e: any) { this.failures.push({ asset, error: e }); this.completedOperations++; @@ -177,7 +149,7 @@ export class AssetPublishing implements IPublishProgress { */ public async publishEntry(asset: IManifestEntry) { try { - if (this.progressEvent(EventType.UPLOAD, `${this.startMessagePrefix} ${asset.id}`)) { return false; } + if (this.progressEvent(EventType.START, `Publishing ${asset.id}`)) { return false; } const handler = this.assetHandler(asset); await handler.publish(); @@ -187,7 +159,7 @@ export class AssetPublishing implements IPublishProgress { } this.completedOperations++; - if (this.progressEvent(EventType.SUCCESS, `${this.successMessagePrefix} ${asset.id}`)) { return false; } + if (this.progressEvent(EventType.SUCCESS, `Published ${asset.id}`)) { return false; } } catch (e: any) { this.failures.push({ asset, error: e }); this.completedOperations++; @@ -212,7 +184,7 @@ export class AssetPublishing implements IPublishProgress { */ private async publishAsset(asset: IManifestEntry) { try { - if (this.progressEvent(EventType.START, `${this.startMessagePrefix} ${asset.id}`)) { return false; } + if (this.progressEvent(EventType.START, `Publishing ${asset.id}`)) { return false; } const handler = this.assetHandler(asset); @@ -229,7 +201,7 @@ export class AssetPublishing implements IPublishProgress { } this.completedOperations++; - if (this.progressEvent(EventType.SUCCESS, `${this.successMessagePrefix} ${asset.id}`)) { return false; } + if (this.progressEvent(EventType.SUCCESS, `Published ${asset.id}`)) { return false; } } catch (e: any) { this.failures.push({ asset, error: e }); this.completedOperations++; diff --git a/packages/cdk-assets/test/docker-images.test.ts b/packages/cdk-assets/test/docker-images.test.ts index 688828a68c021..18b713947c365 100644 --- a/packages/cdk-assets/test/docker-images.test.ts +++ b/packages/cdk-assets/test/docker-images.test.ts @@ -247,7 +247,7 @@ describe('with a complete manifest', () => { test('Displays an error if the ECR repository cannot be found', async () => { aws.mockEcr.describeImages = mockedApiFailure('RepositoryNotFoundException', 'Repository not Found'); - await expect(pub.publish()).rejects.toThrow('Error building and publishing: Repository not Found'); + await expect(pub.publish()).rejects.toThrow('Error publishing: Repository not Found'); }); test('successful run does not need to query account ID', async () => {