Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove config prompting for secrets and text #27216

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
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;
Expand Down Expand Up @@ -245,7 +244,6 @@ private static Environment createEnvironment(
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);
Expand All @@ -254,7 +252,7 @@ private static Environment createEnvironment(
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ protected void execute(Terminal terminal, OptionSet options) throws Exception {
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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Map;
import java.util.function.Function;

import org.elasticsearch.Version;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.Strings;
Expand All @@ -40,45 +41,38 @@ 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.
*/
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);
Expand All @@ -104,7 +98,8 @@ public static Environment prepareEnvironment(Settings input, Terminal terminal,

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

Expand All @@ -128,10 +123,30 @@ static void initializeSettings(final Settings.Builder output, final Settings inp
}

/**
* 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 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
if (Version.CURRENT.toString().startsWith("8")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about Version.CURRENT.major == 8? And I think maybe we should only make this an assertion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, i wasnt aware exactly how to do it

throw new SettingsException("Logic pertaining to config driven prompting should be removed in 8.0.0.");
}
for (String setting : output.keys()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add an assertion here the major property of the current version is not 8; then, when we bump the version to 8.0.0 on master we know that we can remove this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dude, nice. Thats smart. Will do.

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, Terminal terminal) {
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()) {
Expand All @@ -149,53 +164,5 @@ private static void finalizeSettings(Settings.Builder output, Terminal terminal)
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 + "]: ");
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public static final Settings addNodeNameIfNeeded(Settings settings, final String
* @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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,6 @@ public void testReplacePropertiesPlaceholderByEnvironmentVariables() {
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")
Expand Down
Loading