Skip to content

Rate-limit new prebuilds to 50 / user / minute to prevent incidents #8504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Rate-limit new prebuilds to 50 / user / minute to prevent incidents #8504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcc70e3
[server] Rate-limit 'triggerPrebuild' method to 50/min
jankeromnes Mar 1, 2022
35e97c1
[server] Also rate-limit 'PrebuildManager.startPrebuild' (used by app…
jankeromnes Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions components/server/ee/src/prebuilds/prebuild-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { DBWithTracing, TracedWorkspaceDB, WorkspaceDB } from '@gitpod/gitpod-db/lib';
import { CommitContext, Project, ProjectEnvVar, StartPrebuildContext, StartPrebuildResult, TaskConfig, User, WorkspaceConfig, WorkspaceInstance } from '@gitpod/gitpod-protocol';
import { CommitContext, Project, ProjectEnvVar, RateLimiterError, StartPrebuildContext, StartPrebuildResult, TaskConfig, User, WorkspaceConfig, WorkspaceInstance } from '@gitpod/gitpod-protocol';
import { log } from '@gitpod/gitpod-protocol/lib/util/logging';
import { TraceContext } from '@gitpod/gitpod-protocol/lib/util/tracing';
import { inject, injectable } from 'inversify';
Expand All @@ -16,6 +16,9 @@ import { ConfigProvider } from '../../../src/workspace/config-provider';
import { WorkspaceStarter } from '../../../src/workspace/workspace-starter';
import { Config } from '../../../src/config';
import { ProjectsService } from '../../../src/projects/projects-service';
import { startPrebuild, UserRateLimiter } from '../../../src/auth/rate-limiter';
import { ErrorCodes } from '@gitpod/gitpod-protocol/lib/messaging/error';
import { ResponseError } from 'vscode-ws-jsonrpc';

export class WorkspaceRunningError extends Error {
constructor(msg: string, public instance: WorkspaceInstance) {
Expand Down Expand Up @@ -119,12 +122,18 @@ export class PrebuildManager {
const workspace = await this.workspaceFactory.createForContext({span}, user, prebuildContext, contextURL);
const prebuildPromise = this.workspaceDB.trace({span}).findPrebuildByWorkspaceID(workspace.id)!;

// const canBuildNow = await this.prebuildRateLimiter.canBuildNow({ span }, user, cloneURL);
// if (!canBuildNow) {
// // we cannot start building now because the rate limiter prevents it.
// span.setTag("starting", false);
// return { wsid: workspace.id, done: false };;
// }
// rate limiting
try {
await UserRateLimiter.instance(this.config.rateLimiter).consume(user.id, startPrebuild);
} catch (error) {
span.setTag("starting", false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefixing with the current span name, e.g. startPrebuild.starting is nice for discoverability in honeycomb.

if (error instanceof Error) {
log.error({ userId: user.id }, "Unexpected error in the rate limiter", error);
throw error;
}
log.warn({ userId: user.id }, "Rate limiter prevents accessing method due to too many requests.", error, { method: "startPrebuild" });
throw new ResponseError<RateLimiterError>(ErrorCodes.TOO_MANY_REQUESTS, "too many requests", { method: "startPrebuild", retryAfter: Math.ceil(error.msBeforeNext / 1000) || 1 });
}

span.setTag("starting", true);
const projectEnvVars = await projectEnvVarsPromise;
Expand Down
20 changes: 13 additions & 7 deletions components/server/src/auth/rate-limiter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import { RateLimiterMemory, RateLimiterRes } from "rate-limiter-flexible";

export const accessCodeSyncStorage = 'accessCodeSyncStorage';
export const accessHeadlessLogs = 'accessHeadlessLogs';
type GitpodServerMethodType = keyof Omit<GitpodServer, "dispose" | "setClient"> | typeof accessCodeSyncStorage | typeof accessHeadlessLogs;
type GroupKey = "default" | "startWorkspace";
export const startPrebuild = 'startPrebuild';
type GitpodServerMethodType = keyof Omit<GitpodServer, "dispose" | "setClient"> | typeof accessCodeSyncStorage | typeof accessHeadlessLogs | typeof startPrebuild;
type GroupKey = "default" | "startWorkspace" | "startPrebuild";
type GroupsConfig = {
[key: string]: {
points: number,
Expand Down Expand Up @@ -40,6 +41,10 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig {
points: 1, // 1 workspace start per user per 10s
durationsSec: 10
},
startPrebuild: {
points: 50, // 50 prebuild starts per user per minute
durationsSec: 60
},
}
const defaultFunctions: FunctionsConfig = {
"getLoggedInUser": { group: "default", points: 1 },
Expand Down Expand Up @@ -105,7 +110,7 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig {
"deleteProject": { group: "default", points: 1 },
"findPrebuilds": { group: "default", points: 1 },
"getProjectOverview": { group: "default", points: 1 },
"triggerPrebuild": { group: "default", points: 1 },
"triggerPrebuild": { group: "startPrebuild", points: 1 },
"cancelPrebuild": { group: "default", points: 1 },
"fetchProjectRepositoryConfiguration": { group: "default", points: 1 },
"guessProjectConfiguration": { group: "default", points: 1 },
Expand Down Expand Up @@ -157,10 +162,6 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig {
"getLicenseInfo": { group: "default", points: 1 },
"licenseIncludesFeature": { group: "default", points: 1 },

"accessCodeSyncStorage": { group: "default", points: 1 },

accessHeadlessLogs: { group: "default", points: 1 },

"adminAddStudentEmailDomain": { group: "default", points: 1 },
"adminGetAccountStatement": { group: "default", points: 1 },
"adminIsStudent": { group: "default", points: 1 },
Expand Down Expand Up @@ -193,6 +194,11 @@ function getConfig(config: RateLimiterConfig): RateLimiterConfig {
"trackLocation": { group: "default", points: 1},
"identifyUser": { group: "default", points: 1},
"getIDEOptions": { group: "default", points: 1 },

// Non-server methods
"accessCodeSyncStorage": { group: "default", points: 1 },
"accessHeadlessLogs": { group: "default", points: 1 },
"startPrebuild": { group: "startPrebuild", points: 1 },
};

return {
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/websocket/websocket-connection-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ class GitpodJsonRpcProxyFactory<T extends object> extends JsonRpcProxyFactory<T>
throw rlRejected;
}
log.warn({ userId }, "Rate limiter prevents accessing method due to too many requests.", rlRejected, { method });
throw new ResponseError<RateLimiterError>(ErrorCodes.TOO_MANY_REQUESTS, "too many requests", { method, retryAfter: Math.round(rlRejected.msBeforeNext / 1000) || 1 });
throw new ResponseError<RateLimiterError>(ErrorCodes.TOO_MANY_REQUESTS, "too many requests", { method, retryAfter: Math.ceil(rlRejected.msBeforeNext / 1000) || 1 });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

// access guard
Expand Down