feat(aws-s3-sqs): added logS3AccessLogs and S3BucketInterface (#499)

mickychetta · web-flow · commit c8320bdd9cc4 · 2021-11-10T08:36:32.000-05:00
* added logS3AccessLogs and updated tests

* empty commit

* added logS3AccessLogs and S3BucketInterface
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
 });
