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

Feature: SNS Event FilterPolicyScope attribute #2988

Conversation

mustafa-sadiq
Copy link
Contributor

Issue #, if available

N/A

Description of changes

Adds the FilterPolicyScope attribute to the SNS Subscription created for a SNS function event source.
This attribute is supported by cloudformation:
https://aws.amazon.com/blogs/compute/introducing-payload-based-message-filtering-for-amazon-sns/

If you try to deploy a function with event

Events:
  NotificationTopic:
    Type: SNS
    Properties:
      Topic: !Ref MySNSTopic
      Region: region
      FilterPolicy:
        store:
        - example_corp
        event:
        - anything-but: order_cancelled
        customer_interests:
        - rugby
        - football
        - baseball
        price_usd:
        - numeric:
          - '>='
          - 100
      FilterPolicyScope: MessageAttributes

sam deploy gives this output

Error: Failed to create changeset for the stack: sam-hello-world, ex: Waiter ChangeSetCreateComplete failed: Waiter encountered a terminal failure state: For expression "Status" we matched expected path: "FAILED" Status: FAILED. Reason: Transform AWS::Serverless-2016-10-31 failed with: Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [HelloWorldFunctionNotificationTopic] is invalid. property FilterPolicyScope not defined for resource of type SNS

Description of how you validated changes

I deployed a transformed template with the FilterPolicyScope: MessageAttributes added.

Checklist

Examples?

Deploy a Function with a SNS event source and add property:
FilterPolicyScope: MessageAttributes

Please reach out in the comments if you want to add an example. Examples will be
added to sam init through aws/aws-sam-cli-app-templates.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mustafa-sadiq mustafa-sadiq requested a review from a team as a code owner March 2, 2023 02:50
Copy link
Contributor

@GavinZZ GavinZZ left a comment

Choose a reason for hiding this comment

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

Thanks for opening the PR. The changes look good. I have one question before I approve. I see that FilterPolicyScope supports two possible value, MessageAttributes or MessageBody. Is there any reason we aren't testing MessageBody as an input?

edit: I meant integration test, not transform test.

@mustafa-sadiq
Copy link
Contributor Author

Thanks for opening the PR. The changes look good. I have one question before I approve. I see that FilterPolicyScope supports two possible value, MessageAttributes or MessageBody. Is there any reason we aren't testing MessageBody as an input?

The test will pass no matter what I change the value to. I set it to InvalidValue and it would pass through. I am not sure how to set an integration test so that CloudFormation will fail so I will leave that up to the team.

Copy link
Contributor

@GavinZZ GavinZZ left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!!

@hoffa hoffa merged commit cd17985 into aws:develop Mar 6, 2023
@mustafa-sadiq
Copy link
Contributor Author

thank you @hoffa and @GavinZZ! I am new to open-source and appreciate the guidance and reviews! <3

@mustafa-sadiq mustafa-sadiq deleted the feature/sns-event-filter-policy-scope-attribute branch March 7, 2023 06:58
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

Successfully merging this pull request may close these issues.

None yet

4 participants