Skip to content

Commit fab3106

Browse files
jaymodejaymode
committed
remove prompt call from settings and add at end of settings preparer
1 parent 6c7d52a commit fab3106

File tree

5 files changed

+27
-65
lines changed

5 files changed

+27
-65
lines changed

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.elasticsearch.common.logging.ESLogger;
3333
import org.elasticsearch.common.logging.Loggers;
3434
import org.elasticsearch.common.logging.log4j.LogConfigurator;
35+
import org.elasticsearch.common.settings.ImmutableSettings;
3536
import org.elasticsearch.common.settings.Settings;
3637
import org.elasticsearch.env.Environment;
3738
import org.elasticsearch.monitor.jvm.JvmInfo;
@@ -64,7 +65,13 @@ private void setup(boolean addShutdownHook, Settings settings, Environment envir
6465
Natives.tryMlockall();
6566
}
6667

67-
NodeBuilder nodeBuilder = NodeBuilder.nodeBuilder().settings(settings).loadConfigSettings(false);
68+
// We do not need to reload system properties here as we have already applied them in building the settings
69+
Settings nodeSettings = ImmutableSettings.settingsBuilder()
70+
.put(settings)
71+
.put("config.ignore_system_properties", true)
72+
.build();
73+
74+
NodeBuilder nodeBuilder = NodeBuilder.nodeBuilder().settings(nodeSettings).loadConfigSettings(false);
6875
node = nodeBuilder.build();
6976
if (addShutdownHook) {
7077
Runtime.getRuntime().addShutdownHook(new Thread() {

‎src/main/java/org/elasticsearch/common/property/PromptPlaceholderResolver.java‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,53 +19,46 @@
1919

2020
package org.elasticsearch.common.property;
2121

22+
import com.google.common.collect.UnmodifiableIterator;
2223
import org.elasticsearch.common.Strings;
2324
import org.elasticsearch.common.cli.Terminal;
2425
import org.elasticsearch.common.settings.ImmutableSettings;
26+
import org.elasticsearch.common.settings.Settings;
2527

26-
import java.util.HashMap;
27-
import java.util.Iterator;
2828
import java.util.Map;
2929

3030
/**
3131
* Utility class for replacing prompt placeholders <code>__prompt__</code> in settings by prompting a user for the actual
32-
* setting value. This class stores these values as they may need to be resolved again in the future, but we should only
33-
* prompt the user once for these values.
32+
* setting value.
3433
*/
3534
public final class PromptPlaceholderResolver {
3635

3736
public static final String PROMPT_VALUE = "__prompt__";
3837

39-
private static final Map<String, String> RESOLVED_PROPERTY_MAP = new HashMap<>();
40-
4138
private PromptPlaceholderResolver() {}
4239

43-
public synchronized static void replacePromptPlaceholders(ImmutableSettings.Builder builder, Terminal terminal) {
44-
Iterator<Map.Entry<String, String>> iter = builder.internalMap().entrySet().iterator();
40+
public static Settings replacePromptPlaceholders(Settings settings, Terminal terminal) {
41+
UnmodifiableIterator<Map.Entry<String, String>> iter = settings.getAsMap().entrySet().iterator();
42+
ImmutableSettings.Builder builder = ImmutableSettings.builder();
43+
4544
while (iter.hasNext()) {
4645
Map.Entry<String, String> entry = iter.next();
4746
String value = entry.getValue();
47+
String key = entry.getKey();
4848
if (PROMPT_VALUE.equals(value)) {
49-
String key = entry.getKey();
50-
51-
// Settings initialization can sometimes replace a resolved property with an unresolved property.
52-
// we store the user supplied values so that the user is not prompted multiple times.
53-
String actualValue;
54-
if (RESOLVED_PROPERTY_MAP.containsKey(key)) {
55-
actualValue = RESOLVED_PROPERTY_MAP.get(key);
56-
} else if (terminal == null) {
49+
if (terminal == null) {
5750
throw new UnsupportedOperationException("found property [" + key + "] with value [" + PROMPT_VALUE +"]. prompting for property values is only supported when running elasticsearch in the foreground");
58-
} else {
59-
actualValue = new String(terminal.readSecret("Enter value for [%s] :", key));
60-
RESOLVED_PROPERTY_MAP.put(key, actualValue);
6151
}
52+
String actualValue = new String(terminal.readSecret("Enter value for [%s] :", key));
6253

6354
if (Strings.hasLength(actualValue)) {
64-
entry.setValue(actualValue);
65-
} else {
66-
iter.remove();
55+
builder.put(key, actualValue);
6756
}
57+
} else {
58+
builder.put(key, value);
6859
}
6960
}
61+
62+
return builder.build();
7063
}
7164
}

‎src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@
2929
import org.elasticsearch.common.Booleans;
3030
import org.elasticsearch.common.Classes;
3131
import org.elasticsearch.common.Strings;
32-
import org.elasticsearch.common.cli.Terminal;
3332
import org.elasticsearch.common.io.Streams;
3433
import org.elasticsearch.common.io.stream.StreamInput;
3534
import org.elasticsearch.common.io.stream.StreamOutput;
36-
import org.elasticsearch.common.property.PromptPlaceholderResolver;
3735
import org.elasticsearch.common.property.PropertyPlaceholder;
3836
import org.elasticsearch.common.settings.loader.SettingsLoader;
3937
import org.elasticsearch.common.settings.loader.SettingsLoaderFactory;
@@ -1101,17 +1099,6 @@ public boolean shouldIgnoreMissing(String placeholderName) {
11011099
return this;
11021100
}
11031101

1104-
/**
1105-
* Runs through all settings in this builder and prompts the user to provide a value for each <code>__prompt__</code>
1106-
* value that is found in the settings.
1107-
* @param terminal The terminal to user to for input/output
1108-
* @return The builder
1109-
*/
1110-
public Builder replacePromptPlaceholders(Terminal terminal) {
1111-
PromptPlaceholderResolver.replacePromptPlaceholders(this, terminal);
1112-
return this;
1113-
}
1114-
11151102
/**
11161103
* Checks that all settings in the builder start with the specified prefix.
11171104
*

‎src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.Strings;
2525
import org.elasticsearch.common.cli.Terminal;
2626
import org.elasticsearch.common.collect.Tuple;
27+
import org.elasticsearch.common.property.PromptPlaceholderResolver;
2728
import org.elasticsearch.common.settings.ImmutableSettings;
2829
import org.elasticsearch.common.settings.Settings;
2930
import org.elasticsearch.env.Environment;
@@ -73,7 +74,6 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
7374
.putProperties("es.", System.getProperties(), ignorePrefixes);
7475
}
7576
settingsBuilder.replacePropertyPlaceholders();
76-
settingsBuilder.replacePromptPlaceholders(terminal);
7777

7878
Environment environment = new Environment(settingsBuilder.build());
7979

@@ -122,7 +122,6 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
122122
.putProperties("es.", System.getProperties(), ignorePrefixes);
123123
}
124124
settingsBuilder.replacePropertyPlaceholders();
125-
settingsBuilder.replacePromptPlaceholders(terminal);
126125

127126
// allow to force set properties based on configuration of the settings provided
128127
for (Map.Entry<String, String> entry : pSettings.getAsMap().entrySet()) {
@@ -133,7 +132,6 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
133132
}
134133
}
135134
settingsBuilder.replacePropertyPlaceholders();
136-
settingsBuilder.replacePromptPlaceholders(terminal);
137135

138136
// generate the name
139137
if (settingsBuilder.get("name") == null) {
@@ -155,7 +153,7 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
155153
settingsBuilder.put(ClusterName.SETTING, ClusterName.DEFAULT.value());
156154
}
157155

158-
Settings v1 = settingsBuilder.build();
156+
Settings v1 = PromptPlaceholderResolver.replacePromptPlaceholders(settingsBuilder.build(), terminal);
159157
environment = new Environment(v1);
160158

161159
// put back the env settings

‎src/test/java/org/elasticsearch/common/property/PromptPlaceholderResolverTests.java‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ public char[] readSecret(String message, Object... args) {
5757
.put("dont.replace4", "__prompt_")
5858
.put("dont.replace5", "prompt__")
5959
.put("replace_me", PromptPlaceholderResolver.PROMPT_VALUE);
60-
PromptPlaceholderResolver.replacePromptPlaceholders(builder, terminal);
6160
Settings settings = builder.build();
61+
settings = PromptPlaceholderResolver.replacePromptPlaceholders(settings, terminal);
6262

6363
assertThat(replacedProperties.size(), is(2));
6464
assertThat(settings.get("password.replace"), equalTo("replaced"));
@@ -83,33 +83,10 @@ public void testReplacePromptPlaceholdersWithNullTerminal() {
8383
.put("dont.replace4", "__prompt_")
8484
.put("dont.replace5", "prompt__")
8585
.put("replace_me2", PromptPlaceholderResolver.PROMPT_VALUE);
86-
PromptPlaceholderResolver.replacePromptPlaceholders(builder, null);
86+
PromptPlaceholderResolver.replacePromptPlaceholders(builder.build(), null);
8787
fail("an exception should have been thrown since no terminal was provided!");
8888
} catch (UnsupportedOperationException e) {
8989
assertThat(e.getMessage(), containsString("with value [__prompt__]"));
9090
}
9191
}
92-
93-
@Test
94-
public void testReplacePreviouslyResolvedPropertyWithNullTerminal() {
95-
final Terminal terminal = new CliToolTestCase.MockTerminal() {
96-
@Override
97-
public char[] readSecret(String message, Object... args) {
98-
return "replaced".toCharArray();
99-
}
100-
};
101-
102-
ImmutableSettings.Builder builder = settingsBuilder()
103-
.put("first.replace", PromptPlaceholderResolver.PROMPT_VALUE);
104-
PromptPlaceholderResolver.replacePromptPlaceholders(builder, terminal);
105-
Settings settings = builder.build();
106-
assertThat(settings.get("first.replace"), equalTo("replaced"));
107-
108-
builder = settingsBuilder()
109-
.put("first.replace", PromptPlaceholderResolver.PROMPT_VALUE);
110-
PromptPlaceholderResolver.replacePromptPlaceholders(builder, null);
111-
Settings settings2 = builder.build();
112-
113-
assertThat(settings2.get("first.replace"), equalTo("replaced"));
114-
}
11592
}

0 commit comments

Comments
 (0)