Serverless-application-model: Feature Request: Add SQS option to SNS event

[jlhood]

Description:

SAM offers simplified syntax for SNS -> Lambda and with #261 supports adding subscription filter policies. However, a common use case is SNS -> SQS -> Lambda. SAM offers simplified syntax for SQS -> Lambda, but you have to use standard CloudFormation to setup the SNS -> SQS connection and subscription filter policy, which can be verbose.

SAM could add a lot of value by adding an option to the existing SNS event type to create an SQS queue.

Here's an example that uses simple default settings for the queue:

Type: SNS
Properties:
  Topic: arn:aws:sns:us-east-1:123456789012:my_topic
  # new property that creates an SQS queue between the topic and Lambda function
  # When only the boolean is specified, SAM uses the following defaults:
  # AWS::SQS::Queue logicalId: <function logicalId><SNS event key>Queue
  # AWS::SQS::QueuePolicy logicalId: <function logicalId><SNS event key>QueuePolicy
  # BatchSize: default behavior - don't add to `AWS::Lambda::EventSourceMapping` resource so default value is used.
  # Enabled: default behavior - don't add to `AWS::Lambda::EventSourceMapping` resource so default value is used.
  SqsSubscription: true
  FilterPolicy:
    store: 
      - example_corp
    price_usd: 
      - numeric: 
          - ">="
          - 100

Here's an example that specifies additional options for the queue:

Type: SNS
Properties:
  Topic: arn:aws:sns:us-east-1:123456789012:my_topic
  # new property that creates an SQS queue between the topic and Lambda function
  # Default values can be overridden by passing an object instead of a boolean. Supported keys:
  # QueuePolicyLogicalId: overrides default logicalId naming of the AWS::SQS::QueuePolicy resource.
  # QueueArn: allows the user to specify their own queue instead of having SAM create one on their behalf, allowing them to specify non-default properties on the queue.
  # BatchSize: if specified, add BatchSize property with given value to `AWS::Lambda::EventSourceMapping` resource.
  # Enabled: if specified, add BatchSize property with given value to `AWS::Lambda::EventSourceMapping` resource.
  SqsSubscription:
    QueuePolicyLogicalId: CustomQueuePolicyLogicalId
    QueueArn: !GetAtt MyCustomQueue.Arn
    BatchSize: 5
    Enabled: false
  FilterPolicy:
    store: 
      - example_corp
    price_usd: 
      - numeric: 
          - ">="
          - 100

Implementation Note: When SAM expands this into CloudFormation, the AWS::SQS::QueuePolicy resource should look like this:

  ExampleQueuePolicy:
    Type: AWS::SQS::QueuePolicy
    Properties:
      Queues:
        - !Ref ExampleQueue
      PolicyDocument:
        Version: '2012-10-17'
        Id: ExampleQueuePolicy
        Statement:
          - Effect: Allow
            Principal:
              Service:
                - sns.amazonaws.com
            Action:
              - sqs:SendMessage
            Resource:
              - !GetAtt ExampleQueue.Arn
            Condition:
              ArnEquals:
                aws:SourceArn: <SNS Topic ARN>

[otaviofff]

That would be really helpful indeed.

[jlhood]

Happy to work with anyone interested in picking this up!

[Buffer0x7cd]

@jlhood I would like to work on this issue.

[jlhood]

@Buffer0x7cd Awesome, thank you so much! Before implementing, we need to finalize the SAM spec. The draft I created above just has a simple UseSQS boolean, but we'll want a way to surface some of the options of the SQS event source like BatchSize and Enabled. I'll update the spec in the issue description with exactly what we want to support, then I'll post a comment so you'll know when it's ready to start implementation.

[jlhood]

@brettstack @keetonian I've updated the issue description with the proposed SAM spec for this feature. Can you both review?

[brettstack]

Do we have precedent for overriding generated logical resource id like QueuePolicyLogicalId? Do we need it?

What's a better name than UseSQS - [Forward|Push|Send]ToSqs? SqsSubscriptions?

Can we allow an array of queues also?

[jlhood]

Thanks for the feedback, @brettstack!

Do we have precedent for overriding generated logical resource id like QueuePolicyLogicalId? Do we need it?

This would be something new. Maybe this is a little selfish, but in the Event Fork Pipelines apps we just released today, I'm specifying the Queue and QueuePolicy explicitly. Once this SAM feature is released, I'd love to convert over to using it since it makes the template even easier to read. However, without being able to override the logical id, I don't have a way to use the SAM feature while keeping my apps backwards-compatible. 😞 Allowing overrides of logical ids would allow customers to simplify existing templates as new SAM features are added while maintaining backwards-compatibility. However, I won't fight too hard if you really don't want to add it.

