SOLR-17651: Make sure CLI unit tests don't call System.exit() (#3258)

This introduces class ToolRuntime which is initiated and passed as a context for all CLI tools. When running tests, a sub-classes is used instead and blocks calls to System.exit().

Author: psalagnac

gradle/validation/forbidden-apis/defaults.all.txt

@@ -57,6 +57,11 @@ java.util.Collections#shuffle(java.util.List) @ Use shuffle(List, Random) instea
 java.util.Locale#forLanguageTag(java.lang.String) @ use new Locale.Builder().setLanguageTag(...).build() which has error handling
 java.util.Locale#toString() @ use Locale#toLanguageTag() for a standardized BCP47 locale name
 
+
+@defaultMessage Direct calls to force the JVM to quit are forbidden in server code.
+java.lang.System#exit(int)
+java.lang.Runtime#exit(int)
+
 @defaultMessage Constructors for wrapper classes of Java primitives should be avoided in favor of the public static methods available or autoboxing
 java.lang.Integer#<init>(**)
 java.lang.Byte#<init>(**)

solr/CHANGES.txt

@@ -265,6 +265,8 @@ Other Changes
 
 * SOLR-17716: Handle interrupted exception in SolrCores.waitAddPendingCoreOps. (Bruno Roustant)
 
+* SOLR-17651: Add System.exit() in forbidden APIs, and make sure CLI unit tests never call it. (Pierre Salagnac)
+
 ==================  9.8.1 ==================
 Bug Fixes
 ---------------------

solr/core/src/java/org/apache/solr/cli/ApiTool.java

@@ -17,7 +17,6 @@
 
 package org.apache.solr.cli;
 
-import java.io.PrintStream;
 import java.net.URI;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
