From c98048bf642fe576ce53e85ed252722943fdc7de Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 23 Oct 2020 18:33:52 +0200 Subject: [PATCH] KEYCLOAK-16063 Messed output when running './kc.sh' --- .../main/java/org/keycloak/cli/Picocli.java | 73 ++++++++++++++++--- .../org/keycloak/configuration/Messages.java | 9 ++- .../configuration/PropertyMappers.java | 16 +++- .../QuarkusConfigurationException.java | 28 +++++++ .../quarkus/QuarkusLifecycleObserver.java | 14 +++- .../provider/quarkus/QuarkusPlatform.java | 31 ++++++++ 6 files changed, 153 insertions(+), 18 deletions(-) create mode 100644 quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusConfigurationException.java diff --git a/quarkus/runtime/src/main/java/org/keycloak/cli/Picocli.java b/quarkus/runtime/src/main/java/org/keycloak/cli/Picocli.java index 6e394f976e..593f27896a 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/cli/Picocli.java +++ b/quarkus/runtime/src/main/java/org/keycloak/cli/Picocli.java @@ -23,15 +23,20 @@ import java.util.Iterator; import java.util.List; import java.util.function.IntFunction; -import io.quarkus.runtime.Quarkus; +import org.jboss.logging.Logger; import org.keycloak.common.Profile; import org.keycloak.configuration.PropertyMapper; import org.keycloak.configuration.PropertyMappers; +import org.keycloak.platform.Platform; +import org.keycloak.provider.quarkus.QuarkusConfigurationException; +import org.keycloak.provider.quarkus.QuarkusPlatform; import org.keycloak.util.Environment; import picocli.CommandLine; final class Picocli { + private static final Logger logger = Logger.getLogger(Picocli.class); + static CommandLine createCommandLine() { CommandLine.Model.CommandSpec spec = CommandLine.Model.CommandSpec.forAnnotatedObject(new MainCommand()) .name(Environment.getCommand()); @@ -126,22 +131,30 @@ final class Picocli { static void error(CommandLine cmd, String message, Throwable throwable) { List cliArgs = getCliArgs(cmd); - cmd.getErr().println("ERROR: " + message); + logError(cmd, "ERROR: " + message); if (throwable != null) { - Throwable cause = throwable; + boolean verbose = cliArgs.stream().anyMatch((arg) -> "--verbose".equals(arg)); - do { - if (cause.getMessage() != null) { - cmd.getErr().println(String.format("ERROR: %s", cause.getMessage())); + if (throwable instanceof QuarkusConfigurationException) { + QuarkusConfigurationException quarkusConfigException = (QuarkusConfigurationException) throwable; + if (quarkusConfigException.getSuppressed() == null || quarkusConfigException.getSuppressed().length == 0) { + dumpException(cmd, quarkusConfigException, verbose); + } else if (quarkusConfigException.getSuppressed().length == 1) { + dumpException(cmd, quarkusConfigException.getSuppressed()[0], verbose); + } else { + logError(cmd, "ERROR: Multiple configuration errors during startup"); + int counter = 0; + for (Throwable inner : quarkusConfigException.getSuppressed()) { + counter++; + logError(cmd, "ERROR " + counter); + dumpException(cmd, inner, verbose); + } } - } while ((cause = cause.getCause())!= null); + } - if (cliArgs.stream().anyMatch((arg) -> "--verbose".equals(arg))) { - cmd.getErr().println("ERROR: Details:"); - throwable.printStackTrace(); - } else { - cmd.getErr().println("For more details run the same command passing the '--verbose' option."); + if (!verbose) { + logError(cmd, "For more details run the same command passing the '--verbose' option. Also you can use '--help' to see the details about the usage of the particular command."); } } @@ -151,4 +164,40 @@ final class Picocli { static void println(CommandLine cmd, String message) { cmd.getOut().println(message); } + + private static void dumpException(CommandLine cmd, Throwable cause, boolean verbose) { + if (verbose) { + logError(cmd, "ERROR: Details:", cause); + } else { + do { + if (cause.getMessage() != null) { + logError(cmd, String.format("ERROR: %s", cause.getMessage())); + } + } while ((cause = cause.getCause())!= null); + } + } + + private static void logError(CommandLine cmd, String errorMessage) { + logError(cmd, errorMessage, null); + } + + // The "cause" can be null + private static void logError(CommandLine cmd, String errorMessage, Throwable cause) { + QuarkusPlatform platform = (QuarkusPlatform) Platform.getPlatform(); + if (platform.isStarted()) { + // Can delegate to proper logger once the platform is started + if (cause == null) { + logger.error(errorMessage); + } else { + logger.error(errorMessage, cause); + } + } else { + if (cause == null) { + cmd.getErr().println(errorMessage); + } else { + cmd.getErr().println(errorMessage); + cause.printStackTrace(); + } + } + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/configuration/Messages.java b/quarkus/runtime/src/main/java/org/keycloak/configuration/Messages.java index 4858f92a4a..6e9eb99816 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/configuration/Messages.java +++ b/quarkus/runtime/src/main/java/org/keycloak/configuration/Messages.java @@ -17,7 +17,7 @@ package org.keycloak.configuration; -import java.util.List; +import org.keycloak.util.Environment; public final class Messages { @@ -30,6 +30,11 @@ public final class Messages { } static IllegalStateException httpsConfigurationNotSet() { - return new IllegalStateException("Key material not provided to setup HTTPS. Please configure your keys/certificates or enable HTTP."); + StringBuilder builder = new StringBuilder("Key material not provided to setup HTTPS. Please configure your keys/certificates or enable HTTP"); + if (!"dev".equals(Environment.getProfile())) { + builder.append(" or start the server using the 'dev' profile"); + } + builder.append("."); + return new IllegalStateException(builder.toString()); } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/configuration/PropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/configuration/PropertyMappers.java index 9a4c2b4ca0..0fe943d335 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/configuration/PropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/configuration/PropertyMappers.java @@ -32,6 +32,8 @@ import java.util.stream.Collectors; import io.quarkus.runtime.configuration.ProfileManager; import io.smallrye.config.ConfigSourceInterceptorContext; import io.smallrye.config.ConfigValue; +import org.keycloak.platform.Platform; +import org.keycloak.provider.quarkus.QuarkusPlatform; import org.keycloak.util.Environment; /** @@ -65,7 +67,7 @@ public final class PropertyMappers { } if (proceed == null || proceed.getValue() == null) { - throw Messages.httpsConfigurationNotSet(); + addDeferredConfigurationException(Messages.httpsConfigurationNotSet()); } } @@ -112,7 +114,8 @@ public final class PropertyMappers { case "passthrough": return "true"; } - throw Messages.invalidProxyMode(mode); + addDeferredConfigurationException(Messages.invalidProxyMode(mode)); + return "false"; }, "The proxy mode if the server is behind a reverse proxy. Possible values are: none, edge, reencrypt, and passthrough."); } @@ -164,7 +167,8 @@ public final class PropertyMappers { case "postgres-10": return "postgresql"; } - throw invalidDatabaseVendor(db, "h2-file", "h2-mem", "mariadb", "mysql", "postgres", "postgres-95", "postgres-10"); + addDeferredConfigurationException(invalidDatabaseVendor(db, "h2-file", "h2-mem", "mariadb", "mysql", "postgres", "postgres-95", "postgres-10")); + return "h2"; }, "The database vendor. Possible values are: h2-mem, h2-file, mariadb, mysql, postgres95, postgres10."); create("db", "quarkus.datasource.jdbc.transactions", (db, context) -> "xa", null); create("db.url", "db", "quarkus.datasource.jdbc.url", (value, context) -> { @@ -264,4 +268,10 @@ public final class PropertyMappers { } }).findFirst().orElse(null); } + + // Exception to be thrown later, so that we can properly log it and collect all possible configuration issues (not just the first one) + private static void addDeferredConfigurationException(Exception e) { + QuarkusPlatform quarkusPlatform = (QuarkusPlatform) Platform.getPlatform(); + quarkusPlatform.addDeferredException(e); + } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusConfigurationException.java b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusConfigurationException.java new file mode 100644 index 0000000000..f36ccd3bee --- /dev/null +++ b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusConfigurationException.java @@ -0,0 +1,28 @@ +/* + * Copyright 2020 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed 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.keycloak.provider.quarkus; + +/** + * Exception thrown when some error happens during initialization of Quarkus platform. Usually due the incorrect configuration of basic stuff (DB, HTTP protocol etc) + * This exception is supposed to be thrown when Quarkus platform is started (See {@link io.quarkus.runtime.StartupEvent} + * + * @author Marek Posolda + */ +public class QuarkusConfigurationException extends RuntimeException { +} diff --git a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusLifecycleObserver.java b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusLifecycleObserver.java index 948bc35089..fa3d722238 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusLifecycleObserver.java +++ b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusLifecycleObserver.java @@ -38,7 +38,19 @@ public class QuarkusLifecycleObserver { private static final String KEYCLOAK_ADMIN_PASSWORD_ENV_VAR = "KEYCLOAK_ADMIN_PASSWORD"; void onStartupEvent(@Observes StartupEvent event) { - Runnable startupHook = ((QuarkusPlatform) Platform.getPlatform()).startupHook; + QuarkusPlatform platform = (QuarkusPlatform) Platform.getPlatform(); + platform.started(); + + // Check if we had any exceptions during configuration phase + if (!platform.getDeferredExceptions().isEmpty()) { + QuarkusConfigurationException quarkusException = new QuarkusConfigurationException(); + for (Throwable inner : platform.getDeferredExceptions()) { + quarkusException.addSuppressed(inner); + } + throw quarkusException; + } + + Runnable startupHook = platform.startupHook; if (startupHook != null) { startupHook.run(); diff --git a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusPlatform.java b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusPlatform.java index c11490bb56..1510937596 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusPlatform.java +++ b/quarkus/runtime/src/main/java/org/keycloak/provider/quarkus/QuarkusPlatform.java @@ -17,6 +17,10 @@ package org.keycloak.provider.quarkus; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; + import org.keycloak.platform.PlatformProvider; public class QuarkusPlatform implements PlatformProvider { @@ -24,6 +28,9 @@ public class QuarkusPlatform implements PlatformProvider { Runnable startupHook; Runnable shutdownHook; + private AtomicBoolean started = new AtomicBoolean(false); + private List deferredExceptions = new CopyOnWriteArrayList<>(); + @Override public void onStartup(Runnable startupHook) { this.startupHook = startupHook; @@ -39,4 +46,28 @@ public class QuarkusPlatform implements PlatformProvider { throw new RuntimeException(cause); } + /** + * Called when Quarkus platform is started + */ + public void started() { + this.started.set(true); + } + + public boolean isStarted() { + return started.get(); + } + + /** + * Add the exception, which won't be thrown right-away, but should be thrown later after QuarkusPlatform is initialized (including proper logging) + * + * @param t + */ + public void addDeferredException(Throwable t) { + deferredExceptions.add(t); + } + + public List getDeferredExceptions() { + return deferredExceptions; + } + }