What's a better name than UseSQS

Yeah I'm not loving the key name, but I couldn't think of anything better. None of your suggestions are jumping out to me as obviously better. Some others I can think of: EnableSQS or just SQS. I'm kind of liking just SQS, honestly. SQS: true and SQS: <additional properties> seems concise and readable. What do you think?

Can we allow an array of queues also?

You can already achieve this with multiple Events entries. Accepting a list could make the syntax slightly shorter, but I think it adds complexity since then there'd be two ways of doing this within the same resource.

[brettstack]

Given that use case I'm okay with adding the logical resource id name override.

The wording in docs is around "subscribing" an SQS Queue to an SNS Topic, so I'd like to get that word in there to make it clear what it is you achieve by specifying this property. SqsSubscription, SubscribeSqs (variation: replace Sqs with Queue or SqsQueue).

Multiple Event entries wouldn't be as succinct.

[keetonian]

I like SqsSubscription

[jlhood]

Thanks. Updated it to SqsSubscription in the top-level RFC comment.

[jlhood]

@Buffer0x7cd I've updated the top-level issue comment with the finalized spec for this feature, so it should be ready to implement once you're finished with your other PR. Let me know if you have any questions. Thanks!

[ShreyaGangishetty]

Closing this issue as v1.15.0 is released

[53ningen]

Hi, @ShreyaGangishetty
Thank you for release this feature!

I implemented only simple boolean SqsSubscription option to SNSEvent at first in PR #1065.
But I didn't implement whole of this feature all at once because the code-diff might be huge.
(prevent bug, make it easy to review, etc...)

Additional options for specifying the queue by SqsSubscription.QueueArn key has not implemented yet(written in the top-level RFC comment).

Here's an example that specifies additional options for the queue:
...

May I reopen this issue for implementing additional options?
If you don't mind, I would like to work on the remaining task of this issue continuously.

[praneetap]

Yes absolutely! Thanks for bringing it to our attention. Reopening..

[jlhood]

1209 brings up a good point regarding this feature. Users often want to configure additional properties on the SQS queue like DLQ and visibility timeout. We should keep that in mind while we implement this feature. We could do by allowing for passthrough SQS properties on the SNS event type, but I think it might be simpler to allow users to reference an existing SQS queue and SAM will setup all the necessary subscriptions and permissions on the given queue.

[jlilja]

I'd like the option to pass an existing SQS resource and just let SAM handle the connections between the resources, just like @jlhood mentions above.

[53ningen]

Hi, I'm on the way implement the option to pass an existing queue.

Now I'm wondering how to specify the Queue URL of a queue policy.

QueuePolicy resource have to specify both QueueARN and QueueUrl:

    "HogePolicy": {
      "Type": "AWS::SQS::QueuePolicy", 
      "Properties": {
        "Queues": [
          "https://sqs.ap-northeast-1.amazonaws.com/<aws_account_id>/<queue_name>"
        ], 
        "PolicyDocument": {
          "Version": "2012-10-17", 
          "Statement": [
            {
              "Action": "sqs:SendMessage", 
              "Resource": "arn:aws:sqs:ap-northeast-1:<aws_account_id>:<queue_name>", 
              "Effect": "Allow", 
              "Condition": {
                "ArnEquals": {
                  "aws:SourceArn": {
                    "Ref": "TweetTopic"
                  }
                }
              }, 
              "Principal": "*"
            }
          ]
        }
      }
    }

The finalized spec for this feature written in the top-level issue comment defines that SqsSubscription takes QueueARN but does not take QueueURL as a parameter.

When a QueueARN parameter is string, I can easily convert to QueueURL like this.

But when a QueueARN parameter contains Fn::Ref, Fn::Sub, etc.., it is difficult to convert to QueueURL correctly.

In addition, I think users often want to specify a queue using Fn::Ref.

Questions

• Is there any good way to specify the Queue URL of a queue policy?
• How about adding QueueUrl parameter into SqsSubscription?
  
  
  
  • I think this is a good way
  
  
  • This way also make it possible to specify a SQS fips endpoint.
  
  

[jlhood]

@53ningen Thanks so much for working on this! I agree with your proposal to add a QueueUrl parameter to SqsSubscription.

[53ningen]

I added a QueueUrl parameter to SqsSubscription: https://github.com/awslabs/serverless-application-model/pull/1231
pull request is now ready for review.