feat: allow any topic arn #28
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yellowcap
force-pushed
the
allow-non-owned-sns-signals
branch
from
cabe673 to
da57fd8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The proposed changes allow subscribing external SNS topics that are in different regions and from different accounts.
The main conceptual difference is to use
listSubscriptionsinstead oflistSubscriptionsByTopic. Listib by topic is only allowed for topics that are owned by the user.There is one breaking change: to support topics that are in other regions and other accounts, the input is required to be a full SNS ARN. So only providing the topic name will not work with the current implementation. I think declaring the full arn is good practice even for topics that are owned by the user.
I have not yet added tests, but have used this version in my use-case successfully. So it works, but might need some updates on the tests. However, I wanted to get your feedback before polishing