(aws-events-targets): Converve LogGroupResourcePolicies in CloudWatchLogGroup
- 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
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