Remove support for default settings

Today Elasticsearch allows default settings to be used only if the
actual setting is not set. These settings are trappy, and the complexity
invites bugs. This commit removes support for default settings with the
exception of default.path.data, default.path.conf, and default.path.logs
which are maintainted to support packaging. A follow-up will remove
support for these as well.

Relates #24093

core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -311,9 +311,12 @@ public final class ClusterSettings extends AbstractScopedSettings {
                     HunspellService.HUNSPELL_IGNORE_CASE,
                     HunspellService.HUNSPELL_DICTIONARY_OPTIONS,
                     IndicesStore.INDICES_STORE_DELETE_SHARD_TIMEOUT,
+                    Environment.DEFAULT_PATH_CONF_SETTING,
                     Environment.PATH_CONF_SETTING,
+                    Environment.DEFAULT_PATH_DATA_SETTING,
                     Environment.PATH_DATA_SETTING,
                     Environment.PATH_HOME_SETTING,
+                    Environment.DEFAULT_PATH_LOGS_SETTING,
                     Environment.PATH_LOGS_SETTING,
                     Environment.PATH_REPO_SETTING,
                     Environment.PATH_SCRIPTS_SETTING,

core/src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -1063,12 +1063,10 @@ public final class Settings implements ToXContent {
             return this;
         }
 
-        public Builder putProperties(Map<String, String> esSettings, Predicate<String> keyPredicate, Function<String, String> keyFunction) {
+        public Builder putProperties(final Map<String, String> esSettings, final Function<String, String> keyFunction) {
             for (final Map.Entry<String, String> esSetting : esSettings.entrySet()) {
                 final String key = esSetting.getKey();
-                if (keyPredicate.test(key)) {
-                    map.put(keyFunction.apply(key), esSetting.getValue());
-                }
+                map.put(keyFunction.apply(key), esSetting.getValue());
             }
             return this;
         }

core/src/main/java/org/elasticsearch/env/Environment.java

@@ -49,11 +49,17 @@ import static org.elasticsearch.common.Strings.cleanPath;
 // public+forbidden api!
 public class Environment {
     public static final Setting<String> PATH_HOME_SETTING = Setting.simpleString("path.home", Property.NodeScope);
-    public static final Setting<String> PATH_CONF_SETTING = Setting.simpleString("path.conf", Property.NodeScope);
+    public static final Setting<String> DEFAULT_PATH_CONF_SETTING = Setting.simpleString("default.path.conf", Property.NodeScope);
+    public static final Setting<String> PATH_CONF_SETTING =
+            new Setting<>("path.conf", DEFAULT_PATH_CONF_SETTING, Function.identity(), Property.NodeScope);
     public static final Setting<String> PATH_SCRIPTS_SETTING = Setting.simpleString("path.scripts", Property.NodeScope);
+    public static final Setting<List<String>> DEFAULT_PATH_DATA_SETTING =
+            Setting.listSetting("default.path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
     public static final Setting<List<String>> PATH_DATA_SETTING =
-        Setting.listSetting("path.data", Collections.emptyList(), Function.identity(), Property.NodeScope);
-    public static final Setting<String> PATH_LOGS_SETTING = Setting.simpleString("path.logs", Property.NodeScope);
+            Setting.listSetting("path.data", DEFAULT_PATH_DATA_SETTING, Function.identity(), Property.NodeScope);
+    public static final Setting<String> DEFAULT_PATH_LOGS_SETTING = Setting.simpleString("default.path.logs", Property.NodeScope);
+    public static final Setting<String> PATH_LOGS_SETTING =
+            new Setting<>("path.logs", DEFAULT_PATH_LOGS_SETTING, Function.identity(), Property.NodeScope);
     public static final Setting<List<String>> PATH_REPO_SETTING =
         Setting.listSetting("path.repo", Collections.emptyList(), Function.identity(), Property.NodeScope);
     public static final Setting<String> PATH_SHARED_DATA_SETTING = Setting.simpleString("path.shared_data", Property.NodeScope);
@@ -115,7 +121,8 @@ public class Environment {
             throw new IllegalStateException(PATH_HOME_SETTING.getKey() + " is not configured");
         }
 
-        if (PATH_CONF_SETTING.exists(settings)) {
+        // this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
+        if (PATH_CONF_SETTING.exists(settings) || DEFAULT_PATH_CONF_SETTING.exists(settings)) {
             configFile = PathUtils.get(cleanPath(PATH_CONF_SETTING.get(settings)));
         } else {
             configFile = homeFile.resolve("config");
@@ -156,7 +163,9 @@ public class Environment {
         } else {
             repoFiles = new Path[0];
         }
-        if (PATH_LOGS_SETTING.exists(settings)) {
+
+        // this is trappy, Setting#get(Settings) will get a fallback setting yet return false for Settings#exists(Settings)
+        if (PATH_LOGS_SETTING.exists(settings) || DEFAULT_PATH_LOGS_SETTING.exists(settings)) {
             logsFile = PathUtils.get(cleanPath(PATH_LOGS_SETTING.get(settings)));
         } else {
             logsFile = homeFile.resolve("logs");

core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java

@@ -37,17 +37,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
-import java.util.function.Predicate;
-import java.util.function.UnaryOperator;
 
 import static org.elasticsearch.common.Strings.cleanPath;
 
 public class InternalSettingsPreparer {
 
     private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json"};
-    private static final String PROPERTY_DEFAULTS_PREFIX = "default.";
-    private static final Predicate<String> PROPERTY_DEFAULTS_PREDICATE = key -> key.startsWith(PROPERTY_DEFAULTS_PREFIX);
-    private static final UnaryOperator<String> STRIP_PROPERTY_DEFAULTS_PREFIX = key -> key.substring(PROPERTY_DEFAULTS_PREFIX.length());
 
     public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
     public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
@@ -125,22 +120,16 @@ public class InternalSettingsPreparer {
     }
 
     /**
-     * Initializes the builder with the given input settings, and applies settings and default settings from the specified map (these
-     * settings typically come from the command line). The default settings are applied only if the setting does not exist in the specified
-     * output.
+     * Initializes the builder with the given input settings, and applies settings from the specified map (these settings typically come
+     * from the command line).
      *
      * @param output the settings builder to apply the input and default settings to
      * @param input the input settings
-     * @param esSettings a map from which to apply settings and default settings
+     * @param esSettings a map from which to apply settings
      */
     static void initializeSettings(final Settings.Builder output, final Settings input, final Map<String, String> esSettings) {
         output.put(input);
-        output.putProperties(esSettings,
-                PROPERTY_DEFAULTS_PREDICATE
-                        .and(key -> output.get(STRIP_PROPERTY_DEFAULTS_PREFIX.apply(key)) == null)
-                        .and(key -> output.get(STRIP_PROPERTY_DEFAULTS_PREFIX.apply(key) + ".0") == null),
-                STRIP_PROPERTY_DEFAULTS_PREFIX);
-        output.putProperties(esSettings, PROPERTY_DEFAULTS_PREDICATE.negate(), Function.identity());
+        output.putProperties(esSettings, Function.identity());
         output.replacePropertyPlaceholders();
     }
 

core/src/test/java/org/elasticsearch/env/EnvironmentTests.java

@@ -23,10 +23,12 @@ import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
 import java.net.URL;
+import java.nio.file.Path;
 
 import static org.hamcrest.CoreMatchers.endsWith;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.Matchers.equalTo;
 
 /**
  * Simple unit-tests for Environment.java
@@ -71,4 +73,91 @@ public class EnvironmentTests extends ESTestCase {
         assertThat(environment.resolveRepoURL(new URL("jar:http://localhost/test/../repo1?blah!/repo/")), nullValue());
     }
 
+    public void testDefaultPathData() {
+        final Path defaultPathData = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir().toAbsolutePath())
+                .put("default.path.data", defaultPathData)
+                .build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.dataFiles(), equalTo(new Path[] { defaultPathData }));
+    }
+
+    public void testPathDataOverrideDefaultPathData() {
+        final Path pathData = createTempDir().toAbsolutePath();
+        final Path defaultPathData = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir().toAbsolutePath())
+                .put("path.data", pathData)
+                .put("default.path.data", defaultPathData)
+                .build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.dataFiles(), equalTo(new Path[] { pathData }));
+    }
+
+    public void testPathDataWhenNotSet() {
+        final Path pathHome = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder().put("path.home", pathHome).build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.dataFiles(), equalTo(new Path[]{pathHome.resolve("data")}));
+    }
+
+    public void testDefaultPathLogs() {
+        final Path defaultPathLogs = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir().toAbsolutePath())
+                .put("default.path.logs", defaultPathLogs)
+                .build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.logsFile(), equalTo(defaultPathLogs));
+    }
+
+    public void testPathLogsOverrideDefaultPathLogs() {
+        final Path pathLogs = createTempDir().toAbsolutePath();
+        final Path defaultPathLogs = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir().toAbsolutePath())
+                .put("path.logs", pathLogs)
+                .put("default.path.logs", defaultPathLogs)
+                .build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.logsFile(), equalTo(pathLogs));
+    }
+
+    public void testPathLogsWhenNotSet() {
+        final Path pathHome = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder().put("path.home", pathHome).build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.logsFile(), equalTo(pathHome.resolve("logs")));
+    }
+
+    public void testDefaultPathConf() {
+        final Path defaultPathConf = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir().toAbsolutePath())
+                .put("default.path.conf", defaultPathConf)
+                .build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.configFile(), equalTo(defaultPathConf));
+    }
+
+    public void testPathConfOverrideDefaultPathConf() {
+        final Path pathConf = createTempDir().toAbsolutePath();
+        final Path defaultPathConf = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir().toAbsolutePath())
+                .put("path.conf", pathConf)
+                .put("default.path.conf", defaultPathConf)
+                .build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.configFile(), equalTo(pathConf));
+    }
+
+    public void testPathConfWhenNotSet() {
+        final Path pathHome = createTempDir().toAbsolutePath();
+        final Settings settings = Settings.builder().put("path.home", pathHome).build();
+        final Environment environment = new Environment(settings);
+        assertThat(environment.configFile(), equalTo(pathHome.resolve("config")));
+    }
+
 }

core/src/test/java/org/elasticsearch/node/InternalSettingsPreparerTests.java

@@ -182,29 +182,11 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         assertEquals("secret", fakeSetting.get(env.settings()).toString());
     }
 
-    public void testDefaultProperties() throws Exception {
+    public void testDefaultPropertiesDoNothing() throws Exception {
         Map<String, String> props = Collections.singletonMap("default.setting", "foo");
         Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props);
-        assertEquals("foo", env.settings().get("setting"));
-    }
-
-    public void testDefaultPropertiesOverride() throws Exception {
-        Path configDir = homeDir.resolve("config");
-        Files.createDirectories(configDir);
-        Files.write(configDir.resolve("elasticsearch.yml"), Collections.singletonList("setting: bar"), StandardCharsets.UTF_8);
-        Map<String, String> props = Collections.singletonMap("default.setting", "foo");
-        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props);
-        assertEquals("bar", env.settings().get("setting"));
-    }
-
-    public void testDefaultWithArray() {
-        final Settings.Builder output = Settings.builder().put("foobar.0", "bar").put("foobar.1", "baz");
-        final Map<String, String> esSettings = Collections.singletonMap("default.foobar", "foo");
-        InternalSettingsPreparer.initializeSettings(output, Settings.EMPTY, esSettings);
-        final Settings settings = output.build();
-        assertThat(settings.get("foobar.0"), equalTo("bar"));
-        assertThat(settings.get("foobar.1"), equalTo("baz"));
-        assertNull(settings.get("foobar"));
+        assertEquals("foo", env.settings().get("default.setting"));
+        assertNull(env.settings().get("setting"));
     }
 
 }

docs/reference/setup/configuration.asciidoc

@@ -89,23 +89,6 @@ Enter value for [node.name]:
 NOTE: Elasticsearch will not start if `${prompt.text}` or `${prompt.secret}`
 is used in the settings and the process is run as a service or in the background.
 
-[float]
-=== Setting default settings
-
-New default settings may be specified on the command line using the
-`default.` prefix.  This will specify a value that will be used by
-default unless another value is specified in the config file.
-
-For instance, if Elasticsearch is started as follows:
-
-[source,sh]
----------------------------
-./bin/elasticsearch -Edefault.node.name=My_Node
----------------------------
-
-the value for `node.name` will be `My_Node`, unless it is overwritten on the
-command line with `es.node.name` or in the config file with `node.name`.
-
 [float]
 [[logging]]
 == Logging configuration

docs/reference/setup/install/docker.asciidoc

@@ -227,8 +227,6 @@ The image offers several methods for configuring Elasticsearch settings with the
 ===== A. Present the parameters via Docker environment variables
 For example, to define the cluster name with `docker run` you can pass `-e "cluster.name=mynewclustername"`. Double quotes are required.
 
-NOTE: There is a difference between defining <<_setting_default_settings,default settings>> and normal settings. The former are prefixed with `default.` and cannot override normal settings, if defined.
-
 ===== B. Bind-mounted configuration
 Create your custom config file and mount this over the image's corresponding file.
 For example, bind-mounting a `custom_elasticsearch.yml` with `docker run` can be accomplished with the parameter: