From 2fe6c67839a7663087eb0973487d9e94b2ea6bac Mon Sep 17 00:00:00 2001 From: Armin Date: Mon, 13 Aug 2018 13:10:41 +0200 Subject: [PATCH 1/5] NETWORKING: http.publish_host Should Contain CNAME * Closes #22029 --- .../main/java/org/elasticsearch/http/HttpInfo.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index 4e944a0f7fac8..c31aec9337fba 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpInfo.java +++ b/server/src/main/java/org/elasticsearch/http/HttpInfo.java @@ -19,10 +19,13 @@ package org.elasticsearch.http; +import java.net.InetSocketAddress; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -62,7 +65,16 @@ static final class Fields { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.HTTP); builder.array(Fields.BOUND_ADDRESS, (Object[]) address.boundAddresses()); - builder.field(Fields.PUBLISH_ADDRESS, address.publishAddress().toString()); + TransportAddress publishAddress = address.publishAddress(); + InetSocketAddress publishInetAddress = publishAddress.address(); + String hostString = publishInetAddress.getHostString(); + final String publishAddressString; + if (hostString.equals(NetworkAddress.format(publishInetAddress.getAddress()))) { + publishAddressString = publishAddress.toString(); + } else { + publishAddressString = hostString + '/' + publishAddress.toString(); + } + builder.field(Fields.PUBLISH_ADDRESS, publishAddressString); builder.humanReadableField(Fields.MAX_CONTENT_LENGTH_IN_BYTES, Fields.MAX_CONTENT_LENGTH, maxContentLength()); builder.endObject(); return builder; From ace1f4536ac940768559da1917b56155b9397c7f Mon Sep 17 00:00:00 2001 From: Armin Date: Mon, 13 Aug 2018 14:43:24 +0200 Subject: [PATCH 2/5] add tests --- .../org/elasticsearch/http/HttpInfoTests.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/http/HttpInfoTests.java diff --git a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java new file mode 100644 index 0000000000000..dad01fd94d4e4 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java @@ -0,0 +1,73 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.http; + +import java.io.IOException; +import java.net.InetAddress; +import java.util.Map; +import org.elasticsearch.common.network.NetworkAddress; +import org.elasticsearch.common.transport.BoundTransportAddress; +import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.test.ESTestCase; + +public class HttpInfoTests extends ESTestCase { + + public void testCorrectlyDisplayPublishedCname() throws Exception { + InetAddress localhost = InetAddress.getByName("localhost"); + int port = 9200; + assertPublishAddress( + new HttpInfo( + new BoundTransportAddress( + new TransportAddress[]{new TransportAddress(localhost, port)}, + new TransportAddress(localhost, port) + ), 0L + ), "localhost/" + NetworkAddress.format(localhost) + ':' + port + ); + } + + public void testCorrectDisplayPublishedIp() throws Exception { + InetAddress localhost = InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("localhost"))); + int port = 9200; + assertPublishAddress( + new HttpInfo( + new BoundTransportAddress( + new TransportAddress[]{new TransportAddress(localhost, port)}, + new TransportAddress(localhost, port) + ), 0L + ), NetworkAddress.format(localhost) + ':' + port + ); + } + + @SuppressWarnings("unchecked") + private void assertPublishAddress(HttpInfo httpInfo, String expected) throws IOException { + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + httpInfo.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + assertEquals( + expected, + ((Map) createParser(builder).map().get(HttpInfo.Fields.HTTP)) + .get(HttpInfo.Fields.PUBLISH_ADDRESS) + ); + } +} From 33e3c41786bb9e8dd2347cec01ab0b7201a6837c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 30 Aug 2018 14:23:10 +0200 Subject: [PATCH 3/5] CR: Add deprecation behaviour --- .../elasticsearch/gradle/BuildPlugin.groovy | 3 ++ docs/build.gradle | 2 + modules/lang-painless/build.gradle | 1 + .../java/org/elasticsearch/http/HttpInfo.java | 45 ++++++++++++++----- .../org/elasticsearch/http/HttpInfoTests.java | 17 ++++++- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 4c4a8cbe8810d..4169dddd20aa1 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -828,6 +828,9 @@ class BuildPlugin implements Plugin { // TODO: remove this once ctx isn't added to update script params in 7.0 systemProperty 'es.scripting.update.ctx_in_params', 'false' + //TODO: remove this once the cname is prepended to the address by default in 7.0 + systemProperty 'es.http.cname_in_publish_address', 'true' + // Set the system keystore/truststore password if we're running tests in a FIPS-140 JVM if (project.inFipsJvm) { systemProperty 'javax.net.ssl.trustStorePassword', 'password' diff --git a/docs/build.gradle b/docs/build.gradle index 980c99baf832c..85ec01c489596 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -41,6 +41,8 @@ integTestCluster { // TODO: remove this for 7.0, this exists to allow the doc examples in 6.x to continue using the defaults systemProperty 'es.scripting.use_java_time', 'false' systemProperty 'es.scripting.update.ctx_in_params', 'false' + //TODO: remove this once the cname is prepended to the address by default in 7.0 + systemProperty 'es.http.cname_in_publish_address', 'true' } // remove when https://github.com/elastic/elasticsearch/issues/31305 is fixed diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle index ed4b1d631e064..6bec6f5062685 100644 --- a/modules/lang-painless/build.gradle +++ b/modules/lang-painless/build.gradle @@ -26,6 +26,7 @@ integTestCluster { module project.project(':modules:mapper-extras') systemProperty 'es.scripting.use_java_time', 'true' systemProperty 'es.scripting.update.ctx_in_params', 'false' + systemProperty 'es.http.cname_in_publish_address', 'true' } dependencies { diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index c31aec9337fba..cb613ee9ae01f 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpInfo.java +++ b/server/src/main/java/org/elasticsearch/http/HttpInfo.java @@ -20,9 +20,11 @@ package org.elasticsearch.http; import java.net.InetSocketAddress; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; @@ -32,25 +34,38 @@ import java.io.IOException; +import static org.elasticsearch.common.Booleans.parseBoolean; + public class HttpInfo implements Writeable, ToXContentFragment { + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LogManager.getLogger(HttpInfo.class)); + + /** Whether to add hostname to publish host field when serializing. */ + private static final boolean CNAME_IN_PUBLISH_HOST = + parseBoolean(System.getProperty("es.http.cname_in_publish_address"), false); + private final BoundTransportAddress address; private final long maxContentLength; + private final boolean cnameInPublishHost; public HttpInfo(StreamInput in) throws IOException { - address = BoundTransportAddress.readBoundTransportAddress(in); - maxContentLength = in.readLong(); + this(BoundTransportAddress.readBoundTransportAddress(in), in.readLong(), CNAME_IN_PUBLISH_HOST); } - @Override - public void writeTo(StreamOutput out) throws IOException { - address.writeTo(out); - out.writeLong(maxContentLength); + public HttpInfo(BoundTransportAddress address, long maxContentLength) { + this(address, maxContentLength, CNAME_IN_PUBLISH_HOST); } - public HttpInfo(BoundTransportAddress address, long maxContentLength) { + HttpInfo(BoundTransportAddress address, long maxContentLength, boolean cnameInPublishHost) { this.address = address; this.maxContentLength = maxContentLength; + this.cnameInPublishHost = cnameInPublishHost; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + address.writeTo(out); + out.writeLong(maxContentLength); } static final class Fields { @@ -66,13 +81,19 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(Fields.HTTP); builder.array(Fields.BOUND_ADDRESS, (Object[]) address.boundAddresses()); TransportAddress publishAddress = address.publishAddress(); + String publishAddressString = publishAddress.toString(); InetSocketAddress publishInetAddress = publishAddress.address(); String hostString = publishInetAddress.getHostString(); - final String publishAddressString; - if (hostString.equals(NetworkAddress.format(publishInetAddress.getAddress()))) { - publishAddressString = publishAddress.toString(); - } else { - publishAddressString = hostString + '/' + publishAddress.toString(); + if (!hostString.equals(NetworkAddress.format(publishInetAddress.getAddress()))) { + if (cnameInPublishHost) { + publishAddressString = hostString + '/' + publishAddress.toString(); + } else { + DEPRECATION_LOGGER.deprecated( + "[http.publish_host] was printed as [ip:port] instead of [hostname/ip:port]. " + + "This format is deprecated and will change to [hostname/ip:port] in a future version. " + + "Use -Des.http.cname_in_publish_host=true to enforce non-deprecated formatting." + ); + } } builder.field(Fields.PUBLISH_ADDRESS, publishAddressString); builder.humanReadableField(Fields.MAX_CONTENT_LENGTH_IN_BYTES, Fields.MAX_CONTENT_LENGTH, maxContentLength()); diff --git a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java index dad01fd94d4e4..336a67661c126 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java @@ -40,11 +40,24 @@ public void testCorrectlyDisplayPublishedCname() throws Exception { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L + ), 0L, true ), "localhost/" + NetworkAddress.format(localhost) + ':' + port ); } + public void hideCnameIfDeprecatedFormat() throws Exception { + InetAddress localhost = InetAddress.getByName("localhost"); + int port = 9200; + assertPublishAddress( + new HttpInfo( + new BoundTransportAddress( + new TransportAddress[]{new TransportAddress(localhost, port)}, + new TransportAddress(localhost, port) + ), 0L, false + ), NetworkAddress.format(localhost) + ':' + port + ); + } + public void testCorrectDisplayPublishedIp() throws Exception { InetAddress localhost = InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("localhost"))); int port = 9200; @@ -53,7 +66,7 @@ public void testCorrectDisplayPublishedIp() throws Exception { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L + ), 0L, true ), NetworkAddress.format(localhost) + ':' + port ); } From 2a1f1018dbfc4cd09f5ffbd9f6da5417940b49e7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 30 Aug 2018 14:23:58 +0200 Subject: [PATCH 4/5] CR: Add deprecation behaviour --- server/src/main/java/org/elasticsearch/http/HttpInfo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index cb613ee9ae01f..f8e126401bc75 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpInfo.java +++ b/server/src/main/java/org/elasticsearch/http/HttpInfo.java @@ -91,7 +91,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws DEPRECATION_LOGGER.deprecated( "[http.publish_host] was printed as [ip:port] instead of [hostname/ip:port]. " + "This format is deprecated and will change to [hostname/ip:port] in a future version. " - + "Use -Des.http.cname_in_publish_host=true to enforce non-deprecated formatting." + + "Use -Des.http.cname_in_publish_address=true to enforce non-deprecated formatting." ); } } From fe3e1613dcda58a9e8fe937f60725af2f07c437c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 11 Sep 2018 10:12:11 +0200 Subject: [PATCH 5/5] CR: correctly handle IPV6 --- .../main/java/org/elasticsearch/http/HttpInfo.java | 8 +++----- .../java/org/elasticsearch/http/HttpInfoTests.java | 11 +++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index f8e126401bc75..aece813199479 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpInfo.java +++ b/server/src/main/java/org/elasticsearch/http/HttpInfo.java @@ -19,13 +19,12 @@ package org.elasticsearch.http; -import java.net.InetSocketAddress; import org.apache.logging.log4j.LogManager; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.network.NetworkAddress; +import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.unit.ByteSizeValue; @@ -82,9 +81,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.array(Fields.BOUND_ADDRESS, (Object[]) address.boundAddresses()); TransportAddress publishAddress = address.publishAddress(); String publishAddressString = publishAddress.toString(); - InetSocketAddress publishInetAddress = publishAddress.address(); - String hostString = publishInetAddress.getHostString(); - if (!hostString.equals(NetworkAddress.format(publishInetAddress.getAddress()))) { + String hostString = publishAddress.address().getHostString(); + if (InetAddresses.isInetAddress(hostString) == false) { if (cnameInPublishHost) { publishAddressString = hostString + '/' + publishAddress.toString(); } else { diff --git a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java index 336a67661c126..db149bd6d0db0 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java @@ -71,6 +71,17 @@ public void testCorrectDisplayPublishedIp() throws Exception { ); } + public void testCorrectDisplayPublishedIpv6() throws Exception { + int port = 9200; + TransportAddress localhost = + new TransportAddress(InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("0:0:0:0:0:0:0:1"))), port); + assertPublishAddress( + new HttpInfo( + new BoundTransportAddress(new TransportAddress[]{localhost}, localhost), 0L, true + ), localhost.toString() + ); + } + @SuppressWarnings("unchecked") private void assertPublishAddress(HttpInfo httpInfo, String expected) throws IOException { XContentBuilder builder = XContentFactory.jsonBuilder();