feat(aws-iot-sqs): initial implementation #267
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| | **Name** | **Type** | **Description** | | ||
| |:-------------|:----------------|-----------------| | ||
| |encryptionKey?|[`kms.Key`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-kms.Key.html)|Returns an instance of `kms.Key` used for the SQS queue.| | ||
| |iotRole|[`iam.Role`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-iam.Role.html)|Returns an instance of `iam.Role` created by the construct, which allows IoT to publish messages to the SQS Queue| |
There was a problem hiding this comment.
Sounds good. I will rename it to iotActionsRole
| |deadLetterQueueProps?|[`sqs.QueueProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-sqs.QueueProps.html)|Optional user provided properties for the dead letter queue.| | ||
| |deployDeadLetterQueue?|`boolean`|Whether to deploy a secondary queue to be used as a dead letter queue. Default `true`.| | ||
| |maxReceiveCount?|`number`|The number of times a message can be unsuccessfully dequeued before being moved to the dead-letter queue. Required field if `deployDeadLetterQueue`=`true`.| | ||
| |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.| |
There was a problem hiding this comment.
This varies from the design guidelines for SQS and it's not clear what it accomplishes. encryptionKey already allows a client to specify an existing key to encrypt the queue.
There was a problem hiding this comment.
When creating the queue, I was planning to use the buildQueue SQS helper provided by Solutions Constructs. In this snippet it seems like the logic is that if enableEncryptionWithCustomerManagedKey is false, the queue will not have encryption enabled. If true and the user provides a key in encryptionKey, that one will be used. Otherwise a new key will be built.
There was a problem hiding this comment.
I was also using the aws-sns-sqs pattern as a guideline, which does something similar.
There was a problem hiding this comment.
Upon further review, I'm inclined to agree with you. I believe it's the design guidelines that need to be updated.
| |deployDeadLetterQueue?|`boolean`|Whether to deploy a secondary queue to be used as a dead letter queue. Default `true`.| | ||
| |maxReceiveCount?|`number`|The number of times a message can be unsuccessfully dequeued before being moved to the dead-letter queue. Required field if `deployDeadLetterQueue`=`true`.| | ||
| |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/latest/docs/@aws-cdk_aws-kms.Key.html)|An optional, imported encryption key to encrypt the SQS queue, and SNS Topic.| |
There was a problem hiding this comment.
Cut paste error - SNS Topic?
There was a problem hiding this comment.
Yep, that was it. I'll remove the reference to SNS. Thanks!
…utions-constructs into feature/aws-iot-sqs
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| |deadLetterQueueProps?|[`sqs.QueueProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-sqs.QueueProps.html)|Optional user provided properties for the dead letter queue.| | ||
| |deployDeadLetterQueue?|`boolean`|Whether to deploy a secondary queue to be used as a dead letter queue. Default `true`.| | ||
| |maxReceiveCount?|`number`|The number of times a message can be unsuccessfully dequeued before being moved to the dead-letter queue. Required field if `deployDeadLetterQueue`=`true`.| | ||
| |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.| |
There was a problem hiding this comment.
Upon further review, I'm inclined to agree with you. I believe it's the design guidelines that need to be updated.
ericquinones
changed the title
fixes awslabs#266 Added implementation of new pattern Added unit and integration tests
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| test/*.js | ||
| *.d.ts | ||
| coverage | ||
| test/lambda/index.js |
There was a problem hiding this comment.
Can drop this, no lambda function in these tests.
| @@ -0,0 +1,16 @@ | |||
| lib/*.js | |||
| test/*.js | |||
| !test/lambda/* | |||
There was a problem hiding this comment.
Can drop this, no lambda function in these tests.
| * and limitations under the License. | ||
| */ | ||
|
|
||
| import { Queue, QueueProps, DeadLetterQueue } from '@aws-cdk/aws-sqs'; |
There was a problem hiding this comment.
We haven't codified conventions, and occasionally do inexplicable things, but in general imports from cdk get a namespace applied, eg
import * as sqs from '@aws-cdk/aws-sqs'
Let's do that here.
(FYI - inexplicable is lines 20 and 21 of this - that hurts to look at)
There was a problem hiding this comment.
Changed imports to:
import * as cdk from '@aws-cdk/core';
import * as sqs from '@aws-cdk/aws-sqs';
import * as iot from '@aws-cdk/aws-iot';
import * as kms from '@aws-cdk/aws-kms';
import * as iam from '@aws-cdk/aws-iam';
import * as defaults from '@aws-solutions-constructs/core';
Used defaults for @aws-solutions-constructs/core because I saw that used other places in the project.
| @@ -0,0 +1,101 @@ | |||
| # aws-iot-sqs module | |||
There was a problem hiding this comment.
I can't leave a comment on the binary png file that is your architecture diagram. It doesn't match our style, we use a different icon set (perhaps an older set, but it's where we are).
There was a problem hiding this comment.
Updated the architecture diagram. Used the SQS service icon instead of the Queue icon.
| * @param {cdk.App} scope - represents the scope for all the resources. | ||
| * @param {string} id - this is a a scope-unique id. | ||
| * @param {IotToSqsProps} props - user provided props for the construct | ||
| * @since 1.110.1 |
|
|
||
| // Creates a dead letter queue | ||
| expect(stack).toHaveResource("AWS::SQS::Queue", { | ||
| KmsMasterKeyId: "alias/aws/sqs" |
There was a problem hiding this comment.
???
This is the check you use when enableEncryptionWithCustomerManagedKey is false. This test is supply keyProps - shouldn't you be looking for the key alias in those props? And, if so, why does this test pass?
There was a problem hiding this comment.
If you pass enableEncryptionWithCustomerManagedKey=false, you are indicating that you don't want to use a customer managed key but the queue will still have the default encryption: AWS managed CMK for SQS (the alias for which is alias/aws/sqs). So that's why this test passes.
The test for enableEncryptionWithCustomerManagedKey=false similarly tests that the queues have the default SQS key but also specifically checks that a Key was not created
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
* Removed references to lambda from .gitignore and .eslintignore * Use service namespaces instead of named imports * Updated architecture diagram
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Initial PR for new pattern:
aws-iot-sqs: #266Description of changes:
READMEand architecture diagram have been approvedaws-iot-sqsBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.