Skip to content

add variable to configure SNS sender #11930

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 26, 2024
Merged

add variable to configure SNS sender #11930

merged 3 commits into from
Nov 26, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Nov 26, 2024 •
edited
Loading

Motivation

As reported by @HarshCasper, the sender of SNS email notifications was hardcoded to admin@localstack.com with no mean of changing that. We should replace this value by something like no-reply@sns.localstack.cloud or something similar, as AWS is sending those notifications with no-reply@sns.amazonaws.com.

I'm a bit worried about changing this directly however, as some users might expected and test that emails are arriving from that sender.

Changes

  • introduce SNS_SES_SENDER_ADDRESS config variable to make it configurable, and fallback to admin@localstack.com if not configured

TODO

What's left to do:

  • open a PR in the docs to document this variable and the default value

@bentsku bentsku added aws:sns Amazon Simple Notification Service semver: patch Non-breaking changes which can be included in patch releases labels Nov 26, 2024
@bentsku bentsku added this to the 4.0.3 milestone Nov 26, 2024
@bentsku bentsku requested a review from alexrashed November 26, 2024 10:58
@bentsku bentsku self-assigned this Nov 26, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 26, 2024 •
edited
Loading

S3 Image Test Results (AMD64 / ARM64)

  2 files    2 suites   4m 9s ⏱️
433 tests 381 ✅  52 💤 0 ❌
866 runs  762 ✅ 104 💤 0 ❌

Results for commit 9a68e04.

♻️ This comment has been updated with latest results.

@bentsku bentsku mentioned this pull request Nov 26, 2024
Merged
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 26, 2024 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 49m 9s ⏱️ - 1m 52s
3 734 tests +7  3 388 ✅ +7  346 💤 ±0  0 ❌ ±0 
3 736 runs  +7  3 388 ✅ +7  348 💤 ±0  0 ❌ ±0 

Results for commit 9a68e04. ± Comparison against base commit 5f19fbc.

♻️ This comment has been updated with latest results.

alexrashed
alexrashed approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

LGTM! 💯 Nice and clean, would be great to have a test though, but that could also be added in a later iteration (not blocking). 😉

@bentsku bentsku requested a review from alexrashed November 26, 2024 14:49
alexrashed
alexrashed approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for addressing the comment right away and adding a test! 🚀 💯 :shipit:

@bentsku bentsku merged commit 9e21cd5 into master Nov 26, 2024
39 checks passed
@bentsku bentsku deleted the sns-ses-sender branch November 26, 2024 22:42
@bentsku bentsku mentioned this pull request Nov 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexrashed alexrashed alexrashed approved these changes

@baermat baermat Awaiting requested review from baermat baermat is a code owner

@thrau thrau Awaiting requested review from thrau thrau is a code owner

Assignees

@bentsku bentsku

Labels
aws:sns Amazon Simple Notification Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.0.3
Development

Successfully merging this pull request may close these issues.
2 participants
@bentsku @alexrashed