Skip to content

Commit f52a616

Browse files
stekolestekole
authored
fix: apply TLS and auth config to Redis Sentinel connections (#1015)
* fix: apply TLS and auth config to Redis Sentinel connections When using Redis Sentinel with TLS enabled, the client was failing to connect to Sentinel nodes because the TLS configuration was not being applied to the SentinelConnFunc. This caused "SSL wrong version number" errors and connection resets. This fix adds a sentinelDialFunc that properly applies: - TLS configuration (when REDIS_TLS=true) - Authentication settings (when REDIS_AUTH is set) - Connection timeout settings The fix mirrors the approach used for the main Redis connection dial function, ensuring consistent configuration across both Sentinel and data node connections. Fixes connection to Redis Sentinel over TLS. Signed-off-by: Stefan Kolesnikowicz <stefan@sandnetworks.com> Signed-off-by: stekole <stefan@sandnetworks.com> * fix: apply TLS and auth config to Redis Sentinel connections When using Redis Sentinel with TLS enabled, the client was failing to connect to Sentinel nodes because the TLS configuration was not being applied to the SentinelConnFunc. This caused "SSL wrong version number" errors and connection resets. This fix adds a sentinelDialFunc that properly applies: - TLS configuration (when REDIS_TLS=true) - Authentication settings (when REDIS_AUTH is set) - Connection timeout settings The fix mirrors the approach used for the main Redis connection dial function, ensuring consistent configuration across both Sentinel and data node connections. Fixes connection to Redis Sentinel over TLS. Signed-off-by: Stefan Kolesnikowicz <stefan@sandnetworks.com> Signed-off-by: stekole <stefan@sandnetworks.com> * fix: apply TLS and auth config to Redis Sentinel connections When using Redis Sentinel with TLS enabled, the client was failing to connect to Sentinel nodes because the TLS configuration was not being applied to the SentinelConnFunc. This caused "SSL wrong version number" errors and connection resets. This fix adds a sentinelDialFunc that properly applies: - TLS configuration (when REDIS_TLS=true) - Authentication settings (when REDIS_AUTH is set) - Connection timeout settings The fix mirrors the approach used for the main Redis connection dial function, ensuring consistent configuration across both Sentinel and data node connections. Fixes connection to Redis Sentinel over TLS. Signed-off-by: Stefan Kolesnikowicz <stefan@sandnetworks.com> Signed-off-by: stekole <stefan@sandnetworks.com> --------- Signed-off-by: Stefan Kolesnikowicz <stefan@sandnetworks.com> Signed-off-by: stekole <stefan@sandnetworks.com>
1 parent fc44670 commit f52a616

9 files changed

Lines changed: 196 additions & 13 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
cover.out
22

33
bin/
4+
env/
45
.idea/
56
.vscode/
67
vendor

‎README.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,8 +1253,9 @@ As well Ratelimit supports TLS connections and authentication. These can be conf
12531253
1. `REDIS_TLS` & `REDIS_PERSECOND_TLS`: set to `"true"` to enable a TLS connection for the specific connection type.
12541254
1. `REDIS_TLS_CLIENT_CERT`, `REDIS_TLS_CLIENT_KEY`, and `REDIS_TLS_CACERT` to provides files to specify a TLS connection configuration to Redis server that requires client certificate verification. (This is effective when `REDIS_TLS` or `REDIS_PERSECOND_TLS` is set to to `"true"`).
12551255
1. `REDIS_TLS_SKIP_HOSTNAME_VERIFICATION` set to `"true"` will skip hostname verification in environments where the certificate has an invalid hostname, such as GCP Memorystore.
1256-
1. `REDIS_AUTH` & `REDIS_PERSECOND_AUTH`: set to `"password"` to enable password-only authentication to the redis host.
1257-
1. `REDIS_AUTH` & `REDIS_PERSECOND_AUTH`: set to `"username:password"` to enable username-password authentication to the redis host.
1256+
1. `REDIS_AUTH` & `REDIS_PERSECOND_AUTH`: set to `"password"` to enable password-only authentication to the Redis master/replica nodes.
1257+
1. `REDIS_AUTH` & `REDIS_PERSECOND_AUTH`: set to `"username:password"` to enable username-password authentication to the Redis master/replica nodes.
1258+
1. `REDIS_SENTINEL_AUTH` & `REDIS_PERSECOND_SENTINEL_AUTH`: set to `"password"` or `"username:password"` to enable authentication to Redis Sentinel nodes. This is separate from `REDIS_AUTH`/`REDIS_PERSECOND_AUTH` which authenticate to the Redis master/replica nodes. Only used when `REDIS_TYPE` or `REDIS_PERSECOND_TYPE` is set to `"sentinel"`. If not set, no authentication will be attempted when connecting to Sentinel nodes.
12581259
1. `CACHE_KEY_PREFIX`: a string to prepend to all cache keys
12591260

12601261
For controlling the behavior of cache key incrementation when any of them is already over the limit, you can use the following configuration:

‎src/redis/cache_impl.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ func NewRateLimiterCacheImplFromSettings(s settings.Settings, localCache *freeca
1919
if s.RedisPerSecond {
2020
perSecondPool = NewClientImpl(srv.Scope().Scope("redis_per_second_pool"), s.RedisPerSecondTls, s.RedisPerSecondAuth, s.RedisPerSecondSocketType,
2121
s.RedisPerSecondType, s.RedisPerSecondUrl, s.RedisPerSecondPoolSize, s.RedisPerSecondPipelineWindow, s.RedisPerSecondPipelineLimit, s.RedisTlsConfig, s.RedisHealthCheckActiveConnection, srv, s.RedisPerSecondTimeout,
22-
s.RedisPerSecondPoolOnEmptyBehavior, s.RedisPerSecondPoolOnEmptyWaitDuration)
22+
s.RedisPerSecondPoolOnEmptyBehavior, s.RedisPerSecondPoolOnEmptyWaitDuration, s.RedisPerSecondSentinelAuth)
2323
closer.Closers = append(closer.Closers, perSecondPool)
2424
}
2525

2626
otherPool := NewClientImpl(srv.Scope().Scope("redis_pool"), s.RedisTls, s.RedisAuth, s.RedisSocketType, s.RedisType, s.RedisUrl, s.RedisPoolSize,
2727
s.RedisPipelineWindow, s.RedisPipelineLimit, s.RedisTlsConfig, s.RedisHealthCheckActiveConnection, srv, s.RedisTimeout,
28-
s.RedisPoolOnEmptyBehavior, s.RedisPoolOnEmptyWaitDuration)
28+
s.RedisPoolOnEmptyBehavior, s.RedisPoolOnEmptyWaitDuration, s.RedisSentinelAuth)
2929
closer.Closers = append(closer.Closers, otherPool)
3030

