Ghost: Update Nodemailer and allow non-SMTP mail config

[evenfrost]

Would be great if Ghost could bump Nodemailer version from 0.7, which is three years old, to 4.x. This will have some good effects like possibility to use API endpoints instead of SMTP. See https://github.com/TryGhost/Ghost/issues/7930. The one reason why it can be handy is that our project team cannot use Mailgun's SMTP workflow due to our client's restrictions and we are using API key instead.

A bunch of similar issues were closed before (https://github.com/TryGhost/Ghost/issues/4125, https://github.com/TryGhost/Ghost/issues/4817) arguing that there should be a PR opened for this instead. Well, there is now (https://github.com/TryGhost/Ghost/pull/8930), though it's pretty raw one, but nevertheless, it's working for us (just for us, probably). There I was advised to open a PR (we've gone full circle now I suppose :) to discuss this further, so here we are.

So, summarizing all the steps required for this to work as I see it, we need to:

• Bump nodemailer package.
• Add custom Nodemailer transports (e.g. nodemailer-mailgun-transport, possibly not in root folder but in core/server/mail as nested dependencies).
• Rewrite GhostMailer.js for smoothly utilizing new Nodemailer's API without breaking changes to mail config (in order to avoid major Ghost version bump).
• Rewrite GhostMailer_spec.js tests.
• Update documentation.

I think a somewhat good starting point could be my pull request here.

I'd be happy to hear from Ghost devs and community about it. Thanks.

[kevinansfield]

FYI, this comment has a little more information about why we were unable to upgrade to 1.0 in the past https://github.com/TryGhost/Ghost/pull/5111#issuecomment-90901241

TL;DR: Unless we add _all_ the available transports as modules in package.json we end up in a situation where any user that wants to use a different transport than the few base ones are stuck because package.json is never meant to be edited by users - it will be wiped every time they update.

[evenfrost]

That's pretty solid reason indeed. I think, to not cause breaking changes, there could be some mechanism written to utilize old Nodemailer API based on known services as this was just list of SMTP configs which got mapped to options here.

Then, for advanced API usage and since Ghost higly recommends Mailgun, I think there would be no much overhead if Ghost could add single nodemailer-mailgun-transport as its dependecy to allow custom API config.

[evenfrost]

It might look like I'm pretty biased towards Mailgun here (which support I'm interested in indeed), but Mailgun is really high-popular provider and again, is recommended by Ghost, so I think it would be OK if we added transport as a dependency (BTW, SES transport is included by default).

[ErisDS]

I apologise that this has appeared to go around in circles. The problem here, as explained by @kevinansfield is that we cannot update NodeMailer without breaking backwards compatability.

When we first pinned NodeMailer to 0.7, there was promise that that version would always be supported and so we had no need nor plan for how to upgrade. However, the situation has changed and Ghost has also evolved.

The plan we have for how to upgrade is to use adapters. This was mentioned in #7930, but there is some advanced write up on the Ghost-CLI repo in TryGhost/Ghost-CLI#232 and TryGhost/Ghost-CLI/#49.

The reason why this is discussed on the Ghost-CLI repo and not the Ghost repo, is that without having the infrastructure in the cli tool to easily install adapters, we can't really move mail towards using them. The current adapters: scheduling & storage both have sensible defaults and so installing an adapter is a task for a developer who knows they want something custom. With mail, I think keeping the old nodemailer as a sensible default might work for Ghost 1.0. We'd probably switch to using Mailgun's API as the default (we're fairly biased towards Mailgun too 😉 ) and forcing everyone else to setup a new adapter.

So the way forward is this:

• [ ] Refactor mail so that it can be extended with adapters
• [ ] Use the oldschool nodemailer as the default adapter, ensure there are no backwards incompatible changes
• [ ] Ensure it's possible to build a custom mail adapter and document it

Then:

• [ ] Get Ghost-CLI upgraded to support adapter installs

Finally, in Ghost 2.0

• [ ] Change the default adapter / move towards there being no default, and Ghost-CLI handles installing and configuring mail.

[m1guelpf]

Is this fixed now?

[kirrg001]

@m1guelpf No. The issue is still open.

[greghart]

Having read https://github.com/TryGhost/Ghost-CLI/issues/232 looks like there are some great future plans on what adapters can become.

For now I do see that issue is labelled "Later" though, so I'm assuming for an initial approach on this issue we can just keep things similar to how storage/scheduling are setup?

Granted, from what I can tell there's not any common interface between those two currently (aside from how they're configured), so there will likely be some cleanup along the way, but either way I imagine the brunt of the work here will just be moving logic around.

[greghart]

I've done some work on this one that can be seen at https://github.com/TryGhost/Ghost/compare/master...greghart:8933-mail-adapter

A basic TDD can be seen at https://github.com/greghart/Ghost/blob/8933-mail-adapter/core/server/adapters/README.md
but tl;dr: added a first class adapter interface that encapsulates metadata about adapters, DRYed up adapter loading logic, and added a mail adapter

Nearly ready for PR, but there are a few @todos that I was hoping for feedback on, to ensure I'm on the right track.

Adapter related

• [ ] internal adapter paths -- eg. internalStoragePath. Currently the path in the codebase to a given adapters' implementations is a config option in paths. These aren't really needed by code any more, but I'm assuming there is backwards compatibility needs there. Can we remove?
• [ ] default implementation -- we can require adapters to have an explicit default implementation (in code), instead of having a default "active" configuration. Needed?

Mail related

• [ ] direct mail logic -- currently consumers of GhostMailer branch if the transport type is "direct". Maintain this, or shift the logic to adapter?
• [ ] from -- currently from falls back to a smart default based on site settings. Do adapters need access to this same logic?

Other

• [x] tests -- getAdapterImplementation needs tests! This is on me ;)
• [ ] Mailgun proof of concept of custom implementation -- this'll need to move to its' own module but kept it in for this first feedback cycle.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kaizenseed]

Hi

Is there any info on if the nodemailer package will be updated to latest versions?

Thanks