Skip to content

Commit

Permalink
Fix failing Keystore Passphrase test for feature branch (elastic#50154)
Browse files Browse the repository at this point in the history 
One problem with the passphrase-from-file tests, as written, is that
they would leave a SystemD environment variable set when they failed,
and this setting would cause elasticsearch startup to fail for other
tests as well. By using a try-finally, I hope that these tests will fail
more gracefully.

It appears that our Fedora and Ubuntu environments may be configured to
store journald information under /var rather than under /run, so that it
will persist between boots. Our destructive tests that read from the
journal need to account for this in order to avoid trying to limit the
output we check in tests.
  • Loading branch information
@williamrandolph
williamrandolph committed Jan 16, 2020
1 parent e883395 commit 3e06d4a
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 130 deletions.
138 changes: 54 additions & 84 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ public class KeystoreManagementTests extends PackagingTestCase {
public void test10InstallArchiveDistribution() throws Exception {
assumeTrue(distribution().isArchive());

installation = installArchive(distribution);
installation = installArchive(sh, distribution);
verifyArchiveInstallation(installation, distribution());

final Installation.Executables bin = installation.executables();
Shell.Result r = sh.runIgnoreExitCode(bin.elasticsearchKeystore + " has-passwd");
Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
assertThat("has-passwd should fail", r.exitCode, not(is(0)));
assertThat("has-passwd should fail", r.stderr, containsString("ERROR: Elasticsearch keystore not found"));
}
Expand All @@ -72,12 +72,12 @@ public void test11InstallPackageDistribution() throws Exception {
assumeTrue(distribution().isPackage());

assertRemoved(distribution);
installation = installPackage(distribution);
installation = installPackage(sh, distribution);
assertInstalled(distribution);
verifyPackageInstallation(installation, distribution, sh);

final Installation.Executables bin = installation.executables();
Shell.Result r = sh.runIgnoreExitCode(bin.elasticsearchKeystore + " has-passwd");
Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
assertThat("has-passwd should fail", r.exitCode, not(is(0)));
assertThat("has-passwd should fail", r.stderr, containsString("ERROR: Keystore is not password-protected"));
}
Expand All @@ -90,10 +90,7 @@ public void test20CreateKeystoreManually() throws Exception {
final Installation.Executables bin = installation.executables();
verifyKeystorePermissions();

String possibleSudo = distribution().isArchive() && Platforms.LINUX
? "sudo -u " + ARCHIVE_OWNER + " "
: "";
Shell.Result r = sh.run(possibleSudo + bin.elasticsearchKeystore + " list");
Shell.Result r = bin.keystoreTool.run("list");
assertThat(r.stdout, containsString("keystore.seed"));
}

Expand All @@ -109,10 +106,7 @@ public void test30AutoCreateKeystore() throws Exception {
verifyKeystorePermissions();

final Installation.Executables bin = installation.executables();
String possibleSudo = distribution().isArchive() && Platforms.LINUX
? "sudo -u " + ARCHIVE_OWNER + " "
: "";
Shell.Result r = sh.run(possibleSudo + bin.elasticsearchKeystore + " list");
Shell.Result r = bin.keystoreTool.run("list");
assertThat(r.stdout, containsString("keystore.seed"));
}

Expand Down Expand Up @@ -192,87 +186,57 @@ public void test50KeystorePasswordFromFile() throws Exception {

assertPasswordProtectedKeystore();

sh.getEnv().put("ES_KEYSTORE_PASSPHRASE_FILE", esKeystorePassphraseFile.toString());
distribution().packagingConditional()
.forPackage(
() -> sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=$ES_KEYSTORE_PASSPHRASE_FILE")
)
.forArchive(Platforms.NO_ACTION)
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run();
try {
sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=" + esKeystorePassphraseFile);

Files.createFile(esKeystorePassphraseFile);
Files.write(esKeystorePassphraseFile,
(password + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
StandardOpenOption.WRITE);
Files.createFile(esKeystorePassphraseFile);
Files.write(esKeystorePassphraseFile,
(password + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
StandardOpenOption.WRITE);

startElasticsearch();
ServerUtils.runElasticsearchTests();
stopElasticsearch();

distribution().packagingConditional()
.forPackage(
() -> sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE")
)
.forArchive(Platforms.NO_ACTION)
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run();
startElasticsearch();
ServerUtils.runElasticsearchTests();
stopElasticsearch();
} finally {
sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE");
}
}

@Ignore /* Ignored for feature branch, awaits fix: https://github.com/elastic/elasticsearch/issues/50079 */
public void test51WrongKeystorePasswordFromFile() throws Exception {
assumeTrue("only for systemd", Platforms.isSystemd() && distribution().isPackage());
Path esKeystorePassphraseFile = installation.config.resolve("eks");

assertPasswordProtectedKeystore();

sh.getEnv().put("ES_KEYSTORE_PASSPHRASE_FILE", esKeystorePassphraseFile.toString());
distribution().packagingConditional()
.forPackage(
() -> sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=$ES_KEYSTORE_PASSPHRASE_FILE")
)
.forArchive(Platforms.NO_ACTION)
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run();

if (Files.exists(esKeystorePassphraseFile)) {
rm(esKeystorePassphraseFile);
}
try {
sh.run("sudo systemctl set-environment ES_KEYSTORE_PASSPHRASE_FILE=" + esKeystorePassphraseFile);

Files.createFile(esKeystorePassphraseFile);
Files.write(esKeystorePassphraseFile,
("wrongpassword" + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
StandardOpenOption.WRITE);
if (Files.exists(esKeystorePassphraseFile)) {
rm(esKeystorePassphraseFile);
}

Shell.Result result = runElasticsearchStartCommand();
assertElasticsearchFailure(result, PASSWORD_ERROR_MESSAGE);
Files.createFile(esKeystorePassphraseFile);
Files.write(esKeystorePassphraseFile,
("wrongpassword" + System.lineSeparator()).getBytes(StandardCharsets.UTF_8),
StandardOpenOption.WRITE);

distribution().packagingConditional()
.forPackage(
() -> sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE")
)
.forArchive(Platforms.NO_ACTION)
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run();
Shell.Result result = runElasticsearchStartCommand();
assertElasticsearchFailure(result, PASSWORD_ERROR_MESSAGE);
} finally {
sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE");
}
}

private void createKeystore() throws Exception {
Path keystore = installation.config("elasticsearch.keystore");
final Installation.Executables bin = installation.executables();
Platforms.onLinux(() -> {
distribution().packagingConditional()
.forPackage(() -> sh.run(bin.elasticsearchKeystore + " create"))
.forArchive(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create"))
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run();
});
bin.keystoreTool.run("create");

// this is a hack around the fact that we can't run a command in the same session as the same user but not as administrator.
// the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here.
// from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests.
// when we run these commands as a role user we won't have to do this
Platforms.onWindows(() -> {
sh.run(bin.elasticsearchKeystore + " create");
sh.chown(keystore);
});
}
Expand All @@ -288,31 +252,37 @@ private void setKeystorePassword(String password) throws Exception {
final Installation.Executables bin = installation.executables();

// set the password by passing it to stdin twice
Platforms.onLinux(() -> distribution().packagingConditional()
.forPackage(() -> sh.run("( echo \'" + password + "\' ; echo \'" + password + "\' ) | " +
bin.elasticsearchKeystore + " passwd"))
.forArchive(() -> sh.run("( echo \'" + password + "\' ; echo \'" + password + "\' ) | " +
"sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " passwd"))
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run()
);
Platforms.onLinux(() -> {
bin.keystoreTool.run("passwd", password + "\n" + password + "\n");
});

Platforms.onWindows(() -> {
sh.run("Invoke-Command -ScriptBlock {echo \'" + password + "\'; echo \'" + password + "\'} | "
+ bin.elasticsearchKeystore + " passwd");
+ bin.keystoreTool + " passwd");
});
}

private void assertPasswordProtectedKeystore() {
Shell.Result r = sh.runIgnoreExitCode(installation.executables().elasticsearchKeystore.toString() + " has-passwd");
Shell.Result r = installation.executables().keystoreTool.run("has-passwd");
assertThat("keystore should be password protected", r.exitCode, is(0));
}

private void verifyKeystorePermissions() throws Exception {
Path keystore = installation.config("elasticsearch.keystore");
distribution().packagingConditional()
.forPackage(() -> assertThat(keystore, file(File, "root", "elasticsearch", p660)))
.forArchive(() -> assertThat(keystore, file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660)))
.forDocker(/* TODO */ Platforms.NO_ACTION)
.run();
switch (distribution.packaging) {
case TAR:
case ZIP:
assertThat(keystore, file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));
break;
case DEB:
case RPM:
assertThat(keystore, file(File, "root", "elasticsearch", p660));
break;
case DOCKER:
// TODO #49469
break;
default:
throw new IllegalStateException("Unknown Elasticsearch packaging type.");
}
}
}
44 changes: 0 additions & 44 deletions qa/os/src/test/java/org/elasticsearch/packaging/util/Distribution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@
package org.elasticsearch.packaging.util;

import java.nio.file.Path;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;

public class Distribution {

Expand Down Expand Up @@ -88,46 +84,6 @@ public enum Packaging {
}
}

public static class PackagingConditional {
Distribution distribution;

public PackagingConditional(Distribution distribution) {
this.distribution = distribution;
}

private final Map<Packaging, Platforms.PlatformAction> conditions = new HashMap<>();

public PackagingConditional forArchive(Platforms.PlatformAction action) {
conditions.put(Packaging.TAR, action);
conditions.put(Packaging.ZIP, action);
return this;
}

public PackagingConditional forPackage(Platforms.PlatformAction action) {
conditions.put(Packaging.RPM, action);
conditions.put(Packaging.DEB, action);
return this;
}

public PackagingConditional forDocker(Platforms.PlatformAction action) {
conditions.put(Packaging.DOCKER, action);
return this;
}

public void run() throws Exception {
HashSet<Packaging> missingPackaging = new HashSet<>(Arrays.asList(Packaging.values()));
missingPackaging.removeAll(conditions.keySet());
if (missingPackaging.isEmpty() == false) {
throw new IllegalArgumentException("No condition specified for " + missingPackaging);
}
conditions.get(this.distribution.packaging).run();
}
}

public PackagingConditional packagingConditional() {
return new PackagingConditional(this);
}

public enum Platform {
LINUX,
WINDOWS,
Expand Down
18 changes: 16 additions & 2 deletions qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,22 @@ public static Shell.Result runElasticsearchStartCommand(Shell sh) throws IOExcep
*/
public static void clearJournal(Shell sh) {
if (isSystemd()) {
sh.run("rm -rf /run/log/journal/");
sh.run("systemctl restart systemd-journald");
sh.run("rm -rf /run/log/journal/ /var/log/journal/");
final Result result = sh.runIgnoreExitCode("systemctl restart systemd-journald");

// Sometimes the restart fails on Debian 10 with:
// Job for systemd-journald.service failed because the control process exited with error code.
// See "systemctl status systemd-journald.service" and "journalctl -xe" for details.]
//
// ...so run these commands in an attempt to figure out what's going on.
if (result.isSuccess() == false) {
logger.error("Failed to restart systemd-journald: " + result);

logger.error(sh.runIgnoreExitCode("systemctl status systemd-journald.service"));
logger.error(sh.runIgnoreExitCode("journalctl -xe"));

fail("Couldn't clear the systemd journal as restarting systemd-journald failed");
}
}
}

Expand Down

0 comments on commit 3e06d4a

Please sign in to comment.