diff options
author | Ryan Ernst <ryan@iernst.net> | 2017-05-11 13:11:18 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-05-11 13:11:18 -0700 |
commit | f477a6472d523b64a94c496e08f27a307099682e (patch) | |
tree | 6a6a25e703ec0ef4a2f869ba7c6a84c2c85b8919 | |
parent | 2ffdd4468d8a2671a160278a3a94714e72347786 (diff) |
Settings: Deprecate settings in .yml and .json (#24059)
This commit adds a deprecation warning when elasticsearch.yml or
elasticsearch.json is read during startup.
relates #19391
6 files changed, 99 insertions, 27 deletions
diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 1e53faa9ef..d0462efc39 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -35,6 +35,7 @@ import org.elasticsearch.cli.UserException; import org.elasticsearch.common.PidFile; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.inject.CreationException; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.logging.LogConfigurator; import org.elasticsearch.common.logging.Loggers; @@ -297,6 +298,7 @@ final class Bootstrap { throw new BootstrapException(e); } checkForCustomConfFile(); + checkConfigExtension(environment.configExtension()); if (environment.pidFile() != null) { try { @@ -412,6 +414,14 @@ final class Bootstrap { } } + // pkg private for tests + static void checkConfigExtension(String extension) { + if (".yml".equals(extension) || ".json".equals(extension)) { + final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(Bootstrap.class)); + deprecationLogger.deprecated("elasticsearch{} is deprecated; rename your configuration file to elasticsearch.yaml", extension); + } + } + @SuppressForbidden(reason = "Allowed to exit explicitly in bootstrap phase") private static void exit(int status) { System.exit(status); diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index d1ad668e0e..df859ab7de 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -71,6 +71,8 @@ public class Environment { private final Settings settings; + private final String configExtension; + private final Path[] dataFiles; private final Path[] dataWithClusterFiles; @@ -102,6 +104,12 @@ public class Environment { private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir")); public Environment(Settings settings) { + this(settings, null); + } + + // Note: Do not use this ctor, it is for correct deprecation logging in 5.5 and will be removed + public Environment(Settings settings, String configExtension) { + this.configExtension = configExtension; final Path homeFile; if (PATH_HOME_SETTING.exists(settings)) { homeFile = PathUtils.get(cleanPath(PATH_HOME_SETTING.get(settings))); @@ -273,8 +281,14 @@ public class Environment { } } + /** Return then extension of the config file that was loaded, or*/ + public String configExtension() { + return configExtension; + } + + // TODO: rename all these "file" methods to "dir" /** - * The config location. + * The config directory. */ public Path configFile() { return configFile; diff --git a/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java index d9cefc6b22..b8862fb20e 100644 --- a/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java @@ -19,14 +19,6 @@ package org.elasticsearch.node; -import org.elasticsearch.cli.Terminal; -import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsException; -import org.elasticsearch.env.Environment; - import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -38,6 +30,14 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsException; +import org.elasticsearch.env.Environment; + import static org.elasticsearch.common.Strings.cleanPath; public class InternalSettingsPreparer { @@ -116,7 +116,8 @@ public class InternalSettingsPreparer { // we put back the path.logs so we can use it in the logging configuration file output.put(Environment.PATH_LOGS_SETTING.getKey(), cleanPath(environment.logsFile().toAbsolutePath().toString())); - return new Environment(output.build()); + String configExtension = foundSuffixes.isEmpty() ? null : foundSuffixes.iterator().next(); + return new Environment(output.build(), configExtension); } /** diff --git a/core/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java new file mode 100644 index 0000000000..b2fab7746f --- /dev/null +++ b/core/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java @@ -0,0 +1,33 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.bootstrap; + +import org.elasticsearch.test.ESTestCase; + +public class BootstrapTests extends ESTestCase { + + public void testConfigDeprecation() { + Bootstrap.checkConfigExtension(".json"); + assertWarnings("elasticsearch.json is deprecated; rename your configuration file to elasticsearch.yaml"); + Bootstrap.checkConfigExtension(".yml"); + assertWarnings("elasticsearch.yml is deprecated; rename your configuration file to elasticsearch.yaml"); + Bootstrap.checkConfigExtension(".yaml"); // no warnings, will be checked in @After + } +} diff --git a/core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java index 033fa9cdf2..5db397ab16 100644 --- a/core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java +++ b/core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java @@ -28,17 +28,14 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.env.Environment; -import org.elasticsearch.node.InternalSettingsPreparer; import org.elasticsearch.test.ESTestCase; import org.junit.After; import org.junit.Before; import java.io.IOException; import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; import java.util.Collections; import java.util.Map; @@ -155,22 +152,36 @@ public class InternalSettingsPreparerTests extends ESTestCase { public void testMultipleSettingsFileNotAllowed() throws IOException { InputStream yaml = getClass().getResourceAsStream("/config/elasticsearch.yaml"); - InputStream properties = getClass().getResourceAsStream("/config/elasticsearch.properties"); - Path home = createTempDir(); - Path config = home.resolve("config"); + InputStream json = getClass().getResourceAsStream("/config/elasticsearch.json"); + Path config = homeDir.resolve("config"); Files.createDirectory(config); Files.copy(yaml, config.resolve("elasticsearch.yaml")); - Files.copy(properties, config.resolve("elasticsearch.properties")); + Files.copy(json, config.resolve("elasticsearch.json")); + + SettingsException e = expectThrows(SettingsException.class, () -> + InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build(), null) + ); + assertTrue(e.getMessage(), e.getMessage().contains("multiple settings files found with suffixes")); + assertTrue(e.getMessage(), e.getMessage().contains(".yaml")); + assertTrue(e.getMessage(), e.getMessage().contains(".json")); + } - try { - InternalSettingsPreparer.prepareEnvironment(Settings.builder() - .put(baseEnvSettings) - .build(), null); - } catch (SettingsException e) { - assertTrue(e.getMessage(), e.getMessage().contains("multiple settings files found with suffixes")); - assertTrue(e.getMessage(), e.getMessage().contains(".yaml")); - assertTrue(e.getMessage(), e.getMessage().contains(".properties")); - } + public void testYmlExtension() throws IOException { + InputStream yaml = getClass().getResourceAsStream("/config/elasticsearch.yaml"); + Path config = homeDir.resolve("config"); + Files.createDirectory(config); + Files.copy(yaml, config.resolve("elasticsearch.yml")); + Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build(), null); + assertEquals(".yml", env.configExtension()); + } + + public void testJsonExtension() throws IOException { + InputStream yaml = getClass().getResourceAsStream("/config/elasticsearch.json"); + Path config = homeDir.resolve("config"); + Files.createDirectory(config); + Files.copy(yaml, config.resolve("elasticsearch.json")); + Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build(), null); + assertEquals(".json", env.configExtension()); } public void testSecureSettings() { 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 cdab659bdc..56f898b900 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -140,8 +140,11 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static org.elasticsearch.common.util.CollectionUtils.arrayAsArrayList; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; /** * Base testcase for randomized unit testing with Elasticsearch @@ -336,7 +339,7 @@ public abstract class ESTestCase extends LuceneTestCase { final Set<String> actualWarningValues = actualWarnings.stream().map(DeprecationLogger::extractWarningValueFromWarningHeader).collect(Collectors.toSet()); for (String msg : expectedWarnings) { - assertTrue(msg, actualWarningValues.contains(DeprecationLogger.escape(msg))); + assertThat(actualWarningValues, hasItem(DeprecationLogger.escape(msg))); } assertEquals("Expected " + expectedWarnings.length + " warnings but found " + actualWarnings.size() + "\nExpected: " + Arrays.asList(expectedWarnings) + "\nActual: " + actualWarnings, |