feat(s3): autoDeleteObjects log group allows retention period and removal policy definitions

[kishiel]

Issue #24815

Closes #24815

Reason for this change

S3 bucket autoDeleteObjects leaves behind a log group for each bucket that uses the feature. This results in a lot of cruft, especially in test accounts, which should be configurable by the bucket owner. The account limit for log groups is 10,000 and I've got test accounts that have hit this limit several times.

Description of changes

• Creates a log group rather than relying on the underlying custom-resource to create it automatically (a side effect of using CfnResource for AWS::Lambda::Function)
• Sets a default retention period of 90 days on the log group (I picked a number)
• Sets a default removal policy of delete on the log group (I don't think anyone wants these after they delete a bucket)
• Denies the custom-resource Lambda role permission to create a log group (prevents log group recreation on delete)
• Adds log group name as an optional to the interface of the custom-resource. This is plumbed into the loggingConfig and results in an undefined entry if not provided.

Description of how you validated changes

Unit tests in addition to some simple functional tests.

When making a bucket with autoDeleteObjects enabled I wanted to confirm that the log group for the lambda was, in fact, gone after I deleted the stack. This is how I found that I needed to modify the permission of the Lambda role to deny log group creation.

I also confirmed that the custom-resources which do not provide a log group name still produce a log group and logs within.

Also, over 100 snapshot tests (RIP me).

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

[kishiel]

There are about 10 snapshot tests which are failing that I'm unable to resolve on my own and I could use some help in running them. I believe that a few of them are because I'm using an internal AWS account so hopefully they just need to be run from someone's not-quite-so-special account.

[kishiel]

Build is failing for something that is surprising to me. I didn't make any CLI changes. Edit: This isn't a CLI change there appears to be something breaking between s3-assets Asset and s3 Bucket. I ran this test without my changes and it passed. It's not clear where the breakage is introduced. It looks like s3-assets is properly testing the path for an Asset import. Digging.

@aws-cdk/aws-route53resolver-alpha:test

@aws-cdk/aws-route53resolver-alpha: FAIL test/firewall-domain-list.test.ts (12.391 s)
@aws-cdk/aws-route53resolver-alpha:   â—� domain list from asset
@aws-cdk/aws-route53resolver-alpha:     TypeError: Cannot read properties of undefined (reading 'fromBucketAttributes')
@aws-cdk/aws-route53resolver-alpha:       174 |     const kmsKey = location.kmsKeyArn ? kms.Key.fromKeyArn(this, 'Key', location.kmsKeyArn) : undefined;
@aws-cdk/aws-route53resolver-alpha:       175 |
@aws-cdk/aws-route53resolver-alpha:     > 176 |     this.bucket = s3.Bucket.fromBucketAttributes(this, 'AssetBucket', {
@aws-cdk/aws-route53resolver-alpha:           |                             ^
@aws-cdk/aws-route53resolver-alpha:       177 |       bucketName: this.s3BucketName,
@aws-cdk/aws-route53resolver-alpha:       178 |       encryptionKey: kmsKey,
@aws-cdk/aws-route53resolver-alpha:       179 |     });
@aws-cdk/aws-route53resolver-alpha:       at new fromBucketAttributes (../../aws-cdk-lib/aws-s3-assets/lib/asset.ts:176:29)
@aws-cdk/aws-route53resolver-alpha:       at Object.bind (lib/firewall-domain-list.ts:107:23)
@aws-cdk/aws-route53resolver-alpha:       at new bind (lib/firewall-domain-list.ts:217:41)
@aws-cdk/aws-route53resolver-alpha:       at Object.<anonymous> (test/firewall-domain-list.test.ts:58:3)

[kishiel]

Something appears to be misconfigured with the @aws-cdk/aws-route53resolver-alpha directory. I can reproduce the problem with a single test in @aws-cdk/aws-route53resolver-alpha/test:

  const stack = new Stack(app);
  const filePath = path.join(__dirname, 'domains.txt');
  new Asset(stack, 'MyAsset', { path: filePath });

Copying this same test over to @aws-cdk/aws-redshift-alpha/test does not cause the error to manifest.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: d659009
• Result: FAILED
• Build Logs (available for 30 days)

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

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[mwebber]

@kishiel are you able to fix the merge conflicts, so your PR can proceed?

[kishiel]

I haven't had a chance to look at this in about two weeks.

I've got some underlying issue that I can't explain that's manifesting in unit tests where S3 buckets are produced as a byproduct of using another construct. Order of test is affecting the outcome (which is surprising) and leads me to believe that the tests aren't cleaning up something quite right. I've spent a bunch of time trying to figure it out but no dice. I think I'm going to have to abandon this attempt and try a different implementation, but at least now I know that I need to run a bunch of additional tests outside of aws-cdk-lib directly.

[aws-cdk-automation]

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.