Remove config prompting for secrets and text (#27216)

This commit removes the ability to use ${prompt.secret} and
${prompt.text} as valid config settings. Secure settings has obsoleted
the need for this, and it cleans up some of the code in Bootstrap.

core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -30,7 +30,6 @@ import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.StringHelper;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.Version;
-import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.UserException;
 import org.elasticsearch.common.PidFile;
 import org.elasticsearch.common.SuppressForbidden;
@@ -245,7 +244,6 @@ final class Bootstrap {
             final SecureSettings secureSettings,
             final Settings initialSettings,
             final Path configPath) {
-        Terminal terminal = foreground ? Terminal.DEFAULT : null;
         Settings.Builder builder = Settings.builder();
         if (pidFile != null) {
             builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile);
@@ -254,7 +252,7 @@ final class Bootstrap {
         if (secureSettings != null) {
             builder.setSecureSettings(secureSettings);
         }
-        return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap(), configPath);
+        return InternalSettingsPreparer.prepareEnvironment(builder.build(), Collections.emptyMap(), configPath);
     }
 
     private void start() throws NodeValidationException {

core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java

@@ -68,16 +68,16 @@ public abstract class EnvironmentAwareCommand extends Command {
         putSystemPropertyIfSettingIsMissing(settings, "path.home", "es.path.home");
         putSystemPropertyIfSettingIsMissing(settings, "path.logs", "es.path.logs");
 
-        execute(terminal, options, createEnv(terminal, settings));
+        execute(terminal, options, createEnv(settings));
     }
 
     /** Create an {@link Environment} for the command to use. Overrideable for tests. */
-    protected Environment createEnv(final Terminal terminal, final Map<String, String> settings) throws UserException {
+    protected Environment createEnv(final Map<String, String> settings) throws UserException {
         final String esPathConf = System.getProperty("es.path.conf");
         if (esPathConf == null) {
             throw new UserException(ExitCodes.CONFIG, "the system property [es.path.conf] must be set");
         }
-        return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, terminal, settings, getConfigPath(esPathConf));
+        return InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, settings, getConfigPath(esPathConf));
     }
 
     @SuppressForbidden(reason = "need path to construct environment")

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

@@ -28,9 +28,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
 
-import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.Version;
 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;
@@ -38,10 +37,8 @@ import org.elasticsearch.env.Environment;
 
 public class InternalSettingsPreparer {
 
-    private static final String[] ALLOWED_SUFFIXES = {".yml", ".yaml", ".json"};
-
-    public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
-    public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
+    private static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
+    private static final String TEXT_PROMPT_VALUE = "${prompt.text}";
 
     /**
      * Prepares the settings by gathering all elasticsearch system properties and setting defaults.
@@ -49,36 +46,29 @@ public class InternalSettingsPreparer {
     public static Settings prepareSettings(Settings input) {
         Settings.Builder output = Settings.builder();
         initializeSettings(output, input, Collections.emptyMap());
-        finalizeSettings(output, null);
+        finalizeSettings(output);
         return output.build();
     }
 
     /**
-     * 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 a value of <code>${prompt.text}</code> or <code>${prompt.secret}</code> will result in a prompt for
-     * the setting to the user.
+     * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings.
+     *
      * @param input The custom settings to use. These are not overwritten by settings in the configuration file.
-     * @param terminal the Terminal to use for input/output
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
-    public static Environment prepareEnvironment(Settings input, Terminal terminal) {
-        return prepareEnvironment(input, terminal, Collections.emptyMap(), null);
+    public static Environment prepareEnvironment(Settings input) {
+        return prepareEnvironment(input, Collections.emptyMap(), null);
     }
 
     /**
-     * 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 a value of <code>${prompt.text}</code> or <code>${prompt.secret}</code> will result in a prompt for
-     * the setting to the user.
+     * Prepares the settings by gathering all elasticsearch system properties, optionally loading the configuration settings.
      *
      * @param input      the custom settings to use; these are not overwritten by settings in the configuration file
-     * @param terminal   the Terminal to use for input/output
      * @param properties map of properties key/value pairs (usually from the command-line)
      * @param configPath path to config directory; (use null to indicate the default)
      * @return the {@link Settings} and {@link Environment} as a {@link Tuple}
      */
-    public static Environment prepareEnvironment(Settings input, Terminal terminal, Map<String, String> properties, Path configPath) {
+    public static Environment prepareEnvironment(Settings input, Map<String, String> properties, Path configPath) {
         // just create enough settings to build the environment, to get the config dir
         Settings.Builder output = Settings.builder();
         initializeSettings(output, input, properties);
@@ -104,7 +94,8 @@ public class InternalSettingsPreparer {
 
         // re-initialize settings now that the config file has been loaded
         initializeSettings(output, input, properties);
-        finalizeSettings(output, terminal);
+        checkSettingsForTerminalDeprecation(output);
+        finalizeSettings(output);
 
         environment = new Environment(output.build(), configPath);
 
@@ -128,10 +119,28 @@ public class InternalSettingsPreparer {
     }
 
     /**
-     * Finish preparing settings by replacing forced settings, prompts, and any defaults that need to be added.
-     * The provided terminal is used to prompt for settings needing to be replaced.
+     * Checks all settings values to make sure they do not have the old prompt settings. These were deprecated in 6.0.0.
+     * This check should be removed in 8.0.0.
      */
-    private static void finalizeSettings(Settings.Builder output, Terminal terminal) {
+    private static void checkSettingsForTerminalDeprecation(final Settings.Builder output) throws SettingsException {
+        // This method to be removed in 8.0.0, as it was deprecated in 6.0 and removed in 7.0
+        assert Version.CURRENT.major != 8: "Logic pertaining to config driven prompting should be removed";
+        for (String setting : output.keys()) {
+            switch (output.get(setting)) {
+                case SECRET_PROMPT_VALUE:
+                    throw new SettingsException("Config driven secret prompting was deprecated in 6.0.0. Use the keystore" +
+                        " for secure settings.");
+                case TEXT_PROMPT_VALUE:
+                    throw new SettingsException("Config driven text prompting was deprecated in 6.0.0. Use the keystore" +
+                        " for secure settings.");
+            }
+        }
+    }
+
+    /**
+     * Finish preparing settings by replacing forced settings and any defaults that need to be added.
+     */
+    private static void finalizeSettings(Settings.Builder output) {
         // allow to force set properties based on configuration of the settings provided
         List<String> forcedSettings = new ArrayList<>();
         for (String setting : output.keys()) {
@@ -149,53 +158,5 @@ public class InternalSettingsPreparer {
         if (output.get(ClusterName.CLUSTER_NAME_SETTING.getKey()) == null) {
             output.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY).value());
         }
-
-        replacePromptPlaceholders(output, terminal);
-    }
-
-    private static void replacePromptPlaceholders(Settings.Builder settings, Terminal terminal) {
-        List<String> secretToPrompt = new ArrayList<>();
-        List<String> textToPrompt = new ArrayList<>();
-        for (String key : settings.keys()) {
-            switch (settings.get(key)) {
-                case SECRET_PROMPT_VALUE:
-                    secretToPrompt.add(key);
-                    break;
-                case TEXT_PROMPT_VALUE:
-                    textToPrompt.add(key);
-                    break;
-            }
-        }
-        for (String setting : secretToPrompt) {
-            String secretValue = promptForValue(setting, terminal, true);
-            if (Strings.hasLength(secretValue)) {
-                settings.put(setting, secretValue);
-            } else {
-                // TODO: why do we remove settings if prompt returns empty??
-                settings.remove(setting);
-            }
-        }
-        for (String setting : textToPrompt) {
-            String textValue = promptForValue(setting, terminal, false);
-            if (Strings.hasLength(textValue)) {
-                settings.put(setting, textValue);
-            } else {
-                // TODO: why do we remove settings if prompt returns empty??
-                settings.remove(setting);
-            }
-        }
-    }
-
-    private static String promptForValue(String key, Terminal terminal, boolean secret) {
-        if (terminal == null) {
-            throw new UnsupportedOperationException("found property [" + key + "] with value ["
-                + (secret ? SECRET_PROMPT_VALUE : TEXT_PROMPT_VALUE)
-                + "]. prompting for property values is only supported when running elasticsearch in the foreground");
-        }
-
-        if (secret) {
-            return new String(terminal.readSecret("Enter value for [" + key + "]: "));
-        }
-        return terminal.readText("Enter value for [" + key + "]: ");
     }
 }

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

@@ -238,7 +238,7 @@ public class Node implements Closeable {
      * @param preparedSettings Base settings to configure the node with
      */
     public Node(Settings preparedSettings) {
-        this(InternalSettingsPreparer.prepareEnvironment(preparedSettings, null));
+        this(InternalSettingsPreparer.prepareEnvironment(preparedSettings));
     }
 
     public Node(Environment environment) {

core/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java

@@ -37,7 +37,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
     protected Command newCommand() {
         return new AddFileKeyStoreCommand() {
             @Override
-            protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+            protected Environment createEnv(Map<String, String> settings) throws UserException {
                 return env;
             }
         };

core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java

@@ -39,7 +39,7 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase {
     protected Command newCommand() {
         return new AddStringKeyStoreCommand() {
             @Override
-            protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+            protected Environment createEnv(Map<String, String> settings) throws UserException {
                 return env;
             }
             @Override

core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java

@@ -35,7 +35,7 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase {
     protected Command newCommand() {
         return new CreateKeyStoreCommand() {
             @Override
-            protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+            protected Environment createEnv(Map<String, String> settings) throws UserException {
                 return env;
             }
         };

core/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java

@@ -35,7 +35,7 @@ public class ListKeyStoreCommandTests extends KeyStoreCommandTestCase {
     protected Command newCommand() {
         return new ListKeyStoreCommand() {
             @Override
-            protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+            protected Environment createEnv(Map<String, String> settings) throws UserException {
                 return env;
             }
         };

core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java

@@ -36,7 +36,7 @@ public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase {
     protected Command newCommand() {
         return new RemoveSettingKeyStoreCommand() {
             @Override
-            protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+            protected Environment createEnv(Map<String, String> settings) throws UserException {
                 return env;
             }
         };

core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java

@@ -87,16 +87,6 @@ public class SettingsTests extends ESTestCase {
         assertThat(implicitEnvSettings.get("setting1"), equalTo(hostname));
     }
 
-    public void testReplacePropertiesPlaceholderIgnoresPrompt() {
-        Settings settings = Settings.builder()
-                .put("setting1", "${prompt.text}")
-                .put("setting2", "${prompt.secret}")
-                .replacePropertyPlaceholders()
-                .build();
-        assertThat(settings.get("setting1"), is("${prompt.text}"));
-        assertThat(settings.get("setting2"), is("${prompt.secret}"));
-    }
-
     public void testGetAsSettings() {
         Settings settings = Settings.builder()
                 .put("bar", "hello world")

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

@@ -67,7 +67,7 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         assertNotNull(settings.get(ClusterName.CLUSTER_NAME_SETTING.getKey())); // a cluster name was set
         int size = settings.names().size();
 
-        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null);
+        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings);
         settings = env.settings();
         assertNull(settings.get("node.name")); // a name was not set
         assertNotNull(settings.get(ClusterName.CLUSTER_NAME_SETTING.getKey())); // a cluster name was set
@@ -84,57 +84,6 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         assertEquals("foobar", settings.get("cluster.name"));
     }
 
-    public void testReplacePromptPlaceholders() {
-        MockTerminal terminal = new MockTerminal();
-        terminal.addTextInput("text");
-        terminal.addSecretInput("replaced");
-
-        Settings.Builder builder = Settings.builder()
-                .put(baseEnvSettings)
-                .put("password.replace", InternalSettingsPreparer.SECRET_PROMPT_VALUE)
-                .put("dont.replace", "prompt:secret")
-                .put("dont.replace2", "_prompt:secret_")
-                .put("dont.replace3", "_prompt:text__")
-                .put("dont.replace4", "__prompt:text_")
-                .put("dont.replace5", "prompt:secret__")
-                .put("replace_me", InternalSettingsPreparer.TEXT_PROMPT_VALUE);
-        Settings settings = InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal).settings();
-
-        assertThat(settings.get("password.replace"), equalTo("replaced"));
-        assertThat(settings.get("replace_me"), equalTo("text"));
-
-        // verify other values unchanged
-        assertThat(settings.get("dont.replace"), equalTo("prompt:secret"));
-        assertThat(settings.get("dont.replace2"), equalTo("_prompt:secret_"));
-        assertThat(settings.get("dont.replace3"), equalTo("_prompt:text__"));
-        assertThat(settings.get("dont.replace4"), equalTo("__prompt:text_"));
-        assertThat(settings.get("dont.replace5"), equalTo("prompt:secret__"));
-    }
-
-    public void testReplaceSecretPromptPlaceholderWithNullTerminal() {
-        Settings.Builder builder = Settings.builder()
-                .put(baseEnvSettings)
-                .put("replace_me1", InternalSettingsPreparer.SECRET_PROMPT_VALUE);
-        try {
-            InternalSettingsPreparer.prepareEnvironment(builder.build(), null);
-            fail("an exception should have been thrown since no terminal was provided!");
-        } catch (UnsupportedOperationException e) {
-            assertThat(e.getMessage(), containsString("with value [" + InternalSettingsPreparer.SECRET_PROMPT_VALUE + "]"));
-        }
-    }
-
-    public void testReplaceTextPromptPlaceholderWithNullTerminal() {
-        Settings.Builder builder = Settings.builder()
-                .put(baseEnvSettings)
-                .put("replace_me1", InternalSettingsPreparer.TEXT_PROMPT_VALUE);
-        try {
-            InternalSettingsPreparer.prepareEnvironment(builder.build(), null);
-            fail("an exception should have been thrown since no terminal was provided!");
-        } catch (UnsupportedOperationException e) {
-            assertThat(e.getMessage(), containsString("with value [" + InternalSettingsPreparer.TEXT_PROMPT_VALUE + "]"));
-        }
-    }
-
     public void testGarbageIsNotSwallowed() throws IOException {
         try {
             InputStream garbage = getClass().getResourceAsStream("/config/garbage/garbage.yml");
@@ -144,7 +93,7 @@ public class InternalSettingsPreparerTests extends ESTestCase {
             Files.copy(garbage, config.resolve("elasticsearch.yml"));
             InternalSettingsPreparer.prepareEnvironment(Settings.builder()
                 .put(baseEnvSettings)
-                .build(), null);
+                .build());
         } catch (SettingsException e) {
             assertEquals("Failed to load settings from [elasticsearch.yml]", e.getMessage());
         }
@@ -156,7 +105,7 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         Files.createDirectory(config);
         Files.copy(yaml, config.resolve("elasticsearch.yaml"));
         SettingsException e = expectThrows(SettingsException.class, () ->
-            InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build(), null));
+            InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build()));
         assertEquals("elasticsearch.yaml was deprecated in 5.5.0 and must be renamed to elasticsearch.yml", e.getMessage());
     }
 
@@ -166,7 +115,7 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         Files.createDirectory(config);
         Files.copy(yaml, config.resolve("elasticsearch.json"));
         SettingsException e = expectThrows(SettingsException.class, () ->
-            InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build(), null));
+            InternalSettingsPreparer.prepareEnvironment(Settings.builder().put(baseEnvSettings).build()));
         assertEquals("elasticsearch.json was deprecated in 5.5.0 and must be converted to elasticsearch.yml", e.getMessage());
     }
 
@@ -174,14 +123,14 @@ public class InternalSettingsPreparerTests extends ESTestCase {
         MockSecureSettings secureSettings = new MockSecureSettings();
         secureSettings.setString("foo", "secret");
         Settings input = Settings.builder().put(baseEnvSettings).setSecureSettings(secureSettings).build();
-        Environment env = InternalSettingsPreparer.prepareEnvironment(input, null);
+        Environment env = InternalSettingsPreparer.prepareEnvironment(input);
         Setting<SecureString> fakeSetting = SecureSetting.secureString("foo", null);
         assertEquals("secret", fakeSetting.get(env.settings()).toString());
     }
 
     public void testDefaultPropertiesDoNothing() throws Exception {
         Map<String, String> props = Collections.singletonMap("default.setting", "foo");
-        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, null, props, null);
+        Environment env = InternalSettingsPreparer.prepareEnvironment(baseEnvSettings, props, null);
         assertEquals("foo", env.settings().get("default.setting"));
         assertNull(env.settings().get("setting"));
     }

distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java

@@ -69,7 +69,7 @@ public class ListPluginsCommandTests extends ESTestCase {
         MockTerminal terminal = new MockTerminal();
         int status = new ListPluginsCommand() {
             @Override
-            protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+            protected Environment createEnv(Map<String, String> settings) throws UserException {
                 Settings.Builder builder = Settings.builder().put("path.home", home);
                 settings.forEach((k,v) -> builder.put(k, v));
                 final Settings realSettings = builder.build();

distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java

@@ -57,7 +57,7 @@ public class RemovePluginCommandTests extends ESTestCase {
         }
 
         @Override
-        protected Environment createEnv(Terminal terminal, Map<String, String> settings) throws UserException {
+        protected Environment createEnv(Map<String, String> settings) throws UserException {
             return env;
         }
 

docs/reference/setup/configuration.asciidoc

@@ -78,30 +78,3 @@ variable, for instance:
 node.name:    ${HOSTNAME}
 network.host: ${ES_NETWORK_HOST}
 --------------------------------------------------
-
-[float]
-=== Prompting for settings
-
-For settings that you do not wish to store in the configuration file, you can
-use the value `${prompt.text}` or `${prompt.secret}` and start Elasticsearch
-in the foreground. `${prompt.secret}` has echoing disabled so that the value
-entered will not be shown in your terminal; `${prompt.text}` will allow you to
-see the value as you type it in. For example:
-
-[source,yaml]
---------------------------------------------------
-node:
-  name: ${prompt.text}
---------------------------------------------------
-
-When starting Elasticsearch, you will be prompted to enter the actual value
-like so:
-
-[source,sh]
---------------------------------------------------
-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.
-

test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java

@@ -51,7 +51,7 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase {
             final AtomicBoolean init = new AtomicBoolean();
             final int status = Elasticsearch.main(args, new Elasticsearch() {
                 @Override
-                protected Environment createEnv(final Terminal terminal, final Map<String, String> settings) throws UserException {
+                protected Environment createEnv(final Map<String, String> settings) throws UserException {
                     Settings.Builder builder = Settings.builder().put("path.home", home);
                     settings.forEach((k,v) -> builder.put(k, v));
                     final Settings realSettings = builder.build();

test/framework/src/main/java/org/elasticsearch/node/MockNode.java

@@ -66,7 +66,7 @@ public class MockNode extends Node {
     }
 
     public MockNode(Settings settings, Collection<Class<? extends Plugin>> classpathPlugins, Path configPath) {
-        this(InternalSettingsPreparer.prepareEnvironment(settings, null, Collections.emptyMap(), configPath), classpathPlugins);
+        this(InternalSettingsPreparer.prepareEnvironment(settings, Collections.emptyMap(), configPath), classpathPlugins);
     }
 
     public MockNode(Environment environment, Collection<Class<? extends Plugin>> classpathPlugins) {

test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java

@@ -1026,7 +1026,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
 
         ServiceHolder(Settings nodeSettings, Settings indexSettings,
                       Collection<Class<? extends Plugin>> plugins, AbstractQueryTestCase<?> testCase) throws IOException {
-            Environment env = InternalSettingsPreparer.prepareEnvironment(nodeSettings, null);
+            Environment env = InternalSettingsPreparer.prepareEnvironment(nodeSettings);
             PluginsService pluginsService;
             pluginsService = new PluginsService(nodeSettings, null, env.modulesFile(), env.pluginsFile(), plugins);