Skip to content

Commit

Permalink
Fix environment variable substitutions in list setting (#28106)
Browse files Browse the repository at this point in the history
Since Elasticsearch 6.1.0 environment variable substitutions in lists do not work.
Environment variables in a list setting were not resolved, because settings with a list type were skipped during variables resolution.
This commit fixes by processing list settings as well.

Closes #27926
  • Loading branch information
mayya-sharipova authored Jan 10, 2018
1 parent 4182e9e commit caa63ad
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
17 changes: 14 additions & 3 deletions core/src/main/java/org/elasticsearch/common/settings/Settings.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.TreeMap;
import java.util.ListIterator;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.Predicate;
Expand Down Expand Up @@ -414,7 +415,7 @@ public List<String> getAsList(String key, List<String> defaultValue, Boolean com
final Object valueFromPrefix = settings.get(key);
if (valueFromPrefix != null) {
if (valueFromPrefix instanceof List) {
return ((List<String>) valueFromPrefix); // it's already unmodifiable since the builder puts it as a such
return Collections.unmodifiableList((List<String>) valueFromPrefix);
} else if (commaDelimited) {
String[] strings = Strings.splitStringByCommaToArray(get(key));
if (strings.length > 0) {
Expand Down Expand Up @@ -1042,7 +1043,7 @@ public Builder putList(String setting, String... values) {
*/
public Builder putList(String setting, List<String> values) {
remove(setting);
map.put(setting, Collections.unmodifiableList(new ArrayList<>(values)));
map.put(setting, new ArrayList<>(values));
return this;
}

Expand Down Expand Up @@ -1210,10 +1211,20 @@ public boolean shouldRemoveMissingPlaceholder(String placeholderName) {
Iterator<Map.Entry<String, Object>> entryItr = map.entrySet().iterator();
while (entryItr.hasNext()) {
Map.Entry<String, Object> entry = entryItr.next();
if (entry.getValue() == null || entry.getValue() instanceof List) {
if (entry.getValue() == null) {
// a null value obviously can't be replaced
continue;
}
if (entry.getValue() instanceof List) {
final ListIterator<String> li = ((List<String>) entry.getValue()).listIterator();
while (li.hasNext()) {
final String settingValueRaw = li.next();
final String settingValueResolved = propertyPlaceholder.replacePlaceholders(settingValueRaw, placeholderResolver);
li.set(settingValueResolved);
}
continue;
}

String value = propertyPlaceholder.replacePlaceholders(Settings.toString(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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ public void testReplacePropertiesPlaceholderSystemProperty() {
assertThat(settings.get("setting1"), equalTo(value));
}

public void testReplacePropertiesPlaceholderSystemPropertyList() {
final String hostname = randomAlphaOfLength(16);
final String hostip = randomAlphaOfLength(16);
final Settings settings = Settings.builder()
.putList("setting1", "${HOSTNAME}", "${HOSTIP}")
.replacePropertyPlaceholders(name -> name.equals("HOSTNAME") ? hostname : name.equals("HOSTIP") ? hostip : null)
.build();
assertThat(settings.getAsList("setting1"), contains(hostname, hostip));
}

public void testReplacePropertiesPlaceholderSystemVariablesHaveNoEffect() {
final String value = System.getProperty("java.home");
assertNotNull(value);
Expand Down

0 comments on commit caa63ad

Please sign in to comment.