haraka-plugin-geoip-lite

[AuspeXeu]

Where is the code for the lite version of this package? It seems to be outdated even though it's pushed to npm under the most recent version of the non-lite package.

[msimerson]

Also here, in a separate branch.

[AuspeXeu]

Hm ... not sure what your strategy here is but how do you consistently keep track of changes to shared code between the two? The PR I submitted yesterday for example didn't fix my problem since I am using the lite version which is still unpatched.

[msimerson]

Oh boy. A cluster has been made of this.

how do you consistently keep track of changes to shared code between the two?

Short answer, I don't. The lite version hadn't been updated in ... a very long time. So after merging your other PR (which I now realize was in error, because the haraka-plugin- prefix is stripped from the name by the Haraka plugin loader), I merged all the changes from master to the lite version and published it.

The PR I submitted yesterday for example didn't fix my problem since I am using the lite version

It should have broken it! I misread the PR thinking you were removing the haraka-plugin prefix. What version of Haraka are you running?

I'm reducing the diffs between the two plugins again, so that one can see the differences between them easily with git diff $branch.

[AuspeXeu]

I am running Haraka 2.8.25 and I wrote a small 'adhoc' plugin that would simply block everything coming from Russia. But when I wanted to retrieve the result of the geo-ip plugin from the connection using connection.results.get('geoip'). However this did not work so I debugged the result object and it turned out that it indeed did store the result with the haraka-plugin- prefix. So I had to change my plugin code to the following. Which works.

const constants = require('haraka-constants')

exports.hook_ehlo = function (next, connection) {
  const {country} = connection.results.get('haraka-plugin-geoip-lite') // get('geoip') did not work here.
  if (country === 'RU') {
    next(constants.DENY, `GEO-BAN for ${country}`)
  } else {
    next()
  }
}

[msimerson]

// get('geoip') did not work here.

that shouldn't work, but get('geoip-lite') should have.

[msimerson]

it indeed did store the result with the haraka-plugin- prefix

Check your config/plugins contents. Make sure you list the plugin as geoip-lite.

[AuspeXeu]

Regarding your previous comment, but isn't it weird then that get('haraka-plugin-geoip-lite') works for me currently?

[msimerson]

isn't it weird then that get('haraka-plugin-geoip-lite') works for me currently?

Not if that's how you have it listed in config/plugins

[AuspeXeu]

I am confused ... so I thought I have to put the actual package name of the plugin into config/plugins so that the plugin mechanism knows what to require? Or does this mechanism prefix geoip-lite with haraka-plugin- in case it cannot find a geoip-lite module?

[msimerson]

the latter

[AuspeXeu]

Okay, gotcha. This still does not explain why get('haraka-plugin-geoip-lite') works for me though, because of that line you referenced earlier.

[msimerson]

It does. That line doesn't update the plugin name property.

I think this patch would make it behave a little more like people expect:

diff --git a/plugins.js b/plugins.js
index d30daf44..9dea5142 100644
--- a/plugins.js
+++ b/plugins.js
@@ -61,6 +61,7 @@ class Plugin {
         let name = plugin.name;
         if (/^haraka-plugin-/.test(name)) {
             name = name.replace(/^haraka-plugin-/, '');
+            plugin.name = name;
         }
 
         let paths = [];

Caution: not tested.

[AuspeXeu]

Right, or simply.

plugin.name = plugin.name.replace(/^haraka-plugin-/, '');

However, this would require a patch in haraka core?!

[msimerson]

yep

[AuspeXeu]

I wonder now if it wouldn't be way more transparent to everyone (especially third party plugins) if the name of a plugin will just be it's name and not somehow magically mangled inside the plugin system, what do you think?

Edit: Especially since now it's kinda weird. From my understanding you have to call it geoip-lite inside config/plugins however the package is called haraka-plugin-geoip-lite.

[msimerson]

There are many references to plugin names from other plugins. For example, the dmarc plugin fetches results from SPF and DKIM plugins. The watch plugin fetches results from many plugins. Most haraka-plugin-* plugins were formerly "shipped with Haraka" plugins that have been repackaged. It's much easier on everyone if the plugin name remains consistent.