Settings: Fix settings reading to account for defaults (#22871)

In #22762, settings preparation during bootstrap was changed slightly to
account for SecureSettings, by starting with a fresh settings builder
after reading the initial configuration. However, this the defaults from
system properties were never re-read. This change fixes that bug (which
was never released).

closes #22861

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

@@ -30,6 +30,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
 
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cluster.ClusterName;
@@ -44,8 +45,9 @@ import static org.elasticsearch.common.Strings.cleanPath;
 public class InternalSettingsPreparer {
 
     private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json"};
-    static final String PROPERTY_DEFAULTS_PREFIX = "default.";
-    static final Predicate<String> PROPERTY_DEFAULTS_PREDICATE = key -> key.startsWith(PROPERTY_DEFAULTS_PREFIX);
+    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}";
@@ -55,7 +57,7 @@ public class InternalSettingsPreparer {
      */
     public static Settings prepareSettings(Settings input) {
         Settings.Builder output = Settings.builder();
-        initializeSettings(output, input, true, Collections.emptyMap());
+        initializeSettings(output, input, Collections.emptyMap());
         finalizeSettings(output, null);
         return output.build();
     }
@@ -86,7 +88,7 @@ public class InternalSettingsPreparer {
     public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties) {
         // just create enough settings to build the environment, to get the config dir
         Settings.Builder output = Settings.builder();
-        initializeSettings(output, input, true, properties);
+        initializeSettings(output, input, properties);
         Environment environment = new Environment(output.build());
 
         output = Settings.builder(); // start with a fresh output
@@ -112,8 +114,7 @@ public class InternalSettingsPreparer {
         }
 
         // re-initialize settings now that the config file has been loaded
-        // TODO: only re-initialize if a config file was actually loaded
-        initializeSettings(output, input, false, properties);
+        initializeSettings(output, input, properties);
         finalizeSettings(output, terminal);
 
         environment = new Environment(output.build());
@@ -127,11 +128,11 @@ public class InternalSettingsPreparer {
      * Initializes the builder with the given input settings, and loads system properties settings if allowed.
      * If loadDefaults is true, system property default settings are loaded.
      */
-    private static void initializeSettings(Settings.Builder output, Settings input, boolean loadDefaults, Map<String, String> esSettings) {
+    private static void initializeSettings(Settings.Builder output, Settings input, Map<String, String> esSettings) {
         output.put(input);
-        if (loadDefaults) {
-            output.putProperties(esSettings, PROPERTY_DEFAULTS_PREDICATE, key -> key.substring(PROPERTY_DEFAULTS_PREFIX.length()));
-        }
+        output.putProperties(esSettings,
+            PROPERTY_DEFAULTS_PREDICATE.and(key -> output.get(STRIP_PROPERTY_DEFAULTS_PREFIX.apply(key)) == null),
+            STRIP_PROPERTY_DEFAULTS_PREFIX);
         output.putProperties(esSettings, PROPERTY_DEFAULTS_PREDICATE.negate(), Function.identity());
         output.replacePropertyPlaceholders();
     }

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

@@ -35,25 +35,32 @@ 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;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
 public class InternalSettingsPreparerTests extends ESTestCase {
 
+    Path homeDir;
     Settings baseEnvSettings;
 
     @Before
     public void createBaseEnvSettings() {
+        homeDir = createTempDir();
         baseEnvSettings = Settings.builder()
-            .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir())
+            .put(Environment.PATH_HOME_SETTING.getKey(), homeDir)
             .build();
     }
 
     @After
     public void clearBaseEnvSettings() {
+        homeDir = null;
         baseEnvSettings = null;
     }
 
@@ -174,4 +181,19 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         Setting<SecureString> fakeSetting = SecureSetting.secureString("foo", null, false);
         assertEquals("secret", fakeSetting.get(env.settings()).toString());
     }
+
+    public void testDefaultProperties() 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"));
+    }
 }