8000 fix: converting db-connect-timeout to be used for all datasources by shawkins · Pull Request #48055 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

fix: converting db-connect-timeout to be used for all datasources #48055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mabartos merged 2 commits into from
Apr 16, 2026
Merged

fix: converting db-connect-timeout to be used for all datasources #48055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2572af1
fix: converting db-connect-timeout to be used for all datasources
shawkins Apr 14, 2026
cbf82ea
Apply suggestions from code review
shawkins Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions quarkus/config-api/src/main/java/org/keycloak/config/DatabaseOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ public static final class Datasources {
* Example: for `db-dialect`, `db-dialect-<datasource>` is created
*/
public static final List<String> OPTIONS_DATASOURCES = Stream.of(
DB_CONNECT_TIMEOUT,
DB_DIALECT,
DB_DRIVER,
DB,
Expand Down
147 changes: 51 additions & 96 deletions ...main/java/org/keycloak/quarkus/runtime/configuration/mappers/DatabasePropertyMappers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import org.keycloak.config.OptionsUtil;
import org.keycloak.config.TransactionOptions;
import org.keycloak.config.database.Database;
import org.keycloak.config.database.Database.Vendor;
import org.keycloak.quarkus.runtime.cli.Picocli;
import org.keycloak.quarkus.runtime.cli.PropertyException;
import org.keycloak.quarkus.runtime.configuration.Configuration;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.ValueMapper;
import org.keycloak.utils.StringUtil;

import io.quarkus.datasource.common.runtime.DatabaseKind;
Expand Down Expand Up @@ -96,6 +98,18 @@ public List<PropertyMapper<?>> getPropertyMappers() {
.to(JDBC_ACQUISITION_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context) -> computeAcquisitionTimeout(value))
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, getConnectTimeout(EnumSet.of(Database.Vendor.MYSQL, Database.Vendor.MARIADB, Database.Vendor.POSTGRES, Database.Vendor.TIDB), "connectTimeout"))
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(ORACLEDB_CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, getConnectTimeout(EnumSet.of(Database.Vendor.ORACLE), "oracle.net.CONNECT_TIMEOUT"))
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(MSSQL_CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, getConnectTimeout(EnumSet.of(Database.Vendor.MSSQL), "loginTimeout"))
.build(),
fromOption(DatabaseOptions.DB_URL_HOST)
.paramLabel("hostname")
.build(),
Expand Down Expand Up @@ -203,8 +217,8 @@ public List<PropertyMapper<?>> getPropertyMappers() {
DB_POOL_MAX_SIZE, mapper -> mapper.mapFrom(DB_POOL_MAX_SIZE)
));

// finally add mappers that utilize isEnabled - that won't work as expected for multiple datasources
// so these will only affect the primary datasource
// finally add mappers that aren't intended to work with other datasources
// - also this usage of isEnabled won't work correctly with wildcard mappers
result.addAll(List.of(
fromOption(DatabaseOptions.DB_POSTGRESQL_TARGET_SERVER_TYPE)
.to(PG_TARGET_SERVER_TYPE)
Expand All @@ -213,42 +227,6 @@ public List<PropertyMapper<?>> getPropertyMappers() {
fromOption(SYNTHETIC_RUNTIME_DB_OPTION).mapFrom(DB, (name, value, context) -> "false")
.to(MSSQL_SEND_STRING_PARAMETER_AS_UNICODE)
.isEnabled(DatabasePropertyMappers::isMssqlSendStringParametersAsUnicode)
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context)
-> durationToMillis(value))
.isEnabled(DatabasePropertyMappers::isMysqlConnectTimeoutEnabled)
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context)
-> durationToMillis(value))
.isEnabled(DatabasePropertyMappers::isMariadbConnectTimeoutEnabled)
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(ORACLEDB_CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context)
-> durationToMillis(value))
.isEnabled(DatabasePropertyMappers::isOracleConnectTimeoutEnabled)
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(MSSQL_CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context)
-> durationToSeconds(value))
.isEnabled(DatabasePropertyMappers::isMssqlLoginTimeoutEnabled)
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context)
-> durationToSeconds(value))
.isEnabled(DatabasePropertyMappers::isPostgresConnectTimeoutEnabled)
.build(),
fromOption(DatabaseOptions.DB_CONNECT_TIMEOUT)
.to(CONNECT_TIMEOUT)
.mapFrom(DatabaseOptions.DB_CONNECT_TIMEOUT, (name, value, context)
-> durationToMillis(value))
.isEnabled(DatabasePropertyMappers::isTidbConnectTimeoutEnabled)
.build()
));
return result;
Expand Down Expand Up @@ -314,49 +292,36 @@ public static boolean isMssqlSendStringParametersAsUnicode() {
!dbUrlProperties.contains("sendStringParametersAsUnicode");
}

