haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.07k stars 661 forks source link

Plugins to move to npm modules #1287

Closed baudehlo closed 8 years ago

baudehlo commented 8 years ago

This issue is to discuss which plugins we should move out of core and into npm modules so they can be updated more often than the core. This issue will also help us discuss any issues that may arise from this.

Current plugins:

Those marked with * are listed in config/plugins currently.

access.js *
aliases.js
attachment.js *
avg.js
backscatterer.js
block_me.js
bounce.js *
clamd.js *
connect.asn.js *
connect.fcrdns.js *
connect.geoip.js *
connect.p0f.js *
connect.rdns_access.js
daemonize.js
data.headers.js *
data.nomsgid.js
data.noreceived.js
data.rfc5322_header_checks.js
data.signatures.js
data.uribl.js *
dcc.js
delay_deny.js
dkim_sign.js *
dkim_verify.js
dns_list_base.js
dnsbl.js *
dnswl.js
early_talker.js *
esets.js
graph.js
greylist.js
helo.checks.js *
karma.js *
limit.js
log.elasticsearch.js
log.syslog.js *
lookup_rdns.strict.js
mail_from.access.js
mail_from.blocklist.js
mail_from.is_resolvable.js *
mail_from.nobounces.js
max_unrecognized_commands.js *
messagesniffer.js
prevent_credential_leaks.js
process_title.js *
profile.js
rate_limit.js
rcpt_to.access.js
rcpt_to.blocklist.js
rcpt_to.host_list_base.js
rcpt_to.in_host_list.js *
rcpt_to.ldap.js *
rcpt_to.max_count.js
rcpt_to.qmail_deliverable.js *
rcpt_to.routes.js *
rdns.regexp.js
record_envelope_addresses.js
redis.js
relay.js *
relay_acl.js
relay_all.js
relay_force_routing.js
reseed_rng.js
rspamd.js
spamassassin.js *
spf.js *
tarpit.js
test_queue.js
tls.js *
toobusy.js *
xclient.js

plugins/auth:
auth_ldap.js *
auth_proxy.js *
auth_vpopmaild.js
flat_file.js *

plugins/queue:
deliver.js
discard.js *
lmtp.js
qmail-queue.js *
quarantine.js *
rabbitmq.js
rabbitmq_amqplib.js
smtp_forward.js *
smtp_proxy.js *

plugins/watch:
html
index.js
msimerson commented 8 years ago

Easy answer (to say, and to shoot holes in): All of them.

baudehlo commented 8 years ago

If we say all of them, we still need to decide which ones go in package.json as dependencies, and which are left out.

I'm fine with either route, as even in development you need to run npm install.

msimerson commented 8 years ago

If we say all of them, we still need to decide which ones go in package.json as dependencies, and which are left out.

Sure, but those could be pretty easy choices. There's a very short list of plugins that should be considered dependencies. Maybe even, but probably not, zero. I think every other plugin could get added to optionalDependencies for as long as that plugin:

a. has a maintainer b. passes build tests c. has an interested user base

Having npm be the single way to install plugins seems like a win on a few levels, the most obvious being, adding or removing a plugin is simply a change to package.json. Since NPM supports installing from github repos, SMF can have his attachment plugin installed automatically and so can I.

We'd need to create a haraka-mock-plugin module that plugins could depend on, and that lets the plugins run in a simulated environment, and validates that they do The Right Stuff. That mock plugin would itself be versioned and moves in lock-step with Haraka.

baudehlo commented 8 years ago

We also need to consider how npm plugins get tested.

On Jan 4, 2016, at 8:42 PM, Matt Simerson notifications@github.com wrote:

If we say all of them, we still need to decide which ones go in package.json as dependencies, and which are left out.

Sure, but those could be pretty easy choices. There's a very short list of plugins that should be considered dependencies. Maybe even, but probably not, zero. I think every other plugin could get added to optionalDependencies for as long as that plugin:

a. has a maintainer b. passes build tests c. has an interested user base

Having npm be the single way to install plugins seems like a win on a few levels, the most obvious being, adding or removing a plugin is simply a change to package.json. Since NPM supports installing from github repos, SMF can have his attachment plugin installed automatically and so can I.

We'd need to create a haraka-mock-plugin module that plugins could depend on, and that lets the plugins run in a simulated environment, and validates that they do The Right Stuff. That mock plugin would itself be versioned and moves in lock-step with Haraka.

