-
Notifications
You must be signed in to change notification settings - Fork 249
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(aww-eventbridge-sqs): add a dlq for the event rule #1253
Changes from 10 commits
4613c8e
9ea4800
b7d4ae1
6ffc429
cd1d571
628acd8
7bfdaca
4d409e1
e705f3b
3af5e89
99082a4
e99be92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ | |
|
||
import * as sqs from 'aws-cdk-lib/aws-sqs'; | ||
import * as events from 'aws-cdk-lib/aws-events'; | ||
import * as eventtargets from 'aws-cdk-lib/aws-events-targets'; | ||
import * as kms from 'aws-cdk-lib/aws-kms'; | ||
import * as defaults from '@aws-solutions-constructs/core'; | ||
import { ServicePrincipal } from 'aws-cdk-lib/aws-iam'; | ||
|
@@ -42,6 +43,20 @@ export interface EventbridgeToSqsProps { | |
* @default - None | ||
*/ | ||
readonly eventRuleProps: events.RuleProps; | ||
/** | ||
* Whether to deploy a DLQ for the Event Rule. If set to `true`, this DLQ will | ||
* receive any messages that can't be delivered to the target SQS queue. | ||
* | ||
* @default - false | ||
*/ | ||
readonly deployEventRuleDlq?: boolean; | ||
/** | ||
* Properties to define the key created to protect the ruleDlq | ||
* Only valid if deployRuleDlq is set to true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment on L55 has the old |
||
* | ||
* @default - default props are used | ||
*/ | ||
readonly eventRuleDlqKeyProps?: kms.KeyProps; | ||
/** | ||
* Existing instance of SQS queue object, providing both this and queueProps will cause an error. | ||
* | ||
|
@@ -60,6 +75,16 @@ export interface EventbridgeToSqsProps { | |
* @default - "false", disabled by default. | ||
*/ | ||
readonly enableQueuePurging?: boolean; | ||
/** | ||
* Optional user provided properties to define the SQS target on the Event Rule | ||
* | ||
* If you specify a deadLetterQueue for the rule here, you are responsible for adding a resource policy | ||
* to the queue allowing events.amazonaws.com permission to SendMessage, GetQueueUrl and GetQueueAttributes. You | ||
* cannot send a DLQ in this property and set deployRuleDlq to true | ||
* | ||
* @default - undefined (all default values are used) | ||
*/ | ||
readonly targetProps?: eventtargets.SqsQueueProps; | ||
/** | ||
* Optional user provided properties for the dead letter queue | ||
* | ||
|
@@ -105,6 +130,8 @@ export class EventbridgeToSqs extends Construct { | |
public readonly eventBus?: events.IEventBus; | ||
public readonly eventsRule: events.Rule; | ||
public readonly encryptionKey?: kms.IKey; | ||
public readonly eventRuleDlq?: sqs.Queue; | ||
public readonly eventRuleDlqKey?: kms.IKey; | ||
|
||
/** | ||
* @summary Constructs a new instance of the EventbridgeToSqs class. | ||
|
@@ -118,6 +145,14 @@ export class EventbridgeToSqs extends Construct { | |
super(scope, id); | ||
defaults.CheckSqsProps(props); | ||
defaults.CheckEventBridgeProps(props); | ||
// SqsQueueProps does not implement any common interface, so is unique to this construct, | ||
// so we will check it here rather than in core | ||
if ((props.targetProps?.deadLetterQueue) && (props.deployEventRuleDlq)) { | ||
throw new Error('Cannot specify both targetProps.deadLetterQueue and deployDeadLetterQueue == true\n'); | ||
} | ||
if (props.eventRuleDlqKeyProps && !props.deployEventRuleDlq) { | ||
throw new Error('Cannot specify eventRuleDlqKeyProps without setting deployEventRuleDlq=true\n'); | ||
} | ||
|
||
let enableEncryptionParam = props.enableEncryptionWithCustomerManagedKey; | ||
if (props.enableEncryptionWithCustomerManagedKey === undefined || | ||
|
@@ -140,12 +175,28 @@ export class EventbridgeToSqs extends Construct { | |
this.encryptionKey = buildQueueResponse.key; | ||
this.deadLetterQueue = buildQueueResponse.dlq; | ||
|
||
const sqsEventTarget: events.IRuleTarget = { | ||
bind: () => ({ | ||
id: this.sqsQueue.queueName, | ||
arn: this.sqsQueue.queueArn | ||
}) | ||
}; | ||
let constructEventTargetProps: eventtargets.SqsQueueProps = {}; | ||
|
||
if (defaults.CheckBooleanWithDefault(props.deployEventRuleDlq, false)) { | ||
|
||
const buildRuleDlqResponse = defaults.buildQueue(this, 'ruleDlq', { | ||
deployDeadLetterQueue: false, | ||
enableEncryptionWithCustomerManagedKey: enableEncryptionParam, | ||
encryptionKeyProps: props.eventRuleDlqKeyProps | ||
}); | ||
|
||
this.eventRuleDlq = buildRuleDlqResponse.queue; | ||
const ruleDlqKey = buildRuleDlqResponse.key; | ||
ruleDlqKey?.grantEncryptDecrypt(new ServicePrincipal('events.amazonaws.com')); | ||
this.eventRuleDlqKey = ruleDlqKey; | ||
|
||
// TODO: Update docs on encrpytWithCustomerMasterKey | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking, did we want to keep this TODO in here for future or were we supposed to action on this as part of this PR? |
||
|
||
constructEventTargetProps = defaults.consolidateProps(constructEventTargetProps, { deadLetterQueue: this.eventRuleDlq }); | ||
} | ||
|
||
const sqsEventTargetProps = defaults.consolidateProps({}, props.targetProps, constructEventTargetProps); | ||
const sqsEventTarget = new eventtargets.SqsQueue(this.sqsQueue, sqsEventTargetProps); | ||
|
||
// build an event bus if existingEventBus is provided or eventBusProps are provided | ||
this.eventBus = defaults.buildEventBus(this, { | ||
|
@@ -163,7 +214,5 @@ export class EventbridgeToSqs extends Construct { | |
this.sqsQueue.grantPurge(new ServicePrincipal('events.amazonaws.com')); | ||
} | ||
|
||
// Policy for event to be able to send messages to the queue and Grant Event Bridge service access to the SQS queue encryption key | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's allowing events to be sent to the queue now without this in there anymore? (Mostly curious but wanted to point it out just in case too) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a new aws-event-targets library since we first wrote this, the SqsQueue target object adds the permissions automatically. It doesn't add Purge however. |
||
this.sqsQueue.grantSendMessages(new ServicePrincipal('events.amazonaws.com')); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{"version":"39.0.0"} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
{ | ||
"version": "39.0.0", | ||
"files": { | ||
"8221310775997696fa75dec2375830f8d37a0d9d424ea6d55a03f453d4bec94c": { | ||
"source": { | ||
"path": "evtsqs-custom-target.template.json", | ||
"packaging": "file" | ||
}, | ||
"destinations": { | ||
"current_account-current_region": { | ||
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", | ||
"objectKey": "8221310775997696fa75dec2375830f8d37a0d9d424ea6d55a03f453d4bec94c.json", | ||
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" | ||
} | ||
} | ||
} | ||
}, | ||
"dockerImages": {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: There's a typo here in the middle of 118, should be: