Skip to content

Commit

Permalink
chore(toolkit): capture all output from an app (#33259)
Browse files Browse the repository at this point in the history
### Issue #32997

Relates to #32997

### Reason for this change

When a CDK app is invoked by a sub-shell, we need to capture all output
by lines and send it to the IoHost.

### Description of changes

Adds an `EventPublisher` interface to the `execInChildProcess` helper.
This is getting passed in a publisher that uses the IoHost.
Inspired from the [ShellEventPublisher in
cdk-assets](https://github.com/cdklabs/cdk-assets/blame/8751d206ea3fa6dbb6156125c64d0ea0e8df289e/lib/private/shell.ts#L6).

### Describe any new or updated permissions being added

n/a

### Description of how you validated changes

<!--Have you added any unit tests and/or integration tests?-->

### Checklist
- [x] My code adheres to the [CONTRIBUTING
GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and
[DESIGN
GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license*

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
mrgrain and mergify[bot] authored Feb 3, 2025
1 parent af44791 commit c83d4ca
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 32 deletions.
28 changes: 23 additions & 5 deletions packages/@aws-cdk/toolkit/lib/api/cloud-assembly/private/exec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import * as child_process from 'node:child_process';
import * as split2 from 'split2';
import { ToolkitError } from '../../errors';

type EventPublisher = (event: 'open' | 'data_stdout' | 'data_stderr' | 'close', line: string) => void;

interface ExecOptions {
eventPublisher?: EventPublisher;
extraEnv?: { [key: string]: string | undefined };
cwd?: string;
}
Expand All @@ -17,12 +21,10 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp
// number of quoting issues introduced by the intermediate shell layer
// (which would be different between Linux and Windows).
//
// - Inherit stderr from controlling terminal. We don't use the captured value
// anyway, and if the subprocess is printing to it for debugging purposes the
// user gets to see it sooner. Plus, capturing doesn't interact nicely with some
// processes like Maven.
// - We have to capture any output to stdout and stderr sp we can pass it on to the IoHost
// To ensure messages get to the user fast, we will emit every full line we receive.
const proc = child_process.spawn(commandAndArgs, {
stdio: ['ignore', 'inherit', 'inherit'],
stdio: ['ignore', 'pipe', 'pipe'],
detached: false,
shell: true,
cwd: options.cwd,
Expand All @@ -32,6 +34,22 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp
},
});

const eventPublisher: EventPublisher = options.eventPublisher ?? ((type, line) => {
switch (type) {
case 'data_stdout':
process.stdout.write(line);
return;
case 'data_stderr':
process.stderr.write(line);
return;
case 'open':
case 'close':
return;
}
});
proc.stdout.pipe(split2()).on('data', (line) => eventPublisher('data_stdout', line));
proc.stderr.pipe(split2()).on('data', (line) => eventPublisher('data_stderr', line));

proc.on('error', fail);

proc.on('exit', code => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import { assemblyFromDirectory, changeDir, determineOutputDirectory, guessExecut
import { ToolkitServices } from '../../../toolkit/private';
import { Context, ILock, RWLock, Settings } from '../../aws-cdk';
import { ToolkitError } from '../../errors';
import { debug } from '../../io/private';
import { debug, error, info } from '../../io/private';
import { AssemblyBuilder, CdkAppSourceProps } from '../source-builder';

export abstract class CloudAssemblySourceBuilder {
/**
* Helper to provide the CloudAssemblySourceBuilder with required toolkit services
* @deprecated this should move to the toolkit really.
*/
protected abstract toolkitServices(): Promise<ToolkitServices>;
protected abstract sourceBuilderServices(): Promise<ToolkitServices>;

/**
* Create a Cloud Assembly from a Cloud Assembly builder function.
Expand All @@ -27,7 +27,7 @@ export abstract class CloudAssemblySourceBuilder {
builder: AssemblyBuilder,
props: CdkAppSourceProps = {},
): Promise<ICloudAssemblySource> {
const services = await this.toolkitServices();
const services = await this.sourceBuilderServices();
const context = new Context({ bag: new Settings(props.context ?? {}) });
const contextAssemblyProps: ContextAwareCloudAssemblyProps = {
services,
Expand Down Expand Up @@ -65,7 +65,7 @@ export abstract class CloudAssemblySourceBuilder {
* @returns the CloudAssembly source
*/
public async fromAssemblyDirectory(directory: string): Promise<ICloudAssemblySource> {
const services: ToolkitServices = await this.toolkitServices();
const services: ToolkitServices = await this.sourceBuilderServices();
const contextAssemblyProps: ContextAwareCloudAssemblyProps = {
services,
context: new Context(), // @todo there is probably a difference between contextaware and contextlookup sources
Expand All @@ -89,7 +89,7 @@ export abstract class CloudAssemblySourceBuilder {
* @returns the CloudAssembly source
*/
public async fromCdkApp(app: string, props: CdkAppSourceProps = {}): Promise<ICloudAssemblySource> {
const services: ToolkitServices = await this.toolkitServices();
const services: ToolkitServices = await this.sourceBuilderServices();
// @todo this definitely needs to read files from the CWD
const context = new Context({ bag: new Settings(props.context ?? {}) });
const contextAssemblyProps: ContextAwareCloudAssemblyProps = {
Expand Down Expand Up @@ -122,7 +122,20 @@ export abstract class CloudAssemblySourceBuilder {

const env = await prepareDefaultEnvironment(services, { outdir });
return await withContext(context.all, env, props.synthOptions, async (envWithContext, _ctx) => {
await execInChildProcess(commandLine.join(' '), { extraEnv: envWithContext, cwd: props.workingDirectory });
await execInChildProcess(commandLine.join(' '), {
eventPublisher: async (type, line) => {
switch (type) {
case 'data_stdout':
await services.ioHost.notify(info(line, 'CDK_ASSEMBLY_I1001'));
break;
case 'data_stderr':
await services.ioHost.notify(error(line, 'CDK_ASSEMBLY_E1002'));
break;
}
},
extraEnv: envWithContext,
cwd: props.workingDirectory,
});
return assemblyFromDirectory(outdir, services.ioHost);
});
} finally {
Expand Down
22 changes: 4 additions & 18 deletions packages/@aws-cdk/toolkit/lib/api/io/private/codes.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,6 @@
import { IoMessageCode } from '../io-message';

export const CODES = {
// Default codes -- all 0000 codes
CDK_TOOLKIT_I0000: 'Default toolkit info code',
CDK_TOOLKIT_E0000: 'Default toolkit error code',
CDK_TOOLKIT_W0000: 'Default toolkit warning code',
CDK_SDK_I0000: 'Default sdk info code',
CDK_SDK_E0000: 'Default sdk error code',
CDK_SDK_WOOOO: 'Default sdk warning code',
CDK_ASSETS_I0000: 'Default assets info code',
CDK_ASSETS_E0000: 'Default assets error code',
CDK_ASSETS_W0000: 'Default assets warning code',
CDK_ASSEMBLY_I0000: 'Default assembly info code',
CDK_ASSEMBLY_E0000: 'Default assembly error code',
CDK_ASSEMBLY_W0000: 'Default assembly warning code',

// Toolkit Info codes
CDK_TOOLKIT_I0001: 'Display stack data',
CDK_TOOLKIT_I0002: 'Successfully deployed stacks',
Expand All @@ -31,10 +17,10 @@ export const CODES = {
// Assembly Info codes
CDK_ASSEMBLY_I0042: 'Writing updated context',
CDK_ASSEMBLY_I0241: 'Fetching missing context',

// Assembly Warning codes

// Assembly Error codes
CDK_ASSEMBLY_I1000: 'Cloud assembly output starts',
CDK_ASSEMBLY_I1001: 'Output lines emitted by the cloud assembly to stdout',
CDK_ASSEMBLY_E1002: 'Output lines emitted by the cloud assembly to stderr',
CDK_ASSEMBLY_I1003: 'Cloud assembly output finished',
CDK_ASSEMBLY_E1111: 'Incompatible CDK CLI version. Upgrade needed.',
};

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export interface ToolkitOptions {

/**
* Whether to allow ANSI colors and formatting in IoHost messages.
* Setting this value to `falsez enforces that no color or style shows up
* Setting this value to `false` enforces that no color or style shows up
* in messages sent to the IoHost.
* Setting this value to true is a no-op; it is equivalent to the default.
*
Expand Down Expand Up @@ -144,9 +144,9 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
/**
* Helper to provide the CloudAssemblySourceBuilder with required toolkit services
*/
protected override async toolkitServices(): Promise<ToolkitServices> {
protected override async sourceBuilderServices(): Promise<ToolkitServices> {
return {
ioHost: this.ioHost,
ioHost: withAction(this.ioHost, 'assembly'),
sdkProvider: await this.sdkProvider('assembly'),
};
}
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/toolkit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"@types/fs-extra": "^9.0.13",
"@types/jest": "^29.5.14",
"@types/node": "^18.18.14",
"@types/split2": "^4.2.3",
"aws-cdk": "0.0.0",
"aws-cdk-lib": "0.0.0",
"aws-sdk-client-mock": "^4.0.1",
Expand Down Expand Up @@ -105,6 +106,7 @@
"promptly": "^3.2.0",
"proxy-agent": "^6.4.0",
"semver": "^7.6.3",
"split2": "^4.2.0",
"strip-ansi": "^6.0.1",
"table": "^6.8.2",
"uuid": "^8.3.2",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as cdk from 'aws-cdk-lib/core';

console.log('line one');
const app = new cdk.App();
console.log('line two');
new cdk.Stack(app, 'Stack1');
console.log('line three');
app.synth();
console.log('line four');
11 changes: 11 additions & 0 deletions packages/@aws-cdk/toolkit/test/_fixtures/validation-error/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as cdk from 'aws-cdk-lib/core';
import * as sqs from 'aws-cdk-lib/aws-sqs';

const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack1');
new sqs.Queue(stack, 'Queue1', {
queueName: "Queue1",
fifo: true,
});

app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,38 @@ describe('fromCdkApp', () => {
// THEN
expect(JSON.stringify(stack)).toContain('amzn-s3-demo-bucket');
});

test('will capture error output', async () => {
// WHEN
const cx = await appFixture(toolkit, 'validation-error');
try {
await cx.produce();
} catch {
// we are just interested in the output for this test
}

// THEN
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
level: 'error',
code: 'CDK_ASSEMBLY_E1002',
message: expect.stringContaining('ValidationError'),
}));
});

test('will capture all output', async () => {
// WHEN
const cx = await appFixture(toolkit, 'console-output');
await cx.produce();

// THEN
['one', 'two', 'three', 'four'].forEach((line) => {
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
level: 'info',
code: 'CDK_ASSEMBLY_I1001',
message: `line ${line}`,
}));
});
});
});

describe('fromAssemblyDirectory', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/toolkit/cli-io-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export const levelPriority: Record<IoMessageLevel, number> = {
* The current action being performed by the CLI. 'none' represents the absence of an action.
*/
export type ToolkitAction =
| 'assembly'
| 'bootstrap'
| 'synth'
| 'list'
Expand Down
12 changes: 12 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9422,6 +9422,13 @@
resolved "https://registry.npmjs.org/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-8.1.5.tgz#5fd3592ff10c1e9695d377020c033116cc2889f2"
integrity sha512-mQkU2jY8jJEF7YHjHvsQO8+3ughTL1mcnn96igfhONmR+fUPSKIkefQYpSe8bsly2Ep7oQbn/6VG5/9/0qcArQ==

"@types/split2@^4.2.3":
version "4.2.3"
resolved "https://registry.npmjs.org/@types/split2/-/split2-4.2.3.tgz#ddd9b6b8518df6e0a7825851fcd98de12e415f0b"
integrity sha512-59OXIlfUsi2k++H6CHgUQKEb2HKRokUA39HY1i1dS8/AIcqVjtAAFdf8u+HxTWK/4FUHMJQlKSZ4I6irCBJ1Zw==
dependencies:
"@types/node" "*"

"@types/stack-utils@^2.0.0":
version "2.0.3"
resolved "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz#6209321eb2c1712a7e7466422b8cb1fc0d9dd5d8"
Expand Down Expand Up @@ -19005,6 +19012,11 @@ split2@^3.0.0, split2@^3.2.2:
dependencies:
readable-stream "^3.0.0"

split2@^4.2.0:
version "4.2.0"
resolved "https://registry.npmjs.org/split2/-/split2-4.2.0.tgz#c9c5920904d148bab0b9f67145f245a86aadbfa4"
integrity sha512-UcjcJOWknrNkF6PLX83qcHM6KHgVKNkV62Y8a5uYDVv9ydGQVwAHMKqHdJje1VTWpljG0WYpCDhrCdAOYH4TWg==

split@^1.0.0, split@^1.0.1:
version "1.0.1"
resolved "https://registry.npmjs.org/split/-/split-1.0.1.tgz#605bd9be303aa59fb35f9229fbea0ddec9ea07d9"
Expand Down

0 comments on commit c83d4ca

Please sign in to comment.