added logS3AccessLogs and S3BucketInterface to s3-stepfunctions

awslabs · Nov 11, 2021 · 8134b6f · 8134b6f
1 parent c8320bd
commit 8134b6f
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 107 deletions.
diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/README.md b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/README.md
@@ -59,25 +59,27 @@ _Parameters_
 |:-------------|:----------------|-----------------|
 |existingBucketObj?|[`s3.IBucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.IBucket.html)|Existing instance of S3 Bucket object. If this is provided, then also providing bucketProps is an error. |
 |bucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|Optional user provided props to override the default props for the S3 Bucket.|
-|stateMachineProps|[`sfn.StateMachineProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-stepfunctions.StateMachineProps.html)|User provided props to override the default props for sfn.StateMachine|
-|eventRuleProps?|[`events.RuleProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-events.RuleProps.html)|Optional user provided eventRuleProps to override the defaults|
+|stateMachineProps|[`sfn.StateMachineProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-stepfunctions.StateMachineProps.html)|User provided props to override the default props for sfn.StateMachine.|
+|eventRuleProps?|[`events.RuleProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-events.RuleProps.html)|Optional user provided eventRuleProps to override the defaults.|
 |deployCloudTrail?|`boolean`|Whether to deploy a Trail in AWS CloudTrail to log API events in Amazon S3. Defaults to `true`.|
-|createCloudWatchAlarms|`boolean`|Whether to create recommended CloudWatch alarms|
+|createCloudWatchAlarms|`boolean`|Whether to create recommended CloudWatch alarms.|
 |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
 
 | **Name**     | **Type**        | **Description** |
 |:-------------|:----------------|-----------------|
-|stateMachine|[`sfn.StateMachine`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-stepfunctions.StateMachine.html)|Returns an instance of sfn.StateMachine created by the construct|
-|stateMachineLogGroup|[`logs.ILogGroup`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-logs.ILogGroup.html)|Returns an instance of the ILogGroup created by the construct for StateMachine|
-|cloudwatchAlarms?|[`cloudwatch.Alarm[]`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudwatch.Alarm.html)|Returns a list of cloudwatch.Alarm created by the construct|
-|s3Bucket?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html)|Returns an instance of the s3.Bucket created by the construct|
+|stateMachine|[`sfn.StateMachine`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-stepfunctions.StateMachine.html)|Returns an instance of sfn.StateMachine created by the construct.|
+|stateMachineLogGroup|[`logs.ILogGroup`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-logs.ILogGroup.html)|Returns an instance of the ILogGroup created by the construct for StateMachine.|
+|cloudwatchAlarms?|[`cloudwatch.Alarm[]`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudwatch.Alarm.html)|Returns a list of cloudwatch.Alarm created by the construct.|
+|s3Bucket?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html)|Returns an instance of the s3.Bucket created by the construct.|
 |s3LoggingBucket?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html)|Returns an instance of s3.Bucket created by the construct as the logging bucket for the primary bucket.|
-|cloudtrail|[`cloudtrail.Trail`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudtrail.Trail.html)|Returns an instance of the cloudtrail.Trail created by the construct|
-|cloudtrailBucket|[`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 for CloudTrail|
+|cloudtrail|[`cloudtrail.Trail`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudtrail.Trail.html)|Returns an instance of the cloudtrail.Trail created by the construct.|
+|cloudtrailBucket|[`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 for CloudTrail.|
 |cloudtrailLoggingBucket|[`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 CloudTrail 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-stepfunctions/lib/index.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/lib/index.ts
@@ -74,6 +74,13 @@ export interface S3ToStepfunctionsProps {
    * @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 S3ToStepfunctions extends Construct {
@@ -85,6 +92,7 @@ export class S3ToStepfunctions extends Construct {
   public readonly cloudtrail?: cloudtrail.Trail;
   public readonly cloudtrailBucket?: s3.Bucket;
   public readonly cloudtrailLoggingBucket?: s3.Bucket;
+  public readonly s3BucketInterface: s3.IBucket;
 
   /**
    * @summary Constructs a new instance of the S3ToStepfunctions class.
@@ -99,20 +107,19 @@ export class S3ToStepfunctions 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.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;
+
     if (props.deployCloudTrail === undefined || props.deployCloudTrail) {
       [this.cloudtrailBucket, this.cloudtrailLoggingBucket] = defaults.buildS3Bucket(this, {}, 'CloudTrail');
 

diff --git a/...ws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.expected.json b/...ws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.expected.json
@@ -441,8 +441,8 @@
       "Properties": {
         "LogGroupName": "/aws/vendedlogs/states/customloggingbuckettests3stepfunctionseventrulestepfunctionconstructstatemachineloga4e9bc58c9e9"
       },
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain",
+      "UpdateReplacePolicy": "Delete",
+      "DeletionPolicy": "Delete",
       "Metadata": {
         "cfn_nag": {
           "rules_to_suppress": [

diff --git a/...patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.ts b/...patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.ts
@@ -37,6 +37,9 @@ new S3ToStepfunctions(stack, 'test-s3-stepfunctions', {
     bucketName: 'custom-logging-bucket',
     encryption: BucketEncryption.S3_MANAGED,
     versioned: true
+  },
+  logGroupProps: {
+    removalPolicy: RemovalPolicy.DESTROY
   }
 });
 app.synth();
diff --git a/...patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.pre-existing-bucket.ts b/...patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.pre-existing-bucket.ts
@@ -35,7 +35,9 @@ const props: S3ToStepfunctionsProps = {
   logGroupProps: {
     removalPolicy: RemovalPolicy.DESTROY
   },
+  logS3AccessLogs: false
 };
 
 new S3ToStepfunctions(stack, 'test-s3-stepfunctions-pre-existing-bucket-construct', props);
+
 app.synth();
diff --git a/...ons-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json b/...ons-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json
@@ -1,89 +1,5 @@
 {
   "Resources": {
-    "tests3stepfunctionsconstructS3LoggingBucket706AEC25": {
-      "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"
-            }
-          ]
-        }
-      }
-    },
-    "tests3stepfunctionsconstructS3LoggingBucketPolicy4FEACD99": {
-      "Type": "AWS::S3::BucketPolicy",
-      "Properties": {
-        "Bucket": {
-          "Ref": "tests3stepfunctionsconstructS3LoggingBucket706AEC25"
-        },
-        "PolicyDocument": {
-          "Statement": [
-            {
-              "Action": "*",
-              "Condition": {
-                "Bool": {
-                  "aws:SecureTransport": "false"
-                }
-              },
-              "Effect": "Deny",
-              "Principal": {
-                "AWS": "*"
-              },
-              "Resource": [
-                {
-                  "Fn::Join": [
-                    "",
-                    [
-                      {
-                        "Fn::GetAtt": [
-                          "tests3stepfunctionsconstructS3LoggingBucket706AEC25",
-                          "Arn"
-                        ]
-                      },
-                      "/*"
-                    ]
-                  ]
-                },
-                {
-                  "Fn::GetAtt": [
-                    "tests3stepfunctionsconstructS3LoggingBucket706AEC25",
-                    "Arn"
-                  ]
-                }
-              ],
-              "Sid": "HttpsOnly"
-            }
-          ],
-          "Version": "2012-10-17"
-        }
-      }
-    },
     "tests3stepfunctionsconstructS3Bucket78CA0724": {
       "Type": "AWS::S3::Bucket",
       "Properties": {
@@ -109,11 +25,6 @@
             }
           ]
         },
-        "LoggingConfiguration": {
-          "DestinationBucketName": {
-            "Ref": "tests3stepfunctionsconstructS3LoggingBucket706AEC25"
-          }
-        },
         "PublicAccessBlockConfiguration": {
           "BlockPublicAcls": true,
           "BlockPublicPolicy": true,
@@ -125,7 +36,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."
+            }
+          ]
+        }
+      }
     },
     "tests3stepfunctionsconstructS3BucketPolicyC7A413B9": {
       "Type": "AWS::S3::BucketPolicy",

diff --git a/...@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts b/...@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts
@@ -16,6 +16,8 @@ import { App, Stack, RemovalPolicy } from "@aws-cdk/core";
 import { S3ToStepfunctions, S3ToStepfunctionsProps } from "../lib";
 import * as stepfunctions from '@aws-cdk/aws-stepfunctions';
 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();
 const stack = new Stack(app, generateIntegStackName(__filename));
@@ -32,7 +34,15 @@ const props: S3ToStepfunctionsProps = {
   logGroupProps: {
     removalPolicy: RemovalPolicy.DESTROY
   },
+  logS3AccessLogs: false
 };
 
-new S3ToStepfunctions(stack, 'test-s3-stepfunctions-construct', props);
+const construct = new S3ToStepfunctions(stack, 'test-s3-stepfunctions-construct', 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-stepfunctions/test/s3-stepfunctions.test.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/s3-stepfunctions.test.ts
@@ -202,4 +202,24 @@ test('s3 bucket with bucket, loggingBucket, and auto delete objects', () => {
       Ref: "tests3stepfunctionsS3LoggingBucketF7586A92"
     }
   });
+});
+
+// --------------------------------------------------------------
+// s3 bucket with one content bucket and no logging bucket
+// --------------------------------------------------------------
+test('s3 bucket with no logging bucket', () => {
+  const stack = new cdk.Stack();
+  const startState = new sfn.Pass(stack, 'StartState');
+
+  const construct = new S3ToStepfunctions(stack, 's3-stepfunctions', {
+    stateMachineProps: {
+      definition: startState
+    },
+    bucketProps: {
+      removalPolicy: cdk.RemovalPolicy.DESTROY,
+    },
+    logS3AccessLogs: false
+  });
+
+  expect(construct.s3LoggingBucket).toEqual(undefined);
 });