From b1dfda342e8065378234afc2c73a31db96d54f59 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 13 Oct 2021 17:56:00 -0500 Subject: [PATCH 01/12] Exposing the ability to log deprecated settings at non-critical level --- .../common/logging/HeaderWarning.java | 38 +++++++++++++------ .../common/logging/HeaderWarningAppender.java | 4 +- .../common/settings/Setting.java | 24 +++++++++--- .../common/logging/HeaderWarningTests.java | 20 ++++++++++ .../common/settings/SettingTests.java | 23 ++++++++++- .../org/elasticsearch/test/ESTestCase.java | 30 +++++++++++++-- 6 files changed, 116 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java index 1b76e7bbcc76a..d391f70f14a50 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java +++ b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java @@ -8,6 +8,7 @@ package org.elasticsearch.common.logging; +import org.apache.logging.log4j.Level; import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -32,10 +33,11 @@ public class HeaderWarning { /** * Regular expression to test if a string matches the RFC7234 specification for warning headers. This pattern assumes that the warn code - * is always 299. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build hash. + * is always 299 or 300. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build + * hash. */ public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile( - "299 " + // warn code + "(?:299|300) " + // log level code "Elasticsearch-" + // warn agent "\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-" + // warn agent "(?:[a-f0-9]{7}(?:[a-f0-9]{33})?|unknown) " + // warn agent @@ -53,15 +55,16 @@ public class HeaderWarning { /* * RFC7234 specifies the warning format as warn-code warn-agent "warn-text" [ "warn-date"]. Here, warn-code is a - * three-digit number with various standard warn codes specified. The warn code 299 is apt for our purposes as it represents a - * miscellaneous persistent warning (can be presented to a human, or logged, and must not be removed by a cache). The warn-agent is an - * arbitrary token; here we use the Elasticsearch version and build hash. The warn text must be quoted. The warn-date is an optional - * quoted field that can be in a variety of specified date formats; here we use RFC 1123 format. + * three-digit number with various standard warn codes specified, and is left off of this static prefix so that it can be added based + * on the log level received. The warn code will be either 299 or 300 at runtime, which are apt for our purposes as + * they represent miscellaneous persistent warnings (can be presented to a human, or logged, and must not be removed by a cache). + * The warn-agent is an arbitrary token; here we use the Elasticsearch version and build hash. The warn text must be quoted. The + * warn-date is an optional quoted field that can be in a variety of specified date formats; here we use RFC 1123 format. */ private static final String WARNING_PREFIX = String.format( Locale.ROOT, - "299 Elasticsearch-%s%s-%s", + " Elasticsearch-%s%s-%s", Version.CURRENT.toString(), Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "", Build.CURRENT.hash()); @@ -193,10 +196,14 @@ private static boolean assertWarningValue(final String s, final String warningVa * @return a warning value formatted according to RFC 7234 */ public static String formatWarning(final String s) { + return formatWarning(DeprecationLogger.CRITICAL, s); + } + + private static String formatWarning(final Level level, final String s) { // Assume that the common scenario won't have a string to escape and encode. - int length = WARNING_PREFIX.length() + s.length() + 3; + int length = WARNING_PREFIX.length() + s.length() + 6; final StringBuilder sb = new StringBuilder(length); - sb.append(WARNING_PREFIX).append(" \"").append(escapeAndEncode(s)).append("\""); + sb.append(level.intLevel() + WARNING_PREFIX).append(" \"").append(escapeAndEncode(s)).append("\""); return sb.toString(); } @@ -311,15 +318,24 @@ public static String getXOpaqueId() { } public static void addWarning(String message, Object... params) { - addWarning(THREAD_CONTEXT, message, params); + addWarning(THREAD_CONTEXT, DeprecationLogger.CRITICAL, message, params); + } + + public static void addWarning(Level level, String message, Object... params) { + addWarning(THREAD_CONTEXT, level, message, params); } // package scope for testing static void addWarning(Set threadContexts, String message, Object... params) { + addWarning(threadContexts, DeprecationLogger.CRITICAL, message, params); + } + + // package scope for testing + static void addWarning(Set threadContexts, Level level, String message, Object... params) { final Iterator iterator = threadContexts.iterator(); if (iterator.hasNext()) { final String formattedMessage = LoggerMessageFormat.format(message, params); - final String warningHeaderValue = formatWarning(formattedMessage); + final String warningHeaderValue = formatWarning(level, formattedMessage); assert WARNING_HEADER_PATTERN.matcher(warningHeaderValue).matches(); assert extractWarningValueFromWarningHeader(warningHeaderValue, false) .equals(escapeAndEncode(formattedMessage)); diff --git a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarningAppender.java b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarningAppender.java index ad03eccdf638d..992f33a08e597 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarningAppender.java +++ b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarningAppender.java @@ -35,10 +35,10 @@ public void append(LogEvent event) { String messagePattern = esLogMessage.getMessagePattern(); Object[] arguments = esLogMessage.getArguments(); - HeaderWarning.addWarning(messagePattern, arguments); + HeaderWarning.addWarning(event.getLevel(), messagePattern, arguments); } else { final String formattedMessage = event.getMessage().getFormattedMessage(); - HeaderWarning.addWarning(formattedMessage); + HeaderWarning.addWarning(event.getLevel(), formattedMessage); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index ccd25b89846f0..a6e74265128ed 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -99,10 +99,15 @@ public enum Property { Final, /** - * mark this setting as deprecated + * mark this setting as deprecated (critical level) */ Deprecated, + /** + * mark this setting as deprecated (warning level) + */ + DeprecatedWarning, + /** * Node scope */ @@ -371,7 +376,11 @@ public boolean hasIndexScope() { * Returns true if this setting is deprecated, otherwise false */ public boolean isDeprecated() { - return properties.contains(Property.Deprecated); + return properties.contains(Property.Deprecated) || properties.contains(Property.DeprecatedWarning); + } + + private boolean isDeprecatedWarningOnly() { + return properties.contains(Property.DeprecatedWarning) && properties.contains(Property.Deprecated) == false; } /** @@ -552,10 +561,13 @@ void checkDeprecation(Settings settings) { if (this.isDeprecated() && this.exists(settings)) { // It would be convenient to show its replacement key, but replacement is often not so simple final String key = getKey(); - Settings.DeprecationLoggerHolder.deprecationLogger - .critical(DeprecationCategory.SETTINGS, key, - "[{}] setting was deprecated in Elasticsearch and will be removed in a future release! " - + "See the breaking changes documentation for the next major version.", key); + String message = "[{}] setting was deprecated in Elasticsearch and will be removed in a future release! " + + "See the breaking changes documentation for the next major version."; + if (this.isDeprecatedWarningOnly()) { + Settings.DeprecationLoggerHolder.deprecationLogger.warn(DeprecationCategory.SETTINGS, key, message, key); + } else { + Settings.DeprecationLoggerHolder.deprecationLogger.critical(DeprecationCategory.SETTINGS, key, message, key); + } } } diff --git a/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java b/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java index 9af702802a66c..c96660f4ef729 100644 --- a/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java +++ b/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.common.logging; import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator; + +import org.apache.logging.log4j.Level; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.test.ESTestCase; @@ -285,4 +287,22 @@ private String range(int lowerInclusive, int upperInclusive) { .toString(); } + public void testAddWarningNonDefaultLogLevel() { + final int maxWarningHeaderCount = 2; + Settings settings = Settings.builder() + .put("http.max_warning_header_count", maxWarningHeaderCount) + .build(); + ThreadContext threadContext = new ThreadContext(settings); + final Set threadContexts = Collections.singleton(threadContext); + HeaderWarning.addWarning(threadContexts, Level.WARN, "A simple message 1"); + final Map> responseHeaders = threadContext.getResponseHeaders(); + + assertThat(responseHeaders.size(), equalTo(1)); + final List responses = responseHeaders.get("Warning"); + assertThat(responses, hasSize(1)); + assertThat(responses.get(0), warningValueMatcher); + assertThat(responses.get(0), containsString("\"A simple message 1\"")); + assertThat(responses.get(0), containsString(Integer.toString(Level.WARN.intLevel()))); + } + } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 9f7b4092cacf5..ad057aef07155 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -12,13 +12,13 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.LogEvent; import org.elasticsearch.cluster.metadata.IndexMetadata; -import org.elasticsearch.core.Tuple; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.AbstractScopedSettings.SettingUpdater; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.monitor.jvm.JvmInfo; import org.elasticsearch.test.ESTestCase; @@ -1258,4 +1258,25 @@ public void testDynamicTest() { assertTrue(setting.isDynamic()); assertEquals(setting.isOperatorOnly(), property == Property.OperatorDynamic); } + + public void testCheckForDeprecation() { + final String criticalSettingName = "foo.bar"; + final String warningSettingName = "foo.foo"; + final String settingValue = "blat"; + final Setting undeprecatedSetting1 = Setting.simpleString(criticalSettingName, settingValue); + final Setting undeprecatedSetting2 = Setting.simpleString(warningSettingName, settingValue); + final Settings settings = Settings.builder() + .put(criticalSettingName, settingValue) + .put(warningSettingName, settingValue).build(); + undeprecatedSetting1.checkDeprecation(settings); + undeprecatedSetting2.checkDeprecation(settings); + ensureNoWarnings(); + final Setting criticalDeprecatedSetting = Setting.simpleString(criticalSettingName, settingValue, Property.Deprecated); + criticalDeprecatedSetting.checkDeprecation(settings); + assertSettingDeprecationsAndWarningsIncludingLevel(new Setting[]{ criticalDeprecatedSetting }); + final Setting deprecatedSettingWarningOnly = + Setting.simpleString(warningSettingName, settingValue, Property.DeprecatedWarning); + deprecatedSettingWarningOnly.checkDeprecation(settings); + assertSettingDeprecationsAndWarningsIncludingLevel(new Setting[]{ deprecatedSettingWarningOnly }); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 23807633bec22..4c28697fae47e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -35,11 +35,13 @@ import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; import org.apache.lucene.util.TestRuleMarkFailure; import org.apache.lucene.util.TimeUnits; +import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.bootstrap.BootstrapForTesting; import org.elasticsearch.client.Requests; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.CheckedRunnable; import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.SuppressForbidden; @@ -458,11 +460,31 @@ protected final void assertSettingDeprecationsAndWarnings(final String[] setting .toArray(String[]::new)); } + protected final void assertSettingDeprecationsAndWarningsIncludingLevel(final Setting[] settings, final String... warnings) { + assertWarnings( + true, + true, + Stream.concat( + Arrays + .stream(settings) + .map(setting -> String.format( + Locale.ROOT, "%s Elasticsearch-%s%s-%s \"[%s] setting was deprecated in Elasticsearch and will be " + + "removed in a future release! See the breaking changes documentation for the next major version.\"", + setting.getProperties().contains(Setting.Property.Deprecated) ? DeprecationLogger.CRITICAL.intLevel() : + Level.WARN.intLevel(), + Version.CURRENT.toString(), + Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "", + Build.CURRENT.hash(), + setting.getKey())), + Arrays.stream(warnings)) + .toArray(String[]::new)); + } + protected final void assertWarnings(String... expectedWarnings) { - assertWarnings(true, expectedWarnings); + assertWarnings(true, false, expectedWarnings); } - protected final void assertWarnings(boolean stripXContentPosition, String... expectedWarnings) { + protected final void assertWarnings(boolean stripXContentPosition, boolean includeLevelCheck, String... expectedWarnings) { if (enableWarningsCheck() == false) { throw new IllegalStateException("unable to check warning headers if the test is not set to do so"); } @@ -473,7 +495,9 @@ protected final void assertWarnings(boolean stripXContentPosition, String... exp } else { assertNotNull("no warnings, expected: " + Arrays.asList(expectedWarnings), actualWarnings); final Set actualWarningValues = - actualWarnings.stream().map(s -> HeaderWarning.extractWarningValueFromWarningHeader(s, stripXContentPosition)) + actualWarnings.stream() + .map(s -> includeLevelCheck ? HeaderWarning.escapeAndEncode(s) : + HeaderWarning.extractWarningValueFromWarningHeader(s, stripXContentPosition)) .collect(Collectors.toSet()); for (String msg : expectedWarnings) { assertThat(actualWarningValues, hasItem(HeaderWarning.escapeAndEncode(msg))); From 19788f429e3906a337d429105e4992bece9414db Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 13 Oct 2021 18:33:24 -0500 Subject: [PATCH 02/12] replacing a previously existing method --- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 4c28697fae47e..27e8f7c0ae229 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -484,6 +484,10 @@ protected final void assertWarnings(String... expectedWarnings) { assertWarnings(true, false, expectedWarnings); } + protected final void assertWarnings(boolean stripXContentPosition, String... expectedWarnings) { + assertWarnings(stripXContentPosition, false, expectedWarnings); + } + protected final void assertWarnings(boolean stripXContentPosition, boolean includeLevelCheck, String... expectedWarnings) { if (enableWarningsCheck() == false) { throw new IllegalStateException("unable to check warning headers if the test is not set to do so"); From d5884c621259fd476d3d953e832cd973adc99ea0 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 14 Oct 2021 10:30:25 -0500 Subject: [PATCH 03/12] fixing tests --- .../src/main/java/org/elasticsearch/client/Response.java | 6 +++--- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index fe5152d956db8..cfe313c744c52 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -136,14 +136,14 @@ public HttpEntity getEntity() { /** * Tests if a string matches the RFC 7234 specification for warning headers. - * This assumes that the warn code is always 299 and the warn agent is always - * Elasticsearch. + * This assumes that the warn code is always 299 or 300 and the warn agent is + * always Elasticsearch. * * @param s the value of a warning header formatted according to RFC 7234 * @return {@code true} if the input string matches the specification */ private static boolean matchWarningHeaderPatternByPrefix(final String s) { - return s.startsWith("299 Elasticsearch-"); + return s.startsWith("299 Elasticsearch-") || s.startsWith("300 Elasticsearch-"); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 27e8f7c0ae229..9104bf14d41f7 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -481,7 +481,7 @@ protected final void assertSettingDeprecationsAndWarningsIncludingLevel(final Se } protected final void assertWarnings(String... expectedWarnings) { - assertWarnings(true, false, expectedWarnings); + assertWarnings(true, expectedWarnings); } protected final void assertWarnings(boolean stripXContentPosition, String... expectedWarnings) { From e25078d53a1eb37cf94092795feb85aacca779d5 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 14 Oct 2021 18:08:11 -0500 Subject: [PATCH 04/12] code review feedback incorporated --- .../ConstructingObjectParserTests.java | 2 +- .../xcontent/ObjectParserTests.java | 4 ++-- .../ingest/geoip/GeoIpProcessor.java | 4 ++-- .../MetadataIndexTemplateService.java | 5 +++-- .../common/logging/HeaderWarning.java | 11 ++-------- .../common/settings/Setting.java | 7 +++++-- .../common/logging/HeaderWarningTests.java | 8 ++++---- .../common/settings/SettingTests.java | 9 +++++++++ .../util/concurrent/ThreadContextTests.java | 5 +++-- .../test/errorquery/ErrorQueryBuilder.java | 3 ++- .../org/elasticsearch/test/ESTestCase.java | 6 +----- .../rest/yaml/section/DoSectionTests.java | 20 ++++++++++--------- .../license/XPackLicenseState.java | 3 ++- .../TransportPutTrainedModelAliasAction.java | 3 ++- ...ransportStartDataFrameAnalyticsAction.java | 3 ++- .../TransportPreviewTransformAction.java | 5 +++-- 16 files changed, 54 insertions(+), 44 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java index e6662ca3d96bc..560523d2f2ec1 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java @@ -626,7 +626,7 @@ public void testCompatibleFieldDeclarations() throws IOException { RestApiVersion.minimumSupported()); StructWithCompatibleFields o = StructWithCompatibleFields.PARSER.parse(parser, null); assertEquals(1, o.intField); - assertWarnings(false, "[struct_with_compatible_fields][1:14] " + + assertWarnings(false, false, "[struct_with_compatible_fields][1:14] " + "Deprecated field [old_name] used, expected [new_name] instead"); } } diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java index 278702a4bb1e0..4ea33c733b40f 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java @@ -211,7 +211,7 @@ class TestStruct { objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); objectParser.parse(parser, s, null); assertEquals("foo", s.test); - assertWarnings(false, "[foo][1:15] Deprecated field [old_test] used, expected [test] instead"); + assertWarnings(false, false, "[foo][1:15] Deprecated field [old_test] used, expected [test] instead"); } public void testFailOnValueType() throws IOException { @@ -1072,7 +1072,7 @@ public void testCompatibleFieldDeclarations() throws IOException { RestApiVersion.minimumSupported()); StructWithCompatibleFields o = StructWithCompatibleFields.PARSER.parse(parser, null); assertEquals(1, o.intField); - assertWarnings(false, "[struct_with_compatible_fields][1:14] " + + assertWarnings(false, false, "[struct_with_compatible_fields][1:14] " + "Deprecated field [old_name] used, expected [new_name] instead"); } diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java index d1d9cd788d2da..21d52715e598b 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java @@ -460,8 +460,8 @@ public Processor create( boolean valid = metadata.isValid(currentState.metadata().settings()); if (valid && metadata.isCloseToExpiration()) { - HeaderWarning.addWarning("database [{}] was not updated for over 25 days, geoip processor will stop working if there " + - "is no update for 30 days", databaseFile); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, "database [{}] was not updated for over 25 days, geoip processor" + + " will stop working if there is no update for 30 days", databaseFile); } return valid; diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java index 56e41f89aa905..959f514c0256a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -24,6 +24,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.Nullable; import org.elasticsearch.common.Priority; import org.elasticsearch.common.Strings; @@ -498,7 +499,7 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo .collect(Collectors.joining(",")), name); logger.warn(warning); - HeaderWarning.addWarning(warning); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning); } ComposableIndexTemplate finalIndexTemplate = template; @@ -828,7 +829,7 @@ static ClusterState innerPutTemplate(final ClusterState currentState, PutRequest .collect(Collectors.joining(",")), request.name); logger.warn(warning); - HeaderWarning.addWarning(warning); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning); } else { // Otherwise, this is a hard error, the user should use V2 index templates instead String error = String.format(Locale.ROOT, "legacy template [%s] has index patterns %s matching patterns" + diff --git a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java index d391f70f14a50..ab2b263d176d9 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java +++ b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java @@ -192,14 +192,11 @@ private static boolean assertWarningValue(final String s, final String warningVa * Format a warning string in the proper warning format by prepending a warn code, warn agent, wrapping the warning string in quotes, * and appending the RFC 7231 date. * + * @param level the level of the warning - Level.WARN or DeprecationLogger.CRITICAL * @param s the warning string to format * @return a warning value formatted according to RFC 7234 */ - public static String formatWarning(final String s) { - return formatWarning(DeprecationLogger.CRITICAL, s); - } - - private static String formatWarning(final Level level, final String s) { + public static String formatWarning(final Level level, final String s) { // Assume that the common scenario won't have a string to escape and encode. int length = WARNING_PREFIX.length() + s.length() + 6; final StringBuilder sb = new StringBuilder(length); @@ -317,10 +314,6 @@ public static String getXOpaqueId() { .orElse(""); } - public static void addWarning(String message, Object... params) { - addWarning(THREAD_CONTEXT, DeprecationLogger.CRITICAL, message, params); - } - public static void addWarning(Level level, String message, Object... params) { addWarning(THREAD_CONTEXT, level, message, params); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index a6e74265128ed..a4985b4a42114 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -174,6 +174,9 @@ private Setting(Key key, @Nullable Setting fallbackSetting, Functiontrue if this setting is deprecated, otherwise false */ - public boolean isDeprecated() { + private boolean isDeprecated() { return properties.contains(Property.Deprecated) || properties.contains(Property.DeprecatedWarning); } private boolean isDeprecatedWarningOnly() { - return properties.contains(Property.DeprecatedWarning) && properties.contains(Property.Deprecated) == false; + return properties.contains(Property.DeprecatedWarning); } /** diff --git a/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java b/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java index c96660f4ef729..73b974e6bc746 100644 --- a/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java +++ b/server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java @@ -192,16 +192,16 @@ public void testFailsWhenRemovingUnknownThreadContext() throws IOException { public void testWarningValueFromWarningHeader() { final String s = randomAlphaOfLength(16); - final String first = HeaderWarning.formatWarning(s); + final String first = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, s); assertThat(HeaderWarning.extractWarningValueFromWarningHeader(first, false), equalTo(s)); final String withPos = "[context][1:11] Blah blah blah"; - final String formatted = HeaderWarning.formatWarning(withPos); + final String formatted = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, withPos); assertThat(HeaderWarning.extractWarningValueFromWarningHeader(formatted, true), equalTo("Blah blah blah")); final String withNegativePos = "[context][-1:-1] Blah blah blah"; - assertThat(HeaderWarning.extractWarningValueFromWarningHeader(HeaderWarning.formatWarning(withNegativePos), true), - equalTo("Blah blah blah")); + assertThat(HeaderWarning.extractWarningValueFromWarningHeader(HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, + withNegativePos), true), equalTo("Blah blah blah")); } public void testEscapeBackslashesAndQuotes() { diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index ad057aef07155..cadeaddca119a 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -1279,4 +1279,13 @@ public void testCheckForDeprecation() { deprecatedSettingWarningOnly.checkDeprecation(settings); assertSettingDeprecationsAndWarningsIncludingLevel(new Setting[]{ deprecatedSettingWarningOnly }); } + + public void testDeprecationPropertyValidation() { + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + Setting.boolSetting( + "a.bool.setting", + true, + Property.Deprecated, + Property.DeprecatedWarning)); + } } diff --git a/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java b/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java index b0904729bf060..35bc681788f6e 100644 --- a/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.common.util.concurrent; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; @@ -266,11 +267,11 @@ public void testResponseHeaders() { threadContext.addResponseHeader("foo", "bar"); } - final String value = HeaderWarning.formatWarning("qux"); + final String value = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "qux"); threadContext.addResponseHeader("baz", value, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false)); // pretend that another thread created the same response at a different time if (randomBoolean()) { - final String duplicateValue = HeaderWarning.formatWarning("qux"); + final String duplicateValue = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "qux"); threadContext.addResponseHeader("baz", duplicateValue, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false)); } diff --git a/test/external-modules/error-query/src/main/java/org/elasticsearch/test/errorquery/ErrorQueryBuilder.java b/test/external-modules/error-query/src/main/java/org/elasticsearch/test/errorquery/ErrorQueryBuilder.java index 15a6f50244cb0..5d6f69aca03b5 100644 --- a/test/external-modules/error-query/src/main/java/org/elasticsearch/test/errorquery/ErrorQueryBuilder.java +++ b/test/external-modules/error-query/src/main/java/org/elasticsearch/test/errorquery/ErrorQueryBuilder.java @@ -12,6 +12,7 @@ import org.apache.lucene.search.Query; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.SearchExecutionContext; @@ -81,7 +82,7 @@ protected Query doToQuery(SearchExecutionContext context) throws IOException { } final String header = "[" + context.index().getName() + "][" + context.getShardId() + "]"; if (error.getErrorType() == IndexError.ERROR_TYPE.WARNING) { - HeaderWarning.addWarning(header + " " + error.getMessage()); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, header + " " + error.getMessage()); return new MatchAllDocsQuery(); } else { throw new RuntimeException(header + " " + error.getMessage()); diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 9104bf14d41f7..4c28697fae47e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -481,11 +481,7 @@ protected final void assertSettingDeprecationsAndWarningsIncludingLevel(final Se } protected final void assertWarnings(String... expectedWarnings) { - assertWarnings(true, expectedWarnings); - } - - protected final void assertWarnings(boolean stripXContentPosition, String... expectedWarnings) { - assertWarnings(stripXContentPosition, false, expectedWarnings); + assertWarnings(true, false, expectedWarnings); } protected final void assertWarnings(boolean stripXContentPosition, boolean includeLevelCheck, String... expectedWarnings) { diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index c5a83db87dabc..3274608c01233 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.xcontent.XContentLocation; import org.elasticsearch.xcontent.XContentParser; @@ -51,10 +52,10 @@ public void testWarningHeaders() { section.checkWarningHeaders(emptyList()); } - final String testHeader = HeaderWarning.formatWarning("test"); - final String anotherHeader = HeaderWarning.formatWarning("another \"with quotes and \\ backslashes\""); - final String someMoreHeader = HeaderWarning.formatWarning("some more"); - final String catHeader = HeaderWarning.formatWarning("cat"); + final String testHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "test"); + final String anotherHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "another \"with quotes and \\ backslashes\""); + final String someMoreHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "some more"); + final String catHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "cat"); // Any warning headers fail { final DoSection section = new DoSection(new XContentLocation(1, 1)); @@ -129,11 +130,12 @@ public void testWarningHeaders() { public void testWarningHeadersRegex() { - final String testHeader = HeaderWarning.formatWarning("test"); - final String realisticTestHeader = HeaderWarning.formatWarning("index template [my-it] has index patterns [test-*] matching " + - "patterns from existing older templates [global] with patterns (global => [*]); this template [my-it] will take " + - "precedence during new index creation"); - final String testHeaderWithQuotesAndBackslashes = HeaderWarning.formatWarning("test \"with quotes and \\ backslashes\""); + final String testHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "test"); + final String realisticTestHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "index template [my-it] has index " + + "patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template " + + "[my-it] will take precedence during new index creation"); + final String testHeaderWithQuotesAndBackslashes = + HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "test \"with quotes and \\ backslashes\""); //require header and it matches (basic example) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java index 6bb843110743e..358f530c61779 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java @@ -7,6 +7,7 @@ package org.elasticsearch.license; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.settings.Settings; @@ -492,7 +493,7 @@ boolean isAllowed(LicensedFeature feature) { void checkExpiry() { String warning = status.expiryWarning; if (warning != null) { - HeaderWarning.addWarning(warning); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAliasAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAliasAction.java index a9871c6ed50bd..ceb50f59e073f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAliasAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAliasAction.java @@ -21,6 +21,7 @@ import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.license.LicenseUtils; @@ -171,7 +172,7 @@ protected void masterOperation( oldModelId); auditor.warning(oldModelId, warning); logger.warn("[{}] {}", oldModelId, warning); - HeaderWarning.addWarning(warning); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning); } } clusterService.submitStateUpdateTask("update-model-alias", new AckedClusterStateUpdateTask(request, listener) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDataFrameAnalyticsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDataFrameAnalyticsAction.java index 387f2774c0d85..aaa026b88a9a8 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDataFrameAnalyticsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartDataFrameAnalyticsAction.java @@ -27,6 +27,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; @@ -221,7 +222,7 @@ private void estimateMemoryUsageAndUpdateMemoryTracker(StartContext startContext expectedMemoryWithoutDisk); auditor.warning(jobId, warning); logger.warn("[{}] {}", jobId, warning); - HeaderWarning.addWarning(warning); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning); } // Refresh memory requirement for jobs memoryTracker.addDataFrameAnalyticsJobMemoryAndRefreshAllOthers( diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPreviewTransformAction.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPreviewTransformAction.java index bb43eb1226058..d5dc6012a479a 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPreviewTransformAction.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPreviewTransformAction.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xcontent.ToXContent; @@ -242,7 +243,7 @@ private void getPreview( ); List warnings = TransformConfigLinter.getWarnings(function, source, syncConfig); - warnings.forEach(HeaderWarning::addWarning); + warnings.forEach(warning -> HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning)); listener.onResponse(new Response(docs, generatedDestIndexSettings)); }, listener::onFailure); @@ -255,7 +256,7 @@ private void getPreview( Clock.systemUTC() ); List warnings = TransformConfigLinter.getWarnings(function, source, syncConfig); - warnings.forEach(HeaderWarning::addWarning); + warnings.forEach(warning -> HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning)); listener.onResponse(new Response(docs, generatedDestIndexSettings)); } else { List> results = docs.stream().map(doc -> { From 4ac690db52743a83c798414a3f328ad10af3bb4c Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 14 Oct 2021 18:22:22 -0500 Subject: [PATCH 05/12] code review feedback incorporated --- .../xpack/transform/transforms/TransformNodes.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformNodes.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformNodes.java index bf86cec56631b..ff44565818fbc 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformNodes.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformNodes.java @@ -16,6 +16,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; @@ -160,7 +161,7 @@ public static boolean hasAnyTransformNode(DiscoveryNodes nodes) { public static void warnIfNoTransformNodes(ClusterState clusterState) { if (TransformMetadata.getTransformMetadata(clusterState).isResetMode() == false) { if (hasAnyTransformNode(clusterState.getNodes()) == false) { - HeaderWarning.addWarning(TransformMessages.REST_WARN_NO_TRANSFORM_NODES); + HeaderWarning.addWarning(DeprecationLogger.CRITICAL, TransformMessages.REST_WARN_NO_TRANSFORM_NODES); } } } From d8c8dde1f7468f1b2bb57323f046a63175584a0e Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 15 Oct 2021 09:41:01 -0500 Subject: [PATCH 06/12] incorporating code review feedback --- .../discovery/ec2/AwsEc2ServiceImplTests.java | 9 ++-- ...GoogleCloudStorageClientSettingsTests.java | 2 +- .../gcs/GoogleCloudStorageServiceTests.java | 2 +- .../common/logging/EvilLoggerTests.java | 2 +- .../settings/SettingsUpdaterTests.java | 6 +-- .../decider/DiskThresholdDeciderTests.java | 4 +- .../common/settings/SettingTests.java | 8 +-- .../index/IndexSettingsTests.java | 2 +- .../org/elasticsearch/test/ESTestCase.java | 54 +++++++++---------- .../core/ssl/SslSettingsLoaderTests.java | 8 +-- .../test/SettingsFilterTests.java | 2 +- .../LdapUserSearchSessionFactoryTests.java | 2 +- .../ldap/support/SessionFactoryTests.java | 2 +- .../security/authc/pki/PkiRealmTests.java | 4 +- ...OpenLdapUserSearchSessionFactoryTests.java | 2 +- 15 files changed, 54 insertions(+), 55 deletions(-) diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java index 7a73db5d37081..48f1c71c09dfb 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java @@ -15,6 +15,7 @@ import com.amazonaws.auth.BasicSessionCredentials; import com.amazonaws.auth.DefaultAWSCredentialsProviderChain; import org.elasticsearch.common.settings.MockSecureSettings; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.test.ESTestCase; @@ -59,8 +60,8 @@ public void testDeprecationOfLoneAccessKey() { Ec2ClientSettings.getClientSettings(Settings.builder().setSecureSettings(secureSettings).build())).getCredentials(); assertThat(credentials.getAWSAccessKeyId(), is("aws_key")); assertThat(credentials.getAWSSecretKey(), is("")); - assertSettingDeprecationsAndWarnings(new String[]{}, - "Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is not, which will be unsupported in future"); + assertSettingDeprecationsAndWarnings(new Setting[]{}, + false, "Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is not, which will be unsupported in future"); } public void testDeprecationOfLoneSecretKey() { @@ -70,8 +71,8 @@ public void testDeprecationOfLoneSecretKey() { Ec2ClientSettings.getClientSettings(Settings.builder().setSecureSettings(secureSettings).build())).getCredentials(); assertThat(credentials.getAWSAccessKeyId(), is("")); assertThat(credentials.getAWSSecretKey(), is("aws_secret")); - assertSettingDeprecationsAndWarnings(new String[]{}, - "Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is not, which will be unsupported in future"); + assertSettingDeprecationsAndWarnings(new Setting[]{}, + false, "Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is not, which will be unsupported in future"); } public void testRejectionOfLoneSessionToken() { diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java index db167a386adf5..0f664248890d7 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java @@ -70,7 +70,7 @@ public void testLoad() throws Exception { } if (deprecationWarnings.isEmpty() == false) { - assertSettingDeprecationsAndWarnings(deprecationWarnings.toArray(new Setting[0])); + assertSettingDeprecationsAndWarnings(deprecationWarnings.toArray(new Setting[0]), false); } } diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java index 7acba307d54e9..16db7de253af0 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java @@ -59,7 +59,7 @@ public void testClientInitializer() throws Exception { expectThrows(IllegalArgumentException.class, () -> service.client("another_client", "repo", statsCollector)); assertThat(e.getMessage(), Matchers.startsWith("Unknown client name")); assertSettingDeprecationsAndWarnings( - new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) }); + new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) }, false); final Storage storage = service.client(clientName, "repo", statsCollector); assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(applicationName)); assertThat(storage.getOptions().getHost(), Matchers.is(endpoint)); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java index 485db9612aebb..a4e497259bed9 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java @@ -190,7 +190,7 @@ public void testDeprecatedSettings() throws IOException, UserException { final int iterations = randomIntBetween(0, 128); for (int i = 0; i < iterations; i++) { setting.get(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{setting}); + assertSettingDeprecationsAndWarnings(new Setting[]{setting}, false); } final String deprecationPath = diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java index 87d3558618f55..142a3aeecef93 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java @@ -193,15 +193,15 @@ public void testDeprecationLogging() { final Settings toApplyDebug = Settings.builder().put("logger.org.elasticsearch", "debug").build(); final ClusterState afterDebug = settingsUpdater.updateSettings(clusterState, toApplyDebug, Settings.EMPTY, logger); - assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }); + assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }, false); final Settings toApplyUnset = Settings.builder().putNull("logger.org.elasticsearch").build(); final ClusterState afterUnset = settingsUpdater.updateSettings(afterDebug, toApplyUnset, Settings.EMPTY, logger); - assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }); + assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }, false); // we also check that if no settings are changed, deprecation logging still occurs settingsUpdater.updateSettings(afterUnset, toApplyUnset, Settings.EMPTY, logger); - assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }); + assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }, false); } public void testUpdateWithUnknownAndSettings() { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java index e1fd2f1652811..67ab610bb88b5 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java @@ -1009,7 +1009,7 @@ Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLU " actual free: [20.0%]")); if (DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE.exists(diskSettings)) { - assertSettingDeprecationsAndWarnings(new Setting[] { DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }); + assertSettingDeprecationsAndWarnings(new Setting[] { DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }, false); } } @@ -1025,7 +1025,7 @@ public void testSingleDataNodeDeprecationWarning() { equalTo("setting [cluster.routing.allocation.disk.watermark.enable_for_single_data_node=false] is not allowed," + " only true is valid")); - assertSettingDeprecationsAndWarnings(new Setting[]{ DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }); + assertSettingDeprecationsAndWarnings(new Setting[]{ DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }, false); } public void testDiskThresholdWithSnapshotShardSizes() { diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index cadeaddca119a..f397407028eaf 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -152,7 +152,7 @@ public void testMemorySize() { assertEquals(new ByteSizeValue(12), value.get()); assertTrue(settingUpdater.apply(Settings.builder().put("a.byte.size", "20%").build(), Settings.EMPTY)); - assertEquals(new ByteSizeValue((int) (JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() * 0.2)), value.get()); + assertEquals(new ByteSizeValue((long) (JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() * 0.2)), value.get()); } public void testSimpleUpdate() { @@ -627,7 +627,7 @@ public void testListSettingsDeprecated() { .build(); deprecatedListSetting.get(settings); nonDeprecatedListSetting.get(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedListSetting }); + assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedListSetting }, false); } public void testListSettings() { @@ -1273,11 +1273,11 @@ public void testCheckForDeprecation() { ensureNoWarnings(); final Setting criticalDeprecatedSetting = Setting.simpleString(criticalSettingName, settingValue, Property.Deprecated); criticalDeprecatedSetting.checkDeprecation(settings); - assertSettingDeprecationsAndWarningsIncludingLevel(new Setting[]{ criticalDeprecatedSetting }); + assertSettingDeprecationsAndWarnings(new Setting[]{ criticalDeprecatedSetting }, true); final Setting deprecatedSettingWarningOnly = Setting.simpleString(warningSettingName, settingValue, Property.DeprecatedWarning); deprecatedSettingWarningOnly.checkDeprecation(settings); - assertSettingDeprecationsAndWarningsIncludingLevel(new Setting[]{ deprecatedSettingWarningOnly }); + assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedSettingWarningOnly }, true); } public void testDeprecationPropertyValidation() { diff --git a/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java b/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java index 6d596e15ed9fb..0af9cb9a5a62b 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java @@ -543,6 +543,6 @@ public void testCustomDataPathDeprecated() { IndexMetadata metadata = newIndexMeta("test", settings); IndexSettings indexSettings = new IndexSettings(metadata, Settings.EMPTY); assertThat(indexSettings.hasCustomDataPath(), is(true)); - assertSettingDeprecationsAndWarnings(new Setting[] { IndexMetadata.INDEX_DATA_PATH_SETTING }); + assertSettingDeprecationsAndWarnings(new Setting[] { IndexMetadata.INDEX_DATA_PATH_SETTING }, false); } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 351b16fbce263..48b62627b82b2 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -441,41 +441,39 @@ protected List filteredWarnings() { /** * Convenience method to assert warnings for settings deprecations and general deprecation warnings. - * - * @param settings the settings that are expected to be deprecated + * @param settings the settings that are expected to be deprecated + * @param shouldAssertExpectedLogLevel * @param warnings other expected general deprecation warnings */ - protected final void assertSettingDeprecationsAndWarnings(final Setting[] settings, final String... warnings) { - assertSettingDeprecationsAndWarnings(Arrays.stream(settings).map(Setting::getKey).toArray(String[]::new), warnings); - } - - protected final void assertSettingDeprecationsAndWarnings(final String[] settings, final String... warnings) { + protected final void assertSettingDeprecationsAndWarnings(final Setting[] settings, boolean shouldAssertExpectedLogLevel, final String... warnings) { assertWarnings( - Stream.concat( - Arrays - .stream(settings) - .map(k -> "[" + k + "] setting was deprecated in Elasticsearch and will be removed in a future release! " + - "See the breaking changes documentation for the next major version."), - Arrays.stream(warnings)) - .toArray(String[]::new)); - } - - protected final void assertSettingDeprecationsAndWarningsIncludingLevel(final Setting[] settings, final String... warnings) { - assertWarnings( - true, true, + shouldAssertExpectedLogLevel, Stream.concat( Arrays .stream(settings) - .map(setting -> String.format( - Locale.ROOT, "%s Elasticsearch-%s%s-%s \"[%s] setting was deprecated in Elasticsearch and will be " + - "removed in a future release! See the breaking changes documentation for the next major version.\"", - setting.getProperties().contains(Setting.Property.Deprecated) ? DeprecationLogger.CRITICAL.intLevel() : - Level.WARN.intLevel(), - Version.CURRENT.toString(), - Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "", - Build.CURRENT.hash(), - setting.getKey())), + .map(setting -> { + String prefix; + String suffix; + if (shouldAssertExpectedLogLevel) { + prefix = String.format(Locale.ROOT, "%s Elasticsearch-%s%s-%s \"", + setting.getProperties().contains(Setting.Property.Deprecated) ? DeprecationLogger.CRITICAL.intLevel() : + Level.WARN.intLevel(), + Version.CURRENT.toString(), + Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "", + Build.CURRENT.hash()); + suffix = "\""; + } + else { + prefix = ""; + suffix = ""; + } + String warningMessage = String.format( + Locale.ROOT, "[%s] setting was deprecated in Elasticsearch and will be " + + "removed in a future release! See the breaking changes documentation for the next major version.", + setting.getKey()); + return prefix + warningMessage + suffix; + }), Arrays.stream(warnings)) .toArray(String[]::new)); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java index a0349c85b23ad..618d889c118b7 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java @@ -166,7 +166,7 @@ public void testKeystorePasswordBackcompat() { ) ); assertSettingDeprecationsAndWarnings(new Setting[]{ - configurationSettings.x509KeyPair.legacyKeystorePassword}); + configurationSettings.x509KeyPair.legacyKeystorePassword}, false); } public void testKeystoreKeyPassword() { @@ -208,7 +208,7 @@ public void testKeystoreKeyPasswordBackcompat() { assertSettingDeprecationsAndWarnings(new Setting[]{ configurationSettings.x509KeyPair.legacyKeystorePassword, configurationSettings.x509KeyPair.legacyKeystoreKeyPassword - }); + }, false); } public void testInferKeystoreTypeFromJksFile() { @@ -382,7 +382,7 @@ public void testPEMFileBackcompat() { KeyManager keyManager = keyConfig.createKeyManager(); assertNotNull(keyManager); assertCombiningTrustConfigContainsCorrectIssuers(config); - assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}); + assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}, false); } public void testPEMKeyAndTrustFiles() { @@ -427,7 +427,7 @@ public void testPEMKeyAndTrustFilesBackcompat() { assertThat(config.getTrustConfig(), instanceOf(PemTrustConfig.class)); TrustManager trustManager = keyConfig.asTrustConfig().createTrustManager(); assertNotNull(trustManager); - assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}); + assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}, false); } public void testExplicitlyConfigured() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java index 91fef098424d3..add923c8e5d0a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java @@ -136,7 +136,7 @@ public void testFiltering() throws Exception { if (useLegacyLdapBindPassword) { assertSettingDeprecationsAndWarnings(new Setting[]{PoolingSessionFactorySettings.LEGACY_BIND_PASSWORD .apply(LdapRealmSettings.LDAP_TYPE) - .getConcreteSettingForNamespace("ldap1")}); + .getConcreteSettingForNamespace("ldap1")}, false); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java index b1ef66821436e..ed3757006a0d3 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java @@ -595,7 +595,7 @@ private void assertDeprecationWarnings(RealmConfig.RealmIdentifier realmIdentifi .getConcreteSettingForNamespace(realmIdentifier.getName())); } if (deprecatedSettings.size() > 0) { - assertSettingDeprecationsAndWarnings(deprecatedSettings.toArray(new Setting[deprecatedSettings.size()])); + assertSettingDeprecationsAndWarnings(deprecatedSettings.toArray(new Setting[deprecatedSettings.size()]), false); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java index 7b851c9a76840..c07cbc3148b73 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java @@ -96,7 +96,7 @@ public void testSessionFactoryWithResponseTimeout() throws Exception { LDAPConnectionOptions options = SessionFactory.connectionOptions(realmConfig, new SSLService(settings, environment), logger); assertThat(options.getResponseTimeoutMillis(), is(equalTo(7000L))); assertSettingDeprecationsAndWarnings(new Setting[]{SessionFactorySettings.TIMEOUT_TCP_READ_SETTING.apply("ldap") - .getConcreteSettingForNamespace("response_settings")}); + .getConcreteSettingForNamespace("response_settings")}, false); } { Settings settings = Settings.builder() diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java index 26537e0beaceb..ee0edcbc16449 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java @@ -442,7 +442,7 @@ public void testTruststorePathWithLegacyPasswordDoesNotThrow() throws Exception TestEnvironment.newEnvironment(settings), new ThreadContext(settings)), mock(UserRoleMapper.class)); assertSettingDeprecationsAndWarnings(new Setting[]{ PkiRealmSettings.LEGACY_TRUST_STORE_PASSWORD.getConcreteSettingForNamespace(REALM_NAME) - }); + }, false); } public void testCertificateWithOnlyCnExtractsProperly() throws Exception { @@ -500,7 +500,7 @@ public void testPKIRealmSettingsPassValidation() throws Exception { assertSettingDeprecationsAndWarnings(new Setting[]{ PkiRealmSettings.LEGACY_TRUST_STORE_PASSWORD.getConcreteSettingForNamespace("pki1") - }); + }, false); } public void testDelegatedAuthorization() throws Exception { diff --git a/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java b/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java index c6bf52be0cf79..bef600d0a6bf9 100644 --- a/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java +++ b/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java @@ -121,7 +121,7 @@ public void testUserSearchWithBindUserOpenLDAP() throws Exception { if (useSecureBindPassword == false) { assertSettingDeprecationsAndWarnings(new Setting[]{ config.getConcreteSetting(PoolingSessionFactorySettings.LEGACY_BIND_PASSWORD) - }); + }, false); } } From bebb895e7d09c0c9287082ec2a58e60f1070f130 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 15 Oct 2021 10:17:58 -0500 Subject: [PATCH 07/12] getting rid of compiler warning --- .../elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java index 48f1c71c09dfb..6a70092d01edc 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java @@ -60,7 +60,7 @@ public void testDeprecationOfLoneAccessKey() { Ec2ClientSettings.getClientSettings(Settings.builder().setSecureSettings(secureSettings).build())).getCredentials(); assertThat(credentials.getAWSAccessKeyId(), is("aws_key")); assertThat(credentials.getAWSSecretKey(), is("")); - assertSettingDeprecationsAndWarnings(new Setting[]{}, + assertSettingDeprecationsAndWarnings(new Setting[]{}, false, "Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is not, which will be unsupported in future"); } @@ -71,7 +71,7 @@ public void testDeprecationOfLoneSecretKey() { Ec2ClientSettings.getClientSettings(Settings.builder().setSecureSettings(secureSettings).build())).getCredentials(); assertThat(credentials.getAWSAccessKeyId(), is("")); assertThat(credentials.getAWSSecretKey(), is("aws_secret")); - assertSettingDeprecationsAndWarnings(new Setting[]{}, + assertSettingDeprecationsAndWarnings(new Setting[]{}, false, "Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is not, which will be unsupported in future"); } From 841fbb268ba13fd5312c78cb6a47850d6013772f Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 15 Oct 2021 10:38:31 -0500 Subject: [PATCH 08/12] fixing a checkstyle problem --- .../repositories/gcs/GoogleCloudStorageServiceTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java index 16db7de253af0..5ab5ef4884cff 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java @@ -59,7 +59,8 @@ public void testClientInitializer() throws Exception { expectThrows(IllegalArgumentException.class, () -> service.client("another_client", "repo", statsCollector)); assertThat(e.getMessage(), Matchers.startsWith("Unknown client name")); assertSettingDeprecationsAndWarnings( - new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) }, false); + new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) }, + false); final Storage storage = service.client(clientName, "repo", statsCollector); assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(applicationName)); assertThat(storage.getOptions().getHost(), Matchers.is(endpoint)); From d2a82a7d04ef9959b8901cedffeb66c9375a52d7 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 15 Oct 2021 11:04:52 -0500 Subject: [PATCH 09/12] fixing a checkstyle problem --- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 48b62627b82b2..d5ecfd2741701 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -445,7 +445,8 @@ protected List filteredWarnings() { * @param shouldAssertExpectedLogLevel * @param warnings other expected general deprecation warnings */ - protected final void assertSettingDeprecationsAndWarnings(final Setting[] settings, boolean shouldAssertExpectedLogLevel, final String... warnings) { + protected final void assertSettingDeprecationsAndWarnings(final Setting[] settings, boolean shouldAssertExpectedLogLevel, + final String... warnings) { assertWarnings( true, shouldAssertExpectedLogLevel, From 0c18846126c4ba1ca91a710efd6f1116853f5150 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Mon, 18 Oct 2021 16:12:32 -0500 Subject: [PATCH 10/12] Checking log level for all deprecation warnings --- .../discovery/ec2/AwsEc2ServiceImplTests.java | 8 +- ...GoogleCloudStorageClientSettingsTests.java | 2 +- .../gcs/GoogleCloudStorageServiceTests.java | 4 +- .../common/logging/EvilLoggerTests.java | 2 +- .../settings/SettingsUpdaterTests.java | 6 +- .../decider/DiskThresholdDeciderTests.java | 4 +- .../common/settings/SettingTests.java | 8 +- .../index/IndexSettingsTests.java | 2 +- .../org/elasticsearch/test/ESTestCase.java | 94 +++++++++++-------- .../core/ssl/SslSettingsLoaderTests.java | 8 +- .../test/SettingsFilterTests.java | 2 +- .../LdapUserSearchSessionFactoryTests.java | 2 +- .../ldap/support/SessionFactoryTests.java | 2 +- .../security/authc/pki/PkiRealmTests.java | 4 +- ...OpenLdapUserSearchSessionFactoryTests.java | 2 +- 15 files changed, 87 insertions(+), 63 deletions(-) diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java index 6a70092d01edc..7351e0ef86797 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java @@ -14,6 +14,8 @@ import com.amazonaws.auth.AWSCredentialsProvider; import com.amazonaws.auth.BasicSessionCredentials; import com.amazonaws.auth.DefaultAWSCredentialsProviderChain; + +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; @@ -61,7 +63,8 @@ public void testDeprecationOfLoneAccessKey() { assertThat(credentials.getAWSAccessKeyId(), is("aws_key")); assertThat(credentials.getAWSSecretKey(), is("")); assertSettingDeprecationsAndWarnings(new Setting[]{}, - false, "Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is not, which will be unsupported in future"); + new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is " + + "not, which will be unsupported in future")); } public void testDeprecationOfLoneSecretKey() { @@ -72,7 +75,8 @@ public void testDeprecationOfLoneSecretKey() { assertThat(credentials.getAWSAccessKeyId(), is("")); assertThat(credentials.getAWSSecretKey(), is("aws_secret")); assertSettingDeprecationsAndWarnings(new Setting[]{}, - false, "Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is not, which will be unsupported in future"); + new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is " + + "not, which will be unsupported in future")); } public void testRejectionOfLoneSessionToken() { diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java index 0f664248890d7..db167a386adf5 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java @@ -70,7 +70,7 @@ public void testLoad() throws Exception { } if (deprecationWarnings.isEmpty() == false) { - assertSettingDeprecationsAndWarnings(deprecationWarnings.toArray(new Setting[0]), false); + assertSettingDeprecationsAndWarnings(deprecationWarnings.toArray(new Setting[0])); } } diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java index 5ab5ef4884cff..e1dbc2d10dede 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java @@ -59,8 +59,8 @@ public void testClientInitializer() throws Exception { expectThrows(IllegalArgumentException.class, () -> service.client("another_client", "repo", statsCollector)); assertThat(e.getMessage(), Matchers.startsWith("Unknown client name")); assertSettingDeprecationsAndWarnings( - new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) }, - false); + new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) } + ); final Storage storage = service.client(clientName, "repo", statsCollector); assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(applicationName)); assertThat(storage.getOptions().getHost(), Matchers.is(endpoint)); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java index a4e497259bed9..485db9612aebb 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java @@ -190,7 +190,7 @@ public void testDeprecatedSettings() throws IOException, UserException { final int iterations = randomIntBetween(0, 128); for (int i = 0; i < iterations; i++) { setting.get(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{setting}, false); + assertSettingDeprecationsAndWarnings(new Setting[]{setting}); } final String deprecationPath = diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java index 142a3aeecef93..87d3558618f55 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java @@ -193,15 +193,15 @@ public void testDeprecationLogging() { final Settings toApplyDebug = Settings.builder().put("logger.org.elasticsearch", "debug").build(); final ClusterState afterDebug = settingsUpdater.updateSettings(clusterState, toApplyDebug, Settings.EMPTY, logger); - assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }, false); + assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }); final Settings toApplyUnset = Settings.builder().putNull("logger.org.elasticsearch").build(); final ClusterState afterUnset = settingsUpdater.updateSettings(afterDebug, toApplyUnset, Settings.EMPTY, logger); - assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }, false); + assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }); // we also check that if no settings are changed, deprecation logging still occurs settingsUpdater.updateSettings(afterUnset, toApplyUnset, Settings.EMPTY, logger); - assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }, false); + assertSettingDeprecationsAndWarnings(new Setting[] { deprecatedSetting }); } public void testUpdateWithUnknownAndSettings() { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java index 67ab610bb88b5..e1fd2f1652811 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/DiskThresholdDeciderTests.java @@ -1009,7 +1009,7 @@ Settings.EMPTY, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLU " actual free: [20.0%]")); if (DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE.exists(diskSettings)) { - assertSettingDeprecationsAndWarnings(new Setting[] { DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }, false); + assertSettingDeprecationsAndWarnings(new Setting[] { DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }); } } @@ -1025,7 +1025,7 @@ public void testSingleDataNodeDeprecationWarning() { equalTo("setting [cluster.routing.allocation.disk.watermark.enable_for_single_data_node=false] is not allowed," + " only true is valid")); - assertSettingDeprecationsAndWarnings(new Setting[]{ DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }, false); + assertSettingDeprecationsAndWarnings(new Setting[]{ DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE }); } public void testDiskThresholdWithSnapshotShardSizes() { diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 44684c5a59a2e..8f6d4ebd2f818 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -627,7 +627,7 @@ public void testListSettingsDeprecated() { .build(); deprecatedListSetting.get(settings); nonDeprecatedListSetting.get(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedListSetting }, false); + assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedListSetting }); } public void testListSettings() { @@ -1273,11 +1273,11 @@ public void testCheckForDeprecation() { ensureNoWarnings(); final Setting criticalDeprecatedSetting = Setting.simpleString(criticalSettingName, settingValue, Property.Deprecated); criticalDeprecatedSetting.checkDeprecation(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{ criticalDeprecatedSetting }, true); + assertSettingDeprecationsAndWarnings(new Setting[]{ criticalDeprecatedSetting }); final Setting deprecatedSettingWarningOnly = Setting.simpleString(warningSettingName, settingValue, Property.DeprecatedWarning); deprecatedSettingWarningOnly.checkDeprecation(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedSettingWarningOnly }, true); + assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedSettingWarningOnly }); } public void testCheckForDeprecationWithSkipSetting() { @@ -1289,7 +1289,7 @@ public void testCheckForDeprecationWithSkipSetting() { ensureNoWarnings(); final Setting deprecatedSetting = Setting.simpleString(settingName, settingValue, Property.Deprecated); deprecatedSetting.checkDeprecation(settings); - assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedSetting }, false); + assertSettingDeprecationsAndWarnings(new Setting[]{ deprecatedSetting }); final Settings settingsWithSkipDeprecationSetting = Settings.builder().put(settingName, settingValue) .putList("deprecation.skip_deprecated_settings", settingName) .build(); diff --git a/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java b/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java index 0af9cb9a5a62b..6d596e15ed9fb 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java @@ -543,6 +543,6 @@ public void testCustomDataPathDeprecated() { IndexMetadata metadata = newIndexMeta("test", settings); IndexSettings indexSettings = new IndexSettings(metadata, Settings.EMPTY); assertThat(indexSettings.hasCustomDataPath(), is(true)); - assertSettingDeprecationsAndWarnings(new Setting[] { IndexMetadata.INDEX_DATA_PATH_SETTING }, false); + assertSettingDeprecationsAndWarnings(new Setting[] { IndexMetadata.INDEX_DATA_PATH_SETTING }); } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index d5ecfd2741701..e4911067d731d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -35,7 +35,6 @@ import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; import org.apache.lucene.util.TestRuleMarkFailure; import org.apache.lucene.util.TimeUnits; -import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.bootstrap.BootstrapForTesting; import org.elasticsearch.client.Requests; @@ -441,69 +440,70 @@ protected List filteredWarnings() { /** * Convenience method to assert warnings for settings deprecations and general deprecation warnings. - * @param settings the settings that are expected to be deprecated - * @param shouldAssertExpectedLogLevel + * @param settings the settings that are expected to be deprecated * @param warnings other expected general deprecation warnings */ - protected final void assertSettingDeprecationsAndWarnings(final Setting[] settings, boolean shouldAssertExpectedLogLevel, - final String... warnings) { + protected final void assertSettingDeprecationsAndWarnings(final Setting[] settings, final DeprecationWarning... warnings) { assertWarnings( true, - shouldAssertExpectedLogLevel, Stream.concat( Arrays .stream(settings) .map(setting -> { - String prefix; - String suffix; - if (shouldAssertExpectedLogLevel) { - prefix = String.format(Locale.ROOT, "%s Elasticsearch-%s%s-%s \"", - setting.getProperties().contains(Setting.Property.Deprecated) ? DeprecationLogger.CRITICAL.intLevel() : - Level.WARN.intLevel(), - Version.CURRENT.toString(), - Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "", - Build.CURRENT.hash()); - suffix = "\""; - } - else { - prefix = ""; - suffix = ""; - } String warningMessage = String.format( Locale.ROOT, "[%s] setting was deprecated in Elasticsearch and will be " + "removed in a future release! See the breaking changes documentation for the next major version.", setting.getKey()); - return prefix + warningMessage + suffix; + return new DeprecationWarning(setting.getProperties().contains(Setting.Property.Deprecated) ? + DeprecationLogger.CRITICAL : Level.WARN, warningMessage); }), Arrays.stream(warnings)) - .toArray(String[]::new)); + .toArray(DeprecationWarning[]::new)); } + /** + * Convenience method to assert warnings for settings deprecations and general deprecation warnings. All warnings passed to this method + * are assumed to be at DeprecationLogger.CRITICAL level. + * @param expectedWarnings expected general deprecation warnings. + */ protected final void assertWarnings(String... expectedWarnings) { - assertWarnings(true, false, expectedWarnings); + assertWarnings(true, Arrays.stream(expectedWarnings).map(expectedWarning -> new DeprecationWarning(DeprecationLogger.CRITICAL, + expectedWarning)).toArray(DeprecationWarning[]::new)); } - protected final void assertWarnings(boolean stripXContentPosition, boolean includeLevelCheck, String... expectedWarnings) { + protected final void assertWarnings(boolean stripXContentPosition, DeprecationWarning... expectedWarnings) { if (enableWarningsCheck() == false) { throw new IllegalStateException("unable to check warning headers if the test is not set to do so"); } try { - final List actualWarnings = threadContext.getResponseHeaders().get("Warning"); + final List actualWarningStrings = threadContext.getResponseHeaders().get("Warning"); if (expectedWarnings == null || expectedWarnings.length == 0) { - assertNull("expected 0 warnings, actual: " + actualWarnings, actualWarnings); + assertNull("expected 0 warnings, actual: " + actualWarningStrings, actualWarningStrings); } else { - assertNotNull("no warnings, expected: " + Arrays.asList(expectedWarnings), actualWarnings); - final Set actualWarningValues = - actualWarnings.stream() - .map(s -> includeLevelCheck ? HeaderWarning.escapeAndEncode(s) : - HeaderWarning.extractWarningValueFromWarningHeader(s, stripXContentPosition)) + assertNotNull("no warnings, expected: " + Arrays.asList(expectedWarnings), actualWarningStrings); + final Set actualDeprecationWarnings = + actualWarningStrings.stream() + .map(warningString -> { + String warningText = HeaderWarning.extractWarningValueFromWarningHeader(warningString, stripXContentPosition); + final Level level; + if (warningString.startsWith(Integer.toString(DeprecationLogger.CRITICAL.intLevel()))) { + level = DeprecationLogger.CRITICAL; + } else if (warningString.startsWith(Integer.toString(Level.WARN.intLevel()))) { + level = Level.WARN; + } else { + throw new IllegalArgumentException("Unknown level in deprecation message " + warningString); + } + return new DeprecationWarning(level, warningText); + }) .collect(Collectors.toSet()); - for (String msg : expectedWarnings) { - assertThat(actualWarningValues, hasItem(HeaderWarning.escapeAndEncode(msg))); + for (DeprecationWarning expectedWarning : expectedWarnings) { + DeprecationWarning escapedExpectedWarning = new DeprecationWarning(expectedWarning.level, + HeaderWarning.escapeAndEncode(expectedWarning.message)); + assertThat(actualDeprecationWarnings, hasItem(escapedExpectedWarning)); } - assertEquals("Expected " + expectedWarnings.length + " warnings but found " + actualWarnings.size() + "\nExpected: " - + Arrays.asList(expectedWarnings) + "\nActual: " + actualWarnings, - expectedWarnings.length, actualWarnings.size() + assertEquals("Expected " + expectedWarnings.length + " warnings but found " + actualWarningStrings.size() + "\nExpected: " + + Arrays.asList(expectedWarnings) + "\nActual: " + actualWarningStrings, + expectedWarnings.length, actualWarningStrings.size() ); } } finally { @@ -1578,4 +1578,24 @@ protected static InetAddress randomIp(boolean v4) { throw new AssertionError(); } } + + public static final class DeprecationWarning { + private final Level level; + private final String message; + public DeprecationWarning(Level level, String message) { + this.level = level; + this.message = message; + } + + public int hashCode() { + return Objects.hash(level, message); + } + + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DeprecationWarning that = (DeprecationWarning) o; + return Objects.equals(level, that.level) && Objects.equals(message, that.message); + } + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java index 618d889c118b7..a0349c85b23ad 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SslSettingsLoaderTests.java @@ -166,7 +166,7 @@ public void testKeystorePasswordBackcompat() { ) ); assertSettingDeprecationsAndWarnings(new Setting[]{ - configurationSettings.x509KeyPair.legacyKeystorePassword}, false); + configurationSettings.x509KeyPair.legacyKeystorePassword}); } public void testKeystoreKeyPassword() { @@ -208,7 +208,7 @@ public void testKeystoreKeyPasswordBackcompat() { assertSettingDeprecationsAndWarnings(new Setting[]{ configurationSettings.x509KeyPair.legacyKeystorePassword, configurationSettings.x509KeyPair.legacyKeystoreKeyPassword - }, false); + }); } public void testInferKeystoreTypeFromJksFile() { @@ -382,7 +382,7 @@ public void testPEMFileBackcompat() { KeyManager keyManager = keyConfig.createKeyManager(); assertNotNull(keyManager); assertCombiningTrustConfigContainsCorrectIssuers(config); - assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}, false); + assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}); } public void testPEMKeyAndTrustFiles() { @@ -427,7 +427,7 @@ public void testPEMKeyAndTrustFilesBackcompat() { assertThat(config.getTrustConfig(), instanceOf(PemTrustConfig.class)); TrustManager trustManager = keyConfig.asTrustConfig().createTrustManager(); assertNotNull(trustManager); - assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}, false); + assertSettingDeprecationsAndWarnings(new Setting[]{configurationSettings.x509KeyPair.legacyKeyPassword}); } public void testExplicitlyConfigured() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java index add923c8e5d0a..91fef098424d3 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java @@ -136,7 +136,7 @@ public void testFiltering() throws Exception { if (useLegacyLdapBindPassword) { assertSettingDeprecationsAndWarnings(new Setting[]{PoolingSessionFactorySettings.LEGACY_BIND_PASSWORD .apply(LdapRealmSettings.LDAP_TYPE) - .getConcreteSettingForNamespace("ldap1")}, false); + .getConcreteSettingForNamespace("ldap1")}); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java index ed3757006a0d3..b1ef66821436e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java @@ -595,7 +595,7 @@ private void assertDeprecationWarnings(RealmConfig.RealmIdentifier realmIdentifi .getConcreteSettingForNamespace(realmIdentifier.getName())); } if (deprecatedSettings.size() > 0) { - assertSettingDeprecationsAndWarnings(deprecatedSettings.toArray(new Setting[deprecatedSettings.size()]), false); + assertSettingDeprecationsAndWarnings(deprecatedSettings.toArray(new Setting[deprecatedSettings.size()])); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java index c07cbc3148b73..7b851c9a76840 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java @@ -96,7 +96,7 @@ public void testSessionFactoryWithResponseTimeout() throws Exception { LDAPConnectionOptions options = SessionFactory.connectionOptions(realmConfig, new SSLService(settings, environment), logger); assertThat(options.getResponseTimeoutMillis(), is(equalTo(7000L))); assertSettingDeprecationsAndWarnings(new Setting[]{SessionFactorySettings.TIMEOUT_TCP_READ_SETTING.apply("ldap") - .getConcreteSettingForNamespace("response_settings")}, false); + .getConcreteSettingForNamespace("response_settings")}); } { Settings settings = Settings.builder() diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java index ee0edcbc16449..26537e0beaceb 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/pki/PkiRealmTests.java @@ -442,7 +442,7 @@ public void testTruststorePathWithLegacyPasswordDoesNotThrow() throws Exception TestEnvironment.newEnvironment(settings), new ThreadContext(settings)), mock(UserRoleMapper.class)); assertSettingDeprecationsAndWarnings(new Setting[]{ PkiRealmSettings.LEGACY_TRUST_STORE_PASSWORD.getConcreteSettingForNamespace(REALM_NAME) - }, false); + }); } public void testCertificateWithOnlyCnExtractsProperly() throws Exception { @@ -500,7 +500,7 @@ public void testPKIRealmSettingsPassValidation() throws Exception { assertSettingDeprecationsAndWarnings(new Setting[]{ PkiRealmSettings.LEGACY_TRUST_STORE_PASSWORD.getConcreteSettingForNamespace("pki1") - }, false); + }); } public void testDelegatedAuthorization() throws Exception { diff --git a/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java b/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java index bef600d0a6bf9..c6bf52be0cf79 100644 --- a/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java +++ b/x-pack/qa/openldap-tests/src/test/java/org/elasticsearch/xpack/security/authc/ldap/OpenLdapUserSearchSessionFactoryTests.java @@ -121,7 +121,7 @@ public void testUserSearchWithBindUserOpenLDAP() throws Exception { if (useSecureBindPassword == false) { assertSettingDeprecationsAndWarnings(new Setting[]{ config.getConcreteSetting(PoolingSessionFactorySettings.LEGACY_BIND_PASSWORD) - }, false); + }); } } From 9fa7f606874ec31458ae4cb8cca5a767458f9cb4 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Mon, 18 Oct 2021 16:27:57 -0500 Subject: [PATCH 11/12] fixing compilation errors / cleaning up --- .../ConstructingObjectParserTests.java | 4 +-- .../xcontent/ObjectParserTests.java | 8 +++-- .../discovery/ec2/AwsEc2ServiceImplTests.java | 8 ++--- .../gcs/GoogleCloudStorageServiceTests.java | 3 +- .../common/settings/Setting.java | 2 -- .../org/elasticsearch/test/ESTestCase.java | 33 ++++++++++--------- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java index 560523d2f2ec1..0025faa30b0dd 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java @@ -626,8 +626,8 @@ public void testCompatibleFieldDeclarations() throws IOException { RestApiVersion.minimumSupported()); StructWithCompatibleFields o = StructWithCompatibleFields.PARSER.parse(parser, null); assertEquals(1, o.intField); - assertWarnings(false, false, "[struct_with_compatible_fields][1:14] " + - "Deprecated field [old_name] used, expected [new_name] instead"); + assertWarnings(false, new DeprecationWarning(DeprecationLogger.CRITICAL, "[struct_with_compatible_fields][1:14] " + + "Deprecated field [old_name] used, expected [new_name] instead")); } } diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java index 4ea33c733b40f..8882d38358f0c 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.xcontent; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.XContentParserUtils; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.core.RestApiVersion; @@ -211,7 +212,8 @@ class TestStruct { objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); objectParser.parse(parser, s, null); assertEquals("foo", s.test); - assertWarnings(false, false, "[foo][1:15] Deprecated field [old_test] used, expected [test] instead"); + assertWarnings(false, new DeprecationWarning(DeprecationLogger.CRITICAL, "[foo][1:15] Deprecated field [old_test] used, " + + "expected [test] instead")); } public void testFailOnValueType() throws IOException { @@ -1072,8 +1074,8 @@ public void testCompatibleFieldDeclarations() throws IOException { RestApiVersion.minimumSupported()); StructWithCompatibleFields o = StructWithCompatibleFields.PARSER.parse(parser, null); assertEquals(1, o.intField); - assertWarnings(false, false, "[struct_with_compatible_fields][1:14] " + - "Deprecated field [old_name] used, expected [new_name] instead"); + assertWarnings(false, new DeprecationWarning(DeprecationLogger.CRITICAL, "[struct_with_compatible_fields][1:14] " + + "Deprecated field [old_name] used, expected [new_name] instead")); } } diff --git a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java index 7351e0ef86797..bf7c363d94ae7 100644 --- a/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java +++ b/plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java @@ -63,8 +63,8 @@ public void testDeprecationOfLoneAccessKey() { assertThat(credentials.getAWSAccessKeyId(), is("aws_key")); assertThat(credentials.getAWSSecretKey(), is("")); assertSettingDeprecationsAndWarnings(new Setting[]{}, - new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is " + - "not, which will be unsupported in future")); + new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.access_key] is set but " + + "[discovery.ec2.secret_key] is not, which will be unsupported in future")); } public void testDeprecationOfLoneSecretKey() { @@ -75,8 +75,8 @@ public void testDeprecationOfLoneSecretKey() { assertThat(credentials.getAWSAccessKeyId(), is("")); assertThat(credentials.getAWSSecretKey(), is("aws_secret")); assertSettingDeprecationsAndWarnings(new Setting[]{}, - new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is " + - "not, which will be unsupported in future")); + new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.secret_key] is set but " + + "[discovery.ec2.access_key] is not, which will be unsupported in future")); } public void testRejectionOfLoneSessionToken() { diff --git a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java index e1dbc2d10dede..7acba307d54e9 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageServiceTests.java @@ -59,8 +59,7 @@ public void testClientInitializer() throws Exception { expectThrows(IllegalArgumentException.class, () -> service.client("another_client", "repo", statsCollector)); assertThat(e.getMessage(), Matchers.startsWith("Unknown client name")); assertSettingDeprecationsAndWarnings( - new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) } - ); + new Setting[] { GoogleCloudStorageClientSettings.APPLICATION_NAME_SETTING.getConcreteSettingForNamespace(clientName) }); final Storage storage = service.client(clientName, "repo", statsCollector); assertThat(storage.getOptions().getApplicationName(), Matchers.containsString(applicationName)); assertThat(storage.getOptions().getHost(), Matchers.is(endpoint)); diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 486aff5627c12..d9151c9923e2b 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -550,8 +550,6 @@ String innerGetRaw(final Settings settings) { if (secureSettings != null && secureSettings.getSettingNames().contains(key)) { throw new IllegalArgumentException("Setting [" + key + "] is a non-secure setting" + " and must be stored inside elasticsearch.yml, but was found inside the Elasticsearch keystore"); - - } final String found = settings.get(key); if (found != null) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index e4911067d731d..e645fe09391de 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -18,6 +18,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomPicks; import com.carrotsearch.randomizedtesting.generators.RandomStrings; import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter; + import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -40,20 +41,14 @@ import org.elasticsearch.client.Requests; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.metadata.IndexMetadata; -import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.core.CheckedRunnable; -import org.elasticsearch.core.RestApiVersion; -import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.core.Tuple; -import org.elasticsearch.core.PathUtils; -import org.elasticsearch.core.PathUtilsForTesting; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.common.logging.HeaderWarningAppender; import org.elasticsearch.common.logging.LogConfigurator; @@ -66,16 +61,13 @@ import org.elasticsearch.common.util.MockBigArrays; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; -import org.elasticsearch.xcontent.MediaType; -import org.elasticsearch.xcontent.NamedXContentRegistry; -import org.elasticsearch.xcontent.ToXContent; -import org.elasticsearch.xcontent.XContent; -import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.xcontent.XContentParser; -import org.elasticsearch.xcontent.XContentParser.Token; -import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.core.CheckedRunnable; +import org.elasticsearch.core.PathUtils; +import org.elasticsearch.core.PathUtilsForTesting; +import org.elasticsearch.core.RestApiVersion; +import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.core.Tuple; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.env.TestEnvironment; @@ -101,6 +93,15 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.LeakTracker; import org.elasticsearch.transport.nio.MockNioTransportPlugin; +import org.elasticsearch.xcontent.MediaType; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.ToXContent; +import org.elasticsearch.xcontent.XContent; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentFactory; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParser.Token; +import org.elasticsearch.xcontent.XContentType; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; From 0bb8894bbbfe6b4dbdc8759344d226cf6a32d019 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Mon, 18 Oct 2021 17:02:27 -0500 Subject: [PATCH 12/12] fixing a unit test --- .../org/elasticsearch/common/logging/JsonLoggerTests.java | 2 +- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java index 1c0cf3f973c9b..d15edec2698f9 100644 --- a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java +++ b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java @@ -100,7 +100,7 @@ public void testDeprecationWarnMessage() throws IOException { ); } - assertWarnings("deprecated warn message1"); + assertWarnings(true, new DeprecationWarning(Level.WARN, "deprecated warn message1")) ; } public void testDeprecatedMessageWithoutXOpaqueId() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index e645fe09391de..786b991a8a8f9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -1588,15 +1588,22 @@ public DeprecationWarning(Level level, String message) { this.message = message; } + @Override public int hashCode() { return Objects.hash(level, message); } + @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; DeprecationWarning that = (DeprecationWarning) o; return Objects.equals(level, that.level) && Objects.equals(message, that.message); } + + @Override + public String toString() { + return String.format(Locale.ROOT, "%s (%s): %s", level.name(), level.intLevel(), message); + } } }