From ce5a6a6f7aa9c0002b056048339a6edce4c8d98c Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Tue, 19 Sep 2023 18:09:28 +0200 Subject: [PATCH 1/5] fix(cli): report cfn deployment errors from nested stacks --- .../cloudformation/stack-activity-monitor.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts index 5247ac25f8297..8b1990ab2174a 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts @@ -221,14 +221,15 @@ export class StackActivityMonitor { * see a next page and the last event in the page is new to us (and within the time window). * haven't seen the final event */ - private async readNewEvents(): Promise { + private async readNewEvents(stackName?: string): Promise { + const stackToPollForEvents = stackName ?? this.stackName; const events: StackActivity[] = []; - + const CFN_SUCCESS_STATUS = ['UPDATE_COMPLETE', 'CREATE_COMPLETE', 'DELETE_COMPLETE', 'DELETE_SKIPPED']; try { let nextToken: string | undefined; let finished = false; while (!finished) { - const response = await this.cfn.describeStackEvents({ StackName: this.stackName, NextToken: nextToken }).promise(); + const response = await this.cfn.describeStackEvents({ StackName: stackToPollForEvents, NextToken: nextToken }).promise(); const eventPage = response?.StackEvents ?? []; for (const event of eventPage) { @@ -249,6 +250,12 @@ export class StackActivityMonitor { event: event, metadata: this.findMetadataFor(event.LogicalResourceId), }); + + if (event.ResourceType === 'AWS::CloudFormation::Stack' && !CFN_SUCCESS_STATUS.includes(event.ResourceStatus ?? '')) { + if (event.PhysicalResourceId !== stackToPollForEvents) { + await this.readNewEvents(event.PhysicalResourceId); + } + } } // We're also done if there's nothing left to read @@ -258,7 +265,7 @@ export class StackActivityMonitor { } } } catch (e: any) { - if (e.code === 'ValidationError' && e.message === `Stack [${this.stackName}] does not exist`) { + if (e.code === 'ValidationError' && e.message === `Stack [${stackToPollForEvents}] does not exist`) { return; } throw e; @@ -803,4 +810,3 @@ function shorten(maxWidth: number, p: string) { const TIMESTAMP_WIDTH = 12; const STATUS_WIDTH = 20; - From ebd10bd29cf07619ac82299dc44005beb0681695 Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Wed, 27 Sep 2023 17:25:48 +0200 Subject: [PATCH 2/5] fix(cli): report errors from resource failures in nested stacks --- .../cloudformation/stack-activity-monitor.ts | 5 +- .../aws-cdk/test/util/stack-monitor.test.ts | 267 ++++++++++++------ 2 files changed, 184 insertions(+), 88 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts index 8b1990ab2174a..5a72e9a9c7780 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts @@ -252,7 +252,8 @@ export class StackActivityMonitor { }); if (event.ResourceType === 'AWS::CloudFormation::Stack' && !CFN_SUCCESS_STATUS.includes(event.ResourceStatus ?? '')) { - if (event.PhysicalResourceId !== stackToPollForEvents) { + // If the event is not for `this` stack, recursively call for events in the nested stack + if (event.PhysicalResourceId !== stackToPollForEvents && event.LogicalResourceId !== stackToPollForEvents) { await this.readNewEvents(event.PhysicalResourceId); } } @@ -482,7 +483,7 @@ abstract class ActivityPrinterBase implements IActivityPrinter { this.resourcesPrevCompleteState[activity.event.LogicalResourceId] = status; } - if (hookStatus!== undefined && hookStatus.endsWith('_COMPLETE_FAILED') && activity.event.LogicalResourceId !== undefined && hookType !== undefined) { + if (hookStatus !== undefined && hookStatus.endsWith('_COMPLETE_FAILED') && activity.event.LogicalResourceId !== undefined && hookType !== undefined) { if (this.hookFailureMap.has(activity.event.LogicalResourceId)) { this.hookFailureMap.get(activity.event.LogicalResourceId)?.set(hookType, activity.event.HookStatusReason ?? ''); diff --git a/packages/aws-cdk/test/util/stack-monitor.test.ts b/packages/aws-cdk/test/util/stack-monitor.test.ts index ecdedcc4381d2..c34a7a3ae040e 100644 --- a/packages/aws-cdk/test/util/stack-monitor.test.ts +++ b/packages/aws-cdk/test/util/stack-monitor.test.ts @@ -10,95 +10,171 @@ beforeEach(() => { printer = new FakePrinter(); }); -test('continue to the next page if it exists', async () => { - await testMonitorWithEventCalls([ - (request) => { - expect(request.NextToken).toBeUndefined(); - return { - StackEvents: [event(102)], - NextToken: 'some-token', - }; - }, - (request) => { - expect(request.NextToken).toBe('some-token'); - return { - StackEvents: [event(101)], - }; - }, - ]); - - // Printer sees them in chronological order - expect(printer.eventIds).toEqual(['101', '102']); -}); +describe('stack monitor event ordering and pagination', () => { + test('continue to the next page if it exists', async () => { + await testMonitorWithEventCalls([ + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [event(102)], + NextToken: 'some-token', + }; + }, + (request) => { + expect(request.NextToken).toBe('some-token'); + return { + StackEvents: [event(101)], + }; + }, + ]); -test('do not page further if we already saw the last event', async () => { - await testMonitorWithEventCalls([ - (request) => { - expect(request.NextToken).toBeUndefined(); - return { - StackEvents: [event(101)], - }; - }, - (request) => { - expect(request.NextToken).toBeUndefined(); - return { - StackEvents: [event(102), event(101)], - NextToken: 'some-token', - }; - }, - (request) => { - // Did not use the token - expect(request.NextToken).toBeUndefined(); - return {}; - }, - ]); - - // Seen in chronological order - expect(printer.eventIds).toEqual(['101', '102']); -}); + // Printer sees them in chronological order + expect(printer.eventIds).toEqual(['101', '102']); + }); + + test('do not page further if we already saw the last event', async () => { + await testMonitorWithEventCalls([ + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [event(101)], + }; + }, + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [event(102), event(101)], + NextToken: 'some-token', + }; + }, + (request) => { + // Did not use the token + expect(request.NextToken).toBeUndefined(); + return {}; + }, + ]); + + // Seen in chronological order + expect(printer.eventIds).toEqual(['101', '102']); + }); + + test('do not page further if the last event is too old', async () => { + await testMonitorWithEventCalls([ + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [event(101), event(95)], + NextToken: 'some-token', + }; + }, + (request) => { + // Start again from the top + expect(request.NextToken).toBeUndefined(); + return {}; + }, + ]); + + // Seen only the new one + expect(printer.eventIds).toEqual(['101']); + }); + + test('do a final request after the monitor is stopped', async () => { + await testMonitorWithEventCalls([ + // Before stop + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [event(101)], + }; + }, + ], + // After stop + [ + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [event(102), event(101)], + }; + }, + ]); -test('do not page further if the last event is too old', async () => { - await testMonitorWithEventCalls([ - (request) => { - expect(request.NextToken).toBeUndefined(); - return { - StackEvents: [event(101), event(95)], - NextToken: 'some-token', - }; - }, - (request) => { - // Start again from the top - expect(request.NextToken).toBeUndefined(); - return {}; - }, - ]); - - // Seen only the new one - expect(printer.eventIds).toEqual(['101']); + // Seen both + expect(printer.eventIds).toEqual(['101', '102']); + }); }); -test('do a final request after the monitor is stopped', async () => { - await testMonitorWithEventCalls([ - // Before stop - (request) => { - expect(request.NextToken).toBeUndefined(); - return { - StackEvents: [event(101)], - }; - }, - ], - // After stop - [ - (request) => { - expect(request.NextToken).toBeUndefined(); - return { - StackEvents: [event(102), event(101)], - }; - }, - ]); - - // Seen both - expect(printer.eventIds).toEqual(['101', '102']); +describe('stack monitor, collecting errors from events', () => { + test('return errors from the root stack', async () => { + const monitor = await testMonitorWithEventCalls([ + (request) => { + expect(request.NextToken).toBeUndefined(); + return { + StackEvents: [addErrorToStackEvent(event(100))], + }; + }, + ]); + + expect(monitor.errors).toStrictEqual(['Test Error']); + }); + + test('return errors from the nested stack', async () => { + const monitor = await testMonitorWithEventCalls([ + (request) => { + expect(request.StackName).toStrictEqual('StackName'); + return { + StackEvents: [ + addErrorToStackEvent( + event(100), { + logicalResourceId: 'nestedStackLogicalResourceId', + physicalResourceId: 'nestedStackPhysicalResourceId', + resourceType: 'AWS::CloudFormation::Stack', + resourceStatusReason: 'nested stack failed', + }, + ), + ], + }; + }, + (request) => { + expect(request.StackName).toStrictEqual('nestedStackPhysicalResourceId'); + return { + StackEvents: [ + addErrorToStackEvent( + event(101), { + logicalResourceId: 'nestedResource', + resourceType: 'Some::Nested::Resource', + resourceStatusReason: 'actual failure error message', + }, + ), + ], + }; + }, + ]); + + expect(monitor.errors).toStrictEqual(['actual failure error message', 'nested stack failed']); + }); + + test('does not check for nested stacks that have already completed successfully', async () => { + const monitor = await testMonitorWithEventCalls([ + (request) => { + expect(request.StackName).toStrictEqual('StackName'); + return { + StackEvents: [ + addErrorToStackEvent( + event(100), { + logicalResourceId: 'nestedStackLogicalResourceId', + physicalResourceId: 'nestedStackPhysicalResourceId', + resourceType: 'AWS::CloudFormation::Stack', + resourceStatusReason: 'nested stack status reason', + resourceStatus: 'CREATE_COMPLETE', + }, + ), + ], + }; + }, + ]); + + expect(monitor.errors).toStrictEqual([]); + }); }); const T0 = 1597837230504; @@ -115,10 +191,28 @@ function event(nr: number): AWS.CloudFormation.StackEvent { }; } +function addErrorToStackEvent( + eventToUpdate: AWS.CloudFormation.StackEvent, + props: { + resourceStatus?: string, + resourceType?: string, + resourceStatusReason?: string, + logicalResourceId?: string, + physicalResourceId?: string, + } = {}, +): AWS.CloudFormation.StackEvent { + eventToUpdate.ResourceStatus = props.resourceStatus ?? 'UPDATE_FAILED'; + eventToUpdate.ResourceType = props.resourceType ?? 'Test::Resource::Type'; + eventToUpdate.ResourceStatusReason = props.resourceStatusReason ?? 'Test Error'; + eventToUpdate.LogicalResourceId = props.logicalResourceId ?? 'testLogicalId'; + eventToUpdate.PhysicalResourceId = props.physicalResourceId ?? 'testPhysicalResourceId'; + return eventToUpdate; +} + async function testMonitorWithEventCalls( beforeStopInvocations: Array<(x: AWS.CloudFormation.DescribeStackEventsInput) => AWS.CloudFormation.DescribeStackEventsOutput>, afterStopInvocations: Array<(x: AWS.CloudFormation.DescribeStackEventsInput) => AWS.CloudFormation.DescribeStackEventsOutput> = [], -) { +): Promise { let describeStackEvents = (jest.fn() as jest.Mock); let finished = false; @@ -144,6 +238,7 @@ async function testMonitorWithEventCalls( const monitor = new StackActivityMonitor(sdk.cloudFormation(), 'StackName', printer, undefined, new Date(T100)).start(); await waitForCondition(() => finished); await monitor.stop(); + return monitor; } class FakePrinter implements IActivityPrinter { From 7717dc123a4a0d83c42e5b87e4a48ebc54cab173 Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Wed, 27 Sep 2023 17:30:17 +0200 Subject: [PATCH 3/5] try this --- .../lib/api/util/cloudformation/stack-activity-monitor.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts index 5a72e9a9c7780..801970b7f7768 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts @@ -811,3 +811,4 @@ function shorten(maxWidth: number, p: string) { const TIMESTAMP_WIDTH = 12; const STATUS_WIDTH = 20; + From 9a3a6e88e4a7993fc4ab4be65380a26572562eb6 Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Wed, 27 Sep 2023 17:30:33 +0200 Subject: [PATCH 4/5] revert try this --- .../lib/api/util/cloudformation/stack-activity-monitor.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts index 801970b7f7768..5a72e9a9c7780 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts @@ -811,4 +811,3 @@ function shorten(maxWidth: number, p: string) { const TIMESTAMP_WIDTH = 12; const STATUS_WIDTH = 20; - From 61245d58a1da289a40d8b3285217b5cfc9a00bf1 Mon Sep 17 00:00:00 2001 From: Praveen Gupta Date: Wed, 27 Sep 2023 22:18:41 +0200 Subject: [PATCH 5/5] PR updates --- .../lib/api/util/cloudformation/stack-activity-monitor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts index 5a72e9a9c7780..63f2b9e6c2071 100644 --- a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts +++ b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts @@ -253,7 +253,7 @@ export class StackActivityMonitor { if (event.ResourceType === 'AWS::CloudFormation::Stack' && !CFN_SUCCESS_STATUS.includes(event.ResourceStatus ?? '')) { // If the event is not for `this` stack, recursively call for events in the nested stack - if (event.PhysicalResourceId !== stackToPollForEvents && event.LogicalResourceId !== stackToPollForEvents) { + if (event.PhysicalResourceId !== stackToPollForEvents) { await this.readNewEvents(event.PhysicalResourceId); } }