Browse Source

Merge pull request #17024 from rjernst/cli-parsing

Cli: Switch to jopt-simple
Ryan Ernst 9 years ago
parent
commit
5f3d0067f8
55 changed files with 882 additions and 2461 deletions
  1. 1 1
      core/build.gradle
  2. 13 21
      core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java
  3. 0 183
      core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java
  4. 95 0
      core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java
  5. 1 1
      core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java
  6. 112 0
      core/src/main/java/org/elasticsearch/cli/Command.java
  7. 42 0
      core/src/main/java/org/elasticsearch/cli/ExitCodes.java
  8. 71 0
      core/src/main/java/org/elasticsearch/cli/MultiCommand.java
  9. 10 10
      core/src/main/java/org/elasticsearch/cli/Terminal.java
  10. 5 5
      core/src/main/java/org/elasticsearch/cli/UserError.java
  11. 0 138
      core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java
  12. 0 250
      core/src/main/java/org/elasticsearch/common/cli/CliTool.java
  13. 0 302
      core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java
  14. 0 57
      core/src/main/java/org/elasticsearch/common/cli/HelpPrinter.java
  15. 1 1
      core/src/main/java/org/elasticsearch/common/logging/TerminalAppender.java
  16. 1 1
      core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
  17. 59 34
      core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java
  18. 10 9
      core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java
  19. 14 91
      core/src/main/java/org/elasticsearch/plugins/PluginCli.java
  20. 2 2
      core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java
  21. 32 19
      core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java
  22. 0 28
      core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-start.help
  23. 0 16
      core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-version.help
  24. 0 22
      core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch.help
  25. 0 59
      core/src/main/resources/org/elasticsearch/plugins/plugin-install.help
  26. 0 12
      core/src/main/resources/org/elasticsearch/plugins/plugin-list.help
  27. 0 12
      core/src/main/resources/org/elasticsearch/plugins/plugin-remove.help
  28. 0 24
      core/src/main/resources/org/elasticsearch/plugins/plugin.help
  29. 123 0
      core/src/test/java/org/elasticsearch/cli/CommandTests.java
  30. 105 0
      core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java
  31. 5 3
      core/src/test/java/org/elasticsearch/cli/TerminalTests.java
  32. 1 1
      core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java
  33. 1 1
      core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
  34. 0 51
      core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java
  35. 0 1
      distribution/licenses/commons-cli-1.3.1.jar.sha1
  36. 1 0
      distribution/licenses/jopt-simple-4.9.jar.sha1
  37. 24 0
      distribution/licenses/jopt-simple-LICENSE.txt
  38. 0 0
      distribution/licenses/jopt-simple-NOTICE.txt
  39. 0 38
      docs/plugins/mapper-attachments.asciidoc
  40. 11 0
      modules/lang-groovy/build.gradle
  41. 0 189
      plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java
  42. 1 0
      plugins/repository-hdfs/build.gradle
  43. 1 0
      plugins/repository-hdfs/licenses/commons-cli-1.2.jar.sha1
  44. 0 0
      plugins/repository-hdfs/licenses/commons-cli-LICENSE.txt
  45. 0 0
      plugins/repository-hdfs/licenses/commons-cli-NOTICE.txt
  46. 64 173
      qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java
  47. 0 329
      qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CheckFileCommandTests.java
  48. 0 349
      qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java
  49. 3 7
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java
  50. 5 8
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java
  51. 1 1
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java
  52. 3 7
      qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java
  53. 56 0
      test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java
  54. 7 5
      test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java
  55. 1 0
      test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java

+ 1 - 1
core/build.gradle

