Allow keystore add-file to handle multiple settings (#54240)

Today the keystore add-file command can only handle adding a single
setting/file 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-file keystore command to accept adding multiple
settings in a single invocation.
This commit is contained in:
Jason Tedor 2020-03-26 00:04:52 -04:00
parent 4fbc0e9ab8
commit 6af89e62d1
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
3 changed files with 70 additions and 37 deletions

View File

@ -12,7 +12,7 @@ in the {es} keystore.
--------------------------------------------------
bin/elasticsearch-keystore
([add <settings>] [-f] [--stdin] |
[add-file <setting> <path>] | [create] [-p] |
[add-file (<setting> <path>)+] | [create] [-p] |
[list] | [passwd] | [remove <setting>] | [upgrade])
[-h, --help] ([-s, --silent] | [-v, --verbose])
--------------------------------------------------
@ -48,7 +48,7 @@ must confirm that you want to overwrite the current value. If the keystore does
not exist, you must confirm that you want to create a keystore. To avoid these
two confirmation prompts, use the `-f` parameter.
`add-file <setting> <path>`:: Adds a file to the keystore.
`add-file (<setting> <path>)+`:: Adds files to the keystore.
`create`:: Creates the keystore.
@ -173,14 +173,23 @@ Values for multiple settings must be separated by carriage returns or newlines.
==== Add files to the keystore
You can add sensitive files, like authentication key files for Cloud plugins,
using the `add-file` command. Be sure to include your file path as an argument
after the setting name.
using the `add-file` command. Settings and file paths are specified in pairs
consisting of `setting path`.
[source,sh]
----------------------------------------------------------------
bin/elasticsearch-keystore add-file the.setting.name.to.set /path/example-file.json
----------------------------------------------------------------
You can add multiple files with the `add-file` command:
[source,sh]
----------------------------------------------------------------
bin/elasticsearch-keystore add-file \
the.setting.name.to.set /path/example-file.json \
the.other.setting.name.to.set /path/other-example-file.json
----------------------------------------------------------------
If the {es} keystore is password protected, you are prompted to enter the
password.

View File

@ -19,11 +19,6 @@
package org.elasticsearch.common.settings;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import joptsimple.OptionSet;
import joptsimple.OptionSpec;
import org.elasticsearch.cli.ExitCodes;
@ -33,6 +28,11 @@ import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.env.Environment;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
/**
* A subcommand for the keystore cli which adds a file setting.
*/
@ -47,36 +47,39 @@ class AddFileKeyStoreCommand extends BaseKeyStoreCommand {
// jopt simple has issue with multiple non options, so we just get one set of them here
// and convert to File when necessary
// see https://github.com/jopt-simple/jopt-simple/issues/103
this.arguments = parser.nonOptions("setting [filepath]");
this.arguments = parser.nonOptions("(setting path)+");
}
@Override
protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception {
List<String> argumentValues = arguments.values(options);
final List<String> argumentValues = arguments.values(options);
if (argumentValues.size() == 0) {
throw new UserException(ExitCodes.USAGE, "Missing setting name");
}
String setting = argumentValues.get(0);
final KeyStoreWrapper keyStore = getKeyStore();
if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) {
if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) {
terminal.println("Exiting without modifying keystore.");
return;
}
if (argumentValues.size() % 2 != 0) {
throw new UserException(ExitCodes.USAGE, "settings and filenames must come in pairs");
}
if (argumentValues.size() == 1) {
throw new UserException(ExitCodes.USAGE, "Missing file name");
final KeyStoreWrapper keyStore = getKeyStore();
for (int i = 0; i < argumentValues.size(); i += 2) {
final String setting = argumentValues.get(i);
if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) {
if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) {
terminal.println("Exiting without modifying keystore.");
return;
}
}
final Path file = getPath(argumentValues.get(i + 1));
if (Files.exists(file) == false) {
throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist");
}
keyStore.setFile(setting, Files.readAllBytes(file));
}
Path file = getPath(argumentValues.get(1));
if (Files.exists(file) == false) {
throw new UserException(ExitCodes.IO_ERROR, "File [" + file.toString() + "] does not exist");
}
if (argumentValues.size() > 2) {
throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" +
String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath");
}
keyStore.setFile(setting, Files.readAllBytes(file));
keyStore.save(env.configFile(), getKeyStorePassword().getChars());
}
@ -84,4 +87,5 @@ class AddFileKeyStoreCommand extends BaseKeyStoreCommand {
private Path getPath(String file) {
return PathUtils.get(file);
}
}

View File

@ -19,16 +19,20 @@
package org.elasticsearch.common.settings;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Map;
import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.env.Environment;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsString;
@ -49,7 +53,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
for (int i = 0; i < length; ++i) {
bytes[i] = randomByte();
}
Path file = env.configFile().resolve("randomfile");
Path file = env.configFile().resolve(randomAlphaOfLength(16));
Files.write(file, bytes);
return file;
}
@ -164,7 +168,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
terminal.addSecretInput(password);
UserException e = expectThrows(UserException.class, () -> execute("foo"));
assertEquals(ExitCodes.USAGE, e.exitCode);
assertThat(e.getMessage(), containsString("Missing file name"));
assertThat(e.getMessage(), containsString("settings and filenames must come in pairs"));
}
public void testFileDNE() throws Exception {
@ -183,7 +187,7 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
terminal.addSecretInput(password);
UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString(), "bar"));
assertEquals(e.getMessage(), ExitCodes.USAGE, e.exitCode);
assertThat(e.getMessage(), containsString("Unrecognized extra arguments [bar]"));
assertThat(e.getMessage(), containsString("settings and filenames must come in pairs"));
}
public void testIncorrectPassword() throws Exception {
@ -216,4 +220,20 @@ public class AddFileKeyStoreCommandTests extends KeyStoreCommandTestCase {
execute("foo", "path/dne");
assertSecureFile("foo", file, password);
}
public void testAddMultipleFiles() throws Exception {
final String password = "keystorepassword";
createKeystore(password);
final int n = randomIntBetween(1, 8);
final List<Tuple<String, Path>> settingFilePairs = new ArrayList<>(n);
for (int i = 0; i < n; i++) {
settingFilePairs.add(Tuple.tuple("foo" + i, createRandomFile()));
}
terminal.addSecretInput(password);
execute(settingFilePairs.stream().flatMap(t -> Stream.of(t.v1(), t.v2().toString())).toArray(String[]::new));
for (int i = 0; i < n; i++) {
assertSecureFile(settingFilePairs.get(i).v1(), settingFilePairs.get(i).v2(), password);
}
}
}