public static boolean isMysqlConnectTimeoutEnabled() {
return isConnectTimeoutEnabled(Database.Vendor.MYSQL, "connectTimeout");
}

public static boolean isMariadbConnectTimeoutEnabled() {
return isConnectTimeoutEnabled(Database.Vendor.MARIADB, "connectTimeout");
}

public static boolean isOracleConnectTimeoutEnabled() {
return isConnectTimeoutEnabled(Database.Vendor.ORACLE, "oracle.net.CONNECT_TIMEOUT");
}

public static boolean isMssqlLoginTimeoutEnabled() {
return isConnectTimeoutEnabled(Database.Vendor.MSSQL, "loginTimeout");
}
private static ValueMapper getConnectTimeout(Collection<Database.Vendor> validForVendors, String timeoutProperty) {
return (String datasource, String value, ConfigSourceInterceptorContext context) -> {
String db = getDatasourceOptionValue(DB, datasource).orElse(null);
Database.Vendor vendor = Database.getVendor(db).orElse(null);

public static boolean isPostgresConnectTimeoutEnabled() {
return isConnectTimeoutEnabled(Database.Vendor.POSTGRES, "connectTimeout");
}

public static boolean isTidbConnectTimeoutEnabled() {
return isConnectTimeoutEnabled(Database.Vendor.TIDB, "connectTimeout");
}
if (!validForVendors.contains(vendor)) {
// this jdbc property is not for this vendor
return null;
}

private static boolean isConnectTimeoutEnabled(Database.Vendor expectedVendor, String timeoutProperty) {
String db = Configuration.getConfigValue(DB).getValue();
Database.Vendor vendor = Database.getVendor(db).orElse(null);
if (vendor != expectedVendor) {
return false;
}
String dbDriver = getDatasourceOptionValue(DatabaseOptions.DB_DRIVER, datasource).orElse(null);
if (!Objects.equals(Database.getDriver(db, true).orElse(null), dbDriver) &&
@pruivo pruivo Apr 15, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we drop this check? It annoys me that the CLI options only apply to default drivers. cc @ahus1

@shawkins shawkins Apr 15, 2026
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pruivo do you want to address that here or in a follow-up?

@pruivo pruivo Apr 15, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawkins If you agree with the suggestion, do it here 👍

@shawkins shawkins Apr 15, 2026
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of the concern is that an alternative driver, such as datadirect, may not use the same property key/value semantics as the standard one, so it's better to do nothing than something that may be wrong.

!Objects.equals(Database.getDriver(db, false).orElse(null), dbDriver)) {
// Custom JDBC driver (e.g. AWS JDBC Wrapper) — do not inject defaults
return null;
}

String dbDriver = Configuration.getConfigValue(DatabaseOptions.DB_DRIVER).getValue();
if (!Objects.equals(Database.getDriver(db, true).orElse(null), dbDriver) &&
!Objects.equals(Database.getDriver(db, false).orElse(null), dbDriver)) {
// Custom JDBC driver (e.g. AWS JDBC Wrapper) — do not inject defaults
return false;
}
String dbUrl = findDatabaseUrl(datasource).orElse("");
String dbUrlProperties = getDatasourceOptionValue(DatabaseOptions.DB_URL_PROPERTIES, datasource).orElse("");

String dbUrl = Configuration.getConfigValue(DatabaseOptions.DB_URL).getValueOrDefault("");
String dbUrlProperties = Configuration.getKcConfigValue(DatabaseOptions.DB_URL_PROPERTIES.getKey()).getValueOrDefault("");
// Property already set explicitly by the user — do not override
if (dbUrl.contains(timeoutProperty) || dbUrlProperties.contains(timeoutProperty)) {
return null;
}

// Property already set explicitly by the user — do not override
return !dbUrl.contains(timeoutProperty) && !dbUrlProperties.contains(timeoutProperty);
if (vendor == Vendor.MSSQL || vendor == Vendor.POSTGRES) {
return durationToSeconds(value);
}
return durationToMillis(value);
};
}

private static String durationToMillis(String value) {
Expand Down Expand Up @@ -599,41 +564,31 @@ private static String computePostgresSSLFactory(String datasource, String db, Co
return findTlsTrustStoreFile(datasource).isEmpty() ? value : null;
}

private static Optional<String> findDatabaseUrl(String datasource) {
private static Optional<String> getDatasourceOptionValue(Option<?> opt, String datasource) {
var option = datasource == null ?
Optional.of(DB_URL.getKey()) :
getNamedKey(DB_URL, datasource);
Optional.of(opt.getKey()) :
getNamedKey(opt, datasource);
return option.map(Configuration::getKcConfigValue)
.map(ConfigValue::getValue);
}

private static Optional<String> findDatabaseUrl(String datasource) {
return getDatasourceOptionValue(DB_URL, datasource);
}

private static Database.Vendor getDatabaseVendor(String datasource) {
var option = datasource == null ?
Optional.of(DB.getKey()) :
getNamedKey(DB, datasource);
return option.map(Configuration::getKcConfigValue)
.map(ConfigValue::getValue)
.flatMap(Database::getVendor)
.orElseThrow();
return getDatasourceOptionValue(DB, datasource).flatMap(Database::getVendor).orElseThrow();
}

private static DatabaseOptions.DatabaseTlsMode getDatabaseTlsMode(String datasource) {
var option = datasource == null ?
Optional.of(DB_TLS_MODE.getKey()) :
getNamedKey(DB_TLS_MODE, datasource);
return option.map(Configuration::getKcConfigValue)
.map(ConfigValue::getValue)
return getDatasourceOptionValue(DB_TLS_MODE, datasource)
.map(String::toUpperCase)
.map(DatabaseOptions.DatabaseTlsMode::fromCliValue)
.orElse(DatabaseOptions.DatabaseTlsMode.DISABLED);
}

private static Optional<String> findTlsTrustStoreFile(String datasource) {
var option = datasource == null ?
Optional.of(DB_TLS_TRUST_STORE_FILE.getKey()) :
getNamedKey(DB_TLS_TRUST_STORE_FILE, datasource);
return option.map(Configuration::getKcConfigValue)
.map(ConfigValue::getValue);
return getDatasourceOptionValue(DB_TLS_TRUST_STORE_FILE, datasource);
}

private static void validateConnectTimeout(String value) {
Expand Down
2 changes: 1 addition & 1 deletion quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ private void assertUnknownOption(NonRunningPicocli nonRunningPicocli) {
containsString(Help.defaultColorScheme(nonRunningPicocli.getColorMode())
.errorText("Unknown option: '--db-pasword'").toString()));
assertThat(nonRunningPicocli.getErrString(), containsString(
"Possible solutions: --db-url, --db-connect-timeout, --db-url-host, --db-url-database, --db-url-port, --db-url-properties, --db-username, --db-password, --db-schema, --db-pool-initial-size, --db-pool-min-size, --db-pool-max-size, --db-pool-max-lifetime, --db-debug-jpql, --db-log-slow-queries-threshold, --db-tls-mode, --db-tls-trust-store-file, --db-tls-trust-store-type, --db-tls-trust-store-password, --db-driver, --db, --db-url-full-<datasource>, --db-url-host-<datasource>, --db-url-database-<datasource>, --db-url-port-<datasource>, --db-url-properties-<datasource>, --db-username-<datasource>, --db-password-<datasource>, --db-schema-<datasource>, --db-pool-initial-size-<datasource>, --db-pool-min-size-<datasource>, --db-pool-max-size-<datasource>, --db-debug-jpql-<datasource>, --db-log-slow-queries-threshold-<datasource>, --db-tls-mode-<datasource>, --db-tls-trust-store-file-<datasource>, --db-tls-trust-store-type-<datasource>, --db-tls-trust-store-password-<datasource>, --db-enabled-<datasource>, --db-driver-<datasource>, --db-kind-<datasource>"));
"Possible solutions: --db-url, --db-connect-timeout, --db-url-host, --db-url-database, --db-url-port, --db-url-properties, --db-username, --db-password, --db-schema, --db-pool-initial-size, --db-pool-min-size, --db-pool-max-size, --db-pool-max-lifetime, --db-debug-jpql, --db-log-slow-queries-threshold, --db-tls-mode, --db-tls-trust-store-file, --db-tls-trust-store-type, --db-tls-trust-store-password, --db-driver, --db, --db-url-full-<datasource>, --db-connect-timeout-<datasource>, --db-url-host-<datasource>, --db-url-database-<datasource>, --db-url-port-<datasource>, --db-url-properties-<datasource>, --db-username-<datasource>, --db-password-<datasource>, --db-schema-<datasource>, --db-pool-initial-size-<datasource>, --db-pool-min-size-<datasource>, --db-pool-max-size-<datasource>, --db-debug-jpql-<datasource>, --db-log-slow-queries-threshold-<datasource>, --db-tls-mode-<datasource>, --db-tls-trust-store-file-<datasource>, --db-tls-trust-store-type-<datasource>, --db-tls-trust-store-password-<datasource>, --db-enabled-<datasource>, --db-driver-<datasource>, --db-kind-<datasource>"));
Comment thread
mabartos marked this conversation as resolved.
Resolved
}

@Test
Expand Down
Loading
Loading
0