Skip to content

Commit 5fa93ea

Browse files
jaymodejaymode
committed
make prompt placeholders consistent with existing placeholders
In elastic#10918, we introduced the prompt placeholders. These were had a different format than our existing placeholders. This changes the prompt placeholders to follow the format of the existing placeholders. Relates to elastic#11455
1 parent 594ecd8 commit 5fa93ea

File tree

6 files changed

+67
-47
lines changed

6 files changed

+67
-47
lines changed

‎docs/reference/setup/configuration.asciidoc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,15 +271,15 @@ file which will resolve to an environment setting, for example:
271271
--------------------------------------------------
272272

273273
Additionally, for settings that you do not wish to store in the configuration
274-
file, you can use the value `${prompt::text}` or `${prompt::secret}` and start
275-
Elasticsearch in the foreground. `${prompt::secret}` has echoing disabled so
276-
that the value entered will not be shown in your terminal; `${prompt::text}`
274+
file, you can use the value `${prompt.text}` or `${prompt.secret}` and start
275+
Elasticsearch in the foreground. `${prompt.secret}` has echoing disabled so
276+
that the value entered will not be shown in your terminal; `${prompt.text}`
277277
will allow you to see the value as you type it in. For example:
278278

279279
[source,yaml]
280280
--------------------------------------------------
281281
node:
282-
name: ${prompt::text}
282+
name: ${prompt.text}
283283
--------------------------------------------------
284284

285285
On execution of the `elasticsearch` command, you will be prompted to enter
@@ -290,7 +290,7 @@ the actual value like so:
290290
Enter value for [node.name]:
291291
--------------------------------------------------
292292

293-
NOTE: Elasticsearch will not start if `${prompt::text}` or `${prompt::secret}`
293+
NOTE: Elasticsearch will not start if `${prompt.text}` or `${prompt.secret}`
294294
is used in the settings and the process is run as a service or in the background.
295295

