Skip to content

Commit 1196173

Browse files
authored
Cleanup command line setting errors (elastic#124963)
This commit improves the error cases when command line settings are found that are duplicates or conflict with special system properties.
1 parent 50a7eb0 commit 1196173

File tree

3 files changed

+18
-13
lines changed

3 files changed

+18
-13
lines changed

distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public void testElasticsearchSettingCanNotBeEmpty() throws Exception {
185185
}
186186

187187
public void testElasticsearchSettingCanNotBeDuplicated() throws Exception {
188-
assertUsage(containsString("setting [foo] already set, saw [bar] and [baz]"), "-E", "foo=bar", "-E", "foo=baz");
188+
assertUsage(containsString("setting [foo] set twice via command line -E"), "-E", "foo=bar", "-E", "foo=baz");
189189
}
190190

191191
public void testUnknownOption() throws Exception {

server/src/main/java/org/elasticsearch/common/cli/EnvironmentAwareCommand.java

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,7 @@ protected Environment createEnv(OptionSet options, ProcessInfo processInfo) thro
8484
throw new UserException(ExitCodes.USAGE, "setting [" + kvp.key + "] must not be empty");
8585
}
8686
if (settings.containsKey(kvp.key)) {
87-
final String message = String.format(
88-
Locale.ROOT,
89-
"setting [%s] already set, saw [%s] and [%s]",
90-
kvp.key,
91-
settings.get(kvp.key),
92-
kvp.value
93-
);
87+
final String message = String.format(Locale.ROOT, "setting [%s] set twice via command line -E", kvp.key);
9488
throw new UserException(ExitCodes.USAGE, message);
9589
}
9690
settings.put(kvp.key, kvp.value);
@@ -133,18 +127,17 @@ private static void putSystemPropertyIfSettingIsMissing(
133127
final Map<String, String> settings,
134128
final String setting,
135129
final String key
136-
) {
130+
) throws UserException {
137131
final String value = sysprops.get(key);
138132
if (value != null) {
139133
if (settings.containsKey(setting)) {
140134
final String message = String.format(
141135
Locale.ROOT,
142-
"duplicate setting [%s] found via command-line [%s] and system property [%s]",
136+
"setting [%s] found via command-line -E and system property [%s]",
143137
setting,
144-
settings.get(setting),
145-
value
138+
key
146139
);
147-
throw new IllegalArgumentException(message);
140+
throw new UserException(ExitCodes.USAGE, message);
148141
} else {
149142
settings.put(setting, value);
150143
}

server/src/test/java/org/elasticsearch/common/cli/EnvironmentAwareCommandTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.cli.CommandTestCase;
1717
import org.elasticsearch.cli.ProcessInfo;
1818
import org.elasticsearch.cli.Terminal;
19+
import org.elasticsearch.cli.UserException;
1920
import org.elasticsearch.common.settings.Settings;
2021
import org.elasticsearch.env.Environment;
2122
import org.junit.Before;
@@ -127,4 +128,15 @@ public void testDockerEnvVarSettingsOverrideCommandLine() throws Exception {
127128
};
128129
execute("-Esimple.setting=original");
129130
}
131+
132+
public void testDuplicateCommandLineSetting() {
133+
var e = expectThrows(UserException.class, () -> execute("-E", "my.setting=foo", "-E", "my.setting=bar"));
134+
assertThat(e.getMessage(), equalTo("setting [my.setting] set twice via command line -E"));
135+
}
136+
137+
public void testConflictingPathCommandLineSettingWithSysprop() {
138+
sysprops.put("es.path.data", "foo");
139+
var e = expectThrows(UserException.class, () -> execute("-E", "path.data=bar"));
140+
assertThat(e.getMessage(), equalTo("setting [path.data] found via command-line -E and system property [es.path.data]"));
141+
}
130142
}

0 commit comments

Comments
 (0)