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

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);

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;

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);
     }
 
     /**

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
+    }
+}

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"));
 
-        try {
-            InternalSettingsPreparer.prepareEnvironment(Settings.builder()
-                .put(baseEnvSettings)
-                .build(), null);
-        } catch (SettingsException e) {
+        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(".properties"));
+        assertTrue(e.getMessage(), e.getMessage().contains(".json"));
     }
+
+    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() {

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,