296296
The location of the configuration file can be set externally using a

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,11 @@ protected String parseStringValue(String strVal, PlaceholderResolver placeholder
109109
propVal = defaultValue;
110110
}
111111
if (propVal == null && placeholderResolver.shouldIgnoreMissing(placeholder)) {
112-
propVal = "";
112+
if (placeholderResolver.shouldRemoveMissingPlaceholder(placeholder)) {
113+
propVal = "";
114+
} else {
115+
return strVal;
116+
}
113117
}
114118
if (propVal != null) {
115119
// Recursive invocation, parsing placeholders contained in the
@@ -170,5 +174,13 @@ public static interface PlaceholderResolver {
170174
String resolvePlaceholder(String placeholderName);
171175

172176
boolean shouldIgnoreMissing(String placeholderName);
177+
178+
/**
179+
* Allows for special handling for ignored missing placeholders that may be resolved elsewhere
180+
*
181+
* @param placeholderName the name of the placeholder to resolve.
182+
* @return true if the placeholder should be replaced with a empty string
183+
*/
184+
boolean shouldRemoveMissingPlaceholder(String placeholderName);
173185
}
174186
}

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

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,7 @@ public Builder putProperties(String prefix, Properties properties, String[] igno
10651065
* tries and resolve it against an environment variable ({@link System#getenv(String)}), and last, tries
10661066
* and replace it with another setting already set on this builder.
10671067
*/
1068-
public Builder replacePropertyPlaceholders(String... ignoredValues) {
1068+
public Builder replacePropertyPlaceholders() {
10691069
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
10701070
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new PropertyPlaceholder.PlaceholderResolver() {
10711071
@Override
@@ -1088,26 +1088,22 @@ public String resolvePlaceholder(String placeholderName) {
10881088
@Override
10891089
public boolean shouldIgnoreMissing(String placeholderName) {
10901090
// if its an explicit env var, we are ok with not having a value for it and treat it as optional
1091-
if (placeholderName.startsWith("env.")) {
1091+
if (placeholderName.startsWith("env.") || placeholderName.startsWith("prompt.")) {
10921092
return true;
10931093
}
10941094
return false;
10951095
}
1096-
};
1097-
for (Map.Entry<String, String> entry : Maps.newHashMap(map).entrySet()) {
1098-
String possiblePlaceholder = entry.getValue();
1099-
boolean ignored = false;
1100-
for (String ignoredValue : ignoredValues) {
1101-
if (ignoredValue.equals(possiblePlaceholder)) {
1102-
ignored = true;
1103-
break;
1096+
1097+
@Override
1098+
public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
1099+
if (placeholderName.startsWith("prompt.")) {
1100+
return false;
11041101
}
1102+
return true;
11051103
}
1106-
if (ignored) {
1107-
continue;
1108-
}
1109-
1110-
String value = propertyPlaceholder.replacePlaceholders(possiblePlaceholder, placeholderResolver);
1104+
};
1105+
for (Map.Entry<String, String> entry : Maps.newHashMap(map).entrySet()) {
1106+
String value = propertyPlaceholder.replacePlaceholders(entry.getValue(), placeholderResolver);
11111107
// if the values exists and has length, we should maintain it in the map
11121108
// otherwise, the replace process resolved into removing it
11131109
if (Strings.hasLength(value)) {

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ public class InternalSettingsPreparer {
4444

4545
static final List<String> ALLOWED_SUFFIXES = ImmutableList.of(".yml", ".yaml", ".json", ".properties");
4646

47-
public static final String SECRET_PROMPT_VALUE = "${prompt::secret}";
48-
public static final String TEXT_PROMPT_VALUE = "${prompt::text}";
47+
public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
48+
public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
4949
public static final String IGNORE_SYSTEM_PROPERTIES_SETTING = "config.ignore_system_properties";
5050

5151
/**
@@ -72,9 +72,6 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
7272
public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, boolean loadConfigSettings, Terminal terminal) {
7373
// ignore this prefixes when getting properties from es. and elasticsearch.
7474
String[] ignorePrefixes = new String[]{"es.default.", "elasticsearch.default."};
75-
// ignore the special prompt placeholders since they have the same format as property placeholders and will be resolved
76-
// as having a default value because of the ':' in the format
77-
String[] ignoredPlaceholders = new String[] { SECRET_PROMPT_VALUE, TEXT_PROMPT_VALUE };
7875
boolean useSystemProperties = !pSettings.getAsBoolean(IGNORE_SYSTEM_PROPERTIES_SETTING, false);
7976
// just create enough settings to build the environment
8077
ImmutableSettings.Builder settingsBuilder = settingsBuilder().put(pSettings);
@@ -84,7 +81,7 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
8481
.putProperties("elasticsearch.", System.getProperties(), ignorePrefixes)
8582
.putProperties("es.", System.getProperties(), ignorePrefixes);
8683
}
87-
settingsBuilder.replacePropertyPlaceholders(ignoredPlaceholders);
84+
settingsBuilder.replacePropertyPlaceholders();
8885

8986
Environment environment = new Environment(settingsBuilder.build());
9087

@@ -122,7 +119,7 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
122119
settingsBuilder.putProperties("elasticsearch.", System.getProperties(), ignorePrefixes)
123120
.putProperties("es.", System.getProperties(), ignorePrefixes);
124121
}
125-
settingsBuilder.replacePropertyPlaceholders(ignoredPlaceholders);
122+
settingsBuilder.replacePropertyPlaceholders();
126123

127124
// allow to force set properties based on configuration of the settings provided
128125
for (Map.Entry<String, String> entry : pSettings.getAsMap().entrySet()) {
@@ -132,7 +129,7 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
132129
settingsBuilder.put(setting.substring("force.".length()), entry.getValue());
133130
}
134131
}
135-
settingsBuilder.replacePropertyPlaceholders(ignoredPlaceholders);
132+
settingsBuilder.replacePropertyPlaceholders();
136133

137134
// generate the name
138135
if (settingsBuilder.get("name") == null) {

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

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public void testSimple() {
3333
Map<String, String> map = new LinkedHashMap<>();
3434
map.put("foo1", "bar1");
3535
map.put("foo2", "bar2");
36-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
36+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
3737
assertEquals("bar1", propertyPlaceholder.replacePlaceholders("{foo1}", placeholderResolver));
3838
assertEquals("a bar1b", propertyPlaceholder.replacePlaceholders("a {foo1}b", placeholderResolver));
3939
assertEquals("bar1bar2", propertyPlaceholder.replacePlaceholders("{foo1}{foo2}", placeholderResolver));
@@ -48,7 +48,7 @@ public void testVariousPrefixSuffix() {
4848
PropertyPlaceholder ppShorterPrefix = new PropertyPlaceholder("{", "}}", false);
4949
Map<String, String> map = new LinkedHashMap<>();
5050
map.put("foo", "bar");
51-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
51+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
5252
assertEquals("bar", ppEqualsPrefix.replacePlaceholders("{foo}", placeholderResolver));
5353
assertEquals("bar", ppLongerPrefix.replacePlaceholders("${foo}", placeholderResolver));
5454
assertEquals("bar", ppShorterPrefix.replacePlaceholders("{foo}}", placeholderResolver));
@@ -58,7 +58,7 @@ public void testVariousPrefixSuffix() {
5858
public void testDefaultValue() {
5959
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
6060
Map<String, String> map = new LinkedHashMap<>();
61-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
61+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
6262
assertEquals("bar", propertyPlaceholder.replacePlaceholders("${foo:bar}", placeholderResolver));
6363
assertEquals("", propertyPlaceholder.replacePlaceholders("${foo:}", placeholderResolver));
6464
}
@@ -67,23 +67,23 @@ public void testDefaultValue() {
6767
public void testIgnoredUnresolvedPlaceholder() {
6868
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", true);
6969
Map<String, String> map = new LinkedHashMap<>();
70-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
70+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
7171
assertEquals("${foo}", propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver));
7272
}
7373

7474
@Test(expected = IllegalArgumentException.class)
7575
public void testNotIgnoredUnresolvedPlaceholder() {
7676
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
7777
Map<String, String> map = new LinkedHashMap<>();
78-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
78+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
7979
propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver);
8080
}
8181

8282
@Test
8383
public void testShouldIgnoreMissing() {
8484
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
8585
Map<String, String> map = new LinkedHashMap<>();
86-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true);
86+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true, true);
8787
assertEquals("bar", propertyPlaceholder.replacePlaceholders("bar${foo}", placeholderResolver));
8888
}
8989

@@ -94,7 +94,7 @@ public void testRecursive() {
9494
map.put("foo", "${foo1}");
9595
map.put("foo1", "${foo2}");
9696
map.put("foo2", "bar");
97-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
97+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
9898
assertEquals("bar", propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver));
9999
assertEquals("abarb", propertyPlaceholder.replacePlaceholders("a${foo}b", placeholderResolver));
100100
}
@@ -107,7 +107,7 @@ public void testNestedLongerPrefix() {
107107
map.put("foo1", "${foo2}");
108108
map.put("foo2", "bar");
109109
map.put("barbar", "baz");
110-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
110+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
111111
assertEquals("baz", propertyPlaceholder.replacePlaceholders("${bar${foo}}", placeholderResolver));
112112
}
113113

@@ -119,7 +119,7 @@ public void testNestedSameLengthPrefixSuffix() {
119119
map.put("foo1", "{foo2}");
120120
map.put("foo2", "bar");
121121
map.put("barbar", "baz");
122-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
122+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
123123
assertEquals("baz", propertyPlaceholder.replacePlaceholders("{bar{foo}}", placeholderResolver));
124124
}
125125

@@ -131,7 +131,7 @@ public void testNestedShorterPrefix() {
131131
map.put("foo1", "{foo2}}");
132132
map.put("foo2", "bar");
133133
map.put("barbar", "baz");
134-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
134+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
135135
assertEquals("baz", propertyPlaceholder.replacePlaceholders("{bar{foo}}}}", placeholderResolver));
136136
}
137137

@@ -141,17 +141,27 @@ public void testCircularReference() {
141141
Map<String, String> map = new LinkedHashMap<>();
142142
map.put("foo", "${bar}");
143143
map.put("bar", "${foo}");
144-
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
144+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
145145
propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver);
146146
}
147147

148+
@Test
149+
public void testShouldRemoveMissing() {
150+
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
151+
Map<String, String> map = new LinkedHashMap<>();
152+
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true, false);
153+
assertEquals("bar${foo}", propertyPlaceholder.replacePlaceholders("bar${foo}", placeholderResolver));
154+
}
155+
148156
private class SimplePlaceholderResolver implements PropertyPlaceholder.PlaceholderResolver {
149157
private Map<String, String> map;
150158
private boolean shouldIgnoreMissing;
159+
private boolean shouldRemoveMissing;
151160

152-
SimplePlaceholderResolver(Map<String, String> map, boolean shouldIgnoreMissing) {
161+
SimplePlaceholderResolver(Map<String, String> map, boolean shouldIgnoreMissing, boolean shouldRemoveMissing) {
153162
this.map = map;
154163
this.shouldIgnoreMissing = shouldIgnoreMissing;
164+
this.shouldRemoveMissing = shouldRemoveMissing;
155165
}
156166

157167
@Override
@@ -163,5 +173,10 @@ public String resolvePlaceholder(String placeholderName) {
163173
public boolean shouldIgnoreMissing(String placeholderName) {
164174
return shouldIgnoreMissing;
165175
}
176+
177+
@Override
178+
public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
179+
return shouldRemoveMissing;
180+
}
166181
}
167182
}

‎src/test/java/org/elasticsearch/common/settings/ImmutableSettingsTests.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,14 @@ public void testReplacePropertiesPlaceholderIgnoreEnvUnset() {
138138
}
139139

140140
@Test
141-
public void testReplacePropertiesPlaceholderIgnores() {
141+
public void testReplacePropertiesPlaceholderIgnoresPrompt() {
142142
Settings settings = settingsBuilder()
143-
.put("setting1", "${foo.bar}")
144-
.put("setting2", "${foo.bar1}")
145-
.replacePropertyPlaceholders("${foo.bar}", "${foo.bar1}")
143+
.put("setting1", "${prompt.text}")
144+
.put("setting2", "${prompt.secret}")
145+
.replacePropertyPlaceholders()
146146
.build();
147-
assertThat(settings.get("setting1"), is("${foo.bar}"));
148-
assertThat(settings.get("setting2"), is("${foo.bar1}"));
147+
assertThat(settings.get("setting1"), is("${prompt.text}"));
148+
assertThat(settings.get("setting2"), is("${prompt.secret}"));
149149
}
150150

151151
@Test

0 commit comments

Comments
 (0)