Skip to content

Commit b56f31d

Browse files
jasontedorjasontedor
committed
Allow keystore add to handle multiple settings (#54229)
Today the keystore add command can only handle adding a single setting/value pair in a single invocation. This incurs the startup costs of the JVM many times, which in some environments can be expensive. This commit teaches the add keystore command to accept adding multiple settings in a single invocation.
1 parent 705899e commit b56f31d

File tree

3 files changed

+110
-52
lines changed

3 files changed

+110
-52
lines changed

‎docs/reference/commands/keystore.asciidoc‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ in the {es} keystore.
1111
[source,shell]
1212
--------------------------------------------------
1313
bin/elasticsearch-keystore
14-
([add <setting>] [-f] [--stdin] |
14+
([add <settings>] [-f] [--stdin] |
1515
[add-file <setting> <path>] | [create] [-p] |
1616
[list] | [passwd] | [remove <setting>] | [upgrade])
1717
[-h, --help] ([-s, --silent] | [-v, --verbose])
@@ -40,12 +40,13 @@ keystore, see the setting reference.
4040
[[elasticsearch-keystore-parameters]]
4141
=== Parameters
4242

43-
`add <setting>`:: Adds settings to the keystore. By default, you are prompted
44-
for the value of the setting. If the keystore is password protected, you are
45-
also prompted to enter the password. If the setting already exists in the
46-
keystore, you must confirm that you want to overwrite the current value. If the
47-
keystore does not exist, you must confirm that you want to create a keystore. To
48-
avoid these two confirmation prompts, use the `-f` parameter.
43+
`add <settings>`:: Adds settings to the keystore. Multiple setting names can be
44+
specified as arguments to the `add` command. By default, you are prompted for
45+
the values of the settings. If the keystore is password protected, you are also
46+
prompted to enter the password. If a setting already exists in the keystore, you
47+
must confirm that you want to overwrite the current value. If the keystore does
48+
not exist, you must confirm that you want to create a keystore. To avoid these
49+
two confirmation prompts, use the `-f` parameter.
4950

5051
`add-file <setting> <path>`:: Adds a file to the keystore.
5152

@@ -70,12 +71,14 @@ protected, you are prompted to enter the current password and the new one. You
7071
can optionally use an empty string to remove the password. If the keystore is
7172
not password protected, you can use this command to set a password.
7273

73-
`remove <setting>`:: Removes a setting from the keystore.
74+
`remove <settings>`:: Removes settings from the keystore. Multiple setting
75+
names can be specified as arguments to the `add` command.
7476

7577
`-s, --silent`:: Shows minimal output.
7678

77-
`--stdin`:: When used with the `add` parameter, you can pass the setting value
78-
through standard input (stdin). See <<add-string-to-keystore>>.
79+
`--stdin`:: When used with the `add` parameter, you can pass the settings values
80+
through standard input (stdin). Separate multiple values with carriage returns
81+
or newlines. See <<add-string-to-keystore>>.
7982

8083
`upgrade`:: Upgrades the internal format of the keystore.
8184

@@ -143,13 +146,28 @@ bin/elasticsearch-keystore add the.setting.name.to.set
143146
You are prompted to enter the value of the setting. If the {es} keystore is
144147
password protected, you are also prompted to enter the password.
145148

146-
To pass the setting value through standard input (stdin), use the `--stdin` flag:
149+
You can also add multiple settings with the `add` command:
150+
151+
[source,sh]
152+
----------------------------------------------------------------
153+
bin/elasticsearch-keystore add \
154+
the.setting.name.to.set \
155+
the.other.setting.name.to.set
156+
----------------------------------------------------------------
157+
158+
You are prompted to enter the values of the settings. If the {es} keystore is
159+
password protected, you are also prompted to enter the password.
160+
161+
To pass the settings values through standard input (stdin), use the `--stdin`
162+
flag:
147163

148164
[source,sh]
149165
----------------------------------------------------------------
150166
cat /file/containing/setting/value | bin/elasticsearch-keystore add --stdin the.setting.name.to.set
151167
----------------------------------------------------------------
152168

169+
Values for multiple settings must be separated by carriage returns or newlines.
170+
153171
[discrete]
154172
[[add-file-to-keystore]]
155173
==== Add files to the keystore

‎server/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java‎

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,24 @@
1919

2020
package org.elasticsearch.common.settings;
2121

22-
import java.io.BufferedReader;
23-
import java.io.CharArrayWriter;
24-
import java.io.InputStream;
25-
import java.io.InputStreamReader;
26-
import java.nio.charset.StandardCharsets;
27-
import java.util.Arrays;
28-
2922
import joptsimple.OptionSet;
3023
import joptsimple.OptionSpec;
3124
import org.elasticsearch.cli.ExitCodes;
3225
import org.elasticsearch.cli.Terminal;
3326
import org.elasticsearch.cli.UserException;
27+
import org.elasticsearch.common.CheckedFunction;
3428
import org.elasticsearch.env.Environment;
3529

30+
import java.io.BufferedReader;
31+
import java.io.CharArrayWriter;
32+
import java.io.Closeable;
33+
import java.io.IOException;
34+
import java.io.InputStream;
35+
import java.io.InputStreamReader;
36+
import java.nio.charset.StandardCharsets;
37+
import java.util.Arrays;
38+
import java.util.List;
39+
3640
/**
3741
* A subcommand for the keystore cli which adds a string setting.
3842
*/
@@ -42,11 +46,11 @@ class AddStringKeyStoreCommand extends BaseKeyStoreCommand {
4246
private final OptionSpec<String> arguments;
4347

4448
AddStringKeyStoreCommand() {
45-
super("Add a string setting to the keystore", false);
46-
this.stdinOption = parser.acceptsAll(Arrays.asList("x", "stdin"), "Read setting value from stdin");
49+
super("Add a string settings to the keystore", false);
50+
this.stdinOption = parser.acceptsAll(Arrays.asList("x", "stdin"), "Read setting values from stdin");
4751
this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"),
4852
"Overwrite existing setting without prompting, creating keystore if necessary");
49-
this.arguments = parser.nonOptions("setting name");
53+
this.arguments = parser.nonOptions("setting names");
5054
}
5155

5256
// pkg private so tests can manipulate
@@ -56,41 +60,53 @@ InputStream getStdin() {
5660

5761
@Override
5862
protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception {
59-
String setting = arguments.value(options);
60-
if (setting == null) {
61-
throw new UserException(ExitCodes.USAGE, "The setting name can not be null");
63+
final List<String> settings = arguments.values(options);
64+
if (settings.isEmpty()) {
65+
throw new UserException(ExitCodes.USAGE, "the setting names can not be empty");
6266
}
67+
6368
final KeyStoreWrapper keyStore = getKeyStore();
64-
if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) {
65-
if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) {
66-
terminal.println("Exiting without modifying keystore.");
67-
return;
68-
}
69-
}
7069

71-
final char[] value;
70+
final Closeable closeable;
71+
final CheckedFunction<String, char[], IOException> valueSupplier;
7272
if (options.has(stdinOption)) {
73-
try (BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8));
74-
CharArrayWriter writer = new CharArrayWriter()) {
75-
int charInt;
76-
while ((charInt = stdinReader.read()) != -1) {
77-
if ((char) charInt == '\r' || (char) charInt == '\n') {
78-
break;
73+
final BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8));
74+
valueSupplier = s -> {
75+
try (CharArrayWriter writer = new CharArrayWriter()) {
76+
int c;
77+
while ((c = stdinReader.read()) != -1) {
78+
if ((char) c == '\r' || (char) c == '\n') {
79+
break;
80+
}
81+
writer.write((char) c);
7982
}
80-
writer.write((char) charInt);
83+
return writer.toCharArray();
8184
}
82-
value = writer.toCharArray();
83-
}
85+
};
86+
closeable = stdinReader;
8487
} else {
85-
value = terminal.readSecret("Enter value for " + setting + ": ");
88+
valueSupplier = s -> terminal.readSecret("Enter value for " + s + ": ");
89+
closeable = () -> {};
8690
}
8791

88-
try {
89-
keyStore.setString(setting, value);
90-
} catch (IllegalArgumentException e) {
91-
throw new UserException(ExitCodes.DATA_ERROR, e.getMessage());
92+
try (Closeable ignored = closeable) {
93+
for (final String setting : settings) {
94+
if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) {
95+
if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) {
96+
terminal.println("Exiting without modifying keystore.");
97+
return;
98+
}
99+
}
100+
101+
try {
102+
keyStore.setString(setting, valueSupplier.apply(setting));
103+
} catch (final IllegalArgumentException e) {
104+
throw new UserException(ExitCodes.DATA_ERROR, e.getMessage());
105+
}
106+
}
92107
}
93-
keyStore.save(env.configFile(), getKeyStorePassword().getChars());
94108

