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

[9.x] Support AWS SES with IAM Assume Role #40649

Merged
merged 6 commits into from
Jan 27, 2022

Conversation

deleugpn
Copy link
Contributor

On PR #38481, the file https://github.com/laravel/framework/blob/3234a8dfecbb12140e1ce980e415fcd0363f320a/tests/Mail/MailSesTransportTest.php was removed and with it 3 features were lost:

  • Ability to let SES Assume Role through AWS SDK
  • Ability to configure SES ConfigurationSet
  • Ability to define Email tags

This PR addresses point 1. Upon closer look at item 2), it looks quite complex to get that functionality back because it must be configured at Mail Sending time (see symfony/symfony#37897) and during the Mailable build we would end up needing some sort of if (... instanceof Ses).

Item 3) seem to not even be supported by Symfony and is not clearly documented by AsyncAws (see https://async-aws.com/clients/ses.html).

@deleugpn deleugpn changed the title Support AWS SES with IAM Assume Role [9.x] Support AWS SES with IAM Assume Role Jan 26, 2022
@deleugpn
Copy link
Contributor Author

Looks like I can either get it to work on Windows or Linux, but not both 👀

@Jubeki
Copy link
Contributor

Jubeki commented Jan 26, 2022

It seems like you are right. Symfony only supports settings these options via the Header:
https://github.com/symfony/amazon-mailer/blob/99a9a86ae5d3475300b63834c91175bf29a7f19d/Transport/SesApiAsyncAwsTransport.php#L92-L94
And the ConfigurationSetName and Tags are not set while creating via the Factory.

There are four possible solutions going forward:

  1. Let the Developer set them using Headers (not ideal)
  2. Make a PR to symfony/amazon-mailer to support it with the DSN-Object (not sure if this goes through)
  3. Make a custom SesTransport which includes the support.
  4. Create a callback which will be called for all E-Mails which are send using SES and set the correct headers.

@driesvints @taylorotwell what are your opinions on that?

@driesvints
Copy link
Member

I'll have a look at this today. Thanks

@driesvints driesvints marked this pull request as draft January 27, 2022 08:26
@driesvints driesvints marked this pull request as ready for review January 27, 2022 15:07
@driesvints
Copy link
Member

This can already be merged to address 1. and I'll have a look at the other two. Thanks @deleugpn!

@taylorotwell taylorotwell merged commit 99feb1f into laravel:9.x Jan 27, 2022
@driesvints
Copy link
Member

I took a new approach by re-introducing the old transport. @deleugpn can you try it out? #40696

@deleugpn deleugpn deleted the ses-transport branch January 29, 2022 21:02
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.

4 participants