honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reject duplicate settings on the command line 

Today Elasticsearch and other CLI tools that rely on environment aware
command leniently accept duplicate settings with the last one
winning. This commit removes this leniency.

Relates #24053
This commit is contained in:
Jason Tedor 2017-04-11 18:30:05 -04:00 committed by GitHub
parent cf6b03c8f4
commit b4c3bb5d21
2 changed files with 25 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
core/src/main/java/org/elasticsearch/cli/EnvironmentAwareCommand.java
View File

@ -45,7 +45,16 @@ public abstract class EnvironmentAwareCommand extends Command {
final Map<String, String> settings = new HashMap<>();
for (final KeyValuePair kvp : settingOption.values(options)) {
if (kvp.value.isEmpty()) {
throw new UserException(ExitCodes.USAGE, "Setting [" + kvp.key + "] must not be empty");
throw new UserException(ExitCodes.USAGE, "setting [" + kvp.key + "] must not be empty");
}
if (settings.containsKey(kvp.key)) {
final String message = String.format(
Locale.ROOT,
"setting [%s] already set, saw [%s] and [%s]",
kvp.key,
settings.get(kvp.key),
kvp.value);
throw new UserException(ExitCodes.USAGE, message);
}
settings.put(kvp.key, kvp.value);
}

26
core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java
View File

@ -79,22 +79,19 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase {
false,
output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
(foreground, pidFile, quiet, esSettings) -> {},
"foo"
);
"foo");
runTest(
ExitCodes.USAGE,
false,
output -> assertThat(output, containsString("Positional arguments not allowed, found [foo, bar]")),
(foreground, pidFile, quiet, esSettings) -> {},
"foo", "bar"
);
"foo", "bar");
runTest(
ExitCodes.USAGE,
false,
output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")),
(foreground, pidFile, quiet, esSettings) -> {},
"-E", "foo=bar", "foo", "-E", "baz=qux"
);
"-E", "foo=bar", "foo", "-E", "baz=qux");
}
public void testThatPidFileCanBeConfigured() throws Exception {
@ -157,18 +154,25 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase {
assertThat(settings, hasEntry("foo", "bar"));
assertThat(settings, hasEntry("baz", "qux"));
},
"-Efoo=bar", "-E", "baz=qux"
);
"-Efoo=bar", "-E", "baz=qux");
}
public void testElasticsearchSettingCanNotBeEmpty() throws Exception {
runTest(
ExitCodes.USAGE,
false,
output -> assertThat(output, containsString("Setting [foo] must not be empty")),
output -> assertThat(output, containsString("setting [foo] must not be empty")),
(foreground, pidFile, quiet, esSettings) -> {},
"-E", "foo="
);
"-E", "foo=");
}
public void testElasticsearchSettingCanNotBeDuplicated() throws Exception {
runTest(
ExitCodes.USAGE,
false,
output -> assertThat(output, containsString("setting [foo] already set, saw [bar] and [baz]")),
(foreground, pidFile, quiet, initialEnv) -> {},
"-E", "foo=bar", "-E", "foo=baz");
}
public void testUnknownOption() throws Exception {