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(s3): add autoDeleteObjectsLogGroup to pass log group to custom resource lambda #30333

Closed
Show file tree
Hide file tree
Changes from 3 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
6 changes: 6 additions & 0 deletions packages/aws-cdk-lib/aws-s3/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -624,10 +624,16 @@ enable the`autoDeleteObjects` option.

When `autoDeleteObjects` is enabled, `s3:PutBucketPolicy` is added to the bucket policy. This is done to allow the custom resource this feature is built on to add a deny policy for `s3:PutObject` to the bucket policy when a delete stack event occurs. Adding this deny policy prevents new objects from being written to the bucket. Doing this prevents race conditions with external bucket writers during the deletion process.

Pass a custom log group via the `autoDeleteObjectsLogGroup` option, which will be used by the custom resource lambda.

```ts
const bucket = new s3.Bucket(this, 'MyTempFileBucket', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
autoDeleteObjects: true,
autoDeleteObjectsLogGroup: new LogGroup(stack, 'LogGroup', {
logGroupName: 'MyLogGroup',
retention: RetentionDays.TWO_YEARS
})
});
```

Expand Down
18 changes: 16 additions & 2 deletions packages/aws-cdk-lib/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { parseBucketArn, parseBucketName } from './util';
import * as events from '../../aws-events';
import * as iam from '../../aws-iam';
import * as kms from '../../aws-kms';
import * as logs from '../../aws-logs/index';
import {
CustomResource,
Duration,
Expand Down Expand Up @@ -1464,6 +1465,8 @@ export interface BucketProps {
* Whether all objects should be automatically deleted when the bucket is
* removed from the stack or when the stack is deleted.
*
* A custom resource will be created when set to `true`.
*
* Requires the `removalPolicy` to be set to `RemovalPolicy.DESTROY`.
*
* **Warning** if you have deployed a bucket with `autoDeleteObjects: true`,
Expand All @@ -1480,6 +1483,16 @@ export interface BucketProps {
*/
readonly autoDeleteObjects?: boolean;

/**
* The log group to use for the custom resource lambda backing the `autoDeleteObjects` feature
*
* When `autoDeleteObjects` is set to true, a customer resource backed by a Lambda function
* is created. The lambda will use the log group passed in to this prop if defined.
*
* @default the default log group created by Lambda
*/
readonly autoDeleteObjectsLogGroup?: logs.ILogGroup;

Comment on lines +1493 to +1495
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if autoDeleteObjectsLogGroup is set but autoDeleteObjects isn't set to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is ignored.

/**
* Whether this bucket should have versioning turned on or not.
*
Expand Down Expand Up @@ -2008,7 +2021,7 @@ export class Bucket extends BucketBase {
throw new Error('Cannot use \'autoDeleteObjects\' property on a bucket without setting removal policy to \'DESTROY\'.');
}

this.enableAutoDeleteObjects();
this.enableAutoDeleteObjects(props.autoDeleteObjectsLogGroup);
}

if (this.eventBridgeEnabled) {
Expand Down Expand Up @@ -2541,10 +2554,11 @@ export class Bucket extends BucketBase {
});
}

private enableAutoDeleteObjects() {
private enableAutoDeleteObjects(logGroup?: logs.ILogGroup) {
const provider = AutoDeleteObjectsProvider.getOrCreateProvider(this, AUTO_DELETE_OBJECTS_RESOURCE_TYPE, {
useCfnResponseWrapper: false,
description: `Lambda function for auto-deleting objects in ${this.bucketName} S3 bucket.`,
logGroupName: logGroup?.logGroupName,
});

// Use a bucket policy to allow the custom resource to delete
Expand Down
21 changes: 21 additions & 0 deletions packages/aws-cdk-lib/aws-s3/test/bucket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import { Annotations, Match, Template } from '../../assertions';
import * as iam from '../../aws-iam';
import * as kms from '../../aws-kms';
import { LogGroup } from '../../aws-logs';
import * as cdk from '../../core';
import * as s3 from '../lib';

Expand Down Expand Up @@ -3463,6 +3464,26 @@ describe('bucket', () => {
Template.fromStack(stack).resourceCountIs('AWS::Lambda::Function', 1);
});

test('with autoDeleteObjectsLogGroup', () => {
const stack = new cdk.Stack();

new s3.Bucket(stack, 'MyBucket', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
autoDeleteObjects: true,
autoDeleteObjectsLogGroup: new LogGroup(stack, 'LogGroup1', {
logGroupName: 'MyLogGroup',
}),
});

Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', {
LoggingConfig: {
LogGroup: {
'Ref': 'LogGroup106AAD846',
},
},
});
});
Comment on lines +3478 to +3485
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we also be checking if the log group resource was created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Will update


test('autoDeleteObjects throws if RemovalPolicy is not DESTROY', () => {
const stack = new cdk.Stack();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export abstract class CustomResourceProviderBase extends Construct {
Runtime: props.runtimeName,
Environment: this.renderEnvironmentVariables(props.environment),
Description: props.description ?? undefined,
LoggingConfig: props.logGroupName ? { LogGroup: props.logGroupName } : undefined,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,11 @@ export interface CustomResourceProviderOptions {
* @default - No description.
*/
readonly description?: string;

/**
* Name of the Cloudwatch log group to be used by the provider AWS Lambda function.
*
* @default - a default log group created by AWS Lambda.
*/
readonly logGroupName?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ describe('custom resource provider', () => {
}]);
});

test('memorySize, timeout and description', () => {
test('memorySize, timeout, description and logGroupName', () => {
// GIVEN
const stack = new Stack();

Expand All @@ -402,6 +402,7 @@ describe('custom resource provider', () => {
memorySize: Size.gibibytes(2),
timeout: Duration.minutes(5),
description: 'veni vidi vici',
logGroupName: 'some log group name',
});

// THEN
Expand All @@ -410,7 +411,7 @@ describe('custom resource provider', () => {
expect(lambda.Properties.MemorySize).toEqual(2048);
expect(lambda.Properties.Timeout).toEqual(300);
expect(lambda.Properties.Description).toEqual('veni vidi vici');

expect(lambda.Properties.LoggingConfig).toEqual({ LogGroup: 'some log group name' });
});

test('environment variables', () => {
Expand Down
Loading