fix: moving / removing validations out of property mapper transformations (#28225)

* fix: moving / removing validations out of property mappers

closes: #25549

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

* moving httpproperty validation

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

---------

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-04-02 10:16:25 -04:00 committed by GitHub
parent ab1d1ae3d5
commit e90c9c7a6f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 76 additions and 222 deletions

View file

@ -1,27 +0,0 @@
/*
* Copyright 2021 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.quarkus.runtime;
/**
* 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 <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class InitializationException extends RuntimeException {
}

View file

@ -18,7 +18,6 @@
package org.keycloak.quarkus.runtime;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.jboss.logging.Logger;
@ -31,21 +30,13 @@ public final class Messages {
}
public static IllegalArgumentException invalidDatabaseVendor(String db, List<String> availableOptions) {
return new IllegalArgumentException("Invalid database vendor [" + db + "]. Possible values are: " + String.join(", ", availableOptions) + ".");
}
public static IllegalArgumentException invalidProxyMode(String mode) {
return new IllegalArgumentException("Invalid value [" + mode + "] for configuration property [proxy].");
}
public static IllegalStateException httpsConfigurationNotSet() {
public static String httpsConfigurationNotSet() {
StringBuilder builder = new StringBuilder("Key material not provided to setup HTTPS. Please configure your keys/certificates");
if (!Environment.DEV_PROFILE_VALUE.equals(Environment.getProfile())) {
builder.append(" or start the server in development mode");
}
builder.append(".");
return new IllegalStateException(builder.toString());
return builder.toString();
}
public static void cliExecutionError(CommandLine cmd, String message, Throwable cause) {
@ -56,20 +47,13 @@ public final class Messages {
return String.format("You can not '%s' the server in %s mode. Please re-build the server first, using 'kc.sh build' for the default production mode.%n", cmd, Environment.getKeycloakModeFromProfile(Environment.DEV_PROFILE_VALUE));
}
public static Throwable invalidLogLevel(String logLevel) {
public static String invalidLogLevel(String logLevel) {
Set<String> values = Arrays.stream(Logger.Level.values()).map(Logger.Level::name).map(String::toLowerCase).collect(Collectors.toSet());
return new IllegalStateException("Invalid log level: " + logLevel + ". Possible values are: " + String.join(", ", values) + ".");
return "Invalid log level: " + logLevel + ". Possible values are: " + String.join(", ", values) + ".";
}
public static Throwable invalidLogCategoryFormat(String category) {
return new IllegalStateException("Invalid log category format: " + category + ". The format is 'category:level' such as 'org.keycloak:debug'.");
public static String invalidLogCategoryFormat(String category) {
return "Invalid log category format: " + category + ". The format is 'category:level' such as 'org.keycloak:debug'.";
}
public static Throwable emptyValueForKey(String key) {
return new IllegalStateException("Value for configuration key '" + key + "' is empty.");
}
public static Throwable notRecognizedValueInList(String key, String values, String expected) {
return new IllegalStateException("Invalid values in list for key: " + key + " Values: " + values + ". Possible values are a combination of: " + expected);
}
}

View file

@ -25,7 +25,6 @@ import java.util.Optional;
import org.jboss.logging.Logger;
import org.keycloak.platform.Platform;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.InitializationException;
import org.keycloak.quarkus.runtime.Messages;
import org.keycloak.quarkus.runtime.integration.QuarkusPlatform;
@ -61,24 +60,7 @@ public final class ExecutionExceptionHandler implements CommandLine.IExecutionEx
}
if (cause != null) {
if (cause instanceof InitializationException) {
InitializationException initializationException = (InitializationException) cause;
if (initializationException.getSuppressed() == null || initializationException.getSuppressed().length == 0) {
dumpException(errorWriter, initializationException);
} else if (initializationException.getSuppressed().length == 1) {
dumpException(errorWriter, initializationException.getSuppressed()[0]);
} else {
logError(errorWriter, "ERROR: Multiple configuration errors during startup");
int counter = 0;
for (Throwable inner : initializationException.getSuppressed()) {
counter++;
logError(errorWriter, "ERROR " + counter);
dumpException(errorWriter, inner);
}
}
} else {
dumpException(errorWriter, cause);
}
dumpException(errorWriter, cause);
if (!verbose) {
logError(errorWriter, "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.");
@ -107,7 +89,7 @@ public final class ExecutionExceptionHandler implements CommandLine.IExecutionEx
ConfigValue httpsCertFile = getConfig().getConfigValue("kc.https-certificate-file");
if (fse.getFile().equals(Optional.ofNullable(httpsCertFile.getValue()).orElse(null))) {
logError(errorWriter, Messages.httpsConfigurationNotSet().getMessage());
logError(errorWriter, Messages.httpsConfigurationNotSet());
}
}
}

View file

@ -19,6 +19,7 @@ package org.keycloak.quarkus.runtime.cli.command;
import org.keycloak.quarkus.runtime.KeycloakMain;
import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler;
import org.keycloak.quarkus.runtime.configuration.mappers.HttpPropertyMappers;
import picocli.CommandLine;
@ -29,6 +30,7 @@ public abstract class AbstractStartCommand extends AbstractCommand implements Ru
public void run() {
doBeforeRun();
CommandLine cmd = spec.commandLine();
HttpPropertyMappers.validateConfig();
validateConfig();
KeycloakMain.start((ExecutionExceptionHandler) cmd.getExecutionExceptionHandler(), cmd.getErr(), cmd.getParseResult().originalArgs().toArray(new String[0]));
}

View file

@ -10,9 +10,7 @@ import org.keycloak.quarkus.runtime.configuration.Configuration;
import java.util.Optional;
import static java.util.Optional.of;
import static org.keycloak.quarkus.runtime.Messages.invalidDatabaseVendor;
import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption;
import static org.keycloak.quarkus.runtime.integration.QuarkusPlatform.addInitializationException;
final class DatabasePropertyMappers {
@ -31,7 +29,6 @@ final class DatabasePropertyMappers {
.paramLabel("driver")
.build(),
fromOption(DatabaseOptions.DB)
.transformer(DatabasePropertyMappers::resolveDatabaseVendor)
.to("quarkus.datasource.db-kind")
.transformer(DatabasePropertyMappers::toDatabaseKind)
.paramLabel("vendor")
@ -111,23 +108,7 @@ final class DatabasePropertyMappers {
}
private static Optional<String> toDatabaseKind(Optional<String> db, ConfigSourceInterceptorContext context) {
Optional<String> databaseKind = Database.getDatabaseKind(db.get());
if (databaseKind.isPresent()) {
return databaseKind;
}
addInitializationException(invalidDatabaseVendor(db.get(), Database.getDatabaseAliases()));
return of("h2");
}
private static Optional<String> resolveDatabaseVendor(Optional<String> db, ConfigSourceInterceptorContext context) {
if (db.isEmpty()) {
return of("dev-file");
}
return db;
return Database.getDatabaseKind(db.get());
}
private static Optional<String> resolveUsername(Optional<String> value, ConfigSourceInterceptorContext context) {

View file

@ -1,14 +1,14 @@
package org.keycloak.quarkus.runtime.configuration.mappers;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import org.keycloak.common.crypto.FipsMode;
import org.keycloak.config.HttpOptions;
import org.keycloak.config.SecurityOptions;
import org.keycloak.quarkus.runtime.Environment;
import org.keycloak.quarkus.runtime.Messages;
import org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider;
import org.keycloak.quarkus.runtime.cli.PropertyException;
import org.keycloak.quarkus.runtime.configuration.Configuration;
import java.io.File;
import java.nio.file.Paths;
@ -18,10 +18,8 @@ import java.util.function.BiFunction;
import static java.util.Optional.empty;
import static java.util.Optional.of;
import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption;
import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers.getMapper;
import static org.keycloak.quarkus.runtime.integration.QuarkusPlatform.addInitializationException;
final class HttpPropertyMappers {
public final class HttpPropertyMappers {
private static final String QUARKUS_HTTPS_CERT_FILES = "quarkus.http.ssl.certificate.files";
private static final String QUARKUS_HTTPS_CERT_KEY_FILES = "quarkus.http.ssl.certificate.key-files";
@ -118,32 +116,39 @@ final class HttpPropertyMappers {
};
}
public static void validateConfig() {
boolean enabled = isHttpEnabled(Configuration.getOptionalKcValue(HttpOptions.HTTP_ENABLED.getKey()));
if (!enabled) {
Optional<String> value = Configuration.getOptionalKcValue(HttpOptions.HTTPS_CERTIFICATE_FILE.getKey());
if (value.isEmpty()) {
value = Configuration.getOptionalValue("quarkus.http.ssl.certificate.key-store-file");
}
if (value.isEmpty()) {
throw new PropertyException(Messages.httpsConfigurationNotSet());
}
}
}
private static BiFunction<Optional<String>, ConfigSourceInterceptorContext, Optional<String>> validatePath(String key) {
return (value, context) -> Environment.isWindows() ? value.filter(v -> v.equals(context.proceed(key).getValue())).map(p -> p.replace("\\", "/")) : value;
}
private static Optional<String> getHttpEnabledTransformer(Optional<String> value, ConfigSourceInterceptorContext context) {
return of(isHttpEnabled(value) ? "enabled" : "disabled");
}
private static boolean isHttpEnabled(Optional<String> value) {
boolean enabled = Boolean.parseBoolean(value.get());
ConfigValue proxy = context.proceed(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + "proxy");
Optional<String> proxy = Configuration.getOptionalKcValue("proxy");
if (Environment.isDevMode() || Environment.isImportExportMode()
|| (proxy != null && "edge".equalsIgnoreCase(proxy.getValue()))) {
|| ("edge".equalsIgnoreCase(proxy.orElse("")))) {
enabled = true;
}
if (!enabled) {
ConfigValue proceed = context.proceed("kc.https-certificate-file");
if (proceed == null || proceed.getValue() == null) {
proceed = getMapper("quarkus.http.ssl.certificate.key-store-file").getConfigValue(context);
}
if (proceed == null || proceed.getValue() == null) {
addInitializationException(Messages.httpsConfigurationNotSet());
}
}
return of(enabled ? "enabled" : "disabled");
return enabled;
}
private static File getDefaultKeystorePathValue() {

View file

@ -4,22 +4,21 @@ import static java.util.Optional.of;
import static org.keycloak.config.LoggingOptions.GELF_ACTIVATED;
import static org.keycloak.quarkus.runtime.configuration.Configuration.isTrue;
import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption;
import static org.keycloak.quarkus.runtime.integration.QuarkusPlatform.addInitializationException;
import java.io.File;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.logging.Level;
import java.util.stream.Stream;
import org.apache.commons.lang3.ArrayUtils;
import org.jboss.logmanager.LogContext;
import org.keycloak.config.LoggingOptions;
import org.keycloak.quarkus.runtime.Messages;
import org.keycloak.quarkus.runtime.cli.PropertyException;
import io.smallrye.config.ConfigSourceInterceptorContext;
import org.keycloak.quarkus.runtime.configuration.Configuration;
public final class LoggingPropertyMappers {
@ -80,6 +79,8 @@ public final class LoggingPropertyMappers {
fromOption(LoggingOptions.LOG_LEVEL)
.to("quarkus.log.level")
.transformer(LoggingPropertyMappers::resolveLogLevel)
.validator((mapper, value) -> mapper.validateExpectedValues(value,
(c, v) -> validateLogLevel(v)))
.paramLabel("category:level")
.build()
};
@ -158,30 +159,11 @@ public final class LoggingPropertyMappers {
private static BiFunction<Optional<String>, ConfigSourceInterceptorContext, Optional<String>> resolveLogHandler(String handler) {
return (parentValue, context) -> {
//we want to fall back to console to not have nothing shown up when wrong values are set.
String consoleDependantErrorResult = handler.equals(LoggingOptions.DEFAULT_LOG_HANDLER.name()) ? Boolean.TRUE.toString() : Boolean.FALSE.toString();
String handlers = parentValue.get();
if (handlers.isBlank()) {
addInitializationException(Messages.emptyValueForKey("log"));
return of(consoleDependantErrorResult);
}
String[] logHandlerValues = handlers.split(",");
final List<String> availableLogHandlers = LoggingOptions.getAvailableHandlerNames();
if (!availableLogHandlers.containsAll(List.of(logHandlerValues))) {
addInitializationException(Messages.notRecognizedValueInList("log", handlers, String.join(",", availableLogHandlers)));
return of(consoleDependantErrorResult);
}
for (String handlerInput : logHandlerValues) {
if (handlerInput.equals(handler)) {
return of(Boolean.TRUE.toString());
}
}
return of(Boolean.FALSE.toString());
return of(String.valueOf(Stream.of(logHandlerValues).anyMatch(handler::equals)));
};
}
@ -203,37 +185,39 @@ public final class LoggingPropertyMappers {
LogContext.getLogContext().getLogger(category).setLevel(toLevel(level));
}
record CategoryLevel(String category, String levelName) {}
private static CategoryLevel validateLogLevel(String level) {
String[] parts = level.split(":");
String category = null;
String categoryLevel;
if (parts.length == 1) {
categoryLevel = parts[0];
} else if (parts.length == 2) {
category = parts[0];
categoryLevel = parts[1];
} else {
throw new PropertyException(Messages.invalidLogCategoryFormat(level));
}
try {
Level levelType = toLevel(categoryLevel);
return new CategoryLevel(category, levelType.getName());
} catch (IllegalArgumentException iae) {
throw new PropertyException(Messages.invalidLogCategoryFormat(level));
}
}
private static Optional<String> resolveLogLevel(Optional<String> value, ConfigSourceInterceptorContext configSourceInterceptorContext) {
Optional<String> rootLevel = of(LoggingOptions.DEFAULT_LOG_LEVEL.name());
for (String level : value.get().split(",")) {
String[] parts = level.split(":");
String category = null;
String categoryLevel;
if (parts.length == 1) {
categoryLevel = parts[0];
} else if (parts.length == 2) {
category = parts[0];
categoryLevel = parts[1];
var categoryLevel = validateLogLevel(level);
if (categoryLevel.category == null) {
rootLevel = of(categoryLevel.levelName);
} else {
addInitializationException(Messages.invalidLogCategoryFormat(level));
return rootLevel;
}
Level levelType;
try {
levelType = toLevel(categoryLevel);
} catch (IllegalArgumentException iae) {
addInitializationException(Messages.invalidLogLevel(categoryLevel));
return rootLevel;
}
if (category == null) {
rootLevel = of(levelType.getName());
} else {
setCategoryLevel(category, levelType.getName());
setCategoryLevel(categoryLevel.category, categoryLevel.levelName);
}
}

View file

@ -19,23 +19,13 @@ package org.keycloak.quarkus.runtime.integration;
import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;
import org.jboss.logging.Logger;
import org.keycloak.Config;
import org.keycloak.platform.Platform;
import org.keycloak.platform.PlatformProvider;
import org.keycloak.quarkus.runtime.InitializationException;
import org.keycloak.quarkus.runtime.Environment;
import io.quarkus.runtime.Quarkus;
@ -49,36 +39,7 @@ public class QuarkusPlatform implements PlatformProvider {
return "Quarkus";
}
public static void addInitializationException(Throwable throwable) {
QuarkusPlatform platform = (QuarkusPlatform) Platform.getPlatform();
platform.addDeferredException(throwable);
}
/**
* <p>Throws a {@link InitializationException} exception to indicate errors during the startup.
*
* <p>Calling this method after the server is started has no effect but just the exception being thrown.
*
* @throws InitializationException the exception holding all errors during startup.
*/
public static void exitOnError() throws InitializationException {
QuarkusPlatform platform = (QuarkusPlatform) Platform.getPlatform();
// Check if we had any exceptions during initialization phase
if (!platform.getDeferredExceptions().isEmpty()) {
InitializationException quarkusException = new InitializationException();
for (Throwable inner : platform.getDeferredExceptions()) {
quarkusException.addSuppressed(inner);
}
// reset this instance, mainly deferred exceptions, so that the subsequent starts do not fail due to previous errors
// this is mainly important when the server is running in test mode
platform.reset();
throw quarkusException;
}
}
private AtomicBoolean started = new AtomicBoolean(false);
private List<Throwable> deferredExceptions = new CopyOnWriteArrayList<>();
private File tmpDir;
@Override
@ -97,19 +58,6 @@ public class QuarkusPlatform implements PlatformProvider {
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
*/
private void addDeferredException(Throwable t) {
deferredExceptions.add(t);
}
List<Throwable> getDeferredExceptions() {
return deferredExceptions;
}
@Override
public File getTmpDirectory() {
if (tmpDir == null) {
@ -155,10 +103,6 @@ public class QuarkusPlatform implements PlatformProvider {
return tmpDir;
}
private void reset() {
deferredExceptions.clear();
}
@Override
public ClassLoader getScriptEngineClassLoader(Config.Scope scriptProviderConfig) {
// It is fine to return null assuming that nashorn and it's dependencies are included on the classpath (usually "providers" directory)

View file

@ -37,7 +37,6 @@ public class QuarkusKeycloakApplication extends KeycloakApplication {
void onStartupEvent(@Observes StartupEvent event) {
QuarkusPlatform platform = (QuarkusPlatform) Platform.getPlatform();
platform.started();
QuarkusPlatform.exitOnError();
startup();
}

View file

@ -137,7 +137,7 @@ public class IgnoredArtifactsTest {
resultArtifacts,
CoreMatchers.hasItems(includedArtifacts.toArray(new String[0])));
} finally {
System.setProperty(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + DatabaseOptions.DB.getKey(), "");
System.getProperties().remove(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + DatabaseOptions.DB.getKey());
}
}

View file

@ -94,7 +94,7 @@ class BuildCommandDistTest {
distribution.removeProperty("proxy");
CLIResult result = distribution.run("start", "--hostname=mykeycloak", OPTIMIZED_BUILD_OPTION_LONG);
result.assertMessage("Key material not provided to setup HTTPS");
result.assertError("Key material not provided to setup HTTPS");
}
@Test

View file

@ -99,12 +99,12 @@ public class OptionsDistTest {
@Test
@Order(7)
@Launch({"start", "--log=console", "--log-gelf-include-stack-trace=true"})
@Launch({"start-dev", "--log=console", "--log-gelf-include-stack-trace=true"})
public void testDisabledGelfOption(LaunchResult result) {
CLIResult cliResult = (CLIResult) result;
cliResult.assertError("Disabled option: '--log-gelf-include-stack-trace'. Available only when GELF is activated");
cliResult.assertError("Possible solutions: --log, --log-console-output, --log-console-format, --log-console-color, --log-level");
cliResult.assertError("Try '" + KeycloakDistribution.SCRIPT_CMD + " start --help' for more information on the available options.");
cliResult.assertError("Try '" + KeycloakDistribution.SCRIPT_CMD + " start-dev --help' for more information on the available options.");
cliResult.assertError("Specify '--help-all' to obtain information on all options and their availability.");
}

View file

@ -41,8 +41,8 @@ public class StartCommandDistTest {
@Test
@Launch({ "start", "--hostname-strict=false" })
void failNoTls(LaunchResult result) {
assertTrue(result.getOutput().contains("Key material not provided to setup HTTPS"),
() -> "The Output:\n" + result.getOutput() + "doesn't contains the expected string.");
assertTrue(result.getErrorOutput().contains("Key material not provided to setup HTTPS"),
() -> "The Output:\n" + result.getErrorOutput() + "doesn't contains the expected string.");
}
@Test