Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 2 tasks
tmokmss opened this issue Aug 29, 2022 · 4 comments
Open
1 of 2 tasks
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@tmokmss
Copy link
Contributor

tmokmss commented Aug 29, 2022

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:

readonly logRetentionRole?: iam.IRole;

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.

if (props.removalPolicy === cdk.RemovalPolicy.DESTROY) {
role.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['logs:DeleteLogGroup'],
//Only allow deleting the specific log group.
resources: [cdk.Stack.of(this).formatArn({
service: 'logs',
resource: 'log-group',
resourceName: `${props.logGroupName}:*`,
arnFormat: ArnFormat.COLON_RESOURCE_NAME,
})],
}));
role.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['logs:DeleteLogStream'],
//Only allow deleting the specific log group.
resources: [cdk.Stack.of(this).formatArn({
service: 'logs',
resource: `log-group:${props.logGroupName}:log-stream`,
resourceName: '*',
arnFormat: ArnFormat.COLON_RESOURCE_NAME,
})],
}));
}

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

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.39.0

Environment details (OS name and version, etc.)

macOS

@tmokmss tmokmss added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 29, 2022
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Aug 29, 2022
@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 30, 2022
@peterwoodworth
Copy link
Contributor

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
Copy link
Contributor Author

tmokmss commented Aug 30, 2022

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

@rittneje
Copy link

rittneje commented Sep 1, 2022

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
Copy link

unitypark commented Nov 9, 2022

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. (#8815)

Workaround is:

  • define log group explicitly for your lambda function with removalPolicy and retention.
  • attach role policy to your lambda that log:CreateLogGroup is denied.
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

Jimlinz added a commit to linz/geostore that referenced this issue Nov 23, 2022
There is no mechanism in place to clean up CloudWatch Log Groups. While the logs itself can be set to expire, the Log Groups are kept forever. There isn't an elegant way to delete log groups within CDK on stack teardown currently (this may change in the future - see aws/aws-cdk#21804). This is a temporary workaround until a better solution is found.

Note: A --no-paginate option is added to limit the number of queries (~50). This prevents rate limiting as well as stops a potentially endless number of results being returned by AWS (which would result in a very long-running workflow). This means only a maximum number of 50 log groups are removed each workflow run.
kodiakhq bot added a commit to linz/geostore that referenced this issue Nov 23, 2022
There is no mechanism in place to clean up CloudWatch Log Groups. While the logs itself can be set to expire, the Log Groups are kept forever. There isn't an elegant way to delete log groups within CDK on stack teardown currently (this may change in the future - see aws/aws-cdk#21804). This is a temporary workaround until a better solution is found.

Note: A --no-paginate option is added to limit the number of queries (~50). This prevents rate limiting as well as stops a potentially endless number of results being returned by AWS (which would result in a very long-running workflow). This means only a maximum number of 50 log groups are removed each workflow run.

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
@mrgrain mrgrain self-assigned this Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
7 participants