feat(s3): add autoDeleteObjectsLogGroup to pass log group to custom resource lambda

[samson-keung]

Issue # (if applicable)

Closes #24815.

Reason for this change

To allow log group customization on the custom resource lambda created for the autoDeleteObjects feature.

Description of changes

There are 2 changes to implement the autoDeleteObjectsLogGroup prop:

1. CustomResourceProviderBase is updated to accept an optional logGroupName prop. If it is defined, it will be passed to the AWS::Lambda::Function resource definition (namely, setting the LoggingConfig prop).
2. s3.Bucket is updated to accept an optional autoDeleteObjectsLogGroup prop. The idea is to make the API experience similar to the logGroup prop on lambda.Function.

Description of how you validated changes

Unit tests have been added for both the s3.Bucket class and core.CustomResourceProvider class

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 5a201cb
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aaythapa]

thanks for making this change!

I see from this #24815 (comment) comment from the linked issue that someone made a similar prototype and they faced this error.

An unexpected behavior was that the provider lambda was re-creating the log group after I got it to successfully delete itself on stack deletion, but the new log group came back with infinite duration (argh!). This is because the default role for the lambda has permission to create a log group (which was occurring when it issued a final log stream on delete). Fortunately policyStatements was already wired up so I just explicitly denied CreateLogGroup for the Lambda role.

Wondering if you faced something similar (I see similarities between the prototype and your change so just checking)

[aaythapa]

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

[samson-keung]

It is ignored.

[aaythapa]

nit: I think its more valuable if you actually define a log group here. The log group from the integ test should do.

Suggested change

	declare const logGroup: ILogGroup;
	const logGroup = new logs.LogGroup(this, 'LogGroup', {
	logGroupName: 'AutoDeleteObjectsLambdaLogs',
	retention: logs.RetentionDays.THREE_DAYS,
	}),

[samson-keung]

My first revision was defining the log group explicitly. But I found the Lambda REAME that uses this style. But I agree defining the log group is more valuable to the reader. Will update

[aaythapa]

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

[samson-keung]

Good idea. Will update

[samson-keung]

Wondering if you faced something similar (I see similarities between the prototype and you're change so just checking)

Let me try reproducing the issue and get back to you. From your call out, I realized the integ test is not generating expected template, namely, the LoggingConfig on the lambda isn't set.

[samson-keung]

This PR has a problem with the integ test. Looking at the snapshot, it is apparent that the log group is created but it is not referenced anywhere in the template. The expected template should have the custom resource lambda referencing the log group like so:

  "CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F": {
   "Type": "AWS::Lambda::Function",
   "Properties": {
    // ...
    "LoggingConfig": {
     "LogGroup": {
      "Ref": "LogGroupF5B46931"
     }
    }
   }
  }

Upon further investigation, I have learned that the autoDeleteObjects prop creates a stack wide singleton provider lambda. Therefore, with this PR, the following happens:

const myLogGroupA = new LogGroup(...)
const bucket = new s3.Bucket(this, 'BucketA', {
  autoDeleteObjects: true, 
  autoDeleteObjectsLogGroup: myLogGroupA
});

// The above creates a singleton AutoDeleteObjectsProvider lambda and set the LoggingConfig on the lambda to use "myLogGroupA"

const myLogGroupB = new LogGroup(...)
const bucketThatWillBeRemoved = new s3.Bucket(this, 'BucketB', {
  autoDeleteObjects: true,
  autoDeleteObjectsLogGroup: myLogGroupB
});

// This second bucket DOES NOT create a new AutoDeleteObjectsProvider lambda. But rather it will retrieve and re-use the existing one. Hence, "myLogGroupB" is ignored

---

With the above, the approach in this PR creates a poor UX as user must remember to set the autoDeleteObjectsLogGroup on the very first bucket with autoDeleteObjects: true. And all subsequent buckets with autoDeleteObjectsLogGroup will be ignored quietly. Due to this, I am abandoning this approach and closing this PR. Please refer to the issue for further updates.