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.

Sign up for GitHub

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

Jump to bottom

fix(standardize encryption props) Standardize how encryption properties are used for SNS/SQS constructs. #846

Merged
merged 2 commits into from
Nov 21, 2022
Merged

fix(standardize encryption props) Standardize how encryption properties are used for SNS/SQS constructs. #846

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4beac61
Standardize how encryption properties are used for SNS and SQS-based …
georgebearden Oct 27, 2022
39c1708
improve test coverage and standardize semi-colons on interface proper…
georgebearden Nov 21, 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
11 changes: 7 additions & 4 deletions DESIGN_GUIDELINES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,9 @@ Existing Inconsistencies would not be published, that’s for our internal use
| --- | --- | --- |--- |
| existingTopicObj? | [`sns.Topic`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.Topic.html)|An optional, existing SNS topic to be used instead of the default topic. Providing both this and `topicProps` will cause an error|
| topicProps? | [`sns.TopicProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.TopicProps.html)|Optional user provided properties to override the default properties for the SNS topic.
| enableEncryptionWithCustomerManagedKey? | `boolean`|Use a KMS Key, either managed by this CDK app, or imported. If importing an encryption key, it must be specified in the encryptionKey property for this construct.| Sending messages from an AWS service to an encrypted Topic [requires a Customer Master key](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-key-management.html#compatibility-with-aws-services). Those constructs require these properties. |
| 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 SQS queue, and SNS Topic.|
| encryptionKeyProps? | [`kms.KeyProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.KeyProps.html)|An optional, user provided properties to override the default properties for the KMS encryption key |
| enableEncryptionWithCustomerManagedKey? | `boolean`|If no key is provided, this flag determines whether the SNS 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.| Sending messages from an AWS service to an encrypted Topic [requires a Customer Master key](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-key-management.html#compatibility-with-aws-services). Those constructs require these properties. |
| 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.KeyProps.html)|Optional user provided properties to override the default properties for the KMS encryption key used to encrypt the SNS Topic with. |

**Required Construct Properties**

Expand All @@ -331,7 +331,10 @@ Existing Inconsistencies would not be published, that’s for our internal use
| deadLetterQueueProps? | [`sqs.QueueProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sqs.QueueProps.html)|Optional user provided props to override the default props for the SQS queue.|
| maxReceiveCount | `int` | The number of times a message can be unsuccessfully dequeued before being moved to the dead letter queue. Defaults to `15`. |
| enableQueuePurging | `boolean` | Whether to grant additional permissions to the Lambda function enabling it to purge the SQS queue. Defaults to `false`. | This is only on 2 constructs, docs talk about a Lambda function role.|
| encryptionKey? | [`kms.Key`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.Key.html)|Optional imported encryption key to encrypt the SQS queue. | Sending messages from an AWS service to an encrypted queue [requires a Customer Master key](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-key-management.html#compatibility-with-aws-services). Those constructs require these properties. |
|enableEncryptionWithCustomerManagedKey?|`boolean`|If no key is provided, this flag determines whether the queue is encrypted with a new CMK or an AWS managed key. |This flag is ignored if any of the following are defined: queueProps.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 SQS Queue with.|Sending messages from an AWS service to an encrypted queue [requires a Customer Master key](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-key-management.html#compatibility-with-aws-services). Those constructs require these properties. |
|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 SQS queue with.|


**Required Construct Properties**

Expand Down
3 changes: 3 additions & 0 deletions source/patterns/@aws-solutions-constructs/aws-apigateway-sqs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ new ApiGatewayToSqs(this, "ApiGatewayToSqsPattern", new ApiGatewayToSqsProps.Bui
|allowDeleteOperation?|`boolean`|Whether to deploy an API Gateway Method for Delete operations on the queue (i.e. sqs:DeleteMessage).|
|deleteRequestTemplate?|`string`|Override the default API Gateway Request template for Delete method, if allowDeleteOperation set to true.|
|logGroupProps?|[`logs.LogGroupProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_logs.LogGroupProps.html)|User provided props to override the default props for for the CloudWatchLogs LogGroup.|
|enableEncryptionWithCustomerManagedKey?|`boolean`|If no key is provided, this flag determines whether the queue is encrypted with a new CMK or an AWS managed key. This flag is ignored if any of the following are defined: queueProps.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 SQS Queue 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 SQS queue with.|

