Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aws-s3-sns): created new construct #849

Merged
merged 21 commits into from
Dec 4, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
51db29c
Add README for new aws-s3-sns construct.
georgebearden Nov 21, 2022
ee52701
add required directory for build to pass
georgebearden Nov 22, 2022
adf993d
implement the aws-s3-sns construct
georgebearden Nov 23, 2022
70e8994
Add more tests to s3-sns construct and helper classes.
georgebearden Nov 30, 2022
fa4eb4b
Merge branch 'main' into aws-s3-sns
biffgaut Nov 30, 2022
6ee16f9
feat (cloudfront-to-s3): add optional parameter cloudfront.ResponseHe…
tbelmega Nov 23, 2022
69c6ba3
fix (cloudfront-to-s3): add missing line to fix failing test
tbelmega Nov 25, 2022
6a14de7
fix (cloudfront-to-s3): add link in READMEs to https://docs.aws.amazo…
tbelmega Nov 28, 2022
47a9399
refactoring (cloudfront-to-s3): rename _api variable to api …
tbelmega Nov 28, 2022
b7aecd0
refactoring (cloudfront-to-s3): incorporate validateSecurityHeadersBe…
tbelmega Nov 28, 2022
073b35b
chore(release): 2.28.0
aws-solutions-constructs-team Nov 30, 2022
b4b7c4a
chore(changelog): Updated CHANGELOG.md
biffgaut Nov 30, 2022
a16048c
CDK Upgrade Sync
biffgaut Nov 30, 2022
2a08f10
CDK Upgrade Sync
biffgaut Nov 30, 2022
f0a0745
fix (cloudfront-to-s3): add closing bracket
tbelmega Dec 1, 2022
0288b6b
fix (cloudfront-to-s3): flipped logic when changing || to ? :
tbelmega Dec 1, 2022
bace851
fix (cloudfront-to-s3): change httpSecurityHeaders to insertHttpSecur…
tbelmega Dec 1, 2022
3ffe4b1
update test-helper function to assert inline.
georgebearden Dec 2, 2022
d330ada
Merge branch 'main' into aws-s3-sns
georgebearden Dec 2, 2022
7048e3f
fix aws-s3-sns package.json version number
georgebearden Dec 2, 2022
1f6d9bc
Add cfn nag suppression for test that is intended to use unencrypted …
georgebearden Dec 2, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
lib/*.js
test/*.js
*.d.ts
coverage
15 changes: 15 additions & 0 deletions source/patterns/@aws-solutions-constructs/aws-s3-sns/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
lib/*.js
test/*.js
*.js.map
*.d.ts
node_modules
*.generated.ts
dist
.jsii

.LAST_BUILD
.nyc_output
coverage
.nycrc
.LAST_PACKAGE
*.snk
21 changes: 21 additions & 0 deletions source/patterns/@aws-solutions-constructs/aws-s3-sns/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Exclude typescript source and config
*.ts
tsconfig.json
coverage
.nyc_output
*.tgz
*.snk
*.tsbuildinfo

# Include javascript files and typescript declarations
!*.js
!*.d.ts

# Exclude jsii outdir
dist

# Include .jsii
!.jsii

# Include .jsii
!.jsii
112 changes: 112 additions & 0 deletions source/patterns/@aws-solutions-constructs/aws-s3-sns/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# aws-s3-sns module
<!--BEGIN STABILITY BANNER-->

---

![Stability: Experimental](https://img.shields.io/badge/stability-Experimental-important.svg?style=for-the-badge)

> All classes are under active development and subject to non-backward compatible changes or removal in any
> future version. These are not subject to the [Semantic Versioning](https://semver.org/) model.
> This means that while you may use them, you may need to update your source code when upgrading to a newer version of this package.

---
<!--END STABILITY BANNER-->

| **Reference Documentation**:| <span style="font-weight: normal">https://docs.aws.amazon.com/solutions/latest/constructs/</span>|
|:-------------|:-------------|
<div style="height:8px"></div>

| **Language** | **Package** |
|:-------------|-----------------|
|![Python Logo](https://docs.aws.amazon.com/cdk/api/latest/img/python32.png) Python|`aws_solutions_constructs.aws_s3_sns`|
|![Typescript Logo](https://docs.aws.amazon.com/cdk/api/latest/img/typescript32.png) Typescript|`@aws-solutions-constructs/aws-s3-sns`|
|![Java Logo](https://docs.aws.amazon.com/cdk/api/latest/img/java32.png) Java|`software.amazon.awsconstructs.services.s3sns`|

## Overview
This AWS Solutions Construct implements an Amazon S3 Bucket that is configured to send S3 event messages to an Amazon SNS topic.


Here is a minimal deployable pattern definition:

Typescript
``` typescript
import { Construct } from 'constructs';
import { Stack, StackProps } from 'aws-cdk-lib';
import { S3ToSns } from "@aws-solutions-constructs/aws-s3-sns";

new S3ToSns(this, 'S3ToSNSPattern', {});
```

Python
```python
from aws_solutions_constructs.aws_s3_sns import S3ToSns
from aws_cdk import Stack
from constructs import Construct

S3ToSns(self, 'S3ToSNSPattern')
```

Java
``` java
import software.constructs.Construct;

import software.amazon.awscdk.Stack;
import software.amazon.awscdk.StackProps;
import software.amazon.awsconstructs.services.s3sns.*;

new S3ToSns(this, "S3ToSNSPattern", new S3ToSnsProps.Builder()
georgebearden marked this conversation as resolved.
Show resolved Hide resolved
.build());
```

## Pattern Construct Props

| **Name** | **Type** | **Description** |
|:-------------|:----------------|-----------------|
|existingBucketObj?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.Bucket.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/v2/docs/aws-cdk-lib.aws_s3.BucketProps.html)|Optional user provided props to override the default props for the S3 Bucket.|
|s3EventTypes?|[`s3.EventType[]`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.EventType.html)|The S3 event types that will trigger the notification. Defaults to s3.EventType.OBJECT_CREATED.|
|s3EventFilters?|[`s3.NotificationKeyFilter[]`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.NotificationKeyFilter.html)|S3 object key filter rules to determine which objects trigger this event. If not specified no filter rules will be applied.|
|loggingBucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.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|
|existingTopicObj?|[`sns.Topic`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.Topic.html)|Existing SNS topic to be used instead of the default topic. Providing both this and `topicProps` will cause an error. If the SNS Topic is encrypted, the KMS key utilized for encryption must be a customer managed KMS key and it must be specified in the `existingTopicEncryptionKey` property|
|existingTopicEncryptionKey?|[`kms.Key`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.Key.html)|If an existing topic is provided in the `existingTopicObj` property, and that topic is encrypted with a customer managed KMS key, this property also needs to be set with same CMK.|
|topicProps?|[`sns.TopicProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.TopicProps.html)|Optional user provided props to override the default props for the SNS topic.|
|enableEncryptionWithCustomerManagedKey?|`boolean`|If no key is provided, this flag determines whether the topic is encrypted with a new CMK or an AWS managed key. This flag is ignored if any of the following are defined: topicProps.encryptionMasterKey, encryptionKey or encryptionKeyProps.|
|encryptionKey?|[`kms.Key`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.Key.html)|An optional, imported encryption key to encrypt the SNS Topic with.|
|encryptionKeyProps?|[`kms.KeyProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.Key.html#construct-props)|Optional user provided properties to override the default properties for the KMS encryption key used to encrypt the SNS Topic with.|


## Pattern Properties

| **Name** | **Type** | **Description** |
|:-------------|:----------------|-----------------|
|snsTopic|[`sns.Topic`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.Topic.html)|Returns an instance of the SNS Topic created by the pattern.|
|encryptionKey?|[`kms.Key`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.Key.html)|Returns an instance of the kms.Key associated with the SNS Topic|
|s3Bucket?|[`s3.Bucket`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.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/v2/docs/aws-cdk-lib.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/v2/docs/aws-cdk-lib.aws_s3.IBucket.html)|Returns an instance of s3.IBucket created by the construct.|

georgebearden marked this conversation as resolved.
Show resolved Hide resolved

## Default settings

Out of the box implementation of the Construct without any override will set the following defaults:

### Amazon S3 Bucket
* Configure Access logging for the S3 Bucket
* Enable server-side encryption for S3 Bucket using an AWS managed KMS Key
* Enforce encryption of data in transit
* Turn on the versioning for the S3 Bucket
* Don't allow public access for the S3 Bucket
* Retain the S3 Bucket when deleting the CloudFormation stack
* Applies Lifecycle rule to move noncurrent object versions to Glacier storage after 90 days

### Amazon SNS Topic
* Configure least privilege SNS Topic access policy to allow the S3 Bucket to publish messages to it
georgebearden marked this conversation as resolved.
Show resolved Hide resolved
* Enable server-side encryption for the SNS Topic using an AWS managed KMS Key
* Enforce encryption of data in transit

## Architecture
![Architecture Diagram](architecture.png)

***
&copy; Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
184 changes: 184 additions & 0 deletions source/patterns/@aws-solutions-constructs/aws-s3-sns/lib/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
/**
* Copyright 2022 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.
*/