109+
keyStore.save(env.configFile(), getKeyStorePassword().getChars());
95110
}
111+
96112
}

‎server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,17 @@
1919

2020
package org.elasticsearch.common.settings;
2121

22+
import org.elasticsearch.cli.Command;
23+
import org.elasticsearch.cli.ExitCodes;
24+
import org.elasticsearch.cli.UserException;
25+
import org.elasticsearch.env.Environment;
26+
2227
import java.io.ByteArrayInputStream;
2328
import java.io.CharArrayWriter;
2429
import java.io.InputStream;
2530
import java.nio.charset.StandardCharsets;
2631
import java.util.Map;
2732

28-
import org.elasticsearch.cli.Command;
29-
import org.elasticsearch.cli.ExitCodes;
30-
import org.elasticsearch.cli.UserException;
31-
import org.elasticsearch.env.Environment;
32-
3333
import static org.hamcrest.Matchers.anyOf;
3434
import static org.hamcrest.Matchers.containsString;
3535
import static org.hamcrest.Matchers.hasToString;
@@ -155,6 +155,19 @@ public void testPromptForValue() throws Exception {
155155
assertSecureString("foo", "secret value", password);
156156
}
157157

158+
public void testPromptForMultipleValues() throws Exception {
159+
final String password = "keystorepassword";
160+
KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
161+
terminal.addSecretInput(password);
162+
terminal.addSecretInput("bar1");
163+
terminal.addSecretInput("bar2");
164+
terminal.addSecretInput("bar3");
165+
execute("foo1", "foo2", "foo3");
166+
assertSecureString("foo1", "bar1", password);
167+
assertSecureString("foo2", "bar2", password);
168+
assertSecureString("foo3", "bar3", password);
169+
}
170+
158171
public void testStdinShort() throws Exception {
159172
String password = "keystorepassword";
160173
KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
@@ -200,6 +213,17 @@ public void testStdinInputWithCarriageReturn() throws Exception {
200213
assertSecureString("foo", "Typedthisandhitenter", password);
201214
}
202215

216+
public void testStdinWithMultipleValues() throws Exception {
217+
final String password = "keystorepassword";
218+
KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
219+
terminal.addSecretInput(password);
220+
setInput("bar1\nbar2\nbar3");
221+
execute(randomFrom("-x", "--stdin"), "foo1", "foo2", "foo3");
222+
assertSecureString("foo1", "bar1", password);
223+
assertSecureString("foo2", "bar2", password);
224+
assertSecureString("foo3", "bar3", password);
225+
}
226+
203227
public void testAddUtf8String() throws Exception {
204228
String password = "keystorepassword";
205229
KeyStoreWrapper.create().save(env.configFile(), password.toCharArray());
@@ -223,7 +247,7 @@ public void testMissingSettingName() throws Exception {
223247
terminal.addTextInput("");
224248
UserException e = expectThrows(UserException.class, this::execute);
225249
assertEquals(ExitCodes.USAGE, e.exitCode);
226-
assertThat(e.getMessage(), containsString("The setting name can not be null"));
250+
assertThat(e.getMessage(), containsString("the setting names can not be empty"));
227251
}
228252

229253
public void testSpecialCharacterInName() throws Exception {

0 commit comments

Comments
 (0)