From 3ba0eb507469fc578386c979f0912d91bc4b55e7 Mon Sep 17 00:00:00 2001 From: Dhia Ayachi Date: Wed, 22 Mar 2023 15:19:54 -0400 Subject: [PATCH] delete config when nil (#16690) * delete config when nil * fix mock interface implementation * fix handler test to use the right assertion * extract DeleteConfig as a separate API. * fix mock limiter implementation to satisfy the new interface * fix failing tests * add test comments --- agent/consul/multilimiter/mock_RateLimiter.go | 7 +- agent/consul/multilimiter/multilimiter.go | 52 +++-- .../consul/multilimiter/multilimiter_test.go | 192 ++++++++++++++---- agent/consul/rate/handler.go | 2 +- agent/consul/rate/handler_test.go | 3 + 5 files changed, 202 insertions(+), 54 deletions(-) diff --git a/agent/consul/multilimiter/mock_RateLimiter.go b/agent/consul/multilimiter/mock_RateLimiter.go index d36c2e161e..0c33f92bfa 100644 --- a/agent/consul/multilimiter/mock_RateLimiter.go +++ b/agent/consul/multilimiter/mock_RateLimiter.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.15.0. DO NOT EDIT. +// Code generated by mockery v2.20.0. DO NOT EDIT. package multilimiter @@ -27,6 +27,11 @@ func (_m *MockRateLimiter) Allow(entity LimitedEntity) bool { return r0 } +// DeleteConfig provides a mock function with given fields: prefix +func (_m *MockRateLimiter) DeleteConfig(prefix []byte) { + _m.Called(prefix) +} + // Run provides a mock function with given fields: ctx func (_m *MockRateLimiter) Run(ctx context.Context) { _m.Called(ctx) diff --git a/agent/consul/multilimiter/multilimiter.go b/agent/consul/multilimiter/multilimiter.go index 21839d3fe0..13e01f8eba 100644 --- a/agent/consul/multilimiter/multilimiter.go +++ b/agent/consul/multilimiter/multilimiter.go @@ -26,6 +26,7 @@ type RateLimiter interface { Run(ctx context.Context) Allow(entity LimitedEntity) bool UpdateConfig(c LimiterConfig, prefix []byte) + DeleteConfig(prefix []byte) } type limiterWithKey struct { @@ -67,7 +68,6 @@ type LimiterConfig struct { // Config is a MultiLimiter configuration type Config struct { - LimiterConfig ReconcileCheckLimit time.Duration ReconcileCheckInterval time.Duration } @@ -75,14 +75,13 @@ type Config struct { // UpdateConfig will update the MultiLimiter Config // which will cascade to all the Limiter(s) LimiterConfig func (m *MultiLimiter) UpdateConfig(c LimiterConfig, prefix []byte) { - m.configsLock.Lock() - defer m.configsLock.Unlock() - if prefix == nil { - prefix = []byte("") - } - configs := m.limitersConfigs.Load() - newConfigs, _, _ := configs.Insert(prefix, &c) - m.limitersConfigs.Store(newConfigs) + m.updateConfig(&c, prefix) +} + +// DeleteConfig will delete the MultiLimiter Config +// which will cascade to all the Limiter(s) LimiterConfig +func (m *MultiLimiter) DeleteConfig(prefix []byte) { + m.updateConfig(nil, prefix) } // NewMultiLimiter create a new MultiLimiter @@ -143,14 +142,16 @@ func (m *MultiLimiter) Allow(e LimitedEntity) bool { } configs := m.limitersConfigs.Load() - config := &m.defaultConfig.Load().LimiterConfig + p, _, ok := configs.Root().LongestPrefix(e.Key()) - if ok { - c, ok := configs.Get(p) - if ok && c != nil { - config = c.(*LimiterConfig) - } + if !ok { + return true + } + var config *LimiterConfig + c, ok := configs.Get(p) + if ok && c != nil { + config = c.(*LimiterConfig) } limiter := &Limiter{limiter: rate.NewLimiter(config.Rate, config.Burst)} @@ -159,6 +160,24 @@ func (m *MultiLimiter) Allow(e LimitedEntity) bool { return limiter.limiter.Allow() } +// UpdateConfig will update the MultiLimiter Config +// which will cascade to all the Limiter(s) LimiterConfig +func (m *MultiLimiter) updateConfig(c *LimiterConfig, prefix []byte) { + m.configsLock.Lock() + defer m.configsLock.Unlock() + if prefix == nil { + prefix = []byte("") + } + configs := m.limitersConfigs.Load() + var newConfigs *radix.Tree + if c == nil { + newConfigs, _, _ = configs.Delete(prefix) + } else { + newConfigs, _, _ = configs.Insert(prefix, c) + } + m.limitersConfigs.Store(newConfigs) +} + type ticker interface { Ticker() <-chan time.Time } @@ -211,7 +230,10 @@ func (m *MultiLimiter) reconcileConfig(txn *radix.Txn) { // find the prefix for the leaf and check if the defaultConfig is up-to-date // it's possible that the prefix is equal to the key p, _, ok := m.limitersConfigs.Load().Root().LongestPrefix(k) + + // no corresponding config found, need to delete it if !ok { + txn.Delete(k) continue } v, ok := m.limitersConfigs.Load().Get(p) diff --git a/agent/consul/multilimiter/multilimiter_test.go b/agent/consul/multilimiter/multilimiter_test.go index 2b04f29eef..c34440ff40 100644 --- a/agent/consul/multilimiter/multilimiter_test.go +++ b/agent/consul/multilimiter/multilimiter_test.go @@ -5,14 +5,15 @@ import ( "context" "encoding/binary" "fmt" - "github.com/hashicorp/consul/sdk/testutil/retry" - radix "github.com/hashicorp/go-immutable-radix" - "github.com/stretchr/testify/require" - "golang.org/x/time/rate" "math/rand" "sync" "testing" "time" + + "github.com/hashicorp/consul/sdk/testutil/retry" + radix "github.com/hashicorp/go-immutable-radix" + "github.com/stretchr/testify/require" + "golang.org/x/time/rate" ) type Limited struct { @@ -24,17 +25,19 @@ func (l Limited) Key() []byte { } func TestNewMultiLimiter(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}} + c := Config{} m := NewMultiLimiter(c) require.NotNil(t, m) require.NotNil(t, m.limiters) } func TestRateLimiterUpdate(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 1 * time.Hour, ReconcileCheckInterval: 10 * time.Millisecond} + c := Config{ReconcileCheckLimit: 1 * time.Hour, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) key := Key([]byte("test")) + c1 := LimiterConfig{Rate: 10} + m.UpdateConfig(c1, key) //Allow a key m.Allow(Limited{key: key}) storeLimiter(m) @@ -71,21 +74,22 @@ func TestRateLimiterUpdate(t *testing.T) { func TestRateLimiterCleanup(t *testing.T) { // Create a limiter and Allow a key, check the key exists - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 1 * time.Second, ReconcileCheckInterval: 10 * time.Millisecond} + c := Config{ReconcileCheckLimit: 1 * time.Second, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) limiters := m.limiters.Load() ctx, cancel := context.WithCancel(context.Background()) defer cancel() m.Run(ctx) key := Key([]byte("test")) + m.UpdateConfig(LimiterConfig{Rate: 0.1}, key) m.Allow(Limited{key: key}) - retry.RunWith(&retry.Timer{Wait: 100 * time.Millisecond, Timeout: 2 * time.Second}, t, func(r *retry.R) { + retry.RunWith(&retry.Timer{Wait: 100 * time.Millisecond, Timeout: 10 * time.Second}, t, func(r *retry.R) { l := m.limiters.Load() require.NotEqual(r, limiters, l) limiters = l }) - retry.RunWith(&retry.Timer{Wait: 100 * time.Millisecond, Timeout: 2 * time.Second}, t, func(r *retry.R) { + retry.RunWith(&retry.Timer{Wait: 100 * time.Millisecond, Timeout: 10 * time.Second}, t, func(r *retry.R) { v, ok := limiters.Get(key) require.True(r, ok) require.NotNil(t, v) @@ -120,79 +124,104 @@ func reconcile(m *MultiLimiter) { ctx := context.Background() reconcileCheckLimit := m.defaultConfig.Load().ReconcileCheckLimit mockTicker.tickerCh <- time.Now() - m.reconcile(ctx, &mockTicker, txn, reconcileCheckLimit) + txn = m.reconcile(ctx, &mockTicker, txn, reconcileCheckLimit) + m.limiters.Store(txn.Commit()) } func TestRateLimiterStore(t *testing.T) { // Create a MultiLimiter m with a defaultConfig c and check the defaultConfig is applied t.Run("Store multiple transactions", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) ipNoPrefix1 := Key([]byte(""), []byte("127.0.0.1")) + c1 := LimiterConfig{Rate: 1} ipNoPrefix2 := Key([]byte(""), []byte("127.0.0.2")) + c2 := LimiterConfig{Rate: 2} + { + // Update config for ipNoPrefix1 and check it's applied + m.UpdateConfig(c1, ipNoPrefix1) m.Allow(ipLimited{key: ipNoPrefix1}) storeLimiter(m) l, ok := m.limiters.Load().Get(ipNoPrefix1) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c1.isApplied(limiter.limiter)) } + { + // Update config for ipNoPrefix2 and check it's applied + m.UpdateConfig(c2, ipNoPrefix2) m.Allow(ipLimited{key: ipNoPrefix2}) storeLimiter(m) l, ok := m.limiters.Load().Get(ipNoPrefix2) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c2.isApplied(limiter.limiter)) + + //Check that ipNoPrefix1 is unchanged l, ok = m.limiters.Load().Get(ipNoPrefix1) require.True(t, ok) require.NotNil(t, l) limiter = l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c1.isApplied(limiter.limiter)) } }) t.Run("runStore store multiple Limiters", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 10 * time.Second, ReconcileCheckInterval: 10 * time.Millisecond} + c := Config{ReconcileCheckLimit: 10 * time.Second, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) ctx, cancel := context.WithCancel(context.Background()) m.Run(ctx) defer cancel() + + // Create a limiter for ipNoPrefix1 ipNoPrefix1 := Key([]byte(""), []byte("127.0.0.1")) - ipNoPrefix2 := Key([]byte(""), []byte("127.0.0.2")) + c1 := LimiterConfig{Rate: 1} limiters := m.limiters.Load() + m.UpdateConfig(c1, ipNoPrefix1) m.Allow(ipLimited{key: ipNoPrefix1}) retry.RunWith(&retry.Timer{Wait: 1 * time.Second, Timeout: 5 * time.Second}, t, func(r *retry.R) { l := m.limiters.Load() require.NotEqual(r, limiters, l) limiters = l }) + + // Check that ipNoPrefix1 have the expected limiter l, ok := m.limiters.Load().Get(ipNoPrefix1) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c1.isApplied(limiter.limiter)) + + // Create a limiter for ipNoPrefix2 + ipNoPrefix2 := Key([]byte(""), []byte("127.0.0.2")) + c2 := LimiterConfig{Rate: 2} + m.UpdateConfig(c2, ipNoPrefix2) m.Allow(ipLimited{key: ipNoPrefix2}) retry.RunWith(&retry.Timer{Wait: 1 * time.Second, Timeout: 5 * time.Second}, t, func(r *retry.R) { l := m.limiters.Load() require.NotEqual(r, limiters, l) limiters = l }) + + // Check that ipNoPrefix1 have the expected limiter l, ok = m.limiters.Load().Get(ipNoPrefix1) require.True(t, ok) require.NotNil(t, l) limiter = l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c1.isApplied(limiter.limiter)) + + // Check that ipNoPrefix2 have the expected limiter l, ok = m.limiters.Load().Get(ipNoPrefix2) require.True(t, ok) require.NotNil(t, l) limiter = l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c2.isApplied(limiter.limiter)) }) } @@ -202,26 +231,38 @@ func TestRateLimiterUpdateConfig(t *testing.T) { // Create a MultiLimiter m with a defaultConfig c and check the defaultConfig is applied t.Run("Allow a key and check defaultConfig is applied to that key", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ipNoPrefix ipNoPrefix := Key([]byte(""), []byte("127.0.0.1")) + c1 := LimiterConfig{Rate: 1} + m.UpdateConfig(c1, ipNoPrefix) m.Allow(ipLimited{key: ipNoPrefix}) storeLimiter(m) + + // Verify the expected limiter is applied l, ok := m.limiters.Load().Get(ipNoPrefix) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) - require.True(t, c.isApplied(limiter.limiter)) + require.True(t, c1.isApplied(limiter.limiter)) }) t.Run("Update nil prefix and make sure it's written in the root", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for nil prefix := []byte(nil) - c1 := LimiterConfig{Rate: 2} + c1 := LimiterConfig{Rate: 1} m.UpdateConfig(c1, prefix) + + // Verify the expected limiter is applied v, ok := m.limitersConfigs.Load().Get([]byte("")) require.True(t, ok) require.NotNil(t, v) @@ -230,25 +271,39 @@ func TestRateLimiterUpdateConfig(t *testing.T) { }) t.Run("Allow 2 keys with prefix and check defaultConfig is applied to those keys", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ip prefix := []byte("namespace.write") ip := Key(prefix, []byte("127.0.0.1")) + c1 := LimiterConfig{Rate: 1} + m.UpdateConfig(c1, ip) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + //Create a limiter for ip2 ip2 := Key(prefix, []byte("127.0.0.2")) + c2 := LimiterConfig{Rate: 2} + m.UpdateConfig(c2, ip2) m.Allow(ipLimited{key: ip2}) + + //Verify the config is applied for ip l, ok := m.limiters.Load().Get(ip) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) - require.True(t, c.LimiterConfig.isApplied(limiter.limiter)) + require.True(t, c1.isApplied(limiter.limiter)) }) t.Run("Apply a defaultConfig to 'namespace.write' check the defaultConfig is applied to existing keys under that prefix", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ip prefix := []byte("namespace.write") ip := Key(prefix, []byte("127.0.0.1")) c3 := LimiterConfig{Rate: 2} @@ -257,6 +312,8 @@ func TestRateLimiterUpdateConfig(t *testing.T) { m.reconcileConfig(m.limiters.Load().Txn()) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + //Verify the config is applied for ip l3, ok3 := m.limiters.Load().Get(ip) require.True(t, ok3) require.NotNil(t, l3) @@ -264,31 +321,71 @@ func TestRateLimiterUpdateConfig(t *testing.T) { require.True(t, c3.isApplied(limiter3.limiter)) }) t.Run("Allow an IP with prefix and check prefix defaultConfig is applied to new keys under that prefix", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ip c1 := LimiterConfig{Rate: 3} prefix := []byte("namespace.read") m.UpdateConfig(c1, prefix) ip := Key(prefix, []byte("127.0.0.1")) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + //Verify the config is applied for ip l, ok := m.limiters.Load().Get(ip) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) require.True(t, c1.isApplied(limiter.limiter)) }) - t.Run("Allow an IP with prefix and check prefix config is applied to new keys under that prefix", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + + t.Run("Allow an IP with prefix and check prefix defaultConfig is applied to new keys under that prefix, delete config and check default applied", func(t *testing.T) { + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Second, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ip + c1 := LimiterConfig{Rate: 3} + prefix := []byte("namespace.read") + m.UpdateConfig(c1, prefix) + ip := Key(prefix, []byte("127.0.0.1")) + m.Allow(ipLimited{key: ip}) + storeLimiter(m) + + //Verify the config is applied for ip + l, ok := m.limiters.Load().Get(ip) + require.True(t, ok) + require.NotNil(t, l) + limiter := l.(*Limiter) + require.True(t, c1.isApplied(limiter.limiter)) + + // Delete the prefix + m.DeleteConfig(prefix) + reconcile(m) + + // Verify the limiter is removed + _, ok = m.limiters.Load().Get(ip) + require.False(t, ok) + }) + t.Run("Allow an IP with prefix and check prefix config is applied to new keys under that prefix", func(t *testing.T) { + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + m := NewMultiLimiter(c) + require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ip c1 := LimiterConfig{Rate: 3} prefix := Key([]byte("ip.ratelimit"), []byte("127.0")) m.UpdateConfig(c1, prefix) ip := Key([]byte("ip.ratelimit"), []byte("127.0.0.1")) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + //Verify the config is applied for ip load := m.limiters.Load() l, ok := load.Get(ip) require.True(t, ok) @@ -298,27 +395,38 @@ func TestRateLimiterUpdateConfig(t *testing.T) { }) t.Run("Allow an IP with 2 prefixes and check prefix config is applied to new keys under that prefix", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for "127.0" ip with config c1 c1 := LimiterConfig{Rate: 3} prefix := Key([]byte("ip.ratelimit"), []byte("127.0")) m.UpdateConfig(c1, prefix) + + //Create a limiter for "127.0.0" ip with config c2 prefix = Key([]byte("ip.ratelimit"), []byte("127.0.0")) c2 := LimiterConfig{Rate: 6} m.UpdateConfig(c2, prefix) ip := Key([]byte("ip.ratelimit"), []byte("127.0.0.1")) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + // Verify that "127.0.0.1" have the right limiter config load := m.limiters.Load() l, ok := load.Get(ip) require.True(t, ok) require.NotNil(t, l) limiter := l.(*Limiter) require.True(t, c2.isApplied(limiter.limiter)) + + //Create a limiter for "127.0.1.1" ip with config c2 ip = Key([]byte("ip.ratelimit"), []byte("127.0.1.1")) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + // Verify that "127.0.1.1" have the right limiter config load = m.limiters.Load() l, ok = load.Get(ip) require.True(t, ok) @@ -328,9 +436,12 @@ func TestRateLimiterUpdateConfig(t *testing.T) { }) t.Run("Allow an IP with prefix and check after it's cleaned new Allow would give it the right defaultConfig", func(t *testing.T) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + //Create a multilimiter + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(t, *m.defaultConfig.Load(), c) + + //Create a limiter for ip prefix := []byte("namespace.read") ip := Key(prefix, []byte("127.0.0.1")) c1 := LimiterConfig{Rate: 1} @@ -339,6 +450,8 @@ func TestRateLimiterUpdateConfig(t *testing.T) { reconcile(m) m.Allow(ipLimited{key: ip}) storeLimiter(m) + + //Verify the config is applied for ip l, ok := m.limiters.Load().Get(ip) require.True(t, ok) require.NotNil(t, l) @@ -348,14 +461,19 @@ func TestRateLimiterUpdateConfig(t *testing.T) { } func FuzzSingleConfig(f *testing.F) { - c := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} + c := Config{ReconcileCheckLimit: 100 * time.Millisecond, ReconcileCheckInterval: 10 * time.Millisecond} m := NewMultiLimiter(c) require.Equal(f, *m.defaultConfig.Load(), c) f.Add(Key(randIP())) f.Add(Key(randIP(), randIP())) f.Add(Key(randIP(), randIP(), randIP())) f.Add(Key(randIP(), randIP(), randIP(), randIP())) + c1 := LimiterConfig{ + Rate: 100, + Burst: 123, + } f.Fuzz(func(t *testing.T, ff []byte) { + m.UpdateConfig(c1, ff) m.Allow(Limited{key: ff}) storeLimiter(m) checkLimiter(t, ff, m.limiters.Load().Txn()) @@ -391,7 +509,7 @@ func FuzzUpdateConfig(f *testing.F) { f.Add(bytes.Join([][]byte{[]byte(""), Key(randIP()), Key(randIP(), randIP()), Key(randIP(), randIP(), randIP()), Key(randIP(), randIP(), randIP(), randIP())}, []byte(","))) f.Fuzz(func(t *testing.T, ff []byte) { - cm := Config{LimiterConfig: LimiterConfig{Rate: 0.1}, ReconcileCheckLimit: 1 * time.Millisecond, ReconcileCheckInterval: 1 * time.Millisecond} + cm := Config{ReconcileCheckLimit: 1 * time.Millisecond, ReconcileCheckInterval: 1 * time.Millisecond} m := NewMultiLimiter(cm) ctx, cancel := context.WithCancel(context.Background()) m.Run(ctx) @@ -443,7 +561,7 @@ func (i ipLimited) Key() []byte { } func BenchmarkTestRateLimiterFixedIP(b *testing.B) { - var Config = Config{LimiterConfig: LimiterConfig{Rate: 1.0, Burst: 500}, ReconcileCheckLimit: time.Microsecond, ReconcileCheckInterval: time.Millisecond} + var Config = Config{ReconcileCheckLimit: time.Microsecond, ReconcileCheckInterval: time.Millisecond} m := NewMultiLimiter(Config) ctx, cancel := context.WithCancel(context.Background()) m.Run(ctx) @@ -468,7 +586,7 @@ func BenchmarkTestRateLimiterAllowPrefill(b *testing.B) { for _, tc := range cases { b.Run(tc.name, func(b *testing.B) { - var Config = Config{LimiterConfig: LimiterConfig{Rate: 1.0, Burst: 500}, ReconcileCheckLimit: time.Second, ReconcileCheckInterval: time.Second} + var Config = Config{ReconcileCheckLimit: time.Second, ReconcileCheckInterval: time.Second} m := NewMultiLimiter(Config) var i uint64 for i = 0xdeaddead; i < 0xdeaddead+tc.prefill; i++ { @@ -501,7 +619,7 @@ func BenchmarkTestRateLimiterAllowConcurrencyPrefill(b *testing.B) { for _, tc := range cases { b.Run(tc.name, func(b *testing.B) { - var Config = Config{LimiterConfig: LimiterConfig{Rate: 1.0, Burst: 500}, ReconcileCheckLimit: time.Second, ReconcileCheckInterval: 100 * time.Second} + var Config = Config{ReconcileCheckLimit: time.Second, ReconcileCheckInterval: 100 * time.Second} m := NewMultiLimiter(Config) ctx, cancel := context.WithCancel(context.Background()) m.Run(ctx) @@ -530,7 +648,7 @@ func BenchmarkTestRateLimiterAllowConcurrencyPrefill(b *testing.B) { } func BenchmarkTestRateLimiterRandomIP(b *testing.B) { - var Config = Config{LimiterConfig: LimiterConfig{Rate: 1.0, Burst: 500}, ReconcileCheckLimit: time.Microsecond, ReconcileCheckInterval: time.Millisecond} + var Config = Config{ReconcileCheckLimit: time.Microsecond, ReconcileCheckInterval: time.Millisecond} m := NewMultiLimiter(Config) ctx, cancel := context.WithCancel(context.Background()) m.Run(ctx) diff --git a/agent/consul/rate/handler.go b/agent/consul/rate/handler.go index e8cb132f7a..d2c33ab88c 100644 --- a/agent/consul/rate/handler.go +++ b/agent/consul/rate/handler.go @@ -1,4 +1,4 @@ -// package rate implements server-side RPC rate limiting. +// Package rate implements server-side RPC rate limiting. package rate import ( diff --git a/agent/consul/rate/handler_test.go b/agent/consul/rate/handler_test.go index 3cdb34fbcf..6a74fe7caa 100644 --- a/agent/consul/rate/handler_test.go +++ b/agent/consul/rate/handler_test.go @@ -420,3 +420,6 @@ func (m *mockLimiter) Run(ctx context.Context) { m.Called(ctx) } func (m *mockLimiter) UpdateConfig(cfg multilimiter.LimiterConfig, prefix []byte) { m.Called(cfg, prefix) } +func (m *mockLimiter) DeleteConfig(prefix []byte) { + m.Called(prefix) +}