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

Modify Queue shortcut SNS behavior #125

Open
drboyer opened this issue Apr 8, 2021 · 4 comments
Open

Modify Queue shortcut SNS behavior #125

drboyer opened this issue Apr 8, 2021 · 4 comments
Assignees

Comments

@drboyer
Copy link
Contributor

drboyer commented Apr 8, 2021

Currently, the Queue shortcut will also create a SNS topic that the created queue is subscribed to, unless one of two conditions are met:

  1. The queue is a FIFO queue
  2. An ExistingTopicArn property is specified, in which case the queue will be subscribed to that topic instead

There are two things that could be improved about this behavior:

  1. In some cases you may not want to send messages via an intermediate SNS topic - it may be fine to just directly send messages to SQS. In that case, it might be nice to skip creation of the SNS topic. This is mostly to prevent unnecessary infrastructure from being created - there's really no cost benefit to this, as you're only charged for message processing with SNS.
  2. With the recent addition of SNS FIFO topics, you now can subscribe a FIFO queue to an SNS topic, as long as it's a FIFO topic. I'm already aware of one internal use case where this'd be a nice feature to support.

What I'd propose is adding a new property CreateSnsTopic with a default value of true (to prevent a breaking change). If it was set to false, we'd skip creation of the SNS topic. Then instead of ignoring TopicName or ExistingTopicArn if FifoQueue: true, we should accept an existing topic ARN (and let CloudFormation validate that it's a FIFO topic), or create a FIFO Topic if it's not and CreateSnsTopic is not false.

Priority

Not super high, as there are workarounds to both of these, and as I said, even if you don't use the Topic that's created, it doesn't really cost you anything.

@rclark
Copy link
Contributor

rclark commented Apr 8, 2021

ignoring TopicName or ExistingTopicArn if FifoQueue: true

We'd be trading this off for

ignoring TopicName or ExistingTopicArn if CreateSnsTopic: true

Right? I wonder if what we really need is a breaking change.

@drboyer
Copy link
Contributor Author

drboyer commented Apr 12, 2021

A breaking changes might make the interface a bit clearer, yeah. So if you're willing to allow that, we can go that route. In that case, would it make sense to change the behavior to:

  • If ExistingTopicArn is provided, subscribe the queue to the topic (regardless of FIFO status)
  • If QueueName is provided, create a new queue and subscribe the topic to it (create FIFO topic if a FIFO queue)
  • If neither are provided, don't create/subscribe a topic

@rclark
Copy link
Contributor

rclark commented Apr 12, 2021

If QueueName is provided, create a new queue and subscribe the topic to it (create FIFO topic if a FIFO queue)

Meant TopicName I think? If so, I think this sounds right.

@drboyer
Copy link
Contributor Author

drboyer commented Sep 23, 2021

Found another reason this behavior is somewhat problematic today - the Queue will automatically create a Queue Policy that grants the SNS topic access to send events to the queue. Unfortunately, there's no way to modify that autogenerated policy, which can cause challenges if you need to, such as if you're trying to subscribe EventBridge events to an SQS queue. Again there's a few different ways we could fix, but just eliminating the SNS-related generated code in some cases would probably be sufficient to solve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants