Skip to content

Commit e2cbf60

Browse files
authored
Refactor DescribeEvents to be paginated and move page exhaustion loop… (#356)
Refactor DescribeEvents to be paginated and move page exhaustion loop to business layer
1 parent 9e6a9eb commit e2cbf60

File tree

6 files changed

+104
-126
lines changed

6 files changed

+104
-126
lines changed

src/services/CfnService.ts

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -201,35 +201,22 @@ export class CfnService {
201201

202202
@Count({ name: 'describeEvents' })
203203
public async describeEvents(params: {
204-
ChangeSetName: string;
205-
StackName: string;
204+
StackName?: string;
205+
ChangeSetName?: string;
206+
OperationId?: string;
207+
FailedEventsOnly?: boolean;
208+
NextToken?: string;
206209
}): Promise<DescribeEventsCommandOutput> {
207210
return await this.withClient(async (client) => {
208-
let nextToken: string | undefined;
209-
let result: DescribeEventsCommandOutput | undefined;
210-
const operationEvents: DescribeEventsCommandOutput['OperationEvents'] = [];
211-
212-
do {
213-
const response = (await client.send(
214-
new DescribeEventsCommand({
215-
...params,
216-
NextToken: nextToken,
217-
}),
218-
)) as unknown as DescribeEventsCommandOutput;
219-
220-
if (result) {
221-
operationEvents.push(...(response.OperationEvents ?? []));
222-
} else {
223-
result = response;
224-
operationEvents.push(...(result.OperationEvents ?? []));
225-
}
226-
227-
nextToken = response.NextToken;
228-
} while (nextToken);
229-
230-
result.OperationEvents = operationEvents;
231-
result.NextToken = undefined;
232-
return result;
211+
return (await client.send(
212+
new DescribeEventsCommand({
213+
StackName: params.StackName,
214+
ChangeSetName: params.ChangeSetName,
215+
OperationId: params.OperationId,
216+
Filters: params.FailedEventsOnly ? { FailedEvents: true } : undefined,
217+
NextToken: params.NextToken,
218+
}),
219+
)) as unknown as DescribeEventsCommandOutput;
233220
});
234221
}
235222

src/stacks/actions/StackActionOperations.ts

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import {
22
Change,
33
ChangeSetType,
4-
DescribeEventsCommandOutput,
54
StackStatus,
65
OnStackFailure,
76
EventType,
87
HookFailureMode,
8+
OperationEvent,
99
} from '@aws-sdk/client-cloudformation';
1010
import { WaiterState } from '@smithy/util-waiter';
1111
import { dump } from 'js-yaml';
@@ -345,21 +345,17 @@ export function processWorkflowUpdates(
345345
return existingWorkflow;
346346
}
347347

348-
export function parseValidationEvents(events: DescribeEventsCommandOutput, validationName: string): ValidationDetail[] {
349-
const validEvents = events.OperationEvents?.filter((event) => event.EventType === EventType.VALIDATION_ERROR);
350-
351-
return (
352-
validEvents?.map((event) => {
353-
return {
354-
Timestamp: event.Timestamp ? DateTime.fromISO(event.Timestamp.toISOString()) : undefined,
355-
ValidationName: validationName,
356-
LogicalId: event.LogicalResourceId,
357-
Message: [event.ValidationName, event.ValidationStatusReason].filter(Boolean).join(': '),
358-
Severity: event.ValidationFailureMode === HookFailureMode.FAIL ? 'ERROR' : 'INFO',
359-
ResourcePropertyPath: event.ValidationPath,
360-
};
361-
}) ?? []
362-
);
348+
export function parseValidationEvents(events: OperationEvent[], validationName: string): ValidationDetail[] {
349+
const validEvents = events.filter((event) => event.EventType === EventType.VALIDATION_ERROR);
350+
351+
return validEvents.map((event) => ({
352+
Timestamp: event.Timestamp ? DateTime.fromISO(event.Timestamp.toISOString()) : undefined,
353+
ValidationName: validationName,
354+
LogicalId: event.LogicalResourceId,
355+
Message: [event.ValidationName, event.ValidationStatusReason].filter(Boolean).join(': '),
356+
Severity: event.ValidationFailureMode === HookFailureMode.FAIL ? 'ERROR' : 'INFO',
357+
ResourcePropertyPath: event.ValidationPath,
358+
}));
363359
}
364360

365361
export async function publishValidationDiagnostics(

src/stacks/actions/ValidationWorkflow.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ChangeSetType, StackStatus } from '@aws-sdk/client-cloudformation';
1+
import { ChangeSetType, OperationEvent, StackStatus } from '@aws-sdk/client-cloudformation';
22
import { AwsCredentials } from '../../auth/AwsCredentials';
33
import { SyntaxTreeManager } from '../../context/syntaxtree/SyntaxTreeManager';
44
import { DocumentManager } from '../../document/DocumentManager';
@@ -181,12 +181,8 @@ export class ValidationWorkflow implements StackActionWorkflow<CreateValidationP
181181
}
182182

183183
if (this.featureFlag.isEnabled(this.awsCredentials.getIAM().region)) {
184-
const describeEventsResponse = await this.cfnService.describeEvents({
185-
ChangeSetName: changeSetName,
186-
StackName: stackName,
187-
});
188-
189-
const validationDetails = parseValidationEvents(describeEventsResponse, VALIDATION_NAME);
184+
const allEvents = await this.fetchAllFailedEvents(changeSetName, stackName);
185+
const validationDetails = parseValidationEvents(allEvents, VALIDATION_NAME);
190186

191187
existingWorkflow = processWorkflowUpdates(this.workflows, existingWorkflow, {
192188
validationDetails: validationDetails,
@@ -232,6 +228,24 @@ export class ValidationWorkflow implements StackActionWorkflow<CreateValidationP
232228
}
233229
}
234230

231+
private async fetchAllFailedEvents(changeSetName: string, stackName: string): Promise<OperationEvent[]> {
232+
const allEvents: OperationEvent[] = [];
233+
let nextToken: string | undefined = undefined;
234+
235+
do {
236+
const response = await this.cfnService.describeEvents({
237+
ChangeSetName: changeSetName,
238+
StackName: stackName,
239+
FailedEventsOnly: true,
240+
NextToken: nextToken,
241+
});
242+
allEvents.push(...(response.OperationEvents ?? []));
243+
nextToken = response.NextToken;
244+
} while (nextToken);
245+
246+
return allEvents;
247+
}
248+
235249
static create(core: CfnInfraCore, external: CfnExternal, validationManager: ValidationManager): ValidationWorkflow {
236250
return new ValidationWorkflow(
237251
external.cfnService,

tst/unit/services/CfnService.test.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ describe('CfnService', () => {
357357
expect(result).toEqual(MOCK_RESPONSES.DESCRIBE_EVENTS);
358358
});
359359

360-
it('should fetch all pages when paginated', async () => {
360+
it('should return single page with NextToken for pagination', async () => {
361361
const page1 = {
362362
...MOCK_RESPONSES.DESCRIBE_EVENTS,
363363
OperationEvents: [
@@ -376,30 +376,16 @@ describe('CfnService', () => {
376376
],
377377
NextToken: 'token1',
378378
};
379-
const page2 = {
380-
...MOCK_RESPONSES.DESCRIBE_EVENTS,
381-
OperationEvents: [
382-
{
383-
EventId: 'event-3',
384-
EventType: EventType.VALIDATION_ERROR,
385-
Timestamp: new Date('2023-01-01T00:02:00Z'),
386-
LogicalResourceId: 'Resource3',
387-
},
388-
],
389-
NextToken: undefined,
390-
};
391379

392-
cloudFormationMock.on(DescribeEventsCommand).resolvesOnce(page1).resolvesOnce(page2);
380+
cloudFormationMock.on(DescribeEventsCommand).resolvesOnce(page1);
393381

394382
const result = await service.describeEvents({
395383
ChangeSetName: TEST_CONSTANTS.CHANGE_SET_NAME,
396384
StackName: TEST_CONSTANTS.STACK_NAME,
397385
});
398386

399-
expect(result.OperationEvents).toHaveLength(3);
400-
expect(result.OperationEvents?.[0].EventId).toBe('event-1');
401-
expect(result.OperationEvents?.[2].EventId).toBe('event-3');
402-
expect(result.NextToken).toBeUndefined();
387+
expect(result.OperationEvents).toHaveLength(2);
388+
expect(result.NextToken).toBe('token1');
403389
});
404390

405391
it('should throw CloudFormationServiceException when API call fails', async () => {

tst/unit/stackActions/StackActionWorkflowOperations.test.ts

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
OnStackFailure,
66
EventType,
77
HookFailureMode,
8+
OperationEvent,
89
} from '@aws-sdk/client-cloudformation';
910
import { WaiterState } from '@smithy/util-waiter';
1011
import { DateTime } from 'luxon';
@@ -432,36 +433,33 @@ describe('StackActionWorkflowOperations', () => {
432433

433434
describe('parseValidationEvents', () => {
434435
it('should parse validation events correctly', () => {
435-
const events = {
436-
OperationEvents: [
437-
{
438-
EventId: 'event-1',
439-
EventType: EventType.VALIDATION_ERROR,
440-
Timestamp: new Date('2023-01-01T00:00:00Z'),
441-
LogicalResourceId: 'MyS3Bucket',
442-
ValidationPath: '/Resources/MyS3Bucket/Properties/BucketName',
443-
ValidationFailureMode: HookFailureMode.FAIL,
444-
ValidationName: 'S3BucketValidation',
445-
ValidationStatusReason: 'Bucket name must be globally unique',
446-
},
447-
{
448-
EventId: 'event-2',
449-
EventType: EventType.VALIDATION_ERROR,
450-
Timestamp: new Date('2023-01-01T00:01:00Z'),
451-
LogicalResourceId: 'MyLambda',
452-
ValidationFailureMode: HookFailureMode.WARN,
453-
ValidationName: 'LambdaValidation',
454-
ValidationStatusReason: 'Runtime version is deprecated',
455-
},
456-
{
457-
EventId: 'event-3',
458-
EventType: EventType.HOOK_INVOCATION_ERROR,
459-
Timestamp: new Date('2023-01-01T00:02:00Z'),
460-
LogicalResourceId: 'MyResource',
461-
},
462-
],
463-
$metadata: {},
464-
};
436+
const events = [
437+
{
438+
EventId: 'event-1',
439+
EventType: EventType.VALIDATION_ERROR,
440+
Timestamp: new Date('2023-01-01T00:00:00Z'),
441+
LogicalResourceId: 'MyS3Bucket',
442+
ValidationPath: '/Resources/MyS3Bucket/Properties/BucketName',
443+
ValidationFailureMode: HookFailureMode.FAIL,
444+
ValidationName: 'S3BucketValidation',
445+
ValidationStatusReason: 'Bucket name must be globally unique',
446+
},
447+
{
448+
EventId: 'event-2',
449+
EventType: EventType.VALIDATION_ERROR,
450+
Timestamp: new Date('2023-01-01T00:01:00Z'),
451+
LogicalResourceId: 'MyLambda',
452+
ValidationFailureMode: HookFailureMode.WARN,
453+
ValidationName: 'LambdaValidation',
454+
ValidationStatusReason: 'Runtime version is deprecated',
455+
},
456+
{
457+
EventId: 'event-3',
458+
EventType: EventType.HOOK_INVOCATION_ERROR,
459+
Timestamp: new Date('2023-01-01T00:02:00Z'),
460+
LogicalResourceId: 'MyResource',
461+
},
462+
];
465463

466464
const validationName = 'Enhanced Validation';
467465
const result = parseValidationEvents(events, validationName);
@@ -488,10 +486,7 @@ describe('StackActionWorkflowOperations', () => {
488486
});
489487

490488
it('should handle empty events', () => {
491-
const events = {
492-
OperationEvents: [],
493-
$metadata: {},
494-
};
489+
const events: OperationEvent[] = [];
495490

496491
const result = parseValidationEvents(events, 'Test Validation');
497492

tst/unit/stackActions/ValidationWorkflow.test.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { DescribeEventsCommandOutput, EventType, HookFailureMode } from '@aws-sdk/client-cloudformation';
1+
import { EventType, HookFailureMode } from '@aws-sdk/client-cloudformation';
22
import { DateTime } from 'luxon';
33
import { describe, it, expect, vi, beforeEach } from 'vitest';
44
import { AwsCredentials } from '../../../src/auth/AwsCredentials';
@@ -458,15 +458,13 @@ describe('ValidationWorkflow', () => {
458458
return updated;
459459
});
460460

461-
const mockEmptyDescribeEventsResponse: DescribeEventsCommandOutput = {
462-
OperationEvents: [],
463-
$metadata: {},
464-
};
465-
466461
mockFeatureFlag.isEnabled = vi.fn().mockReturnValue(true);
467462
mockAwsCredentials.getIAM = vi.fn().mockReturnValue({ region: 'us-east-1' });
468463

469-
mockCfnService.describeEvents = vi.fn().mockResolvedValue(mockEmptyDescribeEventsResponse);
464+
mockCfnService.describeEvents = vi.fn().mockResolvedValue({
465+
OperationEvents: [],
466+
$metadata: {},
467+
});
470468

471469
(parseValidationEvents as any).mockReturnValue([
472470
{
@@ -696,23 +694,23 @@ describe('ValidationWorkflow', () => {
696694
changes: mockChanges,
697695
});
698696

699-
const mockDescribeEventsResponse: DescribeEventsCommandOutput = {
700-
OperationEvents: [
701-
{
702-
EventId: 'event-1',
703-
EventType: EventType.VALIDATION_ERROR,
704-
Timestamp: new Date('2023-01-01T00:00:00Z'),
705-
LogicalResourceId: 'TestResource',
706-
ValidationPath: '/Resources/TestResource/Properties/BucketName',
707-
ValidationFailureMode: HookFailureMode.FAIL,
708-
ValidationName: 'TestValidation',
709-
ValidationStatusReason: 'Test error',
710-
},
711-
],
712-
$metadata: {},
713-
};
697+
const mockOperationEvents = [
698+
{
699+
EventId: 'event-1',
700+
EventType: EventType.VALIDATION_ERROR,
701+
Timestamp: new Date('2023-01-01T00:00:00Z'),
702+
LogicalResourceId: 'TestResource',
703+
ValidationPath: '/Resources/TestResource/Properties/BucketName',
704+
ValidationFailureMode: HookFailureMode.FAIL,
705+
ValidationName: 'TestValidation',
706+
ValidationStatusReason: 'Test error',
707+
},
708+
];
714709

715-
mockCfnService.describeEvents = vi.fn().mockResolvedValueOnce(mockDescribeEventsResponse);
710+
mockCfnService.describeEvents = vi.fn().mockResolvedValueOnce({
711+
OperationEvents: mockOperationEvents,
712+
$metadata: {},
713+
});
716714

717715
const mockParseValidationEventsResponse = [
718716
{
@@ -736,12 +734,14 @@ describe('ValidationWorkflow', () => {
736734
expect(mockCfnService.describeEvents).toHaveBeenCalledWith({
737735
ChangeSetName: 'changeset-123',
738736
StackName: 'test-stack',
737+
FailedEventsOnly: true,
738+
NextToken: undefined,
739739
});
740740

741741
const workflow = (validationWorkflow as any).workflows.get('test-id');
742742
expect(workflow.changes).toEqual(mockChanges);
743743

744-
expect(parseValidationEvents).toHaveBeenCalledWith(mockDescribeEventsResponse, VALIDATION_NAME);
744+
expect(parseValidationEvents).toHaveBeenCalledWith(mockOperationEvents, VALIDATION_NAME);
745745

746746
const mockValidation = mockValidationManager.get('test-stack');
747747
expect(mockValidation?.setValidationDetails).toHaveBeenCalledWith(mockParseValidationEventsResponse);

0 commit comments

Comments
 (0)