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.61k stars 3.91k forks source link

(lambda): Add property for log removal policy of Lambda function log groups #21804

Open tmokmss opened 2 years ago

tmokmss commented 2 years ago

Describe the feature

21113 introduced removalPolicy for LogRetention custom resource to allow us to delete log groups inside a stack. However, because we don't have a corresponding property in Lambda Function construct, it seems we still cannot remove a log group for a Lambda function automatically when we delete a stack. (Sorry if I'm missing something)

Use Case

Automatically remove log groups for lambda functions inside a stack when we delete it.

Proposed Solution

1st idea: Add a property e.g. logRetentionRemovalPolicy? here: https://github.com/aws/aws-cdk/blob/478b9967f4a5814d36472ecee5fc7157e3c2ad74/packages/%40aws-cdk/aws-lambda/lib/function.ts#L294

The property will be only valid when logRetention is set. There might be a better API for this but at least it should work and won't introduce any breaking change :(

2nd Idea (which might have better DX): Add a property like autoDeleteLog?: boolean. If users specify this, we internally create a logRetention with length of logRetention property or RetentionDays.INFINITY if not specified, and set logRetention.RemovalPolicy to destroy. By this we only have to set autoDeleteLog: true when we just want to delete a log groups on removal of the function.

Other Information

An aspect like below will not work either:

class SetLogGroupRemovalPolicy implements IAspect {
  public visit(node: CfnResource): void {
    if (node.cfnResourceType == 'Custom::LogRetention') {
      node.addOverride('Properties.RemovalPolicy', 'destroy');
    }
  }
}

because we still need to configure IAM policy to allow the lambda to delete the log group, which is set here.

https://github.com/aws/aws-cdk/blob/478b9967f4a5814d36472ecee5fc7157e3c2ad74/packages/%40aws-cdk/aws-logs/lib/log-retention.ts#L162-L183

It results in the bellow error:

10:22:50 PM | DELETE_FAILED        | Custom::LogRetention                       | HandlerLogRetention34184093
Received response status [FAILED] from custom resource. Message returned: User: arn:aws:sts::1:assumed-role/Stack-LogRetentionaae0aa3c
5b4d4f87b02d85b2-1XVR8ES6GII3X/Stack-LogRetentionaae0aa3c5b4d4f87b02d85b2-QJvljA9zM3Gp is not authorized to perform: logs:DeleteLogGroup on resou
rce: arn:aws:logs:ap-northeast-1:123456789012:log-group:/aws/lambda/Stack-Handler886CB40B-WSAN0fvgsbiN:log-stream: because no identity-based poli
cy allows the logs:DeleteLogGroup action (RequestId: 70428621-b81e-4e56-a8f3-8c2a788b9e13)

Acknowledgements

CDK version used

2.39.0

Environment details (OS name and version, etc.)

macOS

peterwoodworth commented 2 years ago

Thanks for submitting this, we accept contributions! Check out our contributing guide if you're interested - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂 I think we would want to be able to set this new property even if logRetention is not set

tmokmss commented 2 years ago

Hi thanks! I agree with that so I'll submit a PR implementing 2nd option.

rittneje commented 2 years ago

For our use case, we need a method akin to applyRemovalPolicy so we can set it in an aspect. Can this be modeled in a similar way instead of just a lambda function constructor parameter?

unitypark commented 1 year ago

Another alternative is changing policies on the role to deny log access , to stop them being created in the first place. e.g. for BucketDeployment, if we're deploying/destroying multiple dev branches i'd rather not have to worry about going in and tidying up after a destroy than having 99/100 logs saying the zip contents were extracted successfully.

const BucketDeploymentHandler = this.bucketDeployment.node.findChild('CustomResourceHandler') as SingletonFunction;
        BucketDeploymentHandler.addToRolePolicy(new PolicyStatement({
            actions:['logs:*'],
            resources:['*'],
            effect: Effect.DENY
       })
)

I want to share with you my working solution to delete LogGroup of CustomResources during stack deletion. and thanks to @greensmith for the idea.

Issue is related on CustomResource Constructor I guess. It creates CloudWatch log group for wrapper lambda. Even if I have defined a log group for the lambda explicitly (/aws/lambda/${lambdaFn.functionName}), this CustomResource Constructor overwrites my log group setting. (https://github.com/aws/aws-cdk/issues/8815)

Workaround is:

declare const onEvent: lambda.Function;

onEvent.addToRolePolicy(new iam.PolicyStatement({
  actions:['logs:CreateLogGroup'],
  resources:['*'],
  effect: iam.Effect.DENY
}));

new logs.LogGroup(this, 'log-group', {
  logGroupName: `/aws/lambda/${onEvent.functionName}`,
  removalPolicy: RemovalPolicy.DESTROY,
  retention: logs.RetentionDays.ONE_DAY,
})

In this way, Lambda Wrapper will not be able to create/overwrite the same log group, but you can keep writing logs in the log group that you have defined as long as your stack is alive. This log group will be but destroyed with your stack deletion together.

You can check out my repository to check whole code as an example.

https://github.com/deloittepark/aws-serverless-golang/tree/main/cognito-react-runtime-config