From 166967f11727a28fc11b9af5de0fad6da2a4ad64 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Tue, 2 Jan 2024 21:54:45 +0800 Subject: [PATCH] fix(events): event bus fails with duplicate policy resource (#28521) #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* --- ...efaultTestDeployAssertE6DF8EA9.assets.json | 2 +- .../Stack.assets.json | 6 ++-- .../Stack.template.json | 12 ++++---- .../test/integ.eventbus.js.snapshot/cdk.out | 2 +- .../integ.eventbus.js.snapshot/integ.json | 2 +- .../integ.eventbus.js.snapshot/manifest.json | 30 +++++++++++++++---- .../test/integ.eventbus.js.snapshot/tree.json | 28 ++++++++--------- .../aws-cdk-lib/aws-events/lib/event-bus.ts | 7 +++-- .../aws-events/test/event-bus.test.ts | 4 +-- 9 files changed, 57 insertions(+), 36 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/IntegTestEventBusStackDefaultTestDeployAssertE6DF8EA9.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/IntegTestEventBusStackDefaultTestDeployAssertE6DF8EA9.assets.json index 4209c700d9bd4..b016e8fe026ce 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/IntegTestEventBusStackDefaultTestDeployAssertE6DF8EA9.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/IntegTestEventBusStackDefaultTestDeployAssertE6DF8EA9.assets.json @@ -1,5 +1,5 @@ { - "version": "34.0.0", + "version": "36.0.0", "files": { "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": { "source": { diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.assets.json index 0f867605130fa..7e02c8ce09ced 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.assets.json @@ -1,7 +1,7 @@ { - "version": "34.0.0", + "version": "36.0.0", "files": { - "57e14b65d6e62634abcf2a64f00eb08e99ee92617490f92b45eff16399d6173a": { + "1ac02a396bc49bcce2ab3c276dd4e234efe7e86f9d1a06c9ae16c054f1764bf1": { "source": { "path": "Stack.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "57e14b65d6e62634abcf2a64f00eb08e99ee92617490f92b45eff16399d6173a.json", + "objectKey": "1ac02a396bc49bcce2ab3c276dd4e234efe7e86f9d1a06c9ae16c054f1764bf1.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.template.json index f13964b9866c7..0fc44e208d8ce 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/Stack.template.json @@ -6,7 +6,7 @@ "Name": "StackBusAA0A1E4B" } }, - "BusStatement1B4D0336C": { + "BuscdkStatement1D7D87B9D": { "Type": "AWS::Events::EventBusPolicy", "Properties": { "EventBusName": { @@ -39,12 +39,12 @@ "Arn" ] }, - "Sid": "Statement1" + "Sid": "cdk-Statement1" }, - "StatementId": "Statement1" + "StatementId": "cdk-Statement1" } }, - "BusStatement2B5FB314B": { + "BuscdkStatement2341A5B58": { "Type": "AWS::Events::EventBusPolicy", "Properties": { "EventBusName": { @@ -77,9 +77,9 @@ "Arn" ] }, - "Sid": "Statement2" + "Sid": "cdk-Statement2" }, - "StatementId": "Statement2" + "StatementId": "cdk-Statement2" } } }, diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/cdk.out b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/cdk.out index 2313ab5436501..1f0068d32659a 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/cdk.out +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/cdk.out @@ -1 +1 @@ -{"version":"34.0.0"} \ No newline at end of file +{"version":"36.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/integ.json b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/integ.json index 6fe4c5f82830c..c5e97be0cb324 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/integ.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/integ.json @@ -1,5 +1,5 @@ { - "version": "34.0.0", + "version": "36.0.0", "testCases": { "IntegTest-EventBusStack/DefaultTest": { "stacks": [ diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/manifest.json index 11bedb11869b1..c943d4cd06dd7 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "34.0.0", + "version": "36.0.0", "artifacts": { "Stack.assets": { "type": "cdk:asset-manifest", @@ -18,7 +18,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/57e14b65d6e62634abcf2a64f00eb08e99ee92617490f92b45eff16399d6173a.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/1ac02a396bc49bcce2ab3c276dd4e234efe7e86f9d1a06c9ae16c054f1764bf1.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -40,16 +40,16 @@ "data": "BusEA82B648" } ], - "/Stack/Bus/Statement1/Resource": [ + "/Stack/Bus/cdk-Statement1/Resource": [ { "type": "aws:cdk:logicalId", - "data": "BusStatement1B4D0336C" + "data": "BuscdkStatement1D7D87B9D" } ], - "/Stack/Bus/Statement2/Resource": [ + "/Stack/Bus/cdk-Statement2/Resource": [ { "type": "aws:cdk:logicalId", - "data": "BusStatement2B5FB314B" + "data": "BuscdkStatement2341A5B58" } ], "/Stack/BootstrapVersion": [ @@ -63,6 +63,24 @@ "type": "aws:cdk:logicalId", "data": "CheckBootstrapVersion" } + ], + "BusStatement1B4D0336C": [ + { + "type": "aws:cdk:logicalId", + "data": "BusStatement1B4D0336C", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] + } + ], + "BusStatement2B5FB314B": [ + { + "type": "aws:cdk:logicalId", + "data": "BusStatement2B5FB314B", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] + } ] }, "displayName": "Stack" diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/tree.json index d6fbe1678d677..d1f3f70812aa6 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.eventbus.js.snapshot/tree.json @@ -26,13 +26,13 @@ "version": "0.0.0" } }, - "Statement1": { - "id": "Statement1", - "path": "Stack/Bus/Statement1", + "cdk-Statement1": { + "id": "cdk-Statement1", + "path": "Stack/Bus/cdk-Statement1", "children": { "Resource": { "id": "Resource", - "path": "Stack/Bus/Statement1/Resource", + "path": "Stack/Bus/cdk-Statement1/Resource", "attributes": { "aws:cdk:cloudformation:type": "AWS::Events::EventBusPolicy", "aws:cdk:cloudformation:props": { @@ -66,9 +66,9 @@ "Arn" ] }, - "Sid": "Statement1" + "Sid": "cdk-Statement1" }, - "statementId": "Statement1" + "statementId": "cdk-Statement1" } }, "constructInfo": { @@ -82,13 +82,13 @@ "version": "0.0.0" } }, - "Statement2": { - "id": "Statement2", - "path": "Stack/Bus/Statement2", + "cdk-Statement2": { + "id": "cdk-Statement2", + "path": "Stack/Bus/cdk-Statement2", "children": { "Resource": { "id": "Resource", - "path": "Stack/Bus/Statement2/Resource", + "path": "Stack/Bus/cdk-Statement2/Resource", "attributes": { "aws:cdk:cloudformation:type": "AWS::Events::EventBusPolicy", "aws:cdk:cloudformation:props": { @@ -122,9 +122,9 @@ "Arn" ] }, - "Sid": "Statement2" + "Sid": "cdk-Statement2" }, - "statementId": "Statement2" + "statementId": "cdk-Statement2" } }, "constructInfo": { @@ -179,7 +179,7 @@ "path": "IntegTest-EventBusStack/DefaultTest/Default", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.2.70" + "version": "10.3.0" } }, "DeployAssert": { @@ -225,7 +225,7 @@ "path": "Tree", "constructInfo": { "fqn": "constructs.Construct", - "version": "10.2.70" + "version": "10.3.0" } } }, diff --git a/packages/aws-cdk-lib/aws-events/lib/event-bus.ts b/packages/aws-cdk-lib/aws-events/lib/event-bus.ts index a59aa8c1b4cca..78f533c5a5536 100644 --- a/packages/aws-cdk-lib/aws-events/lib/event-bus.ts +++ b/packages/aws-cdk-lib/aws-events/lib/event-bus.ts @@ -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 }; diff --git a/packages/aws-cdk-lib/aws-events/test/event-bus.test.ts b/packages/aws-cdk-lib/aws-events/test/event-bus.test.ts index 3d9b4de91994c..da4a96a3763e3 100644 --- a/packages/aws-cdk-lib/aws-events/test/event-bus.test.ts +++ b/packages/aws-cdk-lib/aws-events/test/event-bus.test.ts @@ -548,7 +548,7 @@ describe('event bus', () => { // THEN Template.fromStack(stack).hasResourceProperties('AWS::Events::EventBusPolicy', { - StatementId: '123', + StatementId: 'cdk-123', EventBusName: { Ref: 'BusEA82B648', }, @@ -569,7 +569,7 @@ describe('event bus', () => { ], }, }, - Sid: '123', + Sid: 'cdk-123', Resource: { 'Fn::GetAtt': [ 'BusEA82B648',