Bind to non-localhost for transport in some cases

[jkakavas]

When enrolling a new node to an existing cluster, we sometimes need to
bind transport to non-localhost addresse. If the other nodes of the
cluster are on different hosts than this node, then the default
configuration of binding transport layer to localhost will prevent this
node to join the cluster even after "successful" enrollment.
We check the non-localhost transport addresses that we receive during
enrollment and if any of these are not in the list of non-localhost IP
addresses that we gather from all interfaces of the current host, we
assume that at least some other node in the cluster runs on another host.

• If we don't find any non localhost addresses on all interfaces of the
  current host, we don't attempt to bind to non localhost addresses as this
  would result on an error on startup.
• If the transport layer addresses we found out in enrollment are all
  localhost, we cannot be sure where we are still on the same host, but we
  assume that as it is safer to do so and do not bind to non localhost for
  this node either.

[elasticmachine]

Pinging @elastic/es-security (Team:Security)

[albertzaharovits]

I've found the method just a bit hard to follow.

I believe conceptually it goes over the non-loopback node transport (publish) addresses, which it gets from the node enroll API, and checks if all of them are an addresses on the current host (Network.getAllAddresses).

We can agree to a rewrite in a follow-up.

[jkakavas]

We can agree to a rewrite in a follow-up.

lets do it now! This is a small enough PR, no need to do the work again in a follow up. Do you have any concrete suggestions on what to change ? Comments ? Anything you'd prefer to change in code ?

Your description is spot on:

1. If this is not an enrolling node, we should only bind to localhost ( default behavior )
2. We get all non loopback addresses of the current host.
   a. If there are all localhost, we bind this node to localhost too for the transport layer, otherwise the node will fail to even start
3. We get all transport addresses we found out in the enrollement token, the URI - InetAddress.getByName dance is necessary because we have strings of host:port and we want to get the host
   a. If any of these, is not an IP address that we have on the current host, we assume there is a node of the cluster that is not on the current host, and as such we bind transport to non-localhost so that it can talk to that node.

[albertzaharovits]

Here is a concrete suggestion:

    static boolean anyRemoteHostNodeAddress(List<String> allNodesTransportPublishAddresses, InetAddress[] allHostAddresses) {
        final List<InetAddress> allAddressesList = Arrays.asList(allHostAddresses);
        for (String nodeStringAddress : allNodesTransportPublishAddresses) {
            try {
                final URI uri = new URI("http://" + nodeStringAddress);
                final InetAddress nodeAddress = InetAddress.getByName(uri.getHost());
                if (false == nodeAddress.isLoopbackAddress() && false == allAddressesList.contains(nodeAddress)) {
                    // this node's address is on a remote host
                    return true;
                }
            } catch (URISyntaxException | UnknownHostException e) {
                // I don't know what to do with invalid ip addresses here. Ignoring
            }
        }
        return false;
    }

I think this captures the point 3 above. We do the String->InetAddress conversion in multiple places, and would be reasonable to factor out, but we don't have to deal with that now.
Point 2. I think is covered by the hostSettingValue method, which will avoid to bind to _site_ in any case, if the host doesn't have "site" addresses configured.

[jkakavas]

Point 2. I think is covered by the hostSettingValue method, which will avoid to bind to site in any case, if the host doesn't have "site" addresses configured.

good point!

Here is a concrete suggestion:

Sounds good to me, I incorporated your suggestion

[albertzaharovits]

I understand you're tackling #82740 (comment) .
If it is important to you, take care to align the leading spacing of subsequent rows.

[jkakavas]

If it is important to you, take care to align the leading spacing of subsequent rows.

I don't think that matters much ( in my opinion at least ). I just wanted to make sure that the words do not collide with the unicode symbols ( not sure if we can fix that universally either but I thought I'd try and we can always iterate based on feedback )

[albertzaharovits]

LGTM

[elasticsearchmachine]

Hi @jkakavas, I've created a changelog YAML for you.

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	8.0