diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java index ffb9adee8e2..2acfbad5101 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java @@ -66,7 +66,7 @@ class ListPluginsCommand extends EnvironmentAwareCommand { PluginInfo info = PluginInfo.readFromProperties(env.pluginsFile().resolve(plugin)); terminal.println(Terminal.Verbosity.VERBOSE, info.toString(prefix)); if (info.getElasticsearchVersion().equals(Version.CURRENT) == false) { - terminal.println("WARNING: plugin [" + info.getName() + "] was built for Elasticsearch version " + info.getVersion() + + terminal.errorPrintln("WARNING: plugin [" + info.getName() + "] was built for Elasticsearch version " + info.getVersion() + " but version " + Version.CURRENT + " is required"); } } diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 8a890d8d7ff..45fbd3133d1 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -789,7 +789,7 @@ public class InstallPluginCommandTests extends ESTestCase { public void testBatchFlag() throws Exception { MockTerminal terminal = new MockTerminal(); installPlugin(terminal, true); - assertThat(terminal.getOutput(), containsString("WARNING: plugin requires additional permissions")); + assertThat(terminal.getErrorOutput(), containsString("WARNING: plugin requires additional permissions")); assertThat(terminal.getOutput(), containsString("-> Downloading")); // No progress bar in batch mode assertThat(terminal.getOutput(), not(containsString("100%"))); @@ -1225,7 +1225,7 @@ public class InstallPluginCommandTests extends ESTestCase { UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1())); assertEquals("installation aborted by user", e.getMessage()); - assertThat(terminal.getOutput(), containsString("WARNING: " + warning)); + assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning)); try (Stream fileStream = Files.list(env.v2().pluginsFile())) { assertThat(fileStream.collect(Collectors.toList()), empty()); } @@ -1238,7 +1238,7 @@ public class InstallPluginCommandTests extends ESTestCase { terminal.addTextInput("n"); e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1())); assertEquals("installation aborted by user", e.getMessage()); - assertThat(terminal.getOutput(), containsString("WARNING: " + warning)); + assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning)); try (Stream fileStream = Files.list(env.v2().pluginsFile())) { assertThat(fileStream.collect(Collectors.toList()), empty()); } @@ -1251,7 +1251,7 @@ public class InstallPluginCommandTests extends ESTestCase { } installPlugin(pluginZip, env.v1()); for (String warning : warnings) { - assertThat(terminal.getOutput(), containsString("WARNING: " + warning)); + assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning)); } } diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java index 8144c5f0600..bb839008d91 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java @@ -247,8 +247,11 @@ public class ListPluginsCommandTests extends ESTestCase { MockTerminal terminal = listPlugins(home); String message = "plugin [fake_plugin1] was built for Elasticsearch version 1.0 but version " + Version.CURRENT + " is required"; assertEquals( - "fake_plugin1\n" + "WARNING: " + message + "\n" + "fake_plugin2\n", - terminal.getOutput()); + "fake_plugin1\nfake_plugin2\n", + terminal.getOutput()); + assertEquals( + "WARNING: " + message + "\n", + terminal.getErrorOutput()); String[] params = {"-s"}; terminal = listPlugins(home, params); diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java index c62d37a4e28..40f17196472 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java @@ -237,11 +237,14 @@ public class RemovePluginCommandTests extends ESTestCase { return false; } }.main(new String[] { "-Epath.home=" + home, "fake" }, terminal); - try (BufferedReader reader = new BufferedReader(new StringReader(terminal.getOutput()))) { + try (BufferedReader reader = new BufferedReader(new StringReader(terminal.getOutput())); + BufferedReader errorReader = new BufferedReader(new StringReader(terminal.getErrorOutput())) + ) { assertEquals("-> removing [fake]...", reader.readLine()); assertEquals("ERROR: plugin [fake] not found; run 'elasticsearch-plugin list' to get list of installed plugins", - reader.readLine()); + errorReader.readLine()); assertNull(reader.readLine()); + assertNull(errorReader.readLine()); } } diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java index 34ede7ccf94..2a270153f47 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/Command.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/Command.java @@ -73,7 +73,7 @@ public abstract class Command implements Closeable { StringWriter sw = new StringWriter(); PrintWriter pw = new PrintWriter(sw)) { e.printStackTrace(pw); - terminal.println(sw.toString()); + terminal.errorPrintln(sw.toString()); } catch (final IOException impossible) { // StringWriter#close declares a checked IOException from the Closeable interface but the Javadocs for StringWriter // say that an exception here is impossible @@ -89,14 +89,15 @@ public abstract class Command implements Closeable { try { mainWithoutErrorHandling(args, terminal); } catch (OptionException e) { - printHelp(terminal); - terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + // print help to stderr on exceptions + printHelp(terminal, true); + terminal.errorPrintln(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); return ExitCodes.USAGE; } catch (UserException e) { if (e.exitCode == ExitCodes.USAGE) { - printHelp(terminal); + printHelp(terminal, true); } - terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); + terminal.errorPrintln(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage()); return e.exitCode; } return ExitCodes.OK; @@ -109,7 +110,7 @@ public abstract class Command implements Closeable { final OptionSet options = parser.parse(args); if (options.has(helpOption)) { - printHelp(terminal); + printHelp(terminal, false); return; } @@ -125,11 +126,17 @@ public abstract class Command implements Closeable { } /** Prints a help message for the command to the terminal. */ - private void printHelp(Terminal terminal) throws IOException { - terminal.println(description); - terminal.println(""); - printAdditionalHelp(terminal); - parser.printHelpOn(terminal.getWriter()); + private void printHelp(Terminal terminal, boolean toStdError) throws IOException { + if (toStdError) { + terminal.errorPrintln(description); + terminal.errorPrintln(""); + parser.printHelpOn(terminal.getErrorWriter()); + } else { + terminal.println(description); + terminal.println(""); + printAdditionalHelp(terminal); + parser.printHelpOn(terminal.getWriter()); + } } /** Prints additional help information, specific to the command */ diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java b/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java index a0ebff5d670..718b4796c02 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/Terminal.java @@ -39,9 +39,17 @@ import java.util.Locale; */ public abstract class Terminal { + /** Writer to standard error - not supplied by the {@link Console} API, so we share with subclasses */ + private static final PrintWriter ERROR_WRITER = newErrorWriter(); + /** The default terminal implementation, which will be a console if available, or stdout/stderr if not. */ public static final Terminal DEFAULT = ConsoleTerminal.isSupported() ? new ConsoleTerminal() : new SystemTerminal(); + @SuppressForbidden(reason = "Writer for System.err") + private static PrintWriter newErrorWriter() { + return new PrintWriter(System.err); + } + /** Defines the available verbosity levels of messages to be printed. */ public enum Verbosity { SILENT, /* always printed */ @@ -70,9 +78,14 @@ public abstract class Terminal { /** Reads password text from the terminal input. See {@link Console#readPassword()}}. */ public abstract char[] readSecret(String prompt); - /** Returns a Writer which can be used to write to the terminal directly. */ + /** Returns a Writer which can be used to write to the terminal directly using standard output. */ public abstract PrintWriter getWriter(); + /** Returns a Writer which can be used to write to the terminal directly using standard error. */ + public PrintWriter getErrorWriter() { + return ERROR_WRITER; + } + /** Prints a line to the terminal at {@link Verbosity#NORMAL} verbosity level. */ public final void println(String msg) { println(Verbosity.NORMAL, msg); @@ -83,14 +96,35 @@ public abstract class Terminal { print(verbosity, msg + lineSeparator); } - /** Prints message to the terminal at {@code verbosity} level, without a newline. */ + /** Prints message to the terminal's standard output at {@code verbosity} level, without a newline. */ public final void print(Verbosity verbosity, String msg) { + print(verbosity, msg, false); + } + + /** Prints message to the terminal at {@code verbosity} level, without a newline. */ + private void print(Verbosity verbosity, String msg, boolean isError) { if (isPrintable(verbosity)) { - getWriter().print(msg); - getWriter().flush(); + PrintWriter writer = isError ? getErrorWriter() : getWriter(); + writer.print(msg); + writer.flush(); } } + /** Prints a line to the terminal's standard error at {@link Verbosity#NORMAL} verbosity level, without a newline. */ + public final void errorPrint(Verbosity verbosity, String msg) { + print(verbosity, msg, true); + } + + /** Prints a line to the terminal's standard error at {@link Verbosity#NORMAL} verbosity level. */ + public final void errorPrintln(String msg) { + errorPrintln(Verbosity.NORMAL, msg); + } + + /** Prints a line to the terminal's standard error at {@code verbosity} level. */ + public final void errorPrintln(Verbosity verbosity, String msg) { + errorPrint(verbosity, msg + lineSeparator); + } + /** Checks if is enough {@code verbosity} level to be printed */ public final boolean isPrintable(Verbosity verbosity) { return this.verbosity.ordinal() >= verbosity.ordinal(); @@ -110,7 +144,7 @@ public abstract class Terminal { answer = answer.toLowerCase(Locale.ROOT); boolean answerYes = answer.equals("y"); if (answerYes == false && answer.equals("n") == false) { - println("Did not understand answer '" + answer + "'"); + errorPrintln("Did not understand answer '" + answer + "'"); continue; } return answerYes; @@ -165,7 +199,7 @@ public abstract class Terminal { @Override public String readText(String text) { - getWriter().print(text); + getErrorWriter().print(text); // prompts should go to standard error to avoid mixing with list output BufferedReader reader = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset())); try { final String line = reader.readLine(); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java index e670a4364fe..9e1d6e5710e 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java @@ -40,7 +40,7 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.OK, true, - output -> {}, + (output, error) -> {}, (foreground, pidFile, quiet, esSettings) -> { Settings settings = esSettings.settings(); assertThat(settings.keySet(), hasSize(2)); @@ -55,7 +55,7 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.OK, true, - output -> {}, + (output, error) -> {}, (foreground, pidFile, quiet, esSettings) -> { Settings settings = esSettings.settings(); assertThat(settings.keySet(), hasSize(2)); diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java index 2990101134f..824dd90ec22 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/cli/EvilCommandTests.java @@ -55,7 +55,7 @@ public class EvilCommandTests extends ESTestCase { command.getShutdownHookThread().run(); command.getShutdownHookThread().join(); assertTrue(closed.get()); - final String output = terminal.getOutput(); + final String output = terminal.getErrorOutput(); if (shouldThrow) { // ensure that we dump the exception assertThat(output, containsString("java.io.IOException: fail")); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index d06efb37a3d..0e76d4f3019 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -377,7 +377,7 @@ public class ArchiveTests extends PackagingTestCase { // 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(not(0))); - assertThat(result.stdout, containsString("Unknown command [invalid-command]")); + assertThat(result.stderr, containsString("Unknown command [invalid-command]")); }; Platforms.onLinux(action); Platforms.onWindows(action); diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginSecurity.java b/server/src/main/java/org/elasticsearch/plugins/PluginSecurity.java index d2246259ab7..c845ff4d3a5 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginSecurity.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginSecurity.java @@ -55,15 +55,15 @@ class PluginSecurity { // sort permissions in a reasonable order Collections.sort(requested); - terminal.println(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); - terminal.println(Verbosity.NORMAL, "@ WARNING: plugin requires additional permissions @"); - terminal.println(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); + terminal.errorPrintln(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); + terminal.errorPrintln(Verbosity.NORMAL, "@ WARNING: plugin requires additional permissions @"); + terminal.errorPrintln(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); // print all permissions: for (String permission : requested) { - terminal.println(Verbosity.NORMAL, "* " + permission); + terminal.errorPrintln(Verbosity.NORMAL, "* " + permission); } - terminal.println(Verbosity.NORMAL, "See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html"); - terminal.println(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks."); + terminal.errorPrintln(Verbosity.NORMAL, "See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html"); + terminal.errorPrintln(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks."); prompt(terminal, batch); } } diff --git a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java index 1f0a953a707..fb6925ba2eb 100644 --- a/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java +++ b/server/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java @@ -26,7 +26,7 @@ import org.elasticsearch.monitor.jvm.JvmInfo; import java.nio.file.Path; import java.util.Locale; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; @@ -53,15 +53,15 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { private void runTestThatVersionIsMutuallyExclusiveToOtherOptions(String... args) throws Exception { runTestVersion( ExitCodes.USAGE, - output -> assertThat( - output, + (output, error) -> assertThat( + error, allOf(containsString("ERROR:"), containsString("are unavailable given other options on the command line"))), args); } private void runTestThatVersionIsReturned(String... args) throws Exception { - runTestVersion(ExitCodes.OK, output -> { + runTestVersion(ExitCodes.OK, (output, error) -> { assertThat(output, containsString("Version: " + Build.CURRENT.getQualifiedVersion())); final String expectedBuildOutput = String.format( Locale.ROOT, @@ -75,7 +75,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { }, args); } - private void runTestVersion(int expectedStatus, Consumer outputConsumer, String... args) throws Exception { + private void runTestVersion(int expectedStatus, BiConsumer outputConsumer, String... args) throws Exception { runTest(expectedStatus, false, outputConsumer, (foreground, pidFile, quiet, esSettings) -> {}, args); } @@ -83,19 +83,19 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.USAGE, false, - output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")), + (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo]")), (foreground, pidFile, quiet, esSettings) -> {}, "foo"); runTest( ExitCodes.USAGE, false, - output -> assertThat(output, containsString("Positional arguments not allowed, found [foo, bar]")), + (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo, bar]")), (foreground, pidFile, quiet, esSettings) -> {}, "foo", "bar"); runTest( ExitCodes.USAGE, false, - output -> assertThat(output, containsString("Positional arguments not allowed, found [foo]")), + (output, error) -> assertThat(error, containsString("Positional arguments not allowed, found [foo]")), (foreground, pidFile, quiet, esSettings) -> {}, "-E", "foo=bar", "foo", "-E", "baz=qux"); } @@ -104,12 +104,12 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { Path tmpDir = createTempDir(); Path pidFile = tmpDir.resolve("pid"); runPidFileTest(ExitCodes.USAGE, false, - output -> assertThat(output, containsString("Option p/pidfile requires an argument")), pidFile, "-p"); - runPidFileTest(ExitCodes.OK, true, output -> {}, pidFile, "-p", pidFile.toString()); - runPidFileTest(ExitCodes.OK, true, output -> {}, pidFile, "--pidfile", tmpDir.toString() + "/pid"); + (output, error) -> assertThat(error, containsString("Option p/pidfile requires an argument")), pidFile, "-p"); + runPidFileTest(ExitCodes.OK, true, (output, error) -> {}, pidFile, "-p", pidFile.toString()); + runPidFileTest(ExitCodes.OK, true, (output, error) -> {}, pidFile, "--pidfile", tmpDir.toString() + "/pid"); } - private void runPidFileTest(final int expectedStatus, final boolean expectedInit, Consumer outputConsumer, + private void runPidFileTest(final int expectedStatus, final boolean expectedInit, BiConsumer outputConsumer, Path expectedPidFile, final String... args) throws Exception { runTest( @@ -130,7 +130,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.OK, true, - output -> {}, + (output, error) -> {}, (foreground, pidFile, quiet, esSettings) -> assertThat(foreground, equalTo(!expectedDaemonize)), args); } @@ -145,7 +145,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.OK, true, - output -> {}, + (output, error) -> {}, (foreground, pidFile, quiet, esSettings) -> assertThat(quiet, equalTo(expectedQuiet)), args); } @@ -154,7 +154,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.OK, true, - output -> {}, + (output, error) -> {}, (foreground, pidFile, quiet, env) -> { Settings settings = env.settings(); assertEquals("bar", settings.get("foo")); @@ -167,7 +167,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.USAGE, false, - output -> assertThat(output, containsString("setting [foo] must not be empty")), + (output, error) -> assertThat(error, containsString("setting [foo] must not be empty")), (foreground, pidFile, quiet, esSettings) -> {}, "-E", "foo="); } @@ -176,7 +176,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.USAGE, false, - output -> assertThat(output, containsString("setting [foo] already set, saw [bar] and [baz]")), + (output, error) -> assertThat(error, containsString("setting [foo] already set, saw [bar] and [baz]")), (foreground, pidFile, quiet, initialEnv) -> {}, "-E", "foo=bar", "-E", "foo=baz"); } @@ -185,7 +185,7 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { runTest( ExitCodes.USAGE, false, - output -> assertThat(output, containsString("network.host is not a recognized option")), + (output, error) -> assertThat(error, containsString("network.host is not a recognized option")), (foreground, pidFile, quiet, esSettings) -> {}, "--network.host"); } diff --git a/server/src/test/java/org/elasticsearch/cli/CommandTests.java b/server/src/test/java/org/elasticsearch/cli/CommandTests.java index 2b2437eea65..092e5dfd480 100644 --- a/server/src/test/java/org/elasticsearch/cli/CommandTests.java +++ b/server/src/test/java/org/elasticsearch/cli/CommandTests.java @@ -110,6 +110,31 @@ public class CommandTests extends ESTestCase { assertFalse(command.executed); } + public void testUnknownOptions() throws Exception { + NoopCommand command = new NoopCommand(); + MockTerminal terminal = new MockTerminal(); + String[] args = {"-Z"}; + int status = command.main(args, terminal); + String output = terminal.getOutput(); + String error = terminal.getErrorOutput(); + assertEquals(output, ExitCodes.USAGE, status); + assertTrue(error, error.contains("Does nothing")); + assertFalse(output, output.contains("Some extra help")); // extra help not printed for usage errors + assertTrue(error, error.contains("ERROR: Z is not a recognized option")); + assertFalse(command.executed); + + command = new NoopCommand(); + String[] args2 = {"--foobar"}; + status = command.main(args2, terminal); + output = terminal.getOutput(); + error = terminal.getErrorOutput(); + assertEquals(output, ExitCodes.USAGE, status); + assertTrue(error, error.contains("Does nothing")); + assertFalse(output, output.contains("Some extra help")); // extra help not printed for usage errors + assertTrue(error, error.contains("ERROR: Z is not a recognized option")); + assertFalse(command.executed); + } + public void testVerbositySilentAndVerbose() throws Exception { MockTerminal terminal = new MockTerminal(); NoopCommand command = new NoopCommand(); @@ -155,8 +180,9 @@ public class CommandTests extends ESTestCase { String[] args = {}; int status = command.main(args, terminal); String output = terminal.getOutput(); + String error = terminal.getErrorOutput(); assertEquals(output, ExitCodes.DATA_ERROR, status); - assertTrue(output, output.contains("ERROR: Bad input")); + assertTrue(error, error.contains("ERROR: Bad input")); } public void testUsageError() throws Exception { @@ -165,9 +191,10 @@ public class CommandTests extends ESTestCase { String[] args = {}; int status = command.main(args, terminal); String output = terminal.getOutput(); + String error = terminal.getErrorOutput(); assertEquals(output, ExitCodes.USAGE, status); - assertTrue(output, output.contains("Throws a usage error")); - assertTrue(output, output.contains("ERROR: something was no good")); + assertTrue(error, error.contains("Throws a usage error")); + assertTrue(error, error.contains("ERROR: something was no good")); } } diff --git a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java index 3b409c2add6..99bbe9d6184 100644 --- a/server/src/test/java/org/elasticsearch/cli/TerminalTests.java +++ b/server/src/test/java/org/elasticsearch/cli/TerminalTests.java @@ -41,6 +41,26 @@ public class TerminalTests extends ESTestCase { assertPrinted(terminal, Terminal.Verbosity.VERBOSE, "text"); } + public void testErrorVerbosity() throws Exception { + MockTerminal terminal = new MockTerminal(); + terminal.setVerbosity(Terminal.Verbosity.SILENT); + assertErrorPrinted(terminal, Terminal.Verbosity.SILENT, "text"); + assertErrorNotPrinted(terminal, Terminal.Verbosity.NORMAL, "text"); + assertErrorNotPrinted(terminal, Terminal.Verbosity.VERBOSE, "text"); + + terminal = new MockTerminal(); + assertErrorPrinted(terminal, Terminal.Verbosity.SILENT, "text"); + assertErrorPrinted(terminal, Terminal.Verbosity.NORMAL, "text"); + assertErrorNotPrinted(terminal, Terminal.Verbosity.VERBOSE, "text"); + + terminal = new MockTerminal(); + terminal.setVerbosity(Terminal.Verbosity.VERBOSE); + assertErrorPrinted(terminal, Terminal.Verbosity.SILENT, "text"); + assertErrorPrinted(terminal, Terminal.Verbosity.NORMAL, "text"); + assertErrorPrinted(terminal, Terminal.Verbosity.VERBOSE, "text"); + } + + public void testEscaping() throws Exception { MockTerminal terminal = new MockTerminal(); assertPrinted(terminal, Terminal.Verbosity.NORMAL, "This message contains percent like %20n"); @@ -87,4 +107,18 @@ public class TerminalTests extends ESTestCase { String output = logTerminal.getOutput(); assertTrue(output, output.isEmpty()); } + + private void assertErrorPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception { + logTerminal.errorPrintln(verbosity, text); + String output = logTerminal.getErrorOutput(); + assertTrue(output, output.contains(text)); + logTerminal.reset(); + } + + private void assertErrorNotPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception { + logTerminal.errorPrintln(verbosity, text); + String output = logTerminal.getErrorOutput(); + assertTrue(output, output.isEmpty()); + } + } diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java index 62b2b422f78..d1a2cbb9cc4 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java @@ -28,7 +28,7 @@ import org.elasticsearch.test.ESTestCase; import java.nio.file.Path; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import static org.hamcrest.CoreMatchers.equalTo; @@ -41,7 +41,7 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase { void runTest( final int expectedStatus, final boolean expectedInit, - final Consumer outputConsumer, + final BiConsumer outputConsumer, final InitConsumer initConsumer, final String... args) throws Exception { final MockTerminal terminal = new MockTerminal(); @@ -69,11 +69,12 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase { }, terminal); assertThat(status, equalTo(expectedStatus)); assertThat(init.get(), equalTo(expectedInit)); - outputConsumer.accept(terminal.getOutput()); + outputConsumer.accept(terminal.getOutput(), terminal.getErrorOutput()); } catch (Exception e) { // if an unexpected exception is thrown, we log // terminal output to aid debugging - logger.info(terminal.getOutput()); + logger.info("Stdout:\n" + terminal.getOutput()); + logger.info("Stderr:\n" + terminal.getErrorOutput()); // rethrow so the test fails throw e; } diff --git a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java index 44c968cf507..cff5c1b49fb 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java @@ -33,8 +33,10 @@ import java.util.List; */ public class MockTerminal extends Terminal { - private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - private final PrintWriter writer = new PrintWriter(new OutputStreamWriter(buffer, StandardCharsets.UTF_8)); + private final ByteArrayOutputStream stdoutBuffer = new ByteArrayOutputStream(); + private final ByteArrayOutputStream stderrBuffer = new ByteArrayOutputStream(); + private final PrintWriter writer = new PrintWriter(new OutputStreamWriter(stdoutBuffer, StandardCharsets.UTF_8)); + private final PrintWriter errorWriter = new PrintWriter(new OutputStreamWriter(stderrBuffer, StandardCharsets.UTF_8)); // A deque would be a perfect data structure for the FIFO queue of input values needed here. However, // to support the valid return value of readText being null (defined by Console), we need to be able @@ -73,6 +75,11 @@ public class MockTerminal extends Terminal { return writer; } + @Override + public PrintWriter getErrorWriter() { + return errorWriter; + } + /** Adds an an input that will be return from {@link #readText(String)}. Values are read in FIFO order. */ public void addTextInput(String input) { textInput.add(input); @@ -85,12 +92,18 @@ public class MockTerminal extends Terminal { /** Returns all output written to this terminal. */ public String getOutput() throws UnsupportedEncodingException { - return buffer.toString("UTF-8"); + return stdoutBuffer.toString("UTF-8"); + } + + /** Returns all output written to this terminal. */ + public String getErrorOutput() throws UnsupportedEncodingException { + return stderrBuffer.toString("UTF-8"); } /** Wipes the input and output. */ public void reset() { - buffer.reset(); + stdoutBuffer.reset(); + stderrBuffer.reset(); textIndex = 0; textInput.clear(); secretIndex = 0; 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 4b30224dcd4..a100afe33aa 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 @@ -285,12 +285,12 @@ public class CertificateGenerateTool extends EnvironmentAwareCommand { final List errors = certInfo.validate(); if (errors.size() > 0) { hasError = true; - terminal.println(Terminal.Verbosity.SILENT, "Configuration for instance " + certInfo.name.originalName + terminal.errorPrintln(Terminal.Verbosity.SILENT, "Configuration for instance " + certInfo.name.originalName + " has invalid details"); for (String message : errors) { - terminal.println(Terminal.Verbosity.SILENT, " * " + message); + terminal.errorPrintln(Terminal.Verbosity.SILENT, " * " + message); } - terminal.println(""); + terminal.errorPrintln(""); } } if (hasError) { 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 435305b8a69..53e3fadf168 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 @@ -417,7 +417,7 @@ public class CertificateTool extends LoggingAwareMultiCommand { if (validationErrors.isEmpty()) { return Collections.singleton(information); } else { - validationErrors.forEach(terminal::println); + validationErrors.forEach(terminal::errorPrintln); return Collections.emptyList(); } } @@ -477,7 +477,7 @@ public class CertificateTool extends LoggingAwareMultiCommand { if (Name.isValidFilename(filename)) { return filename; } else { - terminal.println(Terminal.Verbosity.SILENT, "'" + filename + "' is not a valid filename"); + terminal.errorPrintln(Terminal.Verbosity.SILENT, "'" + filename + "' is not a valid filename"); continue; } } @@ -891,11 +891,12 @@ public class CertificateTool extends LoggingAwareMultiCommand { final List errors = certInfo.validate(); if (errors.size() > 0) { hasError = true; - terminal.println(Verbosity.SILENT, "Configuration for instance " + certInfo.name.originalName + " has invalid details"); + terminal.errorPrintln(Verbosity.SILENT, "Configuration for instance " + certInfo.name.originalName + + " has invalid details"); for (String message : errors) { - terminal.println(Verbosity.SILENT, " * " + message); + terminal.errorPrintln(Verbosity.SILENT, " * " + message); } - terminal.println(""); + terminal.errorPrintln(""); } } if (hasError) { @@ -961,7 +962,7 @@ public class CertificateTool extends LoggingAwareMultiCommand { return; } if (Files.exists(parent)) { - terminal.println(Terminal.Verbosity.SILENT, "Path " + parent + " exists, but is not a directory. Cannot write to " + path); + terminal.errorPrintln(Terminal.Verbosity.SILENT, "Path " + parent + " exists, but is not a directory. Cannot write to " + path); throw new UserException(ExitCodes.CANT_CREATE, "Cannot write to " + path); } if (terminal.promptYesNo("Directory " + parent + " does not exist. Do you want to create it?", true)) { diff --git a/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java b/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java index 9e970ea559a..6845edbdc6b 100644 --- a/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java +++ b/x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertificateToolTests.java @@ -242,8 +242,8 @@ public class CertificateToolTests extends ESTestCase { () -> CertificateTool.parseAndValidateFile(terminal, instanceFile)); assertThat(exception.getMessage(), containsString("invalid configuration")); assertThat(exception.getMessage(), containsString(instanceFile.toString())); - assertThat(terminal.getOutput(), containsString("THIS=not a,valid DN")); - assertThat(terminal.getOutput(), containsString("could not be converted to a valid DN")); + assertThat(terminal.getErrorOutput(), containsString("THIS=not a,valid DN")); + assertThat(terminal.getErrorOutput(), containsString("could not be converted to a valid DN")); } public void testGeneratingCsr() throws Exception { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java index 866f3722e6e..5ac81a06480 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java @@ -195,15 +195,15 @@ public class SetupPasswordTool extends LoggingAwareMultiCommand { SecureString password1 = new SecureString(terminal.readSecret("Enter password for [" + user + "]: ")); Validation.Error err = Validation.Users.validatePassword(password1); if (err != null) { - terminal.println(err.toString()); - terminal.println("Try again."); + terminal.errorPrintln(err.toString()); + terminal.errorPrintln("Try again."); password1.close(); continue; } try (SecureString password2 = new SecureString(terminal.readSecret("Reenter password for [" + user + "]: "))) { if (password1.equals(password2) == false) { - terminal.println("Passwords do not match."); - terminal.println("Try again."); + terminal.errorPrintln("Passwords do not match."); + terminal.errorPrintln("Try again."); password1.close(); continue; } @@ -302,53 +302,55 @@ public class SetupPasswordTool extends LoggingAwareMultiCommand { // keystore password is not valid if (httpCode == HttpURLConnection.HTTP_UNAUTHORIZED) { - terminal.println(""); - terminal.println("Failed to authenticate user '" + elasticUser + "' against " + route.toString()); - terminal.println("Possible causes include:"); - terminal.println(" * The password for the '" + elasticUser + "' user has already been changed on this cluster"); - terminal.println(" * Your elasticsearch node is running against a different keystore"); - terminal.println(" This tool used the keystore at " + KeyStoreWrapper.keystorePath(env.configFile())); - terminal.println(""); + terminal.errorPrintln(""); + terminal.errorPrintln("Failed to authenticate user '" + elasticUser + "' against " + route.toString()); + terminal.errorPrintln("Possible causes include:"); + terminal.errorPrintln(" * The password for the '" + elasticUser + "' user has already been changed on this cluster"); + terminal.errorPrintln(" * Your elasticsearch node is running against a different keystore"); + terminal.errorPrintln(" This tool used the keystore at " + KeyStoreWrapper.keystorePath(env.configFile())); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "Failed to verify bootstrap password"); } else if (httpCode != HttpURLConnection.HTTP_OK) { - terminal.println(""); - terminal.println("Unexpected response code [" + httpCode + "] from calling GET " + route.toString()); + terminal.errorPrintln(""); + terminal.errorPrintln("Unexpected response code [" + httpCode + "] from calling GET " + route.toString()); XPackSecurityFeatureConfig xPackSecurityFeatureConfig = getXPackSecurityConfig(terminal); if (xPackSecurityFeatureConfig.isAvailable == false) { - terminal.println("It doesn't look like the X-Pack security feature is available on this Elasticsearch node."); - terminal.println("Please check if you have installed a license that allows access to X-Pack Security feature."); - terminal.println(""); + terminal.errorPrintln("It doesn't look like the X-Pack security feature is available on this Elasticsearch node."); + terminal.errorPrintln("Please check if you have installed a license that allows access to " + + "X-Pack Security feature."); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "X-Pack Security is not available."); } if (xPackSecurityFeatureConfig.isEnabled == false) { - terminal.println("It doesn't look like the X-Pack security feature is enabled on this Elasticsearch node."); - terminal.println("Please check if you have enabled X-Pack security in your elasticsearch.yml configuration file."); - terminal.println(""); + terminal.errorPrintln("It doesn't look like the X-Pack security feature is enabled on this Elasticsearch node."); + terminal.errorPrintln("Please check if you have enabled X-Pack security in your elasticsearch.yml " + + "configuration file."); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "X-Pack Security is disabled by configuration."); } - terminal.println("X-Pack security feature is available and enabled on this Elasticsearch node."); - terminal.println("Possible causes include:"); - terminal.println(" * The relative path of the URL is incorrect. Is there a proxy in-between?"); - terminal.println(" * The protocol (http/https) does not match the port."); - terminal.println(" * Is this really an Elasticsearch server?"); - terminal.println(""); + terminal.errorPrintln("X-Pack security feature is available and enabled on this Elasticsearch node."); + terminal.errorPrintln("Possible causes include:"); + terminal.errorPrintln(" * The relative path of the URL is incorrect. Is there a proxy in-between?"); + terminal.errorPrintln(" * The protocol (http/https) does not match the port."); + terminal.errorPrintln(" * Is this really an Elasticsearch server?"); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "Unknown error"); } } catch (SSLException e) { - terminal.println(""); - terminal.println("SSL connection to " + route.toString() + " failed: " + e.getMessage()); - terminal.println("Please check the elasticsearch SSL settings under " + XPackSettings.HTTP_SSL_PREFIX); - terminal.println(Verbosity.VERBOSE, ""); - terminal.println(Verbosity.VERBOSE, ExceptionsHelper.stackTrace(e)); - terminal.println(""); + terminal.errorPrintln(""); + terminal.errorPrintln("SSL connection to " + route.toString() + " failed: " + e.getMessage()); + terminal.errorPrintln("Please check the elasticsearch SSL settings under " + XPackSettings.HTTP_SSL_PREFIX); + terminal.errorPrintln(Verbosity.VERBOSE, ""); + terminal.errorPrintln(Verbosity.VERBOSE, ExceptionsHelper.stackTrace(e)); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "Failed to establish SSL connection to elasticsearch at " + route.toString() + ". ", e); } catch (IOException e) { - terminal.println(""); - terminal.println("Connection failure to: " + route.toString() + " failed: " + e.getMessage()); - terminal.println(Verbosity.VERBOSE, ""); - terminal.println(Verbosity.VERBOSE, ExceptionsHelper.stackTrace(e)); - terminal.println(""); + terminal.errorPrintln(""); + terminal.errorPrintln("Connection failure to: " + route.toString() + " failed: " + e.getMessage()); + terminal.errorPrintln(Verbosity.VERBOSE, ""); + terminal.errorPrintln(Verbosity.VERBOSE, ExceptionsHelper.stackTrace(e)); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "Failed to connect to elasticsearch at " + route.toString() + ". Is the URL correct and elasticsearch running?", e); } @@ -361,19 +363,20 @@ public class SetupPasswordTool extends LoggingAwareMultiCommand { final HttpResponse httpResponse = client.execute("GET", route, elasticUser, elasticUserPassword, () -> null, is -> responseBuilder(is, terminal)); if (httpResponse.getHttpStatus() != HttpURLConnection.HTTP_OK) { - terminal.println(""); - terminal.println("Unexpected response code [" + httpResponse.getHttpStatus() + "] from calling GET " + route.toString()); + terminal.errorPrintln(""); + terminal.errorPrintln("Unexpected response code [" + httpResponse.getHttpStatus() + "] from calling GET " + + route.toString()); if (httpResponse.getHttpStatus() == HttpURLConnection.HTTP_BAD_REQUEST) { - terminal.println("It doesn't look like the X-Pack is available on this Elasticsearch node."); - terminal.println("Please check that you have followed all installation instructions and that this tool"); - terminal.println(" is pointing to the correct Elasticsearch server."); - terminal.println(""); + terminal.errorPrintln("It doesn't look like the X-Pack is available on this Elasticsearch node."); + terminal.errorPrintln("Please check that you have followed all installation instructions and that this tool"); + terminal.errorPrintln(" is pointing to the correct Elasticsearch server."); + terminal.errorPrintln(""); throw new UserException(ExitCodes.CONFIG, "X-Pack is not available on this Elasticsearch node."); } else { - terminal.println("* Try running this tool again."); - terminal.println("* Verify that the tool is pointing to the correct Elasticsearch server."); - terminal.println("* Check the elasticsearch logs for additional error details."); - terminal.println(""); + terminal.errorPrintln("* Try running this tool again."); + terminal.errorPrintln("* Verify that the tool is pointing to the correct Elasticsearch server."); + terminal.errorPrintln("* Check the elasticsearch logs for additional error details."); + terminal.errorPrintln(""); throw new UserException(ExitCodes.TEMP_FAILURE, "Failed to determine x-pack security feature configuration."); } } @@ -406,33 +409,34 @@ public class SetupPasswordTool extends LoggingAwareMultiCommand { final HttpResponse httpResponse = client.execute("GET", route, elasticUser, elasticUserPassword, () -> null, is -> responseBuilder(is, terminal)); if (httpResponse.getHttpStatus() != HttpURLConnection.HTTP_OK) { - terminal.println(""); - terminal.println("Failed to determine the health of the cluster running at " + url); - terminal.println("Unexpected response code [" + httpResponse.getHttpStatus() + "] from calling GET " + route.toString()); + terminal.errorPrintln(""); + terminal.errorPrintln("Failed to determine the health of the cluster running at " + url); + terminal.errorPrintln("Unexpected response code [" + httpResponse.getHttpStatus() + "] from calling GET " + + route.toString()); final String cause = getErrorCause(httpResponse); if (cause != null) { - terminal.println("Cause: " + cause); + terminal.errorPrintln("Cause: " + cause); } } else { final String clusterStatus = Objects.toString(httpResponse.getResponseBody().get("status"), ""); if (clusterStatus.isEmpty()) { - terminal.println(""); - terminal.println("Failed to determine the health of the cluster running at " + url); - terminal.println("Could not find a 'status' value at " + route.toString()); + terminal.errorPrintln(""); + terminal.errorPrintln("Failed to determine the health of the cluster running at " + url); + terminal.errorPrintln("Could not find a 'status' value at " + route.toString()); } else if ("red".equalsIgnoreCase(clusterStatus)) { - terminal.println(""); - terminal.println("Your cluster health is currently RED."); - terminal.println("This means that some cluster data is unavailable and your cluster is not fully functional."); + terminal.errorPrintln(""); + terminal.errorPrintln("Your cluster health is currently RED."); + terminal.errorPrintln("This means that some cluster data is unavailable and your cluster is not fully functional."); } else { // Cluster is yellow/green -> all OK return; } } - terminal.println(""); - terminal.println( + terminal.errorPrintln(""); + terminal.errorPrintln( "It is recommended that you resolve the issues with your cluster before running elasticsearch-setup-passwords."); - terminal.println("It is very likely that the password changes will fail when run against an unhealthy cluster."); - terminal.println(""); + terminal.errorPrintln("It is very likely that the password changes will fail when run against an unhealthy cluster."); + terminal.errorPrintln(""); if (shouldPrompt) { final boolean keepGoing = terminal.promptYesNo("Do you want to continue with the password setup process", false); if (keepGoing == false) { @@ -465,28 +469,28 @@ public class SetupPasswordTool extends LoggingAwareMultiCommand { } }, is -> responseBuilder(is, terminal)); if (httpResponse.getHttpStatus() != HttpURLConnection.HTTP_OK) { - terminal.println(""); - terminal.println( + terminal.errorPrintln(""); + terminal.errorPrintln( "Unexpected response code [" + httpResponse.getHttpStatus() + "] from calling PUT " + route.toString()); String cause = getErrorCause(httpResponse); if (cause != null) { - terminal.println("Cause: " + cause); - terminal.println(""); + terminal.errorPrintln("Cause: " + cause); + terminal.errorPrintln(""); } - terminal.println("Possible next steps:"); - terminal.println("* Try running this tool again."); - terminal.println("* Try running with the --verbose parameter for additional messages."); - terminal.println("* Check the elasticsearch logs for additional error details."); - terminal.println("* Use the change password API manually. "); - terminal.println(""); + terminal.errorPrintln("Possible next steps:"); + terminal.errorPrintln("* Try running this tool again."); + terminal.errorPrintln("* Try running with the --verbose parameter for additional messages."); + terminal.errorPrintln("* Check the elasticsearch logs for additional error details."); + terminal.errorPrintln("* Use the change password API manually. "); + terminal.errorPrintln(""); throw new UserException(ExitCodes.TEMP_FAILURE, "Failed to set password for user [" + user + "]."); } } catch (IOException e) { - terminal.println(""); - terminal.println("Connection failure to: " + route.toString() + " failed: " + e.getMessage()); - terminal.println(Verbosity.VERBOSE, ""); - terminal.println(Verbosity.VERBOSE, ExceptionsHelper.stackTrace(e)); - terminal.println(""); + terminal.errorPrintln(""); + terminal.errorPrintln("Connection failure to: " + route.toString() + " failed: " + e.getMessage()); + terminal.errorPrintln(Verbosity.VERBOSE, ""); + terminal.errorPrintln(Verbosity.VERBOSE, ExceptionsHelper.stackTrace(e)); + terminal.errorPrintln(""); throw new UserException(ExitCodes.TEMP_FAILURE, "Failed to set password for user [" + user + "].", e); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java index 6d51fc5df93..03bed43499d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/tool/UsersTool.java @@ -475,11 +475,11 @@ public class UsersTool extends LoggingAwareMultiCommand { Set knownRoles = Sets.union(FileRolesStore.parseFileForRoleNames(rolesFile, null), ReservedRolesStore.names()); Set unknownRoles = Sets.difference(Sets.newHashSet(roles), knownRoles); if (!unknownRoles.isEmpty()) { - terminal.println(String.format(Locale.ROOT, "Warning: The following roles [%s] are not in the [%s] file. Make sure the names " + - "are correct. If the names are correct and the roles were created using the API please disregard this message. " + - "Nonetheless the user will still be associated with all specified roles", + terminal.errorPrintln(String.format(Locale.ROOT, "Warning: The following roles [%s] are not in the [%s] file. " + + "Make sure the names are correct. If the names are correct and the roles were created using the API please " + + "disregard this message. Nonetheless the user will still be associated with all specified roles", Strings.collectionToCommaDelimitedString(unknownRoles), rolesFile.toAbsolutePath())); - terminal.println("Known roles: " + knownRoles.toString()); + terminal.errorPrintln("Known roles: " + knownRoles.toString()); } } 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 a60b2204095..68be01a2e3f 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 @@ -224,7 +224,7 @@ public class SamlMetadataCommand extends EnvironmentAwareCommand { if (ContactInfo.TYPES.containsKey(type)) { break; } else { - terminal.println("Type '" + type + "' is not valid. Valid values are " + terminal.errorPrintln("Type '" + type + "' is not valid. Valid values are " + Strings.collectionToCommaDelimitedString(ContactInfo.TYPES.keySet())); } } @@ -263,8 +263,8 @@ public class SamlMetadataCommand extends EnvironmentAwareCommand { } else { errorMessage = "Error building signing credentials from provided keyPair"; } - terminal.println(Terminal.Verbosity.SILENT, errorMessage); - terminal.println("The following errors were found:"); + terminal.errorPrintln(Terminal.Verbosity.SILENT, errorMessage); + terminal.errorPrintln("The following errors were found:"); printExceptions(terminal, e); throw new UserException(ExitCodes.CANT_CREATE, "Unable to create metadata document"); } @@ -351,15 +351,16 @@ public class SamlMetadataCommand extends EnvironmentAwareCommand { SamlUtils.validate(xmlInput, METADATA_SCHEMA); terminal.println(Terminal.Verbosity.VERBOSE, "The generated metadata file conforms to the SAML metadata schema"); } catch (SAXException e) { - terminal.println(Terminal.Verbosity.SILENT, "Error - The generated metadata file does not conform to the SAML metadata schema"); - terminal.println("While validating " + xml.toString() + " the follow errors were found:"); + terminal.errorPrintln(Terminal.Verbosity.SILENT, "Error - The generated metadata file does not conform to the " + + "SAML metadata schema"); + terminal.errorPrintln("While validating " + xml.toString() + " the follow errors were found:"); printExceptions(terminal, e); throw new UserException(ExitCodes.CODE_ERROR, "Generated metadata is not valid"); } } private void printExceptions(Terminal terminal, Throwable throwable) { - terminal.println(" - " + throwable.getMessage()); + terminal.errorPrintln(" - " + throwable.getMessage()); for (Throwable sup : throwable.getSuppressed()) { printExceptions(terminal, sup); } @@ -453,10 +454,10 @@ public class SamlMetadataCommand extends EnvironmentAwareCommand { throw new UserException(ExitCodes.CONFIG, "There is no SAML realm configured in " + env.configFile()); } if (saml.size() > 1) { - terminal.println("Using configuration in " + env.configFile()); - terminal.println("Found multiple SAML realms: " + terminal.errorPrintln("Using configuration in " + env.configFile()); + terminal.errorPrintln("Found multiple SAML realms: " + saml.stream().map(Map.Entry::getKey).map(Object::toString).collect(Collectors.joining(", "))); - terminal.println("Use the -" + optionName(realmSpec) + " option to specify an explicit realm"); + terminal.errorPrintln("Use the -" + optionName(realmSpec) + " option to specify an explicit realm"); throw new UserException(ExitCodes.CONFIG, "Found multiple SAML realms, please specify one with '-" + optionName(realmSpec) + "'"); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FileAttributesChecker.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FileAttributesChecker.java index 6ef8461db82..5dd7c2b9cfb 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FileAttributesChecker.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/FileAttributesChecker.java @@ -51,19 +51,19 @@ public class FileAttributesChecker { PosixFileAttributes newAttributes = view.readAttributes(); PosixFileAttributes oldAttributes = attributes[i]; if (oldAttributes.permissions().equals(newAttributes.permissions()) == false) { - terminal.println(Terminal.Verbosity.SILENT, "WARNING: The file permissions of [" + paths[i] + "] have changed " + terminal.errorPrintln(Terminal.Verbosity.SILENT, "WARNING: The file permissions of [" + paths[i] + "] have changed " + "from [" + PosixFilePermissions.toString(oldAttributes.permissions()) + "] " + "to [" + PosixFilePermissions.toString(newAttributes.permissions()) + "]"); - terminal.println(Terminal.Verbosity.SILENT, + terminal.errorPrintln(Terminal.Verbosity.SILENT, "Please ensure that the user account running Elasticsearch has read access to this file!"); } if (oldAttributes.owner().getName().equals(newAttributes.owner().getName()) == false) { - terminal.println(Terminal.Verbosity.SILENT, "WARNING: Owner of file [" + paths[i] + "] " + terminal.errorPrintln(Terminal.Verbosity.SILENT, "WARNING: Owner of file [" + paths[i] + "] " + "used to be [" + oldAttributes.owner().getName() + "], " + "but now is [" + newAttributes.owner().getName() + "]"); } if (oldAttributes.group().getName().equals(newAttributes.group().getName()) == false) { - terminal.println(Terminal.Verbosity.SILENT, "WARNING: Group of file [" + paths[i] + "] " + terminal.errorPrintln(Terminal.Verbosity.SILENT, "WARNING: Group of file [" + paths[i] + "] " + "used to be [" + oldAttributes.group().getName() + "], " + "but now is [" + newAttributes.group().getName() + "]"); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java index e93950739a1..6e821720069 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java @@ -350,7 +350,7 @@ public class SetupPasswordToolTests extends CommandTestCase { fail("Should have thrown exception"); } catch (UserException e) { assertEquals(ExitCodes.OK, e.exitCode); - assertThat(terminal.getOutput(), Matchers.containsString("Your cluster health is currently RED.")); + assertThat(terminal.getErrorOutput(), Matchers.containsString("Your cluster health is currently RED.")); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java index 367921ad763..734ea0be0d4 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java @@ -165,9 +165,9 @@ public class SamlMetadataCommandTests extends SamlTestCase { final UserException userException = expectThrows(UserException.class, () -> command.buildEntityDescriptor(terminal, options, env)); assertThat(userException.getMessage(), containsString("multiple SAML realms")); - assertThat(terminal.getOutput(), containsString("saml_a")); - assertThat(terminal.getOutput(), containsString("saml_b")); - assertThat(terminal.getOutput(), containsString("Use the -realm option")); + assertThat(terminal.getErrorOutput(), containsString("saml_a")); + assertThat(terminal.getErrorOutput(), containsString("saml_b")); + assertThat(terminal.getErrorOutput(), containsString("Use the -realm option")); } public void testSpecifyRealmNameAsParameter() throws Exception { @@ -423,7 +423,7 @@ public class SamlMetadataCommandTests extends SamlTestCase { final UserException userException = expectThrows(UserException.class, () -> command.possiblySignDescriptor(terminal, options, descriptor, env)); assertThat(userException.getMessage(), containsString("Unable to create metadata document")); - assertThat(terminal.getOutput(), containsString("Error parsing Private Key from")); + assertThat(terminal.getErrorOutput(), containsString("Error parsing Private Key from")); } public void testSigningMetadataWithPem() throws Exception { diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java index 09ef53052b1..47b0f4697dc 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/authc/file/tool/UsersToolTests.java @@ -299,7 +299,7 @@ public class UsersToolTests extends CommandTestCase { public void testParseUnknownRole() throws Exception { UsersTool.parseRoles(terminal, TestEnvironment.newEnvironment(settings), "test_r1,r2,r3"); - String output = terminal.getOutput(); + String output = terminal.getErrorOutput(); assertTrue(output, output.contains("The following roles [r2,r3] are not in the [")); } diff --git a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/support/FileAttributesCheckerTests.java b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/support/FileAttributesCheckerTests.java index 12ae440e8f7..af2959410fd 100644 --- a/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/support/FileAttributesCheckerTests.java +++ b/x-pack/qa/security-tools-tests/src/test/java/org/elasticsearch/xpack/security/support/FileAttributesCheckerTests.java @@ -28,6 +28,7 @@ public class FileAttributesCheckerTests extends ESTestCase { MockTerminal terminal = new MockTerminal(); checker.check(terminal); assertTrue(terminal.getOutput(), terminal.getOutput().isEmpty()); + assertTrue(terminal.getErrorOutput(), terminal.getErrorOutput().isEmpty()); } public void testNoPosix() throws Exception { @@ -38,6 +39,7 @@ public class FileAttributesCheckerTests extends ESTestCase { MockTerminal terminal = new MockTerminal(); checker.check(terminal); assertTrue(terminal.getOutput(), terminal.getOutput().isEmpty()); + assertTrue(terminal.getErrorOutput(), terminal.getErrorOutput().isEmpty()); } } @@ -51,6 +53,7 @@ public class FileAttributesCheckerTests extends ESTestCase { MockTerminal terminal = new MockTerminal(); checker.check(terminal); assertTrue(terminal.getOutput(), terminal.getOutput().isEmpty()); + assertTrue(terminal.getErrorOutput(), terminal.getErrorOutput().isEmpty()); } } @@ -71,7 +74,7 @@ public class FileAttributesCheckerTests extends ESTestCase { MockTerminal terminal = new MockTerminal(); checker.check(terminal); - String output = terminal.getOutput(); + String output = terminal.getErrorOutput(); assertTrue(output, output.contains("permissions of [" + path + "] have changed")); } } @@ -89,7 +92,7 @@ public class FileAttributesCheckerTests extends ESTestCase { MockTerminal terminal = new MockTerminal(); checker.check(terminal); - String output = terminal.getOutput(); + String output = terminal.getErrorOutput(); assertTrue(output, output.contains("Owner of file [" + path + "] used to be")); } } @@ -107,7 +110,7 @@ public class FileAttributesCheckerTests extends ESTestCase { MockTerminal terminal = new MockTerminal(); checker.check(terminal); - String output = terminal.getOutput(); + String output = terminal.getErrorOutput(); assertTrue(output, output.contains("Group of file [" + path + "] used to be")); } } diff --git a/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/S3CleanupTests.java b/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/S3CleanupTests.java index 3358eedf4c8..5a80103f0b5 100644 --- a/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/S3CleanupTests.java +++ b/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/S3CleanupTests.java @@ -155,6 +155,7 @@ public class S3CleanupTests extends ESSingleNodeTestCase { } } finally { logger.info("Cleanup command output:\n" + terminal.getOutput()); + logger.info("Cleanup command standard error:\n" + terminal.getErrorOutput()); } return terminal;