feat(aws-s3-lambda): added optional loggingBucketProps to aws-s3-lamb…

…da (#411) * added optional loggingBucketProps to aws-s3-lambda * delete unnecessary integ test * fixed upstream in s3BucketWithLogging function * update README with loggingBucketProps * update README with optional text Co-authored-by: root <[email protected]>
awslabs · Sep 30, 2021 · 1552e4e · 1552e4e
1 parent 5d41482
commit 1552e4e
Show file tree

Hide file tree

Showing 6 changed files with 133 additions and 27 deletions.
diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/README.md b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/README.md
@@ -52,10 +52,11 @@ _Parameters_
 | **Name**     | **Type**        | **Description** |
 |:-------------|:----------------|-----------------|
 |existingLambdaObj?|[`lambda.Function`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda.Function.html)|Existing instance of Lambda Function object, providing both this and `lambdaFunctionProps` will cause an error.|
-|lambdaFunctionProps?|[`lambda.FunctionProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda.FunctionProps.html)|User provided props to override the default props for the Lambda function.|
+|lambdaFunctionProps?|[`lambda.FunctionProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda.FunctionProps.html)|Optional user provided props to override the default props for the Lambda function.|
 |existingBucketObj?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html)|Existing instance of S3 Bucket object. If this is provided, then also providing bucketProps is an error. |
