aws / aws/aws-cdk

@aws-cdk-lib/aws-events: EventBus resource policy naming collision across stacks

Open
#29,627 3 comments 0 reactions 0 assignees View on GitHub
@aws-cdk/aws-events bug effort/small p2
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the bug

Deploying two instances of the same stack (within the same account) containing an event bus with the same resource policy fails. The issue is that the `StatementId` field of the synthesized [AWS::Events::EventBusPolicy resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-events-eventbuspolicy.html) must be unique across stacks. `StatementId` is internally populated from the `sid` of the provided `iam.PolicyStatement`:

https://github.com/aws/aws-cdk/blob/9a346647970ff006fa7695c2a43dd1109e13dbb5/packages/aws-cdk-lib/aws-events/lib/event-bus.ts#L336-L355

### Expected Behavior

The `sid` is documented in the following manner:

> The Sid (statement ID) is an optional identifier that you provide for the policy statement. You can assign a Sid value to each statement in a statement array. In services that let you specify an ID element, such as SQS and SNS, the Sid value is just a sub-ID of the policy document's ID. In IAM, the Sid value must be unique within a JSON policy.

I don't think it's reasonable to assume that end users should be aware that this value must be globally unique in this particular context. While the EventBridge Bus resource requires adding a `sid` for each statement of its resource policy, the service supports having two buses with the same policy containing the same `sid`.

To troubleshoot this issue, I had to read the underlying CDK source code to understand the underlying assumptions.

### Current Behavior

`addToResourcePolicy` requires providing a `sid`. This value is used as the `StatementId` of the AWS::Events::EventBusPolicy resource which must be unique across stacks.

### Reproduction Steps

Create the following stack:
```typescript
import * as cdk from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as events from 'aws-cdk-lib/aws-events';
import * as iam from 'aws-cdk-lib/aws-iam';

export class EventBusStack extends cdk.Stack {
constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);

const eventBus = new events.EventBus(this, 'EventBus');
eventBus.addToResourcePolicy(
new iam.PolicyStatement({
sid: 'AllowTrustedAccountToPutEvents',
actions: ['events:PutEvents'],
principals: [new iam.AccountPrincipal(process.env.TRUSTED_ACCOUNT_ID)],
resources: [eventBus.eventBusArn],
}
));
}
}
```

Deploy that stack twice:
```typescript
import * as cdk from 'aws-cdk-lib';
import { EventBusStack } from '../lib/event-bus-stack';

const app = new cdk.App();
new EventBusStack(app, 'EventBusStack-12345', {
env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }
});
new EventBusStack(app, 'EventBusStack-67890', {
env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }
});

```

You'll get the following error message:
```bash
❌ EventBusStack-12345 failed: Error: The stack named EventBusStack-12345 failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE: cdk-AllowTrustedAccountToPutEvents already exists in stack arn:aws:cloudformation:us-east-1::stack/EventBusStack-67890/
```

### Possible Solution

I think the CDK should follow the documented best practice:

> A better approach is to specify as few names as possible. If you omit resource names, the AWS CDK will generate them for you in a way that won't cause problems. ([Best practices for developing and deploying cloud infrastructure with the AWS CDK](https://docs.aws.amazon.com/cdk/v2/guide/best-practices.html#best-practices-constructs))

In the context of the addToResourcePolicy method in the EventBus class, it think the `StatementId` should be an autogenerated unique value for each policy statement. That autogenerated value could also be used as the underlying `sid` to respect the typing definition of the `iam.PolicyStatement`.

### Additional Information/Context

Sidenote: the raw [PutPermission](https://docs.aws.amazon.com/eventbridge/latest/APIReference/API_PutPermission.html#API_PutPermission_RequestParameters) API call doesn't require providing a `StatementId` when a raw JSON policy is provided via the `Policy` parameter while the [CloudFormation resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-events-eventbuspolicy.html) requires a `StatementId` but supports providing a `Statement` (which has the same badly copy pasted documentation as the upstream Policy). This is incredibly confusing.

### CDK CLI Version

2.133.0 (build dcc1e75)

### Framework Version

_No response_

### Node.js Version

v20.11.1

### OS

macOS 14.4.1

### Language

TypeScript

### Language Version

5.3.3

### Other information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start with the EventBus implementation in packages/aws-cdk-lib/aws-events/lib/event-bus.ts around lines 336-355, then synthesize the TypeScript reproduction with two stacks using the same policy sid. Done means the generated AWS::Events::EventBusPolicy resources use StatementId values that do not collide across stacks while preserving valid policy statements.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
cloud, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.