fix: adds a better error message if an spi argument lacks a value (#24826)

closes: #22260

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakMain.java

@@ -41,7 +41,7 @@ import org.keycloak.Config;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler;
-import org.keycloak.quarkus.runtime.cli.NonCliPropertyException;
+import org.keycloak.quarkus.runtime.cli.PropertyException;
 import org.keycloak.quarkus.runtime.cli.Picocli;
 import org.keycloak.common.Version;
 import org.keycloak.quarkus.runtime.cli.command.Start;
@@ -64,7 +64,16 @@ public class KeycloakMain implements QuarkusApplication {
 
     public static void main(String[] args) {
         System.setProperty("kc.version", Version.VERSION);
-        List<String> cliArgs = Picocli.parseArgs(args);
+        List<String> cliArgs = null;
+        try {
+            cliArgs = Picocli.parseArgs(args);
+        } catch (PropertyException e) {
+            ExecutionExceptionHandler errorHandler = new ExecutionExceptionHandler();
+            PrintWriter errStream = new PrintWriter(System.err, true);
+            errorHandler.error(errStream, e.getMessage(), null);
+            System.exit(ExitCode.USAGE);
+            return;
+        }
 
         if (cliArgs.isEmpty()) {
             cliArgs = new ArrayList<>(cliArgs);
@@ -83,7 +92,7 @@ public class KeycloakMain implements QuarkusApplication {
 
             try {
                 Picocli.validateNonCliConfig(cliArgs, new Start(), new PrintWriter(System.out, true));
-            } catch (NonCliPropertyException e) {
+            } catch (PropertyException e) {
                 errorHandler.error(errStream, e.getMessage(), null);
                 System.exit(ExitCode.USAGE);
                 return;

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/ExecutionExceptionHandler.java

@@ -42,7 +42,7 @@ public final class ExecutionExceptionHandler implements CommandLine.IExecutionEx
 
     @Override
     public int handleExecutionException(Exception cause, CommandLine cmd, ParseResult parseResult) {
-        if (cause instanceof NonCliPropertyException) {
+        if (cause instanceof PropertyException) {
             PrintWriter writer = cmd.getErr();
             writer.println(cmd.getColorScheme().errorText(cause.getMessage()));
             return ShortErrorMessageHandler.getInvalidInputExitCode(cause, cmd);

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java

@@ -297,7 +297,7 @@ public final class Picocli {
                 }
 
                 if (!PropertyMapperParameterConsumer.isExpectedValue(mapper.getExpectedValues(), value)) {
-                    throw new NonCliPropertyException(PropertyMapperParameterConsumer.getErrorMessage(mapper.getFrom(),
+                    throw new PropertyException(PropertyMapperParameterConsumer.getErrorMessage(mapper.getFrom(),
                             value, mapper.getExpectedValues(), mapper.getExpectedValues()) + ". From ConfigSource " + configSource.getName());
                 }
             }
@@ -595,7 +595,7 @@ public final class Picocli {
         cmd.getOut().println(message);
     }
 
-    public static List<String> parseArgs(String[] rawArgs) {
+    public static List<String> parseArgs(String[] rawArgs) throws PropertyException {
         if (rawArgs.length == 0) {
             return List.of();
         }
@@ -615,6 +615,12 @@ public final class Picocli {
                 iterator.remove();
 
                 if (!arg.contains(ARG_KEY_VALUE_SEPARATOR)) {
+                    if (!iterator.hasNext()) {
+                        if (arg.startsWith("--spi")) {
+                            throw new PropertyException(String.format("spi argument %s requires a value.", arg));
+                        }
+                        return args;
+                    }
                     String next = iterator.next();
 
                     if (!next.startsWith("--")) {

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/PropertyException.java

@@ -17,9 +17,9 @@
 
 package org.keycloak.quarkus.runtime.cli;
 
-public class NonCliPropertyException extends RuntimeException {
+public class PropertyException extends RuntimeException {
 
-    public NonCliPropertyException(String message) {
+    public PropertyException(String message) {
         super(message);
     }
 

quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/StartCommandDistTest.java

@@ -44,6 +44,13 @@ public class StartCommandDistTest {
                 () -> "The Output:\n" + result.getOutput() + "doesn't contains the expected string.");
     }
 
+    @Test
+    @Launch({ "start", "--spi-events-listener-jboss-logging-success-level" })
+    void failSpiArgMissingValue(LaunchResult result) {
+        assertTrue(result.getErrorOutput().contains("spi argument --spi-events-listener-jboss-logging-success-level requires a value"),
+                () -> "The Output:\n" + result.getErrorOutput() + "doesn't contains the expected string.");
+    }
+
     @Test
     @Launch({ "--profile=dev", "start" })
     void failUsingDevProfile(LaunchResult result) {