Skip to content

Commit

Permalink
fix(cli): fix stack monitoring when the stack events do not have phsi…
Browse files Browse the repository at this point in the history
…cal resource id set (#27692)

`PhysicalResourceId` for a `StackEvent` is [not required](https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_StackEvent.html) and can be empty. This causes error in stack deployment activty monitoring where we try to monitor nested stacks but empty `PhysicalResourceId` in the `StackEvent` causes it to fail with the error

```
[ValidationError]: 2 validation errors detected: 
Value '' at 'stackName' failed to satisfy constraint: Member must have length greater than or equal to 1; 
Value '' at 'stackName' failed to satisfy constraint: Member must satisfy regular expression pattern: [a-zA-Z][-a-zA-Z0-9]*|arn:[-a-zA-Z0-9:/._+]*
```
This PR adds a check to avoid that.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Amplifiyer authored Nov 1, 2023
1 parent 89f4f86 commit 857ab7d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ 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) {
// If the event is not for `this` stack and has a physical resource Id, recursively call for events in the nested stack
if (event.PhysicalResourceId && event.PhysicalResourceId !== stackToPollForEvents) {
await this.readNewEvents(event.PhysicalResourceId);
}
}
Expand Down
37 changes: 37 additions & 0 deletions packages/aws-cdk/test/util/stack-monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,43 @@ describe('stack monitor, collecting errors from events', () => {
expect(monitor.errors).toStrictEqual(['actual failure error message', 'nested stack failed']);
});

test('does not consider events without physical resource id for monitoring nested stacks', async () => {
const monitor = await testMonitorWithEventCalls([
(request) => {
expect(request.StackName).toStrictEqual('StackName');
return {
StackEvents: [
addErrorToStackEvent(
event(100), {
logicalResourceId: 'nestedStackLogicalResourceId',
physicalResourceId: '',
resourceType: 'AWS::CloudFormation::Stack',
resourceStatusReason: 'nested stack failed',
},
),
],
};
},
(request) => {
// Note that the second call happened for the top level stack instead of a nested stack
expect(request.StackName).toStrictEqual('StackName');
return {
StackEvents: [
addErrorToStackEvent(
event(101), {
logicalResourceId: 'OtherResource',
resourceType: 'Some::Other::Resource',
resourceStatusReason: 'some failure',
},
),
],
};
},
]);

expect(monitor.errors).toStrictEqual(['nested stack failed', 'some failure']);
});

test('does not check for nested stacks that have already completed successfully', async () => {
const monitor = await testMonitorWithEventCalls([
(request) => {
Expand Down

0 comments on commit 857ab7d

Please sign in to comment.