Skip to content

Commit cf43b03

Browse files
jonnekaunistojonnekaunisto
authoredOct 2, 2022
fix(sns): race condition exists between sqs queue policy and sns subscription (#21797)
---- Fixes #20665 by adding a dependency to sqs policy for sns subscriptions. ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* This is a follow up to #21259, which got closed for failing for too long
·
v2.188.0v2.45.0
1 parent a006b9a commit cf43b03

File tree

9 files changed

+1554
-186
lines changed

9 files changed

+1554
-186
lines changed
 

‎packages/@aws-cdk-containers/ecs-service-extensions/test/publish-subscribe.integ.snapshot/aws-ecs-integ.template.json

+1,343
Large diffs are not rendered by default.

‎packages/@aws-cdk/aws-cloudformation/test/nested-stack.integ.snapshot/nested-stacks-test.template.json

+3
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
},
9292
"SubscriberQueuenestedstackstestNestedStack1topic089C5EB1396F65087": {
9393
"Type": "AWS::SNS::Subscription",
94+
"DependsOn": "SubscriberQueuePolicy25A0799E",
9495
"Properties": {
9596
"Protocol": "sqs",
9697
"TopicArn": {
@@ -109,6 +110,7 @@
109110
},
110111
"SubscriberQueuenestedstackstestNestedStack1topic1150E1A929A2C267E": {
111112
"Type": "AWS::SNS::Subscription",
113+
"DependsOn": "SubscriberQueuePolicy25A0799E",
112114
"Properties": {
113115
"Protocol": "sqs",
114116
"TopicArn": {
@@ -127,6 +129,7 @@
127129
},
128130
"SubscriberQueuenestedstackstestNestedStack1topic209B8719858511914": {
129131
"Type": "AWS::SNS::Subscription",
132+
"DependsOn": "SubscriberQueuePolicy25A0799E",
130133
"Properties": {
131134
"Protocol": "sqs",
132135
"TopicArn": {

‎packages/@aws-cdk/aws-events-targets/test/codebuild/project-events.integ.snapshot/aws-cdk-codebuild-events.template.json

+1
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@
396396
},
397397
"MyQueueawscdkcodebuildeventsMyTopic550011DCF72DE3ED": {
398398
"Type": "AWS::SNS::Subscription",
399+
"DependsOn": "MyQueuePolicy6BBEDDAC",
399400
"Properties": {
400401
"Protocol": "sqs",
401402
"TopicArn": {

‎packages/@aws-cdk/aws-events-targets/test/sns/sns-event-rule-target.integ.snapshot/aws-cdk-sns-event-target.template.json

+1
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
},
9595
"MyQueueawscdksnseventtargetMyTopicB7575CD87304D383": {
9696
"Type": "AWS::SNS::Subscription",
97+
"DependsOn": "MyQueuePolicy6BBEDDAC",
9798
"Properties": {
9899
"Protocol": "sqs",
99100
"TopicArn": {

‎packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ export class SqsSubscription implements sns.ITopicSubscription {
4040

4141
// add a statement to the queue resource policy which allows this topic
4242
// to send messages to the queue.
43-
this.queue.addToResourcePolicy(new iam.PolicyStatement({
43+
const queuePolicyDependable = this.queue.addToResourcePolicy(new iam.PolicyStatement({
4444
resources: [this.queue.queueArn],
4545
actions: ['sqs:SendMessage'],
4646
principals: [snsServicePrincipal],
4747
conditions: {
4848
ArnEquals: { 'aws:SourceArn': topic.topicArn },
4949
},
50-
}));
50+
})).policyDependable;
5151

5252
// if the queue is encrypted, add a statement to the key resource policy
5353
// which allows this topic to decrypt KMS keys
@@ -77,6 +77,7 @@ export class SqsSubscription implements sns.ITopicSubscription {
7777
filterPolicy: this.props.filterPolicy,
7878
region: this.regionFromArn(topic),
7979
deadLetterQueue: this.props.deadLetterQueue,
80+
subscriptionDependency: queuePolicyDependable,
8081
};
8182
}
8283

‎packages/@aws-cdk/aws-sns-subscriptions/test/sns-sqs.lit.integ.snapshot/aws-cdk-sns-sqs.template.json

+181-180
Original file line numberDiff line numberDiff line change
@@ -1,205 +1,206 @@
11
{
2-
"Resources": {
3-
"MyTopic86869434": {
4-
"Type": "AWS::SNS::Topic"
5-
},
6-
"EncryptionMasterKey5BD393B9": {
7-
"Type": "AWS::KMS::Key",
8-
"Properties": {
9-
"KeyPolicy": {
10-
"Statement": [
11-
{
12-
"Action": "kms:*",
13-
"Effect": "Allow",
14-
"Principal": {
15-
"AWS": {
16-
"Fn::Join": [
17-
"",
18-
[
19-
"arn:",
20-
{
21-
"Ref": "AWS::Partition"
22-
},
23-
":iam::",
24-
{
25-
"Ref": "AWS::AccountId"
26-
},
27-
":root"
2+
"Resources": {
3+
"MyTopic86869434": {
4+
"Type": "AWS::SNS::Topic"
5+
},
6+
"EncryptionMasterKey5BD393B9": {
7+
"Type": "AWS::KMS::Key",
8+
"Properties": {
9+
"KeyPolicy": {
10+
"Statement": [
11+
{
12+
"Action": "kms:*",
13+
"Effect": "Allow",
14+
"Principal": {
15+
"AWS": {
16+
"Fn::Join": [
17+
"",
18+
[
19+
"arn:",
20+
{
21+
"Ref": "AWS::Partition"
22+
},
23+
":iam::",
24+
{
25+
"Ref": "AWS::AccountId"
26+
},
27+
":root"
28+
]
2829
]
29-
]
30-
}
31-
},
32-
"Resource": "*"
33-
},
34-
{
35-
"Action": [
36-
"kms:Decrypt",
37-
"kms:GenerateDataKey"
38-
],
39-
"Condition": {
40-
"ArnEquals": {
41-
"aws:SourceArn": {
42-
"Ref": "MyTopic86869434"
4330
}
44-
}
45-
},
46-
"Effect": "Allow",
47-
"Principal": {
48-
"Service": "sns.amazonaws.com"
31+
},
32+
"Resource": "*"
4933
},
50-
"Resource": "*"
51-
}
52-
],
53-
"Version": "2012-10-17"
54-
}
34+
{
35+
"Action": [
36+
"kms:Decrypt",
37+
"kms:GenerateDataKey"
38+
],
39+
"Condition": {
40+
"ArnEquals": {
41+
"aws:SourceArn": {
42+
"Ref": "MyTopic86869434"
43+
}
44+
}
45+
},
46+
"Effect": "Allow",
47+
"Principal": {
48+
"Service": "sns.amazonaws.com"
49+
},
50+
"Resource": "*"
51+
}
52+
],
53+
"Version": "2012-10-17"
54+
}
55+
},
56+
"UpdateReplacePolicy": "Retain",
57+
"DeletionPolicy": "Retain"
5558
},
56-
"UpdateReplacePolicy": "Retain",
57-
"DeletionPolicy": "Retain"
58-
},
59-
"MyQueueE6CA6235": {
60-
"Type": "AWS::SQS::Queue",
61-
"Properties": {
62-
"KmsMasterKeyId": {
63-
"Fn::GetAtt": [
64-
"EncryptionMasterKey5BD393B9",
65-
"Arn"
66-
]
67-
}
59+
"MyQueueE6CA6235": {
60+
"Type": "AWS::SQS::Queue",
61+
"Properties": {
62+
"KmsMasterKeyId": {
63+
"Fn::GetAtt": [
64+
"EncryptionMasterKey5BD393B9",
65+
"Arn"
66+
]
67+
}
68+
},
69+
"UpdateReplacePolicy": "Delete",
70+
"DeletionPolicy": "Delete"
6871
},
69-
"UpdateReplacePolicy": "Delete",
70-
"DeletionPolicy": "Delete"
71-
},
72-
"MyQueuePolicy6BBEDDAC": {
73-
"Type": "AWS::SQS::QueuePolicy",
74-
"Properties": {
75-
"PolicyDocument": {
76-
"Statement": [
77-
{
78-
"Action": "sqs:SendMessage",
79-
"Condition": {
80-
"ArnEquals": {
81-
"aws:SourceArn": {
82-
"Ref": "MyTopic86869434"
72+
"MyQueuePolicy6BBEDDAC": {
73+
"Type": "AWS::SQS::QueuePolicy",
74+
"Properties": {
75+
"PolicyDocument": {
76+
"Statement": [
77+
{
78+
"Action": "sqs:SendMessage",
79+
"Condition": {
80+
"ArnEquals": {
81+
"aws:SourceArn": {
82+
"Ref": "MyTopic86869434"
83+
}
8384
}
85+
},
86+
"Effect": "Allow",
87+
"Principal": {
88+
"Service": "sns.amazonaws.com"
89+
},
90+
"Resource": {
91+
"Fn::GetAtt": [
92+
"MyQueueE6CA6235",
93+
"Arn"
94+
]
8495
}
85-
},
86-
"Effect": "Allow",
87-
"Principal": {
88-
"Service": "sns.amazonaws.com"
89-
},
90-
"Resource": {
91-
"Fn::GetAtt": [
92-
"MyQueueE6CA6235",
93-
"Arn"
94-
]
9596
}
97+
],
98+
"Version": "2012-10-17"
99+
},
100+
"Queues": [
101+
{
102+
"Ref": "MyQueueE6CA6235"
96103
}
97-
],
98-
"Version": "2012-10-17"
99-
},
100-
"Queues": [
101-
{
102-
"Ref": "MyQueueE6CA6235"
103-
}
104-
]
105-
}
106-
},
107-
"MyQueueawscdksnssqsMyTopic9361DEA223429051": {
108-
"Type": "AWS::SNS::Subscription",
109-
"Properties": {
110-
"Protocol": "sqs",
111-
"TopicArn": {
112-
"Ref": "MyTopic86869434"
113-
},
114-
"Endpoint": {
115-
"Fn::GetAtt": [
116-
"MyQueueE6CA6235",
117-
"Arn"
118104
]
119-
},
120-
"RedrivePolicy": {
121-
"deadLetterTargetArn": {
105+
}
106+
},
107+
"MyQueueawscdksnssqsMyTopic9361DEA223429051": {
108+
"DependsOn": "MyQueuePolicy6BBEDDAC",
109+
"Type": "AWS::SNS::Subscription",
110+
"Properties": {
111+
"Protocol": "sqs",
112+
"TopicArn": {
113+
"Ref": "MyTopic86869434"
114+
},
115+
"Endpoint": {
122116
"Fn::GetAtt": [
123-
"DeadLetterQueue9F481546",
117+
"MyQueueE6CA6235",
124118
"Arn"
125119
]
120+
},
121+
"RedrivePolicy": {
122+
"deadLetterTargetArn": {
123+
"Fn::GetAtt": [
124+
"DeadLetterQueue9F481546",
125+
"Arn"
126+
]
127+
}
126128
}
127129
}
128-
}
129-
},
130-
"DeadLetterQueue9F481546": {
131-
"Type": "AWS::SQS::Queue",
132-
"UpdateReplacePolicy": "Delete",
133-
"DeletionPolicy": "Delete"
134-
},
135-
"DeadLetterQueuePolicyB1FB890C": {
136-
"Type": "AWS::SQS::QueuePolicy",
137-
"Properties": {
138-
"PolicyDocument": {
139-
"Statement": [
140-
{
141-
"Action": "sqs:SendMessage",
142-
"Condition": {
143-
"ArnEquals": {
144-
"aws:SourceArn": {
145-
"Ref": "MyTopic86869434"
130+
},
131+
"DeadLetterQueue9F481546": {
132+
"Type": "AWS::SQS::Queue",
133+
"UpdateReplacePolicy": "Delete",
134+
"DeletionPolicy": "Delete"
135+
},
136+
"DeadLetterQueuePolicyB1FB890C": {
137+
"Type": "AWS::SQS::QueuePolicy",
138+
"Properties": {
139+
"PolicyDocument": {
140+
"Statement": [
141+
{
142+
"Action": "sqs:SendMessage",
143+
"Condition": {
144+
"ArnEquals": {
145+
"aws:SourceArn": {
146+
"Ref": "MyTopic86869434"
147+
}
146148
}
149+
},
150+
"Effect": "Allow",
151+
"Principal": {
152+
"Service": "sns.amazonaws.com"
153+
},
154+
"Resource": {
155+
"Fn::GetAtt": [
156+
"DeadLetterQueue9F481546",
157+
"Arn"
158+
]
147159
}
148-
},
149-
"Effect": "Allow",
150-
"Principal": {
151-
"Service": "sns.amazonaws.com"
152-
},
153-
"Resource": {
154-
"Fn::GetAtt": [
155-
"DeadLetterQueue9F481546",
156-
"Arn"
157-
]
158160
}
161+
],
162+
"Version": "2012-10-17"
163+
},
164+
"Queues": [
165+
{
166+
"Ref": "DeadLetterQueue9F481546"
159167
}
160-
],
161-
"Version": "2012-10-17"
162-
},
163-
"Queues": [
168+
]
169+
}
170+
}
171+
},
172+
"Parameters": {
173+
"BootstrapVersion": {
174+
"Type": "AWS::SSM::Parameter::Value<String>",
175+
"Default": "/cdk-bootstrap/hnb659fds/version",
176+
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
177+
}
178+
},
179+
"Rules": {
180+
"CheckBootstrapVersion": {
181+
"Assertions": [
164182
{
165-
"Ref": "DeadLetterQueue9F481546"
183+
"Assert": {
184+
"Fn::Not": [
185+
{
186+
"Fn::Contains": [
187+
[
188+
"1",
189+
"2",
190+
"3",
191+
"4",
192+
"5"
193+
],
194+
{
195+
"Ref": "BootstrapVersion"
196+
}
197+
]
198+
}
199+
]
200+
},
201+
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
166202
}
167203
]
168204
}
169205
}
170-
},
171-
"Parameters": {
172-
"BootstrapVersion": {
173-
"Type": "AWS::SSM::Parameter::Value<String>",
174-
"Default": "/cdk-bootstrap/hnb659fds/version",
175-
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
176-
}
177-
},
178-
"Rules": {
179-
"CheckBootstrapVersion": {
180-
"Assertions": [
181-
{
182-
"Assert": {
183-
"Fn::Not": [
184-
{
185-
"Fn::Contains": [
186-
[
187-
"1",
188-
"2",
189-
"3",
190-
"4",
191-
"5"
192-
],
193-
{
194-
"Ref": "BootstrapVersion"
195-
}
196-
]
197-
}
198-
]
199-
},
200-
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
201-
}
202-
]
203-
}
204-
}
205-
}
206+
}

‎packages/@aws-cdk/aws-sns/lib/subscriber.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Construct } from 'constructs';
1+
import { Construct, IDependable } from 'constructs';
22
import { SubscriptionOptions } from './subscription';
33
import { ITopic } from './topic-base';
44

@@ -24,6 +24,15 @@ export interface TopicSubscriptionConfig extends SubscriptionOptions {
2424
* subscribing to.
2525
*/
2626
readonly subscriberId: string;
27+
28+
/**
29+
* The resources that need to be created before the subscription can be safely created.
30+
* For example for SQS subscription, the subscription needs to have a dependency on the SQS queue policy
31+
* in order for the subscription to successfully deliver messages.
32+
*
33+
* @default - empty list
34+
*/
35+
readonly subscriptionDependency?: IDependable;
2736
}
2837

2938
/**

‎packages/@aws-cdk/aws-sns/lib/topic-base.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ export abstract class TopicBase extends Resource implements ITopic {
8080
/**
8181
* Subscribe some endpoint to this topic
8282
*/
83-
public addSubscription(subscription: ITopicSubscription): Subscription {
84-
const subscriptionConfig = subscription.bind(this);
83+
public addSubscription(topicSubscription: ITopicSubscription): Subscription {
84+
const subscriptionConfig = topicSubscription.bind(this);
8585

8686
const scope = subscriptionConfig.subscriberScope || this;
8787
let id = subscriptionConfig.subscriberId;
@@ -95,10 +95,18 @@ export abstract class TopicBase extends Resource implements ITopic {
9595
throw new Error(`A subscription with id "${id}" already exists under the scope ${scope.node.path}`);
9696
}
9797

98-
return new Subscription(scope, id, {
98+
const subscription = new Subscription(scope, id, {
9999
topic: this,
100100
...subscriptionConfig,
101101
});
102+
103+
// Add dependency for the subscription, for example for SQS subscription
104+
// the queue policy has to deploy before the subscription is created
105+
if (subscriptionConfig.subscriptionDependency) {
106+
subscription.node.addDependency(subscriptionConfig.subscriptionDependency);
107+
}
108+
109+
return subscription;
102110
}
103111

104112
/**

‎packages/@aws-cdk/aws-stepfunctions-tasks/test/sns/publish.integ.snapshot/aws-stepfunctions-tasks-sns-publish-integ.template.json

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
},
4646
"showmethemessagesawsstepfunctionstaskssnspublishintegcooltopic8388C976F1D63091": {
4747
"Type": "AWS::SNS::Subscription",
48+
"DependsOn": "showmethemessagesPolicyB08B04B0",
4849
"Properties": {
4950
"Protocol": "sqs",
5051
"TopicArn": {

0 commit comments

Comments
 (0)
Please sign in to comment.