aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.37k stars 3.78k forks source link

LambdaAction fails if same Lambda added to multiple alarms #30264

Open dfserrano opened 1 month ago

dfserrano commented 1 month ago

Describe the bug

Adding the same lambda as the action for multiple alarms causes an error because of logical id conflicts. The error says There is already a Construct with name 'AlarmPermission' in Function

Expected Behavior

One Lambda function can be configured as action for multiple alarms without the need to use the context @aws-cdk/aws-cloudwatch-actions:changeLambdaPermissionLogicalIdForLambdaAction

Current Behavior

There is already a Construct with name 'AlarmPermission' in Function

Reproduction Steps

alarm1.addAlarmAction(new actions.LambdaAction(lambda));
alarm2.addAlarmAction(new actions.LambdaAction(lambda));

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.140.0

Framework Version

No response

Node.js Version

18

OS

Mac

Language

TypeScript

Language Version

No response

Other information

No response

khushail commented 1 month ago

@dfserrano , I see this issue was reported here and fixed by this PR. As per the expectation and the intention of keeping the flag enabled is described here - Ref

With the feature flag disabled it will throw if the same lambda is added to multiple alarms because it will try to add multiple permissions with the same 'AlarmPermission' logical ID. If the same lambda is added to different actions for one alarm it will not throw.

Please feel free to reach out if this is not helpful or anything is misunderstood. Thanks.

dfserrano commented 1 month ago

Does it mean that the intention of Lambda actions is that users create one Lambda function per Alarm? So, if I want to act on my alarms, and I have 100 alarms, I will need 100 Lambda functions?

jfener commented 1 month ago

Ran into this as well.

khushail commented 1 month ago

@dfserrano ,the Alarm requires a Lambda ARN to invoke. If there are 100 Alarms, same Lambda ARN could be used -

https://github.com/aws/aws-cdk/blob/8c39e8161970705c76f93ec99934f9b3b76da294/packages/aws-cdk-lib/aws-cloudwatch-actions/lib/lambda.ts#L42

dfserrano commented 1 month ago

The problem is that without that flag, then the permissionId in https://github.com/aws/aws-cdk/blob/8c39e8161970705c76f93ec99934f9b3b76da294/packages/aws-cdk-lib/aws-cloudwatch-actions/lib/lambda.ts#L33

will have a collision when trying to add the second permission.

By default, the flag that enables the desired functionality is not set. My question is whether having that flag off by default is the intended behavior or if it should be on by default.

Additionally, using the same example I used before, attempting to add a lambda action to 100 alarms results in 100 new resources being created, one for each alarm-lambda permission. This may reach the CloudFormation stack resource limit per stack (if I end up with more than 500). As a workaround, I created a composite alarm containing all the child alarms I wanted to trigger the lambda. Then I updated my lambda function to extract extra information from those child alarms. This avoids hitting the stack resource limit but requires additional logic in the lambda.

khushail commented 1 month ago

@dfserrano, by default, the flag is not set. As mentioned in this readme , here is a snippet for the flag value mentioned -

Screenshot 2024-05-31 at 2 40 49 PM