## Pattern Properties

Expand Down
25 changes: 24 additions & 1 deletion source/patterns/@aws-solutions-constructs/aws-apigateway-sqs/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

// Imports
import * as api from 'aws-cdk-lib/aws-apigateway';
import * as kms from 'aws-cdk-lib/aws-kms';
import * as sqs from 'aws-cdk-lib/aws-sqs';
import * as iam from 'aws-cdk-lib/aws-iam';
import * as defaults from '@aws-solutions-constructs/core';
Expand Down Expand Up @@ -103,6 +104,25 @@ export interface ApiGatewayToSqsProps {
* @default - Default props are used
*/
readonly logGroupProps?: logs.LogGroupProps
/**
* If no key is provided, this flag determines whether the queue is encrypted with a new CMK or an AWS managed key.
* This flag is ignored if any of the following are defined: queueProps.encryptionMasterKey, encryptionKey or encryptionKeyProps.
*
* @default - False if queueProps.encryptionMasterKey, encryptionKey, and encryptionKeyProps are all undefined.
*/
readonly enableEncryptionWithCustomerManagedKey?: boolean
/**
* An optional, imported encryption key to encrypt the SQS Queue 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 SQS queue with.
*
* @default - None
*/
readonly encryptionKeyProps?: kms.KeyProps
}

