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

[tneely]

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 as EventsLogGroupPolicy${cdk.Names.nodeUniqueId(this.logGroup.node)}
2. Check the stack for an existing child rather than the log group here

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

[tneely]

Happy to make this change if the owners of this repo feel it's a bug worth fixing. I've already duplicated and modified the code locally to get it to work for my code.

[cal5barton]

Running into this same issue. @tneely what does your modification look like?

[ghost]

I also ran into this issue. As a hackish workaround, I manually remove the LogGroupResourcePolicy resources that were automatically created. (I already have a manually created a policy beforehand i.e.: https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-use-resource-based.html#eb-cloudwatchlogs-permissions)

this.node.children
            .filter(child => child instanceof AwsCustomResource && child.constructor.name === 'LogGroupResourcePolicy')
            .forEach(value => {
                this.node.tryRemoveChild(value.node.id);
            });

[khushail]

@tneely , pls feel free to submit a PR. Team would be happy to review it.