Split outbound_ip into 4 and 6; deprecate old key

[Juerd]

Binding to an IP address of a different family has no chance of succeeding, and a dual stack MX can use both. At the time of setting the note, the family cannot be determined yet.

Changes proposed in this pull request:

• introduce transaction.notes.outbound_ip4 and transaction.notes.outbound_ip6
• deprecate transaction.notes.outbound_ip (but use it as a fallback)

Checklist:

• [X] docs updated
• [ ] tests updated
• [X] Changes updated

[msimerson]

I like the idea. I'm wondering about another implementation that extends the existing note to also be an array. If it's an array, then pass the contents to a function that filters the array based on mx.family and returns the [first/last/random] IP in the list that matches. Ex:

        // Allow transaction notes to set outbound IP
        if (!mx.bind && self.todo.notes.outbound_ip) {
            if (Array.isArray(self.todo.notes.outbound_ip)) {
                mx.bind = filterIpsBy(mx.family, self.todo.notes.outbound_ip)
            }
            else {
                mx.bind = self.todo.notes.outbound_ip;
            }
        }

Then you configure it similarly to before:

connection.transaction.notes.outbound_ip = [ '1.2.3.4', 'a:b:c:d:e:f:g:h' ];
connection.transaction.notes.outbound_helo = 'mail-2.example.com';

[Juerd]

That sounds like a lot of complexity for use cases that I'm guessing are pretty rare. And it's hard to actually pick the right one from the list first/last/random; I guess that really depends on the actual use case, which may vary.

Considering that notes are set from plugins, I'd say the most logical thing to do is to move this responsibility to the plugin. Either with a new hook, or by just allowing to store a callable function in the note:

        if (!mx.bind) {
            if (mx.family === 'A' && self.todo.notes.outbound_ip4) {
                mx.bind = self.todo.notes.outbound_ip4;
            }
            else if (mx.family === 'AAAA' && self.todo.notes.outbound_ip6) {
                mx.bind = self.todo.notes.outbound_ip6;
            }
            else {
                // Deprecated
                mx.bind = self.todo.notes.outbound_ip;
            }
        }
        ...
        let host = self.hostlist.shift();

+       if (typeof(mx.bind) === "function") mx.bind = mx.bind(mx.family, host);

That's a one-line change, and also allows other places where mx.bind is set (like the get_mx hook) to take advantage of the functionality.

[msimerson]

the most logical thing to do is to move this responsibility to the plugin...or by allowing to store a callable function in the note

I like that idea!

[Juerd]

That doesn't work, because functions don't get copied to the todo notes.

Since this PR improves functionality in a way that doesn't have to be changed later, and retains backwards compatibility, I suggest merging the PR and recording the programmatic option as a feature request for later.

[msimerson]

Paging @haraka/core. I'd like to hear a couple more opinions on this.

[msimerson]

Upon reviewing this again, and reading the Outbound docs:

** outbound_ip - the IP address to bind to (note do not set this manually,
  use the `get_mx` hook)


Outbound IP address
-------------------

Normally the OS will decide which IP address will be used for outbound 
connections using the IP routing table.  

There are instances where you may want to separate outbound traffic on 
different IP addresses based on sender, domain or some other identifier.  
To do this, the IP address that you want to use *must* be bound to an 
interface (or alias) on the local system.

As described above the outbound IP can be set using the `bind` parameter
and also the outbound helo for the IP can be set using the `bind_ehlo` 
parameter returned by the `get_mx` hook or during the reception of the message 
you can set a transaction note in a plugin to tell Haraka which outbound IP 
address you would like it to use when it tries to deliver the message:

connection.transaction.notes.outbound_ip = '1.2.3.4';
connection.transaction.notes.outbound_helo = 'mail-2.example.com';

Note: if the `get_mx` hook returns a `bind` and `bind_helo` parameter, then
this will be used in preference to the transaction note.

Why do we have two ways to set the outbound bind address? One path forward is to nuke the note option and steer everyone to using the get_mx hook.