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

feat(lambda): allow to delete log group of a lambda function when it is removed #21820

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-lambda/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,8 @@ It is possible to obtain the function's log group as a `logs.ILogGroup` by calli
By default, CDK uses the AWS SDK retry options when creating a log group. The `logRetentionRetryOptions` property
allows you to customize the maximum number of retries and base backoff duration.

To automatically delete the associated log group for a Lambda function, you can use the `autoDeleteLogGroup` property.

*Note* that, if either `logRetention` is set or `logGroup` property is called, a [CloudFormation custom
resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cfn-customresource.html) is added
to the stack that pre-creates the log group as part of the stack deployment, if it already doesn't exist, and sets the
Expand Down
17 changes: 13 additions & 4 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as kms from '@aws-cdk/aws-kms';
import * as logs from '@aws-cdk/aws-logs';
import * as sns from '@aws-cdk/aws-sns';
import * as sqs from '@aws-cdk/aws-sqs';
import { Annotations, ArnFormat, CfnResource, Duration, FeatureFlags, Fn, IAspect, Lazy, Names, Size, Stack, Token } from '@aws-cdk/core';
import { Annotations, ArnFormat, CfnResource, Duration, FeatureFlags, Fn, IAspect, Lazy, Names, RemovalPolicy, Size, Stack, Token } from '@aws-cdk/core';
import { LAMBDA_RECOGNIZE_LAYER_VERSION } from '@aws-cdk/cx-api';
import { Construct, IConstruct } from 'constructs';
import { AliasOptions, Alias } from './alias';
Expand Down Expand Up @@ -276,6 +276,14 @@ export interface FunctionOptions extends EventInvokeConfigOptions {
*/
readonly events?: IEventSource[];

/**
* Whether the associated CloudWatch log group should be automatically deleted
* when the function is removed from the stack or when the stack is deleted.
*
* @default false
*/
readonly autoDeleteLogGroup?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inconsistent with our contract elsewhere when the user sets the Removal Policy. Please see other places where we have done this and follow that model. You use the enum below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @TheRealAmazonKendra thank you for the review!

This API is actually inspired by s3.Bucket.autoDeleteObjects. In this case the only concern for developers is whether the log group is automatically deleted or not. It is only a binary choice and there is no need to use RemovalPolicy enum to specify that. One of the main advantages of CDK constructs is abstraction, and here we abstract away RemovalPolicy to provide better developer experience.

Also internally we do not use CloudFormation RemovalPolicy here but using a feature implemented in the LogRetention custom resource below. I'm not sure if it is inconsistent to use the different contract from other CFn removal policies. Isn't it natural that different things will have different contracts?

RemovalPolicy: props.removalPolicy,

I'm open to discussion about this :) currently I don't see much benefits to use enum here though.


/**
* The number of days log events are kept in CloudWatch Logs. When updating
* this property, unsetting it doesn't remove the log retention policy. To
Expand Down Expand Up @@ -835,12 +843,13 @@ export class Function extends FunctionBase {
}

// Log retention
if (props.logRetention) {
if (props.logRetention || props.autoDeleteLogGroup) {
const logRetention = new logs.LogRetention(this, 'LogRetention', {
logGroupName: `/aws/lambda/${this.functionName}`,
retention: props.logRetention,
retention: props.logRetention ?? logs.RetentionDays.INFINITE,
role: props.logRetentionRole,
logRetentionRetryOptions: props.logRetentionRetryOptions as logs.LogRetentionRetryOptions,
logRetentionRetryOptions: props.logRetentionRetryOptions,
removalPolicy: props.autoDeleteLogGroup ? RemovalPolicy.DESTROY : undefined,
});
this._logGroup = logs.LogGroup.fromLogGroupArn(this, 'LogGroup', logRetention.logGroupArn);
}
Expand Down
60 changes: 60 additions & 0 deletions packages/@aws-cdk/aws-lambda/test/function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,66 @@ describe('function', () => {
});
});

test('specify autoDeleteLogGroup', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new lambda.Function(stack, 'MyLambda', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS,
autoDeleteLogGroup: true,
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::LogRetention', {
LogGroupName: {
'Fn::Join': [
'',
[
'/aws/lambda/',
{
Ref: 'MyLambdaCCE802FB',
},
],
],
},
RemovalPolicy: 'destroy',
});
});

test('specify both log retention and autoDeleteLogGroup', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new lambda.Function(stack, 'MyLambda', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS,
logRetention: logs.RetentionDays.ONE_MONTH,
autoDeleteLogGroup: true,
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::LogRetention', {
LogGroupName: {
'Fn::Join': [
'',
[
'/aws/lambda/',
{
Ref: 'MyLambdaCCE802FB',
},
],
],
},
RetentionInDays: 30,
RemovalPolicy: 'destroy',
});
});

test('imported lambda with imported security group and allowAllOutbound set to false', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-lambda/test/integ.log-retention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,11 @@ new lambda.Function(stack, 'OneYear', {
logRetention: logs.RetentionDays.ONE_YEAR,
});

new lambda.Function(stack, 'AutoDeleteLogGroup', {
code: new lambda.InlineCode('exports.handler = (event) => console.log(JSON.stringify(event));'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_14_X,
autoDeleteLogGroup: true,
});

app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
}
}
},
"565671f07704dec45a94362d4c9166d9e510a62338c5590cbe8fa62564a5043f": {
"c3d2a2489dfb36ca42d0b96528dd574c5d76756e84224a4288d23c9f42c280cb": {
"source": {
"path": "aws-cdk-lambda-log-retention.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "565671f07704dec45a94362d4c9166d9e510a62338c5590cbe8fa62564a5043f.json",
"objectKey": "c3d2a2489dfb36ca42d0b96528dd574c5d76756e84224a4288d23c9f42c280cb.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,34 @@
],
"Effect": "Allow",
"Resource": "*"
},
{
"Action": "logs:DeleteLogGroup",
"Effect": "Allow",
"Resource": {
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":logs:",
{
"Ref": "AWS::Region"
},
":",
{
"Ref": "AWS::AccountId"
},
":log-group:/aws/lambda/",
{
"Ref": "AutoDeleteLogGroupF74A7E0D"
},
":*"
]
]
}
}
],
"Version": "2012-10-17"
Expand Down Expand Up @@ -296,6 +324,79 @@
},
"RetentionInDays": 365
}
},
"AutoDeleteLogGroupServiceRoleF3400DE1": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "lambda.amazonaws.com"
}
}
],
"Version": "2012-10-17"
},
"ManagedPolicyArns": [
{
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
]
]
}
]
}
},
"AutoDeleteLogGroupF74A7E0D": {
"Type": "AWS::Lambda::Function",
"Properties": {
"Code": {
"ZipFile": "exports.handler = (event) => console.log(JSON.stringify(event));"
},
"Role": {
"Fn::GetAtt": [
"AutoDeleteLogGroupServiceRoleF3400DE1",
"Arn"
]
},
"Handler": "index.handler",
"Runtime": "nodejs14.x"
},
"DependsOn": [
"AutoDeleteLogGroupServiceRoleF3400DE1"
]
},
"AutoDeleteLogGroupLogRetentionAC501FFB": {
"Type": "Custom::LogRetention",
"Properties": {
"ServiceToken": {
"Fn::GetAtt": [
"LogRetentionaae0aa3c5b4d4f87b02d85b201efdd8aFD4BFC8A",
"Arn"
]
},
"LogGroupName": {
"Fn::Join": [
"",
[
"/aws/lambda/",
{
"Ref": "AutoDeleteLogGroupF74A7E0D"
}
]
]
},
"RemovalPolicy": "destroy"
}
}
},
"Parameters": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/565671f07704dec45a94362d4c9166d9e510a62338c5590cbe8fa62564a5043f.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/c3d2a2489dfb36ca42d0b96528dd574c5d76756e84224a4288d23c9f42c280cb.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -111,6 +111,24 @@
"data": "OneYearLogRetentionBD83A067"
}
],
"/aws-cdk-lambda-log-retention/AutoDeleteLogGroup/ServiceRole/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "AutoDeleteLogGroupServiceRoleF3400DE1"
}
],
"/aws-cdk-lambda-log-retention/AutoDeleteLogGroup/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "AutoDeleteLogGroupF74A7E0D"
}
],
"/aws-cdk-lambda-log-retention/AutoDeleteLogGroup/LogRetention/Resource": [
{
"type": "aws:cdk:logicalId",
"data": "AutoDeleteLogGroupLogRetentionAC501FFB"
}
],
"/aws-cdk-lambda-log-retention/BootstrapVersion": [
{
"type": "aws:cdk:logicalId",
Expand Down
Loading