From 8134b6fa574834f95c5b4748fba1ec170bc3e87f Mon Sep 17 00:00:00 2001 From: mickychetta Date: Thu, 11 Nov 2021 20:20:27 +0000 Subject: [PATCH] added logS3AccessLogs and S3BucketInterface to s3-stepfunctions --- .../aws-s3-stepfunctions/README.md | 20 ++-- .../aws-s3-stepfunctions/lib/index.ts | 17 ++- .../integ.customLoggingBucket.expected.json | 4 +- .../test/integ.customLoggingBucket.ts | 3 + .../test/integ.pre-existing-bucket.ts | 2 + ...s3-stepfunctions-no-argument.expected.json | 101 ++---------------- .../integ.s3-stepfunctions-no-argument.ts | 12 ++- .../test/s3-stepfunctions.test.ts | 20 ++++ 8 files changed, 72 insertions(+), 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 index 398d08b2e..0d2a21619 100644 --- 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 index ea7f853de..89e042032 100644 --- 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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.expected.json b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.expected.json index 47b278e81..a1cc70c59 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.expected.json +++ b/source/patterns/@aws-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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.ts index 7b18dd21c..9c2829f52 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.customLoggingBucket.ts +++ b/source/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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.pre-existing-bucket.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.pre-existing-bucket.ts index b5b598319..189749ec3 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.pre-existing-bucket.ts +++ b/source/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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json index f55c15e55..fb0781688 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json +++ b/source/patterns/@aws-solutions-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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts index 3c9fb702b..57eac4f36 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts +++ b/source/patterns/@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 index bbc27f4d2..7632dd420 100644 --- 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); }); \ No newline at end of file