Skip to content

Commit

Permalink
make prompt placeholders consistent with existing placeholders
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jaymode
jaymode committed Jun 5, 2015
1 parent 594ecd8 commit 5fa93ea
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 47 deletions.
10 changes: 5 additions & 5 deletions docs/reference/setup/configuration.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,15 +271,15 @@ file which will resolve to an environment setting, for example:
--------------------------------------------------

Additionally, 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}`
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}
name: ${prompt.text}
--------------------------------------------------

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

NOTE: Elasticsearch will not start if `${prompt::text}` or `${prompt::secret}`
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.

The location of the configuration file can be set externally using a
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/org/elasticsearch/common/property/PropertyPlaceholder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ protected String parseStringValue(String strVal, PlaceholderResolver placeholder
propVal = defaultValue;
}
if (propVal == null && placeholderResolver.shouldIgnoreMissing(placeholder)) {
propVal = "";
if (placeholderResolver.shouldRemoveMissingPlaceholder(placeholder)) {
propVal = "";
} else {
return strVal;
}
}
if (propVal != null) {
// Recursive invocation, parsing placeholders contained in the
Expand Down Expand Up @@ -170,5 +174,13 @@ public static interface PlaceholderResolver {
String resolvePlaceholder(String placeholderName);

boolean shouldIgnoreMissing(String placeholderName);

/**
* Allows for special handling for ignored missing placeholders that may be resolved elsewhere
*
* @param placeholderName the name of the placeholder to resolve.
* @return true if the placeholder should be replaced with a empty string
*/
boolean shouldRemoveMissingPlaceholder(String placeholderName);
}
}
26 changes: 11 additions & 15 deletions src/main/java/org/elasticsearch/common/settings/ImmutableSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ public Builder putProperties(String prefix, Properties properties, String[] igno
* tries and resolve it against an environment variable ({@link System#getenv(String)}), and last, tries
* and replace it with another setting already set on this builder.
*/
public Builder replacePropertyPlaceholders(String... ignoredValues) {
public Builder replacePropertyPlaceholders() {
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new PropertyPlaceholder.PlaceholderResolver() {
@Override
Expand All @@ -1088,26 +1088,22 @@ public String resolvePlaceholder(String placeholderName) {
@Override
public boolean shouldIgnoreMissing(String placeholderName) {
// if its an explicit env var, we are ok with not having a value for it and treat it as optional
if (placeholderName.startsWith("env.")) {
if (placeholderName.startsWith("env.") || placeholderName.startsWith("prompt.")) {
return true;
}
return false;
}
};
for (Map.Entry<String, String> entry : Maps.newHashMap(map).entrySet()) {
String possiblePlaceholder = entry.getValue();
boolean ignored = false;
for (String ignoredValue : ignoredValues) {
if (ignoredValue.equals(possiblePlaceholder)) {
ignored = true;
break;

@Override
public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
if (placeholderName.startsWith("prompt.")) {
return false;
}
return true;
}
if (ignored) {
continue;
}

String value = propertyPlaceholder.replacePlaceholders(possiblePlaceholder, placeholderResolver);
};
for (Map.Entry<String, String> entry : Maps.newHashMap(map).entrySet()) {
String value = propertyPlaceholder.replacePlaceholders(entry.getValue(), placeholderResolver);
// if the values exists and has length, we should maintain it in the map
// otherwise, the replace process resolved into removing it
if (Strings.hasLength(value)) {
Expand Down
13 changes: 5 additions & 8 deletions src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public class InternalSettingsPreparer {

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

public static final String SECRET_PROMPT_VALUE = "${prompt::secret}";
public static final String TEXT_PROMPT_VALUE = "${prompt::text}";
public static final String SECRET_PROMPT_VALUE = "${prompt.secret}";
public static final String TEXT_PROMPT_VALUE = "${prompt.text}";
public static final String IGNORE_SYSTEM_PROPERTIES_SETTING = "config.ignore_system_properties";

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

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

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

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

// generate the name
if (settingsBuilder.get("name") == null) {
Expand Down
39 changes: 27 additions & 12 deletions src/test/java/org/elasticsearch/common/property/PropertyPlaceholderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void testSimple() {
Map<String, String> map = new LinkedHashMap<>();
map.put("foo1", "bar1");
map.put("foo2", "bar2");
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("bar1", propertyPlaceholder.replacePlaceholders("{foo1}", placeholderResolver));
assertEquals("a bar1b", propertyPlaceholder.replacePlaceholders("a {foo1}b", placeholderResolver));
assertEquals("bar1bar2", propertyPlaceholder.replacePlaceholders("{foo1}{foo2}", placeholderResolver));
Expand All @@ -48,7 +48,7 @@ public void testVariousPrefixSuffix() {
PropertyPlaceholder ppShorterPrefix = new PropertyPlaceholder("{", "}}", false);
Map<String, String> map = new LinkedHashMap<>();
map.put("foo", "bar");
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("bar", ppEqualsPrefix.replacePlaceholders("{foo}", placeholderResolver));
assertEquals("bar", ppLongerPrefix.replacePlaceholders("${foo}", placeholderResolver));
assertEquals("bar", ppShorterPrefix.replacePlaceholders("{foo}}", placeholderResolver));
Expand All @@ -58,7 +58,7 @@ public void testVariousPrefixSuffix() {
public void testDefaultValue() {
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
Map<String, String> map = new LinkedHashMap<>();
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("bar", propertyPlaceholder.replacePlaceholders("${foo:bar}", placeholderResolver));
assertEquals("", propertyPlaceholder.replacePlaceholders("${foo:}", placeholderResolver));
}
Expand All @@ -67,23 +67,23 @@ public void testDefaultValue() {
public void testIgnoredUnresolvedPlaceholder() {
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", true);
Map<String, String> map = new LinkedHashMap<>();
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("${foo}", propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver));
}

@Test(expected = IllegalArgumentException.class)
public void testNotIgnoredUnresolvedPlaceholder() {
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
Map<String, String> map = new LinkedHashMap<>();
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver);
}

@Test
public void testShouldIgnoreMissing() {
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
Map<String, String> map = new LinkedHashMap<>();
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true, true);
assertEquals("bar", propertyPlaceholder.replacePlaceholders("bar${foo}", placeholderResolver));
}

Expand All @@ -94,7 +94,7 @@ public void testRecursive() {
map.put("foo", "${foo1}");
map.put("foo1", "${foo2}");
map.put("foo2", "bar");
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("bar", propertyPlaceholder.replacePlaceholders("${foo}", placeholderResolver));
assertEquals("abarb", propertyPlaceholder.replacePlaceholders("a${foo}b", placeholderResolver));
}
Expand All @@ -107,7 +107,7 @@ public void testNestedLongerPrefix() {
map.put("foo1", "${foo2}");
map.put("foo2", "bar");
map.put("barbar", "baz");
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("baz", propertyPlaceholder.replacePlaceholders("${bar${foo}}", placeholderResolver));
}

Expand All @@ -119,7 +119,7 @@ public void testNestedSameLengthPrefixSuffix() {
map.put("foo1", "{foo2}");
map.put("foo2", "bar");
map.put("barbar", "baz");
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("baz", propertyPlaceholder.replacePlaceholders("{bar{foo}}", placeholderResolver));
}

Expand All @@ -131,7 +131,7 @@ public void testNestedShorterPrefix() {
map.put("foo1", "{foo2}}");
map.put("foo2", "bar");
map.put("barbar", "baz");
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false);
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, false, true);
assertEquals("baz", propertyPlaceholder.replacePlaceholders("{bar{foo}}}}", placeholderResolver));
}

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

@Test
public void testShouldRemoveMissing() {
PropertyPlaceholder propertyPlaceholder = new PropertyPlaceholder("${", "}", false);
Map<String, String> map = new LinkedHashMap<>();
PropertyPlaceholder.PlaceholderResolver placeholderResolver = new SimplePlaceholderResolver(map, true, false);
assertEquals("bar${foo}", propertyPlaceholder.replacePlaceholders("bar${foo}", placeholderResolver));
}

private class SimplePlaceholderResolver implements PropertyPlaceholder.PlaceholderResolver {
private Map<String, String> map;
private boolean shouldIgnoreMissing;
private boolean shouldRemoveMissing;

SimplePlaceholderResolver(Map<String, String> map, boolean shouldIgnoreMissing) {
SimplePlaceholderResolver(Map<String, String> map, boolean shouldIgnoreMissing, boolean shouldRemoveMissing) {
this.map = map;
this.shouldIgnoreMissing = shouldIgnoreMissing;
this.shouldRemoveMissing = shouldRemoveMissing;
}

@Override
Expand All @@ -163,5 +173,10 @@ public String resolvePlaceholder(String placeholderName) {
public boolean shouldIgnoreMissing(String placeholderName) {
return shouldIgnoreMissing;
}

@Override
public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
return shouldRemoveMissing;
}
}
}
12 changes: 6 additions & 6 deletions src/test/java/org/elasticsearch/common/settings/ImmutableSettingsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,14 @@ public void testReplacePropertiesPlaceholderIgnoreEnvUnset() {
}

@Test
public void testReplacePropertiesPlaceholderIgnores() {
public void testReplacePropertiesPlaceholderIgnoresPrompt() {
Settings settings = settingsBuilder()
.put("setting1", "${foo.bar}")
.put("setting2", "${foo.bar1}")
.replacePropertyPlaceholders("${foo.bar}", "${foo.bar1}")
.put("setting1", "${prompt.text}")
.put("setting2", "${prompt.secret}")
.replacePropertyPlaceholders()
.build();
assertThat(settings.get("setting1"), is("${foo.bar}"));
assertThat(settings.get("setting2"), is("${foo.bar1}"));
assertThat(settings.get("setting1"), is("${prompt.text}"));
assertThat(settings.get("setting2"), is("${prompt.secret}"));
}

@Test
Expand Down

0 comments on commit 5fa93ea

Please sign in to comment.