Skip to content

Commit

Permalink
fix(sns): race condition exists between sqs queue policy and sns subs…
Browse files Browse the repository at this point in the history
…cription (#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
  • Loading branch information
jonnekaunisto authored Oct 2, 2022
1 parent a006b9a commit cf43b03
Show file tree
Hide file tree
Showing 9 changed files with 1,554 additions and 186 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
},
"SubscriberQueuenestedstackstestNestedStack1topic089C5EB1396F65087": {
"Type": "AWS::SNS::Subscription",
"DependsOn": "SubscriberQueuePolicy25A0799E",
"Properties": {
"Protocol": "sqs",
"TopicArn": {
Expand All @@ -109,6 +110,7 @@
},
"SubscriberQueuenestedstackstestNestedStack1topic1150E1A929A2C267E": {
"Type": "AWS::SNS::Subscription",
"DependsOn": "SubscriberQueuePolicy25A0799E",
"Properties": {
"Protocol": "sqs",
"TopicArn": {
Expand All @@ -127,6 +129,7 @@
},
"SubscriberQueuenestedstackstestNestedStack1topic209B8719858511914": {
"Type": "AWS::SNS::Subscription",
"DependsOn": "SubscriberQueuePolicy25A0799E",
"Properties": {
"Protocol": "sqs",
"TopicArn": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@
},
"MyQueueawscdkcodebuildeventsMyTopic550011DCF72DE3ED": {
"Type": "AWS::SNS::Subscription",
"DependsOn": "MyQueuePolicy6BBEDDAC",
"Properties": {
"Protocol": "sqs",
"TopicArn": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
},
"MyQueueawscdksnseventtargetMyTopicB7575CD87304D383": {
"Type": "AWS::SNS::Subscription",
"DependsOn": "MyQueuePolicy6BBEDDAC",
"Properties": {
"Protocol": "sqs",
"TopicArn": {
Expand Down
5 changes: 3 additions & 2 deletions packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ 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;

// if the queue is encrypted, add a statement to the key resource policy
// which allows this topic to decrypt KMS keys
Expand Down Expand Up @@ -77,6 +77,7 @@ export class SqsSubscription implements sns.ITopicSubscription {
filterPolicy: this.props.filterPolicy,
region: this.regionFromArn(topic),
deadLetterQueue: this.props.deadLetterQueue,
subscriptionDependency: queuePolicyDependable,
};
}

Expand Down
Loading

0 comments on commit cf43b03

Please sign in to comment.