Backport 10285 10335 1.6.x (#10340)

* Fix quota conflict error (#10285) Co-authored-by: Scott Miller <[email protected]> * Backport last quota fix changes to OSS (#10335) * Backport last quota fix changes to OSS * Get all unit tests * dupe test Co-authored-by: Vishal Nayak <[email protected]>
hashicorp · Nov 6, 2020 · 82e4a89 · 82e4a89
1 parent 11c3095
commit 82e4a89
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 13 deletions.
diff --git a/vault/external_tests/quotas/quotas_test.go b/vault/external_tests/quotas/quotas_test.go
@@ -161,6 +161,45 @@ func TestQuotas_RateLimit_DupName(t *testing.T) {
 	require.Len(t, s.Data, 1, "incorrect number of quotas")
 }
 
+func TestQuotas_RateLimit_DupPath(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)
+	// create a global rate limit quota
+	_, err := client.Logical().Write("sys/quotas/rate-limit/global-rlq", map[string]interface{}{
+		"rate": 10,
+		"path": "",
+	})
+	require.NoError(t, err)
+
+	// create a rate limit quota w/ 'secret' path
+	_, err = client.Logical().Write("sys/quotas/rate-limit/secret-rlq", map[string]interface{}{
+		"rate": 7.7,
+		"path": "secret",
+	})
+	require.NoError(t, err)
+
+	s, err := client.Logical().Read("sys/quotas/rate-limit/secret-rlq")
+	require.NoError(t, err)
+	require.NotEmpty(t, s.Data)
+
+	// create a rate limit quota w/ empty path (same name)
+	_, err = client.Logical().Write("sys/quotas/rate-limit/secret-rlq", map[string]interface{}{
+		"rate": 7.7,
+		"path": "",
+	})
+
+	if err == nil {
+		t.Fatal("Duplicated paths were accepted")
+	}
+
+}
+
 func TestQuotas_RateLimitQuota_ExemptPaths(t *testing.T) {
 	conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil)
 

diff --git a/vault/logical_system_quotas.go b/vault/logical_system_quotas.go
@@ -191,6 +191,15 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc {
 				return logical.ErrorResponse("invalid mount path %q", mountPath), 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
+		}
 
 		// If a quota already exists, fetch and update it.
 		quota, err := b.Core.quotaManager.QuotaByName(qType, name)
@@ -200,15 +209,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:

diff --git a/vault/quotas/quotas.go b/vault/quotas/quotas.go
@@ -651,19 +651,27 @@ func (m *Manager) Reset() error {
 	m.lock.Lock()
 	defer m.lock.Unlock()
 
-	var err error
-	m.db, err = memdb.NewMemDB(dbSchema())
+	err := m.resetCache()
 	if err != nil {
 		return err
 	}
-
 	m.storage = nil
 	m.ctx = nil
 
 	return m.entManager.Reset()
 }
 
-// dbSchema creates a DB schema for holding all the quota rules. It creates a
+// Must be called with the lock held
+func (m *Manager) resetCache() error {
+	db, err := memdb.NewMemDB(dbSchema())
+	if err != nil {
+		return err
+	}
+	m.db = db
+	return nil
+}
+
+// dbSchema creates a DB schema for holding all the quota rules. It creates
 // table for each supported type of quota.
 func dbSchema() *memdb.DBSchema {
 	schema := &memdb.DBSchema{
@@ -867,6 +875,9 @@ func (m *Manager) Setup(ctx context.Context, storage logical.Storage, isPerfStan
 	m.setEnableRateLimitAuditLoggingLocked(config.EnableRateLimitAuditLogging)
 	m.setEnableRateLimitResponseHeadersLocked(config.EnableRateLimitResponseHeaders)
 	m.setRateLimitExemptPathsLocked(exemptPaths)
+	if err = m.resetCache(); err != nil {
+		return err
+	}
 
 	// Load the quota rules for all supported types from storage and load it in
 	// the quota manager.