From 674b50b59073bd1db97a25d458e52b5d418a6078 Mon Sep 17 00:00:00 2001 From: mickychetta <45010053+mickychetta@users.noreply.github.com> Date: Thu, 11 Nov 2021 06:50:34 -0800 Subject: [PATCH] added logS3AccessLogs and S3BucketInterface (#498) --- .../aws-s3-lambda/README.md | 2 + .../aws-s3-lambda/lib/index.ts | 17 ++- .../test/integ.no-arguments.expected.json | 101 ++---------------- .../aws-s3-lambda/test/integ.no-arguments.ts | 14 ++- .../aws-s3-lambda/test/s3-lambda.test.ts | 39 +++++++ 5 files changed, 76 insertions(+), 97 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 index 28880d7f8..c863edcfd 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/README.md +++ b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/README.md @@ -57,6 +57,7 @@ _Parameters_ |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.| +|logS3AccessLogs?| boolean|Whether to turn on Access Logging for the S3 bucket. Creates an S3 bucket with associated storage costs for the logs. Enabling Access Logging is a best practice. default - true| ## Pattern Properties @@ -65,6 +66,7 @@ _Parameters_ |lambdaFunction|[`lambda.Function`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda.Function.html)|Returns an instance of the lambda.Function created by the construct| |s3Bucket?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html)|Returns an instance of the s3.Bucket created by the construct| |s3LoggingBucket?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html)|Returns an instance of s3.Bucket created by the construct as the logging bucket for the primary bucket.| +|s3BucketInterface|[`s3.IBucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.IBucket.html)|Returns an instance of s3.IBucket created by the construct| ## Default settings 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 index 52abd237a..f5cc866ee 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/lib/index.ts +++ b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/lib/index.ts @@ -58,12 +58,20 @@ export interface S3ToLambdaProps { * @default - Default props are used */ readonly loggingBucketProps?: s3.BucketProps + /** + * Whether to turn on Access Logs for the S3 bucket with the associated storage costs. + * Enabling Access Logging is a best practice. + * + * @default - true + */ + readonly logS3AccessLogs?: boolean; } export class S3ToLambda extends Construct { public readonly lambdaFunction: lambda.Function; public readonly s3Bucket?: s3.Bucket; public readonly s3LoggingBucket?: s3.Bucket; + public readonly s3BucketInterface: s3.IBucket; /** * @summary Constructs a new instance of the S3ToLambda class. @@ -79,10 +87,6 @@ export class S3ToLambda extends Construct { let bucket: s3.Bucket; - if (props.existingBucketObj && props.bucketProps) { - throw new Error('Cannot specify both bucket properties and an existing bucket'); - } - this.lambdaFunction = defaults.buildLambdaFunction(this, { existingLambdaObj: props.existingLambdaObj, lambdaFunctionProps: props.lambdaFunctionProps @@ -91,13 +95,16 @@ export class S3ToLambda extends Construct { if (!props.existingBucketObj) { [this.s3Bucket, this.s3LoggingBucket] = defaults.buildS3Bucket(this, { bucketProps: props.bucketProps, - loggingBucketProps: props.loggingBucketProps + loggingBucketProps: props.loggingBucketProps, + logS3AccessLogs: props.logS3AccessLogs }); bucket = this.s3Bucket; } else { bucket = props.existingBucketObj; } + this.s3BucketInterface = bucket; + // Create S3 trigger to invoke lambda function this.lambdaFunction.addEventSource(new S3EventSource(bucket, defaults.S3EventSourceProps(props.s3EventSourceProps))); diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.expected.json b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.expected.json index a46304ae9..162cd4550 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.expected.json @@ -138,90 +138,6 @@ } } }, - "tests3lambdaS3LoggingBucket0C3BBFDC": { - "Type": "AWS::S3::Bucket", - "Properties": { - "AccessControl": "LogDeliveryWrite", - "BucketEncryption": { - "ServerSideEncryptionConfiguration": [ - { - "ServerSideEncryptionByDefault": { - "SSEAlgorithm": "AES256" - } - } - ] - }, - "PublicAccessBlockConfiguration": { - "BlockPublicAcls": true, - "BlockPublicPolicy": true, - "IgnorePublicAcls": true, - "RestrictPublicBuckets": true - }, - "VersioningConfiguration": { - "Status": "Enabled" - } - }, - "UpdateReplacePolicy": "Delete", - "DeletionPolicy": "Delete", - "Metadata": { - "cfn_nag": { - "rules_to_suppress": [ - { - "id": "W35", - "reason": "This S3 bucket is used as the access logging bucket for another bucket" - } - ] - } - } - }, - "tests3lambdaS3LoggingBucketPolicyC349F74C": { - "Type": "AWS::S3::BucketPolicy", - "Properties": { - "Bucket": { - "Ref": "tests3lambdaS3LoggingBucket0C3BBFDC" - }, - "PolicyDocument": { - "Statement": [ - { - "Action": "*", - "Condition": { - "Bool": { - "aws:SecureTransport": "false" - } - }, - "Effect": "Deny", - "Principal": { - "AWS": "*" - }, - "Resource": [ - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "tests3lambdaS3LoggingBucket0C3BBFDC", - "Arn" - ] - }, - "/*" - ] - ] - }, - { - "Fn::GetAtt": [ - "tests3lambdaS3LoggingBucket0C3BBFDC", - "Arn" - ] - } - ], - "Sid": "HttpsOnly" - } - ], - "Version": "2012-10-17" - } - } - }, "tests3lambdaS3BucketNotifications1943E9B3": { "Type": "Custom::S3BucketNotifications", "Properties": { @@ -280,11 +196,6 @@ } ] }, - "LoggingConfiguration": { - "DestinationBucketName": { - "Ref": "tests3lambdaS3LoggingBucket0C3BBFDC" - } - }, "PublicAccessBlockConfiguration": { "BlockPublicAcls": true, "BlockPublicPolicy": true, @@ -296,7 +207,17 @@ } }, "UpdateReplacePolicy": "Delete", - "DeletionPolicy": "Delete" + "DeletionPolicy": "Delete", + "Metadata": { + "cfn_nag": { + "rules_to_suppress": [ + { + "id": "W35", + "reason": "This S3 bucket is created for unit/ integration testing purposes only." + } + ] + } + } }, "tests3lambdaS3BucketPolicyE0402ABD": { "Type": "AWS::S3::BucketPolicy", diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.ts b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.ts index ed01560a7..b2447a1b7 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.ts +++ b/source/patterns/@aws-solutions-constructs/aws-s3-lambda/test/integ.no-arguments.ts @@ -16,6 +16,8 @@ import { App, Stack, RemovalPolicy } from "@aws-cdk/core"; import { S3ToLambda, S3ToLambdaProps } from "../lib"; import * as lambda from '@aws-cdk/aws-lambda'; import { generateIntegStackName } from '@aws-solutions-constructs/core'; +import * as s3 from "@aws-cdk/aws-s3"; +import * as defaults from '@aws-solutions-constructs/core'; const app = new App(); @@ -30,8 +32,16 @@ const props: S3ToLambdaProps = { }, bucketProps: { removalPolicy: RemovalPolicy.DESTROY, - } + }, + logS3AccessLogs: false }; -new S3ToLambda(stack, 'test-s3-lambda', props); +const construct = new S3ToLambda(stack, 'test-s3-lambda', props); +const s3Bucket = construct.s3Bucket as s3.Bucket; + +defaults.addCfnSuppressRules(s3Bucket, [ + { id: 'W35', + reason: 'This S3 bucket is created for unit/ integration testing purposes only.' }, +]); + app.synth(); \ No newline at end of file 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 index 3efb265c8..360461ccf 100644 --- 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 @@ -16,6 +16,7 @@ import * as lambda from '@aws-cdk/aws-lambda'; import * as s3 from '@aws-cdk/aws-s3'; import * as cdk from "@aws-cdk/core"; import '@aws-cdk/assert/jest'; +import { CreateScrapBucket } from '@aws-solutions-constructs/core'; function deployNewFunc(stack: cdk.Stack) { const props: S3ToLambdaProps = { @@ -100,4 +101,42 @@ test('s3 bucket with bucket, loggingBucket, and auto delete objects', () => { Ref: "s3lambdaS3LoggingBucketAC6FF14E" } }); +}); + +// -------------------------------------------------------------- +// s3 bucket with one content bucket and no logging bucket +// -------------------------------------------------------------- +test('s3 bucket with one content bucket and no logging bucket', () => { + const stack = new cdk.Stack(); + + new S3ToLambda(stack, 's3-lambda', { + lambdaFunctionProps: { + code: lambda.Code.fromAsset(`${__dirname}/lambda`), + runtime: lambda.Runtime.NODEJS_12_X, + handler: 'index.handler' + }, + bucketProps: { + removalPolicy: cdk.RemovalPolicy.DESTROY, + }, + logS3AccessLogs: false + }); + + expect(stack).toCountResources("AWS::S3::Bucket", 1); +}); + +test('check properties with existing S3 bucket', () => { + const stack = new cdk.Stack(); + const existingBucket = CreateScrapBucket(stack, {}); + const construct = new S3ToLambda(stack, 's3-lambda', { + lambdaFunctionProps: { + code: lambda.Code.fromAsset(`${__dirname}/lambda`), + runtime: lambda.Runtime.NODEJS_12_X, + handler: 'index.handler' + }, + existingBucketObj: existingBucket + }); + + expect(construct.s3Bucket).toEqual(undefined); + expect(construct.s3BucketInterface).not.toEqual(undefined); + expect(construct.s3LoggingBucket).toEqual(undefined); }); \ No newline at end of file