aws / aws/aws-cdk

(aws-events-targets): Converve LogGroupResourcePolicies in CloudWatchLogGroup

Open
#21,155 9 comments 6 reactions 0 assignees View on GitHub
@aws-cdk/aws-events-targets bug effort/medium p3
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the bug

The `CloudWatchLogGroup` target is rather overzealous when creating a LogGroupResourcePolicy, especially given that CloudWatch has a hard limit of 10 per region. The way it is configured today, it is impossible to create more than 10 CloudWatch Log Group targets, as each creates its own LogGroupResourcePolicy.

### Expected Behavior

I can target multiple rules at the same log group without fear of hitting a LogGroupResourcePolicy resource limit.

### Current Behavior

After assigning 10 targets, I reach my LogGroupResourcePolicy resource limit.

### Reproduction Steps

```
const testStack = new Stack(new App(), "TestStack");
const eventPattern: EventPattern = { id: ["someId"] };
const logGroup = new LogGroup(testStack, "LogGroup");

for (let i = 0; i < 11; i++) {
const rule = new Rule(testStack, `rule${i}`, { eventPattern });
rule.addTarget(new CloudWatchLogGroup(logGroup));
}
```

### Possible Solution

1. Define the `recourcePolicyId` [here](https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-events-targets/lib/log-group.ts#L95) as `EventsLogGroupPolicy${cdk.Names.nodeUniqueId(this.logGroup.node)}`
2. Check the stack for an existing child rather than the log group [here](https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-events-targets/lib/log-group.ts#L110)

This does two things. First, it makes the resourcePolicyId unique to the log group rather than the rule, since the `LogGroupResourcePolicy` only cares about the log group ARN, not the rule. Second, it tries to retrieve the policy from the stack node rather than the log group node. This second part is most likely a bug since it makes it impossible to find any existing policies since they're always created higher up in the construct hierarchy.

Another option would be to treat `LogGroupResourcePolicy` as a singleton and repeatedly append log group ARNs to it so we only ever create a single policy per stack. This would be a more invasive change and trickier to implement, but allow us to freely target any number of log groups rather than a max of 10.

Both options aren't really backwards compatible with the construct today, and I'm not sure how much tolerance CDK has between minor versions.

### Additional Information/Context

_No response_

### CDK CLI Version

2.30.0 (build 1529743)

### Framework Version

_No response_

### Node.js Version

14

### OS

AL2

### Language

Typescript

### Language Version

_No response_

### Other information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start by reading packages/@aws-cdk/aws-events-targets/lib/log-group.ts around lines 95 and 110, then run the provided reproduction with 11 rules targeting one log group. Compare the policy lookup and construct hierarchy against the two proposed approaches. Done means multiple rules can target the same log group without reaching CloudWatch's LogGroupResourcePolicy limit, with the compatibility implications addressed.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.