opendev/gerrit
Code Issues Proposed changes

Add REST API quota checks

Browse Source 
This commit adds quota groups for all REST API calls. This enables
enforcer implementations to throttle specific calls.

Change-Id: Ideb182015519acecfbe304ef6a80ef38c854fb03
This commit is contained in:
Patrick Hiesel
2018-11-15 15:28:06 +01:00
parent fc9817f3f3
commit c0a28ced66
6 changed files with 187 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
Documentation/quota.txt
View File

@@ -14,8 +14,33 @@ associated.
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
------

5
Documentation/rest-api.txt
View File

@@ -191,6 +191,11 @@ Preconditions] section.
"`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

83
java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java Normal file
View File

@@ -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;
}
}

10
java/com/google/gerrit/httpd/restapi/RestApiServlet.java
View File

@@ -114,6 +114,7 @@ import com.google.gerrit.server.logging.TraceContext;
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 @@ public class RestApiServlet extends HttpServlet {
final AuditService auditService;
final RestApiMetrics metrics;
final Pattern allowOrigin;
final RestApiQuotaEnforcer quotaChecker;
@Inject
Globals(
@@ -236,6 +238,7 @@ public class RestApiServlet extends HttpServlet {
PermissionBackend permissionBackend,
AuditService auditService,
RestApiMetrics metrics,
RestApiQuotaEnforcer quotaChecker,
@GerritServerConfig Config cfg) {
this.currentUser = currentUser;
this.webSession = webSession;
@@ -243,6 +246,7 @@ public class RestApiServlet extends HttpServlet {
this.permissionBackend = permissionBackend;
this.auditService = auditService;
this.metrics = metrics;
this.quotaChecker = quotaChecker;
allowOrigin = makeAllowOrigin(cfg);
}
@@ -317,6 +321,7 @@ public class RestApiServlet extends HttpServlet {
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 @@ public class RestApiServlet extends HttpServlet {
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 @@ public class RestApiServlet extends HttpServlet {
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 @@ public class RestApiServlet extends HttpServlet {
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);

33
java/com/google/gerrit/util/http/RequestUtil.java
View File

@@ -56,5 +56,38 @@ public class RequestUtil {
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() {}
}

49
javatests/com/google/gerrit/util/http/RequestUtilTest.java
View File

@@ -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 @@ import org.junit.Test;
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(