From 6037b9f6dd0fc3aa072445cb731ba9f66b0841ba Mon Sep 17 00:00:00 2001 From: DaWyz Date: Sat, 21 Nov 2020 22:31:37 -0800 Subject: [PATCH 1/7] feat(aws-events): Add DLQ Configuration to Rule Targets --- .../@aws-cdk/aws-events-targets/lib/lambda.ts | 48 +++++++++++++++++++ .../test/lambda/integ.events2.ts | 35 ++++++++++++++ packages/@aws-cdk/aws-events/lib/rule.ts | 1 + packages/@aws-cdk/aws-events/lib/target.ts | 6 +++ 4 files changed, 90 insertions(+) create mode 100644 packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts diff --git a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts index 780cd6d57162c..44179695b789e 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts @@ -1,5 +1,8 @@ import * as events from '@aws-cdk/aws-events'; +import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; +import * as sqs from '@aws-cdk/aws-sqs'; +import { Names, Stack } from '@aws-cdk/core'; import { addLambdaPermission } from './util'; /** @@ -14,6 +17,13 @@ export interface LambdaFunctionProps { * @default the entire EventBridge event */ readonly event?: events.RuleTargetInput; + + /** + * The SQS Queue to be used as deadLetterQueue. + * + * @default none + */ + readonly deadLetterQueue?: sqs.IQueue; } /** @@ -32,11 +42,49 @@ export class LambdaFunction implements events.IRuleTarget { // Allow handler to be called from rule addLambdaPermission(rule, this.handler); + if (this.props.deadLetterQueue) { + addToDeadLetterQueueResourcePolicy(rule, this.props.deadLetterQueue); + } + return { id: '', arn: this.handler.functionArn, + deadLetterConfig: { arn: this.props.deadLetterQueue?.queueArn }, input: this.props.event, targetResource: this.handler, }; } } + + +function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sqs.IQueue) { + const ruleParsedStack = Stack.of(rule); + const queueParsedStack = Stack.of(queue); + + if (ruleParsedStack.region !== queueParsedStack.region) { + throw new Error(`Cannot assign Dead Letter Queue to the rule ${rule}. Both the queue and the rule must be in the same region`); + } + + // Skip Resource Policy creation if the Queue is not in the same account. + // There is no way to add a target onto an imported rule, so we can assume we will run the following code only + // where the rule is created. + + if (ruleParsedStack.account === queueParsedStack.account) { + const policyStatementId = `AllowEventRule${Names.nodeUniqueId(rule.node)}`; + + queue.addToResourcePolicy(new iam.PolicyStatement({ + sid: policyStatementId, + principals: [new iam.ServicePrincipal('events.amazonaws.com')], + effect: iam.Effect.ALLOW, + actions: ['sqs:SendMessage'], + resources: [queue.queueArn], + conditions: { + ArnEquals: { + 'aws:SourceArn': rule.ruleArn, + }, + }, + })); + } else { + // Maybe we could post a warning telling the user to create the permission in the target accout ? + } +} diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts new file mode 100644 index 0000000000000..c093b1d936c98 --- /dev/null +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts @@ -0,0 +1,35 @@ +import * as events from '@aws-cdk/aws-events'; +import * as lambda from '@aws-cdk/aws-lambda'; +import * as sqs from '@aws-cdk/aws-sqs'; +import * as cdk from '@aws-cdk/core'; +import * as targets from '../../lib'; + +const app = new cdk.App(); + +const stack = new cdk.Stack(app, 'lambda-events'); + +const fn = new lambda.Function(stack, 'MyFunc', { + runtime: lambda.Runtime.NODEJS_10_X, + handler: 'index.handler', + code: lambda.Code.fromInline(`exports.handler = ${handler.toString()}`), +}); + +const timer = new events.Rule(stack, 'Timer', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), +}); + +const queue = new sqs.Queue(stack, 'queue', { + queueName: 'MyQueue', +}); + +timer.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, +})); + +app.synth(); + +/* eslint-disable no-console */ +function handler(event: any, _context: any, callback: any) { + console.log(JSON.stringify(event, undefined, 2)); + return callback(); +} diff --git a/packages/@aws-cdk/aws-events/lib/rule.ts b/packages/@aws-cdk/aws-events/lib/rule.ts index 98e629874e382..04feddadb766f 100644 --- a/packages/@aws-cdk/aws-events/lib/rule.ts +++ b/packages/@aws-cdk/aws-events/lib/rule.ts @@ -295,6 +295,7 @@ export class Rule extends Resource implements IRule { kinesisParameters: targetProps.kinesisParameters, runCommandParameters: targetProps.runCommandParameters, batchParameters: targetProps.batchParameters, + deadLetterConfig: targetProps.deadLetterConfig, sqsParameters: targetProps.sqsParameters, input: inputProps && inputProps.input, inputPath: inputProps && inputProps.inputPath, diff --git a/packages/@aws-cdk/aws-events/lib/target.ts b/packages/@aws-cdk/aws-events/lib/target.ts index 319cf3d4f14da..da2ac9a1c7fb1 100644 --- a/packages/@aws-cdk/aws-events/lib/target.ts +++ b/packages/@aws-cdk/aws-events/lib/target.ts @@ -47,6 +47,12 @@ export interface RuleTargetConfig { */ readonly batchParameters?: CfnRule.BatchParametersProperty; + /** + * Contains information about a dead-letter queue configuration. + * @default no dead-letter queue set + */ + readonly deadLetterConfig?: CfnRule.DeadLetterConfigProperty; + /** * The Amazon ECS task definition and task count to use, if the event target * is an Amazon ECS task. From 4dfb84e1b7af85b66359b2c4c5a3ffa5c50fd1f6 Mon Sep 17 00:00:00 2001 From: DaWyz Date: Mon, 23 Nov 2020 21:59:48 -0800 Subject: [PATCH 2/7] Add tests and documentation + code cleanup --- .../@aws-cdk/aws-events-targets/README.md | 32 ++++++- .../@aws-cdk/aws-events-targets/lib/lambda.ts | 39 +-------- .../@aws-cdk/aws-events-targets/lib/util.ts | 36 +++++++- .../test/lambda/integ.events.expected.json | 87 +++++++++++++++++++ .../test/lambda/integ.events.ts | 12 +++ .../test/lambda/integ.events2.ts | 35 -------- .../test/lambda/lambda.test.ts | 47 ++++++++++ 7 files changed, 214 insertions(+), 74 deletions(-) delete mode 100644 packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts diff --git a/packages/@aws-cdk/aws-events-targets/README.md b/packages/@aws-cdk/aws-events-targets/README.md index c99441f05d32a..0364be1685674 100644 --- a/packages/@aws-cdk/aws-events-targets/README.md +++ b/packages/@aws-cdk/aws-events-targets/README.md @@ -29,7 +29,37 @@ Currently supported are: See the README of the `@aws-cdk/aws-events` library for more information on EventBridge. -## LogGroup +## Invoke a Lambda function + +Use the `LambdaFunction` target to invoke a lambda function. + +The code snippet below creates an event rule with a Lambda function as a target triggered for every events from `aws.ec2` source. You can optionally attach a [dead letter queue](https://docs.aws.amazon.com/eventbridge/latest/userguide/rule-dlq.html). + +```ts +import * as lambda from "@aws-cdk/aws-lambda"; +import * as events from "@aws-cdk/aws-events"; +import * as targets from "@aws-cdk/aws-events-targets"; + +const fn = new lambda.Function(stack, 'MyFunc', { + runtime: lambda.Runtime.NODEJS_10_X, + handler: 'index.handler', + code: lambda.Code.fromInline(`exports.handler = ${handler.toString()}`), +}); + +const rule = new events.Rule(this, 'rule', { + eventPattern: { + source: ["aws.ec2"], + }, +}); + +const queue = new sqs.Queue(stack, 'Queue'); + +rule.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, // Optional: add a dead letter queue +})); +``` + +## Log an event into a LogGroup Use the `LogGroup` target to log your events in a CloudWatch LogGroup. diff --git a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts index 44179695b789e..f1b0579b65fc2 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts @@ -1,9 +1,7 @@ import * as events from '@aws-cdk/aws-events'; -import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; import * as sqs from '@aws-cdk/aws-sqs'; -import { Names, Stack } from '@aws-cdk/core'; -import { addLambdaPermission } from './util'; +import { addLambdaPermission, addToDeadLetterQueueResourcePolicy } from './util'; /** * Customize the Lambda Event Target @@ -49,42 +47,9 @@ export class LambdaFunction implements events.IRuleTarget { return { id: '', arn: this.handler.functionArn, - deadLetterConfig: { arn: this.props.deadLetterQueue?.queueArn }, + deadLetterConfig: this.props.deadLetterQueue ? { arn: this.props.deadLetterQueue?.queueArn } : undefined, input: this.props.event, targetResource: this.handler, }; } } - - -function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sqs.IQueue) { - const ruleParsedStack = Stack.of(rule); - const queueParsedStack = Stack.of(queue); - - if (ruleParsedStack.region !== queueParsedStack.region) { - throw new Error(`Cannot assign Dead Letter Queue to the rule ${rule}. Both the queue and the rule must be in the same region`); - } - - // Skip Resource Policy creation if the Queue is not in the same account. - // There is no way to add a target onto an imported rule, so we can assume we will run the following code only - // where the rule is created. - - if (ruleParsedStack.account === queueParsedStack.account) { - const policyStatementId = `AllowEventRule${Names.nodeUniqueId(rule.node)}`; - - queue.addToResourcePolicy(new iam.PolicyStatement({ - sid: policyStatementId, - principals: [new iam.ServicePrincipal('events.amazonaws.com')], - effect: iam.Effect.ALLOW, - actions: ['sqs:SendMessage'], - resources: [queue.queueArn], - conditions: { - ArnEquals: { - 'aws:SourceArn': rule.ruleArn, - }, - }, - })); - } else { - // Maybe we could post a warning telling the user to create the permission in the target accout ? - } -} diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts index fe41154b6037c..3bc9fd616d8b6 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/util.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/util.ts @@ -1,7 +1,8 @@ import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; -import { Construct, ConstructNode, IConstruct, Names } from '@aws-cdk/core'; +import * as sqs from '@aws-cdk/aws-sqs'; +import { Construct, ConstructNode, IConstruct, Names, Stack } from '@aws-cdk/core'; /** * Obtain the Role for the EventBridge event @@ -45,3 +46,36 @@ export function addLambdaPermission(rule: events.IRule, handler: lambda.IFunctio }); } } + + +export function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sqs.IQueue) { + const ruleParsedStack = Stack.of(rule); + const queueParsedStack = Stack.of(queue); + + if (ruleParsedStack.region !== queueParsedStack.region) { + throw new Error(`Cannot assign Dead Letter Queue to the rule ${rule}. Both the queue and the rule must be in the same region`); + } + + // Skip Resource Policy creation if the Queue is not in the same account. + // There is no way to add a target onto an imported rule, so we can assume we will run the following code only + // in the account where the rule is created. + + if (ruleParsedStack.account === queueParsedStack.account) { + const policyStatementId = `AllowEventRule${Names.nodeUniqueId(rule.node)}`; + + queue.addToResourcePolicy(new iam.PolicyStatement({ + sid: policyStatementId, + principals: [new iam.ServicePrincipal('events.amazonaws.com')], + effect: iam.Effect.ALLOW, + actions: ['sqs:SendMessage'], + resources: [queue.queueArn], + conditions: { + ArnEquals: { + 'aws:SourceArn': rule.ruleArn, + }, + }, + })); + } else { + // Maybe we could post a warning telling the user to create the permission in the target account manually ? + } +} diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json index aad7c05f0bd5f..eadeb6c4ff834 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json @@ -123,6 +123,93 @@ ] } } + }, + "Timer30894E3BB": { + "Type": "AWS::Events::Rule", + "Properties": { + "ScheduleExpression": "rate(2 minutes)", + "State": "ENABLED", + "Targets": [ + { + "Arn": { + "Fn::GetAtt": [ + "MyFunc8A243A2C", + "Arn" + ] + }, + "DeadLetterConfig": { + "Arn": { + "Fn::GetAtt": [ + "Queue4A7E3555", + "Arn" + ] + } + }, + "Id": "Target0" + } + ] + } + }, + "Timer3AllowEventRulelambdaeventsTimer3107B9373B17EFAE0": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "MyFunc8A243A2C", + "Arn" + ] + }, + "Principal": "events.amazonaws.com", + "SourceArn": { + "Fn::GetAtt": [ + "Timer30894E3BB", + "Arn" + ] + } + } + }, + "Queue4A7E3555": { + "Type": "AWS::SQS::Queue" + }, + "QueuePolicy25439813": { + "Type": "AWS::SQS::QueuePolicy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "sqs:SendMessage", + "Condition": { + "ArnEquals": { + "aws:SourceArn": { + "Fn::GetAtt": [ + "Timer30894E3BB", + "Arn" + ] + } + } + }, + "Effect": "Allow", + "Principal": { + "Service": "events.amazonaws.com" + }, + "Resource": { + "Fn::GetAtt": [ + "Queue4A7E3555", + "Arn" + ] + }, + "Sid": "AllowEventRulelambdaeventsTimer3107B9373" + } + ], + "Version": "2012-10-17" + }, + "Queues": [ + { + "Ref": "Queue4A7E3555" + } + ] + } } } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts index 17d17a291e244..c37c632d803ae 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts @@ -1,5 +1,6 @@ import * as events from '@aws-cdk/aws-events'; import * as lambda from '@aws-cdk/aws-lambda'; +import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; import * as targets from '../../lib'; @@ -23,6 +24,17 @@ const timer2 = new events.Rule(stack, 'Timer2', { }); timer2.addTarget(new targets.LambdaFunction(fn)); + +const timer3 = new events.Rule(stack, 'Timer3', { + schedule: events.Schedule.rate(cdk.Duration.minutes(2)), +}); + +const queue = new sqs.Queue(stack, 'Queue'); + +timer3.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, +})); + app.synth(); /* eslint-disable no-console */ diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts deleted file mode 100644 index c093b1d936c98..0000000000000 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events2.ts +++ /dev/null @@ -1,35 +0,0 @@ -import * as events from '@aws-cdk/aws-events'; -import * as lambda from '@aws-cdk/aws-lambda'; -import * as sqs from '@aws-cdk/aws-sqs'; -import * as cdk from '@aws-cdk/core'; -import * as targets from '../../lib'; - -const app = new cdk.App(); - -const stack = new cdk.Stack(app, 'lambda-events'); - -const fn = new lambda.Function(stack, 'MyFunc', { - runtime: lambda.Runtime.NODEJS_10_X, - handler: 'index.handler', - code: lambda.Code.fromInline(`exports.handler = ${handler.toString()}`), -}); - -const timer = new events.Rule(stack, 'Timer', { - schedule: events.Schedule.rate(cdk.Duration.minutes(1)), -}); - -const queue = new sqs.Queue(stack, 'queue', { - queueName: 'MyQueue', -}); - -timer.addTarget(new targets.LambdaFunction(fn, { - deadLetterQueue: queue, -})); - -app.synth(); - -/* eslint-disable no-console */ -function handler(event: any, _context: any, callback: any) { - console.log(JSON.stringify(event, undefined, 2)); - return callback(); -} diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts index 28df0932c9ba4..75d1baca0c8c0 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts @@ -1,6 +1,7 @@ import '@aws-cdk/assert/jest'; import * as events from '@aws-cdk/aws-events'; import * as lambda from '@aws-cdk/aws-lambda'; +import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; import * as constructs from 'constructs'; import * as targets from '../../lib'; @@ -126,6 +127,52 @@ test('lambda handler and cloudwatch event across stacks', () => { expect(eventStack).toCountResources('AWS::Lambda::Permission', 1); }); +test('use a Dead Letter Queue for the rule target', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'Stack'); + + const fn = new lambda.Function(stack, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + }); + + const queue = new sqs.Queue(stack, 'Queue'); + + new events.Rule(stack, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + targets: [new targets.LambdaFunction(fn, { + deadLetterQueue: queue, + })], + }); + + expect(() => app.synth()).not.toThrow(); + + // the Permission resource should be in the event stack + expect(stack).toHaveResource('AWS::Events::Rule', { + Targets: [ + { + Arn: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + DeadLetterConfig: { + Arn: { + 'Fn::GetAtt': [ + 'Queue4A7E3555', + 'Arn', + ], + }, + }, + Id: 'Target0', + }, + ], + }); +}); + function newTestLambda(scope: constructs.Construct) { return new lambda.Function(scope, 'MyLambda', { code: new lambda.InlineCode('foo'), From 4155dcff70d593fa17375be34b3c8dd752028f53 Mon Sep 17 00:00:00 2001 From: DaWyz Date: Wed, 2 Dec 2020 22:47:11 -0800 Subject: [PATCH 3/7] Fix nits and add warning --- .../@aws-cdk/aws-events-targets/README.md | 4 +- .../@aws-cdk/aws-events-targets/lib/lambda.ts | 9 +- .../@aws-cdk/aws-events-targets/lib/util.ts | 11 +- .../test/lambda/lambda.test.ts | 131 ++++++++++++++++++ 4 files changed, 147 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/README.md b/packages/@aws-cdk/aws-events-targets/README.md index 0364be1685674..f504c34ffea06 100644 --- a/packages/@aws-cdk/aws-events-targets/README.md +++ b/packages/@aws-cdk/aws-events-targets/README.md @@ -40,7 +40,7 @@ import * as lambda from "@aws-cdk/aws-lambda"; import * as events from "@aws-cdk/aws-events"; import * as targets from "@aws-cdk/aws-events-targets"; -const fn = new lambda.Function(stack, 'MyFunc', { +const fn = new lambda.Function(this, 'MyFunc', { runtime: lambda.Runtime.NODEJS_10_X, handler: 'index.handler', code: lambda.Code.fromInline(`exports.handler = ${handler.toString()}`), @@ -52,7 +52,7 @@ const rule = new events.Rule(this, 'rule', { }, }); -const queue = new sqs.Queue(stack, 'Queue'); +const queue = new sqs.Queue(this, 'Queue'); rule.addTarget(new targets.LambdaFunction(fn, { deadLetterQueue: queue, // Optional: add a dead letter queue diff --git a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts index f1b0579b65fc2..44315579a1300 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts @@ -17,9 +17,14 @@ export interface LambdaFunctionProps { readonly event?: events.RuleTargetInput; /** - * The SQS Queue to be used as deadLetterQueue. + * The SQS queue to be used as deadLetterQueue. + * Check out the [considerations for using a dead-letter queue](https://docs.aws.amazon.com/eventbridge/latest/userguide/rule-dlq.html#dlq-considerations). * - * @default none + * The events not successfully delivered are automatically retried for a specified period of time, + * depending on the retry policy of the target. + * If an event is not delivered before all retry attempts are exhausted, it will be sent to the dead letter queue. + * + * @default - no dead-letter queue */ readonly deadLetterQueue?: sqs.IQueue; } diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts index 3bc9fd616d8b6..b3274e323c02f 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/util.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/util.ts @@ -2,7 +2,7 @@ import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; import * as sqs from '@aws-cdk/aws-sqs'; -import { Construct, ConstructNode, IConstruct, Names, Stack } from '@aws-cdk/core'; +import { Annotations, Construct, ConstructNode, IConstruct, Names, Stack } from '@aws-cdk/core'; /** * Obtain the Role for the EventBridge event @@ -48,18 +48,21 @@ export function addLambdaPermission(rule: events.IRule, handler: lambda.IFunctio } +/** + * Allow a rule to send events with failed invocation to an Amazon SQS queue. + */ export function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sqs.IQueue) { const ruleParsedStack = Stack.of(rule); const queueParsedStack = Stack.of(queue); + if (ruleParsedStack.region !== queueParsedStack.region) { - throw new Error(`Cannot assign Dead Letter Queue to the rule ${rule}. Both the queue and the rule must be in the same region`); + throw new Error(`Cannot assign Dead Letter Queue in region ${queueParsedStack.region} to the rule ${Names.nodeUniqueId(rule.node)} in region ${ruleParsedStack.region}. Both the queue and the rule must be in the same region.`); } // Skip Resource Policy creation if the Queue is not in the same account. // There is no way to add a target onto an imported rule, so we can assume we will run the following code only // in the account where the rule is created. - if (ruleParsedStack.account === queueParsedStack.account) { const policyStatementId = `AllowEventRule${Names.nodeUniqueId(rule.node)}`; @@ -76,6 +79,6 @@ export function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sq }, })); } else { - // Maybe we could post a warning telling the user to create the permission in the target account manually ? + Annotations.of(rule).addWarning(`Cannot add a resource policy to your dead letter queue associated with rule ${rule.ruleName} because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account ${queueParsedStack.account}.`); } } diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts index 75d1baca0c8c0..a5018607dbaa5 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts @@ -171,6 +171,137 @@ test('use a Dead Letter Queue for the rule target', () => { }, ], }); + + expect(stack).toHaveResource('AWS::SQS::QueuePolicy', { + PolicyDocument: { + Statement: [ + { + Action: 'sqs:SendMessage', + Condition: { + ArnEquals: { + 'aws:SourceArn': { + 'Fn::GetAtt': [ + 'Rule4C995B7F', + 'Arn', + ], + }, + }, + }, + Effect: 'Allow', + Principal: { + Service: 'events.amazonaws.com', + }, + Resource: { + 'Fn::GetAtt': [ + 'Queue4A7E3555', + 'Arn', + ], + }, + Sid: 'AllowEventRuleStackRuleF6E31DD0', + }, + ], + Version: '2012-10-17', + }, + Queues: [ + { + Ref: 'Queue4A7E3555', + }, + ], + }); +}); + +test('throw an error when using a Dead Letter Queue for the rule target in a different region', () => { + // GIVEN + const app = new cdk.App(); + const stack1 = new cdk.Stack(app, 'Stack1', { + env: { + region: 'eu-west-1', + }, + }); + const stack2 = new cdk.Stack(app, 'Stack2', { + env: { + region: 'eu-west-2', + }, + }); + + const fn = new lambda.Function(stack1, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + }); + + const queue = new sqs.Queue(stack2, 'Queue'); + + let rule = new events.Rule(stack1, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + }); + + + expect(() => { + rule.addTarget(new targets.LambdaFunction(fn, { + deadLetterQueue: queue, + })); + }).toThrow(/Cannot assign Dead Letter Queue in region eu-west-2 to the rule Stack1Rule92BA1111 in region eu-west-1. Both the queue and the rule must be in the same region./); +}); + +test('must display a warning when using a Dead Letter Queue from another account', () => { + // GIVEN + const app = new cdk.App(); + const stack1 = new cdk.Stack(app, 'Stack1', { + env: { + region: 'eu-west-1', + account: '111111111111', + }, + }); + + const stack2 = new cdk.Stack(app, 'Stack2', { + env: { + region: 'eu-west-1', + account: '222222222222', + }, + }); + + const fn = new lambda.Function(stack1, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + }); + + const queue = sqs.Queue.fromQueueArn(stack2, 'Queue', 'arn:aws:sqs:eu-west-1:444455556666:queue1'); + + new events.Rule(stack1, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + targets: [new targets.LambdaFunction(fn, { + deadLetterQueue: queue, + })], + }); + + expect(() => app.synth()).not.toThrow(); + + // the Permission resource should be in the event stack + expect(stack1).toHaveResource('AWS::Events::Rule', { + ScheduleExpression: 'rate(1 minute)', + State: 'ENABLED', + Targets: [ + { + Arn: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + DeadLetterConfig: { + Arn: 'arn:aws:sqs:eu-west-1:444455556666:queue1', + }, + Id: 'Target0', + }, + ], + }); + + expect(stack1).not.toHaveResource('AWS::SQS::QueuePolicy'); + + let rule = stack1.node.children.find(child => child instanceof events.Rule); + expect(rule?.node.metadata[0].data).toEqual('Cannot add a resource policy to your dead letter queue associated with rule ${Token[TOKEN.251]} because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account 222222222222.'); }); function newTestLambda(scope: constructs.Construct) { From ac9c2617f62127a5ef5bc2cf85360366d426a77b Mon Sep 17 00:00:00 2001 From: DaWyz Date: Wed, 13 Jan 2021 22:19:18 -0800 Subject: [PATCH 4/7] Fixing nits in aws-events-targets README.md + failing test --- packages/@aws-cdk/aws-events-targets/README.md | 7 +++++-- .../@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/README.md b/packages/@aws-cdk/aws-events-targets/README.md index f0f9f3d6a31b5..4ff1b5d702a61 100644 --- a/packages/@aws-cdk/aws-events-targets/README.md +++ b/packages/@aws-cdk/aws-events-targets/README.md @@ -35,15 +35,18 @@ EventBridge. Use the `LambdaFunction` target to invoke a lambda function. -The code snippet below creates an event rule with a Lambda function as a target triggered for every events from `aws.ec2` source. You can optionally attach a [dead letter queue](https://docs.aws.amazon.com/eventbridge/latest/userguide/rule-dlq.html). +The code snippet below creates an event rule with a Lambda function as a target +triggered for every events from `aws.ec2` source. You can optionally attach a +[dead letter queue](https://docs.aws.amazon.com/eventbridge/latest/userguide/rule-dlq.html). ```ts import * as lambda from "@aws-cdk/aws-lambda"; import * as events from "@aws-cdk/aws-events"; +import * as sqs from "@aws-cdk/aws-sqs"; import * as targets from "@aws-cdk/aws-events-targets"; const fn = new lambda.Function(this, 'MyFunc', { - runtime: lambda.Runtime.NODEJS_10_X, + runtime: lambda.Runtime.NODEJS_12_X, handler: 'index.handler', code: lambda.Code.fromInline(`exports.handler = ${handler.toString()}`), }); diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts index a5018607dbaa5..b1f89719f713d 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts @@ -301,7 +301,7 @@ test('must display a warning when using a Dead Letter Queue from another account expect(stack1).not.toHaveResource('AWS::SQS::QueuePolicy'); let rule = stack1.node.children.find(child => child instanceof events.Rule); - expect(rule?.node.metadata[0].data).toEqual('Cannot add a resource policy to your dead letter queue associated with rule ${Token[TOKEN.251]} because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account 222222222222.'); + expect(rule?.node.metadata[0].data).toMatch(/Cannot add a resource policy to your dead letter queue associated with rule .* because the queue is in a different account\. You must add the resource policy manually to the dead letter queue in account 222222222222\./); }); function newTestLambda(scope: constructs.Construct) { From 0c2286f6033c2be4b36566c6fc16e49fc8442c7b Mon Sep 17 00:00:00 2001 From: DaWyz Date: Tue, 9 Feb 2021 19:16:36 -0800 Subject: [PATCH 5/7] Update env comparison to account for tokens --- .../@aws-cdk/aws-events-targets/lib/util.ts | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts index b3274e323c02f..a352dbc5096bd 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/util.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/util.ts @@ -2,7 +2,11 @@ import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; import * as sqs from '@aws-cdk/aws-sqs'; -import { Annotations, Construct, ConstructNode, IConstruct, Names, Stack } from '@aws-cdk/core'; +import { Annotations, ConstructNode, IConstruct, Names, Token, TokenComparison } from '@aws-cdk/core'; + +// keep this import separate from other imports to reduce chance for merge conflicts with v2-main +// eslint-disable-next-line no-duplicate-imports, import/order +import { Construct } from '@aws-cdk/core'; /** * Obtain the Role for the EventBridge event @@ -47,23 +51,18 @@ export function addLambdaPermission(rule: events.IRule, handler: lambda.IFunctio } } - /** * Allow a rule to send events with failed invocation to an Amazon SQS queue. */ export function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sqs.IQueue) { - const ruleParsedStack = Stack.of(rule); - const queueParsedStack = Stack.of(queue); - - - if (ruleParsedStack.region !== queueParsedStack.region) { - throw new Error(`Cannot assign Dead Letter Queue in region ${queueParsedStack.region} to the rule ${Names.nodeUniqueId(rule.node)} in region ${ruleParsedStack.region}. Both the queue and the rule must be in the same region.`); + if (!sameEnvDimension(rule.env.region, queue.env.region)) { + throw new Error(`Cannot assign Dead Letter Queue in region ${queue.env.region} to the rule ${Names.nodeUniqueId(rule.node)} in region ${rule.env.region}. Both the queue and the rule must be in the same region.`); } // Skip Resource Policy creation if the Queue is not in the same account. // There is no way to add a target onto an imported rule, so we can assume we will run the following code only // in the account where the rule is created. - if (ruleParsedStack.account === queueParsedStack.account) { + if (sameEnvDimension(rule.env.account, queue.env.account)) { const policyStatementId = `AllowEventRule${Names.nodeUniqueId(rule.node)}`; queue.addToResourcePolicy(new iam.PolicyStatement({ @@ -79,6 +78,17 @@ export function addToDeadLetterQueueResourcePolicy(rule: events.IRule, queue: sq }, })); } else { - Annotations.of(rule).addWarning(`Cannot add a resource policy to your dead letter queue associated with rule ${rule.ruleName} because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account ${queueParsedStack.account}.`); + Annotations.of(rule).addWarning(`Cannot add a resource policy to your dead letter queue associated with rule ${rule.ruleName} because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account ${queue.env.account}.`); } } + + +/** + * Whether two string probably contain the same environment dimension (region or account) + * + * Used to compare either accounts or regions, and also returns true if both + * are unresolved (in which case both are expted to be "current region" or "current account"). + */ +function sameEnvDimension(dim1: string, dim2: string) { + return [TokenComparison.SAME, TokenComparison.BOTH_UNRESOLVED].includes(Token.compareStrings(dim1, dim2)); +} \ No newline at end of file From 59b45f569d146235850de237a4d1a9d22809ddad Mon Sep 17 00:00:00 2001 From: DaWyz Date: Sun, 28 Feb 2021 21:16:15 -0800 Subject: [PATCH 6/7] update integration test to fix build --- .../aws-events-targets/test/lambda/integ.events.expected.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json index 51067f78725e4..ffc2296b65605 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json @@ -150,7 +150,7 @@ ] } }, - "Timer3AllowEventRulelambdaeventsTimer3107B9373B17EFAE0": { + "Timer3AllowEventRulelambdaeventsMyFunc910E580F79317F73": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", From c51f234c43981d8c4aaf8d5fd6d911668f1f421e Mon Sep 17 00:00:00 2001 From: DaWyz Date: Tue, 2 Mar 2021 19:53:30 -0800 Subject: [PATCH 7/7] fixing integ test --- .../aws-events-targets/test/lambda/integ.events.expected.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json index ffc2296b65605..7df2a4becf021 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json @@ -170,7 +170,9 @@ } }, "Queue4A7E3555": { - "Type": "AWS::SQS::Queue" + "Type": "AWS::SQS::Queue", + "UpdateReplacePolicy": "Delete", + "DeletionPolicy": "Delete" }, "QueuePolicy25439813": { "Type": "AWS::SQS::QueuePolicy",