NETWORKING: http.publish_host Should Contain CNAME

[original-brownbear]

• Restores old 2.x behaviour of prepending the hostname to http.publish_host if available
• closes Should http.publish_host resolve the CNAME configured. #22029

[elasticmachine]

Pinging @elastic/es-core-infra

[jasontedor]

After discussion with @elastic/es-clients we need to take a more careful approach here as this change will break some of the official language clients. Instead, we should:

• in 6.x we will retain the current behavior of not including the hostname in the publish address
• however, we will deprecation warn if the publish address would include the hostname with the behavior that we are proposing in NETWORKING: http.publish_host Should Contain CNAME #32806
• in 6.x we will introduce a system property that will enable users that want the hostname in the publish address to do so by using this system property; this system property can only be set to true and not to false as the system property is unneeded if the user wants to retain the current behavior
• in 7.0.0 we will change the behavior to include the hostname in the publish address (if available)
• in 7.x we can deprecation warn that the system property no longer has any impact

[original-brownbear]

@jasontedor sounds good :) => on it :)

[original-brownbear]

@jasontedor done I think, implemented the 6.x behavior here now. I guess the 7.0 change goes into a separate PR right?

Transferring review ownership to @tbrooks8

[Tim-Brooks]

I'm not super familiar with the behavior we are looking for here. But this looks alright to me in regards to the API response / deprecation.

However, it is trivial to make it have some weird behavior in regards to IPv6.

public void testCorrectDisplayPublishedIpv6() throws Exception {
        InetAddress localhost = InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("0:0:0:0:0:0:0:1")));
        int port = 9200;
        assertPublishAddress(
            new HttpInfo(
                new BoundTransportAddress(
                    new TransportAddress[]{new TransportAddress(localhost, port)},
                    new TransportAddress(localhost, port)
                ), 0L, true
            ), NetworkAddress.format(localhost) + ':' + port
        );
    }

org.junit.ComparisonFailure: expected:<[::1]:9200> but was:<[0:0:0:0:0:0:0:1/[::1]]:9200>

Are we okay that IPv6 address will be in the host string section if the comparison fails due to address compression?

@jasontedor

I don't see an easy fix for that. We could always add like a new field for v7.0. But that introduces some backwards compatibility work.

[original-brownbear]

@tbrooks8 fixed the test case you pointed out now :) We already had a utility method (probably copied from Guava) that can identify whether or not a String is a valid IP (4 and 6) => just used that => works fine :)

[Tim-Brooks]

LGTM

[original-brownbear]

@tbrooks8 thanks!