From 7549be44891dcecbd8e618a6da35227701bcfa40 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 3 Oct 2019 11:10:48 +0200 Subject: [PATCH] Fix es.http.cname_in_publish_address Deprecation Logging (#47451) 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). I didn't add a tests and removed the tests we had in `7.x` that covered this logging. I did move the check out of the `if (InetAddresses.isInetAddress(hostString) == false) {` condition so this is sort-of covered by the REST tests. IMO, any unit-test of this would be somewhat redundant and would've forced adding a field that just indicates that the deprecated property was used to every instance which seemed pointless. Closes #47436 --- .../java/org/elasticsearch/http/HttpInfo.java | 27 +++++++------------ .../org/elasticsearch/http/HttpInfoTests.java | 21 +++------------ 2 files changed, 12 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/http/HttpInfo.java b/server/src/main/java/org/elasticsearch/http/HttpInfo.java index f3457495294..e792647f4a7 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 org.elasticsearch.common.xcontent.XContentBuilder; 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 @@ -82,14 +73,14 @@ public class HttpInfo implements Writeable, ToXContentFragment { 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 (cnameInPublishHostProperty) { - 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()); diff --git a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java index cd0cf7e1894..89abe0b8f98 100644 --- a/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java +++ b/server/src/test/java/org/elasticsearch/http/HttpInfoTests.java @@ -40,26 +40,11 @@ public class HttpInfoTests extends ESTestCase { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L, false + ), 0L ), "localhost/" + NetworkAddress.format(localhost) + ':' + port ); } - 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, true - ), "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 { InetAddress localhost = InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("localhost"))); @@ -69,7 +54,7 @@ public class HttpInfoTests extends ESTestCase { new BoundTransportAddress( new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress(localhost, port) - ), 0L, false + ), 0L ), NetworkAddress.format(localhost) + ':' + port ); } @@ -80,7 +65,7 @@ public class HttpInfoTests extends ESTestCase { 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 + new BoundTransportAddress(new TransportAddress[]{localhost}, localhost), 0L ), localhost.toString() ); }