-|bucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|User provided props to override the default props for the S3 Bucket.|
+|bucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|Optional user provided props to override the default props for the S3 Bucket.|
 |s3EventSourceProps?|[`S3EventSourceProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda-event-sources.S3EventSourceProps.html)|Optional user provided props to override the default props for S3EventSourceProps|
+|loggingBucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|Optional user provided props to override the default props for the S3 Logging Bucket.|
 
 ## Pattern Properties
 

diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/lib/index.ts b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/lib/index.ts
@@ -51,7 +51,13 @@ export interface S3ToLambdaProps {
    *
    * @default - Default props are used
    */
-  readonly s3EventSourceProps?: S3EventSourceProps
+  readonly s3EventSourceProps?: S3EventSourceProps,
+  /**
+   * User provided props to override the default props for the S3 Logging Bucket.
+   *
+   * @default - Default props are used
+   */
+   readonly loggingBucketProps?: s3.BucketProps
 }
 
 export class S3ToLambda extends Construct {
@@ -84,7 +90,8 @@ export class S3ToLambda extends Construct {
 
     if (!props.existingBucketObj) {
       [this.s3Bucket, this.s3LoggingBucket] = defaults.buildS3Bucket(this, {
-        bucketProps: props.bucketProps
+        bucketProps: props.bucketProps,
+        loggingBucketProps: props.loggingBucketProps
       });
       bucket = this.s3Bucket;
     } else {

diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/s3-lambda.test.ts b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/s3-lambda.test.ts
@@ -15,6 +15,7 @@ import { S3ToLambda, S3ToLambdaProps } from "../lib";
 import * as lambda from '@aws-cdk/aws-lambda';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as cdk from "@aws-cdk/core";
+import * as defaults from '../../core/index';
 import '@aws-cdk/assert/jest';
 
 function deployNewFunc(stack: cdk.Stack) {
@@ -62,4 +63,37 @@ test("Test bad call with existingBucket and bucketProps", () => {
   };
   // Assertion
   expect(app).toThrowError();
+});
+
+// --------------------------------------------------------------
+// s3 bucket with bucket, loggingBucket, and auto delete objects
+// --------------------------------------------------------------
+test('s3 bucket with bucket, loggingBucket, and auto delete objects', () => {
+  const stack = new cdk.Stack();
+
+  defaults.buildS3Bucket(stack, {
+    bucketProps: {
+      removalPolicy: cdk.RemovalPolicy.DESTROY,
+    },
+    loggingBucketProps: {
+      removalPolicy: cdk.RemovalPolicy.DESTROY,
+      autoDeleteObjects: true
+    }
+  });
+
+  expect(stack).toHaveResource("AWS::S3::Bucket", {
+    AccessControl: "LogDeliveryWrite"
+  });
+
+  expect(stack).toHaveResource("Custom::S3AutoDeleteObjects", {
+    ServiceToken: {
+      "Fn::GetAtt": [
+        "CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F",
+        "Arn"
+      ]
+    },
+    BucketName: {
+      Ref: "S3LoggingBucket800A2B27"
+    }
+  });
 });
diff --git a/source/patterns/@aws-solutions-constructs/core/lib/cloudfront-distribution-helper.ts b/source/patterns/@aws-solutions-constructs/core/lib/cloudfront-distribution-helper.ts
@@ -23,6 +23,7 @@ import {
 } from './cloudfront-distribution-defaults';
 import { overrideProps, addCfnSuppressRules } from './utils';
 import { createLoggingBucket } from './s3-bucket-helper';
+import { DefaultS3Props } from './s3-bucket-defaults';
 // Note: To ensure CDKv2 compatibility, keep the import statement for Construct separate
 import { Construct } from '@aws-cdk/core';
 
@@ -187,7 +188,7 @@ function getLoggingBucket(cloudFrontDistributionProps: cloudfront.DistributionPr
   const userSuppliedLogBucket = cloudFrontDistributionProps?.logBucket;
   return isLoggingDisabled
     ? undefined
-    : userSuppliedLogBucket ?? createLoggingBucket(scope, 'CloudfrontLoggingBucket');
+    : userSuppliedLogBucket ?? createLoggingBucket(scope, 'CloudfrontLoggingBucket', DefaultS3Props());
 }
 
 function getCloudfrontFunction(httpSecurityHeaders: boolean, scope: Construct) {

diff --git a/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-helper.ts b/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-helper.ts
@@ -20,7 +20,6 @@ import { overrideProps, addCfnSuppressRules } from './utils';
 import { PolicyStatement, Effect, AnyPrincipal } from '@aws-cdk/aws-iam';
 import { StorageClass } from '@aws-cdk/aws-s3';
 import { Duration } from '@aws-cdk/core';
-import { RemovalPolicy } from '@aws-cdk/core';
 // Note: To ensure CDKv2 compatibility, keep the import statement for Construct separate
 import { Construct } from '@aws-cdk/core';
 
@@ -31,14 +30,16 @@ export interface BuildS3BucketProps {
    * @default - Default props are used
    */
   readonly bucketProps?: s3.BucketProps
+  /**
+   * User provided props to override the default props for the S3 Logging Bucket.
+   *
+   * @default - Default props are used
+   */
+  readonly loggingBucketProps?: s3.BucketProps
 }
 
 export function buildS3Bucket(scope: Construct, props: BuildS3BucketProps, bucketId?: string): [s3.Bucket, s3.Bucket?] {
-  if (props.bucketProps) {
-    return s3BucketWithLogging(scope, props.bucketProps, bucketId);
-  } else {
-    return s3BucketWithLogging(scope, DefaultS3Props(), bucketId);
-  }
+  return s3BucketWithLogging(scope, props.bucketProps, bucketId, props.loggingBucketProps);
 }
 
 export function applySecureBucketPolicy(s3Bucket: s3.Bucket): void {
@@ -56,23 +57,18 @@ export function applySecureBucketPolicy(s3Bucket: s3.Bucket): void {
       principals: [new AnyPrincipal()],
       effect: Effect.DENY,
       conditions:
-            {
-              Bool: {
-                'aws:SecureTransport': 'false'
-              }
-            }
+      {
+        Bool: {
+          'aws:SecureTransport': 'false'
+        }
+      }
     })
   );
 }
 
-export function createLoggingBucket(scope: Construct, bucketId: string, removalPolicy?: RemovalPolicy): s3.Bucket {
-  let loggingBucketProps;
-
-  if (removalPolicy) {
-    loggingBucketProps = overrideProps(DefaultS3Props(), { removalPolicy });
-  } else {
-    loggingBucketProps = DefaultS3Props();
-  }
+export function createLoggingBucket(scope: Construct,
+  bucketId: string,
+  loggingBucketProps: s3.BucketProps): s3.Bucket {
 
   // Create the Logging Bucket
   const loggingBucket: s3.Bucket = new s3.Bucket(scope, bucketId, loggingBucketProps);
@@ -104,7 +100,10 @@ export function createLoggingBucket(scope: Construct, bucketId: string, removalP
   return loggingBucket;
 }
 
-function s3BucketWithLogging(scope: Construct, s3BucketProps?: s3.BucketProps, bucketId?: string): [s3.Bucket, s3.Bucket?] {
+function s3BucketWithLogging(scope: Construct,
+  s3BucketProps?: s3.BucketProps,
+  bucketId?: string,
+  userLoggingBucketProps?: s3.BucketProps): [s3.Bucket, s3.Bucket?] {
 
   /** Default Life Cycle policy to transition older versions to Glacier after 90 days */
   const lifecycleRules: s3.LifecycleRule[] = [{
@@ -129,7 +128,17 @@ function s3BucketWithLogging(scope: Construct, s3BucketProps?: s3.BucketProps, b
     }
   } else {
     // Create the Logging Bucket
-    loggingBucket = createLoggingBucket(scope, _loggingBucketId, s3BucketProps?.removalPolicy);
+    let loggingBucketProps;
+
+    if (userLoggingBucketProps) { // User provided logging bucket props
+      loggingBucketProps = overrideProps(DefaultS3Props(), userLoggingBucketProps);
+    } else if (s3BucketProps?.removalPolicy) { // Deletes logging bucket only if it is empty
+      loggingBucketProps = overrideProps(DefaultS3Props(), { removalPolicy: s3BucketProps.removalPolicy });
+    } else { // Default S3 bucket props
+      loggingBucketProps = DefaultS3Props();
+    }
+
+    loggingBucket = createLoggingBucket(scope, _loggingBucketId, loggingBucketProps);
 
     // Attach the Default Life Cycle policy ONLY IF the versioning is ENABLED
     if (s3BucketProps?.versioned === undefined || s3BucketProps.versioned) {

diff --git a/source/patterns/@aws-solutions-constructs/core/test/s3-bucket-helper.test.ts b/source/patterns/@aws-solutions-constructs/core/test/s3-bucket-helper.test.ts
@@ -12,7 +12,7 @@
  */
 
 import { expect as expectCDK, haveResource, ResourcePart } from '@aws-cdk/assert';
-import { Duration, Stack } from '@aws-cdk/core';
+import { Duration, Stack, RemovalPolicy } from '@aws-cdk/core';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as s3n from '@aws-cdk/aws-s3-notifications';
 import * as sqs from '@aws-cdk/aws-sqs';
@@ -58,6 +58,33 @@ test('s3 bucket with bucketProps', () => {
   }));
 });
 
+test('s3 bucket with default props', () => {
+  const stack = new Stack();
+
+  defaults.buildS3Bucket(stack, {});
+
+  expectCDK(stack).to(haveResource("AWS::S3::Bucket", {
+    BucketEncryption: {
+      ServerSideEncryptionConfiguration: [
+        {
+          ServerSideEncryptionByDefault: {
+            SSEAlgorithm: "AES256"
+          }
+        }
+      ]
+    },
+    PublicAccessBlockConfiguration: {
+      BlockPublicAcls: true,
+      BlockPublicPolicy: true,
+      IgnorePublicAcls: true,
+      RestrictPublicBuckets: true
+    },
+    VersioningConfiguration: {
+      Status: "Enabled"
+    }
+  }));
+});
+
 test('s3 bucket with life cycle policy', () => {
   const stack = new Stack();
 
@@ -240,6 +267,33 @@ test('s3 bucket versioning turned off', () => {
   }));
 });
 
+test('s3 bucket with LoggingBucket and auto delete objects', () => {
+  const stack = new Stack();
+
+  defaults.buildS3Bucket(stack, {
+    loggingBucketProps: {
+      removalPolicy: RemovalPolicy.DESTROY,
+      autoDeleteObjects: true
+    }
+  });
+
+  expectCDK(stack).to(haveResource("AWS::S3::Bucket", {
+    AccessControl: "LogDeliveryWrite"
+  }));
+
+  expectCDK(stack).to(haveResource("Custom::S3AutoDeleteObjects", {
+    ServiceToken: {
+      "Fn::GetAtt": [
+        "CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F",
+        "Arn"
+      ]
+    },
+    BucketName: {
+      Ref: "S3LoggingBucket800A2B27"
+    }
+  }));
+});
+
 test('s3 bucket versioning turned on', () => {
   const stack = new Stack();