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.

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

VAULT-8336 Fix default rate limit paths #18273

Merged
merged 2 commits into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
31 changes: 31 additions & 0 deletions vault/external_tests/quotas/quotas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,37 @@ func TestQuotas_RateLimitQuota_ExemptPaths(t *testing.T) {
require.Zero(t, numFail)
}

func TestQuotas_RateLimitQuota_DefaultExemptPaths(t *testing.T) {
conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
opts.NoDefaultQuotas = true

cluster := vault.NewTestCluster(t, conf, opts)
cluster.Start()
defer cluster.Cleanup()

core := cluster.Cores[0].Core
client := cluster.Cores[0].Client
vault.TestWaitActive(t, core)

_, err := client.Logical().Write("sys/quotas/rate-limit/rlq", map[string]interface{}{
"rate": 1,
})
require.NoError(t, err)

resp, err := client.Logical().Read("sys/health")
require.NoError(t, err)
require.NotNil(t, resp)
require.NotNil(t, resp.Data)

// The second sys/health call should not fail as /v1/sys/health is
// part of the default exempt paths
resp, err = client.Logical().Read("sys/health")
require.NoError(t, err)
// If the response is nil, then we are being rate limited
require.NotNil(t, resp)
require.NotNil(t, resp.Data)
}

func TestQuotas_RateLimitQuota_Mount(t *testing.T) {
conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
cluster := vault.NewTestCluster(t, conf, opts)
Expand Down
23 changes: 7 additions & 16 deletions vault/quotas/quotas.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,13 @@ var (
)

var defaultExemptPaths = []string{
"/v1/sys/generate-recovery-token/attempt",
"/v1/sys/generate-recovery-token/update",
"/v1/sys/generate-root/attempt",
"/v1/sys/generate-root/update",
"/v1/sys/health",
"/v1/sys/seal-status",
"/v1/sys/unseal",
"sys/generate-recovery-token/attempt",
"sys/generate-recovery-token/update",
"sys/generate-root/attempt",
"sys/generate-root/update",
"sys/health",
"sys/seal-status",
"sys/unseal",
}

// Access provides information to reach back to the quota checker.
Expand Down Expand Up @@ -724,15 +724,6 @@ func (m *Manager) RateLimitResponseHeadersEnabled() bool {
return m.config.EnableRateLimitResponseHeaders
}

// RateLimitExemptPaths returns the list of exempt paths from all rate limit
// resource quotas from the Manager's configuration.
func (m *Manager) RateLimitExemptPaths() []string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was never called (in OSS or Ent)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for clarifying!

m.quotaConfigLock.RLock()
defer m.quotaConfigLock.RUnlock()

return m.config.RateLimitExemptPaths
}

// RateLimitPathExempt returns a boolean dictating if a given path is exempt from
// any rate limit quota. If not rate limit path manager is defined, false is
// returned.
Expand Down
14 changes: 7 additions & 7 deletions website/content/docs/concepts/resource-quotas.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ By default, the following paths are exempt from rate limiting. However, Vault
operators can override the set of paths that are exempt from all rate limit
resource quotas by updating the `rate_limit_exempt_paths` configuration field.

- `/v1/sys/generate-recovery-token/attempt`
- `/v1/sys/generate-recovery-token/update`
- `/v1/sys/generate-root/attempt`
- `/v1/sys/generate-root/update`
- `/v1/sys/health`
- `/v1/sys/seal-status`
- `/v1/sys/unseal`
- `sys/generate-recovery-token/attempt`
- `sys/generate-recovery-token/update`
- `sys/generate-root/attempt`
- `sys/generate-root/update`
- `sys/health`
- `sys/seal-status`
- `sys/unseal`

## Tutorial

Expand Down