From 9254b1dc8bf9811e685afd01de5b1b259fc3c096 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 2 Oct 2019 16:49:39 +0200 Subject: [PATCH 1/3] Fix es.http.cname_in_publish_address Deprecation Logging Since the property defaulted to `true` this deprecation logging runs every time unless its set to `false` manually (in which case it should've also logged but didn't). Closes #47436 --- .../java/org/elasticsearch/http/HttpInfo.java | 17 ++++------------- .../org/elasticsearch/http/HttpInfoTests.java | 14 ++++---------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index f3457495294a8..b694fc241f291 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpInfo.java +++ b/server/src/main/java/org/elasticsearch/http/HttpInfo.java @@ -33,32 +33,23 @@ import java.io.IOException; -import static org.elasticsearch.common.Booleans.parseBoolean; - public class HttpInfo implements Writeable, ToXContentFragment { private static final DeprecationLogger deprecationLogger = 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"), true); + /** Deprecated property, just here for deprecation logging in 7.x. */ + private static final boolean CNAME_IN_PUBLISH_HOST = System.getProperty("es.http.cname_in_publish_address") != null; private final BoundTransportAddress address; private final long maxContentLength; - private final boolean cnameInPublishHostProperty; public HttpInfo(StreamInput in) throws IOException { - this(new BoundTransportAddress(in), in.readLong(), CNAME_IN_PUBLISH_HOST); + this(new BoundTransportAddress(in), in.readLong()); } public HttpInfo(BoundTransportAddress address, long maxContentLength) { - this(address, maxContentLength, CNAME_IN_PUBLISH_HOST); - } - - HttpInfo(BoundTransportAddress address, long maxContentLength, boolean cnameInPublishHostProperty) { this.address = address; this.maxContentLength = maxContentLength; - this.cnameInPublishHostProperty = cnameInPublishHostProperty; } @Override @@ -84,7 +75,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws String hostString = publishAddress.address().getHostString(); if (InetAddresses.isInetAddress(hostString) == false) { publishAddressString = hostString + '/' + publishAddress.toString(); - if (cnameInPublishHostProperty) { + if (CNAME_IN_PUBLISH_HOST) { deprecationLogger.deprecated( "es.http.cname_in_publish_address system property is deprecated and no longer affects http.publish_address " + "formatting. Remove this property to get rid of this deprecation warning." diff --git a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java index cd0cf7e189429..a9fa129d518bd 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java @@ -40,7 +40,7 @@ public void testCorrectlyDisplayPublishedCname() throws Exception { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L, false + ), 0L ), "localhost/" + NetworkAddress.format(localhost) + ':' + port ); } @@ -53,12 +53,9 @@ public void testDeprecatedWarningIfPropertySpecified() throws Exception { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L, true + ), 0L ), "localhost/" + NetworkAddress.format(localhost) + ':' + port ); - assertWarnings( - "es.http.cname_in_publish_address system property is deprecated and no longer affects http.publish_address " + - "formatting. Remove this property to get rid of this deprecation warning."); } public void testCorrectDisplayPublishedIp() throws Exception { @@ -69,7 +66,7 @@ public void testCorrectDisplayPublishedIp() throws Exception { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L, false + ), 0L ), NetworkAddress.format(localhost) + ':' + port ); } @@ -79,10 +76,7 @@ public void testCorrectDisplayPublishedIpv6() throws Exception { 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, false - ), localhost.toString() - ); + new HttpInfo(new BoundTransportAddress(new TransportAddress[]{localhost}, localhost), 0L), localhost.toString()); } @SuppressWarnings("unchecked") From e94a050e07983de8ec432cadf199d24c2b516f8c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 2 Oct 2019 16:56:58 +0200 Subject: [PATCH 2/3] move check to be tested by REST tests --- .../main/java/org/elasticsearch/http/HttpInfo.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index b694fc241f291..e792647f4a70d 100644 --- a/server/src/main/java/org/elasticsearch/http/HttpInfo.java +++ b/server/src/main/java/org/elasticsearch/http/HttpInfo.java @@ -73,14 +73,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws TransportAddress publishAddress = address.publishAddress(); String publishAddressString = publishAddress.toString(); String hostString = publishAddress.address().getHostString(); + if (CNAME_IN_PUBLISH_HOST) { + deprecationLogger.deprecated( + "es.http.cname_in_publish_address system property is deprecated and no longer affects http.publish_address " + + "formatting. Remove this property to get rid of this deprecation warning." + ); + } if (InetAddresses.isInetAddress(hostString) == false) { publishAddressString = hostString + '/' + publishAddress.toString(); - if (CNAME_IN_PUBLISH_HOST) { - deprecationLogger.deprecated( - "es.http.cname_in_publish_address system property is deprecated and no longer affects http.publish_address " + - "formatting. Remove this property to get rid of this deprecation warning." - ); - } } builder.field(Fields.PUBLISH_ADDRESS, publishAddressString); builder.humanReadableField(Fields.MAX_CONTENT_LENGTH_IN_BYTES, Fields.MAX_CONTENT_LENGTH, maxContentLength()); From 16608b97dfe2ded862e27868a8cefa4c012b5dc3 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 2 Oct 2019 16:58:04 +0200 Subject: [PATCH 3/3] align tests with master --- .../org/elasticsearch/http/HttpInfoTests.java | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java index a9fa129d518bd..89abe0b8f980e 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java @@ -45,18 +45,6 @@ public void testCorrectlyDisplayPublishedCname() throws Exception { ); } - public void testDeprecatedWarningIfPropertySpecified() 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"))); @@ -76,7 +64,10 @@ public void testCorrectDisplayPublishedIpv6() throws Exception { 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), localhost.toString()); + new HttpInfo( + new BoundTransportAddress(new TransportAddress[]{localhost}, localhost), 0L + ), localhost.toString() + ); } @SuppressWarnings("unchecked")