Prevent users configuring max-count=-1 for caches with a default upper-bound

Closes #33146

Signed-off-by: Ryan Emerson <remerson@ibm.com>
This commit is contained in:
Ryan Emerson 2025-10-02 20:58:28 +01:00 committed by GitHub
parent 4f24f93b85
commit 5cb0562fd2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 54 additions and 20 deletions

View File

@ -162,7 +162,7 @@ apply the upper bound to. For example, to apply an upper-bound of `1000` to the
`--cache-embedded-offline-sessions-max-count=1000`. An upper bound can not be defined on the following caches:
`actionToken`, `authenticationSessions`, `loginFailures`, `work`.
Setting a maximum cache size for `sessions`, `clientSessions`, `offlineSessions` and `offlineClientSessions` is not supported when volatile sessions are enabled.
Setting a maximum cache size for `sessions`, `clientSessions` is not supported when volatile sessions are enabled.
=== Specify your own cache configuration file

View File

@ -53,6 +53,7 @@ import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.A
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLIENT_SESSION_CACHE_NAME;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_CACHE_NAMES;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_CACHE_NUM_OWNERS;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_MAX_COUNT_CACHES;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CRL_CACHE_DEFAULT_MAX;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CRL_CACHE_NAME;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.KEYS_CACHE_DEFAULT_MAX;
@ -122,7 +123,7 @@ public final class CacheConfigurator {
public static void applyDefaultConfiguration(ConfigurationBuilderHolder holder, boolean warnMutate) {
var configs = holder.getNamedConfigurationBuilders();
boolean userProvidedConfig = false;
boolean clustered = holder.getGlobalConfigurationBuilder().transport().getTransport() != null;
boolean clustered = isClustered(holder);
for (var name : ALL_CACHES_NAME) {
var config = configs.get(name);
if (config == null) {
@ -169,7 +170,7 @@ public final class CacheConfigurator {
if (cacheBuilder == null) {
throw cacheNotFound(WORK_CACHE_NAME);
}
if (holder.getGlobalConfigurationBuilder().cacheContainer().transport().getTransport() == null) {
if (!isClustered(holder)) {
// non-clustered, Keycloak started in dev mode?
return;
}
@ -200,13 +201,25 @@ public final class CacheConfigurator {
* the {@code holder}. This could indicate a missing or incorrect configuration.
*/
public static void configureCacheMaxCount(Config.Scope keycloakConfig, ConfigurationBuilderHolder holder, Stream<String> caches) {
boolean clustered = isClustered(holder);
for (var it = caches.iterator(); it.hasNext(); ) {
var name = it.next();
var builder = holder.getNamedConfigurationBuilders().get(name);
if (builder == null) {
throw cacheNotFound(name);
}
setMemoryMaxCount(keycloakConfig, name, builder);
var maxCount = keycloakConfig.getLong(maxCountConfigKey(name));
if (maxCount != null) {
if (maxCount < 0) {
// Prevent users setting an unbounded max-count for any cache that already has a default max-count defined
maxCount = getCacheConfiguration(name, clustered).memory().maxCount();
if (maxCount > -1)
logger.infof("Ignoring unbounded max-count for cache '%s', reverting to default max of %d entries.", name, maxCount);
} else {
logger.debugf("Overwriting max-count for cache '%s' to %s entries", name, maxCount);
}
builder.memory().maxCount(maxCount);
}
}
}
@ -217,13 +230,14 @@ public final class CacheConfigurator {
* @throws IllegalStateException if an Infinispan cache from the provided {@code caches} stream is not defined in
* the {@code holder}. This could indicate a missing or incorrect configuration.
*/
public static void configureSessionsCachesForPersistentSessions(ConfigurationBuilderHolder holder) {
public static void configureSessionsCachesForPersistentSessions(Config.Scope keycloakConfig, ConfigurationBuilderHolder holder) {
logger.debug("Configuring session cache (persistent user sessions)");
for (var name : Arrays.asList(USER_SESSION_CACHE_NAME, CLIENT_SESSION_CACHE_NAME, OFFLINE_USER_SESSION_CACHE_NAME, OFFLINE_CLIENT_SESSION_CACHE_NAME)) {
for (var name : CLUSTERED_MAX_COUNT_CACHES) {
var builder = holder.getNamedConfigurationBuilders().get(name);
if (builder == null) {
throw cacheNotFound(name);
}
setMemoryMaxCount(keycloakConfig, name, builder);
if (builder.memory().maxCount() == -1) {
logger.infof("Persistent user sessions enabled and no memory limit found in configuration. Setting max entries for %s to %d entries.", name, SESSIONS_CACHE_DEFAULT_MAX);
builder.memory().maxCount(SESSIONS_CACHE_DEFAULT_MAX);
@ -243,13 +257,15 @@ public final class CacheConfigurator {
* @throws IllegalStateException if an Infinispan cache from the provided {@code caches} stream is not defined in
* the {@code holder}. This could indicate a missing or incorrect configuration.
*/
public static void configureSessionsCachesForVolatileSessions(ConfigurationBuilderHolder holder) {
public static void configureSessionsCachesForVolatileSessions(Config.Scope keycloakConfig, ConfigurationBuilderHolder holder) {
logger.debug("Configuring session cache (volatile user sessions)");
for (var name : Arrays.asList(USER_SESSION_CACHE_NAME, CLIENT_SESSION_CACHE_NAME)) {
var builder = holder.getNamedConfigurationBuilders().get(name);
if (builder == null) {
throw cacheNotFound(name);
}
setMemoryMaxCount(keycloakConfig, name, builder);
if (builder.memory().maxCount() != -1) {
logger.infof("Persistent user sessions disabled and memory limit is set. Ignoring cache limits to avoid losing sessions for cache %s.", name);
builder.memory().maxCount(-1);
@ -262,11 +278,13 @@ public final class CacheConfigurator {
}
}
for (var name : Arrays.asList( OFFLINE_USER_SESSION_CACHE_NAME, OFFLINE_CLIENT_SESSION_CACHE_NAME)) {
for (var name : Arrays.asList(OFFLINE_USER_SESSION_CACHE_NAME, OFFLINE_CLIENT_SESSION_CACHE_NAME)) {
var builder = holder.getNamedConfigurationBuilders().get(name);
if (builder == null) {
throw cacheNotFound(name);
}
setMemoryMaxCount(keycloakConfig, name, builder);
if (builder.memory().maxCount() == -1) {
logger.infof("Offline sessions should have a max count set to avoid excessive memory usage. Setting a default cache limit of %d for cache %s.", name, SESSIONS_CACHE_DEFAULT_MAX);
builder.memory().maxCount(SESSIONS_CACHE_DEFAULT_MAX);
@ -401,9 +419,8 @@ public final class CacheConfigurator {
}
private static void setMemoryMaxCount(Config.Scope keycloakConfig, String name, ConfigurationBuilder builder) {
var maxCount = keycloakConfig.getInt(maxCountConfigKey(name));
var maxCount = keycloakConfig.getLong(maxCountConfigKey(name));
if (maxCount != null) {
logger.debugf("Overwriting max-count for cache '%s' to %s entries", name, maxCount);
builder.memory().maxCount(maxCount);
}
}
@ -507,4 +524,8 @@ public final class CacheConfigurator {
return null;
}
}
private static boolean isClustered(ConfigurationBuilderHolder holder) {
return holder.getGlobalConfigurationBuilder().transport().getTransport() != null;
}
}

View File

@ -17,6 +17,13 @@
package org.keycloak.spi.infinispan.impl.embedded;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.ALL_CACHES_NAME;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_CACHE_NUM_OWNERS;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_MAX_COUNT_CACHES;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.LOCAL_CACHE_NAMES;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.LOCAL_MAX_COUNT_CACHES;
import static org.keycloak.spi.infinispan.impl.embedded.JGroupsConfigurator.createJGroupsProperties;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.nio.file.Paths;
@ -25,7 +32,6 @@ import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
import io.micrometer.core.instrument.Metrics;
import org.infinispan.configuration.cache.ConfigurationBuilder;
import org.infinispan.configuration.cache.StatisticsConfigurationBuilder;
import org.infinispan.configuration.global.ShutdownHookBehavior;
@ -51,11 +57,7 @@ import org.keycloak.spi.infinispan.CacheEmbeddedConfigProviderFactory;
import org.keycloak.spi.infinispan.JGroupsCertificateProvider;
import org.keycloak.spi.infinispan.impl.Util;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.ALL_CACHES_NAME;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_MAX_COUNT_CACHES;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.CLUSTERED_CACHE_NUM_OWNERS;
import static org.keycloak.connections.infinispan.InfinispanConnectionProvider.LOCAL_CACHE_NAMES;
import static org.keycloak.spi.infinispan.impl.embedded.JGroupsConfigurator.createJGroupsProperties;
import io.micrometer.core.instrument.Metrics;
/**
* The default implementation of {@link CacheEmbeddedConfigProviderFactory}.
@ -122,7 +124,7 @@ public class DefaultCacheEmbeddedConfigProviderFactory implements CacheEmbeddedC
var builder = ProviderConfigurationBuilder.create();
Util.copyFromOption(builder, CONFIG, "file", ProviderConfigProperty.STRING_TYPE, CachingOptions.CACHE_CONFIG_FILE, false);
Util.copyFromOption(builder, HISTOGRAMS, "enabled", ProviderConfigProperty.BOOLEAN_TYPE, CachingOptions.CACHE_METRICS_HISTOGRAMS_ENABLED, false);
Stream.concat(Arrays.stream(LOCAL_CACHE_NAMES), Arrays.stream(CLUSTERED_MAX_COUNT_CACHES))
Stream.concat(Arrays.stream(LOCAL_MAX_COUNT_CACHES), Arrays.stream(CLUSTERED_MAX_COUNT_CACHES))
.forEach(name -> Util.copyFromOption(builder, CacheConfigurator.maxCountConfigKey(name), "max-count", ProviderConfigProperty.INTEGER_TYPE, CachingOptions.maxCountOption(name), false));
Arrays.stream(CLUSTERED_CACHE_NUM_OWNERS)
.forEach(name -> builder.property()
@ -170,13 +172,13 @@ public class DefaultCacheEmbeddedConfigProviderFactory implements CacheEmbeddedC
private static ConfigurationBuilderHolder configureSingleSiteWithVolatileSessions(ConfigurationBuilderHolder holder, Config.Scope keycloakConfig, KeycloakSessionFactory factory) {
singleSiteConfiguration(keycloakConfig, holder, factory);
CacheConfigurator.configureSessionsCachesForVolatileSessions(holder);
CacheConfigurator.configureSessionsCachesForVolatileSessions(keycloakConfig, holder);
return holder;
}
private static ConfigurationBuilderHolder configureSingleSiteWithPersistentSessions(ConfigurationBuilderHolder holder, Config.Scope keycloakConfig, KeycloakSessionFactory factory) {
singleSiteConfiguration(keycloakConfig, holder, factory);
CacheConfigurator.configureSessionsCachesForPersistentSessions(holder);
CacheConfigurator.configureSessionsCachesForPersistentSessions(keycloakConfig, holder);
return holder;
}
@ -221,7 +223,6 @@ public class DefaultCacheEmbeddedConfigProviderFactory implements CacheEmbeddedC
private static void singleSiteConfiguration(Config.Scope config, ConfigurationBuilderHolder holder, KeycloakSessionFactory factory) {
logger.debug("Configuring Infinispan for single-site deployment");
CacheConfigurator.checkCachesExist(holder, Arrays.stream(ALL_CACHES_NAME));
CacheConfigurator.configureCacheMaxCount(config, holder, Arrays.stream(CLUSTERED_MAX_COUNT_CACHES));
CacheConfigurator.configureNumOwners(config, holder);
CacheConfigurator.validateWorkCacheConfiguration(holder);
CacheConfigurator.ensureMinimumOwners(holder);

View File

@ -209,6 +209,18 @@ public class ClusterConfigDistTest {
result.assertNoMessage("Modifying the default cache configuration in the config file without setting cache-config-mutate=true is deprecated.");
}
@Test
@Launch({ "start-dev", "--cache-embedded-users-max-count=-1" })
void testNegativeMaxCountIgnoredForBoundedCache(CLIResult result) {
result.assertMessage("Ignoring unbounded max-count for cache 'users'");
}
@Test
@Launch({ "start-dev", "--cache-embedded-sessions-max-count=-1", "--features-disabled=persistent-user-sessions" })
void testNegativeMaxCountAllowedForVolatileCache(CLIResult result) {
result.assertNoMessage("Ignoring unbounded max-count for cache 'sessions'");
}
public static class ConfigureCacheUsingAsyncEncryption implements Consumer<KeycloakDistribution> {
@Override