From 519dbe913bf83d9bbae0fdff981b1be0b1bbc8e5 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Thu, 3 Mar 2022 12:50:54 +0000 Subject: [PATCH] Add rate limits for prebuilds Prebuilds are rate-limited to N in the last S seconds on a rolling window basis. By default, 50 prebuilds are allowed in a 1 minute window. A configuration property `prebuildLimiter` is added which controls default rate limit but allows for explicit overrides by cloneURL. --- chart/templates/server-configmap.yaml | 5 +- chart/values.yaml | 2 + .../typeorm/entity/db-prebuilt-workspace.ts | 4 +- ...PrebuildWorskace-rate-limiter-migration.ts | 24 +++++++ .../src/typeorm/workspace-db-impl.ts | 13 +++- .../gitpod-db/src/workspace-db.spec.db.ts | 61 +++++++++++++++- components/gitpod-db/src/workspace-db.ts | 1 + .../ee/src/prebuilds/prebuild-manager.ts | 72 +++++++++++++++---- .../swot-js/index.d.ts} | 0 components/server/src/config.ts | 6 ++ components/server/tsconfig.json | 8 ++- .../pkg/components/server/configmap.go | 4 ++ .../installer/pkg/components/server/types.go | 3 + 13 files changed, 180 insertions(+), 23 deletions(-) create mode 100644 components/gitpod-db/src/typeorm/migration/1646739309660-PrebuildWorskace-rate-limiter-migration.ts rename components/server/ee/src/{auth/swot-js.d.ts => typings/swot-js/index.d.ts} (100%) diff --git a/chart/templates/server-configmap.yaml b/chart/templates/server-configmap.yaml index 37fb867c6efda6..ab4596ed4b29a0 100644 --- a/chart/templates/server-configmap.yaml +++ b/chart/templates/server-configmap.yaml @@ -67,5 +67,6 @@ data: {{- end }} "enablePayment": {{ $comp.enablePayment }}, "insecureNoDomain": {{ $comp.insecureNoDomain }}, - "chargebeeProviderOptionsFile": {{ $comp.chargebeeProviderOptionsFile | quote }} - } \ No newline at end of file + "chargebeeProviderOptionsFile": {{ $comp.chargebeeProviderOptionsFile | quote }}, + "prebuildLimiter": {{ $comp.prebuildLimiter | toJson }} + } diff --git a/chart/values.yaml b/chart/values.yaml index e9f586c5206e48..e269794ebe965e 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -332,6 +332,8 @@ components: timeoutSeconds: 300 insecureNoDomain: false chargebeeProviderOptionsFile: "/chargebee/providerOptions" + prebuildLimiter: + '*': 50 serviceWaiter: imageName: "service-waiter" diff --git a/components/gitpod-db/src/typeorm/entity/db-prebuilt-workspace.ts b/components/gitpod-db/src/typeorm/entity/db-prebuilt-workspace.ts index ace8882c02c6af..c18f6e181929c0 100644 --- a/components/gitpod-db/src/typeorm/entity/db-prebuilt-workspace.ts +++ b/components/gitpod-db/src/typeorm/entity/db-prebuilt-workspace.ts @@ -9,9 +9,11 @@ import { PrimaryColumn, Column, Entity, Index } from "typeorm"; import { PrebuiltWorkspace, PrebuiltWorkspaceState } from "@gitpod/gitpod-protocol"; import { TypeORM } from "../typeorm"; import { Transformer } from "../transformer"; +import { PrebuildWorkspaceRateLimiterMigration1646739309660 } from "../migration/1646739309660-PrebuildWorskace-rate-limiter-migration"; @Entity() @Index("ind_ac4a9aece1a455da0dc653888f", ["cloneURL", "commit"]) +@Index(PrebuildWorkspaceRateLimiterMigration1646739309660.INDEX_NAME, PrebuildWorkspaceRateLimiterMigration1646739309660.FIELDS) // on DB but not Typeorm: @Index("ind_lastModified", ["_lastModified"]) // DBSync export class DBPrebuiltWorkspace implements PrebuiltWorkspace { @@ -64,4 +66,4 @@ export class DBPrebuiltWorkspace implements PrebuiltWorkspace { transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED }) error?: string; -} \ No newline at end of file +} diff --git a/components/gitpod-db/src/typeorm/migration/1646739309660-PrebuildWorskace-rate-limiter-migration.ts b/components/gitpod-db/src/typeorm/migration/1646739309660-PrebuildWorskace-rate-limiter-migration.ts new file mode 100644 index 00000000000000..e2a40bfc3abb74 --- /dev/null +++ b/components/gitpod-db/src/typeorm/migration/1646739309660-PrebuildWorskace-rate-limiter-migration.ts @@ -0,0 +1,24 @@ +/** + * Copyright (c) 2022 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License-AGPL.txt in the project root for license information. + */ + +import {MigrationInterface, QueryRunner} from "typeorm"; +import { indexExists } from "./helper/helper"; + +export class PrebuildWorkspaceRateLimiterMigration1646739309660 implements MigrationInterface { + + public static readonly TABLE_NAME = "d_b_prebuilt_workspace"; + public static readonly INDEX_NAME = "ind_prebuiltWorkspace_cloneURL_creationTime_state"; + public static readonly FIELDS = ["cloneURL", "creationTime", "state"]; + + public async up(queryRunner: QueryRunner): Promise { + if(!(await indexExists(queryRunner, PrebuildWorkspaceRateLimiterMigration1646739309660.TABLE_NAME, PrebuildWorkspaceRateLimiterMigration1646739309660.INDEX_NAME))) { + await queryRunner.query(`CREATE INDEX ${PrebuildWorkspaceRateLimiterMigration1646739309660.INDEX_NAME} ON ${PrebuildWorkspaceRateLimiterMigration1646739309660.TABLE_NAME} (${PrebuildWorkspaceRateLimiterMigration1646739309660.FIELDS.join(', ')})`); + } + } + + public async down(queryRunner: QueryRunner): Promise {} + +} diff --git a/components/gitpod-db/src/typeorm/workspace-db-impl.ts b/components/gitpod-db/src/typeorm/workspace-db-impl.ts index 7393df8075847f..779f887d73d539 100644 --- a/components/gitpod-db/src/typeorm/workspace-db-impl.ts +++ b/components/gitpod-db/src/typeorm/workspace-db-impl.ts @@ -7,7 +7,7 @@ import { injectable, inject } from "inversify"; import { Repository, EntityManager, DeepPartial, UpdateQueryBuilder, Brackets } from "typeorm"; import { MaybeWorkspace, MaybeWorkspaceInstance, WorkspaceDB, FindWorkspacesOptions, PrebuiltUpdatableAndWorkspace, WorkspaceInstanceSessionWithWorkspace, PrebuildWithWorkspace, WorkspaceAndOwner, WorkspacePortsAuthData, WorkspaceOwnerAndSoftDeleted } from "../workspace-db"; -import { Workspace, WorkspaceInstance, WorkspaceInfo, WorkspaceInstanceUser, WhitelistedRepository, Snapshot, LayoutData, PrebuiltWorkspace, RunningWorkspaceInfo, PrebuiltWorkspaceUpdatable, WorkspaceAndInstance, WorkspaceType, PrebuildInfo, AdminGetWorkspacesQuery, SnapshotState } from "@gitpod/gitpod-protocol"; +import { Workspace, WorkspaceInstance, WorkspaceInfo, WorkspaceInstanceUser, WhitelistedRepository, Snapshot, LayoutData, PrebuiltWorkspace, RunningWorkspaceInfo, PrebuiltWorkspaceUpdatable, WorkspaceAndInstance, WorkspaceType, PrebuildInfo, AdminGetWorkspacesQuery, SnapshotState, PrebuiltWorkspaceState } from "@gitpod/gitpod-protocol"; import { TypeORM } from "./typeorm"; import { DBWorkspace } from "./entity/db-workspace"; import { DBWorkspaceInstance } from "./entity/db-workspace-instance"; @@ -646,6 +646,17 @@ export abstract class AbstractTypeORMWorkspaceDBImpl implements WorkspaceDB { }); } + public async countUnabortedPrebuildsSince(cloneURL: string, date: Date): Promise { + const abortedState: PrebuiltWorkspaceState = 'aborted'; + const repo = await this.getPrebuiltWorkspaceRepo(); + + let query = repo.createQueryBuilder('pws'); + query = query.where('pws.cloneURL = :cloneURL', { cloneURL }) + query = query.andWhere('pws.creationTime >= :time', {time: date.toISOString()}) + query = query.andWhere('pws.state != :state', { state: abortedState }) + return query.getCount(); + } + public async findQueuedPrebuilds(cloneURL?: string): Promise { const repo = await this.getPrebuiltWorkspaceRepo(); diff --git a/components/gitpod-db/src/workspace-db.spec.db.ts b/components/gitpod-db/src/workspace-db.spec.db.ts index 79b4ec676c15ec..b6407c3a3b6a95 100644 --- a/components/gitpod-db/src/workspace-db.spec.db.ts +++ b/components/gitpod-db/src/workspace-db.spec.db.ts @@ -7,15 +7,16 @@ import * as chai from 'chai'; const expect = chai.expect; import { suite, test, timeout } from 'mocha-typescript'; +import { fail } from 'assert'; -import { WorkspaceInstance, Workspace } from '@gitpod/gitpod-protocol'; +import { WorkspaceInstance, Workspace, PrebuiltWorkspace } from '@gitpod/gitpod-protocol'; import { testContainer } from './test-container'; import { TypeORMWorkspaceDBImpl } from './typeorm/workspace-db-impl'; -import { fail } from 'assert'; import { TypeORM } from './typeorm/typeorm'; import { DBWorkspace } from './typeorm/entity/db-workspace'; import { DBPrebuiltWorkspace } from './typeorm/entity/db-prebuilt-workspace'; import { DBWorkspaceInstance } from './typeorm/entity/db-workspace-instance'; +import { secondsBefore } from '@gitpod/gitpod-protocol/lib/util/timeutil'; @suite class WorkspaceDBSpec { @@ -504,5 +505,61 @@ import { DBWorkspaceInstance } from './typeorm/entity/db-workspace-instance'; expect(dbResult[0].workspace.id).to.eq(this.ws2.id); expect(dbResult[1].workspace.id).to.eq(this.ws3.id); } + + @test(timeout(10000)) + public async testCountUnabortedPrebuildsSince() { + const now = new Date(); + const cloneURL = "https://github.com/gitpod-io/gitpod"; + + await Promise.all([ + // Created now, and queued + this.storePrebuiltWorkspace({ + id: 'prebuild123', + buildWorkspaceId: 'apples', + creationTime: now.toISOString(), + cloneURL: cloneURL, + commit: '', + state: 'queued' + }), + // now and aborted + this.storePrebuiltWorkspace({ + id: 'prebuild456', + buildWorkspaceId: 'bananas', + creationTime: now.toISOString(), + cloneURL: cloneURL, + commit: '', + state: 'aborted' + }), + // completed over a minute ago + this.storePrebuiltWorkspace({ + id: 'prebuild789', + buildWorkspaceId: 'oranges', + creationTime: secondsBefore(now.toISOString(), 62), + cloneURL: cloneURL, + commit: '', + state: 'available' + }), + ]); + + const minuteAgo = secondsBefore(now.toISOString(), 60); + const unabortedCount = await this.db.countUnabortedPrebuildsSince(cloneURL, new Date(minuteAgo)); + expect(unabortedCount).to.eq(1) + } + + private async storePrebuiltWorkspace(pws: PrebuiltWorkspace) { + // store the creationTime directly, before it is modified by the store function in the ORM layer + const creationTime = pws.creationTime + await this.db.storePrebuiltWorkspace(pws) + + const conn = await this.typeorm.getConnection(); + const repo = conn.getRepository(DBPrebuiltWorkspace); + + if (!!creationTime) { + // MySQL requires the time format to be 2022-03-07 15:44:01.746141 + // Looks almost like an ISO time string, hack it a bit. + const mysqlTimeFormat = creationTime.replace("T", " ").replace("Z", ""); + await repo.query("UPDATE d_b_prebuilt_workspace SET creationTime = ? WHERE id = ?", [mysqlTimeFormat, pws.id]); + } + } } module.exports = new WorkspaceDBSpec() diff --git a/components/gitpod-db/src/workspace-db.ts b/components/gitpod-db/src/workspace-db.ts index 3dca3d2cbb813f..1b38d542a9f12b 100644 --- a/components/gitpod-db/src/workspace-db.ts +++ b/components/gitpod-db/src/workspace-db.ts @@ -109,6 +109,7 @@ export interface WorkspaceDB { findPrebuildByWorkspaceID(wsid: string): Promise; findPrebuildByID(pwsid: string): Promise; countRunningPrebuilds(cloneURL: string): Promise; + countUnabortedPrebuildsSince(cloneURL: string, date: Date): Promise; findQueuedPrebuilds(cloneURL?: string): Promise; attachUpdatableToPrebuild(pwsid: string, update: PrebuiltWorkspaceUpdatable): Promise; findUpdatablesForPrebuild(pwsid: string): Promise; diff --git a/components/server/ee/src/prebuilds/prebuild-manager.ts b/components/server/ee/src/prebuilds/prebuild-manager.ts index 2ef2b4a67c5831..75dcd9f6f0cb7b 100644 --- a/components/server/ee/src/prebuilds/prebuild-manager.ts +++ b/components/server/ee/src/prebuilds/prebuild-manager.ts @@ -8,14 +8,17 @@ import { DBWithTracing, TracedWorkspaceDB, WorkspaceDB } from '@gitpod/gitpod-db import { CommitContext, Project, ProjectEnvVar, 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'; -import { URL } from 'url'; import { HostContextProvider } from '../../../src/auth/host-context-provider'; import { WorkspaceFactory } from '../../../src/workspace/workspace-factory'; 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 { secondsBefore } from '@gitpod/gitpod-protocol/lib/util/timeutil'; + +import { inject, injectable } from 'inversify'; +import * as opentracing from 'opentracing'; +import { URL } from 'url'; export class WorkspaceRunningError extends Error { constructor(msg: string, public instance: WorkspaceInstance) { @@ -32,6 +35,9 @@ export interface StartPrebuildParams { project?: Project; } +const PREBUILD_LIMITER_WINDOW_SECONDS = 60; +const PREBUILD_LIMITER_DEFAULT_LIMIT = 50; + @injectable() export class PrebuildManager { @inject(TracedWorkspaceDB) protected readonly workspaceDB: DBWithTracing; @@ -66,6 +72,7 @@ export class PrebuildManager { span.setTag("contextURL", contextURL); span.setTag("cloneURL", cloneURL); span.setTag("commit", commit); + try { if (user.blocked) { throw new Error("Blocked users cannot start prebuilds."); @@ -113,26 +120,32 @@ export class PrebuildManager { prebuildContext.commitHistory = await contextParser.fetchCommitHistory({ span }, user, contextURL, commit, maxDepth); } - log.debug("Created prebuild context", prebuildContext); + const projectEnvVarsPromise = project ? this.projectService.getProjectEnvironmentVariables(project.id) : []; const workspace = await this.workspaceFactory.createForContext({span}, user, prebuildContext, contextURL); - const prebuildPromise = this.workspaceDB.trace({span}).findPrebuildByWorkspaceID(workspace.id)!; + const prebuild = await this.workspaceDB.trace({span}).findPrebuildByWorkspaceID(workspace.id)!; + if (!prebuild) { + throw new Error(`Failed to create a prebuild for: ${contextURL}`); + } - // 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 };; - // } + if (await this.shouldRateLimitPrebuild(span, cloneURL)) { + prebuild.state = "aborted"; + prebuild.error = "Prebuild is rate limited. Please contact Gitpod if you believe this happened in error."; + + await this.workspaceDB.trace({ span }).storePrebuiltWorkspace(prebuild); + span.setTag("starting", false); + span.setTag("ratelimited", true); + return { + wsid: workspace.id, + prebuildId: prebuild.id, + done: false, + }; + } span.setTag("starting", true); const projectEnvVars = await projectEnvVarsPromise; await this.workspaceStarter.startWorkspace({ span }, workspace, user, [], projectEnvVars, {excludeFeatureFlags: ['full_workspace_backup']}); - const prebuild = await prebuildPromise; - if (!prebuild) { - throw new Error(`Failed to create a prebuild for: ${contextURL}`); - } return { prebuildId: prebuild.id, wsid: workspace.id, done: false }; } catch (err) { TraceContext.setError({ span }, err); @@ -228,4 +241,33 @@ export class PrebuildManager { } return hostContext.contextParser; } -} \ No newline at end of file + + private async shouldRateLimitPrebuild(span: opentracing.Span, cloneURL: string): Promise { + const windowStart = secondsBefore(new Date().toISOString(), PREBUILD_LIMITER_WINDOW_SECONDS); + const unabortedCount = await this.workspaceDB.trace({span}).countUnabortedPrebuildsSince(cloneURL, new Date(windowStart)); + const limit = this.getPrebuildRateLimitForCloneURL(cloneURL); + + if (unabortedCount >= limit) { + log.debug("Prebuild exceeds rate limit", { limit, unabortedPrebuildsCount: unabortedCount, cloneURL }); + return true; + } + return false; + } + + private getPrebuildRateLimitForCloneURL(cloneURL: string): number { + // First we use any explicit overrides for a given cloneURL + let limit = this.config.prebuildLimiter[cloneURL]; + if (limit > 0) { + return limit; + } + + // Find if there is a default value set under the '*' key + limit = this.config.prebuildLimiter['*']; + if (limit > 0) { + return limit; + } + + // Last resort default + return PREBUILD_LIMITER_DEFAULT_LIMIT; + } +} diff --git a/components/server/ee/src/auth/swot-js.d.ts b/components/server/ee/src/typings/swot-js/index.d.ts similarity index 100% rename from components/server/ee/src/auth/swot-js.d.ts rename to components/server/ee/src/typings/swot-js/index.d.ts diff --git a/components/server/src/config.ts b/components/server/src/config.ts index 3b5ece134dba26..2bf50da41014e6 100644 --- a/components/server/src/config.ts +++ b/components/server/src/config.ts @@ -150,6 +150,12 @@ export interface ConfigSerialized { */ chargebeeProviderOptionsFile?: string; enablePayment?: boolean; + + /** + * Number of prebuilds that can be started in the last 1 minute. + * Key '*' specifies the default rate limit for a cloneURL, unless overriden by a specific cloneURL. + */ + prebuildLimiter: {[cloneURL: string]: number } & {'*': number}; } export namespace ConfigFile { diff --git a/components/server/tsconfig.json b/components/server/tsconfig.json index 3b2130bd8948fd..5683469d10a616 100644 --- a/components/server/tsconfig.json +++ b/components/server/tsconfig.json @@ -20,10 +20,14 @@ "declaration": true, "declarationMap": true, "skipLibCheck": true, - "useUnknownInCatchVariables": false + "useUnknownInCatchVariables": false, + "typeRoots": [ + "../../node_modules/@types", + "./ee/src/typings" + ] }, "include": [ "src", "ee/src" ] -} \ No newline at end of file +} diff --git a/install/installer/pkg/components/server/configmap.go b/install/installer/pkg/components/server/configmap.go index 81734d56872cce..5be3dafa27316c 100644 --- a/install/installer/pkg/components/server/configmap.go +++ b/install/installer/pkg/components/server/configmap.go @@ -99,6 +99,10 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) { EnablePayment: false, InsecureNoDomain: false, ChargebeeProviderOptionsFile: "/chargebee/providerOptions", + PrebuildLimiter: map[string]int{ + // default limit for all cloneURLs + "*": 50, + }, } fc, err := common.ToJSONString(scfg) diff --git a/install/installer/pkg/components/server/types.go b/install/installer/pkg/components/server/types.go index 46237e329f5efb..8bcba3a17ffccc 100644 --- a/install/installer/pkg/components/server/types.go +++ b/install/installer/pkg/components/server/types.go @@ -45,6 +45,9 @@ type ConfigSerialized struct { OAuthServer OAuthServer `json:"oauthServer"` RateLimiter RateLimiter `json:"rateLimiter"` CodeSync CodeSync `json:"codeSync"` + // PrebuildLimiter defines the number of prebuilds allowed for each cloneURL in a given 1 minute interval + // Key of "*" defines the default limit, unless there exists a cloneURL in the map which overrides it. + PrebuildLimiter map[string]int `json:"prebuildLimiter"` } type CodeSyncResources struct {