feat(sns): Add FilterPolicyScope support

[brennanho]

---

closes #23079

Description

This PR adds support for FilterPolicyScope for SNS subscriptions. This is in response to AWS introducing payload-based message filtering: https://aws.amazon.com/blogs/compute/introducing-payload-based-message-filtering-for-amazon-sns/.

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

[gitpod-io]

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

[peterwoodworth]

Thanks for the PR for this feature @brennanho, This feature works and I'm able to deploy stacks with it, however we cannot manually add properties to the CfnSpec. We will need to wait until this property is defined in the specification, and then merged. This PR should be a great place to return to once we get L1 support, I'm going to convert this PR to a draft in the meantime

[wolfm89]

I think this PR is not sufficient to cover this: https://docs.aws.amazon.com/sns/latest/dg/subscription-filter-policy-constraints.html#subscription-filter-policy-payload-constraints

Amazon SNS accepts a nested filter policy for payload-based filtering.

This will still not be supported after merging this PR because the filter policy will not be parsable.

[brennanho]

I think this PR is not sufficient to cover this: https://docs.aws.amazon.com/sns/latest/dg/subscription-filter-policy-constraints.html#subscription-filter-policy-payload-constraints

Amazon SNS accepts a nested filter policy for payload-based filtering.

This will still not be supported after merging this PR because the filter policy will not be parsable.

Ah thats interesting. I think some changes need to be made with how the Subscription class is creating the filterPolicy. I was thinking either having the key strings follow a dot notation e.g. {'key_a.key_b.key_c': sns.stringFilter({...})} OR passing in an object as how the keys appear e.g. {key_a: { key_b: { key_c: sns.stringFilter({...}) }}. The former would be easier to implement i believe but the latter would probably be a more intuitive / easier developer experience. Let me know what you think!

Edit:
Actually I tested it out locally and the parser seems to work fine with nested properties, I'll make sure to add some unit tests in the next commit though 👍 .

[wolfm89]

@brennanho I certainly would prefer the latter version {key_a: { key_b: { key_c: sns.stringFilter({...}) }} because it's closer to how the real filter would look like in the end.

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

[MrArnoldPalmer]

Hey @brennanho, thanks for the update. I was working off the code you pushed as well and came up with this. I was misunderstanding the type of the top level input for the filterPolicyWithMessageBody prop as FilterOrPolicy, when I see it's required to be an object { [attribute: string]: FilterOrPolicy }. With that I think just using your buildFilterPolicyWithMessageBody as a free floating function does in fact make sense, where the bind pattern would work better if the top level type was FilterOrPolicy that could recursively build itself.

I also changed the body of buildFilterPolicyWithMessageBody, the important bit i using the isFilter or isPolicy methods to disambiguate the two more explicitly.

I also started going down the path of making buildFilterPolicyWithMessageBody return the totalCombinationValues alongside the built object so that we can make the type number instead of [number] in order to not have to use a reference and mutate the variable etc, but that's not that important and can be refactored later if we care to.

You don't have to take all these changes, but it does feel more simplified to me (closer to what you had originally before I made you complicate it 😄 ), but take what you think feels useful.

On the integ test, it's a little confusing but within the mono-repo, before the package is published, it just exists under the @aws-cdk/integ-tests package. You can add it to the package.json and run yarn install, which does linking of local dependencies where npm will not, since we use yarn workspaces. Checkout this appsync test for an example.

[MrArnoldPalmer]

Thanks @brennanho!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Pull request has been modified.

[brennanho]

@MrArnoldPalmer No problem and thanks for your help! Sorry I had to merge from main, do you think you could approve again please? 😄

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[brennanho]

@MrArnoldPalmer Looks like the automatic merge failed. I have the permissions enabled to update from main and all the previous checks passed. I have installed Mergify on my aws account and given it permissions for read/write. Looks like I still need to enable workflow permissions and some solutions online state I need to repush with a new PAT that includes workflow permissions. If that is the case I would need another approval (no code change). Unfortunately I am not seeing anything addressing this in the contribution guideline. Please advise thanks!

[brennanho]

@Mergifyio refresh

[mergify]

refresh

✅ Pull request refreshed

Pull request has been modified.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 0489978
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[jkatnik]

@brennanho could you please release this change? 🙏🏻

[brennanho]

@brennanho could you please release this change? 🙏🏻

I think there are automatic chore releases done weekly. Like the following for example from last week 539d036. Correct me if I am wrong.

[MrArnoldPalmer]

Yeah we release weekly usually, this will go out as part of the next release.

[kzenia-1]

After this feature was released, we're unable to deploy stacks that use filterPolicy (not filterPolicyWithMessageBody).

I believe it's due to the following change where the FilterPolicyScope is undefined unless filterPolicyWithMessageBody is used.

The error we are getting:

Invalid parameter: FilterPolicyScope: Invalid value [null]. Please use either MessageBody or MessageAttributes

If I revert to the previous release (2.65.0), the stack successfully deploys.

[peterwoodworth]

@Ksenia11 can you please create a new issue with a quick reproduction of how you've defined this construct?

[ksenia-exe]

@peterwoodworth done - issue.

[pabloizquierdoalegra]

@brennanho Hi, I'm having a problem with this feature.

As you see on my screenshot, I'm creating a filter policy for a keyCountry attribute in the message body. But the result after deploying to aws is that the filter policy is set to Message Attributes instead of Message Body.

Any idea on why this could be happening?

[robpark]

I believe you may need to use a CfnSubscription instead and use CfnSubscriptionProps with `filterPolicyScope: 'MessageBody',` .. this is what we are doing anyway. @robpark

…

On Thu, Jul 27, 2023 at 3:37 PM pabloizquierdoalegra < ***@***.***> wrote: Hi, I'm having a problem with this feature. [image: image] <https://user-images.githubusercontent.com/110133105/256645544-b1121a50-3fea-416c-ab8c-7154d1ae64f2.png> As you see on my screenshot, I'm creating a filter policy for a keyCountry attribute in the message body. But the result after deploying to aws is that the filter policy is set to Message Attributes instead of Message Body. [image: image] <https://user-images.githubusercontent.com/110133105/256646359-1af52fdf-30c7-4260-ad48-6b9a936b6738.png> Any idea on why this could be happening? — Reply to this email directly, view it on GitHub <#23108 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZZBBW4TIXD52R5W75SLDXSK7NZANCNFSM6AAAAAASMWNT2M> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>