@@ -46,12 +45,8 @@ public class ApiTool extends ToolBase {
           .desc("Send a GET request to a Solr API endpoint.")
           .build();
 
-  public ApiTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public ApiTool(PrintStream stdout) {
-    super(stdout);
+  public ApiTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override

solr/core/src/java/org/apache/solr/cli/AssertTool.java

@@ -17,7 +17,6 @@
 package org.apache.solr.cli;
 
 import java.io.IOException;
-import java.io.PrintStream;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -144,12 +143,8 @@ public class AssertTool extends ToolBase {
           .longOpt("exitcode")
           .build();
 
-  public AssertTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public AssertTool(PrintStream stdout) {
-    super(stdout);
+  public AssertTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override
@@ -262,8 +257,8 @@ protected int runAssert(CommandLine cli) throws Exception {
     return ret;
   }
 
-  public static int assertSolrRunning(String url, String credentials) throws Exception {
-    StatusTool status = new StatusTool();
+  public int assertSolrRunning(String url, String credentials) throws Exception {
+    StatusTool status = new StatusTool(runtime);
     try {
       status.waitToSeeSolrUp(url, credentials, timeoutMs, TimeUnit.MILLISECONDS);
     } catch (Exception se) {
@@ -280,8 +275,8 @@ public static int assertSolrRunning(String url, String credentials) throws Excep
     return 0;
   }
 
-  public static int assertSolrNotRunning(String url, String credentials) throws Exception {
-    StatusTool status = new StatusTool();
+  public int assertSolrNotRunning(String url, String credentials) throws Exception {
+    StatusTool status = new StatusTool(runtime);
     long timeout =
         System.nanoTime() + TimeUnit.NANOSECONDS.convert(timeoutMs, TimeUnit.MILLISECONDS);
     try (SolrClient solrClient = CLIUtils.getSolrClient(url, credentials)) {
@@ -316,7 +311,7 @@ public static int assertSolrNotRunning(String url, String credentials) throws Ex
             + " seconds");
   }
 
-  public static int assertSolrRunningInCloudMode(String url, String credentials) throws Exception {
+  public int assertSolrRunningInCloudMode(String url, String credentials) throws Exception {
     if (!isSolrRunningOn(url, credentials)) {
       return exitOrException(
           "Solr is not running on url "
@@ -332,8 +327,7 @@ public static int assertSolrRunningInCloudMode(String url, String credentials) t
     return 0;
   }
 
-  public static int assertSolrNotRunningInCloudMode(String url, String credentials)
-      throws Exception {
+  public int assertSolrNotRunningInCloudMode(String url, String credentials) throws Exception {
     if (!isSolrRunningOn(url, credentials)) {
       return exitOrException(
           "Solr is not running on url "
@@ -411,8 +405,8 @@ private static int exitOrException(String msg) throws AssertionFailureException
     }
   }
 
-  private static boolean isSolrRunningOn(String url, String credentials) throws Exception {
-    StatusTool status = new StatusTool();
+  private boolean isSolrRunningOn(String url, String credentials) throws Exception {
+    StatusTool status = new StatusTool(runtime);
     try {
       status.waitToSeeSolrUp(url, credentials, timeoutMs, TimeUnit.MILLISECONDS);
       return true;

solr/core/src/java/org/apache/solr/cli/AuthTool.java

@@ -23,7 +23,6 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import java.io.Console;
 import java.io.IOException;
-import java.io.PrintStream;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -101,12 +100,8 @@ public class AuthTool extends ToolBase {
               "This is where any authentication related configuration files, if any, would be placed.  Defaults to $SOLR_HOME.")
           .build();
 
-  public AuthTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public AuthTool(PrintStream stdout) {
-    super(stdout);
+  public AuthTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override
@@ -155,7 +150,7 @@ private void ensureArgumentIsValidBooleanIfPresent(CommandLine cli, Option optio
                 + "] must be either true or false, but was ["
                 + value
                 + "]");
-        SolrCLI.exit(1);
+        runtime.exit(1);
       }
     }
   }
@@ -169,12 +164,12 @@ private void handleBasicAuth(CommandLine cli) throws Exception {
       case "enable":
         if (!prompt && !cli.hasOption(CommonCLIOptions.CREDENTIALS_OPTION)) {
           CLIO.out("Option --credentials or --prompt is required with enable.");
-          SolrCLI.exit(1);
+          runtime.exit(1);
         } else if (!prompt
             && (cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION) == null
                 || !cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION).contains(":"))) {
           CLIO.out("Option --credentials is not in correct format.");
-          SolrCLI.exit(1);
+          runtime.exit(1);
         }
 
         String zkHost = null;
@@ -190,7 +185,7 @@ private void handleBasicAuth(CommandLine cli) throws Exception {
               CLIO.out(
                   "Couldn't get ZooKeeper host. Please make sure Solr is running in cloud mode, or a zk-host has been passed in.");
             }
-            SolrCLI.exit(1);
+            runtime.exit(1);
           }
           if (zkHost == null) {
             if (cli.hasOption(CommonCLIOptions.ZK_HOST_OPTION)) {
@@ -200,7 +195,7 @@ private void handleBasicAuth(CommandLine cli) throws Exception {
               CLIO.out(
                   "Couldn't get ZooKeeper host. Please make sure Solr is running in cloud mode, or a zk-host has been passed in.");
             }
-            SolrCLI.exit(1);
+            runtime.exit(1);
           }
 
           // check if security is already enabled or not
@@ -264,15 +259,15 @@ private void handleBasicAuth(CommandLine cli) throws Exception {
           CLIO.out(
               "Solr include file " + solrIncludeFilename + " doesn't exist or is not writeable.");
           printAuthEnablingInstructions(username, password);
-          System.exit(0);
+          runtime.exit(0);
         }
         String authConfDir = cli.getOptionValue(AUTH_CONF_DIR_OPTION);
         Path basicAuthConfFile = Path.of(authConfDir, "basicAuth.conf");
 
         if (!Files.isWritable(basicAuthConfFile.getParent())) {
           CLIO.out("Cannot write to file: " + basicAuthConfFile.toAbsolutePath());
           printAuthEnablingInstructions(username, password);
-          System.exit(0);
+          runtime.exit(0);
         }
 
         Files.writeString(
@@ -297,19 +292,19 @@ private void handleBasicAuth(CommandLine cli) throws Exception {
               "Solr include file " + solrIncludeFilename + " doesn't exist or is not writeable.");
           CLIO.out(
               "Security has been disabled. Please remove any SOLR_AUTH_TYPE or SOLR_AUTHENTICATION_OPTS configuration from solr.in.sh/solr.in.cmd.\n");
-          System.exit(0);
+          runtime.exit(0);
         }
 
         // update the solr.in.sh file to comment out the necessary authentication lines
         updateIncludeFileDisableAuth(includeFile);
         return;
       default:
         CLIO.out("Valid auth commands are: enable, disable.");
-        SolrCLI.exit(1);
+        runtime.exit(1);
     }
 
     CLIO.out("Options not understood.");
-    SolrCLI.exit(1);
+    runtime.exit(1);
   }
 
   private void checkSecurityJsonExists(SolrZkClient zkClient)
@@ -320,7 +315,7 @@ private void checkSecurityJsonExists(SolrZkClient zkClient)
         CLIO.out(
             "Security is already enabled. You can disable it with 'bin/solr auth disable'. Existing security.json: \n"
                 + new String(oldSecurityBytes, StandardCharsets.UTF_8));
-        SolrCLI.exit(1);
+        runtime.exit(1);
       }
     }
   }
@@ -330,8 +325,8 @@ private void clearSecurityJson(CommandLine cli, boolean updateIncludeFileOnly) t
     if (!updateIncludeFileOnly) {
       zkHost = CLIUtils.getZkHost(cli);
       if (zkHost == null) {
-        stdout.print("ZK Host not found. Solr should be running in cloud mode.");
-        SolrCLI.exit(1);
+        runtime.print("ZK Host not found. Solr should be running in cloud mode.");
+        runtime.exit(1);
       }
 
       echoIfVerbose("Uploading following security.json: {}");

solr/core/src/java/org/apache/solr/cli/CLIUtils.java

@@ -234,8 +234,7 @@ public static String getZkHost(CommandLine cli) throws Exception {
               new GenericSolrRequest(SolrRequest.METHOD.GET, CommonParams.SYSTEM_INFO_PATH));
 
       // convert raw JSON into user-friendly output
-      StatusTool statusTool = new StatusTool();
-      Map<String, Object> status = statusTool.reportStatus(systemInfo, solrClient);
+      Map<String, Object> status = StatusTool.reportStatus(systemInfo, solrClient);
       @SuppressWarnings("unchecked")
       Map<String, Object> cloud = (Map<String, Object>) status.get("cloud");
       if (cloud != null) {

solr/core/src/java/org/apache/solr/cli/ClusterTool.java

@@ -18,7 +18,6 @@
 package org.apache.solr.cli;
 
 import java.io.IOException;
-import java.io.PrintStream;
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
@@ -53,12 +52,8 @@ public class ClusterTool extends ToolBase {
           .desc("Set the property to this value.")
           .build();
 
-  public ClusterTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public ClusterTool(PrintStream stdout) {
-    super(stdout);
+  public ClusterTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override

solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java

@@ -16,7 +16,6 @@
  */
 package org.apache.solr.cli;
 
-import java.io.PrintStream;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -50,12 +49,8 @@ public class ConfigSetDownloadTool extends ToolBase {
           .desc("Local directory with configs.")
           .build();
 
-  public ConfigSetDownloadTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public ConfigSetDownloadTool(PrintStream stdout) {
-    super(stdout);
+  public ConfigSetDownloadTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override

solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java

@@ -16,7 +16,6 @@
  */
 package org.apache.solr.cli;
 
-import java.io.PrintStream;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -52,12 +51,8 @@ public class ConfigSetUploadTool extends ToolBase {
           .desc("Local directory with configs.")
           .build();
 
-  public ConfigSetUploadTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public ConfigSetUploadTool(PrintStream stdout) {
-    super(stdout);
+  public ConfigSetUploadTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override

solr/core/src/java/org/apache/solr/cli/ConfigTool.java

@@ -17,7 +17,6 @@
 
 package org.apache.solr.cli;
 
-import java.io.PrintStream;
 import java.util.HashMap;
 import java.util.Map;
 import org.apache.commons.cli.CommandLine;
@@ -72,12 +71,8 @@ public class ConfigTool extends ToolBase {
           .desc("Set the property to this value; accepts JSON objects and strings.")
           .build();
 
-  public ConfigTool() {
-    this(CLIO.getOutStream());
-  }
-
-  public ConfigTool(PrintStream stdout) {
-    super(stdout);
+  public ConfigTool(ToolRuntime runtime) {
+    super(runtime);
   }
 
   @Override