Allow ratelimits to be specified in global section
Similar to how quotas could be specified in global section, allow
ratelimits to be specified there. All the limits specified in global
section are applied in addition to the limits specified in individual
groups.
Change-Id: If6e7687f49de6b74d78ddf43f6535e2613b9d8e1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java b/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java
index 504dd2f..753e8a5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java
@@ -38,6 +38,7 @@
Pattern.compile("^\\s*(\\d+)\\s*/\\s*(.*)\\s*burst\\s*(\\d+)$");
private static final Logger log = LoggerFactory.getLogger(AccountLimitsConfig.class);
static final String GROUP_SECTION = "group";
+ static final String GLOBAL_SECTION = "global";
static final SectionParser<AccountLimitsConfig> KEY =
new SectionParser<AccountLimitsConfig>() {
@Override
@@ -96,14 +97,17 @@
rateLimits = ArrayTable.create(Arrays.asList(Type.values()), groups);
for (String groupName : groups) {
- parseRateLimit(c, groupName, UPLOADPACK);
- parseRateLimit(c, groupName, RESTAPI);
+ parseRateLimit(c, GROUP_SECTION, groupName, UPLOADPACK);
+ parseRateLimit(c, GROUP_SECTION, groupName, RESTAPI);
}
+
+ parseRateLimit(c, GLOBAL_SECTION, null, UPLOADPACK);
+ parseRateLimit(c, GLOBAL_SECTION, null, RESTAPI);
}
- void parseRateLimit(Config c, String groupName, Type type) {
+ void parseRateLimit(Config c, String group, String groupName, Type type) {
String name = type.toConfigValue();
- String value = c.getString(GROUP_SECTION, groupName, name);
+ String value = c.getString(group, groupName, name);
if (value == null) {
return;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsFinder.java b/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsFinder.java
index 87413af..53651eb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsFinder.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsFinder.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.quota;
+import static com.googlesource.gerrit.plugins.quota.AccountLimitsConfig.GLOBAL_SECTION;
import static com.googlesource.gerrit.plugins.quota.AccountLimitsConfig.KEY;
import com.google.gerrit.entities.GroupDescription;
@@ -80,6 +81,14 @@
/**
* @param type type of rate limit
+ * @return global rate limit
+ */
+ public Optional<RateLimit> getGlobalRateLimit(Type type) {
+ return getRatelimits(type).map(map -> map.get(GLOBAL_SECTION));
+ }
+
+ /**
+ * @param type type of rate limit
* @param groupName name of group to lookup up rate limit for
* @return rate limit
*/
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/GlobalQuotaSection.java b/src/main/java/com/googlesource/gerrit/plugins/quota/GlobalQuotaSection.java
index a378cb4..9204629 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/GlobalQuotaSection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/GlobalQuotaSection.java
@@ -23,7 +23,7 @@
* @param cfg quota.cfg file
*/
public record GlobalQuotaSection(Config cfg) implements QuotaSection {
- public static final String GLOBAL_QUOTA = "global-quota";
+ public static final String GLOBAL_QUOTA = "global";
public String getNamespace() {
return GLOBAL_QUOTA;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/HttpModule.java b/src/main/java/com/googlesource/gerrit/plugins/quota/HttpModule.java
index fec49cd..28c0a60 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/HttpModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/HttpModule.java
@@ -33,7 +33,8 @@
import com.googlesource.gerrit.plugins.quota.AccountLimitsConfig.Type;
class HttpModule extends CacheModule {
- static final String CACHE_NAME_RESTAPI_ACCOUNTID = "restapi_rate_limits_by_account";
+ static final String CACHE_NAME_RESTAPI_ACCOUNTID = "restapi_rate_scoped_limits_by_account";
+ static final String CACHE_NAME_GLOBAL = "restapi_rate_global";
static final String CACHE_NAME_RESTAPI_REMOTEHOST = "restapi_rate_limits_by_ip";
private final String restapiLimitExceededMsg;
@@ -65,6 +66,15 @@
}
@Provides
+ @Named(CACHE_NAME_GLOBAL)
+ @Singleton
+ public LoadingCache<String, Module.Holder> getGlobalRestApiLoadingCacheByAccountId(
+ GenericFactory userFactory, AccountLimitsFinder finder) {
+ return CacheBuilder.newBuilder()
+ .build(new Module.HolderCacheLoaderByGlobalAccount(Type.RESTAPI, finder));
+ }
+
+ @Provides
@Named(CACHE_NAME_RESTAPI_REMOTEHOST)
@Singleton
public LoadingCache<String, Module.Holder> getRestApiLoadingCacheByRemoteHost(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java b/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java
index eb19c00..781787a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java
@@ -55,6 +55,7 @@
class Module extends CacheModule {
static final String CACHE_NAME_ACCOUNTID = "rate_limits_by_account";
+ static final String CACHE_NAME_GLOBAL = "rate_limits_global";
static final String CACHE_NAME_REMOTEHOST = "rate_limits_by_ip";
private final String uploadpackLimitExceededMsg;
@@ -204,6 +205,22 @@
}
}
+ static class HolderCacheLoaderByGlobalAccount extends AbstractHolderCacheLoader<String> {
+
+ protected HolderCacheLoaderByGlobalAccount(Type limitsConfigType, AccountLimitsFinder finder) {
+ super(limitsConfigType, finder);
+ }
+
+ private final Holder createWithBurstyRateLimiter() throws Exception {
+ return createWithBurstyRateLimiter(finder.getGlobalRateLimit(limitsConfigType));
+ }
+
+ @Override
+ public final Holder load(String key) throws Exception {
+ return createWithBurstyRateLimiter();
+ }
+ }
+
@Provides
@Named(CACHE_NAME_ACCOUNTID)
@Singleton
@@ -214,6 +231,15 @@
}
@Provides
+ @Named(CACHE_NAME_GLOBAL)
+ @Singleton
+ public LoadingCache<String, Module.Holder> getLoadingCacheByGlobal(
+ GenericFactory userFactory, AccountLimitsFinder finder) {
+ return CacheBuilder.newBuilder()
+ .build(new HolderCacheLoaderByGlobalAccount(Type.UPLOADPACK, finder));
+ }
+
+ @Provides
@Named(CACHE_NAME_REMOTEHOST)
@Singleton
public LoadingCache<String, Module.Holder> getLoadingCacheByRemoteHost(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListener.java b/src/main/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListener.java
index 2b2f171..37dda43 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListener.java
@@ -102,6 +102,7 @@
private final Provider<CurrentUser> user;
private final LoadingCache<Account.Id, Holder> limitsPerAccount;
private final LoadingCache<String, Holder> limitsPerRemoteHost;
+ private final LoadingCache<String, Holder> globalLimits;
private final String limitExceededMsg;
@Inject
@@ -109,10 +110,12 @@
Provider<CurrentUser> user,
@Named(Module.CACHE_NAME_ACCOUNTID) LoadingCache<Account.Id, Holder> limitsPerAccount,
@Named(Module.CACHE_NAME_REMOTEHOST) LoadingCache<String, Holder> limitsPerRemoteHost,
+ @Named(Module.CACHE_NAME_GLOBAL) LoadingCache<String, Holder> globalLimits,
@Named(RateMsgHelper.UPLOADPACK_CONFIGURABLE_MSG_ANNOTATION) String limitExceededMsg) {
this.user = user;
this.limitsPerAccount = limitsPerAccount;
this.limitsPerRemoteHost = limitsPerRemoteHost;
+ this.globalLimits = globalLimits;
this.limitExceededMsg = limitExceededMsg;
}
@@ -126,25 +129,33 @@
int cntOffered)
throws ValidationException {
RateLimiter limiter = null;
+ RateLimiter globalLimiter = null;
CurrentUser u = user.get();
if (u.isIdentifiedUser()) {
Account.Id accountId = u.asIdentifiedUser().getAccountId();
try {
limiter = limitsPerAccount.get(accountId).get();
+ globalLimiter = globalLimits.get(accountId.toString()).get();
} catch (ExecutionException e) {
log.warn("Cannot get rate limits for account ''{}''", accountId, e);
}
} else {
try {
limiter = limitsPerRemoteHost.get(remoteHost).get();
+ globalLimiter = globalLimits.get(remoteHost).get();
} catch (ExecutionException e) {
log.warn(
"Cannot get rate limits for anonymous access from remote host ''{}''", remoteHost, e);
}
}
- if (limiter != null && !limiter.tryAcquire()) {
+ validateLimiter(limiter);
+ validateLimiter(globalLimiter);
+ }
+
+ private void validateLimiter(RateLimiter l) throws RateLimitException {
+ if (l != null && !l.tryAcquire()) {
throw new RateLimitException(
- MessageFormat.format(limitExceededMsg, limiter.getRate() * SECONDS_PER_HOUR));
+ MessageFormat.format(limitExceededMsg, l.getRate() * SECONDS_PER_HOUR));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiter.java
index 6f5c334..c87ad3e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiter.java
@@ -45,6 +45,7 @@
private final Provider<CurrentUser> user;
private final LoadingCache<Account.Id, Holder> limitsPerAccount;
+ private final LoadingCache<String, Holder> globalLimitsPerAccount;
private final LoadingCache<String, Holder> limitsPerRemoteHost;
private final Pattern servletPath =
@@ -58,13 +59,15 @@
RestApiRateLimiter(
Provider<CurrentUser> user,
@Named(HttpModule.CACHE_NAME_RESTAPI_ACCOUNTID)
- LoadingCache<Account.Id, Holder> limitsPerAccount,
+ LoadingCache<Account.Id, Holder> scopedLimitsPerAccount,
@Named(HttpModule.CACHE_NAME_RESTAPI_REMOTEHOST)
LoadingCache<String, Holder> limitsPerRemoteHost,
+ @Named(HttpModule.CACHE_NAME_GLOBAL) LoadingCache<String, Holder> globalLimitsPerAccount,
@Named(RateMsgHelper.RESTAPI_CONFIGURABLE_MSG_ANNOTATION) String limitExceededMsg) {
this.user = user;
- this.limitsPerAccount = limitsPerAccount;
+ this.limitsPerAccount = scopedLimitsPerAccount;
this.limitsPerRemoteHost = limitsPerRemoteHost;
+ this.globalLimitsPerAccount = globalLimitsPerAccount;
this.limitExceededMsg = limitExceededMsg;
}
@@ -73,41 +76,53 @@
throws IOException, ServletException {
if (isRest(req)) {
Holder rateLimiterHolder;
+ Holder globalRateLimiterHolder;
CurrentUser u = user.get();
if (u.isIdentifiedUser()) {
Account.Id accountId = u.asIdentifiedUser().getAccountId();
try {
rateLimiterHolder = limitsPerAccount.get(accountId);
+ globalRateLimiterHolder = globalLimitsPerAccount.get(accountId.toString());
} catch (ExecutionException e) {
rateLimiterHolder = Holder.EMPTY;
+ globalRateLimiterHolder = Holder.EMPTY;
log.warn("Cannot get rate limits for account ''{}''", accountId, e);
}
} else {
try {
rateLimiterHolder = limitsPerRemoteHost.get(req.getRemoteHost());
+ globalRateLimiterHolder = globalLimitsPerAccount.get(req.getRemoteHost());
} catch (ExecutionException e) {
rateLimiterHolder = Holder.EMPTY;
+ globalRateLimiterHolder = Holder.EMPTY;
log.warn(
"Cannot get rate limits for anonymous access from remote host ''{}''",
req.getRemoteHost(),
e);
}
}
- if (!rateLimiterHolder.hasGracePermits()
- && rateLimiterHolder.get() != null
- && !rateLimiterHolder.get().tryAcquire()) {
- String msg =
- MessageFormat.format(
- limitExceededMsg,
- rateLimiterHolder.get().getRate() * SECONDS_PER_HOUR,
- rateLimiterHolder.getBurstPermits());
- ((HttpServletResponse) res).sendError(SC_TOO_MANY_REQUESTS, msg);
+ if (!isAllowed(rateLimiterHolder, res) || !isAllowed(globalRateLimiterHolder, res)) {
return;
}
}
chain.doFilter(req, res);
}
+ private boolean isAllowed(Holder rateLimiterHolder, ServletResponse res) throws IOException {
+ if (!rateLimiterHolder.hasGracePermits()
+ && rateLimiterHolder.get() != null
+ && !rateLimiterHolder.get().tryAcquire()) {
+ String msg =
+ MessageFormat.format(
+ limitExceededMsg,
+ rateLimiterHolder.get().getRate() * SECONDS_PER_HOUR,
+ rateLimiterHolder.getBurstPermits());
+ ((HttpServletResponse) res).sendError(SC_TOO_MANY_REQUESTS, msg);
+ return false;
+ }
+ return true;
+ }
+
boolean isRest(ServletRequest req) {
return req instanceof HttpServletRequest
&& servletPath.matcher(((HttpServletRequest) req).getServletPath()).matches();
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 544df87..76a0491 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -122,7 +122,7 @@
the projects (including the namespaces that are already defined).
```
- [global-quota]
+ [global]
maxProjects = 500
maxRepoSize = 300 m
maxTotalSize = 200 m
@@ -180,6 +180,11 @@
Use group "Registered Users" to define the default rate limit for all logged
in users.
+Rate limits can also be specified in the global section; these limits are
+always applied, regardless of group matching. If a user matches a group, the
+limits from the first matching group are applied in addition to the global
+limits.
+
Format of the rate limit entries in `quota.config`:
```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListenerTest.java b/src/test/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListenerTest.java
index fc4dc76..2ff64c4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListenerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/quota/RateLimitUploadListenerTest.java
@@ -47,6 +47,7 @@
private RateLimitUploadListener uploadHook;
private LoadingCache<Account.Id, Holder> limitsPerAccount;
private LoadingCache<String, Holder> limitsPerRemoteHost;
+ private LoadingCache<String, Holder> globalLimits;
@Mock @GerritServerConfig Config cfg;
@Mock GenericFactory userFactory;
SystemGroupBackend systemGroupBackend;
@@ -67,6 +68,12 @@
CacheBuilder.newBuilder()
.build(new Module.HolderCacheLoaderByAccountId(Type.UPLOADPACK, userFactory, finder));
limitsPerAccount.put(accountId, holder);
+
+ globalLimits =
+ CacheBuilder.newBuilder()
+ .build(new Module.HolderCacheLoaderByGlobalAccount(Type.UPLOADPACK, finder));
+ globalLimits.put(accountId.toString(), holder);
+
limitsPerRemoteHost =
CacheBuilder.newBuilder()
.build(
@@ -76,7 +83,7 @@
uploadHook =
spy(
new RateLimitUploadListener(
- user, limitsPerAccount, limitsPerRemoteHost, LIMIT_EXCEEDED_MSG));
+ user, limitsPerAccount, limitsPerRemoteHost, globalLimits, LIMIT_EXCEEDED_MSG));
when(user.get()).thenReturn(currentUser);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiterTest.java
index 7dafad3..01afdbc 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/quota/RestApiRateLimiterTest.java
@@ -76,6 +76,7 @@
private RestApiRateLimiter restReqFilter;
private SystemGroupBackend systemGroupBackend;
private LoadingCache<Account.Id, Holder> limitsPerAccount;
+ private LoadingCache<String, Holder> globalLimitsPerAccount;
private LoadingCache<String, Holder> limitsPerRemoteHost;
@Before
@@ -87,6 +88,10 @@
.build(new Module.HolderCacheLoaderByAccountId(Type.UPLOADPACK, userFactory, finder));
limitsPerAccount.put(accountId, holder);
+ globalLimitsPerAccount =
+ CacheBuilder.newBuilder()
+ .build(new Module.HolderCacheLoaderByGlobalAccount(Type.UPLOADPACK, finder));
+
limitsPerRemoteHost =
CacheBuilder.newBuilder()
.build(
@@ -97,7 +102,11 @@
restReqFilter =
spy(
new RestApiRateLimiter(
- user, limitsPerAccount, limitsPerRemoteHost, LIMIT_EXCEEDED_MSG));
+ user,
+ limitsPerAccount,
+ globalLimitsPerAccount,
+ limitsPerRemoteHost,
+ LIMIT_EXCEEDED_MSG));
doReturn(true).when(restReqFilter).isRest(req);
when(user.get()).thenReturn(currentUser);
doNothing().when(chain).doFilter(req, res);