/**
Expand Down Expand Up @@ -140,7 +160,10 @@ export class ApiGatewayToSqs extends Construct {
[this.sqsQueue] = defaults.buildQueue(this, 'queue', {
existingQueueObj: props.existingQueueObj,
queueProps: props.queueProps,
deadLetterQueue: this.deadLetterQueue
deadLetterQueue: this.deadLetterQueue,
enableEncryptionWithCustomerManagedKey: props.enableEncryptionWithCustomerManagedKey,
encryptionKey: props.encryptionKey,
encryptionKeyProps: props.encryptionKeyProps
});

// Setup the API Gateway
Expand Down
90 changes: 90 additions & 0 deletions source/patterns/@aws-solutions-constructs/aws-apigateway-sqs/test/apigateway-sqs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Stack } from "aws-cdk-lib";
import { ApiGatewayToSqs } from '../lib';
import '@aws-cdk/assert/jest';
import * as api from "aws-cdk-lib/aws-apigateway";
import * as kms from 'aws-cdk-lib/aws-kms';
import * as sqs from "aws-cdk-lib/aws-sqs";

test('Test deployment w/o DLQ', () => {
Expand Down Expand Up @@ -171,4 +172,93 @@ test('Test deployment with existing queue object', () => {
});

expect(stack).toCountResources("AWS::SQS::Queue", 1);
});

test('Queue is encrypted with imported CMK when set on encryptionKey prop', () => {
const stack = new Stack();
const cmk = new kms.Key(stack, 'cmk');
new ApiGatewayToSqs(stack, 'api-gateway-sqs', {
encryptionKey: cmk
});

expect(stack).toHaveResource("AWS::SQS::Queue", {
KmsMasterKeyId: {
"Fn::GetAtt": [
"cmk01DE03DA",
"Arn"
]
}
});
});

test('Queue is encrypted with imported CMK when set on queueProps.encryptionKeyProps prop', () => {
const stack = new Stack();
const cmk = new kms.Key(stack, 'cmk');
new ApiGatewayToSqs(stack, 'api-gateway-sqs', {
queueProps: {
encryptionMasterKey: cmk
}
});

expect(stack).toHaveResource("AWS::SQS::Queue", {
KmsMasterKeyId: {
"Fn::GetAtt": [
"cmk01DE03DA",
"Arn"
]
}
});
});

test('Queue is encrypted with provided encrytionKeyProps', () => {
const stack = new Stack();
new ApiGatewayToSqs(stack, 'api-gateway-sqs', {
encryptionKeyProps: {
alias: 'new-key-alias-from-props'
}
});

expect(stack).toHaveResource("AWS::SQS::Queue", {
KmsMasterKeyId: {
"Fn::GetAtt": [
"apigatewaysqsEncryptionKey4A698F7C",
"Arn"
]
}
});

expect(stack).toHaveResource('AWS::KMS::Alias', {
AliasName: 'alias/new-key-alias-from-props',
TargetKeyId: {
'Fn::GetAtt': [
'apigatewaysqsEncryptionKey4A698F7C',
'Arn'
]
}
});
});

test('Queue is encrypted by default with AWS-managed KMS key when no other encryption properties are set', () => {
const stack = new Stack();
new ApiGatewayToSqs(stack, 'api-gateway-sqs', {});

expect(stack).toHaveResource('AWS::SQS::Queue', {
KmsMasterKeyId: "alias/aws/sqs"
});
});

test('Queue is encrypted with customer managed KMS Key when enable encryption flag is true', () => {
const stack = new Stack();
new ApiGatewayToSqs(stack, 'api-gateway-sqs', {
enableEncryptionWithCustomerManagedKey: true
});

expect(stack).toHaveResource("AWS::SQS::Queue", {
KmsMasterKeyId: {
"Fn::GetAtt": [
"apigatewaysqsEncryptionKey4A698F7C",
"Arn"
]
}
});
});
6 changes: 3 additions & 3 deletions source/patterns/@aws-solutions-constructs/aws-eventbridge-sns/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ constructStack.getEncryptionKey().addToResourcePolicy(policyStatement);
|topicProps?|[`sns.TopicProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sns.TopicProps.html)|User provided props to override the default props for the SNS Topic. |
|existingEventBusInterface?|[`events.IEventBus`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_events.IEventBus.html)| Optional user-provided custom EventBus for construct to use. Providing both this and `eventBusProps` results an error.|
|eventBusProps?|[`events.EventBusProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_events.EventBusProps.html)|Optional user-provided properties to override the default properties when creating a custom EventBus. Setting this value to `{}` will create a custom EventBus using all default properties. If neither this nor `existingEventBusInterface` is provided the construct will use the `default` EventBus. Providing both this and `existingEventBusInterface` results an error.|
|enableEncryptionWithCustomerManagedKey?|`boolean`|Use a KMS Key, either managed by this CDK app, or imported. If importing an encryption key, it must be specified in the encryptionKey property for this construct.|
|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.|
|encryptionKeyProps?|[`kms.KeyProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.KeyProps.html)|An optional, user provided properties to override the default properties for the KMS encryption key.|
|enableEncryptionWithCustomerManagedKey?|`boolean`|If no key is provided, this flag determines whether the SNS 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.|
|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

Expand Down
20 changes: 10 additions & 10 deletions source/patterns/@aws-solutions-constructs/aws-eventbridge-sns/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,24 +52,24 @@ export interface EventbridgeToSnsProps {
*/
readonly existingTopicObj?: sns.Topic;
/**
* Use a KMS Key, either managed by this CDK app, or imported. If importing an encryption key, it must be specified in
* the encryptionKey property for this construct.
* 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 - true (encryption enabled, managed by this CDK app).
* @default - True if topicProps.masterKey, encryptionKey, and encryptionKeyProps are all undefined.
*/
readonly enableEncryptionWithCustomerManagedKey?: boolean;
readonly enableEncryptionWithCustomerManagedKey?: boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we dropping semicolons?

It makes us inconsistent across the library. I realize the Javascript (so therefore probably Typescript) provides "Automatic Semicolon Insertion" in "certain situations." Rather than depend upon everyone memorizing these "certain situations" we'll use semicolons everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The identified constructs above (eventbridge-sqs, lambda-sns, lambda-sqs) have updated tests and are now at 100% coverage.

/**
* An optional, imported encryption key to encrypt the SQS queue, and SNS Topic.
* An optional, imported encryption key to encrypt the SNS topic with.
*
* @default - not specified.
* @default - None.
*/
readonly encryptionKey?: kms.Key;
readonly encryptionKey?: kms.Key
/**
* Optional user-provided props to override the default props for the encryption key.
* Optional user provided properties to override the default properties for the KMS encryption key used to encrypt the SNS topic with.
*
* @default - Default props are used.
* @default - None
*/
readonly encryptionKeyProps?: kms.KeyProps;
readonly encryptionKeyProps?: kms.KeyProps
}

