Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

add variable to configure SNS sender#11930

Merged
bentsku merged 3 commits intomasterfrom
sns-ses-sender
Nov 26, 2024
Merged

add variable to configure SNS sender#11930
bentsku merged 3 commits intomasterfrom
sns-ses-sender

Conversation

@bentsku
Copy link
Copy Markdown
Contributor

@bentsku bentsku commented Nov 26, 2024

Motivation

As reported by @HarshCasper, the sender of SNS email notifications was hardcoded to [email protected] with no mean of changing that. We should replace this value by something like [email protected] or something similar, as AWS is sending those notifications with [email protected].

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 [email protected] 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
Copy link
Copy Markdown

github-actions bot commented Nov 26, 2024

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 26, 2024

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.

Copy link
Copy Markdown
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
Copy link
Copy Markdown
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
@bentsku bentsku deleted the sns-ses-sender branch November 26, 2024 22:42
@bentsku bentsku mentioned this pull request Nov 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

aws:sns Amazon Simple Notification Service semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants