Add REST API quota checks
This commit adds quota groups for all REST API calls. This enables
enforcer implementations to throttle specific calls.
Change-Id: Ideb182015519acecfbe304ef6a80ef38c854fb03
diff --git a/Documentation/quota.txt b/Documentation/quota.txt
index 611bba8..a647e33 100644
--- a/Documentation/quota.txt
+++ b/Documentation/quota.txt
@@ -14,8 +14,33 @@
The following quota groups are defined in core Gerrit:
=== REST API
+[[rest-api]]
-TODO(hiesel,luca.milanesio,matthias.sohn,david.pursehouse): Add more :)
+The REST API enforces quota after the resource was parsed (if applicable) and before the
+endpoint's logic is executed. This enables quota enforcer implementations to throttle calls
+to specific endpoints while knowing the general context (user and top-level entity such as
+change, project or account).
+
+If the quota enforcer wants to throttle HTTP requests, they should use
+link:quota.html#http-requests[HTTP Requests] instead.
+
+The quota groups used for checking follow the exact definition of the endoint in the REST
+API, but remove all IDs. The schema is:
+
+/restapi/<ENDPOINT>:<HTTP-METHOD>
+
+Examples:
+
+[options="header",cols="1,6"]
+|=======================
+|HTTP call |Quota Group |Metadata
+|GET /a/changes/1/revisions/current/detail |/changes/revisions/detail:GET |CurrentUser, Change.Id, Project.NameKey
+|POST /a/changes/ |/changes/:POST |CurrentUser
+|GET /a/accounts/self/detail |/accounts/detail:GET |CurrentUser, Account.Id
+|=======================
+
+The user provided in the check's metadata is always the calling user (having the
+impersonation bit and real user set in case the user is impersonating another user).
GERRIT
------
diff --git a/Documentation/rest-api.txt b/Documentation/rest-api.txt
index 8f6a47b..a8ab353 100644
--- a/Documentation/rest-api.txt
+++ b/Documentation/rest-api.txt
@@ -191,6 +191,11 @@
"`422 Unprocessable Entity`" is returned if the ID of a resource that is
specified in the request body cannot be resolved.
+==== 429 Too Many Requests
+"`429 Too Many Requests`" is returned if the request exhausted any set
+quota limits. Depending on the exhausted quota, the request may be retried
+with exponential backoff.
+
[[tracing]]
=== Request Tracing
For each REST endpoint tracing can be enabled by setting the
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java
new file mode 100644
index 0000000..c7678c2
--- /dev/null
+++ b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java
@@ -0,0 +1,83 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// 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 com.google.gerrit.httpd.restapi;
+
+import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.quota.QuotaBackend;
+import com.google.gerrit.server.quota.QuotaException;
+import com.google.gerrit.util.http.RequestUtil;
+import javax.inject.Inject;
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Enforces quota on specific REST API endpoints.
+ *
+ * <p>Examples:
+ *
+ * <ul>
+ * <li>GET /a/accounts/self/detail => /restapi/accounts/detail:GET
+ * <li>GET /changes/123/revisions/current/detail => /restapi/changes/revisions/detail:GET
+ * <li>PUT /changes/10/reviewed => /changes/reviewed:PUT
+ * </ul>
+ *
+ * <p>Adds context (change, project, account) to the quota check if the call is for an existing
+ * entity that was successfully parsed. This quota check is generally enforced after the resource
+ * was parsed, but before the view is executed. If a quota enforcer desires to throttle earlier,
+ * they should consider quota groups in the {@code /http/*} space.
+ */
+public class RestApiQuotaEnforcer {
+ private final QuotaBackend quotaBackend;
+
+ @Inject
+ RestApiQuotaEnforcer(QuotaBackend quotaBackend) {
+ this.quotaBackend = quotaBackend;
+ }
+
+ /** Enforce quota on a request not tied to any {@code RestResource}. */
+ void enforce(HttpServletRequest req) throws QuotaException {
+ String pathForQuotaReporting = RequestUtil.getRestPathWithoutIds(req);
+ quotaBackend
+ .currentUser()
+ .requestToken(quotaGroup(pathForQuotaReporting, req.getMethod()))
+ .throwOnError();
+ }
+
+ /** Enforce quota on a request for a given resource. */
+ void enforce(RestResource rsrc, HttpServletRequest req) throws QuotaException {
+ String pathForQuotaReporting = RequestUtil.getRestPathWithoutIds(req);
+ // Enrich the quota request we are operating on an interesting collection
+ QuotaBackend.WithResource report = quotaBackend.currentUser();
+ if (rsrc instanceof ChangeResource) {
+ ChangeResource changeResource = (ChangeResource) rsrc;
+ report =
+ quotaBackend.currentUser().change(changeResource.getId(), changeResource.getProject());
+ } else if (rsrc instanceof AccountResource) {
+ AccountResource accountResource = (AccountResource) rsrc;
+ report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId());
+ } else if (rsrc instanceof ProjectResource) {
+ ProjectResource accountResource = (ProjectResource) rsrc;
+ report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId());
+ }
+
+ report.requestToken(quotaGroup(pathForQuotaReporting, req.getMethod())).throwOnError();
+ }
+
+ private static String quotaGroup(String path, String method) {
+ return "/restapi" + path + ":" + method;
+ }
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index ec71d8f..c9f3a77 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -114,6 +114,7 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.quota.QuotaException;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.util.http.CacheHeaders;
@@ -227,6 +228,7 @@
final AuditService auditService;
final RestApiMetrics metrics;
final Pattern allowOrigin;
+ final RestApiQuotaEnforcer quotaChecker;
@Inject
Globals(
@@ -236,6 +238,7 @@
PermissionBackend permissionBackend,
AuditService auditService,
RestApiMetrics metrics,
+ RestApiQuotaEnforcer quotaChecker,
@GerritServerConfig Config cfg) {
this.currentUser = currentUser;
this.webSession = webSession;
@@ -243,6 +246,7 @@
this.permissionBackend = permissionBackend;
this.auditService = auditService;
this.metrics = metrics;
+ this.quotaChecker = quotaChecker;
allowOrigin = makeAllowOrigin(cfg);
}
@@ -317,6 +321,7 @@
viewData = new ViewData(null, null);
if (path.isEmpty()) {
+ globals.quotaChecker.enforce(req);
if (rc instanceof NeedsParams) {
((NeedsParams) rc).setParams(qp.params());
}
@@ -339,6 +344,7 @@
IdString id = path.remove(0);
try {
rsrc = rc.parse(rsrc, id);
+ globals.quotaChecker.enforce(rsrc, req);
if (path.isEmpty()) {
checkPreconditions(req);
}
@@ -346,6 +352,7 @@
if (!path.isEmpty()) {
throw e;
}
+ globals.quotaChecker.enforce(req);
if (isPost(req) || isPut(req)) {
RestView<RestResource> createView = rc.views().get(PluginName.GERRIT, "CREATE./");
@@ -602,6 +609,9 @@
status = SC_INTERNAL_SERVER_ERROR;
responseBytes = handleException(e, req, res);
}
+ } catch (QuotaException e) {
+ responseBytes =
+ replyError(req, res, status = 429, messageOr(e, "Quota limit reached"), e.caching(), e);
} catch (Exception e) {
status = SC_INTERNAL_SERVER_ERROR;
responseBytes = handleException(e, req, res);
diff --git a/java/com/google/gerrit/util/http/RequestUtil.java b/java/com/google/gerrit/util/http/RequestUtil.java
index 2a359ca..2543ada 100644
--- a/java/com/google/gerrit/util/http/RequestUtil.java
+++ b/java/com/google/gerrit/util/http/RequestUtil.java
@@ -56,5 +56,38 @@
return pathInfo;
}
+ /**
+ * Trims leading '/' and 'a/'. Removes the context path, but keeps the servlet path. Removes all
+ * IDs from the rest of the URI.
+ *
+ * <p>The returned string is a good fit for cases where one wants the full context of the request
+ * without any identifiable data. For example: Logging or quota checks.
+ *
+ * <p>Examples:
+ *
+ * <ul>
+ * <li>/a/accounts/self/detail => /accounts/detail
+ * <li>/changes/123/revisions/current/detail => /changes/revisions/detail
+ * <li>/changes/ => /changes
+ * </ul>
+ */
+ public static String getRestPathWithoutIds(HttpServletRequest req) {
+ String encodedPathInfo = req.getRequestURI().substring(req.getContextPath().length());
+ if (encodedPathInfo.startsWith("/")) {
+ encodedPathInfo = encodedPathInfo.substring(1);
+ }
+ if (encodedPathInfo.startsWith("a/")) {
+ encodedPathInfo = encodedPathInfo.substring(2);
+ }
+
+ String[] parts = encodedPathInfo.split("/");
+ StringBuilder result = new StringBuilder(parts.length);
+ for (int i = 0; i < parts.length; i = i + 2) {
+ result.append("/");
+ result.append(parts[i]);
+ }
+ return result.toString();
+ }
+
private RequestUtil() {}
}
diff --git a/javatests/com/google/gerrit/util/http/RequestUtilTest.java b/javatests/com/google/gerrit/util/http/RequestUtilTest.java
index 0bf34e7..adda5e7 100644
--- a/javatests/com/google/gerrit/util/http/RequestUtilTest.java
+++ b/javatests/com/google/gerrit/util/http/RequestUtilTest.java
@@ -15,6 +15,8 @@
package com.google.gerrit.util.http;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.util.http.RequestUtil.getEncodedPathInfo;
+import static com.google.gerrit.util.http.RequestUtil.getRestPathWithoutIds;
import com.google.gerrit.testing.GerritBaseTests;
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
@@ -22,36 +24,45 @@
public class RequestUtilTest extends GerritBaseTests {
@Test
- public void emptyContextPath() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/s", "/foo/bar")))
- .isEqualTo("/foo/bar");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/s", "/foo%2Fbar")))
- .isEqualTo("/foo%2Fbar");
+ public void getEncodedPathInfo_emptyContextPath() {
+ assertThat(getEncodedPathInfo(fakeRequest("", "/s", "/foo/bar"))).isEqualTo("/foo/bar");
+ assertThat(getEncodedPathInfo(fakeRequest("", "/s", "/foo%2Fbar"))).isEqualTo("/foo%2Fbar");
}
@Test
- public void emptyServletPath() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/c", "/foo/bar")))
- .isEqualTo("/foo/bar");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/c", "/foo%2Fbar")))
- .isEqualTo("/foo%2Fbar");
+ public void getEncodedPathInfo_emptyServletPath() {
+ assertThat(getEncodedPathInfo(fakeRequest("", "/c", "/foo/bar"))).isEqualTo("/foo/bar");
+ assertThat(getEncodedPathInfo(fakeRequest("", "/c", "/foo%2Fbar"))).isEqualTo("/foo%2Fbar");
}
@Test
- public void trailingSlashes() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar/")))
- .isEqualTo("/foo/bar/");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar///")))
- .isEqualTo("/foo/bar/");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar/")))
- .isEqualTo("/foo%2Fbar/");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar///")))
+ public void getEncodedPathInfo_trailingSlashes() {
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar/"))).isEqualTo("/foo/bar/");
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar///"))).isEqualTo("/foo/bar/");
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar/"))).isEqualTo("/foo%2Fbar/");
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar///")))
.isEqualTo("/foo%2Fbar/");
}
@Test
public void emptyPathInfo() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", ""))).isNull();
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", ""))).isNull();
+ }
+
+ @Test
+ public void getRestPathWithoutIds_emptyContextPath() {
+ assertThat(getRestPathWithoutIds(fakeRequest("", "/a/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
+ assertThat(getRestPathWithoutIds(fakeRequest("", "/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
+ }
+
+ @Test
+ public void getRestPathWithoutIds_nonEmptyContextPath() {
+ assertThat(getRestPathWithoutIds(fakeRequest("/c", "/a/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
+ assertThat(getRestPathWithoutIds(fakeRequest("/c", "/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
}
private FakeHttpServletRequest fakeRequest(
