From 94d4862a7eeef16394d4485e99e3fa635cb1a8cc Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 1 Feb 2019 10:25:29 +1100 Subject: [PATCH] Fix exit code for Security CLI tools (#37956) The certgen, certutil and saml-metadata tools did not correctly return their exit code to the calling shell. These commands now explicitly exit with the code that was returned from the main(args, terminal) method. Backport of #38078 --- .../packaging/test/ArchiveTestCase.java | 16 +++++++++------- .../security/cli/CertificateGenerateTool.java | 3 +-- .../xpack/security/cli/CertificateTool.java | 2 +- .../security/authc/saml/SamlMetadataCommand.java | 2 +- .../test/resources/packaging/tests/certgen.bash | 9 +++++++++ 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java index c225bff80744c..6b6c5e3eb831f 100644 --- a/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java +++ b/qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java @@ -311,15 +311,17 @@ public void test90SecurityCliPackaging() { if (distribution().equals(Distribution.DEFAULT_TAR) || distribution().equals(Distribution.DEFAULT_ZIP)) { assertTrue(Files.exists(installation.lib.resolve("tools").resolve("security-cli"))); - Platforms.onLinux(() -> { - final Result result = sh.run(bin.elasticsearchCertutil + " help"); + final Platforms.PlatformAction action = () -> { + Result result = sh.run(bin.elasticsearchCertutil + " --help"); assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); - }); - Platforms.onWindows(() -> { - final Result result = sh.run(bin.elasticsearchCertutil + " help"); - assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack")); - }); + // Ensure that the exit code from the java command is passed back up through the shell script + result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command"); + assertThat(result.exitCode, is(64)); + assertThat(result.stdout, containsString("Unknown command [invalid-command]")); + }; + Platforms.onLinux(action); + Platforms.onWindows(action); } else if (distribution().equals(Distribution.OSS_TAR) || distribution().equals(Distribution.OSS_ZIP)) { assertFalse(Files.exists(installation.lib.resolve("tools").resolve("security-cli"))); } diff --git a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateGenerateTool.java b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateGenerateTool.java index 809e4a6d30524..4b30224dcd481 100644 --- a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateGenerateTool.java +++ b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateGenerateTool.java @@ -38,7 +38,6 @@ import org.elasticsearch.xpack.core.ssl.PemUtils; import javax.security.auth.x500.X500Principal; - import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; @@ -154,7 +153,7 @@ private static class InputFileParser { } public static void main(String[] args) throws Exception { - new CertificateGenerateTool().main(args, Terminal.DEFAULT); + exit(new CertificateGenerateTool().main(args, Terminal.DEFAULT)); } @Override diff --git a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java index a966cac9109d9..435305b8a6914 100644 --- a/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java +++ b/x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertificateTool.java @@ -134,7 +134,7 @@ private static class CertificateToolParser { public static void main(String[] args) throws Exception { - new CertificateTool().main(args, Terminal.DEFAULT); + exit(new CertificateTool().main(args, Terminal.DEFAULT)); } CertificateTool() { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java index 2a77508ac446c..b1f7bbc571ba2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java @@ -93,7 +93,7 @@ public class SamlMetadataCommand extends EnvironmentAwareCommand { private KeyStoreWrapper keyStoreWrapper; public static void main(String[] args) throws Exception { - new SamlMetadataCommand().main(args, Terminal.DEFAULT); + exit(new SamlMetadataCommand().main(args, Terminal.DEFAULT)); } public SamlMetadataCommand() { diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash index 678c3139e3742..2fb2a4014ec42 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash @@ -406,3 +406,12 @@ DATA_SETTINGS echo "$testSearch" | grep '"_index":"books"' echo "$testSearch" | grep '"_id":"0"' } + +@test "[$GROUP] exit code on failure" { + run sudo -E -u $MASTER_USER "$MASTER_HOME/bin/elasticsearch-certgen" --not-a-valid-option + [ "$status" -ne 0 ] || { + echo "Expected elasticsearch-certgen tool exit code to be non-zero" + echo "$output" + false + } +}