Skip to content

Commit 5a968e2

Browse files
deansheatherdeansheather
authored
feat: add flag to disaable all rate limits (#5570)
1 parent ab7e676 commit 5a968e2

File tree

16 files changed

+290
-108
lines changed

16 files changed

+290
-108
lines changed

‎cli/deployment/config.go

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -429,11 +429,22 @@ func newConfig() *codersdk.DeploymentConfig {
429429
Default: 10 * time.Minute,
430430
},
431431
},
432-
APIRateLimit: &codersdk.DeploymentConfigField[int]{
433-
Name: "API Rate Limit",
434-
Usage: "Maximum number of requests per minute allowed to the API per user, or per IP address for unauthenticated users. Negative values mean no rate limit. Some API endpoints are always rate limited regardless of this value to prevent denial-of-service attacks.",
435-
Flag: "api-rate-limit",
436-
Default: 512,
432+
RateLimit: &codersdk.RateLimitConfig{
433+
DisableAll: &codersdk.DeploymentConfigField[bool]{
434+
Name: "Disable All Rate Limits",
435+
Usage: "Disables all rate limits. This is not recommended in production.",
436+
Flag: "dangerous-disable-rate-limits",
437+
Default: false,
438+
},
439+
API: &codersdk.DeploymentConfigField[int]{
440+
Name: "API Rate Limit",
441+
Usage: "Maximum number of requests per minute allowed to the API per user, or per IP address for unauthenticated users. Negative values mean no rate limit. Some API endpoints have separate strict rate limits regardless of this value to prevent denial-of-service or brute force attacks.",
442+
// Change the env from the auto-generated CODER_RATE_LIMIT_API to the
443+
// old value to avoid breaking existing deployments.
444+
EnvOverride: "CODER_API_RATE_LIMIT",
445+
Flag: "api-rate-limit",
446+
Default: 512,
447+
},
437448
},
438449
Experimental: &codersdk.DeploymentConfigField[bool]{
439450
Name: "Experimental",
@@ -498,21 +509,30 @@ func setConfig(prefix string, vip *viper.Viper, target interface{}) {
498509
// assigned a value.
499510
if strings.HasPrefix(typ.Name(), "DeploymentConfigField[") {
500511
value := val.FieldByName("Value").Interface()
512+
513+
env, ok := val.FieldByName("EnvOverride").Interface().(string)
514+
if !ok {
515+
panic("DeploymentConfigField[].EnvOverride must be a string")
516+
}
517+
if env == "" {
518+
env = formatEnv(prefix)
519+
}
520+
501521
switch value.(type) {
502522
case string:
503-
vip.MustBindEnv(prefix, formatEnv(prefix))
523+
vip.MustBindEnv(prefix, env)
504524
val.FieldByName("Value").SetString(vip.GetString(prefix))
505525
case bool:
506-
vip.MustBindEnv(prefix, formatEnv(prefix))
526+
vip.MustBindEnv(prefix, env)
507527
val.FieldByName("Value").SetBool(vip.GetBool(prefix))
508528
case int:
509-
vip.MustBindEnv(prefix, formatEnv(prefix))
529+
vip.MustBindEnv(prefix, env)
510530
val.FieldByName("Value").SetInt(int64(vip.GetInt(prefix)))
511531
case time.Duration:
512-
vip.MustBindEnv(prefix, formatEnv(prefix))
532+
vip.MustBindEnv(prefix, env)
513533
val.FieldByName("Value").SetInt(int64(vip.GetDuration(prefix)))
514534
case []string:
515-
vip.MustBindEnv(prefix, formatEnv(prefix))
535+
vip.MustBindEnv(prefix, env)
516536
// As of October 21st, 2022 we supported delimiting a string
517537
// with a comma, but Viper only supports with a space. This
518538
// is a small hack around it!
@@ -580,6 +600,9 @@ func readSliceFromViper[T any](vip *viper.Viper, key string, value any) []T {
580600

581601
// Ensure the env entry for this key is registered
582602
// before checking value.
603+
//
604+
// We don't support DeploymentConfigField[].EnvOverride for array flags so
605+
// this is fine to just use `formatEnv` here.
583606
vip.MustBindEnv(configKey, formatEnv(configKey))
584607

585608
value := vip.Get(configKey)
@@ -626,7 +649,7 @@ func setViperDefaults(prefix string, vip *viper.Viper, target interface{}) {
626649
val := reflect.ValueOf(target).Elem()
627650
val = reflect.Indirect(val)
628651
typ := val.Type()
629-
if strings.HasPrefix(typ.Name(), "DeploymentConfigField") {
652+
if strings.HasPrefix(typ.Name(), "DeploymentConfigField[") {
630653
value := val.FieldByName("Default").Interface()
631654
vip.SetDefault(prefix, value)
632655
return
@@ -663,7 +686,7 @@ func AttachFlags(flagset *pflag.FlagSet, vip *viper.Viper, enterprise bool) {
663686
func setFlags(prefix string, flagset *pflag.FlagSet, vip *viper.Viper, target interface{}, enterprise bool) {
664687
val := reflect.Indirect(reflect.ValueOf(target))
665688
typ := val.Type()
666-
if strings.HasPrefix(typ.Name(), "DeploymentConfigField") {
689+
if strings.HasPrefix(typ.Name(), "DeploymentConfigField[") {
667690
isEnt := val.FieldByName("Enterprise").Bool()
668691
if enterprise != isEnt {
669692
return
@@ -672,15 +695,24 @@ func setFlags(prefix string, flagset *pflag.FlagSet, vip *viper.Viper, target in
672695
if flg == "" {
673696
return
674697
}
698+
699+
env, ok := val.FieldByName("EnvOverride").Interface().(string)
700+
if !ok {
701+
panic("DeploymentConfigField[].EnvOverride must be a string")
702+
}
703+
if env == "" {
704+
env = formatEnv(prefix)
705+
}
706+
675707
usage := val.FieldByName("Usage").String()
676-
usage = fmt.Sprintf("%s\n%s", usage, cliui.Styles.Placeholder.Render("Consumes $"+formatEnv(prefix)))
708+
usage = fmt.Sprintf("%s\n%s", usage, cliui.Styles.Placeholder.Render("Consumes $"+env))
677709
shorthand := val.FieldByName("Shorthand").String()
678710
hidden := val.FieldByName("Hidden").Bool()
679711
value := val.FieldByName("Default").Interface()
680712

681713
// Allow currently set environment variables
682714
// to override default values in help output.
683-
vip.MustBindEnv(prefix, formatEnv(prefix))
715+
vip.MustBindEnv(prefix, env)
684716

685717
switch value.(type) {
686718
case string:

‎cli/scaletest.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func requireAdmin(ctx context.Context, client *codersdk.Client) (codersdk.User,
265265
// Only owners can do scaletests. This isn't a very strong check but there's
266266
// not much else we can do. Ratelimits are enforced for non-owners so
267267
// hopefully that limits the damage if someone disables this check and runs
268-
// it against a non-owner account.
268+
// it against a non-owner account on a production deployment.
269269
ok := false
270270
for _, role := range me.Roles {
271271
if role.Name == "owner" {
@@ -488,7 +488,9 @@ func scaletestCreateWorkspaces() *cobra.Command {
488488
cmd := &cobra.Command{
489489
Use: "create-workspaces",
490490
Short: "Creates many workspaces and waits for them to be ready",
491-
Long: "Creates many users, then creates a workspace for each user and waits for them finish building and fully come online. Optionally runs a command inside each workspace, and connects to the workspace over WireGuard.",
491+
Long: `Creates many users, then creates a workspace for each user and waits for them finish building and fully come online. Optionally runs a command inside each workspace, and connects to the workspace over WireGuard.
492+
493+
It is recommended that all rate limits are disabled on the server before running this scaletest. This test generates many login events which will be rate limited against the (most likely single) IP.`,
492494
RunE: func(cmd *cobra.Command, args []string) error {
493495
ctx := cmd.Context()
494496
client, err := CreateClient(cmd)

‎cli/server.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,16 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
104104
return xerrors.Errorf("either HTTP or TLS must be enabled")
105105
}
106106

107+
// Disable rate limits if the `--dangerous-disable-rate-limits` flag
108+
// was specified.
109+
loginRateLimit := 60
110+
filesRateLimit := 12
111+
if cfg.RateLimit.DisableAll.Value {
112+
cfg.RateLimit.API.Value = -1
113+
loginRateLimit = -1
114+
filesRateLimit = -1
115+
}
116+
107117
printLogo(cmd)
108118
logger := slog.Make(sloghuman.Sink(cmd.ErrOrStderr()))
109119
if ok, _ := cmd.Flags().GetBool(varVerbose); ok {
@@ -432,7 +442,9 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
432442
AgentStatsRefreshInterval: cfg.AgentStatRefreshInterval.Value,
433443
DeploymentConfig: cfg,
434444
PrometheusRegistry: prometheus.NewRegistry(),
435-
APIRateLimit: cfg.APIRateLimit.Value,
445+
APIRateLimit: cfg.RateLimit.API.Value,
446+
LoginRateLimit: loginRateLimit,
447+
FilesRateLimit: filesRateLimit,
436448
HTTPClient: httpClient,
437449
}
438450
if tlsConfig != nil {

‎cli/testdata/coder_scaletest_create-workspaces_--help.golden

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
Creates many users, then creates a workspace for each user and waits for them finish building and fully come online. Optionally runs a command inside each workspace, and connects to the workspace over WireGuard.
22

3+
It is recommended that all rate limits are disabled on the server before running this scaletest. This test generates many login events which will be rate limited against the (most likely single) IP.
4+
35
Usage:
46
coder scaletest create-workspaces [flags]
57

‎cli/testdata/coder_server_--help.golden

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,20 @@ Flags:
1818
allowed to the API per user, or per IP
1919
address for unauthenticated users.
2020
Negative values mean no rate limit. Some
21-
API endpoints are always rate limited
22-
regardless of this value to prevent
23-
denial-of-service attacks.
21+
API endpoints have separate strict rate
22+
limits regardless of this value to
23+
prevent denial-of-service or brute force
24+
attacks.
2425
Consumes $CODER_API_RATE_LIMIT (default 512)
2526
--cache-dir string The directory to cache temporary files.
2627
If unspecified and $CACHE_DIRECTORY is
2728
set, it will be used for compatibility
2829
with systemd.
2930
Consumes $CODER_CACHE_DIRECTORY (default
3031
"/tmp/coder-cli-test-cache")
32+
--dangerous-disable-rate-limits Disables all rate limits. This is not
33+
recommended in production.
34+
Consumes $CODER_RATE_LIMIT_DISABLE_ALL
3135
--derp-config-path string Path to read a DERP mapping from. See:
3236
https://tailscale.com/kb/1118/custom-derp-servers/
3337
Consumes $CODER_DERP_CONFIG_PATH

‎coderd/apidoc/docs.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2325,9 +2325,6 @@ const docTemplate = `{
23252325
"agent_stat_refresh_interval": {
23262326
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
23272327
},
2328-
"api_rate_limit": {
2329-
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
2330-
},
23312328
"audit_logging": {
23322329
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
23332330
},
@@ -2385,6 +2382,9 @@ const docTemplate = `{
23852382
"proxy_trusted_origins": {
23862383
"$ref": "#/definitions/codersdk.DeploymentConfigField-array_string"
23872384
},
2385+
"rate_limit": {
2386+
"$ref": "#/definitions/codersdk.RateLimitConfig"
2387+
},
23882388
"scim_api_key": {
23892389
"$ref": "#/definitions/codersdk.DeploymentConfigField-string"
23902390
},
@@ -2950,6 +2950,17 @@ const docTemplate = `{
29502950
}
29512951
}
29522952
},
2953+
"codersdk.RateLimitConfig": {
2954+
"type": "object",
2955+
"properties": {
2956+
"api": {
2957+
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
2958+
},
2959+
"disable_all": {
2960+
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
2961+
}
2962+
}
2963+
},
29532964
"codersdk.Response": {
29542965
"type": "object",
29552966
"properties": {

‎coderd/apidoc/swagger.json

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2058,9 +2058,6 @@
20582058
"agent_stat_refresh_interval": {
20592059
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
20602060
},
2061-
"api_rate_limit": {
2062-
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
2063-
},
20642061
"audit_logging": {
20652062
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
20662063
},
@@ -2118,6 +2115,9 @@
21182115
"proxy_trusted_origins": {
21192116
"$ref": "#/definitions/codersdk.DeploymentConfigField-array_string"
21202117
},
2118+
"rate_limit": {
2119+
"$ref": "#/definitions/codersdk.RateLimitConfig"
2120+
},
21212121
"scim_api_key": {
21222122
"$ref": "#/definitions/codersdk.DeploymentConfigField-string"
21232123
},
@@ -2662,6 +2662,17 @@
26622662
}
26632663
}
26642664
},
2665+
"codersdk.RateLimitConfig": {
2666+
"type": "object",
2667+
"properties": {
2668+
"api": {
2669+
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
2670+
},
2671+
"disable_all": {
2672+
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
2673+
}
2674+
}
2675+
},
26652676
"codersdk.Response": {
26662677
"type": "object",
26672678
"properties": {

0 commit comments

Comments
 (0)