Skip to content
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

feat: make all internal rate limits configurable #5095

Merged
merged 3 commits into from
Oct 26, 2023
Merged

feat: make all internal rate limits configurable #5095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63df6af
feat: All rate limits are now configurable
chriswk Oct 19, 2023
e02f683
docs: updated to include information that the rate limit is per ip
chriswk Oct 19, 2023
8f5843a
fix: updated with named object for labelnames
chriswk Oct 19, 2023
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
4 changes: 4 additions & 0 deletions src/lib/__snapshots__/create-config.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ exports[`should create default config 1`] = `
"preRouterHook": undefined,
"prometheusApi": undefined,
"publicFolder": undefined,
"rateLimiting": {
"createUserMaxPerMinute": 20,
"simpleLoginMaxPerMinute": 10,
},
"secureHeaders": false,
"segmentValuesLimit": 1000,
"server": {
Expand Down
21 changes: 21 additions & 0 deletions src/lib/create-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
ICspDomainOptions,
IClientCachingOption,
IMetricsRateLimiting,
IRateLimiting,
Copy link
Contributor

Choose a reason for hiding this comment

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

Irate limiting? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

indeed, I was quite IRate while writing this :P

} from './types/option';
import { getDefaultLogProvider, LogLevel, validateLogProvider } from './logger';
import { defaultCustomAuthDenyAll } from './default-custom-auth-deny-all';
Expand Down Expand Up @@ -132,6 +133,23 @@ function loadMetricsRateLimitingConfig(
]);
}

function loadRateLimitingConfig(options: IUnleashOptions): IRateLimiting {
const createUserMaxPerMinute = parseEnvVarNumber(
process.env.CREATE_USER_RATE_LIMIT_PER_MINUTE,
20,
);
const simpleLoginMaxPerMinute = parseEnvVarNumber(
process.env.SIMPLE_LOGIN_LIMIT_PER_MINUTE,
10,
Comment on lines +138 to +143
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious as to why one is called "x_RATE_LIMIT_PER_MINUTE" and the other is just called "x_LIMIT_PER_MINUTE"? Not a big deal, but feels like they should have similar names to make it easier to remember.

);

const defaultRateLimitOptions: IRateLimiting = {
createUserMaxPerMinute,
simpleLoginMaxPerMinute,
};
return mergeAll([defaultRateLimitOptions, options.rateLimiting || {}]);
}

function loadUI(options: IUnleashOptions): IUIConfig {
const uiO = options.ui || {};
const ui: IUIConfig = {
Expand Down Expand Up @@ -525,6 +543,8 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig {

const metricsRateLimiting = loadMetricsRateLimitingConfig(options);

const rateLimiting = loadRateLimitingConfig(options);

return {
db,
session,
Expand Down Expand Up @@ -559,6 +579,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig {
disableScheduler: options.disableScheduler,
isEnterprise: isEnterprise,
metricsRateLimiting,
rateLimiting,
};
}

Expand Down
42 changes: 42 additions & 0 deletions src/lib/metrics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ export default class MetricsMonitor {
labelNames: ['environment'],
});

const rateLimits = new client.Gauge({
name: 'rate_limits',
help: 'Rate limits (per minute) for METHOD/ENDPOINT pairs',
labelNames: ['endpoint', 'method'],
});

async function collectStaticCounters() {
try {
const stats = await instanceStatsService.getStats();
Expand Down Expand Up @@ -259,6 +265,42 @@ export default class MetricsMonitor {
.labels({ range: clientStat.range })
.set(clientStat.count),
);

rateLimits.reset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason we need to reset it here? Will we have gotten any inputs before this point that we need to get rid of?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still not sure, I just followed the patterns for the rest of our static metrics.

Copy link
Member Author

Choose a reason for hiding this comment

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

In my mind since it's a gauge there shouldn't really be a need to reset it.

rateLimits
.labels({ endpoint: '/api/client/metrics', method: 'POST' })
.set(config.metricsRateLimiting.clientMetricsMaxPerMinute);
rateLimits
.labels({
endpoint: '/api/client/register',
method: 'POST',
})
.set(config.metricsRateLimiting.clientRegisterMaxPerMinute);
rateLimits
.labels({
endpoint: '/api/frontend/metrics',
method: 'POST',
})
.set(
config.metricsRateLimiting.frontendMetricsMaxPerMinute,
);
rateLimits
.labels({
endpoint: '/api/frontend/register',
method: 'POST',
})
.set(
config.metricsRateLimiting.frontendRegisterMaxPerMinute,
);
Comment on lines +270 to +294
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity: what were all of these before? If these options are old, how were they applied? Did we do it somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

there were no limits for register and metrics earlier besides what's configured on the loadbalancer side.
For the user creation and login it was 10 and 20.

rateLimits
.labels({
endpoint: '/api/admin/user-admin',
method: 'POST',
})
.set(config.rateLimiting.createUserMaxPerMinute);
rateLimits
.labels({ endpoint: '/auth/simple', method: 'POST' })
.set(config.rateLimiting.simpleLoginMaxPerMinute);
} catch (e) {}
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/admin-api/user-admin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ export default class UserAdminController extends Controller {
}),
rateLimit({
windowMs: minutesToMilliseconds(1),
max: 20,
max: config.rateLimiting.createUserMaxPerMinute,
validate: false,
standardHeaders: true,
legacyHeaders: false,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class IndexRouter extends Controller {
new SimplePasswordProvider(config, services).router,
rateLimit({
windowMs: minutesToMilliseconds(1),
max: 10,
max: config.rateLimiting.simpleLoginMaxPerMinute,
validate: false,
standardHeaders: true,
legacyHeaders: false,
Expand Down
7 changes: 7 additions & 0 deletions src/lib/types/option.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export interface IUnleashOptions {
publicFolder?: string;
disableScheduler?: boolean;
metricsRateLimiting?: Partial<IMetricsRateLimiting>;
rateLimiting?: Partial<IRateLimiting>;
}

export interface IEmailOption {
Expand Down Expand Up @@ -193,6 +194,11 @@ export interface IMetricsRateLimiting {
frontendRegisterMaxPerMinute: number;
}

export interface IRateLimiting {
createUserMaxPerMinute: number;
simpleLoginMaxPerMinute: number;
}

export interface IUnleashConfig {
db: IDBOption;
session: ISessionOption;
Expand Down Expand Up @@ -227,4 +233,5 @@ export interface IUnleashConfig {
publicFolder?: string;
disableScheduler?: boolean;
isEnterprise: boolean;
rateLimiting: IRateLimiting;
}
11 changes: 7 additions & 4 deletions website/docs/reference/deploy/configuring-unleash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,13 @@ unleash.start(unleashOptions);
- **keepAliveTimeout** - Use this to tweak connection keepalive timeout in seconds. Useful for hosted situations where you need to make sure your connections are closed before terminating the instance. Defaults to `15`. Overridable with the `SERVER_KEEPALIVE_TIMEOUT` environment variable.
You can also set the environment variable `ENABLED_ENVIRONMENTS` to a comma delimited string of environment names to override environments.
- **metricsRateLimiting** - Use the following to tweak the rate limits for `/api/client/register`, `/api/client/metrics`, `/api/frontend/register` and `/api/frontend/metrics` POST endpoints
- `clientMetricsMaxPerMinute` - How many requests per minute is allowed against POST `/api/client/metrics` before returning 429. Set to 6000 by default (100rps) - Overridable with `REGISTER_CLIENT_RATE_LIMIT_PER_MINUTE` environment variable
- `clientRegisterMaxPerMinute` - How many requests per minute is allowed against POST `/api/client/register` before returning 429. Set to 6000 by default (100rps) - Overridable with `CLIENT_METRICS_RATE_LIMIT_PER_MINUTE` environment variable
- `frontendMetricsMaxPerMinute` - How many requests per minute is allowed against POST `/api/frontend/metrics` before returning 429. Set to 6000 by default (100rps) - Overridable with `FRONTEND_METRICS_RATE_LIMIT_PER_MINUTE` environment variable
- `frontendRegisterMaxPerMinute` - How many requests per minute is allowed against POST `/api/frontend/register` before returning 429. Set to 6000 by default (100rps) - Overridable with `REGISTER_FRONTEND_RATE_LIMIT_PER_MINUTE` environment variable
- `clientMetricsMaxPerMinute` - How many requests per minute per IP is allowed against POST `/api/client/metrics` before returning 429. Set to 6000 by default (100rps) - Overridable with `REGISTER_CLIENT_RATE_LIMIT_PER_MINUTE` environment variable
- `clientRegisterMaxPerMinute` - How many requests per minute per IP is allowed against POST `/api/client/register` before returning 429. Set to 6000 by default (100rps) - Overridable with `CLIENT_METRICS_RATE_LIMIT_PER_MINUTE` environment variable
- `frontendMetricsMaxPerMinute` - How many requests per minute per IP is allowed against POST `/api/frontend/metrics` before returning 429. Set to 6000 by default (100rps) - Overridable with `FRONTEND_METRICS_RATE_LIMIT_PER_MINUTE` environment variable
- `frontendRegisterMaxPerMinute` - How many requests per minute per IP is allowed against POST `/api/frontend/register` before returning 429. Set to 6000 by default (100rps) - Overridable with `REGISTER_FRONTEND_RATE_LIMIT_PER_MINUTE` environment variable
- **rateLimiting** - Use the following to tweak the rate limits for `POST /auth/simple` (Password-based login) and `POST /api/admin/user-admin` (Creating users)
- `simpleLoginMaxPerMinute` - How many requests per minute per IP is allowed against POST `/auth/simple` before returning 429. Set to 10 by default - Overridable with `SIMPLE_LOGIN_LIMIT_PER_MINUTE` environment variable
- `createUserMaxPerMinute` - How many requests per minute per IP is allowed against POST `/api/admin/user-admin` before returning 429. Set to 20 by default - Overridable with `CREATE_USER_RATE_LIMIT_PER_MINUTE` environment variable
### Disabling Auto-Start {#disabling-auto-start}

If you're using Unleash as part of a larger express app, you can disable the automatic server start by calling `server.create`. It takes the same options as `server.start`, but will not begin listening for connections.
Expand Down
Loading