3131
return NewFixedRateLimitCacheImpl(

‎src/redis/driver_impl.go‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func checkError(err error) {
7272

7373
func NewClientImpl(scope stats.Scope, useTls bool, auth, redisSocketType, redisType, url string, poolSize int,
7474
pipelineWindow time.Duration, pipelineLimit int, tlsConfig *tls.Config, healthCheckActiveConnection bool, srv server.Server,
75-
timeout time.Duration, poolOnEmptyBehavior string, poolOnEmptyWaitDuration time.Duration,
75+
timeout time.Duration, poolOnEmptyBehavior string, poolOnEmptyWaitDuration time.Duration, sentinelAuth string,
7676
) Client {
7777
maskedUrl := utils.MaskCredentialsInUrl(url)
7878
logger.Warnf("connecting to redis on %s with pool size %d", maskedUrl, poolSize)
@@ -148,7 +148,29 @@ func NewClientImpl(scope stats.Scope, useTls bool, auth, redisSocketType, redisT
148148
if len(urls) < 2 {
149149
panic(RedisError("Expected master name and a list of urls for the sentinels, in the format: <redis master name>,<sentinel1>,...,<sentineln>"))
150150
}
151-
client, err = radix.NewSentinel(urls[0], urls[1:], radix.SentinelPoolFunc(poolFunc))
151+
sentinelDialFunc := func(network, addr string) (radix.Conn, error) {
152+
var dialOpts []radix.DialOpt
153+
// Always set the dial timeout consistent with the main dial func
154+
dialOpts = append(dialOpts, radix.DialTimeout(timeout))
155+
if useTls {
156+
logger.Warnf("enabling TLS to redis sentinel on %s", addr)
157+
dialOpts = append(dialOpts, radix.DialUseTLS(tlsConfig))
158+
}
159+
// Use sentinelAuth for authenticating to Sentinel nodes, not auth
160+
// auth is used for Redis master/replica authentication
161+
if sentinelAuth != "" {
162+
user, pass, found := strings.Cut(sentinelAuth, ":")
163+
if found {
164+
logger.Warnf("enabling authentication to redis sentinel on %s with user %s", addr, user)
165+
dialOpts = append(dialOpts, radix.DialAuthUser(user, pass))
166+
} else {
167+
logger.Warnf("enabling authentication to redis sentinel on %s without user", addr)
168+
dialOpts = append(dialOpts, radix.DialAuthPass(sentinelAuth))
169+
}
170+
}
171+
return radix.Dial(network, addr, dialOpts...)
172+
}
173+
client, err = radix.NewSentinel(urls[0], urls[1:], radix.SentinelConnFunc(sentinelDialFunc), radix.SentinelPoolFunc(poolFunc))
152174
default:
153175
panic(RedisError("Unrecognized redis type " + redisType))
154176
}

‎src/redis/fixed_cache_impl.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ func pipelineAppendtoGet(client Client, pipeline *Pipeline, key string, result *
4242
}
4343

4444
func (this *fixedRateLimitCacheImpl) getHitsAddend(hitsAddend uint64, isCacheKeyOverlimit, isCacheKeyNearlimit,
45-
isNearLimt bool) uint64 {
45+
isNearLimt bool,
46+
) uint64 {
4647
// If stopCacheKeyIncrementWhenOverlimit is false, then we always increment the cache key.
4748
if !this.stopCacheKeyIncrementWhenOverlimit {
4849
return hitsAddend

‎src/settings/settings.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,14 @@ type Settings struct {
151151
RedisPerSecondPoolSize int `envconfig:"REDIS_PERSECOND_POOL_SIZE" default:"10"`
152152
RedisPerSecondAuth string `envconfig:"REDIS_PERSECOND_AUTH" default:""`
153153
RedisPerSecondTls bool `envconfig:"REDIS_PERSECOND_TLS" default:"false"`
154+
// RedisSentinelAuth is the password for authenticating to Redis Sentinel nodes (not the Redis master/replica).
155+
// This is separate from RedisAuth which is used for authenticating to the Redis master/replica nodes.
156+
// If empty, no authentication will be attempted when connecting to Sentinel nodes.
157+
RedisSentinelAuth string `envconfig:"REDIS_SENTINEL_AUTH" default:""`
158+
// RedisPerSecondSentinelAuth is the password for authenticating to per-second Redis Sentinel nodes.
159+
// This is separate from RedisPerSecondAuth which is used for authenticating to the Redis master/replica nodes.
160+
// If empty, no authentication will be attempted when connecting to per-second Sentinel nodes.
161+
RedisPerSecondSentinelAuth string `envconfig:"REDIS_PERSECOND_SENTINEL_AUTH" default:""`
154162
// RedisPerSecondPipelineWindow sets the duration after which internal pipelines will be flushed for per second redis.
155163
// See comments of RedisPipelineWindow for details.
156164
RedisPerSecondPipelineWindow time.Duration `envconfig:"REDIS_PERSECOND_PIPELINE_WINDOW" default:"0"`

‎test/common/common.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ func NewRateLimitRequest(domain string, descriptors [][][2]string, hitsAddend ui
7474
}
7575

7676
func NewRateLimitRequestWithPerDescriptorHitsAddend(domain string, descriptors [][][2]string,
77-
hitsAddends []uint64) *pb.RateLimitRequest {
77+
hitsAddends []uint64,
78+
) *pb.RateLimitRequest {
7879
request := NewRateLimitRequest(domain, descriptors, 1)
7980
for i, hitsAddend := range hitsAddends {
8081
request.Descriptors[i].HitsAddend = &wrapperspb.UInt64Value{Value: hitsAddend}

‎test/redis/bench_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func BenchmarkParallelDoLimit(b *testing.B) {
4444
return func(b *testing.B) {
4545
statsStore := gostats.NewStore(gostats.NewNullSink(), false)
4646
sm := stats.NewMockStatManager(statsStore)
47-
client := redis.NewClientImpl(statsStore, false, "", "tcp", "single", "127.0.0.1:6379", poolSize, pipelineWindow, pipelineLimit, nil, false, nil, 10*time.Second, "", 0)
47+
client := redis.NewClientImpl(statsStore, false, "", "tcp", "single", "127.0.0.1:6379", poolSize, pipelineWindow, pipelineLimit, nil, false, nil, 10*time.Second, "", 0, "")
4848
defer client.Close()
4949

5050
cache := redis.NewFixedRateLimitCacheImpl(client, nil, utils.NewTimeSourceImpl(), rand.New(utils.NewLockedSource(time.Now().Unix())), 10, nil, 0.8, "", sm, true)

‎test/redis/driver_impl_test.go‎

Lines changed: 153 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package redis_test
22

33
import (
44
"fmt"
5+
"strings"
56
"testing"
67
"time"
78

@@ -38,7 +39,7 @@ func testNewClientImpl(t *testing.T, pipelineWindow time.Duration, pipelineLimit
3839
statsStore := stats.NewStore(stats.NewNullSink(), false)
3940

4041
mkRedisClient := func(auth, addr string) redis.Client {
41-
return redis.NewClientImpl(statsStore, false, auth, "tcp", "single", addr, 1, pipelineWindow, pipelineLimit, nil, false, nil, 10*time.Second, "", 0)
42+
return redis.NewClientImpl(statsStore, false, auth, "tcp", "single", addr, 1, pipelineWindow, pipelineLimit, nil, false, nil, 10*time.Second, "", 0, "")
4243
}
4344

4445
t.Run("connection refused", func(t *testing.T) {
@@ -131,7 +132,7 @@ func TestDoCmd(t *testing.T) {
131132
statsStore := stats.NewStore(stats.NewNullSink(), false)
132133

133134
mkRedisClient := func(addr string) redis.Client {
134-
return redis.NewClientImpl(statsStore, false, "", "tcp", "single", addr, 1, 0, 0, nil, false, nil, 10*time.Second, "", 0)
135+
return redis.NewClientImpl(statsStore, false, "", "tcp", "single", addr, 1, 0, 0, nil, false, nil, 10*time.Second, "", 0, "")
135136
}
136137

137138
t.Run("SETGET ok", func(t *testing.T) {
@@ -176,7 +177,7 @@ func testPipeDo(t *testing.T, pipelineWindow time.Duration, pipelineLimit int) f
176177
statsStore := stats.NewStore(stats.NewNullSink(), false)
177178

178179
mkRedisClient := func(addr string) redis.Client {
179-
return redis.NewClientImpl(statsStore, false, "", "tcp", "single", addr, 1, pipelineWindow, pipelineLimit, nil, false, nil, 10*time.Second, "", 0)
180+
return redis.NewClientImpl(statsStore, false, "", "tcp", "single", addr, 1, pipelineWindow, pipelineLimit, nil, false, nil, 10*time.Second, "", 0, "")
180181
}
181182

182183
t.Run("SETGET ok", func(t *testing.T) {
@@ -238,7 +239,7 @@ func TestPoolOnEmptyBehavior(t *testing.T) {
238239

239240
// Helper to create client with specific on-empty behavior
240241
mkRedisClientWithBehavior := func(addr, behavior string, waitDuration time.Duration) redis.Client {
241-
return redis.NewClientImpl(statsStore, false, "", "tcp", "single", addr, 1, 0, 0, nil, false, nil, 10*time.Second, behavior, waitDuration)
242+
return redis.NewClientImpl(statsStore, false, "", "tcp", "single", addr, 1, 0, 0, nil, false, nil, 10*time.Second, behavior, waitDuration, "")
242243
}
243244

244245
t.Run("default behavior (empty string)", func(t *testing.T) {
@@ -379,3 +380,151 @@ func TestPoolOnEmptyBehavior(t *testing.T) {
379380
assert.Equal(t, "value7", res)
380381
})
381382
}
383+
384+
func TestNewClientImplSentinel(t *testing.T) {
385+
statsStore := stats.NewStore(stats.NewNullSink(), false)
386+
387+
mkSentinelClient := func(auth, sentinelAuth, url string, useTls bool, timeout time.Duration) redis.Client {
388+
// Pass nil for tlsConfig - we can't test TLS without a real TLS server,
389+
// but we can verify the code path is executed (logs will show TLS is enabled)
390+
return redis.NewClientImpl(statsStore, useTls, auth, "tcp", "sentinel", url, 1, 0, 0, nil, false, nil, timeout, "", 0, sentinelAuth)
391+
}
392+
393+
t.Run("invalid url format - missing sentinel addresses", func(t *testing.T) {
394+
panicErr := expectPanicError(t, func() {
395+
mkSentinelClient("", "", "mymaster", false, 10*time.Second)
396+
})
397+
assert.Contains(t, panicErr.Error(), "Expected master name and a list of urls for the sentinels")
398+
})
399+
400+
t.Run("invalid url format - only master name", func(t *testing.T) {
401+
panicErr := expectPanicError(t, func() {
402+
mkSentinelClient("", "", "mymaster,", false, 10*time.Second)
403+
})
404+
// Empty sentinel address causes "missing address" error from radix
405+
assert.True(t,
406+
containsAny(panicErr.Error(), []string{"Expected master name", "missing address"}),
407+
"Expected format validation error, got: %s", panicErr.Error())
408+
})
409+
410+
t.Run("connection refused - sentinel not available", func(t *testing.T) {
411+
// Use a port that's unlikely to have a sentinel running
412+
url := "mymaster,localhost:12345"
413+
panicErr := expectPanicError(t, func() {
414+
mkSentinelClient("", "", url, false, 1*time.Second)
415+
})
416+
// Should fail with connection error or timeout
417+
assert.NotNil(t, panicErr)
418+
assert.True(t,
419+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect"}),
420+
"Expected connection error, got: %s", panicErr.Error())
421+
})
422+
423+
t.Run("sentinel auth password only", func(t *testing.T) {
424+
// This will fail to connect, but we're testing that sentinelAuth parameter is accepted
425+
// The log output will show "enabling authentication to redis sentinel" which confirms the code path
426+
url := "mymaster,localhost:12345"
427+
panicErr := expectPanicError(t, func() {
428+
mkSentinelClient("", "sentinel-password", url, false, 1*time.Second)
429+
})
430+
// Should fail with connection error, not auth error (since we can't connect)
431+
assert.NotNil(t, panicErr)
432+
assert.True(t,
433+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect"}),
434+
"Expected connection error, got: %s", panicErr.Error())
435+
})
436+
437+
t.Run("sentinel auth user:password", func(t *testing.T) {
438+
// This will fail to connect, but we're testing that sentinelAuth parameter with user:password format is accepted
439+
// The log output will show "enabling authentication to redis sentinel on ... with user sentinel-user"
440+
url := "mymaster,localhost:12345"
441+
panicErr := expectPanicError(t, func() {
442+
mkSentinelClient("", "sentinel-user:sentinel-pass", url, false, 1*time.Second)
443+
})
444+
// Should fail with connection error, not auth error (since we can't connect)
445+
assert.NotNil(t, panicErr)
446+
assert.True(t,
447+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect"}),
448+
"Expected connection error, got: %s", panicErr.Error())
449+
})
450+
451+
t.Run("sentinel with timeout", func(t *testing.T) {
452+
// Test that timeout parameter is used
453+
url := "mymaster,localhost:12345"
454+
start := time.Now()
455+
panicErr := expectPanicError(t, func() {
456+
mkSentinelClient("", "", url, false, 500*time.Millisecond)
457+
})
458+
duration := time.Since(start)
459+
assert.NotNil(t, panicErr)
460+
// Timeout should be respected (with some tolerance)
461+
assert.True(t, duration < 2*time.Second, "Timeout should be respected, took %v", duration)
462+
})
463+
464+
t.Run("sentinel with multiple addresses", func(t *testing.T) {
465+
// Test that multiple sentinel addresses are accepted in URL format
466+
url := "mymaster,localhost:12345,localhost:12346,localhost:12347"
467+
panicErr := expectPanicError(t, func() {
468+
mkSentinelClient("", "", url, false, 1*time.Second)
469+
})
470+
// Should fail with connection error, not format error
471+
assert.NotNil(t, panicErr)
472+
assert.NotContains(t, panicErr.Error(), "Expected master name")
473+
assert.True(t,
474+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect"}),
475+
"Expected connection error, got: %s", panicErr.Error())
476+
})
477+
478+
t.Run("sentinel with redis auth but no sentinel auth", func(t *testing.T) {
479+
// Test that redis auth and sentinel auth are separate
480+
// redisAuth is for master/replica, sentinelAuth is for sentinel nodes
481+
url := "mymaster,localhost:12345"
482+
panicErr := expectPanicError(t, func() {
483+
mkSentinelClient("redis-password", "", url, false, 1*time.Second)
484+
})
485+
// Should fail with connection error (can't test auth without real sentinel)
486+
assert.NotNil(t, panicErr)
487+
assert.True(t,
488+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect"}),
489+
"Expected connection error, got: %s", panicErr.Error())
490+
})
491+
492+
t.Run("sentinel with TLS enabled", func(t *testing.T) {
493+
// Test that TLS configuration is accepted (will fail to connect without real TLS server)
494+
// The log output will show "enabling TLS to redis sentinel" which confirms the code path
495+
url := "mymaster,localhost:12345"
496+
panicErr := expectPanicError(t, func() {
497+
mkSentinelClient("", "", url, true, 1*time.Second)
498+
})
499+
// Should fail with connection/TLS error (can't test TLS without real TLS server)
500+
assert.NotNil(t, panicErr)
501+
// Error could be connection refused, TLS handshake failure, or timeout
502+
assert.True(t,
503+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect", "tls", "handshake"}),
504+
"Expected connection/TLS error, got: %s", panicErr.Error())
505+
})
506+
507+
t.Run("sentinel with TLS and sentinel auth", func(t *testing.T) {
508+
// Test that both TLS and sentinel auth can be configured together
509+
// The log output will show both TLS and auth messages
510+
url := "mymaster,localhost:12345"
511+
panicErr := expectPanicError(t, func() {
512+
mkSentinelClient("redis-password", "sentinel-password", url, true, 1*time.Second)
513+
})
514+
// Should fail with connection/TLS error (can't test without real servers)
515+
assert.NotNil(t, panicErr)
516+
assert.True(t,
517+
containsAny(panicErr.Error(), []string{"connection refused", "timeout", "no such host", "connect", "tls", "handshake"}),
518+
"Expected connection/TLS error, got: %s", panicErr.Error())
519+
})
520+
}
521+
522+
// Helper function to check if error message contains any of the given strings
523+
func containsAny(s string, substrs []string) bool {
524+
for _, substr := range substrs {
525+
if strings.Contains(strings.ToLower(s), strings.ToLower(substr)) {
526+
return true
527+
}
528+
}
529+
return false
530+
}

0 commit comments

Comments
 (0)