// Imports
import { Construct } from 'constructs';
import * as iam from 'aws-cdk-lib/aws-iam';
import * as kms from 'aws-cdk-lib/aws-kms';
import * as sns from 'aws-cdk-lib/aws-sns';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as defaults from '@aws-solutions-constructs/core';
import * as s3n from 'aws-cdk-lib/aws-s3-notifications';
import { addCfnNagS3BucketNotificationRulesToSuppress } from "@aws-solutions-constructs/core";
import { Stack } from 'aws-cdk-lib';

/**
* @summary The properties for the S3ToSns class.
*/
export interface S3ToSnsProps {
/**
* Existing instance of S3 Bucket object, providing both this and `bucketProps` will cause an error.
*
* @default - None
*/
readonly existingBucketObj?: s3.Bucket;
/**
* Optional user provided props to override the default props for the S3 Bucket.
*
* @default - Default props are used
*/
readonly bucketProps?: s3.BucketProps;
/**
* The S3 event types that will trigger the notification.
*
* @default - If not specified the s3.EventType.OBJECT_CREATED event will trigger the notification.
*/
readonly s3EventTypes?: s3.EventType[];
/**
* S3 object key filter rules to determine which objects trigger this event.
*
* @default - If not specified no filter rules will be applied.
*/
readonly s3EventFilters?: s3.NotificationKeyFilter[];
/**
* 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;
/**
* Existing SNS topic to be used instead of the default topic. Providing both this and `topicProps` will cause an error.
* If the SNS Topic is encrypted, the KMS key utilized for encryption must be a customer managed KMS key and it must be
* specified in the `existingTopicEncryptionKey` property.
*
* @default - Default props are used
*/
readonly existingTopicObj?: sns.Topic;
/**
* If an existing topic is provided in the `existingTopicObj` property, and that topic is encrypted with a customer managed KMS key,
* this property also needs to be set with same CMK.
*
* @default - None
*/
readonly existingTopicEncryptionKey?: kms.Key;
/**
* Optional user provided properties
*
* @default - Default props are used
*/
readonly topicProps?: sns.TopicProps;
/**
* If no key is provided, this flag determines whether the topic is encrypted with a new CMK or an AWS managed key.
* This flag is ignored if any of the following are defined: topicProps.masterKey, encryptionKey or encryptionKeyProps.
*
* @default - False if topicProps.masterKey, encryptionKey, and encryptionKeyProps are all undefined.
*/
readonly enableEncryptionWithCustomerManagedKey?: boolean;
/**
* An optional, imported encryption key to encrypt the SNS Topic with.
*
* @default - None
*/
readonly encryptionKey?: kms.Key;
/**
* Optional user provided properties to override the default properties for the KMS encryption key used to encrypt the SNS Topic with.
*
* @default - None
*/
readonly encryptionKeyProps?: kms.KeyProps;
}