@@ -49,7 +49,7 @@ dependencies {
   compile 'org.elasticsearch:securesm:1.0'
 
   // utilities
-  compile 'commons-cli:commons-cli:1.3.1'
+  compile 'net.sf.jopt-simple:jopt-simple:4.9'
   compile 'com.carrotsearch:hppc:0.7.1'
 
   // time handling, remove with java 8 time

+ 13 - 21
core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -19,15 +19,22 @@
 
 package org.elasticsearch.bootstrap;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.nio.file.Path;
+import java.util.Locale;
+import java.util.concurrent.CountDownLatch;
+
 import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.StringHelper;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.Version;
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.common.PidFile;
 import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.Terminal;
 import org.elasticsearch.common.inject.CreationException;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.LogConfigurator;
@@ -40,13 +47,6 @@ import org.elasticsearch.monitor.process.ProcessProbe;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
 
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.PrintStream;
-import java.nio.file.Path;
-import java.util.Locale;
-import java.util.concurrent.CountDownLatch;
-
 import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
 
 /**
@@ -222,11 +222,11 @@ final class Bootstrap {
         // Set the system property before anything has a chance to trigger its use
         initLoggerPrefix();
 
-        BootstrapCLIParser bootstrapCLIParser = new BootstrapCLIParser();
-        CliTool.ExitStatus status = bootstrapCLIParser.execute(args);
+        BootstrapCliParser parser = new BootstrapCliParser();
+        int status = parser.main(args, Terminal.DEFAULT);
 
-        if (CliTool.ExitStatus.OK != status) {
-            exit(status.status());
+        if (parser.shouldRun() == false || status != ExitCodes.OK) {
+            exit(status);
         }
 
         INSTANCE = new Bootstrap();
@@ -307,14 +307,6 @@ final class Bootstrap {
         System.err.close();
     }
 
-    @SuppressForbidden(reason = "System#err")
-    private static void sysError(String line, boolean flush) {
-        System.err.println(line);
-        if (flush) {
-            System.err.flush();
-        }
-    }
-
     private static void checkForCustomConfFile() {
         String confFileSetting = System.getProperty("es.default.config");
         checkUnsetAndMaybeExit(confFileSetting, "es.default.config");

+ 0 - 183
core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java

@@ -1,183 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.bootstrap;
-
-import org.apache.commons.cli.CommandLine;
-import org.apache.commons.cli.Option;
-import org.elasticsearch.Build;
-import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolConfig;
-import org.elasticsearch.common.cli.UserError;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.monitor.jvm.JvmInfo;
-
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Properties;
-
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder;
-
-final class BootstrapCLIParser extends CliTool {
-
-    private static final CliToolConfig CONFIG = CliToolConfig.config("elasticsearch", BootstrapCLIParser.class)
-            .cmds(Start.CMD, Version.CMD)
-            .build();
-
-    public BootstrapCLIParser() {
-        super(CONFIG);
-    }
-
-    public BootstrapCLIParser(Terminal terminal) {
-        super(CONFIG, terminal);
-    }
-
-    @Override
-    protected Command parse(String cmdName, CommandLine cli) throws Exception {
-        switch (cmdName.toLowerCase(Locale.ROOT)) {
-            case Start.NAME:
-                return Start.parse(terminal, cli);
-            case Version.NAME:
-                return Version.parse(terminal, cli);
-            default:
-                assert false : "should never get here, if the user enters an unknown command, an error message should be shown before parse is called";
-                return null;
-        }
-    }
-
-    static class Version extends CliTool.Command {
-
-        private static final String NAME = "version";
-
-        private static final CliToolConfig.Cmd CMD = cmd(NAME, Version.class).build();
-
-        public static Command parse(Terminal terminal, CommandLine cli) {
-            return new Version(terminal);
-        }
-
-        public Version(Terminal terminal) {
-            super(terminal);
-        }
-
-        @Override
-        public ExitStatus execute(Settings settings, Environment env) throws Exception {
-            terminal.println("Version: " + org.elasticsearch.Version.CURRENT
-                    + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
-                    + ", JVM: " + JvmInfo.jvmInfo().version());
-            return ExitStatus.OK_AND_EXIT;
-        }
-    }
-
-    static class Start extends CliTool.Command {
-
-        private static final String NAME = "start";
-
-        private static final CliToolConfig.Cmd CMD = cmd(NAME, Start.class)
-                .options(
-                        optionBuilder("d", "daemonize").hasArg(false).required(false),
-                        optionBuilder("p", "pidfile").hasArg(true).required(false),
-                        optionBuilder("V", "version").hasArg(false).required(false),
-                        Option.builder("D").argName("property=value").valueSeparator('=').numberOfArgs(2)
-                )
-                .stopAtNonOption(true) // needed to parse the --foo.bar options, so this parser must be lenient
-                .build();
-
-        // TODO: don't use system properties as a way to do this, its horrible...
-        @SuppressForbidden(reason = "Sets system properties passed as CLI parameters")
-        public static Command parse(Terminal terminal, CommandLine cli) throws UserError {
-            if (cli.hasOption("V")) {
-                return Version.parse(terminal, cli);
-            }
-
-            if (cli.hasOption("d")) {
-                System.setProperty("es.foreground", "false");
-            }
-
-            String pidFile = cli.getOptionValue("pidfile");
-            if (!Strings.isNullOrEmpty(pidFile)) {
-                System.setProperty("es.pidfile", pidFile);
-            }
-
-            if (cli.hasOption("D")) {
-                Properties properties = cli.getOptionProperties("D");
-                for (Map.Entry<Object, Object> entry : properties.entrySet()) {
-                    String key = (String) entry.getKey();
-                    String propertyName = key.startsWith("es.") ? key : "es." + key;
-                    System.setProperty(propertyName, entry.getValue().toString());
-                }
-            }
-
-            // hacky way to extract all the fancy extra args, there is no CLI tool helper for this
-            Iterator<String> iterator = cli.getArgList().iterator();
-            final Map<String, String> properties = new HashMap<>();
-            while (iterator.hasNext()) {
-                String arg = iterator.next();
-                if (!arg.startsWith("--")) {
-                    if (arg.startsWith("-D") || arg.startsWith("-d") || arg.startsWith("-p")) {
-                        throw new UserError(ExitStatus.USAGE,
-                                "Parameter [" + arg + "] starting with \"-D\", \"-d\" or \"-p\" must be before any parameters starting with --"
-                        );
-                    } else {
-                        throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "]does not start with --");
-                    }
-                }
-                // if there is no = sign, we have to get the next argu
-                arg = arg.replace("--", "");
-                if (arg.contains("=")) {
-                    String[] splitArg = arg.split("=", 2);
-                    String key = splitArg[0];
-                    String value = splitArg[1];
-                    properties.put("es." + key, value);
-                } else {
-                    if (iterator.hasNext()) {
-                        String value = iterator.next();
-                        if (value.startsWith("--")) {
-                            throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
-                        }
-                        properties.put("es." + arg, value);
-                    } else {
-                        throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
-                    }
-                }
-            }
-            for (Map.Entry<String, String> entry : properties.entrySet()) {
-                System.setProperty(entry.getKey(), entry.getValue());
-            }
-            return new Start(terminal);
-        }
-
-        public Start(Terminal terminal) {
-            super(terminal);
-
-        }
-
-        @Override
-        public ExitStatus execute(Settings settings, Environment env) throws Exception {
-            return ExitStatus.OK;
-        }
-    }
-
-}

+ 95 - 0
core/src/main/java/org/elasticsearch/bootstrap/BootstrapCliParser.java

@@ -0,0 +1,95 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.bootstrap;
+
+import java.util.Arrays;
+
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
+import org.elasticsearch.Build;
+import org.elasticsearch.cli.Command;
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.UserError;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.monitor.jvm.JvmInfo;
+
+final class BootstrapCliParser extends Command {
+
+    private final OptionSpec<Void> versionOption;
+    private final OptionSpec<Void> daemonizeOption;
+    private final OptionSpec<String> pidfileOption;
+    private final OptionSpec<String> propertyOption;
+    private boolean shouldRun = false;
+
+    BootstrapCliParser() {
+        super("Starts elasticsearch");
+        // TODO: in jopt-simple 5.0, make this mutually exclusive with all other options
+        versionOption = parser.acceptsAll(Arrays.asList("V", "version"),
+            "Prints elasticsearch version information and exits");
+        daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"),
+            "Starts Elasticsearch in the background");
+        // TODO: in jopt-simple 5.0 this option type can be a Path
+        pidfileOption = parser.acceptsAll(Arrays.asList("p", "pidfile"),
+            "Creates a pid file in the specified path on start")
+            .withRequiredArg();
+        propertyOption = parser.accepts("D", "Configures an Elasticsearch setting")
+            .withRequiredArg();
+    }
+
+    // TODO: don't use system properties as a way to do this, its horrible...
+    @SuppressForbidden(reason = "Sets system properties passed as CLI parameters")
+    @Override
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+        if (options.has(versionOption)) {
+            terminal.println("Version: " + org.elasticsearch.Version.CURRENT
+                + ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
+                + ", JVM: " + JvmInfo.jvmInfo().version());
+            return;
+        }
+
+        // TODO: don't use sysprops for any of these! pass the args through to bootstrap...
+        if (options.has(daemonizeOption)) {
+            System.setProperty("es.foreground", "false");
+        }
+        String pidFile = pidfileOption.value(options);
+        if (Strings.isNullOrEmpty(pidFile) == false) {
+            System.setProperty("es.pidfile", pidFile);
+        }
+
+        for (String property : propertyOption.values(options)) {
+            String[] keyValue = property.split("=", 2);
+            if (keyValue.length != 2) {
+                throw new UserError(ExitCodes.USAGE, "Malformed elasticsearch setting, must be of the form key=value");
+            }
+            String key = keyValue[0];
+            if (key.startsWith("es.") == false) {
+                key = "es." + key;
+            }
+            System.setProperty(key, keyValue[1]);
+        }
+        shouldRun = true;
+    }
+
+    boolean shouldRun() {
+        return shouldRun;
+    }
+}

+ 1 - 1
core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -32,7 +32,7 @@ public final class Elasticsearch {
     /**
      * Main entry point for starting elasticsearch
      */
-    public static void main(String[] args) throws StartupError {
+    public static void main(String[] args) throws Exception {
         try {
             Bootstrap.init(args);
         } catch (Throwable t) {

+ 112 - 0
core/src/main/java/org/elasticsearch/cli/Command.java

@@ -0,0 +1,112 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+import joptsimple.OptionException;
+import joptsimple.OptionParser;
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
+import org.elasticsearch.common.SuppressForbidden;
+
+/**
+ * An action to execute within a cli.
+ */
+public abstract class Command {
+
+    /** A description of the command, used in the help output. */
+    protected final String description;
+
+    /** The option parser for this command. */
+    protected final OptionParser parser = new OptionParser();
+
+    private final OptionSpec<Void> helpOption = parser.acceptsAll(Arrays.asList("h", "help"), "show help").forHelp();
+    private final OptionSpec<Void> silentOption = parser.acceptsAll(Arrays.asList("s", "silent"), "show minimal output");
+    private final OptionSpec<Void> verboseOption = parser.acceptsAll(Arrays.asList("v", "verbose"), "show verbose output");
+
+    public Command(String description) {
+        this.description = description;
+    }
+
+    /** Parses options for this command from args and executes it. */
+    public final int main(String[] args, Terminal terminal) throws Exception {
+        try {
+            mainWithoutErrorHandling(args, terminal);
+        } catch (OptionException e) {
+            printHelp(terminal);
+            terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage());
+            return ExitCodes.USAGE;
+        } catch (UserError e) {
+            terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + e.getMessage());
+            return e.exitCode;
+        }
+        return ExitCodes.OK;
+    }
+
+    /**
+     * Executes the command, but all errors are thrown.
+     */
+    void mainWithoutErrorHandling(String[] args, Terminal terminal) throws Exception {
+        final OptionSet options = parser.parse(args);
+
+        if (options.has(helpOption)) {
+            printHelp(terminal);
+            return;
+        }
+
+        if (options.has(silentOption)) {
+            if (options.has(verboseOption)) {
+                // mutually exclusive, we can remove this with jopt-simple 5.0, which natively supports it
+                throw new UserError(ExitCodes.USAGE, "Cannot specify -s and -v together");
+            }
+            terminal.setVerbosity(Terminal.Verbosity.SILENT);
+        } else if (options.has(verboseOption)) {
+            terminal.setVerbosity(Terminal.Verbosity.VERBOSE);
+        } else {
+            terminal.setVerbosity(Terminal.Verbosity.NORMAL);
+        }
+
+        execute(terminal, options);
+    }
+
+    /** 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());
+    }
+
+    /** Prints additional help information, specific to the command */
+    protected void printAdditionalHelp(Terminal terminal) {}
+
+    @SuppressForbidden(reason = "Allowed to exit explicitly from #main()")
+    protected static void exit(int status) {
+        System.exit(status);
+    }
+
+    /**
+     * Executes this command.
+     *
+     * Any runtime user errors (like an input file that does not exist), should throw a {@link UserError}. */
+    protected abstract void execute(Terminal terminal, OptionSet options) throws Exception;
+}

+ 42 - 0
core/src/main/java/org/elasticsearch/cli/ExitCodes.java

@@ -0,0 +1,42 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+/**
+ * POSIX exit codes.
+ */
+public class ExitCodes {
+    public static final int OK = 0;
+    public static final int USAGE = 64;          /* command line usage error */
+    public static final int DATA_ERROR = 65;     /* data format error */
+    public static final int NO_INPUT = 66;       /* cannot open input */
+    public static final int NO_USER = 67;        /* addressee unknown */
+    public static final int NO_HOST = 68;        /* host name unknown */
+    public static final int UNAVAILABLE = 69;    /* service unavailable */
+    public static final int CODE_ERROR = 70;     /* internal software error */
+    public static final int CANT_CREATE = 73;    /* can't create (user) output file */
+    public static final int IO_ERROR = 74;       /* input/output error */
+    public static final int TEMP_FAILURE = 75;   /* temp failure; user is invited to retry */
+    public static final int PROTOCOL = 76;       /* remote error in protocol */
+    public static final int NOPERM = 77;         /* permission denied */
+    public static final int CONFIG = 78;         /* configuration error */
+
+    private ExitCodes() { /* no instance, just constants */ }
+}

+ 71 - 0
core/src/main/java/org/elasticsearch/cli/MultiCommand.java

@@ -0,0 +1,71 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import joptsimple.NonOptionArgumentSpec;
+import joptsimple.OptionSet;
+
+/**
+ * A cli tool which is made up of multiple subcommands.
+ */
+public class MultiCommand extends Command {
+
+    protected final Map<String, Command> subcommands = new LinkedHashMap<>();
+
+    private final NonOptionArgumentSpec<String> arguments = parser.nonOptions("command");
+
+    public MultiCommand(String description) {
+        super(description);
+        parser.posixlyCorrect(true);
+    }
+
+    @Override
+    protected void printAdditionalHelp(Terminal terminal) {
+        if (subcommands.isEmpty()) {
+            throw new IllegalStateException("No subcommands configured");
+        }
+        terminal.println("Commands");
+        terminal.println("--------");
+        for (Map.Entry<String, Command> subcommand : subcommands.entrySet()) {
+            terminal.println(subcommand.getKey() + " - " + subcommand.getValue().description);
+        }
+        terminal.println("");
+    }
+
+    @Override
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+        if (subcommands.isEmpty()) {
+            throw new IllegalStateException("No subcommands configured");
+        }
+        String[] args = arguments.values(options).toArray(new String[0]);
+        if (args.length == 0) {
+            throw new UserError(ExitCodes.USAGE, "Missing command");
+        }
+        Command subcommand = subcommands.get(args[0]);
+        if (subcommand == null) {
+            throw new UserError(ExitCodes.USAGE, "Unknown command [" + args[0] + "]");
+        }
+        subcommand.mainWithoutErrorHandling(Arrays.copyOfRange(args, 1, args.length), terminal);
+    }
+}

+ 10 - 10
core/src/main/java/org/elasticsearch/common/cli/Terminal.java → core/src/main/java/org/elasticsearch/cli/Terminal.java

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-package org.elasticsearch.common.cli;
+package org.elasticsearch.cli;
 
 import java.io.BufferedReader;
 import java.io.Console;
@@ -29,7 +29,7 @@ import java.nio.charset.Charset;
 import org.elasticsearch.common.SuppressForbidden;
 
 /**
- * A Terminal wraps access to reading input and writing output for a {@link CliTool}.
+ * A Terminal wraps access to reading input and writing output for a cli.
  *
  * The available methods are similar to those of {@link Console}, with the ability
  * to read either normal text or a password, and the ability to print a line
@@ -61,7 +61,7 @@ public abstract class Terminal {
     }
 
     /** Sets the verbosity of the terminal. */
-    void setVerbosity(Verbosity verbosity) {
+    public void setVerbosity(Verbosity verbosity) {
         this.verbosity = verbosity;
     }
 
@@ -89,35 +89,35 @@ public abstract class Terminal {
 
     private static class ConsoleTerminal extends Terminal {
 
-        private static final Console console = System.console();
+        private static final Console CONSOLE = System.console();
 
         ConsoleTerminal() {
             super(System.lineSeparator());
         }
 
         static boolean isSupported() {
-            return console != null;
+            return CONSOLE != null;
         }
 
         @Override
         public PrintWriter getWriter() {
-            return console.writer();
+            return CONSOLE.writer();
         }
 
         @Override
         public String readText(String prompt) {
-            return console.readLine("%s", prompt);
+            return CONSOLE.readLine("%s", prompt);
         }
 
         @Override
         public char[] readSecret(String prompt) {
-            return console.readPassword("%s", prompt);
+            return CONSOLE.readPassword("%s", prompt);
         }
     }
 
     private static class SystemTerminal extends Terminal {
 
-        private final PrintWriter writer = newWriter();
+        private static final PrintWriter WRITER = newWriter();
 
         SystemTerminal() {
             super(System.lineSeparator());
@@ -130,7 +130,7 @@ public abstract class Terminal {
 
         @Override
         public PrintWriter getWriter() {
-            return writer;
+            return WRITER;
         }
 
         @Override

+ 5 - 5
core/src/main/java/org/elasticsearch/common/cli/UserError.java → core/src/main/java/org/elasticsearch/cli/UserError.java

@@ -17,19 +17,19 @@
  * under the License.
  */
 
-package org.elasticsearch.common.cli;
+package org.elasticsearch.cli;
 
 /**
- * An exception representing a user fixable problem in {@link CliTool} usage.
+ * An exception representing a user fixable problem in {@link Command} usage.
  */
 public class UserError extends Exception {
 
     /** The exist status the cli should use when catching this user error. */
-    public final CliTool.ExitStatus exitStatus;
+    public final int exitCode;
 
     /** Constructs a UserError with an exit status and message to show the user. */
-    public UserError(CliTool.ExitStatus exitStatus, String msg) {
+    public UserError(int exitCode, String msg) {
         super(msg);
-        this.exitStatus = exitStatus;
+        this.exitCode = exitCode;
     }
 }

+ 0 - 138
core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java

@@ -1,138 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.cli;
-
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.attribute.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFileAttributes;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Set;
-
-/**
- * A helper command that checks if configured paths have been changed when running a CLI command.
- * It is only executed in case of specified paths by the command and if the paths underlying filesystem
- * supports posix permissions.
- *
- * If this is the case, a warn message is issued whenever an owner, a group or the file permissions is changed by
- * the command being executed and not configured back to its prior state, which should be the task of the command
- * being executed.
- *
- */
-public abstract class CheckFileCommand extends CliTool.Command {
-
-    public CheckFileCommand(Terminal terminal) {
-        super(terminal);
-    }
-
-    /**
-     * abstract method, which should implement the same logic as CliTool.Command.execute(), but is wrapped
-     */
-    public abstract CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception;
-
-    /**
-     * Returns the array of paths, that should be checked if the permissions, user or groups have changed
-     * before and after execution of the command
-     *
-     */
-    protected abstract Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception;
-
-    @Override
-    public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-        Path[] paths = pathsForPermissionsCheck(settings, env);
-
-        if (paths == null || paths.length == 0) {
-            return doExecute(settings, env);
-        }
-
-        Map<Path, Set<PosixFilePermission>> permissions = new HashMap<>(paths.length);
-        Map<Path, String> owners = new HashMap<>(paths.length);
-        Map<Path, String> groups = new HashMap<>(paths.length);
-
-        if (paths != null && paths.length > 0) {
-            for (Path path : paths) {
-                try {
-                    boolean supportsPosixPermissions = Environment.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
-                    if (supportsPosixPermissions) {
-                        PosixFileAttributes attributes = Files.readAttributes(path, PosixFileAttributes.class);
-                        permissions.put(path, attributes.permissions());
-                        owners.put(path, attributes.owner().getName());
-                        groups.put(path, attributes.group().getName());
-                    }
-                } catch (IOException e) {
-                    // silently swallow if not supported, no need to log things
-                }
-            }
-        }
-
-        CliTool.ExitStatus status = doExecute(settings, env);
-
-        // check if permissions differ
-        for (Map.Entry<Path, Set<PosixFilePermission>> entry : permissions.entrySet()) {
-            if (!Files.exists(entry.getKey())) {
-                continue;
-            }
-
-            Set<PosixFilePermission> permissionsBeforeWrite = entry.getValue();
-            Set<PosixFilePermission> permissionsAfterWrite = Files.getPosixFilePermissions(entry.getKey());
-            if (!permissionsBeforeWrite.equals(permissionsAfterWrite)) {
-                terminal.println(Terminal.Verbosity.SILENT, "WARNING: The file permissions of [" + entry.getKey() + "] have changed "
-                        + "from [" + PosixFilePermissions.toString(permissionsBeforeWrite) + "] "
-                        + "to [" + PosixFilePermissions.toString(permissionsAfterWrite) + "]");
-                terminal.println(Terminal.Verbosity.SILENT, "Please ensure that the user account running Elasticsearch has read access to this file!");
-            }
-        }
-
-        // check if owner differs
-        for (Map.Entry<Path, String> entry : owners.entrySet()) {
-            if (!Files.exists(entry.getKey())) {
-                continue;
-            }
-
-            String ownerBeforeWrite = entry.getValue();
-            String ownerAfterWrite = Files.getOwner(entry.getKey()).getName();
-            if (!ownerAfterWrite.equals(ownerBeforeWrite)) {
-                terminal.println(Terminal.Verbosity.SILENT, "WARNING: Owner of file [" + entry.getKey() + "] used to be [" + ownerBeforeWrite + "], but now is [" + ownerAfterWrite + "]");
-            }
-        }
-
-        // check if group differs
-        for (Map.Entry<Path, String> entry : groups.entrySet()) {
-            if (!Files.exists(entry.getKey())) {
-                continue;
-            }
-
-            String groupBeforeWrite = entry.getValue();
-            String groupAfterWrite = Files.readAttributes(entry.getKey(), PosixFileAttributes.class).group().getName();
-            if (!groupAfterWrite.equals(groupBeforeWrite)) {
-                terminal.println(Terminal.Verbosity.SILENT, "WARNING: Group of file [" + entry.getKey() + "] used to be [" + groupBeforeWrite + "], but now is [" + groupAfterWrite + "]");
-            }
-        }
-
-        return status;
-    }
-}

+ 0 - 250
core/src/main/java/org/elasticsearch/common/cli/CliTool.java

@@ -1,250 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.cli;
-
-import org.apache.commons.cli.AlreadySelectedException;
-import org.apache.commons.cli.CommandLine;
-import org.apache.commons.cli.CommandLineParser;
-import org.apache.commons.cli.DefaultParser;
-import org.apache.commons.cli.MissingArgumentException;
-import org.apache.commons.cli.MissingOptionException;
-import org.apache.commons.cli.UnrecognizedOptionException;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.node.internal.InternalSettingsPreparer;
-
-import java.util.Locale;
-
-import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
-
-/**
- * A base class for command-line interface tool.
- *
- * Two modes are supported:
- *
- * - Single command mode. The tool exposes a single command that can potentially accept arguments (eg. CLI options).
- * - Multi command mode. The tool support multiple commands, each for different tasks, each potentially accepts arguments.
- *
- * In a multi-command mode. The first argument must be the command name. For example, the plugin manager
- * can be seen as a multi-command tool with two possible commands: install and uninstall
- *
- * The tool is configured using a {@link CliToolConfig} which encapsulates the tool's commands and their
- * potential options. The tool also comes with out of the box simple help support (the -h/--help option is
- * automatically handled) where the help text is configured in a dedicated *.help files located in the same package
- * as the tool.
- */
-public abstract class CliTool {
-
-    // based on sysexits.h
-    public enum ExitStatus {
-        OK(0),
-        OK_AND_EXIT(0),
-        USAGE(64),          /* command line usage error */
-        DATA_ERROR(65),     /* data format error */
-        NO_INPUT(66),       /* cannot open input */
-        NO_USER(67),        /* addressee unknown */
-        NO_HOST(68),        /* host name unknown */
-        UNAVAILABLE(69),    /* service unavailable */
-        CODE_ERROR(70),     /* internal software error */
-        CANT_CREATE(73),    /* can't create (user) output file */
-        IO_ERROR(74),       /* input/output error */
-        TEMP_FAILURE(75),   /* temp failure; user is invited to retry */
-        PROTOCOL(76),       /* remote error in protocol */
-        NOPERM(77),         /* permission denied */
-        CONFIG(78);          /* configuration error */
-
-        final int status;
-
-        ExitStatus(int status) {
-            this.status = status;
-        }
-
-        public int status() {
-            return status;
-        }
-    }
-
-    protected final Terminal terminal;
-    protected final Environment env;
-    protected final Settings settings;
-
-    private final CliToolConfig config;
-
-    protected CliTool(CliToolConfig config) {
-        this(config, Terminal.DEFAULT);
-    }
-
-    protected CliTool(CliToolConfig config, Terminal terminal) {
-        if (config.cmds().size() == 0) {
-            throw new IllegalArgumentException("At least one command must be configured");
-        }
-        this.config = config;
-        this.terminal = terminal;
-        env = InternalSettingsPreparer.prepareEnvironment(EMPTY_SETTINGS, terminal);
-        settings = env.settings();
-    }
-
-    public final ExitStatus execute(String... args) throws Exception {
-
-        // first lets see if the user requests tool help. We're doing it only if
-        // this is a multi-command tool. If it's a single command tool, the -h/--help
-        // option will be taken care of on the command level
-        if (!config.isSingle() && args.length > 0 && (args[0].equals("-h") || args[0].equals("--help"))) {
-            config.printUsage(terminal);
-            return ExitStatus.OK_AND_EXIT;
-        }
-
-        CliToolConfig.Cmd cmd;
-        if (config.isSingle()) {
-            cmd = config.single();
-        } else {
-
-            if (args.length == 0) {
-                terminal.println(Terminal.Verbosity.SILENT, "ERROR: command not specified");
-                config.printUsage(terminal);
-                return ExitStatus.USAGE;
-            }
-
-            String cmdName = args[0];
-            cmd = config.cmd(cmdName);
-            if (cmd == null) {
-                terminal.println(Terminal.Verbosity.SILENT, "ERROR: unknown command [" + cmdName + "]. Use [-h] option to list available commands");
-                return ExitStatus.USAGE;
-            }
-
-            // we now remove the command name from the args
-            if (args.length == 1) {
-                args = new String[0];
-            } else {
-                String[] cmdArgs = new String[args.length - 1];
-                System.arraycopy(args, 1, cmdArgs, 0, cmdArgs.length);
-                args = cmdArgs;
-            }
-        }
-
-        try {
-            return parse(cmd, args).execute(settings, env);
-        } catch (UserError error) {
-            terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + error.getMessage());
-            return error.exitStatus;
-        }
-    }
-
-    public Command parse(String cmdName, String[] args) throws Exception {
-        CliToolConfig.Cmd cmd = config.cmd(cmdName);
-        return parse(cmd, args);
-    }
-
-    public Command parse(CliToolConfig.Cmd cmd, String[] args) throws Exception {
-        CommandLineParser parser = new DefaultParser();
-        CommandLine cli = parser.parse(CliToolConfig.OptionsSource.HELP.options(), args, true);
-        if (cli.hasOption("h")) {
-            return helpCmd(cmd);
-        }
-        try {
-            cli = parser.parse(cmd.options(), args, cmd.isStopAtNonOption());
-        } catch (AlreadySelectedException|MissingArgumentException|MissingOptionException|UnrecognizedOptionException e) {
-            // intentionally drop the stack trace here as these are really user errors,
-            // the stack trace into cli parsing lib is not important
-            throw new UserError(ExitStatus.USAGE, e.toString());
-        }
-
-        if (cli.hasOption("v")) {
-            terminal.setVerbosity(Terminal.Verbosity.VERBOSE);
-        } else if (cli.hasOption("s")) {
-            terminal.setVerbosity(Terminal.Verbosity.SILENT);
-        } else {
-            terminal.setVerbosity(Terminal.Verbosity.NORMAL);
-        }
-        return parse(cmd.name(), cli);
-    }
-
-    protected Command.Help helpCmd(CliToolConfig.Cmd cmd) {
-        return new Command.Help(cmd, terminal);
-    }
-
-    protected static Command.Exit exitCmd(ExitStatus status) {
-        return new Command.Exit(null, status, null);
-    }
-
-    protected static Command.Exit exitCmd(ExitStatus status, Terminal terminal, String msg, Object... args) {
-        return new Command.Exit(String.format(Locale.ROOT, msg, args), status, terminal);
-    }
-
-    protected abstract Command parse(String cmdName, CommandLine cli) throws Exception;
-
-    public static abstract class Command {
-
-        protected final Terminal terminal;
-
-        protected Command(Terminal terminal) {
-            this.terminal = terminal;
-        }
-
-        public abstract ExitStatus execute(Settings settings, Environment env) throws Exception;
-
-        public static class Help extends Command {
-
-            private final CliToolConfig.Cmd cmd;
-
-            private Help(CliToolConfig.Cmd cmd, Terminal terminal) {
-                super(terminal);
-                this.cmd = cmd;
-            }
-
-            @Override
-            public ExitStatus execute(Settings settings, Environment env) throws Exception {
-                cmd.printUsage(terminal);
-                return ExitStatus.OK_AND_EXIT;
-            }
-        }
-
-        public static class Exit extends Command {
-            private final String msg;
-            private final ExitStatus status;
-
-            private Exit(String msg, ExitStatus status, Terminal terminal) {
-                super(terminal);
-                this.msg = msg;
-                this.status = status;
-            }
-
-            @Override
-            public ExitStatus execute(Settings settings, Environment env) throws Exception {
-                if (msg != null) {
-                    if (status != ExitStatus.OK) {
-                        terminal.println(Terminal.Verbosity.SILENT, "ERROR: " + msg);
-                    } else {
-                        terminal.println(msg);
-                    }
-                }
-                return status;
-            }
-
-            public ExitStatus status() {
-                return status;
-            }
-        }
-    }
-
-
-
-}
-

+ 0 - 302
core/src/main/java/org/elasticsearch/common/cli/CliToolConfig.java

@@ -1,302 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.cli;
-
-import org.apache.commons.cli.Option;
-import org.apache.commons.cli.OptionGroup;
-import org.apache.commons.cli.Options;
-
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
-/**
- *
- */
-public class CliToolConfig {
-
-    public static Builder config(String name, Class<? extends CliTool> toolType) {
-        return new Builder(name, toolType);
-    }
-
-    private final Class<? extends CliTool> toolType;
-    private final String name;
-    private final Map<String, Cmd> cmds;
-
-    private static final HelpPrinter helpPrinter = new HelpPrinter();
-
-    private CliToolConfig(String name, Class<? extends CliTool> toolType, Cmd[] cmds) {
-        this.name = name;
-        this.toolType = toolType;
-        final Map<String, Cmd> cmdsMapping = new HashMap<>();
-        for (int i = 0; i < cmds.length; i++) {
-            cmdsMapping.put(cmds[i].name, cmds[i]);
-        }
-        this.cmds = Collections.unmodifiableMap(cmdsMapping);
-    }
-
-    public boolean isSingle() {
-        return cmds.size() == 1;
-    }
-
-    public Cmd single() {
-        assert isSingle() : "Requesting single command on a multi-command tool";
-        return cmds.values().iterator().next();
-    }
-
-    public Class<? extends CliTool> toolType() {
-        return toolType;
-    }
-
-    public String name() {
-        return name;
-    }
-
-    public Collection<Cmd> cmds() {
-        return cmds.values();
-    }
-
-    public Cmd cmd(String name) {
-        return cmds.get(name);
-    }
-
-    public void printUsage(Terminal terminal) {
-        helpPrinter.print(this, terminal);
-    }
-
-    public static class Builder {
-
-        public static Cmd.Builder cmd(String name, Class<? extends CliTool.Command> cmdType) {
-            return new Cmd.Builder(name, cmdType);
-        }
-
-        public static OptionBuilder option(String shortName, String longName) {
-            return new OptionBuilder(shortName, longName);
-        }
-
-        public static Option.Builder optionBuilder(String shortName, String longName) {
-            return Option.builder(shortName).argName(longName).longOpt(longName);
-        }
-
-        public static OptionGroupBuilder optionGroup(boolean required) {
-            return new OptionGroupBuilder(required);
-        }
-
-        private final Class<? extends CliTool> toolType;
-        private final String name;
-        private Cmd[] cmds;
-
-        private Builder(String name, Class<? extends CliTool> toolType) {
-            this.name = name;
-            this.toolType = toolType;
-        }
-
-        public Builder cmds(Cmd.Builder... cmds) {
-            this.cmds = new Cmd[cmds.length];
-            for (int i = 0; i < cmds.length; i++) {
-                this.cmds[i] = cmds[i].build();
-                this.cmds[i].toolName = name;
-            }
-            return this;
-        }
-
-        public Builder cmds(Cmd... cmds) {
-            for (int i = 0; i < cmds.length; i++) {
-                cmds[i].toolName = name;
-            }
-            this.cmds = cmds;
-            return this;
-        }
-
-        public CliToolConfig build() {
-            return new CliToolConfig(name, toolType, cmds);
-        }
-    }
-
-    public static class Cmd {
-
-        private String toolName;
-        private final String name;
-        private final Class<? extends CliTool.Command> cmdType;
-        private final Options options;
-        private final boolean stopAtNonOption;
-
-        private Cmd(String name, Class<? extends CliTool.Command> cmdType, Options options, boolean stopAtNonOption) {
-            this.name = name;
-            this.cmdType = cmdType;
-            this.options = options;
-            this.stopAtNonOption = stopAtNonOption;
-            OptionsSource.VERBOSITY.populate(options);
-        }
-
-        public Class<? extends CliTool.Command> cmdType() {
-            return cmdType;
-        }
-
-        public String name() {
-            return name;
-        }
-
-        public Options options() {
-            return options;
-        }
-
-        public boolean isStopAtNonOption() {
-            return stopAtNonOption;
-        }
-
-        public void printUsage(Terminal terminal) {
-            helpPrinter.print(toolName, this, terminal);
-        }
-
-        public static class Builder {
-
-            private final String name;
-            private final Class<? extends CliTool.Command> cmdType;
-            private Options options = new Options();
-            private boolean stopAtNonOption = false;
-
-            private Builder(String name, Class<? extends CliTool.Command> cmdType) {
-                this.name = name;
-                this.cmdType = cmdType;
-            }
-
-            public Builder options(OptionBuilder... optionBuilder) {
-                for (int i = 0; i < optionBuilder.length; i++) {
-                    options.addOption(optionBuilder[i].build());
-                }
-                return this;
-            }
-
-            public Builder options(Option.Builder... optionBuilders) {
-                for (int i = 0; i < optionBuilders.length; i++) {
-                    options.addOption(optionBuilders[i].build());
-                }
-                return this;
-            }
-
-            public Builder optionGroups(OptionGroupBuilder... optionGroupBuilders) {
-                for (OptionGroupBuilder builder : optionGroupBuilders) {
-                    options.addOptionGroup(builder.build());
-                }
-                return this;
-            }
-
-            /**
-              * @param stopAtNonOption if <tt>true</tt> an unrecognized argument stops
-              *     the parsing and the remaining arguments are added to the
-              *     args list. If <tt>false</tt> an unrecognized
-              *     argument triggers a ParseException.
-              */
-            public Builder stopAtNonOption(boolean stopAtNonOption) {
-                this.stopAtNonOption = stopAtNonOption;
-                return this;
-            }
-
-            public Cmd build() {
-                return new Cmd(name, cmdType, options, stopAtNonOption);
-            }
-        }
-    }
-
-    public static class OptionBuilder {
-
-        private final Option option;
-
-        private OptionBuilder(String shortName, String longName) {
-            option = new Option(shortName, "");
-            option.setLongOpt(longName);
-            option.setArgName(longName);
-        }
-
-        public OptionBuilder required(boolean required) {
-            option.setRequired(required);
-            return this;
-        }
-
-        public OptionBuilder hasArg(boolean optional) {
-            option.setOptionalArg(optional);
-            option.setArgs(1);
-            return this;
-        }
-
-        public Option build() {
-            return option;
-        }
-    }
-
-    public static class OptionGroupBuilder {
-
-        private OptionGroup group;
-
-        private OptionGroupBuilder(boolean required) {
-            group = new OptionGroup();
-            group.setRequired(required);
-        }
-
-        public OptionGroupBuilder options(OptionBuilder... optionBuilders) {
-            for (OptionBuilder builder : optionBuilders) {
-                group.addOption(builder.build());
-            }
-            return this;
-        }
-
-        public OptionGroup build() {
-            return group;
-        }
-
-    }
-
-    static abstract class OptionsSource {
-
-        static final OptionsSource HELP = new OptionsSource() {
-
-            @Override
-            void populate(Options options) {
-                options.addOption(new OptionBuilder("h", "help").required(false).build());
-            }
-        };
-
-        static final OptionsSource VERBOSITY = new OptionsSource() {
-            @Override
-            void populate(Options options) {
-                OptionGroup verbosityGroup = new OptionGroup();
-                verbosityGroup.setRequired(false);
-                verbosityGroup.addOption(new OptionBuilder("s", "silent").required(false).build());
-                verbosityGroup.addOption(new OptionBuilder("v", "verbose").required(false).build());
-                options.addOptionGroup(verbosityGroup);
-            }
-        };
-
-        private Options options;
-
-        Options options() {
-            if (options == null) {
-                options = new Options();
-                populate(options);
-            }
-            return options;
-        }
-
-        abstract void populate(Options options);
-
-    }
-}

+ 0 - 57
core/src/main/java/org/elasticsearch/common/cli/HelpPrinter.java

@@ -1,57 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.cli;
-
-import org.elasticsearch.common.io.Streams;
-import org.elasticsearch.common.util.Callback;
-
-import java.io.IOException;
-import java.io.InputStream;
-
-/**
- *
- */
-public class HelpPrinter {
-
-    private static final String HELP_FILE_EXT = ".help";
-
-    public void print(CliToolConfig config, Terminal terminal) {
-        print(config.toolType(), config.name(), terminal);
-    }
-
-    public void print(String toolName, CliToolConfig.Cmd cmd, Terminal terminal) {
-        print(cmd.cmdType(), toolName + "-" + cmd.name(), terminal);
-    }
-
-    private static void print(Class clazz, String name, final Terminal terminal) {
-        terminal.println(Terminal.Verbosity.SILENT, "");
-        try (InputStream input = clazz.getResourceAsStream(name + HELP_FILE_EXT)) {
-            Streams.readAllLines(input, new Callback<String>() {
-                @Override
-                public void handle(String line) {
-                    terminal.println(Terminal.Verbosity.SILENT, line);
-                }
-            });
-        } catch (IOException ioe) {
-            throw new RuntimeException(ioe);
-        }
-        terminal.println(Terminal.Verbosity.SILENT, "");
-    }
-}

+ 1 - 1
core/src/main/java/org/elasticsearch/common/logging/TerminalAppender.java

@@ -22,7 +22,7 @@ package org.elasticsearch.common.logging;
 
 import org.apache.log4j.AppenderSkeleton;
 import org.apache.log4j.spi.LoggingEvent;
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.Terminal;
 
 /**
  * TerminalAppender logs event to Terminal.DEFAULT. It is used for example by the PluginCli.

+ 1 - 1
core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java

@@ -23,7 +23,7 @@ import org.elasticsearch.bootstrap.BootstrapInfo;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.Randomness;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;

+ 59 - 34
core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -19,16 +19,18 @@
 
 package org.elasticsearch.plugins;
 
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.Build;
 import org.elasticsearch.Version;
 import org.elasticsearch.bootstrap.JarHell;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.cli.UserError;
+import org.elasticsearch.cli.Command;
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.cli.UserError;
 import org.elasticsearch.common.hash.MessageDigests;
 import org.elasticsearch.common.io.FileSystemUtils;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 
 import java.io.BufferedReader;
@@ -48,6 +50,7 @@ import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
@@ -55,7 +58,7 @@ import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
 import static java.util.Collections.unmodifiableSet;
-import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE;
+import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE;
 import static org.elasticsearch.common.util.set.Sets.newHashSet;
 
 /**
@@ -88,7 +91,7 @@ import static org.elasticsearch.common.util.set.Sets.newHashSet;
  * elasticsearch config directory, using the name of the plugin. If any files to be installed
  * already exist, they will be skipped.
  */
-class InstallPluginCommand extends CliTool.Command {
+class InstallPluginCommand extends Command {
 
     private static final String PROPERTY_SUPPORT_STAGING_URLS = "es.plugins.staging";
 
@@ -98,7 +101,7 @@ class InstallPluginCommand extends CliTool.Command {
         "lang-groovy"));
 
     // TODO: make this a resource file generated by gradle
-    static final Set<String> OFFICIAL_PLUGINS = unmodifiableSet(newHashSet(
+    static final Set<String> OFFICIAL_PLUGINS = unmodifiableSet(new LinkedHashSet<>(Arrays.asList(
         "analysis-icu",
         "analysis-kuromoji",
         "analysis-phonetic",
@@ -117,19 +120,43 @@ class InstallPluginCommand extends CliTool.Command {
         "repository-azure",
         "repository-hdfs",
         "repository-s3",
-        "store-smb"));
-
-    private final String pluginId;
-    private final boolean batch;
+        "store-smb")));
+
+    private final Environment env;
+    private final OptionSpec<Void> batchOption;
+    private final OptionSpec<String> arguments;
+
+    InstallPluginCommand(Environment env) {
+        super("Install a plugin");
+        this.env = env;
+        this.batchOption = parser.acceptsAll(Arrays.asList("b", "batch"),
+            "Enable batch mode explicitly, automatic confirmation of security permission");
+        this.arguments = parser.nonOptions("plugin id");
+    }
 
-    InstallPluginCommand(Terminal terminal, String pluginId, boolean batch) {
-        super(terminal);
-        this.pluginId = pluginId;
-        this.batch = batch;
+    @Override
+    protected void printAdditionalHelp(Terminal terminal) {
+        terminal.println("The following official plugins may be installed by name:");
+        for (String plugin : OFFICIAL_PLUGINS) {
+            terminal.println("  " + plugin);
+        }
+        terminal.println("");
     }
 
     @Override
-    public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+        // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args
+        List<String> args = arguments.values(options);
+        if (args.size() != 1) {
+            throw new UserError(ExitCodes.USAGE, "Must supply a single plugin id argument");
+        }
+        String pluginId = args.get(0);
+        boolean isBatch = options.has(batchOption) || System.console() == null;
+        execute(terminal, pluginId, isBatch);
+    }
+
+    // pkg private for testing
+    void execute(Terminal terminal, String pluginId, boolean isBatch) throws Exception {
 
         // TODO: remove this leniency!! is it needed anymore?
         if (Files.exists(env.pluginsFile()) == false) {
@@ -137,15 +164,13 @@ class InstallPluginCommand extends CliTool.Command {
             Files.createDirectory(env.pluginsFile());
         }
 
-        Path pluginZip = download(pluginId, env.tmpFile());
+        Path pluginZip = download(terminal, pluginId, env.tmpFile());
         Path extractedZip = unzip(pluginZip, env.pluginsFile());
-        install(extractedZip, env);
-
-        return CliTool.ExitStatus.OK;
+        install(terminal, isBatch, extractedZip);
     }
 
     /** Downloads the plugin and returns the file it was downloaded to. */
-    private Path download(String pluginId, Path tmpDir) throws Exception {
+    private Path download(Terminal terminal, String pluginId, Path tmpDir) throws Exception {
         if (OFFICIAL_PLUGINS.contains(pluginId)) {
             final String version = Version.CURRENT.toString();
             final String url;
@@ -195,14 +220,14 @@ class InstallPluginCommand extends CliTool.Command {
             BufferedReader checksumReader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8));
             expectedChecksum = checksumReader.readLine();
             if (checksumReader.readLine() != null) {
-                throw new UserError(CliTool.ExitStatus.IO_ERROR, "Invalid checksum file at " + checksumUrl);
+                throw new UserError(ExitCodes.IO_ERROR, "Invalid checksum file at " + checksumUrl);
             }
         }
 
         byte[] zipbytes = Files.readAllBytes(zip);
         String gotChecksum = MessageDigests.toHexString(MessageDigests.sha1().digest(zipbytes));
         if (expectedChecksum.equals(gotChecksum) == false) {
-            throw new UserError(CliTool.ExitStatus.IO_ERROR, "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum);
+            throw new UserError(ExitCodes.IO_ERROR, "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum);
         }
 
         return zip;
@@ -244,13 +269,13 @@ class InstallPluginCommand extends CliTool.Command {
         Files.delete(zip);
         if (hasEsDir == false) {
             IOUtils.rm(target);
-            throw new UserError(CliTool.ExitStatus.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip");
+            throw new UserError(ExitCodes.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip");
         }
         return target;
     }
 
     /** Load information about the plugin, and verify it can be installed with no errors. */
-    private PluginInfo verify(Path pluginRoot, Environment env) throws Exception {
+    private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) throws Exception {
         // read and validate the plugin descriptor
         PluginInfo info = PluginInfo.readFromProperties(pluginRoot);
         terminal.println(VERBOSE, info.toString());
@@ -258,7 +283,7 @@ class InstallPluginCommand extends CliTool.Command {
         // don't let luser install plugin as a module...
         // they might be unavoidably in maven central and are packaged up the same way)
         if (MODULES.contains(info.getName())) {
-            throw new UserError(CliTool.ExitStatus.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module");
+            throw new UserError(ExitCodes.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module");
         }
 
         // check for jar hell before any copying
@@ -268,7 +293,7 @@ class InstallPluginCommand extends CliTool.Command {
         // if it exists, confirm or warn the user
         Path policy = pluginRoot.resolve(PluginInfo.ES_PLUGIN_POLICY);
         if (Files.exists(policy)) {
-            PluginSecurity.readPolicy(policy, terminal, env, batch);
+            PluginSecurity.readPolicy(policy, terminal, env, isBatch);
         }
 
         return info;
@@ -305,16 +330,16 @@ class InstallPluginCommand extends CliTool.Command {
      * Installs the plugin from {@code tmpRoot} into the plugins dir.
      * If the plugin has a bin dir and/or a config dir, those are copied.
      */
-    private void install(Path tmpRoot, Environment env) throws Exception {
+    private void install(Terminal terminal, boolean isBatch, Path tmpRoot) throws Exception {
         List<Path> deleteOnFailure = new ArrayList<>();
         deleteOnFailure.add(tmpRoot);
 
         try {
-            PluginInfo info = verify(tmpRoot, env);
+            PluginInfo info = verify(terminal, tmpRoot, isBatch);
 
             final Path destination = env.pluginsFile().resolve(info.getName());
             if (Files.exists(destination)) {
-                throw new UserError(CliTool.ExitStatus.USAGE, "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command");
+                throw new UserError(ExitCodes.USAGE, "plugin directory " + destination.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + info.getName() + "' command");
             }
 
             Path tmpBinDir = tmpRoot.resolve("bin");
@@ -347,7 +372,7 @@ class InstallPluginCommand extends CliTool.Command {
     /** Copies the files from {@code tmpBinDir} into {@code destBinDir}, along with permissions from dest dirs parent. */
     private void installBin(PluginInfo info, Path tmpBinDir, Path destBinDir) throws Exception {
         if (Files.isDirectory(tmpBinDir) == false) {
-            throw new UserError(CliTool.ExitStatus.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory");
+            throw new UserError(ExitCodes.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory");
         }
         Files.createDirectory(destBinDir);
 
@@ -365,7 +390,7 @@ class InstallPluginCommand extends CliTool.Command {
         try (DirectoryStream<Path> stream  = Files.newDirectoryStream(tmpBinDir)) {
             for (Path srcFile : stream) {
                 if (Files.isDirectory(srcFile)) {
-                    throw new UserError(CliTool.ExitStatus.DATA_ERROR, "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName());
+                    throw new UserError(ExitCodes.DATA_ERROR, "Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName());
                 }
 
                 Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile));
@@ -386,7 +411,7 @@ class InstallPluginCommand extends CliTool.Command {
      */
     private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws Exception {
         if (Files.isDirectory(tmpConfigDir) == false) {
-            throw new UserError(CliTool.ExitStatus.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
+            throw new UserError(ExitCodes.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
         }
 
         // create the plugin's config dir "if necessary"
@@ -395,7 +420,7 @@ class InstallPluginCommand extends CliTool.Command {
         try (DirectoryStream<Path> stream  = Files.newDirectoryStream(tmpConfigDir)) {
             for (Path srcFile : stream) {
                 if (Files.isDirectory(srcFile)) {
-                    throw new UserError(CliTool.ExitStatus.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName());
+                    throw new UserError(ExitCodes.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName());
                 }
 
                 Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));

+ 10 - 9
core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java

@@ -24,22 +24,25 @@ import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.settings.Settings;
+import joptsimple.OptionSet;
+import org.elasticsearch.cli.Command;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.env.Environment;
 
 /**
  * A command for the plugin cli to list plugins installed in elasticsearch.
  */
-class ListPluginsCommand extends CliTool.Command {
+class ListPluginsCommand extends Command {
 
-    ListPluginsCommand(Terminal terminal) {
-        super(terminal);
+    private final Environment env;
+
+    ListPluginsCommand(Environment env) {
+        super("Lists installed elasticsearch plugins");
+        this.env = env;
     }
 
     @Override
-    public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
         if (Files.exists(env.pluginsFile()) == false) {
             throw new IOException("Plugins directory missing: " + env.pluginsFile());
         }
@@ -50,7 +53,5 @@ class ListPluginsCommand extends CliTool.Command {
                 terminal.println(plugin.getFileName().toString());
             }
         }
-
-        return CliTool.ExitStatus.OK;
     }
 }

+ 14 - 91
core/src/main/java/org/elasticsearch/plugins/PluginCli.java

@@ -19,106 +19,29 @@
 
 package org.elasticsearch.plugins;
 
-import org.apache.commons.cli.CommandLine;
-import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolConfig;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.logging.LogConfigurator;
+import org.apache.log4j.BasicConfigurator;
+import org.apache.log4j.varia.NullAppender;
+import org.elasticsearch.cli.MultiCommand;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
 
-import java.util.Locale;
-
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.option;
-
 /**
  * A cli tool for adding, removing and listing plugins for elasticsearch.
  */
-public class PluginCli extends CliTool {
-
-    // commands
-    private static final String LIST_CMD_NAME = "list";
-    private static final String INSTALL_CMD_NAME = "install";
-    private static final String REMOVE_CMD_NAME = "remove";
-
-    // usage config
-    private static final CliToolConfig.Cmd LIST_CMD = cmd(LIST_CMD_NAME, ListPluginsCommand.class).build();
-    private static final CliToolConfig.Cmd INSTALL_CMD = cmd(INSTALL_CMD_NAME, InstallPluginCommand.class)
-        .options(option("b", "batch").required(false))
-        .build();
-    private static final CliToolConfig.Cmd REMOVE_CMD = cmd(REMOVE_CMD_NAME, RemovePluginCommand.class).build();
-
-    static final CliToolConfig CONFIG = CliToolConfig.config("plugin", PluginCli.class)
-            .cmds(LIST_CMD, INSTALL_CMD, REMOVE_CMD)
-            .build();
-
-    public static void main(String[] args) throws Exception {
-        // initialize default for es.logger.level because we will not read the logging.yml
-        String loggerLevel = System.getProperty("es.logger.level", "INFO");
-        // Set the appender for all potential log files to terminal so that other components that use the logger print out the
-        // same terminal.
-        // The reason for this is that the plugin cli cannot be configured with a file appender because when the plugin command is
-        // executed there is no way of knowing where the logfiles should be placed. For example, if elasticsearch
-        // is run as service then the logs should be at /var/log/elasticsearch but when started from the tar they should be at es.home/logs.
-        // Therefore we print to Terminal.
-        Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.builder()
-                .put("appender.terminal.type", "terminal")
-                .put("rootLogger", "${es.logger.level}, terminal")
-                .put("es.logger.level", loggerLevel)
-                .build(), Terminal.DEFAULT);
-        // configure but do not read the logging conf file
-        LogConfigurator.configure(env.settings(), false);
-        int status = new PluginCli(Terminal.DEFAULT).execute(args).status();
-        exit(status);
-    }
-
-    @SuppressForbidden(reason = "Allowed to exit explicitly from #main()")
-    private static void exit(int status) {
-        System.exit(status);
-    }
-
-    PluginCli(Terminal terminal) {
-        super(CONFIG, terminal);
-    }
+public class PluginCli extends MultiCommand {
 
-    @Override
-    protected Command parse(String cmdName, CommandLine cli) throws Exception {
-        switch (cmdName.toLowerCase(Locale.ROOT)) {
-            case LIST_CMD_NAME:
-                return new ListPluginsCommand(terminal);
-            case INSTALL_CMD_NAME:
-                return parseInstallPluginCommand(cli);
-            case REMOVE_CMD_NAME:
-                return parseRemovePluginCommand(cli);
-            default:
-                assert false : "can't get here as cmd name is validated before this method is called";
-                return exitCmd(ExitStatus.USAGE);
-        }
+    public PluginCli(Environment env) {
+        super("A tool for managing installed elasticsearch plugins");
+        subcommands.put("list", new ListPluginsCommand(env));
+        subcommands.put("install", new InstallPluginCommand(env));
+        subcommands.put("remove", new RemovePluginCommand(env));
     }
 
-    private Command parseInstallPluginCommand(CommandLine cli) {
-        String[] args = cli.getArgs();
-        if (args.length != 1) {
-            return exitCmd(ExitStatus.USAGE, terminal, "Must supply a single plugin id argument");
-        }
-
-        boolean batch = System.console() == null;
-        if (cli.hasOption("b")) {
-            batch = true;
-        }
-
-        return new InstallPluginCommand(terminal, args[0], batch);
-    }
-
-    private Command parseRemovePluginCommand(CommandLine cli) {
-        String[] args = cli.getArgs();
-        if (args.length != 1) {
-            return exitCmd(ExitStatus.USAGE, terminal, "Must supply a single plugin name argument");
-        }
-
-        return new RemovePluginCommand(terminal, args[0]);
+    public static void main(String[] args) throws Exception {
+        BasicConfigurator.configure(new NullAppender());
+        Environment env = InternalSettingsPreparer.prepareEnvironment(Settings.EMPTY, Terminal.DEFAULT);
+        exit(new PluginCli(env).main(args, Terminal.DEFAULT));
     }
 }

+ 2 - 2
core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java

@@ -20,8 +20,8 @@
 package org.elasticsearch.plugins;
 
 import org.apache.lucene.util.IOUtils;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.cli.Terminal.Verbosity;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.cli.Terminal.Verbosity;
 import org.elasticsearch.env.Environment;
 
 import java.io.IOException;

+ 32 - 19
core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java

@@ -19,40 +19,55 @@
 
 package org.elasticsearch.plugins;
 
-import org.apache.lucene.util.IOUtils;
-import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.cli.UserError;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.List;
 
-import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE;
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
+import org.apache.lucene.util.IOUtils;
+import org.elasticsearch.cli.Command;
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.UserError;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.env.Environment;
+
+import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE;
 
 /**
  * A command for the plugin cli to remove a plugin from elasticsearch.
  */
-class RemovePluginCommand extends CliTool.Command {
-    private final String pluginName;
+class RemovePluginCommand extends Command {
+
+    private final Environment env;
+    private final OptionSpec<String> arguments;
 
-    public RemovePluginCommand(Terminal terminal, String pluginName) {
-        super(terminal);
-        this.pluginName = pluginName;
+    RemovePluginCommand(Environment env) {
+        super("Removes a plugin from elasticsearch");
+        this.env = env;
+        this.arguments = parser.nonOptions("plugin name");
     }
 
     @Override
-    public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
+    protected void execute(Terminal terminal, OptionSet options) throws Exception {
+        // TODO: in jopt-simple 5.0 we can enforce a min/max number of positional args
+        List<String> args = arguments.values(options);
+        if (args.size() != 1) {
+            throw new UserError(ExitCodes.USAGE, "Must supply a single plugin id argument");
+        }
+        execute(terminal, args.get(0));
+    }
+
+    // pkg private for testing
+    void execute(Terminal terminal, String pluginName) throws Exception {
         terminal.println("-> Removing " + Strings.coalesceToEmpty(pluginName) + "...");
 
         Path pluginDir = env.pluginsFile().resolve(pluginName);
         if (Files.exists(pluginDir) == false) {
-            throw new UserError(CliTool.ExitStatus.USAGE, "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins.");
+            throw new UserError(ExitCodes.USAGE, "Plugin " + pluginName + " not found. Run 'plugin list' to get list of installed plugins.");
         }
 
         List<Path> pluginPaths = new ArrayList<>();
@@ -60,7 +75,7 @@ class RemovePluginCommand extends CliTool.Command {
         Path pluginBinDir = env.binFile().resolve(pluginName);
         if (Files.exists(pluginBinDir)) {
             if (Files.isDirectory(pluginBinDir) == false) {
-                throw new UserError(CliTool.ExitStatus.IO_ERROR, "Bin dir for " + pluginName + " is not a directory");
+                throw new UserError(ExitCodes.IO_ERROR, "Bin dir for " + pluginName + " is not a directory");
             }
             pluginPaths.add(pluginBinDir);
             terminal.println(VERBOSE, "Removing: " + pluginBinDir);
@@ -72,7 +87,5 @@ class RemovePluginCommand extends CliTool.Command {
         pluginPaths.add(tmpPluginDir);
 
         IOUtils.rm(pluginPaths.toArray(new Path[pluginPaths.size()]));
-
-        return CliTool.ExitStatus.OK;
     }
 }

+ 0 - 28
core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-start.help

@@ -1,28 +0,0 @@
-NAME
-
-    start - Start Elasticsearch
-
-SYNOPSIS
-
-    elasticsearch start
-
-DESCRIPTION
-
-    This command starts Elasticsearch. You can configure it to run in the foreground, write a pid file
-    and configure arbitrary options that override file-based configuration.
-
-OPTIONS
-
-    -h,--help                    Shows this message
-
-    -p,--pidfile <pidfile>       Creates a pid file in the specified path on start
-
-    -d,--daemonize               Starts Elasticsearch in the background
-
-    -Dproperty=value             Configures an Elasticsearch specific property, like -Dnetwork.host=127.0.0.1
-
-    --property=value             Configures an elasticsearch specific property, like --network.host 127.0.0.1
-    --property value
-
-    NOTE: The -d, -p, and -D arguments must appear before any --property arguments.
-

+ 0 - 16
core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch-version.help

@@ -1,16 +0,0 @@
-NAME
-
-    version - Show version information and exit
-
-SYNOPSIS
-
-    elasticsearch version
-
-DESCRIPTION
-
-    This command shows Elasticsearch version, timestamp and build information as well as JVM info
-
-OPTIONS
-
-    -h,--help                    Shows this message
-

+ 0 - 22
core/src/main/resources/org/elasticsearch/bootstrap/elasticsearch.help

@@ -1,22 +0,0 @@
-NAME
-
-    elasticsearch - Manages elasticsearch
-
-SYNOPSIS
-
-    elasticsearch <command>
-
-DESCRIPTION
-
-    Start an elasticsearch node
-
-COMMANDS
-
-    start      Start elasticsearch
-
-    version    Show version information and exit
-
-NOTES
-
-    [*] For usage help on specific commands please type "elasticsearch <command> -h"
-

+ 0 - 59
core/src/main/resources/org/elasticsearch/plugins/plugin-install.help

@@ -1,59 +0,0 @@
-NAME
-
-    install - Install a plugin
-
-SYNOPSIS
-
-    plugin install <name or url>
-
-DESCRIPTION
-
-    This command installs an elasticsearch plugin. It can be used as follows:
-
-    Officially supported or commercial plugins require just the plugin name:
-
-        plugin install analysis-icu
-        plugin install x-pack
-
-    Plugins from Maven Central require 'groupId:artifactId:version':
-
-        plugin install org.elasticsearch:mapper-attachments:3.0.0
-
-    Plugins can be installed from a custom URL or file location as follows:
-
-        plugin install http://some.domain.name//my-plugin-1.0.0.zip
-        plugin install file:/path/to/my-plugin-1.0.0.zip
-
-OFFICIAL PLUGINS
-
-    The following plugins are officially supported and can be installed by just referring to their name
-
-    - analysis-icu
-    - analysis-kuromoji
-    - analysis-phonetic
-    - analysis-smartcn
-    - analysis-stempel
-    - delete-by-query
-    - discovery-azure
-    - discovery-ec2
-    - discovery-gce
-    - ingest-geoip
-    - lang-javascript
-    - lang-painless
-    - lang-python
-    - mapper-attachments
-    - mapper-murmur3
-    - mapper-size
-    - repository-azure
-    - repository-hdfs
-    - repository-s3
-    - store-smb
-
-
-OPTIONS
-
-    -v,--verbose                 Verbose output
-
-    -h,--help                    Shows this message
-
-    -b,--batch                   Enable batch mode explicitly, automatic confirmation of security permissions

+ 0 - 12
core/src/main/resources/org/elasticsearch/plugins/plugin-list.help

@@ -1,12 +0,0 @@
-NAME
-
-    list - List all plugins
-
-SYNOPSIS
-
-    plugin list
-
-DESCRIPTION
-
-    This command lists all installed elasticsearch plugins
-

+ 0 - 12
core/src/main/resources/org/elasticsearch/plugins/plugin-remove.help

@@ -1,12 +0,0 @@
-NAME
-
-    remove - Remove a plugin
-
-SYNOPSIS
-
-    plugin remove <name>
-
-DESCRIPTION
-
-    This command removes an elasticsearch plugin
-

+ 0 - 24
core/src/main/resources/org/elasticsearch/plugins/plugin.help

@@ -1,24 +0,0 @@
-NAME
-
-    plugin - Manages plugins
-
-SYNOPSIS
-
-    plugin <command>
-
-DESCRIPTION
-
-    Manage plugins
-
-COMMANDS
-
-    install    Install a plugin
-
-    remove     Remove a plugin
-
-    list       List installed plugins
-
-NOTES
-
-    [*] For usage help on specific commands please type "plugin <command> -h"
-

+ 123 - 0
core/src/test/java/org/elasticsearch/cli/CommandTests.java

@@ -0,0 +1,123 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+import joptsimple.OptionSet;
+import org.elasticsearch.test.ESTestCase;
+
+public class CommandTests extends ESTestCase {
+
+    static class UserErrorCommand extends Command {
+        UserErrorCommand() {
+            super("Throws a user error");
+        }
+        @Override
+        protected void execute(Terminal terminal, OptionSet options) throws Exception {
+            throw new UserError(ExitCodes.DATA_ERROR, "Bad input");
+        }
+    }
+
+    static class NoopCommand extends Command {
+        boolean executed = false;
+        NoopCommand() {
+            super("Does nothing");
+        }
+        @Override
+        protected void execute(Terminal terminal, OptionSet options) throws Exception {
+            terminal.println("Normal output");
+            terminal.println(Terminal.Verbosity.SILENT, "Silent output");
+            terminal.println(Terminal.Verbosity.VERBOSE, "Verbose output");
+            executed = true;
+        }
+        @Override
+        protected void printAdditionalHelp(Terminal terminal) {
+            terminal.println("Some extra help");
+        }
+    }
+
+    public void testHelp() throws Exception {
+        NoopCommand command = new NoopCommand();
+        MockTerminal terminal = new MockTerminal();
+        String[] args = {"-h"};
+        int status = command.main(args, terminal);
+        String output = terminal.getOutput();
+        assertEquals(output, ExitCodes.OK, status);
+        assertTrue(output, output.contains("Does nothing"));
+        assertTrue(output, output.contains("Some extra help"));
+        assertFalse(command.executed);
+
+        command = new NoopCommand();
+        String[] args2 = {"--help"};
+        status = command.main(args2, terminal);
+        output = terminal.getOutput();
+        assertEquals(output, ExitCodes.OK, status);
+        assertTrue(output, output.contains("Does nothing"));
+        assertTrue(output, output.contains("Some extra help"));
+        assertFalse(command.executed);
+    }
+
+    public void testVerbositySilentAndVerbose() throws Exception {
+        MockTerminal terminal = new MockTerminal();
+        NoopCommand command = new NoopCommand();
+        String[] args = {"-v", "-s"};
+        UserError e = expectThrows(UserError.class, () -> {
+            command.mainWithoutErrorHandling(args, terminal);
+        });
+        assertTrue(e.getMessage(), e.getMessage().contains("Cannot specify -s and -v together"));
+    }
+
+    public void testSilentVerbosity() throws Exception {
+        MockTerminal terminal = new MockTerminal();
+        NoopCommand command = new NoopCommand();
+        String[] args = {"-s"};
+        command.main(args, terminal);
+        String output = terminal.getOutput();
+        assertTrue(output, output.contains("Silent output"));
+    }
+
+    public void testNormalVerbosity() throws Exception {
+        MockTerminal terminal = new MockTerminal();
+        terminal.setVerbosity(Terminal.Verbosity.SILENT);
+        NoopCommand command = new NoopCommand();
+        String[] args = {};
+        command.main(args, terminal);
+        String output = terminal.getOutput();
+        assertTrue(output, output.contains("Normal output"));
+    }
+
+    public void testVerboseVerbosity() throws Exception {
+        MockTerminal terminal = new MockTerminal();
+        NoopCommand command = new NoopCommand();
+        String[] args = {"-v"};
+        command.main(args, terminal);
+        String output = terminal.getOutput();
+        assertTrue(output, output.contains("Verbose output"));
+    }
+
+    public void testUserError() throws Exception {
+        MockTerminal terminal = new MockTerminal();
+        UserErrorCommand command = new UserErrorCommand();
+        String[] args = {};
+        int status = command.main(args, terminal);
+        String output = terminal.getOutput();
+        assertEquals(output, ExitCodes.DATA_ERROR, status);
+        assertTrue(output, output.contains("ERROR: Bad input"));
+    }
+}

+ 105 - 0
core/src/test/java/org/elasticsearch/cli/MultiCommandTests.java

@@ -0,0 +1,105 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+import joptsimple.OptionSet;
+import org.junit.Before;
+
+public class MultiCommandTests extends CommandTestCase {
+
+    static class DummyMultiCommand extends MultiCommand {
+        DummyMultiCommand() {
+            super("A dummy multi command");
+        }
+    }
+
+    static class DummySubCommand extends Command {
+        DummySubCommand() {
+            super("A dummy subcommand");
+        }
+        @Override
+        protected void execute(Terminal terminal, OptionSet options) throws Exception {
+            terminal.println("Arguments: " + options.nonOptionArguments().toString());
+        }
+    }
+
+    DummyMultiCommand multiCommand;
+
+    @Before
+    public void setupCommand() {
+        multiCommand = new DummyMultiCommand();
+    }
+
+    @Override
+    protected Command newCommand() {
+        return multiCommand;
+    }
+
+    public void testNoCommandsConfigured() throws Exception {
+        IllegalStateException e = expectThrows(IllegalStateException.class, () -> {
+            execute();
+        });
+        assertEquals("No subcommands configured", e.getMessage());
+    }
+
+    public void testUnknownCommand() throws Exception {
+        multiCommand.subcommands.put("something", new DummySubCommand());
+        UserError e = expectThrows(UserError.class, () -> {
+            execute("somethingelse");
+        });
+        assertEquals(ExitCodes.USAGE, e.exitCode);
+        assertEquals("Unknown command [somethingelse]", e.getMessage());
+    }
+
+    public void testMissingCommand() throws Exception {
+        multiCommand.subcommands.put("command1", new DummySubCommand());
+        UserError e = expectThrows(UserError.class, () -> {
+            execute();
+        });
+        assertEquals(ExitCodes.USAGE, e.exitCode);
+        assertEquals("Missing command", e.getMessage());
+    }
+
+    public void testHelp() throws Exception {
+        multiCommand.subcommands.put("command1", new DummySubCommand());
+        multiCommand.subcommands.put("command2", new DummySubCommand());
+        execute("-h");
+        String output = terminal.getOutput();
+        assertTrue(output, output.contains("command1"));
+        assertTrue(output, output.contains("command2"));
+    }
+
+    public void testSubcommandHelp() throws Exception {
+        multiCommand.subcommands.put("command1", new DummySubCommand());
+        multiCommand.subcommands.put("command2", new DummySubCommand());
+        execute("command2", "-h");
+        String output = terminal.getOutput();
+        assertFalse(output, output.contains("command1"));
+        assertTrue(output, output.contains("A dummy subcommand"));
+    }
+
+    public void testSubcommandArguments() throws Exception {
+        multiCommand.subcommands.put("command1", new DummySubCommand());
+        execute("command1", "foo", "bar");
+        String output = terminal.getOutput();
+        assertFalse(output, output.contains("command1"));
+        assertTrue(output, output.contains("Arguments: [foo, bar]"));
+    }
+}

+ 5 - 3
core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java → core/src/test/java/org/elasticsearch/cli/TerminalTests.java

@@ -17,9 +17,11 @@
  * under the License.
  */
 
-package org.elasticsearch.common.cli;
+package org.elasticsearch.cli;
 
-public class TerminalTests extends CliToolTestCase {
+import org.elasticsearch.test.ESTestCase;
+
+public class TerminalTests extends ESTestCase {
     public void testVerbosity() throws Exception {
         MockTerminal terminal = new MockTerminal();
         terminal.setVerbosity(Terminal.Verbosity.SILENT);
@@ -48,7 +50,7 @@ public class TerminalTests extends CliToolTestCase {
         logTerminal.println(verbosity, text);
         String output = logTerminal.getOutput();
         assertTrue(output, output.contains(text));
-        logTerminal.resetOutput();
+        logTerminal.reset();
     }
 
     private void assertNotPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception {

+ 1 - 1
core/src/test/java/org/elasticsearch/common/logging/LoggingConfigurationTests.java

@@ -27,7 +27,7 @@ import java.util.Arrays;
 
 import org.apache.log4j.Appender;
 import org.apache.log4j.Logger;
-import org.elasticsearch.common.cli.MockTerminal;
+import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;

+ 1 - 1
core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java

@@ -24,7 +24,7 @@ import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
-import org.elasticsearch.common.cli.MockTerminal;
+import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;

+ 0 - 51
core/src/test/java/org/elasticsearch/plugins/PluginCliTests.java

@@ -1,51 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.plugins;
-
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasItem;
-import static org.hamcrest.Matchers.is;
-
-public class PluginCliTests extends CliToolTestCase {
-    public void testHelpWorks() throws Exception {
-        MockTerminal terminal = new MockTerminal();
-        assertThat(new PluginCli(terminal).execute(args("--help")), is(OK_AND_EXIT));
-        assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin.help");
-
-        terminal.resetOutput();
-        assertThat(new PluginCli(terminal).execute(args("install -h")), is(OK_AND_EXIT));
-        assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-install.help");
-        for (String plugin : InstallPluginCommand.OFFICIAL_PLUGINS) {
-            assertThat(terminal.getOutput(), containsString(plugin));
-        }
-
-        terminal.resetOutput();
-        assertThat(new PluginCli(terminal).execute(args("remove --help")), is(OK_AND_EXIT));
-        assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-remove.help");
-
-        terminal.resetOutput();
-        assertThat(new PluginCli(terminal).execute(args("list -h")), is(OK_AND_EXIT));
-        assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/plugins/plugin-list.help");
-    }
-}

+ 0 - 1
distribution/licenses/commons-cli-1.3.1.jar.sha1

@@ -1 +0,0 @@
-1303efbc4b181e5a58bf2e967dc156a3132b97c0

+ 1 - 0
distribution/licenses/jopt-simple-4.9.jar.sha1

@@ -0,0 +1 @@
+ee9e9eaa0a35360dcfeac129ff4923215fd65904

+ 24 - 0
distribution/licenses/jopt-simple-LICENSE.txt

@@ -0,0 +1,24 @@
+/*
+ The MIT License
+
+ Copyright (c) 2004-2015 Paul R. Holser, Jr.
+
+ Permission is hereby granted, free of charge, to any person obtaining
+ a copy of this software and associated documentation files (the
+ "Software"), to deal in the Software without restriction, including
+ without limitation the rights to use, copy, modify, merge, publish,
+ distribute, sublicense, and/or sell copies of the Software, and to
+ permit persons to whom the Software is furnished to do so, subject to
+ the following conditions:
+
+ The above copyright notice and this permission notice shall be
+ included in all copies or substantial portions of the Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+ LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+ OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/

+ 0 - 0
distribution/licenses/jopt-simple-NOTICE.txt


+ 0 - 38
docs/plugins/mapper-attachments.asciidoc

@@ -405,41 +405,3 @@ It gives back:
    }
 }
 --------------------------
-
-[[mapper-attachments-standalone]]
-==== Stand alone runner
-
-If you want to run some tests within your IDE, you can use `StandaloneRunner` class.
-It accepts arguments:
-
-* `-u file://URL/TO/YOUR/DOC`
-* `--size` set extracted size (default to mapper attachment size)
-* `BASE64` encoded binary
-
-Example:
-
-[source,sh]
---------------------------
-StandaloneRunner BASE64Text
-StandaloneRunner -u /tmp/mydoc.pdf
-StandaloneRunner -u /tmp/mydoc.pdf --size 1000000
---------------------------
-
-It produces something like:
-
-[source,text]
---------------------------
-## Extracted text
---------------------- BEGIN -----------------------
-This is the extracted text
----------------------- END ------------------------
-## Metadata
-- author: null
-- content_length: null
-- content_type: application/pdf
-- date: null
-- keywords: null
-- language: null
-- name: null
-- title: null
---------------------------

+ 11 - 0
modules/lang-groovy/build.gradle

@@ -38,6 +38,17 @@ thirdPartyAudit.excludes = [
   // for example we do not need ivy, scripts arent allowed to download code
   'com.thoughtworks.xstream.XStream', 
   'groovyjarjarasm.asm.util.Textifiable', 
+  // commons-cli is referenced by groovy, even though they supposedly
+  // jarjar it. Since we don't use the cli, we don't need the dep.
+  'org.apache.commons.cli.CommandLine',
+  'org.apache.commons.cli.CommandLineParser',
+  'org.apache.commons.cli.GnuParser',
+  'org.apache.commons.cli.HelpFormatter',
+  'org.apache.commons.cli.Option',
+  'org.apache.commons.cli.OptionBuilder',
+  'org.apache.commons.cli.Options',
+  'org.apache.commons.cli.Parser',
+  'org.apache.commons.cli.PosixParser',
   'org.apache.ivy.Ivy', 
   'org.apache.ivy.core.event.IvyListener', 
   'org.apache.ivy.core.event.download.PrepareDownloadEvent', 

+ 0 - 189
plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java

@@ -1,189 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.mapper.attachments;
-
-import org.apache.commons.cli.CommandLine;
-import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolConfig;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.compress.CompressedXContent;
-import org.elasticsearch.common.io.PathUtils;
-import org.elasticsearch.common.io.stream.BytesStreamOutput;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.index.MapperTestUtils;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.DocumentMapperParser;
-import org.elasticsearch.index.mapper.ParseContext;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.Locale;
-
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.option;
-import static org.elasticsearch.common.io.Streams.copy;
-import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-import static org.elasticsearch.mapper.attachments.AttachmentUnitTestCase.getIndicesModuleWithRegisteredAttachmentMapper;
-import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
-
-/**
- * This class provides a simple main class which can be used to test what is extracted from a given binary file.
- * You can run it using
- *  -u file://URL/TO/YOUR/DOC
- *  --size set extracted size (default to mapper attachment size)
- *  BASE64 encoded binary
- *
- * Example:
- *  StandaloneRunner BASE64Text
- *  StandaloneRunner -u /tmp/mydoc.pdf
- *  StandaloneRunner -u /tmp/mydoc.pdf --size 1000000
- */
-@SuppressForbidden(reason = "commandline tool")
-public class StandaloneRunner extends CliTool {
-
-    private static final CliToolConfig CONFIG = CliToolConfig.config("tika", StandaloneRunner.class)
-                        .cmds(TikaRunner.CMD)
-                .build();
-
-    static {
-        System.setProperty("es.path.home", "/tmp");
-    }
-
-    static class TikaRunner extends Command {
-        private static final String NAME = "tika";
-        private final String url;
-        private final Integer size;
-        private final String base64text;
-        private final DocumentMapper docMapper;
-
-        private static final CliToolConfig.Cmd CMD = cmd(NAME, TikaRunner.class)
-                .options(option("u", "url").required(false).hasArg(false))
-                .options(option("t", "size").required(false).hasArg(false))
-                .build();
-
-        protected TikaRunner(Terminal terminal, String url, Integer size, String base64text) throws IOException {
-            super(terminal);
-            this.size = size;
-            this.url = url;
-            this.base64text = base64text;
-            DocumentMapperParser mapperParser = MapperTestUtils.newMapperService(PathUtils.get("."), Settings.EMPTY, getIndicesModuleWithRegisteredAttachmentMapper()).documentMapperParser(); // use CWD b/c it won't be used
-
-            String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/attachment/test/standalone/standalone-mapping.json");
-            docMapper = mapperParser.parse("person", new CompressedXContent(mapping));
-        }
-
-        @Override
-        public ExitStatus execute(Settings settings, Environment env) throws Exception {
-            XContentBuilder builder = jsonBuilder().startObject().field("file").startObject();
-
-            if (base64text != null) {
-                // If base64 is provided
-                builder.field("_content", base64text);
-            } else {
-                // A file is provided
-                byte[] bytes = copyToBytes(PathUtils.get(url));
-                builder.field("_content", bytes);
-            }
-
-            if (size >= 0) {
-                builder.field("_indexed_chars", size);
-            }
-
-            BytesReference json = builder.endObject().endObject().bytes();
-
-            ParseContext.Document doc = docMapper.parse("person", "person", "1", json).rootDoc();
-
-            terminal.println("## Extracted text");
-            terminal.println("--------------------- BEGIN -----------------------");
-            terminal.println(doc.get("file.content"));
-            terminal.println("---------------------- END ------------------------");
-            terminal.println("## Metadata");
-            printMetadataContent(doc, AttachmentMapper.FieldNames.AUTHOR);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.CONTENT_LENGTH);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.CONTENT_TYPE);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.DATE);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.KEYWORDS);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.LANGUAGE);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.NAME);
-            printMetadataContent(doc, AttachmentMapper.FieldNames.TITLE);
-
-            return ExitStatus.OK;
-        }
-
-        private void printMetadataContent(ParseContext.Document doc, String field) {
-            terminal.println("- " + field + ":" + doc.get(docMapper.mappers().getMapper("file." + field).fieldType().name()));
-        }
-
-        public static byte[] copyToBytes(Path path) throws IOException {
-            try (InputStream is = Files.newInputStream(path);
-                 BytesStreamOutput out = new BytesStreamOutput()) {
-                copy(is, out);
-                return out.bytes().toBytes();
-            }
-        }
-
-        public static Command parse(Terminal terminal, CommandLine cli) throws IOException {
-            String url = cli.getOptionValue("u");
-            String base64text = null;
-            String sSize = cli.getOptionValue("size");
-            Integer size = sSize != null ? Integer.parseInt(sSize) : -1;
-            if (url == null && cli.getArgs().length == 0) {
-                    return exitCmd(ExitStatus.USAGE, terminal, "url or BASE64 content should be provided (type -h for help)");
-            }
-            if (url == null) {
-                if (cli.getArgs().length == 0) {
-                    return exitCmd(ExitStatus.USAGE, terminal, "url or BASE64 content should be provided (type -h for help)");
-                }
-                base64text = cli.getArgs()[0];
-            } else {
-                if (cli.getArgs().length == 1) {
-                    return exitCmd(ExitStatus.USAGE, terminal, "url or BASE64 content should be provided. Not both. (type -h for help)");
-                }
-            }
-            return new TikaRunner(terminal, url, size, base64text);
-        }
-    }
-
-    public StandaloneRunner() {
-        super(CONFIG);
-    }
-
-
-    public static void main(String[] args) throws Exception {
-        StandaloneRunner pluginManager = new StandaloneRunner();
-        pluginManager.execute(args);
-    }
-
-    @Override
-    protected Command parse(String cmdName, CommandLine cli) throws Exception {
-        switch (cmdName.toLowerCase(Locale.ROOT)) {
-            case TikaRunner.NAME: return TikaRunner.parse(terminal, cli);
-            default:
-                    assert false : "can't get here as cmd name is validated before this method is called";
-                    return exitCmd(ExitStatus.CODE_ERROR);
-        }
-    }
-}

+ 1 - 0
plugins/repository-hdfs/build.gradle

@@ -45,6 +45,7 @@ dependencies {
   compile 'com.google.guava:guava:16.0.1'
   compile 'com.google.protobuf:protobuf-java:2.5.0'
   compile 'commons-logging:commons-logging:1.1.3'
+  compile 'commons-cli:commons-cli:1.2'
   compile 'commons-collections:commons-collections:3.2.2'
   compile 'commons-configuration:commons-configuration:1.6'
   compile 'commons-io:commons-io:2.4'

+ 1 - 0
plugins/repository-hdfs/licenses/commons-cli-1.2.jar.sha1

@@ -0,0 +1 @@
+2bf96b7aa8b611c177d329452af1dc933e14501c

+ 0 - 0
distribution/licenses/commons-cli-LICENSE.txt → plugins/repository-hdfs/licenses/commons-cli-LICENSE.txt


+ 0 - 0
distribution/licenses/commons-cli-NOTICE.txt → plugins/repository-hdfs/licenses/commons-cli-NOTICE.txt


+ 64 - 173
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/BootstrapCliParserTests.java

@@ -19,18 +19,6 @@
 
 package org.elasticsearch.bootstrap;
 
-import org.elasticsearch.Build;
-import org.elasticsearch.Version;
-import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.cli.CliTool.ExitStatus;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.UserError;
-import org.elasticsearch.common.collect.Tuple;
-import org.elasticsearch.monitor.jvm.JvmInfo;
-import org.junit.After;
-import org.junit.Before;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -38,18 +26,27 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK;
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT;
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.USAGE;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasItem;
+import joptsimple.OptionException;
+import org.elasticsearch.Build;
+import org.elasticsearch.Version;
+import org.elasticsearch.cli.Command;
+import org.elasticsearch.cli.CommandTestCase;
+import org.elasticsearch.cli.UserError;
+import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.monitor.jvm.JvmInfo;
+import org.junit.After;
+import org.junit.Before;
+
 import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.nullValue;
 
 @SuppressForbidden(reason = "modifies system properties intentionally")
-public class BootstrapCliParserTests extends CliToolTestCase {
+public class BootstrapCliParserTests extends CommandTestCase {
+
+    @Override
+    protected Command newCommand() {
+        return new BootstrapCliParser();
+    }
 
-    private MockTerminal terminal = new MockTerminal();
     private List<String> propertiesToClear = new ArrayList<>();
     private Map<Object, Object> properties;
 
@@ -67,195 +64,93 @@ public class BootstrapCliParserTests extends CliToolTestCase {
         assertEquals("properties leaked", properties, new HashMap<>(System.getProperties()));
     }
 
-    public void testThatVersionIsReturned() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        ExitStatus status = parser.execute(args("version"));
-        assertStatus(status, OK_AND_EXIT);
-
-        String output = terminal.getOutput();
-        assertTrue(output, output.contains(Version.CURRENT.toString()));
-        assertTrue(output, output.contains(Build.CURRENT.shortHash()));
-        assertTrue(output, output.contains(Build.CURRENT.date()));
-        assertTrue(output, output.contains(JvmInfo.jvmInfo().version()));
+    void assertShouldRun(boolean shouldRun) {
+        BootstrapCliParser parser = (BootstrapCliParser)command;
+        assertEquals(shouldRun, parser.shouldRun());
     }
 
-    public void testThatVersionIsReturnedAsStartParameter() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        ExitStatus status = parser.execute(args("start -V"));
-        assertStatus(status, OK_AND_EXIT);
-
-        String output = terminal.getOutput();
+    public void testVersion() throws Exception {
+        String output = execute("-V");
         assertTrue(output, output.contains(Version.CURRENT.toString()));
         assertTrue(output, output.contains(Build.CURRENT.shortHash()));
         assertTrue(output, output.contains(Build.CURRENT.date()));
         assertTrue(output, output.contains(JvmInfo.jvmInfo().version()));
+        assertShouldRun(false);
 
-        terminal.resetOutput();
-        parser = new BootstrapCLIParser(terminal);
-        status = parser.execute(args("start --version"));
-        assertStatus(status, OK_AND_EXIT);
-
-        output = terminal.getOutput();
+        terminal.reset();
+        output = execute("--version");
         assertTrue(output, output.contains(Version.CURRENT.toString()));
         assertTrue(output, output.contains(Build.CURRENT.shortHash()));
         assertTrue(output, output.contains(Build.CURRENT.date()));
         assertTrue(output, output.contains(JvmInfo.jvmInfo().version()));
+        assertShouldRun(false);
     }
 
-    public void testThatPidFileCanBeConfigured() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
+    public void testPidfile() throws Exception {
         registerProperties("es.pidfile");
 
-        ExitStatus status = parser.execute(args("start --pidfile")); // missing pid file
-        assertStatus(status, USAGE);
+        // missing argument
+        OptionException e = expectThrows(OptionException.class, () -> {
+           execute("-p");
+        });
+        assertEquals("Option p/pidfile requires an argument", e.getMessage());
+        assertShouldRun(false);
 
         // good cases
-        status = parser.execute(args("start --pidfile /tmp/pid"));
-        assertStatus(status, OK);
+        terminal.reset();
+        execute("--pidfile", "/tmp/pid");
         assertSystemProperty("es.pidfile", "/tmp/pid");
+        assertShouldRun(true);
 
         System.clearProperty("es.pidfile");
-        status = parser.execute(args("start -p /tmp/pid"));
-        assertStatus(status, OK);
+        terminal.reset();
+        execute("-p", "/tmp/pid");
         assertSystemProperty("es.pidfile", "/tmp/pid");
+        assertShouldRun(true);
     }
 
-    public void testThatParsingDaemonizeWorks() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
+    public void testNoDaemonize() throws Exception {
         registerProperties("es.foreground");
 
-        ExitStatus status = parser.execute(args("start -d"));
-        assertStatus(status, OK);
-        assertThat(System.getProperty("es.foreground"), is("false"));
+        execute();
+        assertSystemProperty("es.foreground", null);
+        assertShouldRun(true);
     }
 
-    public void testThatNotDaemonizingDoesNotConfigureProperties() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
+    public void testDaemonize() throws Exception {
         registerProperties("es.foreground");
 
-        ExitStatus status = parser.execute(args("start"));
-        assertStatus(status, OK);
-        assertThat(System.getProperty("es.foreground"), is(nullValue()));
+        execute("-d");
+        assertSystemProperty("es.foreground", "false");
+        assertShouldRun(true);
+
+        System.clearProperty("es.foreground");
+        execute("--daemonize");
+        assertSystemProperty("es.foreground", "false");
+        assertShouldRun(true);
     }
 
-    public void testThatJavaPropertyStyleArgumentsCanBeParsed() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
+    public void testConfig() throws Exception {
         registerProperties("es.foo", "es.spam");
 
-        ExitStatus status = parser.execute(args("start -Dfoo=bar -Dspam=eggs"));
-        assertStatus(status, OK);
+        execute("-Dfoo=bar", "-Dspam=eggs");
         assertSystemProperty("es.foo", "bar");
         assertSystemProperty("es.spam", "eggs");
+        assertShouldRun(true);
     }
 
-    public void testThatJavaPropertyStyleArgumentsWithEsPrefixAreNotPrefixedTwice() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        registerProperties("es.spam", "es.pidfile");
-
-        ExitStatus status = parser.execute(args("start -Des.pidfile=/path/to/foo/elasticsearch/distribution/zip/target/integ-tests/es.pid -Dspam=eggs"));
-        assertStatus(status, OK);
-        assertThat(System.getProperty("es.es.pidfile"), is(nullValue()));
-        assertSystemProperty("es.pidfile", "/path/to/foo/elasticsearch/distribution/zip/target/integ-tests/es.pid");
-        assertSystemProperty("es.spam", "eggs");
-    }
-
-    public void testThatUnknownLongOptionsCanBeParsed() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        registerProperties("es.network.host", "es.my.option");
-
-        ExitStatus status = parser.execute(args("start --network.host 127.0.0.1 --my.option=true"));
-        assertStatus(status, OK);
-        assertSystemProperty("es.network.host", "127.0.0.1");
-        assertSystemProperty("es.my.option", "true");
-    }
-
-    public void testThatUnknownLongOptionsNeedAValue() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        registerProperties("es.network.host");
-
-        ExitStatus status = parser.execute(args("start --network.host"));
-        assertStatus(status, USAGE);
-        String output = terminal.getOutput();
-        assertTrue(output, output.contains("Parameter [network.host] needs value"));
-
-        terminal.resetOutput();
-        status = parser.execute(args("start --network.host --foo"));
-        assertStatus(status, USAGE);
-        output = terminal.getOutput();
-        assertTrue(output, output.contains("Parameter [network.host] needs value"));
-    }
-
-    public void testParsingErrors() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-
-        // unknown params
-        ExitStatus status = parser.execute(args("version --unknown-param /tmp/pid"));
-        assertStatus(status, USAGE);
-        String output = terminal.getOutput();
-        assertTrue(output, output.contains("Unrecognized option: --unknown-param"));
-
-        // single dash in extra params
-        terminal.resetOutput();
-        parser = new BootstrapCLIParser(terminal);
-        status = parser.execute(args("start -network.host 127.0.0.1"));
-        assertStatus(status, USAGE);
-        output = terminal.getOutput();
-        assertTrue(output, output.contains("Parameter [-network.host]does not start with --"));
-
-        // never ended parameter
-        terminal = new MockTerminal();
-        parser = new BootstrapCLIParser(terminal);
-        status = parser.execute(args("start --network.host"));
-        assertStatus(status, USAGE);
-        output = terminal.getOutput();
-        assertTrue(output, output.contains("Parameter [network.host] needs value"));
-
-        // free floating value
-        terminal = new MockTerminal();
-        parser = new BootstrapCLIParser(terminal);
-        status = parser.execute(args("start 127.0.0.1"));
-        assertStatus(status, USAGE);
-        output = terminal.getOutput();
-        assertTrue(output, output.contains("Parameter [127.0.0.1]does not start with --"));
-    }
-
-    public void testHelpWorks() throws Exception {
-        List<Tuple<String, String>> tuples = new ArrayList<>();
-        tuples.add(new Tuple<>("version --help", "elasticsearch-version.help"));
-        tuples.add(new Tuple<>("version -h", "elasticsearch-version.help"));
-        tuples.add(new Tuple<>("start --help", "elasticsearch-start.help"));
-        tuples.add(new Tuple<>("start -h", "elasticsearch-start.help"));
-        tuples.add(new Tuple<>("--help", "elasticsearch.help"));
-        tuples.add(new Tuple<>("-h", "elasticsearch.help"));
-
-        for (Tuple<String, String> tuple : tuples) {
-            terminal.resetOutput();
-            BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-            ExitStatus status = parser.execute(args(tuple.v1()));
-            assertStatus(status, OK_AND_EXIT);
-            assertTerminalOutputContainsHelpFile(terminal, "/org/elasticsearch/bootstrap/" + tuple.v2());
-        }
-    }
-
-    public void testThatSpacesInParametersAreSupported() throws Exception {
-        // emulates: bin/elasticsearch --node.name "'my node with spaces'" --pidfile "'/tmp/my pid.pid'"
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        registerProperties("es.pidfile", "es.my.param");
-
-        ExitStatus status = parser.execute("start", "--pidfile", "foo with space", "--my.param", "my awesome neighbour");
-        assertStatus(status, OK);
-        assertSystemProperty("es.pidfile", "foo with space");
-        assertSystemProperty("es.my.param", "my awesome neighbour");
-
+    public void testConfigMalformed() throws Exception {
+        UserError e = expectThrows(UserError.class, () -> {
+            execute("-Dfoo");
+        });
+        assertTrue(e.getMessage(), e.getMessage().contains("Malformed elasticsearch setting"));
     }
 
-    public void testThatHelpfulErrorMessageIsGivenWhenParametersAreOutOfOrder() throws Exception {
-        BootstrapCLIParser parser = new BootstrapCLIParser(terminal);
-        UserError e = expectThrows(UserError.class, () -> {
-                parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"});
+    public void testUnknownOption() throws Exception {
+        OptionException e = expectThrows(OptionException.class, () -> {
+            execute("--network.host");
         });
-        assertThat(e.getMessage(), containsString("must be before any parameters starting with --"));
-        assertNull(System.getProperty("es.foo"));
+        assertTrue(e.getMessage(), e.getMessage().contains("network.host is not a recognized option"));
     }
 
     private void registerProperties(String ... systemProperties) {
@@ -266,8 +161,4 @@ public class BootstrapCliParserTests extends CliToolTestCase {
         String msg = String.format(Locale.ROOT, "Expected property %s to be %s, terminal output was %s", name, expectedValue, terminal.getOutput());
         assertThat(msg, System.getProperty(name), is(expectedValue));
     }
-
-    private void assertStatus(ExitStatus status, ExitStatus expectedStatus) throws Exception {
-        assertThat(String.format(Locale.ROOT, "Expected status to be [%s], but was [%s], terminal output was %s", expectedStatus, status, terminal.getOutput()), status, is(expectedStatus));
-    }
 }

+ 0 - 329
qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CheckFileCommandTests.java

@@ -1,329 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.cli;
-
-import com.google.common.jimfs.Configuration;
-import com.google.common.jimfs.Jimfs;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.set.Sets;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.test.ESTestCase;
-
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.FileSystem;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.attribute.GroupPrincipal;
-import java.nio.file.attribute.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFileAttributes;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.UserPrincipal;
-import java.util.Set;
-
-import static org.hamcrest.Matchers.allOf;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasItem;
-import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.is;
-
-/**
- *
- */
-public class CheckFileCommandTests extends ESTestCase {
-
-    private MockTerminal captureOutputTerminal = new MockTerminal();
-
-    private Configuration jimFsConfiguration = Configuration.unix().toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build();
-    private Configuration jimFsConfigurationWithoutPermissions = randomBoolean() ? Configuration.unix().toBuilder().setAttributeViews("basic").build() : Configuration.windows();
-
-    private enum Mode {
-        CHANGE, KEEP, DISABLED
-    }
-
-    public void testThatCommandLogsErrorMessageOnFail() throws Exception {
-        executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.CHANGE));
-        assertThat(captureOutputTerminal.getOutput(), containsString("Please ensure that the user account running Elasticsearch has read access to this file"));
-    }
-
-    public void testThatCommandLogsNothingWhenPermissionRemains() throws Exception {
-        executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.KEEP));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsNothingWhenDisabled() throws Exception {
-        executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsNothingIfFilesystemDoesNotSupportPermissions() throws Exception {
-        executeCommand(jimFsConfigurationWithoutPermissions, new PermissionCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsOwnerChange() throws Exception {
-        executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.CHANGE));
-        assertThat(captureOutputTerminal.getOutput(), allOf(containsString("Owner of file ["), containsString("] used to be ["), containsString("], but now is [")));
-    }
-
-    public void testThatCommandLogsNothingIfOwnerRemainsSame() throws Exception {
-        executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.KEEP));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsNothingIfOwnerIsDisabled() throws Exception {
-        executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsNothingIfFileSystemDoesNotSupportOwners() throws Exception {
-        executeCommand(jimFsConfigurationWithoutPermissions, new OwnerCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsIfGroupChanges() throws Exception {
-        executeCommand(jimFsConfiguration, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.CHANGE));
-        assertThat(captureOutputTerminal.getOutput(), allOf(containsString("Group of file ["), containsString("] used to be ["), containsString("], but now is [")));
-    }
-
-    public void testThatCommandLogsNothingIfGroupRemainsSame() throws Exception {
-        executeCommand(jimFsConfiguration, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.KEEP));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsNothingIfGroupIsDisabled() throws Exception {
-        executeCommand(jimFsConfiguration, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandLogsNothingIfFileSystemDoesNotSupportGroups() throws Exception {
-        executeCommand(jimFsConfigurationWithoutPermissions, new GroupCheckFileCommand(createTempDir(), captureOutputTerminal, Mode.DISABLED));
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandDoesNotLogAnythingOnFileCreation() throws Exception {
-        Configuration configuration = randomBoolean() ? jimFsConfiguration : jimFsConfigurationWithoutPermissions;
-
-        try (FileSystem fs = Jimfs.newFileSystem(configuration)) {
-            Path path = fs.getPath(randomAsciiOfLength(10));
-            Settings settings = Settings.builder()
-                    .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
-                    .build();
-            new CreateFileCommand(captureOutputTerminal, path).execute(settings, new Environment(settings));
-            assertThat(Files.exists(path), is(true));
-        }
-
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    public void testThatCommandWorksIfFileIsDeletedByCommand() throws Exception {
-        Configuration configuration = randomBoolean() ? jimFsConfiguration : jimFsConfigurationWithoutPermissions;
-
-        try (FileSystem fs = Jimfs.newFileSystem(configuration)) {
-            Path path = fs.getPath(randomAsciiOfLength(10));
-            Files.write(path, "anything".getBytes(StandardCharsets.UTF_8));
-
-            Settings settings = Settings.builder()
-                    .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
-                    .build();
-            new DeleteFileCommand(captureOutputTerminal, path).execute(settings, new Environment(settings));
-            assertThat(Files.exists(path), is(false));
-        }
-
-        assertTrue(captureOutputTerminal.getOutput().isEmpty());
-    }
-
-    private void executeCommand(Configuration configuration, AbstractTestCheckFileCommand command) throws Exception {
-        try (FileSystem fs = Jimfs.newFileSystem(configuration)) {
-            command.execute(fs);
-        }
-    }
-
-    abstract class AbstractTestCheckFileCommand extends CheckFileCommand {
-
-        protected final Mode mode;
-        protected FileSystem fs;
-        protected Path[] paths;
-        final Path baseDir;
-
-        public AbstractTestCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException {
-            super(terminal);
-            this.mode = mode;
-            this.baseDir = baseDir;
-        }
-
-        public CliTool.ExitStatus execute(FileSystem fs) throws Exception {
-            this.fs = fs;
-            this.paths = new Path[] { writePath(fs, "p1", "anything"), writePath(fs, "p2", "anything"), writePath(fs, "p3", "anything") };
-            Settings settings = Settings.settingsBuilder()
-                    .put(Environment.PATH_HOME_SETTING.getKey(), baseDir.toString())
-                    .build();
-            return super.execute(Settings.EMPTY, new Environment(settings));
-        }
-
-        private Path writePath(FileSystem fs, String name, String content) throws IOException {
-            Path path = fs.getPath(name);
-            Files.write(path, content.getBytes(StandardCharsets.UTF_8));
-            return path;
-        }
-
-        @Override
-        protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
-            return paths;
-        }
-    }
-
-    /**
-     * command that changes permissions from a file if enabled
-     */
-    class PermissionCheckFileCommand extends AbstractTestCheckFileCommand {
-
-        public PermissionCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException {
-            super(baseDir, terminal, mode);
-        }
-
-        @Override
-        public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
-            int randomInt = randomInt(paths.length - 1);
-            Path randomPath = paths[randomInt];
-            switch (mode) {
-                case CHANGE:
-                    Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8));
-                    Files.setPosixFilePermissions(randomPath, Sets.newHashSet(PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.OTHERS_EXECUTE, PosixFilePermission.GROUP_EXECUTE));
-                    break;
-                case KEEP:
-                    Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8));
-                    Set<PosixFilePermission> posixFilePermissions = Files.getPosixFilePermissions(randomPath);
-                    Files.setPosixFilePermissions(randomPath, posixFilePermissions);
-                    break;
-            }
-            return CliTool.ExitStatus.OK;
-        }
-
-    }
-
-    /**
-     * command that changes the owner of a file if enabled
-     */
-    class OwnerCheckFileCommand extends AbstractTestCheckFileCommand {
-
-        public OwnerCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException {
-            super(baseDir, terminal, mode);
-        }
-
-        @Override
-        public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
-            int randomInt = randomInt(paths.length - 1);
-            Path randomPath = paths[randomInt];
-            switch (mode) {
-                case CHANGE:
-                    Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8));
-                    UserPrincipal randomOwner = fs.getUserPrincipalLookupService().lookupPrincipalByName(randomAsciiOfLength(10));
-                    Files.setOwner(randomPath, randomOwner);
-                    break;
-                case KEEP:
-                    Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8));
-                    UserPrincipal originalOwner = Files.getOwner(randomPath);
-                    Files.setOwner(randomPath, originalOwner);
-                    break;
-            }
-
-            return CliTool.ExitStatus.OK;
-        }
-    }
-
-    /**
-     * command that changes the group of a file if enabled
-     */
-    class GroupCheckFileCommand extends AbstractTestCheckFileCommand {
-
-        public GroupCheckFileCommand(Path baseDir, Terminal terminal, Mode mode) throws IOException {
-            super(baseDir, terminal, mode);
-        }
-
-        @Override
-        public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
-            int randomInt = randomInt(paths.length - 1);
-            Path randomPath = paths[randomInt];
-            switch (mode) {
-                case CHANGE:
-                    Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8));
-                    GroupPrincipal randomPrincipal = fs.getUserPrincipalLookupService().lookupPrincipalByGroupName(randomAsciiOfLength(10));
-                    Files.getFileAttributeView(randomPath, PosixFileAttributeView.class).setGroup(randomPrincipal);
-                    break;
-                case KEEP:
-                    Files.write(randomPath, randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8));
-                    GroupPrincipal groupPrincipal = Files.readAttributes(randomPath, PosixFileAttributes.class).group();
-                    Files.getFileAttributeView(randomPath, PosixFileAttributeView.class).setGroup(groupPrincipal);
-                    break;
-            }
-
-            return CliTool.ExitStatus.OK;
-        }
-    }
-
-    /**
-     * A command that creates a non existing file
-     */
-    class CreateFileCommand extends CheckFileCommand {
-
-        private final Path pathToCreate;
-
-        public CreateFileCommand(Terminal terminal, Path pathToCreate) {
-            super(terminal);
-            this.pathToCreate = pathToCreate;
-        }
-
-        @Override
-        public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
-            Files.write(pathToCreate, "anything".getBytes(StandardCharsets.UTF_8));
-            return CliTool.ExitStatus.OK;
-        }
-
-        @Override
-        protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception {
-            return new Path[] { pathToCreate };
-        }
-    }
-
-    /**
-     * A command that deletes an existing file
-     */
-    class DeleteFileCommand extends CheckFileCommand {
-
-        private final Path pathToDelete;
-
-        public DeleteFileCommand(Terminal terminal, Path pathToDelete) {
-            super(terminal);
-            this.pathToDelete = pathToDelete;
-        }
-
-        @Override
-        public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
-            Files.delete(pathToDelete);
-            return CliTool.ExitStatus.OK;
-        }
-
-        @Override
-        protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception {
-            return new Path[] {pathToDelete};
-        }
-    }
-}

+ 0 - 349
qa/evil-tests/src/test/java/org/elasticsearch/common/cli/CliToolTests.java

@@ -1,349 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.common.cli;
-
-import org.apache.commons.cli.CommandLine;
-import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.SuppressForbidden;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.node.internal.InternalSettingsPreparer;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicReference;
-
-import static java.util.Collections.unmodifiableMap;
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK;
-import static org.elasticsearch.common.cli.CliTool.ExitStatus.USAGE;
-import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
-import static org.hamcrest.Matchers.arrayContaining;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasItem;
-import static org.hamcrest.Matchers.hasSize;
-import static org.hamcrest.Matchers.is;
-
-@SuppressForbidden(reason = "modifies system properties intentionally")
-public class CliToolTests extends CliToolTestCase {
-    public void testOK() throws Exception {
-        Terminal terminal = new MockTerminal();
-        final AtomicReference<Boolean> executed = new AtomicReference<>(false);
-        final NamedCommand cmd = new NamedCommand("cmd", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) {
-                executed.set(true);
-                return OK;
-            }
-        };
-        SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
-        CliTool.ExitStatus status = tool.execute();
-        assertStatus(status, OK);
-        assertCommandHasBeenExecuted(executed);
-    }
-
-    public void testUsageError() throws Exception {
-        Terminal terminal = new MockTerminal();
-        final AtomicReference<Boolean> executed = new AtomicReference<>(false);
-        final NamedCommand cmd = new NamedCommand("cmd", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws UserError {
-                executed.set(true);
-                throw new UserError(CliTool.ExitStatus.USAGE, "bad usage");
-            }
-        };
-        SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
-        CliTool.ExitStatus status = tool.execute();
-        assertStatus(status, CliTool.ExitStatus.USAGE);
-        assertCommandHasBeenExecuted(executed);
-    }
-
-    public void testMultiCommand() throws Exception {
-        Terminal terminal = new MockTerminal();
-        int count = randomIntBetween(2, 7);
-        List<AtomicReference<Boolean>> executed = new ArrayList<>(count);
-        for (int i = 0; i < count; i++) {
-            executed.add(new AtomicReference<>(false));
-        }
-        NamedCommand[] cmds = new NamedCommand[count];
-        for (int i = 0; i < count; i++) {
-            final int index = i;
-            cmds[i] = new NamedCommand("cmd" + index, terminal) {
-                @Override
-                public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                    executed.get(index).set(true);
-                    return OK;
-                }
-            };
-        }
-        MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds);
-        int cmdIndex = randomIntBetween(0, count-1);
-        CliTool.ExitStatus status = tool.execute("cmd" + cmdIndex);
-        assertThat(status, is(OK));
-        for (int i = 0; i < count; i++) {
-            assertThat(executed.get(i).get(), is(i == cmdIndex));
-        }
-    }
-
-    public void testMultiCommandUnknownCommand() throws Exception {
-        Terminal terminal = new MockTerminal();
-        int count = randomIntBetween(2, 7);
-        List<AtomicReference<Boolean>> executed = new ArrayList<>(count);
-        for (int i = 0; i < count; i++) {
-            executed.add(new AtomicReference<>(false));
-        }
-        NamedCommand[] cmds = new NamedCommand[count];
-        for (int i = 0; i < count; i++) {
-            final int index = i;
-            cmds[i] = new NamedCommand("cmd" + index, terminal) {
-                @Override
-                public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                    executed.get(index).set(true);
-                    return OK;
-                }
-            };
-        }
-        MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds);
-        CliTool.ExitStatus status = tool.execute("cmd" + count); // "cmd" + count doesn't exist
-        assertThat(status, is(CliTool.ExitStatus.USAGE));
-        for (int i = 0; i < count; i++) {
-            assertThat(executed.get(i).get(), is(false));
-        }
-    }
-
-    public void testSingleCommandToolHelp() throws Exception {
-        MockTerminal terminal = new MockTerminal();
-        final AtomicReference<Boolean> executed = new AtomicReference<>(false);
-        final NamedCommand cmd = new NamedCommand("cmd1", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                executed.set(true);
-                throw new IOException("io error");
-            }
-        };
-        SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
-        CliTool.ExitStatus status = tool.execute(args("-h"));
-        assertStatus(status, CliTool.ExitStatus.OK_AND_EXIT);
-        assertThat(terminal.getOutput(), containsString("cmd1 help"));
-    }
-
-    public void testMultiCommandToolHelp() throws Exception {
-        MockTerminal terminal = new MockTerminal();
-        NamedCommand[] cmds = new NamedCommand[2];
-        cmds[0] = new NamedCommand("cmd0", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                return OK;
-            }
-        };
-        cmds[1] = new NamedCommand("cmd1", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                return OK;
-            }
-        };
-        MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds);
-        CliTool.ExitStatus status = tool.execute(args("-h"));
-        assertStatus(status, CliTool.ExitStatus.OK_AND_EXIT);
-        assertThat(terminal.getOutput(), containsString("tool help"));
-    }
-
-    public void testMultiCommandCmdHelp() throws Exception {
-        MockTerminal terminal = new MockTerminal();
-        NamedCommand[] cmds = new NamedCommand[2];
-        cmds[0] = new NamedCommand("cmd0", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                return OK;
-            }
-        };
-        cmds[1] = new NamedCommand("cmd1", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                return OK;
-            }
-        };
-        MultiCmdTool tool = new MultiCmdTool("tool", terminal, cmds);
-        CliTool.ExitStatus status = tool.execute(args("cmd1 -h"));
-        assertStatus(status, CliTool.ExitStatus.OK_AND_EXIT);
-        assertThat(terminal.getOutput(), containsString("cmd1 help"));
-    }
-
-    public void testNonUserErrorPropagates() throws Exception {
-        MockTerminal terminal = new MockTerminal();
-        NamedCommand cmd = new NamedCommand("cmd", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
-                throw new IOException("error message");
-            }
-        };
-        SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
-        IOException e = expectThrows(IOException.class, () -> {
-           tool.execute();
-        });
-        assertEquals("error message", e.getMessage());
-    }
-
-    public void testMultipleLaunch() throws Exception {
-        Terminal terminal = new MockTerminal();
-        final AtomicReference<Boolean> executed = new AtomicReference<>(false);
-        final NamedCommand cmd = new NamedCommand("cmd", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) {
-                executed.set(true);
-                return OK;
-            }
-        };
-        SingleCmdTool tool = new SingleCmdTool("tool", terminal, cmd);
-        tool.parse("cmd", Strings.splitStringByCommaToArray("--verbose"));
-        tool.parse("cmd", Strings.splitStringByCommaToArray("--silent"));
-        tool.parse("cmd", Strings.splitStringByCommaToArray("--help"));
-    }
-
-    public void testPromptForSetting() throws Exception {
-        final AtomicReference<String> promptedSecretValue = new AtomicReference<>(null);
-        final AtomicReference<String> promptedTextValue = new AtomicReference<>(null);
-        final MockTerminal terminal = new MockTerminal();
-        terminal.addTextInput("replaced");
-        terminal.addSecretInput("changeit");
-        final NamedCommand cmd = new NamedCommand("noop", terminal) {
-            @Override
-            public CliTool.ExitStatus execute(Settings settings, Environment env) {
-                promptedSecretValue.set(settings.get("foo.password"));
-                promptedTextValue.set(settings.get("replace"));
-                return OK;
-            }
-        };
-
-        System.setProperty("es.foo.password", InternalSettingsPreparer.SECRET_PROMPT_VALUE);
-        System.setProperty("es.replace", InternalSettingsPreparer.TEXT_PROMPT_VALUE);
-        try {
-            new SingleCmdTool("tool", terminal, cmd).execute();
-        } finally {
-            System.clearProperty("es.foo.password");
-            System.clearProperty("es.replace");
-        }
-
-        assertThat(promptedSecretValue.get(), is("changeit"));
-        assertThat(promptedTextValue.get(), is("replaced"));
-    }
-
-    public void testStopAtNonOptionParsing() throws Exception {
-        final CliToolConfig.Cmd lenientCommand = cmd("lenient", CliTool.Command.Exit.class).stopAtNonOption(true).build();
-        final CliToolConfig.Cmd strictCommand = cmd("strict", CliTool.Command.Exit.class).stopAtNonOption(false).build();
-        final CliToolConfig config = CliToolConfig.config("elasticsearch", CliTool.class).cmds(lenientCommand, strictCommand).build();
-
-        MockTerminal terminal = new MockTerminal();
-        final CliTool cliTool = new CliTool(config, terminal) {
-            @Override
-            protected Command parse(String cmdName, CommandLine cli) throws Exception {
-                return new NamedCommand(cmdName, terminal) {
-                    @Override
-                    public ExitStatus execute(Settings settings, Environment env) throws Exception {
-                        return OK;
-                    }
-                };
-            }
-        };
-
-        // known parameters, no error
-        assertStatus(cliTool.execute(args("lenient --verbose")), OK);
-        assertStatus(cliTool.execute(args("lenient -v")), OK);
-
-        // unknown parameters, no error
-        assertStatus(cliTool.execute(args("lenient --unknown")), OK);
-        assertStatus(cliTool.execute(args("lenient -u")), OK);
-
-        // unknown parameters, error
-        assertStatus(cliTool.execute(args("strict --unknown")), USAGE);
-        assertThat(terminal.getOutput(), containsString("Unrecognized option: --unknown"));
-
-        terminal.resetOutput();
-        assertStatus(cliTool.execute(args("strict -u")), USAGE);
-        assertThat(terminal.getOutput(), containsString("Unrecognized option: -u"));
-    }
-
-    private void assertStatus(CliTool.ExitStatus status, CliTool.ExitStatus expectedStatus) {
-        assertThat(status, is(expectedStatus));
-    }
-
-    private void assertCommandHasBeenExecuted(AtomicReference<Boolean> executed) {
-        assertThat("Expected command atomic reference counter to be set to true", executed.get(), is(Boolean.TRUE));
-    }
-
-    private static class SingleCmdTool extends CliTool {
-
-        private final Command command;
-
-        private SingleCmdTool(String name, Terminal terminal, NamedCommand command) {
-            super(CliToolConfig.config(name, SingleCmdTool.class)
-                    .cmds(cmd(command.name, command.getClass()))
-                    .build(), terminal);
-            this.command = command;
-        }
-
-        @Override
-        protected Command parse(String cmdName, CommandLine cli) throws Exception {
-            return command;
-        }
-    }
-
-    private static class MultiCmdTool extends CliTool {
-
-        private final Map<String, Command> commands;
-
-        private MultiCmdTool(String name, Terminal terminal, NamedCommand... commands) {
-            super(CliToolConfig.config(name, MultiCmdTool.class)
-                    .cmds(cmds(commands))
-                    .build(), terminal);
-            Map<String, Command> commandByName = new HashMap<>();
-            for (int i = 0; i < commands.length; i++) {
-                commandByName.put(commands[i].name, commands[i]);
-            }
-            this.commands = unmodifiableMap(commandByName);
-        }
-
-        @Override
-        protected Command parse(String cmdName, CommandLine cli) throws Exception {
-            return commands.get(cmdName);
-        }
-
-        private static CliToolConfig.Cmd[] cmds(NamedCommand... commands) {
-            CliToolConfig.Cmd[] cmds = new CliToolConfig.Cmd[commands.length];
-            for (int i = 0; i < commands.length; i++) {
-                cmds[i] = cmd(commands[i].name, commands[i].getClass()).build();
-            }
-            return cmds;
-        }
-    }
-
-    private static abstract class NamedCommand extends CliTool.Command {
-
-        private final String name;
-
-        private NamedCommand(String name, Terminal terminal) {
-            super(terminal);
-            this.name = name;
-        }
-    }
-}

+ 3 - 7
qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -44,11 +44,8 @@ import java.util.zip.ZipOutputStream;
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.Version;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.cli.UserError;
+import org.elasticsearch.cli.MockTerminal;
+import org.elasticsearch.cli.UserError;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
@@ -119,8 +116,7 @@ public class InstallPluginCommandTests extends ESTestCase {
 
     static MockTerminal installPlugin(String pluginUrl, Environment env) throws Exception {
         MockTerminal terminal = new MockTerminal();
-        CliTool.ExitStatus status = new InstallPluginCommand(terminal, pluginUrl, true).execute(env.settings(), env);
-        assertEquals(CliTool.ExitStatus.OK, status);
+        new InstallPluginCommand(env).execute(terminal, pluginUrl, true);
         return terminal;
     }
 

+ 5 - 8
qa/evil-tests/src/test/java/org/elasticsearch/plugins/ListPluginsCommandTests.java

@@ -22,14 +22,10 @@ package org.elasticsearch.plugins;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Collections;
-import java.util.List;
 
 import org.apache.lucene.util.LuceneTestCase;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
@@ -48,8 +44,9 @@ public class ListPluginsCommandTests extends ESTestCase {
 
     static MockTerminal listPlugins(Environment env) throws Exception {
         MockTerminal terminal = new MockTerminal();
-        CliTool.ExitStatus status = new ListPluginsCommand(terminal).execute(env.settings(), env);
-        assertEquals(CliTool.ExitStatus.OK, status);
+        String[] args = {};
+        int status = new ListPluginsCommand(env).main(args, terminal);
+        assertEquals(ExitCodes.OK, status);
         return terminal;
     }
 

+ 1 - 1
qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java

@@ -19,7 +19,7 @@
 
 package org.elasticsearch.plugins;
 
-import org.elasticsearch.common.cli.Terminal;
+import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.test.ESTestCase;
 
 import java.nio.file.Path;

+ 3 - 7
qa/evil-tests/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java

@@ -25,11 +25,8 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 
 import org.apache.lucene.util.LuceneTestCase;
-import org.elasticsearch.common.cli.CliTool;
-import org.elasticsearch.common.cli.CliToolTestCase;
-import org.elasticsearch.common.cli.MockTerminal;
-import org.elasticsearch.common.cli.Terminal;
-import org.elasticsearch.common.cli.UserError;
+import org.elasticsearch.cli.UserError;
+import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
@@ -51,8 +48,7 @@ public class RemovePluginCommandTests extends ESTestCase {
 
     static MockTerminal removePlugin(String name, Environment env) throws Exception {
         MockTerminal terminal = new MockTerminal();
-        CliTool.ExitStatus status = new RemovePluginCommand(terminal, name).execute(env.settings(), env);
-        assertEquals(CliTool.ExitStatus.OK, status);
+        new RemovePluginCommand(env).execute(terminal, name);
         return terminal;
     }
 

+ 56 - 0
test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java

@@ -0,0 +1,56 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cli;
+
+import org.elasticsearch.test.ESTestCase;
+import org.junit.Before;
+
+/**
+ * A base test case for cli tools.
+ */
+public abstract class CommandTestCase extends ESTestCase {
+
+    /** The terminal that execute uses. */
+    protected final MockTerminal terminal = new MockTerminal();
+
+    /** The last command that was executed. */
+    protected Command command;
+
+    @Before
+    public void resetTerminal() {
+        terminal.reset();
+        terminal.setVerbosity(Terminal.Verbosity.NORMAL);
+    }
+
+    /** Creates a Command to test execution. */
+    protected abstract Command newCommand();
+
+    /**
+     * Runs the command with the given args.
+     *
+     * Output can be found in {@link #terminal}.
+     * The command created can be found in {@link #command}.
+     */
+    public String execute(String... args) throws Exception {
+        command = newCommand();
+        command.mainWithoutErrorHandling(args, terminal);
+        return terminal.getOutput();
+    }
+}

+ 7 - 5
test/framework/src/main/java/org/elasticsearch/common/cli/MockTerminal.java → test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-package org.elasticsearch.common.cli;
+package org.elasticsearch.cli;
 
 import java.io.ByteArrayOutputStream;
 import java.io.OutputStreamWriter;
@@ -45,7 +45,7 @@ public class MockTerminal extends Terminal {
     @Override
     public String readText(String prompt) {
         if (textInput.isEmpty()) {
-            return null;
+            throw new IllegalStateException("No text input configured for prompt [" + prompt + "]");
         }
         return textInput.removeFirst();
     }
@@ -53,7 +53,7 @@ public class MockTerminal extends Terminal {
     @Override
     public char[] readSecret(String prompt) {
         if (secretInput.isEmpty()) {
-            return null;
+            throw new IllegalStateException("No secret input configured for prompt [" + prompt + "]");
         }
         return secretInput.removeFirst().toCharArray();
     }
@@ -78,8 +78,10 @@ public class MockTerminal extends Terminal {
         return buffer.toString("UTF-8");
     }
 
-    /** Wipes the output. */
-    public void resetOutput() {
+    /** Wipes the input and output. */
+    public void reset() {
         buffer.reset();
+        textInput.clear();
+        secretInput.clear();
     }
 }

+ 1 - 0
test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java

@@ -21,6 +21,7 @@ package org.elasticsearch.common.cli;
 
 import java.io.IOException;
 
+import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.test.ESTestCase;