diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/README.md b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/README.md index e86b02788..2788cc214 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/README.md +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/README.md @@ -62,6 +62,7 @@ _Parameters_ |encryptionKey?|[`kms.Key`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-kms.Key.html)|Optional imported encryption key to encrypt the SQS queue.| |encryptionKeyProps?|[`kms.KeyProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-kms.KeyProps.html)|Optional user provided properties to override the default properties for the KMS encryption key.| |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 @@ -72,6 +73,7 @@ _Parameters_ |encryptionKey|[`kms.IKey`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-kms.IKey.html)|Returns an instance of kms.Key used for the SQS queue.| |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-sqs/lib/index.ts b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/lib/index.ts index 59d0e9449..a1d28af13 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/lib/index.ts +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/lib/index.ts @@ -103,6 +103,13 @@ export interface S3ToSqsProps { * @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; } /** @@ -114,6 +121,7 @@ export class S3ToSqs extends Construct { public readonly s3Bucket?: s3.Bucket; public readonly s3LoggingBucket?: s3.Bucket; public readonly encryptionKey?: kms.IKey; + public readonly s3BucketInterface: s3.IBucket; /** * @summary Constructs a new instance of the S3ToSqs class. @@ -130,10 +138,6 @@ export class S3ToSqs extends Construct { let bucket: s3.Bucket; let enableEncryptionParam = props.enableEncryptionWithCustomerManagedKey; - if (props.existingBucketObj && props.bucketProps) { - throw new Error('Cannot specify both bucket properties and an existing bucket'); - } - if (props.enableEncryptionWithCustomerManagedKey === undefined || props.enableEncryptionWithCustomerManagedKey === true) { enableEncryptionParam = true; @@ -143,13 +147,16 @@ export class S3ToSqs 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; + // Setup the dead letter queue, if applicable this.deadLetterQueue = defaults.buildDeadLetterQueue(this, { existingQueueObj: props.existingQueueObj, diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.expected.json b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.expected.json index 1d0643004..552ef573d 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.expected.json @@ -37,90 +37,6 @@ "UpdateReplacePolicy": "Retain", "DeletionPolicy": "Retain" }, - "tests3sqsS3LoggingBucket0B0BC86A": { - "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" - } - ] - } - } - }, - "tests3sqsS3LoggingBucketPolicy3A15958C": { - "Type": "AWS::S3::BucketPolicy", - "Properties": { - "Bucket": { - "Ref": "tests3sqsS3LoggingBucket0B0BC86A" - }, - "PolicyDocument": { - "Statement": [ - { - "Action": "*", - "Condition": { - "Bool": { - "aws:SecureTransport": "false" - } - }, - "Effect": "Deny", - "Principal": { - "AWS": "*" - }, - "Resource": [ - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "tests3sqsS3LoggingBucket0B0BC86A", - "Arn" - ] - }, - "/*" - ] - ] - }, - { - "Fn::GetAtt": [ - "tests3sqsS3LoggingBucket0B0BC86A", - "Arn" - ] - } - ], - "Sid": "HttpsOnly" - } - ], - "Version": "2012-10-17" - } - } - }, "tests3sqsS3BucketNotifications32539247": { "Type": "Custom::S3BucketNotifications", "Properties": { @@ -194,11 +110,6 @@ } ] }, - "LoggingConfiguration": { - "DestinationBucketName": { - "Ref": "tests3sqsS3LoggingBucket0B0BC86A" - } - }, "PublicAccessBlockConfiguration": { "BlockPublicAcls": true, "BlockPublicPolicy": true, @@ -210,7 +121,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." + } + ] + } + } }, "tests3sqsS3BucketPolicyA477877B": { "Type": "AWS::S3::BucketPolicy", diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.ts b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.ts index 024c7f503..00b9e663a 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.ts +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.creatingNewQueue.ts @@ -17,6 +17,7 @@ import { S3ToSqs, S3ToSqsProps } from "../lib"; import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import { generateIntegStackName } from '@aws-solutions-constructs/core'; +import * as defaults from '@aws-solutions-constructs/core'; // Setup const app = new App(); @@ -48,10 +49,17 @@ const props: S3ToSqsProps = { s3EventFilters: [filter], bucketProps: { removalPolicy: RemovalPolicy.DESTROY, - } + }, + logS3AccessLogs: false }; -new S3ToSqs(stack, 'test-s3-sqs', props); +const construct = new S3ToSqs(stack, 'test-s3-sqs', 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-s3-sqs/test/integ.existingQueue.expected.json b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.expected.json index c7b700841..a0cc64961 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.expected.json @@ -181,90 +181,6 @@ ] } }, - "tests3sqsS3LoggingBucket0B0BC86A": { - "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" - } - ] - } - } - }, - "tests3sqsS3LoggingBucketPolicy3A15958C": { - "Type": "AWS::S3::BucketPolicy", - "Properties": { - "Bucket": { - "Ref": "tests3sqsS3LoggingBucket0B0BC86A" - }, - "PolicyDocument": { - "Statement": [ - { - "Action": "*", - "Condition": { - "Bool": { - "aws:SecureTransport": "false" - } - }, - "Effect": "Deny", - "Principal": { - "AWS": "*" - }, - "Resource": [ - { - "Fn::Join": [ - "", - [ - { - "Fn::GetAtt": [ - "tests3sqsS3LoggingBucket0B0BC86A", - "Arn" - ] - }, - "/*" - ] - ] - }, - { - "Fn::GetAtt": [ - "tests3sqsS3LoggingBucket0B0BC86A", - "Arn" - ] - } - ], - "Sid": "HttpsOnly" - } - ], - "Version": "2012-10-17" - } - } - }, "tests3sqsS3BucketNotifications32539247": { "Type": "Custom::S3BucketNotifications", "Properties": { @@ -324,11 +240,6 @@ } ] }, - "LoggingConfiguration": { - "DestinationBucketName": { - "Ref": "tests3sqsS3LoggingBucket0B0BC86A" - } - }, "PublicAccessBlockConfiguration": { "BlockPublicAcls": true, "BlockPublicPolicy": true, @@ -340,7 +251,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." + } + ] + } + } }, "tests3sqsS3BucketPolicyA477877B": { "Type": "AWS::S3::BucketPolicy", diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.ts b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.ts index f86cf6a25..ffe07b640 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.ts +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/integ.existingQueue.ts @@ -16,6 +16,7 @@ import { App, Stack, RemovalPolicy } from "@aws-cdk/core"; import { S3ToSqs, S3ToSqsProps } from "../lib"; import * as defaults from '@aws-solutions-constructs/core'; import { generateIntegStackName } from '@aws-solutions-constructs/core'; +import * as s3 from "@aws-cdk/aws-s3"; const app = new App(); @@ -29,8 +30,16 @@ const props: S3ToSqsProps = { existingQueueObj: myQueue, bucketProps: { removalPolicy: RemovalPolicy.DESTROY, - } + }, + logS3AccessLogs: false }; -new S3ToSqs(stack, 'test-s3-sqs', props); +const construct = new S3ToSqs(stack, 'test-s3-sqs', 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(); diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/test.s3-sqs.test.ts b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/test.s3-sqs.test.ts index a95d1781e..c5bab0e70 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/test.s3-sqs.test.ts +++ b/source/patterns/@aws-solutions-constructs/aws-s3-sqs/test/test.s3-sqs.test.ts @@ -232,7 +232,7 @@ test("Test bad call with existingBucket and bucketProps", () => { }); }; // Assertion - expect(app).toThrowError(); + expect(app).toThrowError('Error - Either provide bucketProps or existingBucketObj, but not both.\n'); }); // -------------------------------------------------------------- @@ -266,4 +266,20 @@ test('s3 bucket with bucket, loggingBucket, and auto delete objects', () => { Ref: "s3sqsS3LoggingBucketD877FC52" } }); +}); + +// -------------------------------------------------------------- +// 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 S3ToSqs(stack, 's3-sqs', { + bucketProps: { + removalPolicy: RemovalPolicy.DESTROY, + }, + logS3AccessLogs: false + }); + + expect(stack).toCountResources("AWS::S3::Bucket", 1); }); \ No newline at end of file