/**
* @summary The S3ToSns class.
*/
export class S3ToSns extends Construct {
public readonly snsTopic: sns.Topic;
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 S3ToSns class.
* @param {cdk.App} scope - represents the scope for all the resources.
* @param {string} id - this is a a scope-unique id.
* @param {S3ToSnsProps} props - user provided props for the construct.
* @since 0.8.0
* @access public
*/
constructor(scope: Construct, id: string, props: S3ToSnsProps) {
super(scope, id);
defaults.CheckProps(props);

let bucket: s3.Bucket;
let enableEncryptionParam = props.enableEncryptionWithCustomerManagedKey;
georgebearden marked this conversation as resolved.
Show resolved Hide resolved

if (props.enableEncryptionWithCustomerManagedKey === undefined ||
props.enableEncryptionWithCustomerManagedKey === true) {
enableEncryptionParam = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just using

if (props.enableEncryptionWithCustomerManagedKey !== false)

directly in the code achieves the same result without hiding the meaning within another variable. Could even:

if (props.enableEncryptionWithCustomerManagedKey !== false) // default to true

for clarity

(quick code snippet from Typescript Playground for proof:

interface test {
    attrib?: boolean
}

let testValue: test;

testValue = { attrib: false };

if (testValue.attrib !== false) {
    console.log('attrib: false, incorrect');
} else {;
    console.log('attrib: false, correct')
}

testValue = {};

if (testValue.attrib !== false) {
    console.log('attrib: undefined, correct');
} else {;
    console.log('attrib: undefined, incorrect')
}

testValue = { attrib: true };

if (testValue.attrib !== false) {
    console.log('attrib: true, correct');
} else {;
    console.log('attrib: true, incorrect')
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait - all we do is pass this to an optional prop attribute on BuildTopicProps? Why not just handle this default inside of BuildTopicProps? That ensures consistent behavior across constructs better than code reviews that check if each construct is using the correct default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do on the simplified if logic.

Regarding the default value- I'd love to do that, but, unfortunately the sns-helper defaults the value to False, which is not overridden in other constructs, so we would be changing the behavior there as well. Specifically, the following constructs keep the default as False - aws-fargate-sns, aws-lambda-sns, aws-sns-lambda.

Let me know how you would prefer to proceed, leaving it alone, or updating it as described. If updating the default to True, which will use a CMK results in a better security posture anyway, then maybe we should do it for both consistency and security's sake.

}

// Setup the S3 bucket
if (!props.existingBucketObj) {
[this.s3Bucket, this.s3LoggingBucket] = defaults.buildS3Bucket(this, {
bucketProps: props.bucketProps,
loggingBucketProps: props.loggingBucketProps,
logS3AccessLogs: props.logS3AccessLogs
});
bucket = this.s3Bucket;
} else {
bucket = props.existingBucketObj;
}

this.s3BucketInterface = bucket;

// Setup the topic
[this.snsTopic, this.encryptionKey] = defaults.buildTopic(this, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really hate how dependent we are becoming on this paradigm of returning an array of anonymous values (there's a lot more of this in firehose-s3).

I think we need to start considering defining an interface for return values from our build* functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I love that idea too. I'd prefer to keep this PR for the new construct and then do that refactor as a quick follow-up. I've created #853 to track it.

existingTopicObj: props.existingTopicObj,
topicProps: props.topicProps,
enableEncryptionWithCustomerManagedKey: enableEncryptionParam,
encryptionKey: props.encryptionKey,
encryptionKeyProps: props.encryptionKeyProps
});

if (props.existingTopicEncryptionKey) {
georgebearden marked this conversation as resolved.
Show resolved Hide resolved
this.encryptionKey = props.existingTopicEncryptionKey;
}

// Setup the S3 bucket event types
const s3EventTypes = props.s3EventTypes ? props.s3EventTypes : defaults.defaultS3NotificationEventTypes;
georgebearden marked this conversation as resolved.
Show resolved Hide resolved

// Setup the S3 bucket event filters
const s3Eventfilters = props.s3EventFilters ? props.s3EventFilters : [];
georgebearden marked this conversation as resolved.
Show resolved Hide resolved

// Setup the S3 bucket event notifications
s3EventTypes.forEach(type => bucket.addEventNotification(type, new s3n.SnsDestination(this.snsTopic), ...s3Eventfilters));

// Grant S3 permission to use the topic's encryption key so it can publish messages to it
if (this.encryptionKey) {
this.encryptionKey.grant(new iam.ServicePrincipal("s3.amazonaws.com"),
'kms:Decrypt',
'kms:GenerateDataKey*',
);
}

addCfnNagS3BucketNotificationRulesToSuppress(Stack.of(this), 'BucketNotificationsHandler050a0587b7544547bf325f094a3db834');
}
}
Loading