Cleanup command line setting errors (#124963) (#125133)

This commit improves the error cases when command line settings are
found that are duplicates or conflict with special system properties.
This commit is contained in:
Ryan Ernst 2025-03-18 10:57:35 -07:00 committed by GitHub
parent 6924c4e7de
commit d94f06670a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 18 additions and 13 deletions

View file

@ -185,7 +185,7 @@ public class ServerCliTests extends CommandTestCase {
} }
public void testElasticsearchSettingCanNotBeDuplicated() throws Exception { public void testElasticsearchSettingCanNotBeDuplicated() throws Exception {
assertUsage(containsString("setting [foo] already set, saw [bar] and [baz]"), "-E", "foo=bar", "-E", "foo=baz"); assertUsage(containsString("setting [foo] set twice via command line -E"), "-E", "foo=bar", "-E", "foo=baz");
} }
public void testUnknownOption() throws Exception { public void testUnknownOption() throws Exception {

View file

@ -84,13 +84,7 @@ public abstract class EnvironmentAwareCommand extends Command {
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)) { if (settings.containsKey(kvp.key)) {
final String message = String.format( final String message = String.format(Locale.ROOT, "setting [%s] set twice via command line -E", kvp.key);
Locale.ROOT,
"setting [%s] already set, saw [%s] and [%s]",
kvp.key,
settings.get(kvp.key),
kvp.value
);
throw new UserException(ExitCodes.USAGE, message); throw new UserException(ExitCodes.USAGE, message);
} }
settings.put(kvp.key, kvp.value); settings.put(kvp.key, kvp.value);
@ -133,18 +127,17 @@ public abstract class EnvironmentAwareCommand extends Command {
final Map<String, String> settings, final Map<String, String> settings,
final String setting, final String setting,
final String key final String key
) { ) throws UserException {
final String value = sysprops.get(key); final String value = sysprops.get(key);
if (value != null) { if (value != null) {
if (settings.containsKey(setting)) { if (settings.containsKey(setting)) {
final String message = String.format( final String message = String.format(
Locale.ROOT, Locale.ROOT,
"duplicate setting [%s] found via command-line [%s] and system property [%s]", "setting [%s] found via command-line -E and system property [%s]",
setting, setting,
settings.get(setting), key
value
); );
throw new IllegalArgumentException(message); throw new UserException(ExitCodes.USAGE, message);
} else { } else {
settings.put(setting, value); settings.put(setting, value);
} }

View file

@ -16,6 +16,7 @@ import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.CommandTestCase; import org.elasticsearch.cli.CommandTestCase;
import org.elasticsearch.cli.ProcessInfo; import org.elasticsearch.cli.ProcessInfo;
import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment; import org.elasticsearch.env.Environment;
import org.junit.Before; import org.junit.Before;
@ -127,4 +128,15 @@ public class EnvironmentAwareCommandTests extends CommandTestCase {
}; };
execute("-Esimple.setting=original"); execute("-Esimple.setting=original");
} }
public void testDuplicateCommandLineSetting() {
var e = expectThrows(UserException.class, () -> execute("-E", "my.setting=foo", "-E", "my.setting=bar"));
assertThat(e.getMessage(), equalTo("setting [my.setting] set twice via command line -E"));
}
public void testConflictingPathCommandLineSettingWithSysprop() {
sysprops.put("es.path.data", "foo");
var e = expectThrows(UserException.class, () -> execute("-E", "path.data=bar"));
assertThat(e.getMessage(), equalTo("setting [path.data] found via command-line -E and system property [es.path.data]"));
}
} }