Skip to content

Commit

Permalink
Allow keystore add-file to handle multiple settings (elastic#54240)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jasontedor
jasontedor authored Mar 26, 2020
1 parent c120388 commit 18843a0
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 38 deletions.
54 changes: 28 additions & 26 deletions .../keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,6 +28,11 @@
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.
*/
Expand All @@ -49,43 +49,45 @@ 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);
if (argumentValues.size() % 2 != 0) {
throw new UserException(ExitCodes.USAGE, "settings and filenames must come in pairs");
}

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;

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;
}
}
}

if (argumentValues.size() == 1) {
throw new UserException(ExitCodes.USAGE, "Missing file name");
}
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"
);
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));
}
keyStore.setFile(setting, Files.readAllBytes(file));

keyStore.save(env.configFile(), getKeyStorePassword().getChars());
}

@SuppressForbidden(reason = "file arg for cli")
private Path getPath(String file) {
return PathUtils.get(file);
}

}
36 changes: 28 additions & 8 deletions ...tore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -49,7 +53,7 @@ private Path createRandomFile() throws IOException {
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;
}
Expand Down Expand Up @@ -164,7 +168,7 @@ public void testMissingFileName() throws Exception {
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 {
Expand All @@ -183,7 +187,7 @@ public void testExtraArguments() throws Exception {
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 {
Expand Down Expand Up @@ -216,4 +220,20 @@ public void testAddToUnprotectedKeystore() throws Exception {
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);
}
}

}
17 changes: 13 additions & 4 deletions docs/reference/commands/keystore.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])
--------------------------------------------------
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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.

Expand Down

0 comments on commit 18843a0

Please sign in to comment.