— Reply to this email directly or view it on GitHub.

baudehlo commented 8 years ago

Another thing to consider is npm/node_modules plugins don't get constants loaded. Nor can they easily require() core Haraka libs.

On Jan 4, 2016, at 8:42 PM, Matt Simerson notifications@github.com wrote:

If we say all of them, we still need to decide which ones go in package.json as dependencies, and which are left out.

Sure, but those could be pretty easy choices. There's a very short list of plugins that should be considered dependencies. Maybe even, but probably not, zero. I think every other plugin could get added to optionalDependencies for as long as that plugin:

a. has a maintainer b. passes build tests c. has an interested user base

Having npm be the single way to install plugins seems like a win on a few levels, the most obvious being, adding or removing a plugin is simply a change to package.json. Since NPM supports installing from github repos, SMF can have his attachment plugin installed automatically and so can I.

We'd need to create a haraka-mock-plugin module that plugins could depend on, and that lets the plugins run in a simulated environment, and validates that they do The Right Stuff. That mock plugin would itself be versioned and moves in lock-step with Haraka.

— Reply to this email directly or view it on GitHub.

baudehlo commented 8 years ago

Maybe we should have a plugin.require() method that loads core modules.

Or perhaps that's even overkill because basically plugins only ever require outbound, utils, net_utils, or constants. So just putting those on every plugin wouldn't hurt too much.

I worry about the constants though. I wonder if one thing we could do is just make them globals (via removing "use strict". It would be interesting to know what that might do to the optimizer in v8.

msimerson commented 8 years ago

What about moving constants to an external package haraka-constants and having plugins (whether plugins/file.js or name/package.json) require it explicitly? That way there's less "special" handling required for plugin loading, making it easier to get rid of vm.runInContext.

baudehlo commented 8 years ago

Whatever solution we come up with requires a bunch of rewriting. The runInContext was meant to provide a level of security for bad coding but in retrospect is probably a bad idea. The only real benefit it provides is overriding require().

On Jan 14, 2016, at 11:00 PM, Matt Simerson notifications@github.com wrote:

What about moving constants to an external package haraka-constants and having plugins (whether plugins/file.js or name/package.json) require it explicitly? That way there's less "special" handling required for plugin loading, making it easier to get rid of vm.runInContext.

— Reply to this email directly or view it on GitHub.

smfreegard commented 8 years ago

Surprisingly - I don't hate this idea. Provided it doesn't add a massive amount of complications in doing so.

I haven't really been following the whole discussion of loading plugins as npm modules, but question that occurs to me is how is config handled? e.g. if I create a plugin as an npm module - how do I supply a default configuration? Is it a manual step to copy the relevant config file to $HARAKA_HOME/conifg and add the plugin to $HARAKA_HOME/config/plugins?

If we're going to rewrite how plugins work and get loaded, then we should have a plan of exactly what we want to achieve in doing so.

baudehlo commented 8 years ago

The idea would be to support config files in the plugin's directory, which can be overridden by something in /config.

On Sat, Jan 16, 2016 at 10:08 AM, Steve Freegard notifications@github.com wrote:

Surprisingly - I don't hate this idea. Provided it doesn't add a massive amount of complications in doing so.

I haven't really been following the whole discussion of loading plugins as npm modules, but question that occurs to me is how is config handled? e.g. if I create a plugin as an npm module - how do I supply a default configuration? Is it a manual step to copy the relevant config file to $HARAKA_HOME/conifg and add the plugin to $HARAKA_HOME/config/plugins?

If we're going to rewrite how plugins work and get loaded, then we should have a plan of exactly what we want to achieve in doing so.

— Reply to this email directly or view it on GitHub https://github.com/haraka/Haraka/issues/1287#issuecomment-172214752.

msimerson commented 8 years ago

The idea would be to support config files in the plugin's directory, which can be overridden by something in /config.

I think this is exactly the right approach. I would suggest that by default, plugins have no config file, and that the defaults are a) documented and b) baked into the config loading section of the plugin. That way, when Haraka users upgrade, they get the plugins current default behavior. In most cases, this is going to be The Right Thing. For cases where the defaults need to be overridden, users place a config file in /config, which for better or worse, never gets updated.

pvandijk commented 8 years ago

