Simplify rate limiters by making them all dynamic

This commit is contained in:
Jonathan Klabunde Tomer
2025-05-21 10:29:26 -07:00
committed by GitHub
parent aafcd63a9f
commit 35604cf151
12 changed files with 449 additions and 472 deletions

View File

@@ -0,0 +1,273 @@
/*
* Copyright 2025 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.limits;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import io.lettuce.core.ScriptOutputType;
import java.io.IOException;
import java.time.Duration;
import java.time.Instant;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.redis.ClusterLuaScript;
import org.whispersystems.textsecuregcm.redis.RedisClusterExtension;
import org.whispersystems.textsecuregcm.util.TestClock;
class DynamicRateLimiterTest {
private ClusterLuaScript validateRateLimitScript;
private static final TestClock CLOCK = TestClock.pinned(Instant.now());
@RegisterExtension
private static final RedisClusterExtension REDIS_CLUSTER_EXTENSION = RedisClusterExtension.builder().build();
@BeforeEach
void setUp() throws IOException {
validateRateLimitScript = ClusterLuaScript.fromResource(
REDIS_CLUSTER_EXTENSION.getRedisCluster(), "lua/validate_rate_limit.lua", ScriptOutputType.INTEGER);
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void validate(final boolean failOpen) {
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, Duration.ofHours(1), failOpen),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void validateAsync(final boolean failOpen) {
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, Duration.ofHours(1), failOpen),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validateAsync(key).toCompletableFuture().join());
final CompletionException completionException =
assertThrows(CompletionException.class, () -> rateLimiter.validateAsync(key).toCompletableFuture().join());
assertInstanceOf(RateLimitExceededException.class, completionException.getCause());
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void validateFailOpen(final boolean failOpen) {
final ClusterLuaScript failingScript = mock(ClusterLuaScript.class);
when(failingScript.execute(any(), any())).thenThrow(new RuntimeException("OH NO"));
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, Duration.ofHours(1), failOpen),
failingScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
if (failOpen) {
assertDoesNotThrow(() -> rateLimiter.validate(key));
} else {
assertThrows(RuntimeException.class, () -> rateLimiter.validate(key));
}
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void validateFailOpenAsync(final boolean failOpen) {
final ClusterLuaScript failingScript = mock(ClusterLuaScript.class);
when(failingScript.executeAsync(any(), any())).thenReturn(CompletableFuture.failedFuture(new RuntimeException("OH NO")));
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, Duration.ofHours(1), failOpen),
failingScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
if (failOpen) {
assertDoesNotThrow(() -> rateLimiter.validate(key));
} else {
final CompletionException completionException =
assertThrows(CompletionException.class, () -> rateLimiter.validateAsync(key).toCompletableFuture().join());
assertInstanceOf(RuntimeException.class, completionException.getCause());
}
}
@Test
void configChange_ReduceRefillRate() {
final AtomicReference<Duration> refillRate = new AtomicReference<>(Duration.ofMinutes(5));
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, refillRate.get(), false),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
CLOCK.pin(CLOCK.instant().plus(Duration.ofMinutes(1)));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
refillRate.set(Duration.ofMinutes(1));
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
}
@Test
void configChange_IncreaseRefillRate() {
final AtomicReference<Duration> refillRate = new AtomicReference<>(Duration.ofMinutes(5));
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, refillRate.get(), false),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
CLOCK.pin(CLOCK.instant().plus(Duration.ofMinutes(5)));
assertTrue(rateLimiter.hasAvailablePermits(key, 1));
refillRate.set(Duration.ofMinutes(10));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
CLOCK.pin(CLOCK.instant().plus(Duration.ofMinutes(5)));
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
}
@Test
void configChange_ReduceBucketSize() {
final AtomicInteger bucketSize = new AtomicInteger(5);
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(bucketSize.get(), Duration.ofMinutes(1), false),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertTrue(rateLimiter.hasAvailablePermits(key, 4));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key, 5));
bucketSize.set(1);
// Changing the bucket size doesn't spend the tokens remaining in existing buckets, but does
// effectively make those buckets overflow if it got smaller. There were 4 tokens available
// before, so changing the bucket size to 1 effectively means there is 1 token left, not 0
assertTrue(rateLimiter.hasAvailablePermits(key, 1));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key, 2));
}
@Test
void configChange_IncreaseBucketSize() {
final AtomicInteger bucketSize = new AtomicInteger(5);
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(bucketSize.get(), Duration.ofMinutes(1), false),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertTrue(rateLimiter.hasAvailablePermits(key, 4));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key, 5));
bucketSize.set(10);
// Increasing the bucket size doesn't retroactively refill buckets in redis, so we have to wait
// until the bucket fills up
CLOCK.pin(CLOCK.instant().plus(Duration.ofMinutes(10)));
assertTrue(rateLimiter.hasAvailablePermits(key, 10));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key, 11));
}
@Test
void configChange_enableFailOpen() {
final ClusterLuaScript failingScript = mock(ClusterLuaScript.class);
when(failingScript.execute(any(), any())).thenThrow(new RuntimeException("OH NO"));
final AtomicBoolean failOpen = new AtomicBoolean(false);
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, Duration.ofMinutes(1), failOpen.get()),
failingScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertThrows(RuntimeException.class, () -> rateLimiter.validate(key));
failOpen.set(true);
assertDoesNotThrow(() -> rateLimiter.validate(key));
}
@Test
void configChange_disableFailOpen() {
final ClusterLuaScript failingScript = mock(ClusterLuaScript.class);
when(failingScript.execute(any(), any())).thenThrow(new RuntimeException("OH NO"));
final AtomicBoolean failOpen = new AtomicBoolean(true);
final DynamicRateLimiter rateLimiter = new DynamicRateLimiter(
"test",
() -> new RateLimiterConfig(1, Duration.ofMinutes(1), failOpen.get()),
failingScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
failOpen.set(false);
assertThrows(RuntimeException.class, () -> rateLimiter.validate(key));
}
}

View File

@@ -57,9 +57,11 @@ public class RateLimitersLuaScriptTest {
@Test
public void testWithEmbeddedRedis() throws Exception {
final RateLimiters.For descriptor = RateLimiters.For.REGISTRATION;
final Map<String, RateLimiterConfig> limiterConfig = Map.of(descriptor.id(), new RateLimiterConfig(60, Duration.ofSeconds(1), false));
when(configuration.getLimits()).thenReturn(limiterConfig);
final FaultTolerantRedisClusterClient redisCluster = REDIS_CLUSTER_EXTENSION.getRedisCluster();
final RateLimiters limiters = new RateLimiters(
Map.of(descriptor.id(), new RateLimiterConfig(60, Duration.ofSeconds(1), false)),
dynamicConfig,
RateLimiters.defaultScript(redisCluster),
redisCluster,
@@ -74,9 +76,11 @@ public class RateLimitersLuaScriptTest {
@Test
public void testTtl() throws Exception {
final RateLimiters.For descriptor = RateLimiters.For.REGISTRATION;
final Map<String, RateLimiterConfig> limiterConfig = Map.of(descriptor.id(), new RateLimiterConfig(1000, Duration.ofSeconds(1), false));
when(configuration.getLimits()).thenReturn(limiterConfig);
final FaultTolerantRedisClusterClient redisCluster = REDIS_CLUSTER_EXTENSION.getRedisCluster();
final RateLimiters limiters = new RateLimiters(
Map.of(descriptor.id(), new RateLimiterConfig(1000, Duration.ofSeconds(1), false)),
dynamicConfig,
RateLimiters.defaultScript(redisCluster),
redisCluster,
@@ -126,8 +130,11 @@ public class RateLimitersLuaScriptTest {
public void testFailOpen(final boolean failOpen) {
final RateLimiters.For descriptor = RateLimiters.For.REGISTRATION;
final FaultTolerantRedisClusterClient redisCluster = mock(FaultTolerantRedisClusterClient.class);
final Map<String, RateLimiterConfig> limiterConfig = Map.of(descriptor.id(), new RateLimiterConfig(1, Duration.ofSeconds(1), failOpen));
when(configuration.getLimits()).thenReturn(limiterConfig);
final RateLimiters limiters = new RateLimiters(
Map.of(descriptor.id(), new RateLimiterConfig(1000, Duration.ofSeconds(1), failOpen)),
dynamicConfig,
RateLimiters.defaultScript(redisCluster),
redisCluster,

View File

@@ -5,17 +5,12 @@
package org.whispersystems.textsecuregcm.limits;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import com.fasterxml.jackson.annotation.JsonProperty;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;
import java.time.Duration;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.Test;
@@ -40,48 +35,6 @@ public class RateLimitersTest {
private final MutableClock clock = MockUtils.mutableClock(0);
private static final String BAD_YAML = """
limits:
prekeys:
bucketSize: 150
permitRegenerationDuration: PT6S
unexpected:
bucketSize: 4
permitRegenerationDuration: PT30S
""";
private static final String GOOD_YAML = """
limits:
prekeys:
bucketSize: 150
permitRegenerationDuration: PT6S
failOpen: true
attachmentCreate:
bucketSize: 4
permitRegenerationDuration: PT30S
failOpen: true
""";
public record SimpleDynamicConfiguration(@Valid @NotNull @JsonProperty Map<String, RateLimiterConfig> limits) {
}
@Test
public void testValidateConfigs() throws Exception {
assertThrows(IllegalArgumentException.class, () -> {
final SimpleDynamicConfiguration dynamicConfiguration =
DynamicConfigurationManager.parseConfiguration(BAD_YAML, SimpleDynamicConfiguration.class).orElseThrow();
final RateLimiters rateLimiters = new RateLimiters(dynamicConfiguration.limits(), dynamicConfig, validateScript, redisCluster, clock);
rateLimiters.validateValuesAndConfigs();
});
final SimpleDynamicConfiguration dynamicConfiguration =
DynamicConfigurationManager.parseConfiguration(GOOD_YAML, SimpleDynamicConfiguration.class).orElseThrow();
final RateLimiters rateLimiters = new RateLimiters(dynamicConfiguration.limits(), dynamicConfig, validateScript, redisCluster, clock);
assertDoesNotThrow(rateLimiters::validateValuesAndConfigs);
}
@Test
public void testValidateDuplicates() throws Exception {
final TestDescriptor td1 = new TestDescriptor("id1");
@@ -91,7 +44,6 @@ public class RateLimitersTest {
assertThrows(IllegalStateException.class, () -> new BaseRateLimiters<>(
new TestDescriptor[] { td1, td2, td3, tdDup },
Collections.emptyMap(),
dynamicConfig,
validateScript,
redisCluster,
@@ -99,7 +51,6 @@ public class RateLimitersTest {
new BaseRateLimiters<>(
new TestDescriptor[] { td1, td2, td3 },
Collections.emptyMap(),
dynamicConfig,
validateScript,
redisCluster,
@@ -108,10 +59,10 @@ public class RateLimitersTest {
@Test
void testUnchangingConfiguration() {
final RateLimiters rateLimiters = new RateLimiters(Collections.emptyMap(), dynamicConfig, validateScript, redisCluster, clock);
final RateLimiters rateLimiters = new RateLimiters(dynamicConfig, validateScript, redisCluster, clock);
final RateLimiter limiter = rateLimiters.getRateLimitResetLimiter();
final RateLimiterConfig expected = RateLimiters.For.RATE_LIMIT_RESET.defaultConfig();
assertEquals(expected, config(limiter));
assertEquals(expected, limiter.config());
}
@Test
@@ -127,78 +78,49 @@ public class RateLimitersTest {
when(configuration.getLimits()).thenReturn(limitsConfigMap);
final RateLimiters rateLimiters = new RateLimiters(Collections.emptyMap(), dynamicConfig, validateScript, redisCluster, clock);
final RateLimiters rateLimiters = new RateLimiters(dynamicConfig, validateScript, redisCluster, clock);
final RateLimiter limiter = rateLimiters.getRateLimitResetLimiter();
limitsConfigMap.put(RateLimiters.For.RATE_LIMIT_RESET.id(), initialRateLimiterConfig);
assertEquals(initialRateLimiterConfig, config(limiter));
assertEquals(initialRateLimiterConfig, limiter.config());
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeAttemptLimiter()));
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeSuccessLimiter()));
assertEquals(baseConfig, rateLimiters.getCaptchaChallengeAttemptLimiter().config());
assertEquals(baseConfig, rateLimiters.getCaptchaChallengeSuccessLimiter().config());
limitsConfigMap.put(RateLimiters.For.RATE_LIMIT_RESET.id(), updatedRateLimiterCongig);
assertEquals(updatedRateLimiterCongig, config(limiter));
assertEquals(updatedRateLimiterCongig, limiter.config());
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeAttemptLimiter()));
assertEquals(baseConfig, config(rateLimiters.getCaptchaChallengeSuccessLimiter()));
assertEquals(baseConfig, rateLimiters.getCaptchaChallengeAttemptLimiter().config());
assertEquals(baseConfig, rateLimiters.getCaptchaChallengeSuccessLimiter().config());
}
@Test
public void testRateLimiterHasItsPrioritiesStraight() throws Exception {
final RateLimiters.For descriptor = RateLimiters.For.CAPTCHA_CHALLENGE_ATTEMPT;
final RateLimiterConfig configForDynamic = new RateLimiterConfig(1, Duration.ofMinutes(1), false);
final RateLimiterConfig configForStatic = new RateLimiterConfig(2, Duration.ofSeconds(30), false);
final RateLimiterConfig defaultConfig = descriptor.defaultConfig();
final Map<String, RateLimiterConfig> mapForDynamic = new HashMap<>();
final Map<String, RateLimiterConfig> mapForStatic = new HashMap<>();
when(configuration.getLimits()).thenReturn(mapForDynamic);
final RateLimiters rateLimiters = new RateLimiters(mapForStatic, dynamicConfig, validateScript, redisCluster, clock);
final RateLimiters rateLimiters = new RateLimiters(dynamicConfig, validateScript, redisCluster, clock);
final RateLimiter limiter = rateLimiters.forDescriptor(descriptor);
// test only default is present
mapForDynamic.remove(descriptor.id());
mapForStatic.remove(descriptor.id());
assertEquals(defaultConfig, config(limiter));
assertEquals(defaultConfig, limiter.config());
// test dynamic and no static
// test dynamic config is present
mapForDynamic.put(descriptor.id(), configForDynamic);
mapForStatic.remove(descriptor.id());
assertEquals(configForDynamic, config(limiter));
// test dynamic and static
mapForDynamic.put(descriptor.id(), configForDynamic);
mapForStatic.put(descriptor.id(), configForStatic);
assertEquals(configForDynamic, config(limiter));
// test static, but no dynamic
mapForDynamic.remove(descriptor.id());
mapForStatic.put(descriptor.id(), configForStatic);
assertEquals(configForStatic, config(limiter));
assertEquals(configForDynamic, limiter.config());
}
private record TestDescriptor(String id) implements RateLimiterDescriptor {
@Override
public boolean isDynamic() {
return false;
}
@Override
public RateLimiterConfig defaultConfig() {
return new RateLimiterConfig(1, Duration.ofMinutes(1), false);
}
}
private static RateLimiterConfig config(final RateLimiter rateLimiter) {
if (rateLimiter instanceof StaticRateLimiter rm) {
return rm.config();
}
if (rateLimiter instanceof DynamicRateLimiter rm) {
return rm.config();
}
throw new IllegalArgumentException("Rate limiter is of an unexpected type: " + rateLimiter.getClass().getName());
}
}

View File

@@ -1,74 +0,0 @@
/*
* Copyright 2025 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.limits;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
import io.lettuce.core.ScriptOutputType;
import java.io.IOException;
import java.time.Duration;
import java.time.Instant;
import java.util.concurrent.CompletionException;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.redis.ClusterLuaScript;
import org.whispersystems.textsecuregcm.redis.RedisClusterExtension;
import org.whispersystems.textsecuregcm.util.TestClock;
class StaticRateLimiterTest {
private ClusterLuaScript validateRateLimitScript;
private static final TestClock CLOCK = TestClock.pinned(Instant.now());
@RegisterExtension
private static final RedisClusterExtension REDIS_CLUSTER_EXTENSION = RedisClusterExtension.builder().build();
@BeforeEach
void setUp() throws IOException {
validateRateLimitScript = ClusterLuaScript.fromResource(
REDIS_CLUSTER_EXTENSION.getRedisCluster(), "lua/validate_rate_limit.lua", ScriptOutputType.INTEGER);
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void validate(final boolean failOpen) {
final StaticRateLimiter rateLimiter = new StaticRateLimiter("test",
new RateLimiterConfig(1, Duration.ofHours(1), failOpen),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validate(key));
assertThrows(RateLimitExceededException.class, () -> rateLimiter.validate(key));
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void validateAsync(final boolean failOpen) {
final StaticRateLimiter rateLimiter = new StaticRateLimiter("test",
new RateLimiterConfig(1, Duration.ofHours(1), failOpen),
validateRateLimitScript,
REDIS_CLUSTER_EXTENSION.getRedisCluster(),
CLOCK);
final String key = RandomStringUtils.insecure().nextAlphanumeric(16);
assertDoesNotThrow(() -> rateLimiter.validateAsync(key).toCompletableFuture().join());
final CompletionException completionException =
assertThrows(CompletionException.class, () -> rateLimiter.validateAsync(key).toCompletableFuture().join());
assertInstanceOf(RateLimitExceededException.class, completionException.getCause());
}
}