Skip to content

Commit

Permalink
fix(events): event bus fails with duplicate policy resource (#28521)
Browse files Browse the repository at this point in the history
#27340 introduced the ability to create multiple event bus policies on a single event bus. To facilitate this, the logical Id was changed from `"Policy"` to the statementId. This triggers a replacement, which fails in CloudFormation because the statement ID does not change. The idea behind this PR is simple -- we are updating the statement ID of the policy to trigger a change for anyone who updates to the new version.

I think we are okay with this change because no one should be depending on the statementIds of their policies. And since the policy is not a stateful resource, updating the policy should not harm anyone. I have checked the feasibility of this PR on my own, hence the lack of an integ test.

closes #28520 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Jan 2, 2024
1 parent ca91626 commit 166967f
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 36 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"Name": "StackBusAA0A1E4B"
}
},
"BusStatement1B4D0336C": {
"BuscdkStatement1D7D87B9D": {
"Type": "AWS::Events::EventBusPolicy",
"Properties": {
"EventBusName": {
Expand Down Expand Up @@ -39,12 +39,12 @@
"Arn"
]
},
"Sid": "Statement1"
"Sid": "cdk-Statement1"
},
"StatementId": "Statement1"
"StatementId": "cdk-Statement1"
}
},
"BusStatement2B5FB314B": {
"BuscdkStatement2341A5B58": {
"Type": "AWS::Events::EventBusPolicy",
"Properties": {
"EventBusName": {
Expand Down Expand Up @@ -77,9 +77,9 @@
"Arn"
]
},
"Sid": "Statement2"
"Sid": "cdk-Statement2"
},
"StatementId": "Statement2"
"StatementId": "cdk-Statement2"
}
}
},
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions packages/aws-cdk-lib/aws-events/lib/event-bus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,13 @@ export class EventBus extends EventBusBase {
throw new Error('Event Bus policy statements must have a sid');
}

const policy = new EventBusPolicy(this, statement.sid, {
// In order to generate new statementIDs for the change in https://github.com/aws/aws-cdk/pull/27340
const statementId = `cdk-${statement.sid}`.slice(0, 64);
statement.sid = statementId;
const policy = new EventBusPolicy(this, statementId, {
eventBus: this,
statement: statement.toJSON(),
statementId: statement.sid,
statementId,
});

return { statementAdded: true, policyDependable: policy };
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/aws-events/test/event-bus.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ describe('event bus', () => {

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Events::EventBusPolicy', {
StatementId: '123',
StatementId: 'cdk-123',
EventBusName: {
Ref: 'BusEA82B648',
},
Expand All @@ -569,7 +569,7 @@ describe('event bus', () => {
],
},
},
Sid: '123',
Sid: 'cdk-123',
Resource: {
'Fn::GetAtt': [
'BusEA82B648',
Expand Down

0 comments on commit 166967f

Please sign in to comment.