diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/README.md b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/README.md index 7359f65d8..fe5fbf35f 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/README.md +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/README.md @@ -61,6 +61,7 @@ _Parameters_ |deployVpc?|`boolean`|Whether to create a new VPC based on `vpcProps` into which to deploy this pattern. Setting this to true will deploy the minimal, most private VPC to run the pattern:If this property is `true` then `existingVpc` cannot be specified. Defaults to `false`.| |bucketEnvironmentVariableName?|`string`|Optional name for the S3 bucket environment variable set for the Lambda function.| |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 @@ -70,6 +71,7 @@ _Parameters_ |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 pattern.| |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.| |vpc?|[`ec2.IVpc`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ec2.IVpc.html)|Returns an interface on the VPC used by the pattern (if any). This may be a VPC created by the pattern or the VPC supplied to the pattern constructor.| +|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-lambda-s3/lib/index.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/lib/index.ts index dfffda76e..0f5ffcce1 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/lib/index.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/lib/index.ts @@ -80,6 +80,13 @@ export interface LambdaToS3Props { * @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; } /** @@ -90,6 +97,7 @@ export class LambdaToS3 extends Construct { public readonly s3Bucket?: s3.Bucket; public readonly s3LoggingBucket?: s3.Bucket; public readonly vpc?: ec2.IVpc; + public readonly s3BucketInterface: s3.IBucket; /** * @summary Constructs a new instance of the LambdaToS3 class. @@ -105,15 +113,7 @@ export class LambdaToS3 extends Construct { let bucket: s3.IBucket; - if (props.existingBucketObj && props.bucketProps) { - throw new Error('Cannot specify both bucket properties and an existing bucket'); - } - if (props.deployVpc || props.existingVpc) { - if (props.deployVpc && props.existingVpc) { - throw new Error("More than 1 VPC specified in the properties"); - } - this.vpc = defaults.buildVpc(scope, { defaultVpcProps: defaults.DefaultIsolatedVpcProps(), existingVpc: props.existingVpc, @@ -138,13 +138,16 @@ export class LambdaToS3 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; + // Configure environment variables const bucketEnvironmentVariableName = props.bucketEnvironmentVariableName || 'S3_BUCKET_NAME'; this.lambdaFunction.addEnvironment(bucketEnvironmentVariableName, bucket.bucketName); diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.expected.json b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.expected.json index 1189d24f6..f69110df6 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.expected.json @@ -146,7 +146,7 @@ } }, "Handler": "index.handler", - "Runtime": "nodejs10.x", + "Runtime": "nodejs14.x", "TracingConfig": { "Mode": "Active" } diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.ts index 187bb79e3..57176e7ac 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.customLoggingBucket.ts @@ -25,7 +25,7 @@ const stack = new Stack(app, generateIntegStackName(__filename)); new LambdaToS3(stack, 'test-lambda-s3', { lambdaFunctionProps: { - runtime: lambda.Runtime.NODEJS_10_X, + runtime: lambda.Runtime.NODEJS_14_X, handler: 'index.handler', code: lambda.Code.fromAsset(`${__dirname}/lambda`) }, diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.expected.json b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.expected.json index 54b851555..0daa8c41c 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.expected.json @@ -147,7 +147,7 @@ } }, "Handler": "index.handler", - "Runtime": "nodejs10.x", + "Runtime": "nodejs14.x", "TracingConfig": { "Mode": "Active" } @@ -175,90 +175,6 @@ } } }, - "testlambdas3S3LoggingBucketD42FC73D": { - "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" - } - ] - } - } - }, - "testlambdas3S3LoggingBucketPolicyCEAFB213": { - "Type": "AWS::S3::BucketPolicy", - "Properties": { - "Bucket": { - "Ref": "testlambdas3S3LoggingBucketD42FC73D" - }, - "PolicyDocument": { - "Statement": [ - { - "Action": "*", - "Condition": { - "Bool": { - "aws:SecureTransport": "false" - } - }, - "Effect": "Deny", - "Principal": { - "AWS": "*" - }, - "Resource": [ - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "testlambdas3S3LoggingBucketD42FC73D", - "Arn" - ] - }, - "/*" - ] - ] - }, - { - "Fn::GetAtt": [ - "testlambdas3S3LoggingBucketD42FC73D", - "Arn" - ] - } - ], - "Sid": "HttpsOnly" - } - ], - "Version": "2012-10-17" - } - } - }, "testlambdas3S3Bucket179A52E6": { "Type": "AWS::S3::Bucket", "Properties": { @@ -284,11 +200,6 @@ } ] }, - "LoggingConfiguration": { - "DestinationBucketName": { - "Ref": "testlambdas3S3LoggingBucketD42FC73D" - } - }, "PublicAccessBlockConfiguration": { "BlockPublicAcls": true, "BlockPublicPolicy": true, @@ -300,7 +211,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." + } + ] + } + } }, "testlambdas3S3BucketPolicyE899B211": { "Type": "AWS::S3::BucketPolicy", diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.ts index 57ac0964a..306a64e48 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunction.ts @@ -16,6 +16,8 @@ import { App, Stack, RemovalPolicy } from "@aws-cdk/core"; import { LambdaToS3, LambdaToS3Props } 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'; // Setup const app = new App(); @@ -25,16 +27,24 @@ stack.templateOptions.description = 'Integration Test for aws-lambda-s3'; // Definitions const props: LambdaToS3Props = { lambdaFunctionProps: { - runtime: lambda.Runtime.NODEJS_10_X, + runtime: lambda.Runtime.NODEJS_14_X, handler: 'index.handler', code: lambda.Code.fromAsset(`${__dirname}/lambda`) }, bucketProps: { removalPolicy: RemovalPolicy.DESTROY, - } + }, + logS3AccessLogs: false }; -new LambdaToS3(stack, 'test-lambda-s3', props); +const construct = new LambdaToS3(stack, 'test-lambda-s3', 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.' }, +]); // Synth app.synth(); \ No newline at end of file diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.expected.json b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.expected.json index 4a3068dc0..931cb330d 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.expected.json @@ -188,7 +188,7 @@ } }, "Handler": "index.handler", - "Runtime": "nodejs10.x", + "Runtime": "nodejs14.x", "TracingConfig": { "Mode": "Active" }, @@ -237,90 +237,6 @@ } } }, - "testlambdas3S3LoggingBucketD42FC73D": { - "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" - } - ] - } - } - }, - "testlambdas3S3LoggingBucketPolicyCEAFB213": { - "Type": "AWS::S3::BucketPolicy", - "Properties": { - "Bucket": { - "Ref": "testlambdas3S3LoggingBucketD42FC73D" - }, - "PolicyDocument": { - "Statement": [ - { - "Action": "*", - "Condition": { - "Bool": { - "aws:SecureTransport": "false" - } - }, - "Effect": "Deny", - "Principal": { - "AWS": "*" - }, - "Resource": [ - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "testlambdas3S3LoggingBucketD42FC73D", - "Arn" - ] - }, - "/*" - ] - ] - }, - { - "Fn::GetAtt": [ - "testlambdas3S3LoggingBucketD42FC73D", - "Arn" - ] - } - ], - "Sid": "HttpsOnly" - } - ], - "Version": "2012-10-17" - } - } - }, "testlambdas3S3Bucket179A52E6": { "Type": "AWS::S3::Bucket", "Properties": { @@ -346,11 +262,6 @@ } ] }, - "LoggingConfiguration": { - "DestinationBucketName": { - "Ref": "testlambdas3S3LoggingBucketD42FC73D" - } - }, "PublicAccessBlockConfiguration": { "BlockPublicAcls": true, "BlockPublicPolicy": true, @@ -362,7 +273,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." + } + ] + } + } }, "testlambdas3S3BucketPolicyE899B211": { "Type": "AWS::S3::BucketPolicy", diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.ts index 867e3f8bb..b02da7d58 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.deployFunctionWithVpc.ts @@ -21,21 +21,31 @@ import { generateIntegStackName } from '@aws-solutions-constructs/core'; const app = new App(); const stack = new Stack(app, generateIntegStackName(__filename)); stack.templateOptions.description = "Integration Test for aws-lambda-s3"; +import * as s3 from "@aws-cdk/aws-s3"; +import * as defaults from '@aws-solutions-constructs/core'; // Definitions const props: LambdaToS3Props = { lambdaFunctionProps: { - runtime: lambda.Runtime.NODEJS_10_X, + runtime: lambda.Runtime.NODEJS_14_X, handler: "index.handler", code: lambda.Code.fromAsset(`${__dirname}/lambda`), }, deployVpc: true, bucketProps: { removalPolicy: RemovalPolicy.DESTROY, - } + }, + logS3AccessLogs: false }; -new LambdaToS3(stack, "test-lambda-s3", props); +const construct = new LambdaToS3(stack, "test-lambda-s3", 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.' }, +]); // Synth app.synth(); diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.expected.json b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.expected.json index ac56611c5..99f43e96e 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.expected.json @@ -147,7 +147,7 @@ } }, "Handler": "index.handler", - "Runtime": "nodejs10.x", + "Runtime": "nodejs14.x", "TracingConfig": { "Mode": "Active" } diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.ts index baf44688b..75c1ffbcd 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.existingFunction.ts @@ -25,7 +25,7 @@ stack.templateOptions.description = 'Integration Test for aws-lambda-s3'; // Definitions const lambdaFunctionProps = { - runtime: lambda.Runtime.NODEJS_10_X, + runtime: lambda.Runtime.NODEJS_14_X, handler: 'index.handler', code: lambda.Code.fromAsset(`${__dirname}/lambda`) }; diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.expected.json b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.expected.json index 7e37ca8b2..2c7c73c9d 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.expected.json @@ -195,7 +195,7 @@ } }, "Handler": "index.handler", - "Runtime": "nodejs10.x", + "Runtime": "nodejs14.x", "TracingConfig": { "Mode": "Active" } diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.ts index 28651e3d8..9365aca98 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/integ.pre-existing-bucket.ts @@ -30,10 +30,10 @@ const mybucket: s3.IBucket = s3.Bucket.fromBucketName(stack, 'mybucket', existin const props: LambdaToS3Props = { existingBucketObj: mybucket, lambdaFunctionProps: { - runtime: lambda.Runtime.NODEJS_10_X, + runtime: lambda.Runtime.NODEJS_14_X, handler: 'index.handler', code: lambda.Code.fromAsset(`${__dirname}/lambda`) - }, + } }; new LambdaToS3(stack, 'test-lambda-s3-pre-existing-bucket', props); diff --git a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/lambda-s3.test.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/lambda-s3.test.ts index cfe802b09..eb328d55e 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/lambda-s3.test.ts +++ b/source/patterns/@aws-solutions-constructs/aws-lambda-s3/test/lambda-s3.test.ts @@ -33,7 +33,7 @@ test('Test the properties', () => { handler: 'index.handler', code: lambda.Code.fromAsset(`${__dirname}/lambda`) }, - bucketPermissions: ['Write'] + bucketPermissions: ['Write', 'Delete', 'Put', 'Read', 'ReadWrite'] }); // Assertion 1 const func = pattern.lambdaFunction; @@ -273,7 +273,7 @@ test("Test bad call with existingVpc and deployVpc", () => { }); }; // Assertion - expect(app).toThrowError(); + expect(app).toThrowError('Error - Either provide an existingVpc or some combination of deployVpc and vpcProps, but not both.\n'); }); // -------------------------------------------------------------- @@ -402,4 +402,25 @@ test('s3 bucket with bucket, loggingBucket, and auto delete objects', () => { Ref: "lambdas3S3LoggingBucket498F3BDD" } }); +}); + +// -------------------------------------------------------------- +// s3 bucket with one content bucket and no logging bucket +// -------------------------------------------------------------- +test('s3 bucket with one content bucket and no logging bucket', () => { + const stack = new Stack(); + + new LambdaToS3(stack, 'lambda-s3', { + lambdaFunctionProps: { + code: lambda.Code.fromAsset(`${__dirname}/lambda`), + runtime: lambda.Runtime.NODEJS_12_X, + handler: 'index.handler' + }, + bucketProps: { + removalPolicy: RemovalPolicy.DESTROY, + }, + logS3AccessLogs: false + }); + + expect(stack).toCountResources("AWS::S3::Bucket", 1); }); \ No newline at end of file