From 302872a365e67218e5bbecb12e01b97a79a9d3a2 Mon Sep 17 00:00:00 2001 From: Madeline Kusters Date: Mon, 10 May 2021 17:06:49 -0700 Subject: [PATCH 1/2] fix(events-targets): remove circular dependency when adding an SQS Queue encrypted with KMS as a target of an AWS Events Rule fixes #11158 --- .../@aws-cdk/aws-events-targets/lib/sqs.ts | 14 ++-- .../@aws-cdk/aws-events-targets/package.json | 2 + .../integ.sqs-event-rule-target.expected.json | 66 ++++++++++++++++--- .../test/sqs/integ.sqs-event-rule-target.ts | 9 ++- 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/lib/sqs.ts b/packages/@aws-cdk/aws-events-targets/lib/sqs.ts index 8d711b4b9f5be..b7cc164a34fa9 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/sqs.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/sqs.ts @@ -52,14 +52,18 @@ export class SqsQueue implements events.IRuleTarget { * @see https://docs.aws.amazon.com/eventbridge/latest/userguide/resource-based-policies-eventbridge.html#sqs-permissions */ public bind(rule: events.IRule, _id?: string): events.RuleTargetConfig { - // deduplicated automatically - this.queue.grantSendMessages(new iam.ServicePrincipal('events.amazonaws.com', - { + // Only add the rule as a condition if the queue is not encrypted, to avoid circular dependency. See issue #11158. + var servicePrincipalOpts:iam.ServicePrincipalOpts = {}; + if (this.queue.encryptionMasterKey == null) { + servicePrincipalOpts = { conditions: { ArnEquals: { 'aws:SourceArn': rule.ruleArn }, }, - }), - ); + }; + } + + // deduplicated automatically + this.queue.grantSendMessages(new iam.ServicePrincipal('events.amazonaws.com', servicePrincipalOpts)); return { arn: this.queue.queueArn, diff --git a/packages/@aws-cdk/aws-events-targets/package.json b/packages/@aws-cdk/aws-events-targets/package.json index e36df0baf5c71..f262d5897aa5a 100644 --- a/packages/@aws-cdk/aws-events-targets/package.json +++ b/packages/@aws-cdk/aws-events-targets/package.json @@ -93,6 +93,7 @@ "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/aws-kinesis": "0.0.0", "@aws-cdk/aws-kinesisfirehose": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", "@aws-cdk/aws-logs": "0.0.0", "@aws-cdk/aws-sns": "0.0.0", @@ -114,6 +115,7 @@ "@aws-cdk/aws-iam": "0.0.0", "@aws-cdk/aws-kinesis": "0.0.0", "@aws-cdk/aws-kinesisfirehose": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", "@aws-cdk/aws-logs": "0.0.0", "@aws-cdk/aws-sns": "0.0.0", diff --git a/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json b/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json index eebbc3a996344..eb2a7dd26ef5f 100644 --- a/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json @@ -1,5 +1,53 @@ { "Resources": { + "MyKey6AB29FA6": { + "Type": "AWS::KMS::Key", + "Properties": { + "KeyPolicy": { + "Statement": [ + { + "Action": "kms:*", + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::", + { + "Ref": "AWS::AccountId" + }, + ":root" + ] + ] + } + }, + "Resource": "*" + }, + { + "Action": [ + "kms:Decrypt", + "kms:Encrypt", + "kms:ReEncrypt*", + "kms:GenerateDataKey*" + ], + "Effect": "Allow", + "Principal": { + "Service": "events.amazonaws.com" + }, + "Resource": "*" + } + ], + "Version": "2012-10-17" + } + }, + "UpdateReplacePolicy": "Retain", + "DeletionPolicy": "Retain" + }, "MyRuleA44AB831": { "Type": "AWS::Events::Rule", "Properties": { @@ -20,6 +68,14 @@ }, "MyQueueE6CA6235": { "Type": "AWS::SQS::Queue", + "Properties": { + "KmsMasterKeyId": { + "Fn::GetAtt": [ + "MyKey6AB29FA6", + "Arn" + ] + } + }, "UpdateReplacePolicy": "Delete", "DeletionPolicy": "Delete" }, @@ -34,16 +90,6 @@ "sqs:GetQueueAttributes", "sqs:GetQueueUrl" ], - "Condition": { - "ArnEquals": { - "aws:SourceArn": { - "Fn::GetAtt": [ - "MyRuleA44AB831", - "Arn" - ] - } - } - }, "Effect": "Allow", "Principal": { "Service": "events.amazonaws.com" diff --git a/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.ts b/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.ts index b58641f727d03..b2b8fb334bff6 100644 --- a/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.ts +++ b/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.ts @@ -1,4 +1,5 @@ import * as events from '@aws-cdk/aws-events'; +import * as kms from '@aws-cdk/aws-kms'; import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; import * as targets from '../../lib'; @@ -12,11 +13,17 @@ const app = new cdk.App(); const stack = new cdk.Stack(app, 'aws-cdk-sqs-event-target'); +const key = new kms.Key(stack, 'MyKey'); + const event = new events.Rule(stack, 'MyRule', { schedule: events.Schedule.rate(cdk.Duration.minutes(1)), }); -const queue = new sqs.Queue(stack, 'MyQueue'); +const queue = new sqs.Queue(stack, 'MyQueue', { + encryption: sqs.QueueEncryption.KMS, + encryptionMasterKey: key, +}); + event.addTarget(new targets.SqsQueue(queue)); app.synth(); From 3bcd331cde7a6aa59ee26ad521820891ccb61fb5 Mon Sep 17 00:00:00 2001 From: Madeline Kusters Date: Wed, 12 May 2021 09:28:24 -0700 Subject: [PATCH 2/2] Incorporate feedback from njlynch --- packages/@aws-cdk/aws-events-targets/lib/sqs.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/aws-events-targets/lib/sqs.ts b/packages/@aws-cdk/aws-events-targets/lib/sqs.ts index b7cc164a34fa9..501414ecee348 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/sqs.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/sqs.ts @@ -53,17 +53,14 @@ export class SqsQueue implements events.IRuleTarget { */ public bind(rule: events.IRule, _id?: string): events.RuleTargetConfig { // Only add the rule as a condition if the queue is not encrypted, to avoid circular dependency. See issue #11158. - var servicePrincipalOpts:iam.ServicePrincipalOpts = {}; - if (this.queue.encryptionMasterKey == null) { - servicePrincipalOpts = { - conditions: { - ArnEquals: { 'aws:SourceArn': rule.ruleArn }, - }, - }; - } + const principalOpts = this.queue.encryptionMasterKey ? {} : { + conditions: { + ArnEquals: { 'aws:SourceArn': rule.ruleArn }, + }, + }; // deduplicated automatically - this.queue.grantSendMessages(new iam.ServicePrincipal('events.amazonaws.com', servicePrincipalOpts)); + this.queue.grantSendMessages(new iam.ServicePrincipal('events.amazonaws.com', principalOpts)); return { arn: this.queue.queueArn,