From fcd797f9e36167b2a431de005cb29b6e81fdfc0b Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 16 Jun 2020 16:18:27 -0400 Subject: [PATCH 1/3] Enable TTY password OS tests, plus refactoring (#57759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two keystore tests were unintentionally ignored when the password-protected keystore work was merged. I've reënabled those tests here. I've also refactored the test methods a little bit to reduce the API surface: instead of having a "startElasticsearchTtyPassword" method and a "startElasticsearchStandardInputPassword" method, I've made a single "startElasticsearch" method with a "useTty" boolean argument. --- .../test/KeystoreManagementTests.java | 38 +++++++++---------- .../packaging/test/PackageTests.java | 2 +- .../packaging/test/PackagingTestCase.java | 33 ++++++++-------- 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java index c0fdfbcbd1ab5..003c1b39049f4 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java @@ -27,7 +27,6 @@ import org.elasticsearch.packaging.util.Platforms; import org.elasticsearch.packaging.util.ServerUtils; import org.elasticsearch.packaging.util.Shell; -import org.junit.Ignore; import java.io.IOException; import java.nio.file.Files; @@ -125,7 +124,7 @@ public void test12InstallDockerDistribution() throws Exception { public void test20CreateKeystoreManually() throws Exception { rmKeystoreIfExists(); - createKeystore(); + createKeystore(null); final Installation.Executables bin = installation.executables(); verifyKeystorePermissions(); @@ -157,28 +156,27 @@ public void test40KeystorePasswordOnStandardInput() throws Exception { String password = "^|<>\\&exit"; // code insertion on Windows if special characters are not escaped rmKeystoreIfExists(); - createKeystore(); - setKeystorePassword(password); + createKeystore(password); assertPasswordProtectedKeystore(); - awaitElasticsearchStartup(startElasticsearchStandardInputPassword(password, true)); + awaitElasticsearchStartup(runElasticsearchStartCommand(password, true, false)); ServerUtils.runElasticsearchTests(); stopElasticsearch(); } - public void test41WrongKeystorePasswordOnStandardInput() { + public void test41WrongKeystorePasswordOnStandardInput() throws Exception { assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive()); assumeThat(installation, is(notNullValue())); assertPasswordProtectedKeystore(); - Shell.Result result = startElasticsearchStandardInputPassword("wrong", false); + Shell.Result result = runElasticsearchStartCommand("wrong", false, false); assertElasticsearchFailure(result, Arrays.asList(ERROR_INCORRECT_PASSWORD, ERROR_CORRUPTED_KEYSTORE), null); } - @Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ public void test42KeystorePasswordOnTty() throws Exception { + /* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS); assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive()); assumeThat(installation, is(notNullValue())); @@ -186,25 +184,24 @@ public void test42KeystorePasswordOnTty() throws Exception { String password = "keystorepass"; rmKeystoreIfExists(); - createKeystore(); - setKeystorePassword(password); + createKeystore(password); assertPasswordProtectedKeystore(); - awaitElasticsearchStartup(startElasticsearchTtyPassword(password, true)); + awaitElasticsearchStartup(runElasticsearchStartCommand(password, true, true)); ServerUtils.runElasticsearchTests(); stopElasticsearch(); } - @Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ public void test43WrongKeystorePasswordOnTty() throws Exception { + /* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS); assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive()); assumeThat(installation, is(notNullValue())); assertPasswordProtectedKeystore(); - Shell.Result result = startElasticsearchTtyPassword("wrong", false); + Shell.Result result = runElasticsearchStartCommand("wrong", false, true); // error will be on stdout for "expect" assertThat(result.stdout, anyOf(containsString(ERROR_INCORRECT_PASSWORD), containsString(ERROR_CORRUPTED_KEYSTORE))); } @@ -219,8 +216,7 @@ public void test44EncryptedKeystoreAllowsHelpMessage() throws Exception { String password = "keystorepass"; rmKeystoreIfExists(); - createKeystore(); - setKeystorePassword(password); + createKeystore(password); assertPasswordProtectedKeystore(); Shell.Result r = installation.executables().elasticsearch.run("--help"); @@ -233,8 +229,7 @@ public void test50KeystorePasswordFromFile() throws Exception { Path esKeystorePassphraseFile = installation.config.resolve("eks"); rmKeystoreIfExists(); - createKeystore(); - setKeystorePassword(password); + createKeystore(password); assertPasswordProtectedKeystore(); @@ -269,7 +264,7 @@ public void test51WrongKeystorePasswordFromFile() throws Exception { Files.write(esKeystorePassphraseFile, singletonList("wrongpassword")); Packages.JournaldWrapper journaldWrapper = new Packages.JournaldWrapper(sh); - Shell.Result result = runElasticsearchStartCommand(false); + Shell.Result result = runElasticsearchStartCommand(null, false, false); assertElasticsearchFailure(result, Arrays.asList(ERROR_INCORRECT_PASSWORD, ERROR_CORRUPTED_KEYSTORE), journaldWrapper); } finally { sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE"); @@ -400,7 +395,8 @@ private Path getKeystoreFileFromDockerContainer(String password, Path dockerKeys return tempDirectory.resolve("elasticsearch.keystore"); } - private void createKeystore() throws Exception { + /** Create a keystore. Provide a password to password-protect it, otherwise use null */ + private void createKeystore(String password) throws Exception { Path keystore = installation.config("elasticsearch.keystore"); final Installation.Executables bin = installation.executables(); bin.keystoreTool.run("create"); @@ -418,6 +414,10 @@ private void createKeystore() throws Exception { throw new RuntimeException(e); } } + + if (password != null) { + setKeystorePassword(password); + } } private void rmKeystoreIfExists() { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index 72658af146fbe..c3bdbc86d52fc 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -360,7 +360,7 @@ public void test90DoNotCloseStderrWhenQuiet() throws Exception { // Make sure we don't pick up the journal entries for previous ES instances. Packages.JournaldWrapper journald = new Packages.JournaldWrapper(sh); - runElasticsearchStartCommand(true); + runElasticsearchStartCommand(null, true, false); final Result logs = journald.getLogs(); assertThat(logs.stdout, containsString("Failed to load settings from [elasticsearch.yml]")); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index 2d7392dc078e2..0401c2526085d 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -206,7 +206,7 @@ protected static void install() throws Exception { */ protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Exception { try { - awaitElasticsearchStartup(runElasticsearchStartCommand(true)); + awaitElasticsearchStartup(runElasticsearchStartCommand(null, true, false)); } catch (Exception e) { if (Files.exists(installation.home.resolve("elasticsearch.pid"))) { String pid = FileUtils.slurp(installation.home.resolve("elasticsearch.pid")).trim(); @@ -238,14 +238,27 @@ protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Ex * Run the command to start Elasticsearch, but don't wait or test for success. * This method is useful for testing failure conditions in startup. To await success, * use {@link #startElasticsearch()}. + * @param password Password for password-protected keystore, null for no password; + * this option will fail for non-archive distributions + * @param daemonize Run Elasticsearch in the background + * @param useTty Use a tty for inputting the password rather than standard input; + * this option will fail for non-archive distributions * @return Shell results of the startup command. * @throws Exception when command fails immediately. */ - public Shell.Result runElasticsearchStartCommand(boolean daemonize) throws Exception { + public Shell.Result runElasticsearchStartCommand(String password, boolean daemonize, boolean useTty) throws Exception { + if (password != null) { + assertTrue("Only archives support user-entered passwords", distribution().isArchive()); + } + switch (distribution.packaging) { case TAR: case ZIP: - return Archives.runElasticsearchStartCommand(installation, sh, null, daemonize); + if (useTty) { + return Archives.startElasticsearchWithTty(installation, sh, password, daemonize); + } else { + return Archives.runElasticsearchStartCommand(installation, sh, password, daemonize); + } case DEB: case RPM: return Packages.runElasticsearchStartCommand(sh); @@ -296,21 +309,11 @@ public void awaitElasticsearchStartup(Shell.Result result) throws Exception { /** * Start Elasticsearch and wait until it's up and running. If you just want to run - * the start command, use {@link #runElasticsearchStartCommand(boolean)}. + * the start command, use {@link #runElasticsearchStartCommand(String, boolean, boolean)}. * @throws Exception if Elasticsearch can't start */ public void startElasticsearch() throws Exception { - awaitElasticsearchStartup(runElasticsearchStartCommand(true)); - } - - public Shell.Result startElasticsearchStandardInputPassword(String password, boolean daemonize) { - assertTrue("Only archives support passwords on standard input", distribution().isArchive()); - return Archives.runElasticsearchStartCommand(installation, sh, password, daemonize); - } - - public Shell.Result startElasticsearchTtyPassword(String password, boolean daemonize) throws Exception { - assertTrue("Only archives support passwords on TTY", distribution().isArchive()); - return Archives.startElasticsearchWithTty(installation, sh, password, daemonize); + awaitElasticsearchStartup(runElasticsearchStartCommand(null, true, false)); } public void assertElasticsearchFailure(Shell.Result result, String expectedMessage, Packages.JournaldWrapper journaldWrapper) { From 6f30abf1e08f56165b6c3067227c8e5d35e514ab Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 17 Jun 2020 20:56:38 -0400 Subject: [PATCH 2/3] Remove -d option for tty startup Centos 6 uses a version of expect that kills the elasticsearch process when it tries to daemonize. I will fix this in future work but for now I'm replacing it with a todo. --- .../java/org/elasticsearch/packaging/util/Archives.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java index db9abb5d2971c..ea8e4082c5b9b 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java @@ -244,9 +244,10 @@ public static Shell.Result startElasticsearchWithTty(Installation installation, // requires the "expect" utility to be installed List command = new ArrayList<>(); command.add("sudo -E -u %s %s -p %s"); - if (daemonize) { - command.add("-d"); - } + + // TODO: daemonization isn't working with expect versions prior to 5.45 + // centos-6 has 5.45.1.15 + String script = String.format( Locale.ROOT, "expect -c \"$(cat< Date: Wed, 24 Jun 2020 16:34:43 -0400 Subject: [PATCH 3/3] Test daemonization and non-daemonization case for tty --- .../test/KeystoreManagementTests.java | 34 ++++++++++++++++--- .../packaging/util/Archives.java | 9 +++-- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java index 003c1b39049f4..2ced9781a2e46 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java @@ -27,6 +27,7 @@ import org.elasticsearch.packaging.util.Platforms; import org.elasticsearch.packaging.util.ServerUtils; import org.elasticsearch.packaging.util.Shell; +import org.junit.Ignore; import java.io.IOException; import java.nio.file.Files; @@ -38,6 +39,7 @@ import java.util.List; import java.util.Map; +import static com.carrotsearch.randomizedtesting.RandomizedTest.randomBoolean; import static java.util.Collections.singletonList; import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER; import static org.elasticsearch.packaging.util.Archives.installArchive; @@ -175,7 +177,29 @@ public void test41WrongKeystorePasswordOnStandardInput() throws Exception { assertElasticsearchFailure(result, Arrays.asList(ERROR_INCORRECT_PASSWORD, ERROR_CORRUPTED_KEYSTORE), null); } - public void test42KeystorePasswordOnTty() throws Exception { + /** + * This test simulates a user starting Elasticsearch on the command line without daemonizing + */ + public void test42KeystorePasswordOnTtyRunningInForeground() throws Exception { + /* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ + assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS); + assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive()); + assumeThat(installation, is(notNullValue())); + + String password = "keystorepass"; + + rmKeystoreIfExists(); + createKeystore(password); + + assertPasswordProtectedKeystore(); + + awaitElasticsearchStartup(runElasticsearchStartCommand(password, false, true)); + ServerUtils.runElasticsearchTests(); + stopElasticsearch(); + } + + @Ignore // awaits fix: https://github.com/elastic/elasticsearch/issues/49340 + public void test43KeystorePasswordOnTtyDaemonized() throws Exception { /* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS); assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive()); @@ -193,7 +217,7 @@ public void test42KeystorePasswordOnTty() throws Exception { stopElasticsearch(); } - public void test43WrongKeystorePasswordOnTty() throws Exception { + public void test44WrongKeystorePasswordOnTty() throws Exception { /* Windows issue awaits fix: https://github.com/elastic/elasticsearch/issues/49340 */ assumeTrue("expect command isn't on Windows", distribution.platform != Distribution.Platform.WINDOWS); assumeTrue("packages will use systemd, which doesn't handle stdin", distribution.isArchive()); @@ -201,7 +225,9 @@ public void test43WrongKeystorePasswordOnTty() throws Exception { assertPasswordProtectedKeystore(); - Shell.Result result = runElasticsearchStartCommand("wrong", false, true); + // daemonization shouldn't matter for this test + boolean daemonize = randomBoolean(); + Shell.Result result = runElasticsearchStartCommand("wrong", daemonize, true); // error will be on stdout for "expect" assertThat(result.stdout, anyOf(containsString(ERROR_INCORRECT_PASSWORD), containsString(ERROR_CORRUPTED_KEYSTORE))); } @@ -210,7 +236,7 @@ public void test43WrongKeystorePasswordOnTty() throws Exception { * If we have an encrypted keystore, we shouldn't require a password to * view help information. */ - public void test44EncryptedKeystoreAllowsHelpMessage() throws Exception { + public void test45EncryptedKeystoreAllowsHelpMessage() throws Exception { assumeTrue("users call elasticsearch directly in archive case", distribution.isArchive()); String password = "keystorepass"; diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java index ea8e4082c5b9b..2cda7f7e5de72 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java @@ -241,12 +241,15 @@ public static Shell.Result startElasticsearchWithTty(Installation installation, final Path pidFile = installation.home.resolve("elasticsearch.pid"); final Installation.Executables bin = installation.executables(); - // requires the "expect" utility to be installed List command = new ArrayList<>(); command.add("sudo -E -u %s %s -p %s"); - // TODO: daemonization isn't working with expect versions prior to 5.45 - // centos-6 has 5.45.1.15 + // requires the "expect" utility to be installed + // TODO: daemonization isn't working with expect versions prior to 5.45, but centos-6 has 5.45.1.15 + // TODO: try using pty4j to make daemonization work + if (daemonize) { + command.add("-d"); + } String script = String.format( Locale.ROOT,