From a8ac13bd317fb753111ef5e78b1d69a62afc2458 Mon Sep 17 00:00:00 2001 From: Jonne Kaunisto Date: Wed, 20 Jul 2022 22:52:58 +0000 Subject: [PATCH 1/3] fix(sns): add dependency to sqs policy for sns subscription --- packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts | 13 ++++++++++--- .../aws-cdk-sns-sqs.template.json | 1 + packages/@aws-cdk/aws-sns/lib/subscriber.ts | 11 ++++++++++- packages/@aws-cdk/aws-sns/lib/topic-base.ts | 13 ++++++++++--- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts index 3d9acff2d2a65..d61939bad636b 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts @@ -3,7 +3,7 @@ import * as sns from '@aws-cdk/aws-sns'; import * as sqs from '@aws-cdk/aws-sqs'; import { ArnFormat, FeatureFlags, Names, Stack, Token } from '@aws-cdk/core'; import * as cxapi from '@aws-cdk/cx-api'; -import { Construct } from 'constructs'; +import { Construct, IDependable } from 'constructs'; import { SubscriptionProps } from './subscription'; /** @@ -40,14 +40,20 @@ export class SqsSubscription implements sns.ITopicSubscription { // add a statement to the queue resource policy which allows this topic // to send messages to the queue. - this.queue.addToResourcePolicy(new iam.PolicyStatement({ + const queuePolicyDependable = this.queue.addToResourcePolicy(new iam.PolicyStatement({ resources: [this.queue.queueArn], actions: ['sqs:SendMessage'], principals: [snsServicePrincipal], conditions: { ArnEquals: { 'aws:SourceArn': topic.topicArn }, }, - })); + })).policyDependable; + + // Add the subscription policy as a dependency for the subscription + const subscriptionDependencies: IDependable[] = []; + if (queuePolicyDependable) { + subscriptionDependencies.push(queuePolicyDependable); + } // if the queue is encrypted, add a statement to the key resource policy // which allows this topic to decrypt KMS keys @@ -77,6 +83,7 @@ export class SqsSubscription implements sns.ITopicSubscription { filterPolicy: this.props.filterPolicy, region: this.regionFromArn(topic), deadLetterQueue: this.props.deadLetterQueue, + subscriptionDependencies, }; } diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json b/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json index 20a1f73ef916f..99b7fbcf761d2 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json @@ -53,6 +53,7 @@ }, "MyQueueawscdksnssqsMyTopic9361DEA223429051": { "Type": "AWS::SNS::Subscription", + "DependsOn" : "MyQueuePolicy6BBEDDAC", "Properties": { "Protocol": "sqs", "TopicArn": { diff --git a/packages/@aws-cdk/aws-sns/lib/subscriber.ts b/packages/@aws-cdk/aws-sns/lib/subscriber.ts index a52ee834cab11..87a84861d9a77 100644 --- a/packages/@aws-cdk/aws-sns/lib/subscriber.ts +++ b/packages/@aws-cdk/aws-sns/lib/subscriber.ts @@ -1,4 +1,4 @@ -import { Construct } from 'constructs'; +import { Construct, IDependable } from 'constructs'; import { SubscriptionOptions } from './subscription'; import { ITopic } from './topic-base'; @@ -24,6 +24,15 @@ export interface TopicSubscriptionConfig extends SubscriptionOptions { * subscribing to. */ readonly subscriberId: string; + + /** + * The resources that need to be created before the subscription can be safely created. + * For example for SQS subscription, the subscription needs to have a dependency on the SQS queue policy + * in order for the subscription to successfully deliver messages. + * + * @default - empty list + */ + readonly subscriptionDependencies?: IDependable[]; } /** diff --git a/packages/@aws-cdk/aws-sns/lib/topic-base.ts b/packages/@aws-cdk/aws-sns/lib/topic-base.ts index 01a12275d7bb0..af2ad5cf1828a 100644 --- a/packages/@aws-cdk/aws-sns/lib/topic-base.ts +++ b/packages/@aws-cdk/aws-sns/lib/topic-base.ts @@ -80,8 +80,8 @@ export abstract class TopicBase extends Resource implements ITopic { /** * Subscribe some endpoint to this topic */ - public addSubscription(subscription: ITopicSubscription): Subscription { - const subscriptionConfig = subscription.bind(this); + public addSubscription(topicSubscription: ITopicSubscription): Subscription { + const subscriptionConfig = topicSubscription.bind(this); const scope = subscriptionConfig.subscriberScope || this; let id = subscriptionConfig.subscriberId; @@ -95,10 +95,17 @@ export abstract class TopicBase extends Resource implements ITopic { throw new Error(`A subscription with id "${id}" already exists under the scope ${scope.node.path}`); } - return new Subscription(scope, id, { + const subscription = new Subscription(scope, id, { topic: this, ...subscriptionConfig, }); + + // Add dependencies for the subscription + subscriptionConfig.subscriptionDependencies?.forEach(subscriptionDependency => { + subscription.node.addDependency(subscriptionDependency); + }); + + return subscription; } /** From dde620f9d3880ba2adc9899a9a0577e8a44cc279 Mon Sep 17 00:00:00 2001 From: Jonne Kaunisto Date: Wed, 20 Jul 2022 23:27:20 +0000 Subject: [PATCH 2/3] Fix linting error --- packages/@aws-cdk/aws-sns/lib/topic-base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-sns/lib/topic-base.ts b/packages/@aws-cdk/aws-sns/lib/topic-base.ts index af2ad5cf1828a..fd9296701c7bd 100644 --- a/packages/@aws-cdk/aws-sns/lib/topic-base.ts +++ b/packages/@aws-cdk/aws-sns/lib/topic-base.ts @@ -104,7 +104,7 @@ export abstract class TopicBase extends Resource implements ITopic { subscriptionConfig.subscriptionDependencies?.forEach(subscriptionDependency => { subscription.node.addDependency(subscriptionDependency); }); - + return subscription; } From 87a1b3d93496584e8facdbec2111c5115a6b4ff6 Mon Sep 17 00:00:00 2001 From: Jonne Kaunisto Date: Thu, 21 Jul 2022 00:33:47 +0000 Subject: [PATCH 3/3] Update CloudFormation package snapshot --- .../nested-stacks-test.template.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/@aws-cdk/aws-cloudformation/test/nested-stack.integ.snapshot/nested-stacks-test.template.json b/packages/@aws-cdk/aws-cloudformation/test/nested-stack.integ.snapshot/nested-stacks-test.template.json index 878b751983ba0..2b7fda6cf226e 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/nested-stack.integ.snapshot/nested-stacks-test.template.json +++ b/packages/@aws-cdk/aws-cloudformation/test/nested-stack.integ.snapshot/nested-stacks-test.template.json @@ -91,6 +91,7 @@ }, "SubscriberQueuenestedstackstestNestedStack1topic089C5EB1396F65087": { "Type": "AWS::SNS::Subscription", + "DependsOn": "SubscriberQueuePolicy25A0799E", "Properties": { "Protocol": "sqs", "TopicArn": { @@ -109,6 +110,7 @@ }, "SubscriberQueuenestedstackstestNestedStack1topic1150E1A929A2C267E": { "Type": "AWS::SNS::Subscription", + "DependsOn": "SubscriberQueuePolicy25A0799E", "Properties": { "Protocol": "sqs", "TopicArn": { @@ -127,6 +129,7 @@ }, "SubscriberQueuenestedstackstestNestedStack1topic209B8719858511914": { "Type": "AWS::SNS::Subscription", + "DependsOn": "SubscriberQueuePolicy25A0799E", "Properties": { "Protocol": "sqs", "TopicArn": {