I haven't really been following the whole discussion of loading plugins as npm modules, but question that occurs to me is how is config handled? e.g. if I create a plugin as an npm module - how do I supply a default configuration? Is it a manual step to copy the relevant config file to $HARAKA_HOME/conifg and add the plugin to $HARAKA_HOME/config/plugins?

(Disclaimer: i havent thought this through as much as i'd like)

I'd definitely agree having sensible defaults for everything would be the way to go, Though it got me thinking that if i was going to use haraka for a specific purpose i'd probably want to proceduralise the adding of plugins rather than relying on editing or dumping specific configuration files in place (in reference to package.json or the config dir generally).

I was thinking to myself, would i just want to run something like: (warning: contrived example, smtp isnt a plugin)

./enable_plugin smtp port=25 public_ip=xxx... or just ./enable_plugin smtp (and then prompt for required config in a friendly way)

Might be a few steps too far for what's needed right now, but if the aim is to decouple the plugins, i'd suggest having a trivial way to enable them would be extremely useful. Developers may need to know how it all works and fits together, but systems administrators not so much.

msimerson commented 8 years ago

a trivial way to enable them would be extremely useful.

An excellent point @pvandijk

For those who are provisioning Haraka using automated tools (docker, chef, puppet, ansible, etc..), it's straight forward to use CLI tools (bash / sed / awk / tee / perl) to write a build script that smashes in the preferred plugins and config settings for our instances. But it certainly would be nice to have a tool that:

./haraka-plugin --enable spamassassin --disable spf

That tool could embody some of the domain knowledge the Haraka experienced have, being aware that plugin ordering matters, and could make optimally locating plugins within config/plugins mostly automatic.

baudehlo commented 8 years ago

To be fair, even something incredibly popular like hubot requires people to edit a config file to add a plugin. Even worse for express.

msimerson commented 8 years ago

To be fair

Aye, but OTOH, the npm model with npm install --save and npm install --save-dev is quite a bit more friendly. With that in mind, perhaps something that wraps npm and has some haraka config smarts like this would be even better:

./haraka-plugin --install --enable spamassassin
./haraka-plugin --disable spf

Anyhow, this is really a tangential conversation to this issue and really ought to have it's own issue since a) it's a good idea and b) would be useful regardless of which way we load plugins.

baudehlo commented 8 years ago

Agreed. Also didn't we have some conversation at some point about letting plugins specify their need to be in some sort of order? I can't find the ticket for it now. That would be relevant to this.

msimerson commented 8 years ago

Maybe you're looking for this: https://github.com/haraka/Haraka/blob/master/docs/Plugins.md#plugin-run-order

baudehlo commented 8 years ago

Correct. And the discussion on PR #951 that led to that.

On Jan 21, 2016, at 8:29 PM, Matt Simerson notifications@github.com wrote:

Maybe you're looking for this: https://github.com/haraka/Haraka/blob/master/docs/Plugins.md#plugin-run-order

— Reply to this email directly or view it on GitHub.

msimerson commented 8 years ago
msimerson commented 8 years ago

Update

We have completed most of the infrastructure work that enables plugins to be installed and run as npm modules. This issue is nearly resolved.

haraka core modules

haraka plugins

bardiharborow commented 6 years ago

It might be advisable to better document the fact that certain "core" plugins are no longer actually included in the core. I would consider myself a fairly technically oriented user (I'm testing Haraka in a systemd-managed SELinux-enforced docker container on CoreOS in preparation for migrating to Kubernetes), but I just spent far far too long trying to configure the access plugin to realise it's not even installed. 😂

baudehlo commented 6 years ago

It should get installed. It's listed as an optional dependency, which means npm should try and install it but be OK if it fails.

On Wed, Dec 20, 2017 at 8:41 AM, Bardi Harborow notifications@github.com wrote:

It might be advisable to better document the fact that certain "core" plugins are no longer actually included in the core. I would consider myself a fairly technically oriented user (I'm testing Haraka in a systemd-managed SELinux-enforced docker container on CoreOS in preparation for migrating to Kubernetes), but I just spent far far too long trying to configure the access plugin to realise it's not even installed. 😂

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/haraka/Haraka/issues/1287#issuecomment-353065824, or mute the thread https://github.com/notifications/unsubscribe-auth/AAobY8xLry4Eyt3hmE0-0r-QeBkliwx6ks5tCQ6LgaJpZM4G9zQU .

bardiharborow commented 6 years ago

@baudehlo, ah, I was using yarn, which may have different behaviour.