diff --git a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java index dbdaa5a1aeb..7a824cd1ed0 100644 --- a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java @@ -50,8 +50,8 @@ public class InternalSettingsPreparer { /** * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings, - * and then replacing all property placeholders. This method will not work with settings that have __prompt__ - * as their value unless they have been resolved previously. + * and then replacing all property placeholders. This method will not work with settings that have ${prompt.text} + * or ${prompt.secret} as their value unless they have been resolved previously. * @param pSettings The initial settings to use * @param loadConfigSettings flag to indicate whether to load settings from the configuration directory/file * @return the {@link Settings} and {@link Environment} as a {@link Tuple} @@ -63,7 +63,8 @@ public class InternalSettingsPreparer { /** * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings, * and then replacing all property placeholders. If a {@link Terminal} is provided and configuration settings are loaded, - * settings with the __prompt__ value will result in a prompt for the setting to the user. + * settings with a value of ${prompt.text} or ${prompt.secret} will result in a prompt for + * the setting to the user. * @param pSettings The initial settings to use * @param loadConfigSettings flag to indicate whether to load settings from the configuration directory/file * @param terminal the Terminal to use for input/output @@ -131,16 +132,9 @@ public class InternalSettingsPreparer { } settingsBuilder.replacePropertyPlaceholders(); - // generate the name + // check if name is set in settings, if not look for system property and set it if (settingsBuilder.get("name") == null) { String name = System.getProperty("name"); - if (name == null || name.isEmpty()) { - name = settingsBuilder.get("node.name"); - if (name == null || name.isEmpty()) { - name = Names.randomNodeName(environment.resolveConfig("names.txt")); - } - } - if (name != null) { settingsBuilder.put("name", name); } @@ -155,17 +149,33 @@ public class InternalSettingsPreparer { if (v != null) { Settings.setSettingsRequireUnits(Booleans.parseBoolean(v, true)); } - Settings v1 = replacePromptPlaceholders(settingsBuilder.build(), terminal); - environment = new Environment(v1); + + Settings settings = replacePromptPlaceholders(settingsBuilder.build(), terminal); + // all settings placeholders have been resolved. resolve the value for the name setting by checking for name, + // then looking for node.name, and finally generate one if needed + if (settings.get("name") == null) { + final String name = settings.get("node.name"); + if (name == null || name.isEmpty()) { + settings = settingsBuilder().put(settings) + .put("name", Names.randomNodeName(environment.resolveConfig("names.txt"))) + .build(); + } else { + settings = settingsBuilder().put(settings) + .put("name", name) + .build(); + } + } + + environment = new Environment(settings); // put back the env settings - settingsBuilder = settingsBuilder().put(v1); + settingsBuilder = settingsBuilder().put(settings); // we put back the path.logs so we can use it in the logging configuration file settingsBuilder.put("path.logs", cleanPath(environment.logsFile().toAbsolutePath().toString())); - v1 = settingsBuilder.build(); + settings = settingsBuilder.build(); - return new Tuple<>(v1, environment); + return new Tuple<>(settings, environment); } static Settings replacePromptPlaceholders(Settings settings, Terminal terminal) { diff --git a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java index 82b6a2a601b..2574b62ffb7 100644 --- a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java +++ b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java @@ -31,22 +31,23 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.common.settings.Settings.settingsBuilder; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.*; public class InternalSettingsPreparerTests extends ElasticsearchTestCase { @Before public void setupSystemProperties() { System.setProperty("es.node.zone", "foo"); + System.setProperty("name", "sys-prop-name"); } @After public void cleanupSystemProperties() { System.clearProperty("es.node.zone"); + System.clearProperty("name"); } @Test @@ -151,4 +152,72 @@ public class InternalSettingsPreparerTests extends ElasticsearchTestCase { assertThat(e.getMessage(), containsString("with value [" + InternalSettingsPreparer.TEXT_PROMPT_VALUE + "]")); } } + + @Test + public void testNameSettingsPreference() { + // Test system property overrides node.name + Settings settings = settingsBuilder() + .put("node.name", "node-name") + .put("path.home", createTempDir().toString()) + .build(); + Tuple tuple = InternalSettingsPreparer.prepareSettings(settings, true); + assertThat(tuple.v1().get("name"), equalTo("sys-prop-name")); + + // test name in settings overrides sys prop and node.name + settings = settingsBuilder() + .put("name", "name-in-settings") + .put("node.name", "node-name") + .put("path.home", createTempDir().toString()) + .build(); + tuple = InternalSettingsPreparer.prepareSettings(settings, true); + assertThat(tuple.v1().get("name"), equalTo("name-in-settings")); + + // test only node.name in settings + System.clearProperty("name"); + settings = settingsBuilder() + .put("node.name", "node-name") + .put("path.home", createTempDir().toString()) + .build(); + tuple = InternalSettingsPreparer.prepareSettings(settings, true); + assertThat(tuple.v1().get("name"), equalTo("node-name")); + + // test no name at all results in name being set + settings = settingsBuilder() + .put("path.home", createTempDir().toString()) + .build(); + tuple = InternalSettingsPreparer.prepareSettings(settings, true); + assertThat(tuple.v1().get("name"), not("name-in-settings")); + assertThat(tuple.v1().get("name"), not("sys-prop-name")); + assertThat(tuple.v1().get("name"), not("node-name")); + assertThat(tuple.v1().get("name"), notNullValue()); + } + + @Test + public void testPromptForNodeNameOnlyPromptsOnce() { + final AtomicInteger counter = new AtomicInteger(); + final Terminal terminal = new CliToolTestCase.MockTerminal() { + @Override + public char[] readSecret(String message, Object... args) { + fail("readSecret should never be called by this test"); + return null; + } + + @Override + public String readText(String message, Object... args) { + int count = counter.getAndIncrement(); + return "prompted name " + count; + } + }; + + System.clearProperty("name"); + Settings settings = Settings.builder() + .put("path.home", createTempDir()) + .put("node.name", InternalSettingsPreparer.TEXT_PROMPT_VALUE) + .build(); + Tuple tuple = InternalSettingsPreparer.prepareSettings(settings, false, terminal); + settings = tuple.v1(); + assertThat(counter.intValue(), is(1)); + assertThat(settings.get("name"), is("prompted name 0")); + assertThat(settings.get("node.name"), is("prompted name 0")); + } }