From 6fab3e50de4ef73d2e9f2dbde358d9d6f14e9831 Mon Sep 17 00:00:00 2001
From: mickychetta <45010053+mickychetta@users.noreply.github.com>
Date: Wed, 3 Nov 2021 06:27:41 -0700
Subject: [PATCH] feat(aws-kinesisfirehose-s3): added custom logging bucket
 props to kinesisfirehose-s3 (#478)

* added custom logging bucket props to kinesisfirehose-s3

* added log bucket condition in input validation

* Added logS3AccessLogs for enabling/disabling s3 logs

* added cfn suppress rule for no logging

* fix lint issue

* redeploy stack for cfn nag suppress changes

* added logS3AccessLogs property

* refactored s3 bucket helper and improved tests

* readded test for s3-bucket

* moved test to s3 bucket helper test file
---
 .../aws-kinesisfirehose-s3/README.md          |   2 +
 .../aws-kinesisfirehose-s3/lib/index.ts       |  43 +-
 .../integ.customLoggingBucket.expected.json   | 400 ++++++++++++++++++
 .../test/integ.customLoggingBucket.ts         |  36 ++
 .../test/integ.no-arguments.ts                |   3 +
 .../test/integ.noLoggingBucket.expected.json  | 320 ++++++++++++++
 .../test/integ.noLoggingBucket.ts             |  40 ++
 .../test/test.kinesisfirehose-s3.test.ts      | 102 ++++-
 .../core/lib/input-validation.ts              |  19 +
 .../core/lib/s3-bucket-defaults.ts            |   4 +-
 .../core/lib/s3-bucket-helper.ts              |  72 ++--
 .../core/test/input-validation.test.ts        |  68 +++
 .../core/test/s3-bucket-helper.test.ts        |  19 +
 13 files changed, 1066 insertions(+), 62 deletions(-)
 create mode 100644 source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.expected.json
 create mode 100644 source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.ts
 create mode 100644 source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.expected.json
 create mode 100644 source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.ts

diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/README.md b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/README.md
index 9448b3cfc..387a50218 100644
--- a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/README.md
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/README.md
@@ -50,6 +50,8 @@ _Parameters_
 |existingLoggingBucketObj?|[`s3.IBucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.IBucket.html)|Optional existing instance of logging S3 Bucket for the S3 Bucket created by the pattern.|
 |kinesisFirehoseProps?|[`kinesisfirehose.CfnDeliveryStreamProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-kinesisfirehose.CfnDeliveryStreamProps.html)\|`any`|Optional user provided props to override the default props for Kinesis Firehose Delivery Stream.|
 |logGroupProps?|[`logs.LogGroupProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-logs.LogGroupProps.html)|Optional user provided props to override the default props for for the CloudWatchLogs LogGroup.|
+|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
 
diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/lib/index.ts b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/lib/index.ts
index 79f095b13..d3555b920 100644
--- a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/lib/index.ts
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/lib/index.ts
@@ -56,6 +56,19 @@ export interface KinesisFirehoseToS3Props {
    * @default - Default props are used
    */
   readonly logGroupProps?: logs.LogGroupProps;
+  /**
+   * Optional user provided props to override the default props for the S3 Logging Bucket.
+   *
+   * @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 KinesisFirehoseToS3 extends Construct {
@@ -64,6 +77,7 @@ export class KinesisFirehoseToS3 extends Construct {
   public readonly kinesisFirehoseRole: iam.Role;
   public readonly s3Bucket?: s3.Bucket;
   public readonly s3LoggingBucket?: s3.Bucket;
+  public readonly s3BucketInterface: s3.IBucket;
 
   /**
    * Constructs a new instance of the KinesisFirehoseToS3 class.
@@ -79,27 +93,18 @@ export class KinesisFirehoseToS3 extends Construct {
 
     let bucket: s3.IBucket;
 
-    if (props.existingBucketObj && props.bucketProps) {
-      throw new Error('Cannot specify both bucket properties and an existing bucket');
-    }
-
     // Setup S3 Bucket
     if (!props.existingBucketObj) {
-      let { bucketProps } = props;
+      let bucketProps = props.bucketProps ?? {};
+      bucketProps = props.existingLoggingBucketObj ?
+        overrideProps(bucketProps, { serverAccessLogsBucket: props.existingLoggingBucketObj }) :
+        bucketProps;
 
       // Setup logging S3 Bucket
-      if (props.existingLoggingBucketObj) {
-        if (!bucketProps) {
-          bucketProps = {};
-        }
-
-        bucketProps = overrideProps(bucketProps, {
-          serverAccessLogsBucket: props.existingLoggingBucketObj
-        });
-      }
-
       [this.s3Bucket, this.s3LoggingBucket] = defaults.buildS3Bucket(this, {
-        bucketProps
+        bucketProps,
+        loggingBucketProps: props.loggingBucketProps,
+        logS3AccessLogs: props.logS3AccessLogs,
       });
 
       bucket = this.s3Bucket;
@@ -107,6 +112,8 @@ export class KinesisFirehoseToS3 extends Construct {
       bucket = props.existingBucketObj;
     }
 
+    this.s3BucketInterface = bucket;
+
     // Setup Cloudwatch Log group & stream for Kinesis Firehose
     this.kinesisFirehoseLogGroup = defaults.buildLogGroup(
       this,
@@ -166,8 +173,8 @@ export class KinesisFirehoseToS3 extends Construct {
     printWarning(`kinesisFirehoseProps: ${JSON.stringify(props.kinesisFirehoseProps, null, 2)}`);
     // if the client didn't explicity say it was a Kinesis client, then turn on encryption
     if (!props.kinesisFirehoseProps ||
-        !props.kinesisFirehoseProps.deliveryStreamType ||
-        props.kinesisFirehoseProps.deliveryStreamType !== 'KinesisStreamAsSource'
+      !props.kinesisFirehoseProps.deliveryStreamType ||
+      props.kinesisFirehoseProps.deliveryStreamType !== 'KinesisStreamAsSource'
     ) {
       defaultKinesisFirehoseProps = defaults.overrideProps(
         defaultKinesisFirehoseProps,
diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.expected.json b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.expected.json
new file mode 100644
index 000000000..2cc28d71c
--- /dev/null
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.expected.json
@@ -0,0 +1,400 @@
+{
+  "Resources": {
+    "testkinesisfirehoses3S3LoggingBucketDD0F9F56": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "AccessControl": "LogDeliveryWrite",
+        "BucketEncryption": {
+          "ServerSideEncryptionConfiguration": [
+            {
+              "ServerSideEncryptionByDefault": {
+                "SSEAlgorithm": "AES256"
+              }
+            }
+          ]
+        },
+        "BucketName": "custom-logging-bucket",
+        "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"
+            }
+          ]
+        }
+      }
+    },
+    "testkinesisfirehoses3S3LoggingBucketPolicyE1990698": {
+      "Type": "AWS::S3::BucketPolicy",
+      "Properties": {
+        "Bucket": {
+          "Ref": "testkinesisfirehoses3S3LoggingBucketDD0F9F56"
+        },
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "*",
+              "Condition": {
+                "Bool": {
+                  "aws:SecureTransport": "false"
+                }
+              },
+              "Effect": "Deny",
+              "Principal": {
+                "AWS": "*"
+              },
+              "Resource": [
+                {
+                  "Fn::Join": [
+                    "",
+                    [
+                      {
+                        "Fn::GetAtt": [
+                          "testkinesisfirehoses3S3LoggingBucketDD0F9F56",
+                          "Arn"
+                        ]
+                      },
+                      "/*"
+                    ]
+                  ]
+                },
+                {
+                  "Fn::GetAtt": [
+                    "testkinesisfirehoses3S3LoggingBucketDD0F9F56",
+                    "Arn"
+                  ]
+                }
+              ],
+              "Sid": "HttpsOnly"
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "testkinesisfirehoses3S3BucketA8942735": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketEncryption": {
+          "ServerSideEncryptionConfiguration": [
+            {
+              "ServerSideEncryptionByDefault": {
+                "SSEAlgorithm": "AES256"
+              }
+            }
+          ]
+        },
+        "LifecycleConfiguration": {
+          "Rules": [
+            {
+              "NoncurrentVersionTransitions": [
+                {
+                  "StorageClass": "GLACIER",
+                  "TransitionInDays": 90
+                }
+              ],
+              "Status": "Enabled"
+            }
+          ]
+        },
+        "LoggingConfiguration": {
+          "DestinationBucketName": {
+            "Ref": "testkinesisfirehoses3S3LoggingBucketDD0F9F56"
+          }
+        },
+        "PublicAccessBlockConfiguration": {
+          "BlockPublicAcls": true,
+          "BlockPublicPolicy": true,
+          "IgnorePublicAcls": true,
+          "RestrictPublicBuckets": true
+        },
+        "VersioningConfiguration": {
+          "Status": "Enabled"
+        }
+      },
+      "UpdateReplacePolicy": "Delete",
+      "DeletionPolicy": "Delete"
+    },
+    "testkinesisfirehoses3S3BucketPolicy506CD3DD": {
+      "Type": "AWS::S3::BucketPolicy",
+      "Properties": {
+        "Bucket": {
+          "Ref": "testkinesisfirehoses3S3BucketA8942735"
+        },
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "*",
+              "Condition": {
+                "Bool": {
+                  "aws:SecureTransport": "false"
+                }
+              },
+              "Effect": "Deny",
+              "Principal": {
+                "AWS": "*"
+              },
+              "Resource": [
+                {
+                  "Fn::Join": [
+                    "",
+                    [
+                      {
+                        "Fn::GetAtt": [
+                          "testkinesisfirehoses3S3BucketA8942735",
+                          "Arn"
+                        ]
+                      },
+                      "/*"
+                    ]
+                  ]
+                },
+                {
+                  "Fn::GetAtt": [
+                    "testkinesisfirehoses3S3BucketA8942735",
+                    "Arn"
+                  ]
+                }
+              ],
+              "Sid": "HttpsOnly"
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "testkinesisfirehoses3firehoseloggroup3D11FB0D": {
+      "Type": "AWS::Logs::LogGroup",
+      "UpdateReplacePolicy": "Retain",
+      "DeletionPolicy": "Retain",
+      "Metadata": {
+        "cfn_nag": {
+          "rules_to_suppress": [
+            {
+              "id": "W86",
+              "reason": "Retention period for CloudWatchLogs LogGroups are set to 'Never Expire' to preserve customer data indefinitely"
+            },
+            {
+              "id": "W84",
+              "reason": "By default CloudWatchLogs LogGroups data is encrypted using the CloudWatch server-side encryption keys (AWS Managed Keys)"
+            }
+          ]
+        }
+      }
+    },
+    "testkinesisfirehoses3firehoseloggroupfirehoselogstream3C52AF00": {
+      "Type": "AWS::Logs::LogStream",
+      "Properties": {
+        "LogGroupName": {
+          "Ref": "testkinesisfirehoses3firehoseloggroup3D11FB0D"
+        }
+      },
+      "UpdateReplacePolicy": "Retain",
+      "DeletionPolicy": "Retain"
+    },
+    "testkinesisfirehoses3KinesisFirehoseRoleB23C3E93": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "AssumeRolePolicyDocument": {
+          "Statement": [
+            {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": {
+                "Service": "firehose.amazonaws.com"
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "testkinesisfirehoses3KinesisFirehosePolicy148BE6A6": {
+      "Type": "AWS::IAM::Policy",
+      "Properties": {
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": [
+                "s3:AbortMultipartUpload",
+                "s3:GetBucketLocation",
+                "s3:GetObject",
+                "s3:ListBucket",
+                "s3:ListBucketMultipartUploads",
+                "s3:PutObject"
+              ],
+              "Effect": "Allow",
+              "Resource": [
+                {
+                  "Fn::GetAtt": [
+                    "testkinesisfirehoses3S3BucketA8942735",
+                    "Arn"
+                  ]
+                },
+                {
+                  "Fn::Join": [
+                    "",
+                    [
+                      {
+                        "Fn::GetAtt": [
+                          "testkinesisfirehoses3S3BucketA8942735",
+                          "Arn"
+                        ]
+                      },
+                      "/*"
+                    ]
+                  ]
+                }
+              ]
+            },
+            {
+              "Action": "logs:PutLogEvents",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":logs:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":log-group:",
+                    {
+                      "Ref": "testkinesisfirehoses3firehoseloggroup3D11FB0D"
+                    },
+                    ":log-stream:",
+                    {
+                      "Ref": "testkinesisfirehoses3firehoseloggroupfirehoselogstream3C52AF00"
+                    }
+                  ]
+                ]
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        },
+        "PolicyName": "testkinesisfirehoses3KinesisFirehosePolicy148BE6A6",
+        "Roles": [
+          {
+            "Ref": "testkinesisfirehoses3KinesisFirehoseRoleB23C3E93"
+          }
+        ]
+      }
+    },
+    "testkinesisfirehoses3KinesisFirehose92F73280": {
+      "Type": "AWS::KinesisFirehose::DeliveryStream",
+      "Properties": {
+        "DeliveryStreamEncryptionConfigurationInput": {
+          "KeyType": "AWS_OWNED_CMK"
+        },
+        "ExtendedS3DestinationConfiguration": {
+          "BucketARN": {
+            "Fn::GetAtt": [
+              "testkinesisfirehoses3S3BucketA8942735",
+              "Arn"
+            ]
+          },
+          "BufferingHints": {
+            "IntervalInSeconds": 300,
+            "SizeInMBs": 5
+          },
+          "CloudWatchLoggingOptions": {
+            "Enabled": true,
+            "LogGroupName": {
+              "Ref": "testkinesisfirehoses3firehoseloggroup3D11FB0D"
+            },
+            "LogStreamName": {
+              "Ref": "testkinesisfirehoses3firehoseloggroupfirehoselogstream3C52AF00"
+            }
+          },
+          "CompressionFormat": "GZIP",
+          "EncryptionConfiguration": {
+            "KMSEncryptionConfig": {
+              "AWSKMSKeyARN": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":kms:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":alias/aws/s3"
+                  ]
+                ]
+              }
+            }
+          },
+          "RoleARN": {
+            "Fn::GetAtt": [
+              "testkinesisfirehoses3KinesisFirehoseRoleB23C3E93",
+              "Arn"
+            ]
+          }
+        }
+      }
+    }
+  },
+  "Parameters": {
+    "BootstrapVersion": {
+      "Type": "AWS::SSM::Parameter::Value<String>",
+      "Default": "/cdk-bootstrap/hnb659fds/version",
+      "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store."
+    }
+  },
+  "Rules": {
+    "CheckBootstrapVersion": {
+      "Assertions": [
+        {
+          "Assert": {
+            "Fn::Not": [
+              {
+                "Fn::Contains": [
+                  [
+                    "1",
+                    "2",
+                    "3",
+                    "4",
+                    "5"
+                  ],
+                  {
+                    "Ref": "BootstrapVersion"
+                  }
+                ]
+              }
+            ]
+          },
+          "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
+        }
+      ]
+    }
+  }
+}
\ No newline at end of file
diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.ts b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.ts
new file mode 100644
index 000000000..22f44a703
--- /dev/null
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.customLoggingBucket.ts
@@ -0,0 +1,36 @@
+/**
+ *  Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance
+ *  with the License. A copy of the License is located at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  or in the 'license' file accompanying this file. This file is distributed on an 'AS IS' BASIS, WITHOUT WARRANTIES
+ *  OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions
+ *  and limitations under the License.
+ */
+
+/// !cdk-integ *
+import { App, Stack, RemovalPolicy } from "@aws-cdk/core";
+import { BucketEncryption } from "@aws-cdk/aws-s3";
+import { KinesisFirehoseToS3 } from "../lib";
+import { generateIntegStackName } from '@aws-solutions-constructs/core';
+
+const app = new App();
+
+// Empty arguments
+const stack = new Stack(app, generateIntegStackName(__filename));
+
+new KinesisFirehoseToS3(stack, 'test-kinesisfirehose-s3', {
+  bucketProps: {
+    removalPolicy: RemovalPolicy.DESTROY,
+  },
+  loggingBucketProps: {
+    removalPolicy: RemovalPolicy.DESTROY,
+    bucketName: 'custom-logging-bucket',
+    encryption: BucketEncryption.S3_MANAGED,
+    versioned: true
+  }
+});
+app.synth();
\ No newline at end of file
diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.no-arguments.ts b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.no-arguments.ts
index 669c87989..76a83444d 100644
--- a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.no-arguments.ts
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.no-arguments.ts
@@ -24,6 +24,9 @@ stack.templateOptions.description = 'Integration Test for aws-cdk-apl-kinesisfir
 new KinesisFirehoseToS3(stack, 'test-firehose-s3', {
   bucketProps: {
     removalPolicy: RemovalPolicy.DESTROY,
+  },
+  loggingBucketProps: {
+    removalPolicy: RemovalPolicy.DESTROY
   }
 });
 
diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.expected.json b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.expected.json
new file mode 100644
index 000000000..5c24633bb
--- /dev/null
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.expected.json
@@ -0,0 +1,320 @@
+{
+  "Resources": {
+    "testkinesisfirehoses3S3BucketA8942735": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "BucketEncryption": {
+          "ServerSideEncryptionConfiguration": [
+            {
+              "ServerSideEncryptionByDefault": {
+                "SSEAlgorithm": "AES256"
+              }
+            }
+          ]
+        },
+        "LifecycleConfiguration": {
+          "Rules": [
+            {
+              "NoncurrentVersionTransitions": [
+                {
+                  "StorageClass": "GLACIER",
+                  "TransitionInDays": 90
+                }
+              ],
+              "Status": "Enabled"
+            }
+          ]
+        },
+        "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 created for unit/ integration testing purposes only."
+            }
+          ]
+        }
+      }
+    },
+    "testkinesisfirehoses3S3BucketPolicy506CD3DD": {
+      "Type": "AWS::S3::BucketPolicy",
+      "Properties": {
+        "Bucket": {
+          "Ref": "testkinesisfirehoses3S3BucketA8942735"
+        },
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "*",
+              "Condition": {
+                "Bool": {
+                  "aws:SecureTransport": "false"
+                }
+              },
+              "Effect": "Deny",
+              "Principal": {
+                "AWS": "*"
+              },
+              "Resource": [
+                {
+                  "Fn::Join": [
+                    "",
+                    [
+                      {
+                        "Fn::GetAtt": [
+                          "testkinesisfirehoses3S3BucketA8942735",
+                          "Arn"
+                        ]
+                      },
+                      "/*"
+                    ]
+                  ]
+                },
+                {
+                  "Fn::GetAtt": [
+                    "testkinesisfirehoses3S3BucketA8942735",
+                    "Arn"
+                  ]
+                }
+              ],
+              "Sid": "HttpsOnly"
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "testkinesisfirehoses3firehoseloggroup3D11FB0D": {
+      "Type": "AWS::Logs::LogGroup",
+      "UpdateReplacePolicy": "Retain",
+      "DeletionPolicy": "Retain",
+      "Metadata": {
+        "cfn_nag": {
+          "rules_to_suppress": [
+            {
+              "id": "W86",
+              "reason": "Retention period for CloudWatchLogs LogGroups are set to 'Never Expire' to preserve customer data indefinitely"
+            },
+            {
+              "id": "W84",
+              "reason": "By default CloudWatchLogs LogGroups data is encrypted using the CloudWatch server-side encryption keys (AWS Managed Keys)"
+            }
+          ]
+        }
+      }
+    },
+    "testkinesisfirehoses3firehoseloggroupfirehoselogstream3C52AF00": {
+      "Type": "AWS::Logs::LogStream",
+      "Properties": {
+        "LogGroupName": {
+          "Ref": "testkinesisfirehoses3firehoseloggroup3D11FB0D"
+        }
+      },
+      "UpdateReplacePolicy": "Retain",
+      "DeletionPolicy": "Retain"
+    },
+    "testkinesisfirehoses3KinesisFirehoseRoleB23C3E93": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "AssumeRolePolicyDocument": {
+          "Statement": [
+            {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": {
+                "Service": "firehose.amazonaws.com"
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "testkinesisfirehoses3KinesisFirehosePolicy148BE6A6": {
+      "Type": "AWS::IAM::Policy",
+      "Properties": {
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": [
+                "s3:AbortMultipartUpload",
+                "s3:GetBucketLocation",
+                "s3:GetObject",
+                "s3:ListBucket",
+                "s3:ListBucketMultipartUploads",
+                "s3:PutObject"
+              ],
+              "Effect": "Allow",
+              "Resource": [
+                {
+                  "Fn::GetAtt": [
+                    "testkinesisfirehoses3S3BucketA8942735",
+                    "Arn"
+                  ]
+                },
+                {
+                  "Fn::Join": [
+                    "",
+                    [
+                      {
+                        "Fn::GetAtt": [
+                          "testkinesisfirehoses3S3BucketA8942735",
+                          "Arn"
+                        ]
+                      },
+                      "/*"
+                    ]
+                  ]
+                }
+              ]
+            },
+            {
+              "Action": "logs:PutLogEvents",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":logs:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":log-group:",
+                    {
+                      "Ref": "testkinesisfirehoses3firehoseloggroup3D11FB0D"
+                    },
+                    ":log-stream:",
+                    {
+                      "Ref": "testkinesisfirehoses3firehoseloggroupfirehoselogstream3C52AF00"
+                    }
+                  ]
+                ]
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        },
+        "PolicyName": "testkinesisfirehoses3KinesisFirehosePolicy148BE6A6",
+        "Roles": [
+          {
+            "Ref": "testkinesisfirehoses3KinesisFirehoseRoleB23C3E93"
+          }
+        ]
+      }
+    },
+    "testkinesisfirehoses3KinesisFirehose92F73280": {
+      "Type": "AWS::KinesisFirehose::DeliveryStream",
+      "Properties": {
+        "DeliveryStreamEncryptionConfigurationInput": {
+          "KeyType": "AWS_OWNED_CMK"
+        },
+        "ExtendedS3DestinationConfiguration": {
+          "BucketARN": {
+            "Fn::GetAtt": [
+              "testkinesisfirehoses3S3BucketA8942735",
+              "Arn"
+            ]
+          },
+          "BufferingHints": {
+            "IntervalInSeconds": 300,
+            "SizeInMBs": 5
+          },
+          "CloudWatchLoggingOptions": {
+            "Enabled": true,
+            "LogGroupName": {
+              "Ref": "testkinesisfirehoses3firehoseloggroup3D11FB0D"
+            },
+            "LogStreamName": {
+              "Ref": "testkinesisfirehoses3firehoseloggroupfirehoselogstream3C52AF00"
+            }
+          },
+          "CompressionFormat": "GZIP",
+          "EncryptionConfiguration": {
+            "KMSEncryptionConfig": {
+              "AWSKMSKeyARN": {
+                "Fn::Join": [
+                  "",
+                  [
+                    "arn:",
+                    {
+                      "Ref": "AWS::Partition"
+                    },
+                    ":kms:",
+                    {
+                      "Ref": "AWS::Region"
+                    },
+                    ":",
+                    {
+                      "Ref": "AWS::AccountId"
+                    },
+                    ":alias/aws/s3"
+                  ]
+                ]
+              }
+            }
+          },
+          "RoleARN": {
+            "Fn::GetAtt": [
+              "testkinesisfirehoses3KinesisFirehoseRoleB23C3E93",
+              "Arn"
+            ]
+          }
+        }
+      }
+    }
+  },
+  "Parameters": {
+    "BootstrapVersion": {
+      "Type": "AWS::SSM::Parameter::Value<String>",
+      "Default": "/cdk-bootstrap/hnb659fds/version",
+      "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store."
+    }
+  },
+  "Rules": {
+    "CheckBootstrapVersion": {
+      "Assertions": [
+        {
+          "Assert": {
+            "Fn::Not": [
+              {
+                "Fn::Contains": [
+                  [
+                    "1",
+                    "2",
+                    "3",
+                    "4",
+                    "5"
+                  ],
+                  {
+                    "Ref": "BootstrapVersion"
+                  }
+                ]
+              }
+            ]
+          },
+          "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
+        }
+      ]
+    }
+  }
+}
\ No newline at end of file
diff --git a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.ts b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.ts
new file mode 100644
index 000000000..7b52ed350
--- /dev/null
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/integ.noLoggingBucket.ts
@@ -0,0 +1,40 @@
+/**
+ *  Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance
+ *  with the License. A copy of the License is located at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  or in the 'license' file accompanying this file. This file is distributed on an 'AS IS' BASIS, WITHOUT WARRANTIES
+ *  OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions
+ *  and limitations under the License.
+ */
+
+/// !cdk-integ *
+import { App, Stack, RemovalPolicy } from "@aws-cdk/core";
+import { KinesisFirehoseToS3 } from "../lib";
+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();
+
+// Empty arguments
+const stack = new Stack(app, generateIntegStackName(__filename));
+
+const construct = new KinesisFirehoseToS3(stack, 'test-kinesisfirehose-s3', {
+  bucketProps: {
+    removalPolicy: RemovalPolicy.DESTROY,
+  },
+  logS3AccessLogs: false
+});
+
+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-kinesisfirehose-s3/test/test.kinesisfirehose-s3.test.ts b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/test.kinesisfirehose-s3.test.ts
index 222cd5d68..8bc99c7ef 100644
--- a/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/test.kinesisfirehose-s3.test.ts
+++ b/source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3/test/test.kinesisfirehose-s3.test.ts
@@ -27,7 +27,7 @@ test('check s3Bucket default encryption', () => {
   expect(stack).toHaveResource('AWS::S3::Bucket', {
     BucketEncryption: {
       ServerSideEncryptionConfiguration: [{
-        ServerSideEncryptionByDefault : {
+        ServerSideEncryptionByDefault: {
           SSEAlgorithm: "AES256"
         }
       }]
@@ -92,7 +92,8 @@ test('test kinesisFirehose override ', () => {
         IntervalInSeconds: 600,
         SizeInMBs: 55
       }
-    }});
+    }
+  });
 });
 
 test('check default properties', () => {
@@ -206,4 +207,101 @@ 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();
+
+  new KinesisFirehoseToS3(stack, 'kinsisfirehose-s3', {
+    kinesisFirehoseProps: {
+      deliveryStreamType: 'KinesisStreamAsSource'
+    },
+    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: "kinsisfirehoses3S3LoggingBucket81EC2970"
+    }
+  });
+});
+
+// --------------------------------------------------------------
+// Test bad call with existingLoggingBucketObj and loggingBucketProps
+// --------------------------------------------------------------
+test("Test bad call with existingLoggingBucketObj and loggingBucketProps", () => {
+  // Stack
+  const stack = new cdk.Stack();
+
+  const testBucket = new s3.Bucket(stack, 'test-bucket', {});
+
+  const app = () => {
+    // Helper declaration
+    new KinesisFirehoseToS3(stack, "bad-s3-args", {
+      existingLoggingBucketObj: testBucket,
+      loggingBucketProps: {
+        removalPolicy: cdk.RemovalPolicy.DESTROY
+      },
+    });
+  };
+  // Assertion
+  expect(app).toThrowError('Error - Either provide existingLoggingBucketObj or loggingBucketProps, but not both.\n');
+});
+
+// --------------------------------------------------------------
+// Test bad call with logS3AccessLogs as false and bucketProps
+// --------------------------------------------------------------
+test("Test bad call with logS3AccessLogs as false and bucketProps", () => {
+  // Stack
+  const stack = new cdk.Stack();
+
+  const app = () => {
+    // Helper declaration
+    new KinesisFirehoseToS3(stack, "bad-s3-args", {
+      loggingBucketProps: {
+        removalPolicy: cdk.RemovalPolicy.DESTROY
+      },
+      logS3AccessLogs: false
+    });
+  };
+  // Assertion
+  expect(app).toThrowError('Error - If logS3AccessLogs is false, supplying loggingBucketProps or existingLoggingBucketObj is invalid.\n');
+});
+
+// --------------------------------------------------------------
+// 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 KinesisFirehoseToS3(stack, 'kinsisfirehose-s3', {
+    kinesisFirehoseProps: {
+      deliveryStreamType: 'KinesisStreamAsSource'
+    },
+    bucketProps: {
+      removalPolicy: cdk.RemovalPolicy.DESTROY,
+    },
+    logS3AccessLogs: false
+  });
+
+  expect(stack).toCountResources("AWS::S3::Bucket", 1);
 });
\ No newline at end of file
diff --git a/source/patterns/@aws-solutions-constructs/core/lib/input-validation.ts b/source/patterns/@aws-solutions-constructs/core/lib/input-validation.ts
index 8a9416815..22b18a109 100644
--- a/source/patterns/@aws-solutions-constructs/core/lib/input-validation.ts
+++ b/source/patterns/@aws-solutions-constructs/core/lib/input-validation.ts
@@ -72,6 +72,10 @@ export interface VerifiedProps {
 
   readonly logAlbAccessLogs?: boolean;
   readonly albLoggingBucketProps?: s3.BucketProps;
+
+  readonly existingLoggingBucketObj?: s3.IBucket;
+  readonly loggingBucketProps?: s3.BucketProps;
+  readonly logS3AccessLogs?: boolean;
 }
 
 export function CheckProps(propsObject: VerifiedProps | any) {
@@ -183,6 +187,21 @@ export function CheckProps(propsObject: VerifiedProps | any) {
     errorFound = true;
   }
 
+  if (propsObject.existingLoggingBucketObj && propsObject.loggingBucketProps) {
+    errorMessages += 'Error - Either provide existingLoggingBucketObj or loggingBucketProps, but not both.\n';
+    errorFound = true;
+  }
+
+  if ((propsObject?.logS3AccessLogs === false) && (propsObject.loggingBucketProps || propsObject.existingLoggingBucketObj)) {
+    errorMessages += 'Error - If logS3AccessLogs is false, supplying loggingBucketProps or existingLoggingBucketObj is invalid.\n';
+    errorFound = true;
+  }
+
+  if (propsObject.existingBucketObj && (propsObject.loggingBucketProps || propsObject.logS3AccessLogs)) {
+    errorMessages += 'Error - If existingBucketObj is provided, supplying loggingBucketProps or logS3AccessLogs is an error.\n';
+    errorFound = true;
+  }
+
   if (errorFound) {
     throw new Error(errorMessages);
   }
diff --git a/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-defaults.ts b/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-defaults.ts
index 2a7c5c476..8a4ef323e 100644
--- a/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-defaults.ts
+++ b/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-defaults.ts
@@ -15,7 +15,7 @@ import * as s3 from '@aws-cdk/aws-s3';
 import { RemovalPolicy } from '@aws-cdk/core';
 import { Bucket, BucketProps } from '@aws-cdk/aws-s3';
 
-export function DefaultS3Props(loggingBucket ?: Bucket, lifecycleRules?: s3.LifecycleRule[]): s3.BucketProps {
+export function DefaultS3Props(loggingBucket?: Bucket, lifecycleRules?: s3.LifecycleRule[]): s3.BucketProps {
   return {
     encryption: s3.BucketEncryption.S3_MANAGED,
     versioned: true,
@@ -36,4 +36,4 @@ export function DefaultLoggingBucketProps(): s3.BucketProps {
 }
 
 // Default event types to trigger S3 notifications
-export const defaultS3NotificationEventTypes = [s3.EventType.OBJECT_CREATED];
+export const defaultS3NotificationEventTypes = [s3.EventType.OBJECT_CREATED];
\ No newline at end of file
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
index 5c56a15ad..e20a3d20a 100644
--- a/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-helper.ts
+++ b/source/patterns/@aws-solutions-constructs/core/lib/s3-bucket-helper.ts
@@ -29,23 +29,24 @@ export interface BuildS3BucketProps {
    *
    * @default - Default props are used
    */
-  readonly bucketProps?: s3.BucketProps
+  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?] {
-  return s3BucketWithLogging(scope, props.bucketProps, bucketId, props.loggingBucketProps);
+  readonly loggingBucketProps?: s3.BucketProps;
+  /**
+   * Whether to turn on Access Logs for S3. Uses an S3 bucket with associated storage costs.
+   * Enabling Access Logging is a best practice.
+   *
+   * @default - true
+   */
+  readonly logS3AccessLogs?: boolean;
 }
 
 export function applySecureBucketPolicy(s3Bucket: s3.Bucket): void {
-
   // Apply bucket policy to enforce encryption of data in transit
-
   s3Bucket.addToResourcePolicy(
     new PolicyStatement({
       sid: 'HttpsOnly',
@@ -122,10 +123,9 @@ export function createAlbLoggingBucket(scope: Construct,
   return loggingBucket;
 }
 
-function s3BucketWithLogging(scope: Construct,
-  s3BucketProps?: s3.BucketProps,
-  bucketId?: string,
-  userLoggingBucketProps?: s3.BucketProps): [s3.Bucket, s3.Bucket?] {
+export function buildS3Bucket(scope: Construct,
+  props: BuildS3BucketProps,
+  bucketId?: string): [s3.Bucket, s3.Bucket?] {
 
   /** Default Life Cycle policy to transition older versions to Glacier after 90 days */
   const lifecycleRules: s3.LifecycleRule[] = [{
@@ -136,45 +136,37 @@ function s3BucketWithLogging(scope: Construct,
   }];
 
   // Create the Application Bucket
-  let bucketprops: s3.BucketProps;
+  let customBucketProps: s3.BucketProps;
   let loggingBucket;
   const _bucketId = bucketId ? bucketId + 'S3Bucket' : 'S3Bucket';
   const _loggingBucketId = bucketId ? bucketId + 'S3LoggingBucket' : 'S3LoggingBucket';
 
-  if (s3BucketProps?.serverAccessLogsBucket) {
-    // Attach the Default Life Cycle policy ONLY IF the versioning is ENABLED
-    if (s3BucketProps.versioned === undefined || s3BucketProps.versioned) {
-      bucketprops = DefaultS3Props(undefined, lifecycleRules);
-    } else {
-      bucketprops = DefaultS3Props();
-    }
-  } else {
+  // If logging S3 access logs is enabled/undefined and an existing bucket object is not provided
+  if (props.logS3AccessLogs !== false && !(props.bucketProps?.serverAccessLogsBucket)) {
     // Create the Logging Bucket
-    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();
+    let loggingBucketProps = DefaultS3Props();
+
+    if (props.loggingBucketProps) {
+      // User provided logging bucket props
+      loggingBucketProps = overrideProps(loggingBucketProps, props.loggingBucketProps);
+    } else if (props.bucketProps?.removalPolicy) {
+      // If the client explicitly specified a removal policy for the main bucket,
+      // then replicate that policy on the logging bucket
+      loggingBucketProps = overrideProps(loggingBucketProps, { removalPolicy: props.bucketProps.removalPolicy });
     }
 
     loggingBucket = createLoggingBucket(scope, _loggingBucketId, loggingBucketProps);
-
-    // Attach the Default Life Cycle policy ONLY IF the versioning is ENABLED
-    if (s3BucketProps?.versioned === undefined || s3BucketProps.versioned) {
-      bucketprops = DefaultS3Props(loggingBucket, lifecycleRules);
-    } else {
-      bucketprops = DefaultS3Props(loggingBucket);
-    }
   }
-
-  if (s3BucketProps) {
-    bucketprops = overrideProps(bucketprops, s3BucketProps);
+  // Attach the Default Life Cycle policy ONLY IF the versioning is ENABLED
+  if (props.bucketProps?.versioned === undefined || props.bucketProps.versioned) {
+    customBucketProps = DefaultS3Props(loggingBucket, lifecycleRules);
+  } else {
+    customBucketProps = DefaultS3Props(loggingBucket);
   }
 
-  const s3Bucket: s3.Bucket = new s3.Bucket(scope, _bucketId, bucketprops);
+  customBucketProps = props.bucketProps ? overrideProps(customBucketProps, props.bucketProps) : customBucketProps;
+
+  const s3Bucket: s3.Bucket = new s3.Bucket(scope, _bucketId, customBucketProps);
 
   applySecureBucketPolicy(s3Bucket);
 
diff --git a/source/patterns/@aws-solutions-constructs/core/test/input-validation.test.ts b/source/patterns/@aws-solutions-constructs/core/test/input-validation.test.ts
index 2184d08e3..007a55d60 100644
--- a/source/patterns/@aws-solutions-constructs/core/test/input-validation.test.ts
+++ b/source/patterns/@aws-solutions-constructs/core/test/input-validation.test.ts
@@ -427,3 +427,71 @@ test('Test fail multiple failures message', () => {
     'Error - Either provide secretProps or existingSecretObj, but not both.\n'
   );
 });
+
+test('Test fail existing log bucket and log bucket prop check', () => {
+  const stack = new Stack();
+
+  const props: defaults.VerifiedProps = {
+    existingLoggingBucketObj: new s3.Bucket(stack, 'logging-bucket'),
+    loggingBucketProps: {
+      autoDeleteObjects: true
+    }
+  };
+
+  const app = () => {
+    defaults.CheckProps(props);
+  };
+
+  // Assertion
+  expect(app).toThrowError('Error - Either provide existingLoggingBucketObj or loggingBucketProps, but not both.\n');
+});
+
+test('Test fail false logS3Accesslogs and loggingBucketProps check', () => {
+  const stack = new Stack();
+
+  const props: defaults.VerifiedProps = {
+    existingLoggingBucketObj: new s3.Bucket(stack, 'logging-bucket'),
+    logS3AccessLogs: false
+  };
+
+  const app = () => {
+    defaults.CheckProps(props);
+  };
+
+  // Assertion
+  expect(app).toThrowError('Error - If logS3AccessLogs is false, supplying loggingBucketProps or existingLoggingBucketObj is invalid.\n');
+});
+
+test('Test fail existingBucketObj and loggingBucketProps check', () => {
+  const stack = new Stack();
+
+  const props: defaults.VerifiedProps = {
+    existingBucketObj: new s3.Bucket(stack, 'temp-bucket'),
+    loggingBucketProps: {
+      autoDeleteObjects: true
+    }
+  };
+
+  const app = () => {
+    defaults.CheckProps(props);
+  };
+
+  // Assertion
+  expect(app).toThrowError('Error - If existingBucketObj is provided, supplying loggingBucketProps or logS3AccessLogs is an error.\n');
+});
+
+test('Test fail false logAlbAccessLogs and albLoggingBucketProps check', () => {
+  const props: defaults.VerifiedProps = {
+    logAlbAccessLogs: false,
+    albLoggingBucketProps: {
+      autoDeleteObjects: true
+    }
+  };
+
+  const app = () => {
+    defaults.CheckProps(props);
+  };
+
+  // Assertion
+  expect(app).toThrowError('Error - If logAlbAccessLogs is false, supplying albLoggingBucketProps is invalid.\n');
+});
\ No newline at end of file
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
index 963986f1c..45c0baa5c 100644
--- 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
@@ -387,3 +387,22 @@ test('Suppress cfn-nag warning for s3 bucket notification', () => {
     }
   }, ResourcePart.CompleteDefinition));
 });
+
+test('test s3Bucket removalPolicy override', () => {
+  const stack = new Stack();
+
+  defaults.buildS3Bucket(stack, {
+    bucketProps: {
+      removalPolicy: RemovalPolicy.DESTROY,
+    },
+  }, 'test-bucket');
+
+  expect(stack).toHaveResourceLike("AWS::S3::Bucket", {
+    Type: 'AWS::S3::Bucket',
+    Properties: {
+      AccessControl: "LogDeliveryWrite"
+    },
+    UpdateReplacePolicy: "Delete",
+    DeletionPolicy: "Delete"
+  }, ResourcePart.CompleteDefinition);
+});
\ No newline at end of file