Disable Request Limiter by default (#25442)

This PR flips the logic for the Request Limiter, setting it to default
disabled.

We allow users to turn on the global Request Limiter, but leave the
Listener configuration as a "disable per Listener".
This commit is contained in:
Mike Palmiotto
2024-02-16 17:50:18 -05:00
committed by GitHub
parent db957a50c5
commit 7ad778541e
6 changed files with 40 additions and 40 deletions

View File

@@ -1,3 +1,3 @@
```release-note:improvement ```release-note:improvement
limits: Introduce a reloadable disable configuration for the Request Limiter. limits: Introduce a reloadable opt-in configuration for the Request Limiter.
``` ```

View File

@@ -85,31 +85,6 @@ func TestServer_ReloadRequestLimiter(t *testing.T) {
configAfter string configAfter string
expectedResponse *vault.RequestLimiterResponse expectedResponse *vault.RequestLimiterResponse
}{ }{
{
"enable after default",
baseHCL + requestLimiterEnableHCL,
enabledResponse,
},
{
"enable after enable",
baseHCL + requestLimiterEnableHCL,
enabledResponse,
},
{
"disable after enable",
baseHCL + requestLimiterDisableHCL,
disabledResponse,
},
{
"default after disable",
baseHCL,
enabledResponse,
},
{
"default after default",
baseHCL,
enabledResponse,
},
{ {
"disable after default", "disable after default",
baseHCL + requestLimiterDisableHCL, baseHCL + requestLimiterDisableHCL,
@@ -120,6 +95,31 @@ func TestServer_ReloadRequestLimiter(t *testing.T) {
baseHCL + requestLimiterDisableHCL, baseHCL + requestLimiterDisableHCL,
disabledResponse, disabledResponse,
}, },
{
"enable after disable",
baseHCL + requestLimiterEnableHCL,
enabledResponse,
},
{
"default after enable",
baseHCL,
disabledResponse,
},
{
"default after default",
baseHCL,
disabledResponse,
},
{
"enable after default",
baseHCL + requestLimiterEnableHCL,
enabledResponse,
},
{
"enable after enable",
baseHCL + requestLimiterEnableHCL,
enabledResponse,
},
} }
ui, srv := command.TestServerCommand(t) ui, srv := command.TestServerCommand(t)
@@ -166,7 +166,7 @@ func TestServer_ReloadRequestLimiter(t *testing.T) {
cli.SetToken(initResp.RootToken) cli.SetToken(initResp.RootToken)
output = ui.ErrorWriter.String() + ui.OutputWriter.String() output = ui.ErrorWriter.String() + ui.OutputWriter.String()
require.Contains(t, output, "Request Limiter: enabled") require.Contains(t, output, "Request Limiter: disabled")
verifyLimiters := func(t *testing.T, expectedResponse *vault.RequestLimiterResponse) { verifyLimiters := func(t *testing.T, expectedResponse *vault.RequestLimiterResponse) {
t.Helper() t.Helper()
@@ -187,8 +187,8 @@ func TestServer_ReloadRequestLimiter(t *testing.T) {
require.Equal(t, expectedResponse, limiters) require.Equal(t, expectedResponse, limiters)
} }
// Start off with default enabled // Start off with default disabled
verifyLimiters(t, enabledResponse) verifyLimiters(t, disabledResponse)
for _, tc := range cases { for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {

View File

@@ -1441,9 +1441,9 @@ func (c *ServerCommand) Run(args []string) int {
info["administrative namespace"] = config.AdministrativeNamespacePath info["administrative namespace"] = config.AdministrativeNamespacePath
infoKeys = append(infoKeys, "request limiter") infoKeys = append(infoKeys, "request limiter")
info["request limiter"] = "enabled" info["request limiter"] = "disabled"
if config.RequestLimiter != nil && config.RequestLimiter.Disable { if config.RequestLimiter != nil && !config.RequestLimiter.Disable {
info["request limiter"] = "disabled" info["request limiter"] = "enabled"
} }
sort.Strings(infoKeys) sort.Strings(infoKeys)
@@ -3120,6 +3120,8 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.
if config.RequestLimiter != nil { if config.RequestLimiter != nil {
coreConfig.DisableRequestLimiter = config.RequestLimiter.Disable coreConfig.DisableRequestLimiter = config.RequestLimiter.Disable
} else {
coreConfig.DisableRequestLimiter = true
} }
if c.flagDev { if c.flagDev {

View File

@@ -112,7 +112,7 @@ request_limiter {
name: "invalid disable", name: "invalid disable",
inConfig: ` inConfig: `
request_limiter { request_limiter {
disable = "whywouldyoudothis" disable = "people make mistakes"
}`, }`,
outErr: true, outErr: true,
}, },

View File

@@ -28,7 +28,7 @@ func (r *RequestLimiter) GoString() string {
} }
var DefaultRequestLimiter = &RequestLimiter{ var DefaultRequestLimiter = &RequestLimiter{
Disable: false, Disable: true,
} }
func parseRequestLimiter(result *SharedConfig, list *ast.ObjectList) error { func parseRequestLimiter(result *SharedConfig, list *ast.ObjectList) error {
@@ -45,14 +45,13 @@ func parseRequestLimiter(result *SharedConfig, list *ast.ObjectList) error {
return multierror.Prefix(err, "request_limiter:") return multierror.Prefix(err, "request_limiter:")
} }
result.RequestLimiter.Disable = true
if result.RequestLimiter.DisableRaw != nil { if result.RequestLimiter.DisableRaw != nil {
var err error var err error
if result.RequestLimiter.Disable, err = parseutil.ParseBool(result.RequestLimiter.DisableRaw); err != nil { if result.RequestLimiter.Disable, err = parseutil.ParseBool(result.RequestLimiter.DisableRaw); err != nil {
return err return err
} }
result.RequestLimiter.DisableRaw = nil result.RequestLimiter.DisableRaw = nil
} else {
result.RequestLimiter.Disable = false
} }
return nil return nil

View File

@@ -1317,9 +1317,8 @@ func NewCore(conf *CoreConfig) (*Core, error) {
c.limiterRegistry = conf.LimiterRegistry c.limiterRegistry = conf.LimiterRegistry
c.limiterRegistryLock.Lock() c.limiterRegistryLock.Lock()
if conf.DisableRequestLimiter { c.limiterRegistry.Disable()
c.limiterRegistry.Disable() if !conf.DisableRequestLimiter {
} else {
c.limiterRegistry.Enable() c.limiterRegistry.Enable()
} }
c.limiterRegistryLock.Unlock() c.limiterRegistryLock.Unlock()
@@ -4112,7 +4111,7 @@ func (c *Core) ReloadRequestLimiter() {
return return
} }
disable := false disable := true
requestLimiterConfig := conf.(*server.Config).RequestLimiter requestLimiterConfig := conf.(*server.Config).RequestLimiter
if requestLimiterConfig != nil { if requestLimiterConfig != nil {
disable = requestLimiterConfig.Disable disable = requestLimiterConfig.Disable