export class EventbridgeToSns extends Construct {
Expand Down
97 changes: 97 additions & 0 deletions ...patterns/@aws-solutions-constructs/aws-eventbridge-sns/test/eventbridge-sns-topic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,101 @@ test('check custom event bus resource with props when deploy:true', () => {
expect(stack).toHaveResource('AWS::Events::EventBus', {
Name: 'testcustomeventbus'
});
});

test('Topic is encrypted when key is provided on topicProps.masterKey prop', () => {
const stack = new cdk.Stack();
const key = defaults.buildEncryptionKey(stack, {
description: 'my-key'
});

const props: EventbridgeToSnsProps = {
eventRuleProps: {
schedule: events.Schedule.rate(cdk.Duration.minutes(5))
},
topicProps: {
masterKey: key
}
};

new EventbridgeToSns(stack, 'test-events-rule-sqs', props);

expect(stack).toHaveResource('AWS::SNS::Topic', {
KmsMasterKeyId: {
"Fn::GetAtt": [
"EncryptionKey1B843E66",
"Arn"
]
}
});

expect(stack).toHaveResource('AWS::KMS::Key', {
Description: "my-key",
EnableKeyRotation: true
});
});

test('Topic is encrypted when keyProps are provided', () => {
const stack = new cdk.Stack();

const props: EventbridgeToSnsProps = {
eventRuleProps: {
schedule: events.Schedule.rate(cdk.Duration.minutes(5))
},
encryptionKeyProps: {
description: 'my-key'
}
};

new EventbridgeToSns(stack, 'test-events-rule-sqs', props);

expect(stack).toHaveResource('AWS::SNS::Topic', {
KmsMasterKeyId: {
"Fn::GetAtt": [
"testeventsrulesqsEncryptionKey19AB0C02",
"Arn"
]
}
});

expect(stack).toHaveResource('AWS::KMS::Key', {
Description: "my-key",
EnableKeyRotation: true
});
});

test('Topic is encrypted with AWS-managed KMS key when enableEncryptionWithCustomerManagedKey property is false', () => {
const stack = new cdk.Stack();

const props: EventbridgeToSnsProps = {
eventRuleProps: {
schedule: events.Schedule.rate(cdk.Duration.minutes(5))
},
enableEncryptionWithCustomerManagedKey: false
};

new EventbridgeToSns(stack, 'test-events-rule-sqs', props);

expect(stack).toHaveResource('AWS::SNS::Topic', {
KmsMasterKeyId: {
"Fn::Join": [
"",
[
"arn:",
{
Ref: "AWS::Partition"
},
":kms:",
{
Ref: "AWS::Region"
},
":",
{
Ref: "AWS::AccountId"
},
":alias/aws/sns"
]
]
}
});
});
6 changes: 3 additions & 3 deletions source/patterns/@aws-solutions-constructs/aws-eventbridge-sqs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ constructStack.getEncryptionKey().addToResourcePolicy(policyStatement);
|deployDeadLetterQueue?|`boolean`|Whether to create a secondary queue to be used as a dead letter queue. Defaults to `true`.|
|deadLetterQueueProps?|[`sqs.QueueProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sqs.QueueProps.html)|Optional user-provided props to override the default props for the dead letter queue. Only used if the `deployDeadLetterQueue` property is set to true.|
|maxReceiveCount?|`number`|The number of times a message can be unsuccessfully dequeued before being moved to the dead letter queue. Defaults to `15`.|
|enableEncryptionWithCustomerManagedKey?|`boolean`|Use a KMS Key, either managed by this CDK app, or imported. If importing an encryption key, it must be specified in the encryptionKey property for this construct.|
|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 SQS queue.|
|encryptionKeyProps?|[`kms.KeyProps`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kms.KeyProps.html)|An optional, user provided properties to override the default properties for the KMS encryption key.|
|enableEncryptionWithCustomerManagedKey?|`boolean`|If no key is provided, this flag determines whether the queue is encrypted with a new CMK or an AWS managed key. This flag is ignored if any of the following are defined: queueProps.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 SQS Queue 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 SQS queue with.|

## Pattern Properties

Expand Down
Loading