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

@@ -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)

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()
+	m.updateConfig(&c, prefix)
-	defer m.configsLock.Unlock()
+}
-	if prefix == nil {
+
-		prefix = []byte("")
+// DeleteConfig will delete the MultiLimiter Config
-	}
+// which will cascade to all the Limiter(s) LimiterConfig
-	configs := m.limitersConfigs.Load()
+func (m *MultiLimiter) DeleteConfig(prefix []byte) {
-	newConfigs, _, _ := configs.Insert(prefix, &c)
+	m.updateConfig(nil, prefix)
-	m.limitersConfigs.Store(newConfigs)
 }
 
 // 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 {
+	if !ok {
-		c, ok := configs.Get(p)
+		return true
-		if ok && c != nil {
+	}
-			config = c.(*LimiterConfig)
+	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)

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)

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 (

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)
+}