Skip to content

Commit

Permalink
Fix quota conflict error (#10285)
Browse files Browse the repository at this point in the history
Co-authored-by: Scott Miller <[email protected]>
  • Loading branch information
vishalnayak and sgmiller committed Nov 6, 2020
1 parent 11c3095 commit 18ae233
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 10 deletions.
40 changes: 40 additions & 0 deletions vault/external_tests/quotas/quotas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,46 @@ func waitForRemovalOrTimeout(c *api.Client, path string, tick, to time.Duration)
}
}

func TestQuotas_RateLimit_DupFactors(t *testing.T) {
conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
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.Sys().Mount("pki", &api.MountInput{
Type: "pki",
})
if err != nil {
t.Fatal(err)
}

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

// Create a pki mount rlq
_, err = client.Logical().Write("sys/quotas/rate-limit/kv", map[string]interface{}{
"path": "pki",
"rate": 16.67,
})
require.NoError(t, err)

// Attempt to overwrite the pki mount rlq without the path param, making it the global quota
_, err = client.Logical().Write("sys/quotas/rate-limit/kv", map[string]interface{}{
"rate": 10,
})
require.Error(t, err)

// Successfully read the global quota without the conflicting quotas error
_, err = client.Logical().Read("sys/quotas/rate-limit/rlq")
require.NoError(t, err)
}

func TestQuotas_RateLimit_DupName(t *testing.T) {
conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
cluster := vault.NewTestCluster(t, conf, opts)
Expand Down
20 changes: 10 additions & 10 deletions vault/logical_system_quotas.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,16 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc {
}
}

// Disallow creation of new quota that has properties similar to an
// existing quota.
quotaByFactors, err := b.Core.quotaManager.QuotaByFactors(ctx, qType, ns.Path, mountPath)
if err != nil {
return nil, err
}
if quotaByFactors != nil && quotaByFactors.QuotaName() != name {
return logical.ErrorResponse("quota rule with similar properties exists under the name %q", quotaByFactors.QuotaName()), nil
}

// If a quota already exists, fetch and update it.
quota, err := b.Core.quotaManager.QuotaByName(qType, name)
if err != nil {
Expand All @@ -200,16 +210,6 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc {

switch {
case quota == nil:
// Disallow creation of new quota that has properties similar to an
// existing quota.
quotaByFactors, err := b.Core.quotaManager.QuotaByFactors(ctx, qType, ns.Path, mountPath)
if err != nil {
return nil, err
}
if quotaByFactors != nil && quotaByFactors.QuotaName() != name {
return logical.ErrorResponse("quota rule with similar properties exists under the name %q", quotaByFactors.QuotaName()), nil
}

quota = quotas.NewRateLimitQuota(name, ns.Path, mountPath, rate, interval, blockInterval)
default:
rlq := quota.(*quotas.RateLimitQuota)
Expand Down

0 comments on commit 18ae233

Please sign in to comment.