diff --git a/vault/external_tests/quotas/quotas_test.go b/vault/external_tests/quotas/quotas_test.go index 696889045bae..4e8ee553845e 100644 --- a/vault/external_tests/quotas/quotas_test.go +++ b/vault/external_tests/quotas/quotas_test.go @@ -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) diff --git a/vault/logical_system_quotas.go b/vault/logical_system_quotas.go index d6fe22132c4f..29188ab2210a 100644 --- a/vault/logical_system_quotas.go +++ b/vault/logical_system_quotas.go @@ -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 { @@ -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)