Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix SNS subscription deleted when SQS endpoint was deleted #6645

Merged
merged 1 commit into from Aug 11, 2022

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Aug 10, 2022

This PR fixes the scenario where we would delete an SNS subscription (we would actually only delete the logic/publish part but not the resource in moto, which could be listed) when its SQS endpoint was deleted. It would send a message to the DLQ once if configured then delete the subscription.
AWS does not do this, it keeps the subscription and you can even set a RedrivePolicy afterwards.

I wrote a new AWS validated and snapshotted test to validate the behaviour.

There was an old PR fixing this #6160, which had a lot of merge conflicts from all the changes since.

fixes #5459

@bentsku bentsku requested a review from thrau August 10, 2022 15:35
@bentsku bentsku temporarily deployed to localstack-ext-tests August 10, 2022 15:35 Inactive
@github-actions
Copy link

LocalStack integration with Pro

       3 files         3 suites   1h 8m 39s ⏱️
1 183 tests 1 140 ✔️ 43 💤 0
1 548 runs  1 474 ✔️ 74 💤 0

Results for commit 396ebdd.

Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

snapshot.match("subscriptions-attrs-with-redrive", sub_attrs)

# AWS takes some time to delete the queue, which make the test fails as it delivers the message correctly
assert poll_condition(lambda: not sqs_queue_exists(queue_url), timeout=5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

poll_condition 💯

@bentsku bentsku merged commit 615b51d into master Aug 11, 2022
@bentsku bentsku deleted the fix/sns-sub-removed branch August 11, 2022 12:04
@localstack localstack locked and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Publishing to an SNS topic with a redrive policy delivers failed messages to dead letter queue only once
2 participants