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
This commit is contained in:
Armin Braun 2019-10-03 11:10:48 +02:00 committed by GitHub
parent 3bb17a0c28
commit 7549be4489
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 36 deletions

View File

@ -33,32 +33,23 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import java.io.IOException; import java.io.IOException;
import static org.elasticsearch.common.Booleans.parseBoolean;
public class HttpInfo implements Writeable, ToXContentFragment { public class HttpInfo implements Writeable, ToXContentFragment {
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(HttpInfo.class)); private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(HttpInfo.class));
/** Whether to add hostname to publish host field when serializing. */ /** Deprecated property, just here for deprecation logging in 7.x. */
private static final boolean CNAME_IN_PUBLISH_HOST = private static final boolean CNAME_IN_PUBLISH_HOST = System.getProperty("es.http.cname_in_publish_address") != null;
parseBoolean(System.getProperty("es.http.cname_in_publish_address"), true);
private final BoundTransportAddress address; private final BoundTransportAddress address;
private final long maxContentLength; private final long maxContentLength;
private final boolean cnameInPublishHostProperty;
public HttpInfo(StreamInput in) throws IOException { 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) { public HttpInfo(BoundTransportAddress address, long maxContentLength) {
this(address, maxContentLength, CNAME_IN_PUBLISH_HOST);
}
HttpInfo(BoundTransportAddress address, long maxContentLength, boolean cnameInPublishHostProperty) {
this.address = address; this.address = address;
this.maxContentLength = maxContentLength; this.maxContentLength = maxContentLength;
this.cnameInPublishHostProperty = cnameInPublishHostProperty;
} }
@Override @Override
@ -82,14 +73,14 @@ public class HttpInfo implements Writeable, ToXContentFragment {
TransportAddress publishAddress = address.publishAddress(); TransportAddress publishAddress = address.publishAddress();
String publishAddressString = publishAddress.toString(); String publishAddressString = publishAddress.toString();
String hostString = publishAddress.address().getHostString(); String hostString = publishAddress.address().getHostString();
if (InetAddresses.isInetAddress(hostString) == false) { if (CNAME_IN_PUBLISH_HOST) {
publishAddressString = hostString + '/' + publishAddress.toString();
if (cnameInPublishHostProperty) {
deprecationLogger.deprecated( deprecationLogger.deprecated(
"es.http.cname_in_publish_address system property is deprecated and no longer affects http.publish_address " + "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." "formatting. Remove this property to get rid of this deprecation warning."
); );
} }
if (InetAddresses.isInetAddress(hostString) == false) {
publishAddressString = hostString + '/' + publishAddress.toString();
} }
builder.field(Fields.PUBLISH_ADDRESS, publishAddressString); builder.field(Fields.PUBLISH_ADDRESS, publishAddressString);
builder.humanReadableField(Fields.MAX_CONTENT_LENGTH_IN_BYTES, Fields.MAX_CONTENT_LENGTH, maxContentLength()); builder.humanReadableField(Fields.MAX_CONTENT_LENGTH_IN_BYTES, Fields.MAX_CONTENT_LENGTH, maxContentLength());

View File

@ -40,26 +40,11 @@ public class HttpInfoTests extends ESTestCase {
new BoundTransportAddress( new BoundTransportAddress(
new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress[]{new TransportAddress(localhost, port)},
new TransportAddress(localhost, port) new TransportAddress(localhost, port)
), 0L, false ), 0L
), "localhost/" + NetworkAddress.format(localhost) + ':' + port ), "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 { public void testCorrectDisplayPublishedIp() throws Exception {
InetAddress localhost = InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("localhost"))); InetAddress localhost = InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("localhost")));
@ -69,7 +54,7 @@ public class HttpInfoTests extends ESTestCase {
new BoundTransportAddress( new BoundTransportAddress(
new TransportAddress[]{new TransportAddress(localhost, port)}, new TransportAddress[]{new TransportAddress(localhost, port)},
new TransportAddress(localhost, port) new TransportAddress(localhost, port)
), 0L, false ), 0L
), NetworkAddress.format(localhost) + ':' + port ), 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); new TransportAddress(InetAddress.getByName(NetworkAddress.format(InetAddress.getByName("0:0:0:0:0:0:0:1"))), port);
assertPublishAddress( assertPublishAddress(
new HttpInfo( new HttpInfo(
new BoundTransportAddress(new TransportAddress[]{localhost}, localhost), 0L, false new BoundTransportAddress(new TransportAddress[]{localhost}, localhost), 0L
), localhost.toString() ), localhost.toString()
); );
} }