diff --git a/changelog/10689.txt b/changelog/10689.txt new file mode 100644 index 000000000000..72433fc29bba --- /dev/null +++ b/changelog/10689.txt @@ -0,0 +1,3 @@ +```changelog:bug +quotas/rate: Fix quotas enforcing old rate limit quota paths +``` diff --git a/vault/logical_system_quotas.go b/vault/logical_system_quotas.go index 5907062d6d22..ef500a4efa8e 100644 --- a/vault/logical_system_quotas.go +++ b/vault/logical_system_quotas.go @@ -5,9 +5,10 @@ import ( "strings" "time" - "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" + + "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/vault/quotas" ) @@ -209,15 +210,18 @@ func (b *SystemBackend) handleRateLimitQuotasUpdate() framework.OperationFunc { switch { case quota == nil: - quota = quotas.NewRateLimitQuota(name, ns.Path, mountPath, rate, interval, blockInterval) default: rlq := quota.(*quotas.RateLimitQuota) + // Re-inserting the already indexed object in memdb might cause problems. + // So, clone the object. See https://github.com/hashicorp/go-memdb/issues/76. + rlq = rlq.Clone() rlq.NamespacePath = ns.Path rlq.MountPath = mountPath rlq.Rate = rate rlq.Interval = interval rlq.BlockInterval = blockInterval + quota = rlq } entry, err := logical.StorageEntryJSON(quotas.QuotaStoragePath(qType, name), quota) diff --git a/vault/quotas/quotas_rate_limit.go b/vault/quotas/quotas_rate_limit.go index 8a7532e4b376..847aa32c2ea1 100644 --- a/vault/quotas/quotas_rate_limit.go +++ b/vault/quotas/quotas_rate_limit.go @@ -100,6 +100,20 @@ func NewRateLimitQuota(name, nsPath, mountPath string, rate float64, interval, b } } +func (q *RateLimitQuota) Clone() *RateLimitQuota { + rlq := &RateLimitQuota{ + ID: q.ID, + Name: q.Name, + MountPath: q.MountPath, + Type: q.Type, + NamespacePath: q.NamespacePath, + BlockInterval: q.BlockInterval, + Rate: q.Rate, + Interval: q.Interval, + } + return rlq +} + // initialize ensures the namespace and max requests are initialized, sets the ID // if it's currently empty, sets the purge interval and stale age to default // values, and finally starts the client purge go routine if it has been started diff --git a/vault/quotas/quotas_test.go b/vault/quotas/quotas_test.go index 3f867ced719d..2995c602b850 100644 --- a/vault/quotas/quotas_test.go +++ b/vault/quotas/quotas_test.go @@ -12,6 +12,33 @@ import ( "github.com/stretchr/testify/require" ) +func TestQuotas_MountPathOverwrite(t *testing.T) { + qm, err := NewManager(logging.NewVaultLogger(log.Trace), nil, metricsutil.BlackholeSink()) + require.NoError(t, err) + + quota := NewRateLimitQuota("tq", "", "kv1/", 10, time.Second, 0) + require.NoError(t, qm.SetQuota(context.Background(), TypeRateLimit.String(), quota, false)) + quota = quota.Clone() + quota.MountPath = "kv2/" + require.NoError(t, qm.SetQuota(context.Background(), TypeRateLimit.String(), quota, false)) + + q, err := qm.QueryQuota(&Request{ + Type: TypeRateLimit, + MountPath: "kv1/", + }) + require.NoError(t, err) + require.Nil(t, q) + + require.NoError(t, qm.DeleteQuota(context.Background(), TypeRateLimit.String(), "tq")) + + q, err = qm.QueryQuota(&Request{ + Type: TypeRateLimit, + MountPath: "kv1/", + }) + require.NoError(t, err) + require.Nil(t, q) +} + func TestQuotas_Precedence(t *testing.T) { qm, err := NewManager(logging.NewVaultLogger(log.Trace), nil, metricsutil.BlackholeSink()) require.NoError(t, err)