From 0f2eca0a4dd590eeba9b865079942e41cd289bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Millet?= Date: Fri, 26 Apr 2024 11:45:53 +0200 Subject: [PATCH 1/4] Add a Redis Sentinel backend This new backend allows using a Redis Sentinel cluster to reliably store objects in the cache. With the redundancy and automatic master switching in a Redis Sentinel cluster, it ensures that the service and data stays available even if some of the storage nodes go down. Please note that due to the delay in Redis replication, some of the most recent data inserted in the current master can be lost if this host goes down before having a chance to replicate the change. It also takes some time for Sentinel to detect a host going down and elect a new master, so a short outage may occur when the current master fails. Redis Sentinel documentation (https://redis.io/docs/latest/operate/oss_and_stack/management/sentinel/) recommends a minimum of 3 nodes and gives several architecture examples. --- README.md | 25 +++ backends/config/config.go | 2 + backends/redis-sentinel.go | 98 +++++++++++ backends/redis-sentinel_test.go | 195 ++++++++++++++++++++++ config.yaml | 11 +- config/backends.go | 48 ++++-- config/configtest/sample_full_config.yaml | 9 + go.mod | 3 +- go.sum | 9 +- 9 files changed, 381 insertions(+), 19 deletions(-) create mode 100644 backends/redis-sentinel.go create mode 100644 backends/redis-sentinel_test.go diff --git a/README.md b/README.md index 6ac453ea..27872a09 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,7 @@ backend: ### Aerospike Prebid Cache makes use of an Aerospike Go client that requires Aerospike server version 4.9+ and will not work properly with older versions. Full documentation of the Aerospike Go client can be found [here](https://github.com/aerospike/aerospike-client-go/tree/v6). + | Configuration field | Type | Description | | --- | --- | --- | | host | string | aerospike server URI | @@ -229,6 +230,7 @@ Prebid Cache makes use of an Aerospike Go client that requires Aerospike server ### Cassandra Prebid Cache makes use of a Cassandra client that supports latest 3 major releases of Cassandra (2.1.x, 2.2.x, and 3.x.x). Full documentation of the Cassandra Go client can be found [here](https://github.com/gocql/gocql). + | Configuration field | Type | Description | | --- | --- | --- | | hosts | string | Cassandra server URI | @@ -243,6 +245,7 @@ Prebid Cache makes use of a Cassandra client that supports latest 3 major releas ### Redis: Prebid Cache makes use of a Redis Go client compatible with Redis 6. Full documentation of the Redis Go client Prebid Cache uses can be found [here](https://github.com/go-redis/redis). + | Configuration field | Type | Description | | --- | --- | --- | | host | string | Redis server URI | @@ -252,6 +255,19 @@ Prebid Cache makes use of a Redis Go client compatible with Redis 6. Full docume | expiration | integer | Availability in the Redis system in Minutes | | tls | field | Subfields:
`enabled`: whether or not pass the InsecureSkipVerify value to the Redis client's TLS config
`insecure_skip_verify`: In Redis, InsecureSkipVerify controls whether a client verifies the server's certificate chain and host name. If InsecureSkipVerify is true, crypto/t | +### Redis Sentinel: +Prebid Cache makes use of a Redis Go client compatible with Redis Sentinel. Full documentation of the Redis Go client can be found [here](https://github.com/go-redis/redis). + +| Configuration field | Type | Description | +|---------------------|--------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| sentinel_addrs | string array | List of Sentinel nodes (host:port) managing the same master | +| master_name | string | Name of the Sentinel master (as declared in the `sentinel monitor` line of your Sentinel configurations) | +| password | string | Redis password | +| db | integer | Database to be selected after connecting to the server | +| expiration | integer | Availability in the Redis system in Minutes | +| tls | field | Subfields:
`enabled`: whether or not pass the InsecureSkipVerify value to the Redis client's TLS config
`insecure_skip_verify`: In Redis, InsecureSkipVerify controls whether a client verifies the server's certificate chain and host name. If InsecureSkipVerify is true, crypto/t | + + Sample configuration file `config/configtest/sample_full_config.yaml` shown below: ```yaml port: 9000 @@ -291,6 +307,15 @@ backend: tls: enabled: false insecure_skip_verify: false + redis_sentinel: + sentinel_addrs: [ "127.0.0.1:26379", "127.0.0.1:26380", "127.0.0.1:26381" ] + master_name: "mymaster" + password: "" + db: 1 + expiration: 10 # in Minutes + tls: + enabled: false + insecure_skip_verify: false compression: type: "snappy" metrics: diff --git a/backends/config/config.go b/backends/config/config.go index 22c9c010..d5e5bb8a 100644 --- a/backends/config/config.go +++ b/backends/config/config.go @@ -63,6 +63,8 @@ func newBaseBackend(cfg config.Backend, appMetrics *metrics.Metrics) backends.Ba return backends.NewAerospikeBackend(cfg.Aerospike, appMetrics) case config.BackendRedis: return backends.NewRedisBackend(cfg.Redis, ctx) + case config.BackendRedisSentinel: + return backends.NewRedisSentinelBackend(cfg.RedisSentinel, ctx) case config.BackendIgnite: return backends.NewIgniteBackend(cfg.Ignite) default: diff --git a/backends/redis-sentinel.go b/backends/redis-sentinel.go new file mode 100644 index 00000000..025eb51d --- /dev/null +++ b/backends/redis-sentinel.go @@ -0,0 +1,98 @@ +package backends + +import ( + "context" + "crypto/tls" + "time" + + "github.com/prebid/prebid-cache/config" + "github.com/prebid/prebid-cache/utils" + "github.com/redis/go-redis/v9" + log "github.com/sirupsen/logrus" +) + +// RedisSentinelDB is an interface that helps us communicate with an instance of a +// Redis Sentinel database. Its implementation is intended to use the "github.com/redis/go-redis" +// client +type RedisSentinelDB interface { + Get(ctx context.Context, key string) (string, error) + Put(ctx context.Context, key string, value string, ttlSeconds int) (bool, error) +} + +// RedisSentinelDBClient is a wrapper for the Redis client that implements the RedisSentinelDB interface +type RedisSentinelDBClient struct { + client *redis.Client +} + +// Get returns the value associated with the provided `key` parameter +func (db RedisSentinelDBClient) Get(ctx context.Context, key string) (string, error) { + return db.client.Get(ctx, key).Result() +} + +// Put will set 'key' to hold string 'value' if 'key' does not exist in the redis storage. +// When key already holds a value, no operation is performed. That's the reason this adapter +// uses the 'github.com/go-redis/redis's library SetNX. SetNX is short for "SET if Not eXists". +func (db RedisSentinelDBClient) Put(ctx context.Context, key, value string, ttlSeconds int) (bool, error) { + return db.client.SetNX(ctx, key, value, time.Duration(ttlSeconds)*time.Second).Result() +} + +// RedisSentinelBackend when initialized will instantiate and configure the Redis client. It implements +// the Backend interface. +type RedisSentinelBackend struct { + cfg config.RedisSentinel + client RedisDB +} + +// NewRedisSentinelBackend initializes the Redis Sentinel client and pings to make sure connection was successful +func NewRedisSentinelBackend(cfg config.RedisSentinel, ctx context.Context) *RedisSentinelBackend { + options := &redis.FailoverOptions{ + MasterName: cfg.MasterName, + SentinelAddrs: cfg.SentinelAddrs, + Password: cfg.Password, + DB: cfg.Db, + } + + if cfg.TLS.Enabled { + options.TLSConfig = &tls.Config{InsecureSkipVerify: cfg.TLS.InsecureSkipVerify} + } + + client := RedisSentinelDBClient{client: redis.NewFailoverClient(options)} + + _, err := client.client.Ping(ctx).Result() + if err != nil { + log.Fatalf("Error creating Redis Sentinel backend: %v", err) + } + log.Infof("Connected to Redis Sentinels at %v", cfg.SentinelAddrs) + + return &RedisSentinelBackend{ + cfg: cfg, + client: client, + } +} + +// Get calls the Redis Sentinel client to return the value associated with the provided `key` +// parameter and interprets its response. A `Nil` error reply of the Redis client means +// the `key` does not exist. +func (b *RedisSentinelBackend) Get(ctx context.Context, key string) (string, error) { + res, err := b.client.Get(ctx, key) + if err == redis.Nil { + err = utils.NewPBCError(utils.KEY_NOT_FOUND) + } + + return res, err +} + +// Put writes the `value` under the provided `key` in the Redis Sentinel storage server. Because the backend +// implementation of Put calls SetNX(item *Item), a `false` return value is interpreted as the data +// not being written because the `key` already holds a value, and a RecordExistsError is returned +func (b *RedisSentinelBackend) Put(ctx context.Context, key string, value string, ttlSeconds int) error { + success, err := b.client.Put(ctx, key, value, ttlSeconds) + if err != nil && err != redis.Nil { + return err + } + if !success { + return utils.NewPBCError(utils.RECORD_EXISTS) + } + + return nil +} diff --git a/backends/redis-sentinel_test.go b/backends/redis-sentinel_test.go new file mode 100644 index 00000000..0e5155aa --- /dev/null +++ b/backends/redis-sentinel_test.go @@ -0,0 +1,195 @@ +package backends + +import ( + "context" + "errors" + "testing" + + "github.com/prebid/prebid-cache/utils" + "github.com/redis/go-redis/v9" + "github.com/stretchr/testify/assert" +) + +func TestRedisSentinelClientGet(t *testing.T) { + redisSentinelBackend := &RedisSentinelBackend{} + + type testInput struct { + client RedisDB + key string + } + + type testExpectedValues struct { + value string + err error + } + + testCases := []struct { + desc string + in testInput + expected testExpectedValues + }{ + { + desc: "RedisSentinelBackend.Get() throws a redis.Nil error", + in: testInput{ + client: FakeRedisClient{ + Success: false, + ServerError: redis.Nil, + }, + key: "someKeyThatWontBeFound", + }, + expected: testExpectedValues{ + value: "", + err: utils.NewPBCError(utils.KEY_NOT_FOUND), + }, + }, + { + desc: "RedisBackend.Get() throws an error different from redis.Nil", + in: testInput{ + client: FakeRedisClient{ + Success: false, + ServerError: errors.New("some other get error"), + }, + key: "someKey", + }, + expected: testExpectedValues{ + value: "", + err: errors.New("some other get error"), + }, + }, + { + desc: "RedisBackend.Get() doesn't throw an error", + in: testInput{ + client: FakeRedisClient{ + Success: true, + StoredData: map[string]string{"defaultKey": "aValue"}, + }, + key: "defaultKey", + }, + expected: testExpectedValues{ + value: "aValue", + err: nil, + }, + }, + } + + for _, tt := range testCases { + redisSentinelBackend.client = tt.in.client + + // Run test + actualValue, actualErr := redisSentinelBackend.Get(context.Background(), tt.in.key) + + // Assertions + assert.Equal(t, tt.expected.value, actualValue, tt.desc) + assert.Equal(t, tt.expected.err, actualErr, tt.desc) + } +} + +func TestRedisSentinelClientPut(t *testing.T) { + redisSentinelBackend := &RedisSentinelBackend{} + + type testInput struct { + redisSentinelClient RedisDB + key string + valueToStore string + ttl int + } + + type testExpectedValues struct { + writtenValue string + redisClientErr error + } + + testCases := []struct { + desc string + in testInput + expected testExpectedValues + }{ + { + desc: "Try to overwrite already existing key. From redis client documentation, SetNX returns 'false' because no operation is performed", + in: testInput{ + redisSentinelClient: FakeRedisClient{ + Success: false, + StoredData: map[string]string{"key": "original value"}, + ServerError: redis.Nil, + }, + key: "key", + valueToStore: "overwrite value", + ttl: 10, + }, + expected: testExpectedValues{ + redisClientErr: utils.NewPBCError(utils.RECORD_EXISTS), + writtenValue: "original value", + }, + }, + { + desc: "When key does not exist, redis.Nil is returned. Other errors should be interpreted as a server side error. Expect error.", + in: testInput{ + redisSentinelClient: FakeRedisClient{ + Success: true, + StoredData: map[string]string{}, + ServerError: errors.New("A Redis client side error"), + }, + key: "someKey", + valueToStore: "someValue", + ttl: 10, + }, + expected: testExpectedValues{ + redisClientErr: errors.New("A Redis client side error"), + }, + }, + { + desc: "In Redis, a zero ttl value means no expiration. Expect value to be successfully set", + in: testInput{ + redisSentinelClient: FakeRedisClient{ + StoredData: map[string]string{}, + Success: true, + ServerError: redis.Nil, + }, + key: "defaultKey", + valueToStore: "aValue", + ttl: 0, + }, + expected: testExpectedValues{ + writtenValue: "aValue", + }, + }, + { + desc: "RedisBackend.Put() successful, no need to set defaultTTL because ttl is greater than zero", + in: testInput{ + redisSentinelClient: FakeRedisClient{ + StoredData: map[string]string{}, + Success: true, + ServerError: redis.Nil, + }, + key: "defaultKey", + valueToStore: "aValue", + ttl: 1, + }, + expected: testExpectedValues{ + writtenValue: "aValue", + }, + }, + } + + for _, tt := range testCases { + // Assign redis backend client + redisSentinelBackend.client = tt.in.redisSentinelClient + + // Run test + actualErr := redisSentinelBackend.Put(context.Background(), tt.in.key, tt.in.valueToStore, tt.in.ttl) + + // Assertions + assert.Equal(t, tt.expected.redisClientErr, actualErr, tt.desc) + + // Put error + assert.Equal(t, tt.expected.redisClientErr, actualErr, tt.desc) + + if actualErr == nil || actualErr == utils.NewPBCError(utils.RECORD_EXISTS) { + // Either a value was inserted successfully or the record already existed. + // Assert data in the backend + storage, ok := tt.in.redisSentinelClient.(FakeRedisClient) + assert.True(t, ok, tt.desc) + assert.Equal(t, tt.expected.writtenValue, storage.StoredData[tt.in.key], tt.desc) + } + } +} diff --git a/config.yaml b/config.yaml index 729d419b..635a87cd 100644 --- a/config.yaml +++ b/config.yaml @@ -22,7 +22,7 @@ backend: # memcache: # config_host: "" # Configuration endpoint for auto discovery. Replaced at docker build. # poll_interval_seconds: 30 # Node change polling interval when auto discovery is used -# hosts: "10.0.0.1:11211" # List of nodes when not using auto discovery. Can also use an array for multiple hosts. +# hosts: "10.0.0.1:11211" # List of nodes when not using auto discovery. Can also use an array for multiple hosts. # redis: # host: "127.0.0.1" # port: 6379 @@ -32,6 +32,15 @@ backend: # tls: # enabled: false # insecure_skip_verify: false +# redis_sentinel: +# sentinel_addrs: [ "127.0.0.1:26379", "127.0.0.1:26380", "127.0.0.1:26381" ] +# master_name: "mymaster" +# password: "" +# db: 1 +# expiration: 10 # in Minutes +# tls: +# enabled: false +# insecure_skip_verify: false # ignite: # scheme: "http" # host: "127.0.0.1" diff --git a/config/backends.go b/config/backends.go index 2f5c8694..122617a6 100644 --- a/config/backends.go +++ b/config/backends.go @@ -8,12 +8,13 @@ import ( ) type Backend struct { - Type BackendType `mapstructure:"type"` - Aerospike Aerospike `mapstructure:"aerospike"` - Cassandra Cassandra `mapstructure:"cassandra"` - Memcache Memcache `mapstructure:"memcache"` - Redis Redis `mapstructure:"redis"` - Ignite Ignite `mapstructure:"ignite"` + Type BackendType `mapstructure:"type"` + Aerospike Aerospike `mapstructure:"aerospike"` + Cassandra Cassandra `mapstructure:"cassandra"` + Memcache Memcache `mapstructure:"memcache"` + Redis Redis `mapstructure:"redis"` + RedisSentinel RedisSentinel `mapstructure:"redis_sentinel"` + Ignite Ignite `mapstructure:"ignite"` } func (cfg *Backend) validateAndLog() error { @@ -28,25 +29,27 @@ func (cfg *Backend) validateAndLog() error { return cfg.Memcache.validateAndLog() case BackendRedis: return cfg.Redis.validateAndLog() + case BackendRedisSentinel: + return cfg.RedisSentinel.validateAndLog() case BackendIgnite: return cfg.Ignite.validateAndLog() case BackendMemory: return nil default: - return fmt.Errorf(`invalid config.backend.type: %s. It must be "aerospike", "cassandra", "memcache", "redis", "ignite", or "memory".`, cfg.Type) + return fmt.Errorf(`invalid config.backend.type: %s. It must be "%s", "%s", "%s", "%s", "%s", "%s", or "%s"`, cfg.Type, BackendAerospike, BackendCassandra, BackendMemcache, BackendRedis, BackendRedisSentinel, BackendIgnite, BackendMemory) } - return nil } type BackendType string const ( - BackendAerospike BackendType = "aerospike" - BackendCassandra BackendType = "cassandra" - BackendMemcache BackendType = "memcache" - BackendMemory BackendType = "memory" - BackendRedis BackendType = "redis" - BackendIgnite BackendType = "ignite" + BackendAerospike BackendType = "aerospike" + BackendCassandra BackendType = "cassandra" + BackendMemcache BackendType = "memcache" + BackendMemory BackendType = "memory" + BackendRedis BackendType = "redis" + BackendRedisSentinel BackendType = "redis_sentinel" + BackendIgnite BackendType = "ignite" ) type Aerospike struct { @@ -176,6 +179,23 @@ func (cfg *Redis) validateAndLog() error { return nil } +type RedisSentinel struct { + SentinelAddrs []string `mapstructure:"sentinel_addrs"` + MasterName string `mapstructure:"master_name"` + Password string `mapstructure:"password"` + Db int `mapstructure:"db"` + TLS RedisTLS `mapstructure:"tls"` +} + +func (cfg *RedisSentinel) validateAndLog() error { + log.Infof("config.backend.redis-sentinel.sentinel_addrs: %s", cfg.SentinelAddrs) + log.Infof("config.backend.redis-sentinel.master_name: %s", cfg.MasterName) + log.Infof("config.backend.redis-sentinel.db: %d", cfg.Db) + log.Infof("config.backend.redis-sentinel.tls.enabled: %t", cfg.TLS.Enabled) + log.Infof("config.backend.redis-sentinel.tls.insecure_skip_verify: %t", cfg.TLS.InsecureSkipVerify) + return nil +} + type Ignite struct { Scheme string `mapstructure:"scheme"` Host string `mapstructure:"host"` diff --git a/config/configtest/sample_full_config.yaml b/config/configtest/sample_full_config.yaml index 3c40fbb7..c51726f8 100644 --- a/config/configtest/sample_full_config.yaml +++ b/config/configtest/sample_full_config.yaml @@ -37,6 +37,15 @@ backend: tls: enabled: false insecure_skip_verify: false + redis_sentinel: + sentinel_addrs: [ "127.0.0.1:26379", "127.0.0.1:26380", "127.0.0.1:26381" ] + master_name: "mymaster" + password: "" + db: 1 + expiration: 10 # in Minutes + tls: + enabled: false + insecure_skip_verify: false ignite: scheme: "http" host: "127.0.0.1" diff --git a/go.mod b/go.mod index 04c26750..0a9087a0 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/prometheus/client_golang v1.12.2 github.com/prometheus/client_model v0.2.0 github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 + github.com/redis/go-redis/v9 v9.4.0 github.com/rs/cors v1.8.2 github.com/sirupsen/logrus v1.6.0 github.com/spf13/viper v1.11.0 @@ -24,7 +25,7 @@ require ( require ( github.com/beorn7/perks v1.0.1 // indirect github.com/bitly/go-hostpool v0.1.0 // indirect - github.com/cespare/xxhash/v2 v2.1.2 // indirect + github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/fsnotify/fsnotify v1.5.1 // indirect diff --git a/go.sum b/go.sum index b5f8ea8f..9de446e5 100644 --- a/go.sum +++ b/go.sum @@ -54,10 +54,13 @@ github.com/bitly/go-hostpool v0.1.0 h1:XKmsF6k5el6xHG3WPJ8U0Ku/ye7njX7W81Ng7O2io github.com/bitly/go-hostpool v0.1.0/go.mod h1:4gOCgp6+NZnVqlKyZ/iBZFTAJKembaVENUpMkpg42fw= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= +github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs= +github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/cespare/xxhash/v2 v2.1.2 h1:YRXhKfTDauu4ajMg1TPgFO5jnlC2HCbmLXMcTG5cbYE= github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= +github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= @@ -263,6 +266,8 @@ github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1 github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5XpJzTSTfLsJV/mx9Q9g7kxmchpfZyxgzM= github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= +github.com/redis/go-redis/v9 v9.4.0 h1:Yzoz33UZw9I/mFhx4MNrB6Fk+XHO1VukNcCa1+lwyKk= +github.com/redis/go-redis/v9 v9.4.0/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rs/cors v1.8.2 h1:KCooALfAYGs415Cwu5ABvv9n9509fSiG5SQJn/AQo4U= github.com/rs/cors v1.8.2/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= @@ -629,8 +634,6 @@ google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGj google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw= -google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= From 9bca40c8781e79fb80837a9b256c2ec1c76fae98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Millet?= Date: Fri, 26 Apr 2024 11:59:09 +0200 Subject: [PATCH 2/4] Fix configuration test and remove unused expiration field --- README.md | 1 - config.yaml | 1 - config/config_test.go | 10 ++++++++++ config/configtest/sample_full_config.yaml | 1 - 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 27872a09..eea1accb 100644 --- a/README.md +++ b/README.md @@ -264,7 +264,6 @@ Prebid Cache makes use of a Redis Go client compatible with Redis Sentinel. Full | master_name | string | Name of the Sentinel master (as declared in the `sentinel monitor` line of your Sentinel configurations) | | password | string | Redis password | | db | integer | Database to be selected after connecting to the server | -| expiration | integer | Availability in the Redis system in Minutes | | tls | field | Subfields:
`enabled`: whether or not pass the InsecureSkipVerify value to the Redis client's TLS config
`insecure_skip_verify`: In Redis, InsecureSkipVerify controls whether a client verifies the server's certificate chain and host name. If InsecureSkipVerify is true, crypto/t | diff --git a/config.yaml b/config.yaml index 635a87cd..93a815b9 100644 --- a/config.yaml +++ b/config.yaml @@ -37,7 +37,6 @@ backend: # master_name: "mymaster" # password: "" # db: 1 -# expiration: 10 # in Minutes # tls: # enabled: false # insecure_skip_verify: false diff --git a/config/config_test.go b/config/config_test.go index 8ab6da94..a6957c10 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1277,6 +1277,16 @@ func getExpectedFullConfigForTestFile() Configuration { InsecureSkipVerify: false, }, }, + RedisSentinel: RedisSentinel{ + SentinelAddrs: []string{"127.0.0.1:26379", "127.0.0.1:26380", "127.0.0.1:26381"}, + MasterName: "mymaster", + Password: "", + Db: 1, + TLS: RedisTLS{ + Enabled: false, + InsecureSkipVerify: false, + }, + }, Ignite: Ignite{ Scheme: "http", Host: "127.0.0.1", diff --git a/config/configtest/sample_full_config.yaml b/config/configtest/sample_full_config.yaml index c51726f8..3fe41b87 100644 --- a/config/configtest/sample_full_config.yaml +++ b/config/configtest/sample_full_config.yaml @@ -42,7 +42,6 @@ backend: master_name: "mymaster" password: "" db: 1 - expiration: 10 # in Minutes tls: enabled: false insecure_skip_verify: false From 742e42487477749fb33642f046ea580ee32f1672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Millet?= Date: Tue, 3 Sep 2024 14:48:52 +0200 Subject: [PATCH 3/4] Fix redis_sentinel configuration reading This fix solves these issues: * The log messages were incorrectly using `redis-sentinel` instead of `redis_sentinel` when dumping the configuration in the logs, which was misleading. * The configuration entries were not provided default values, preventing Viper from reading the associated environment variables * The default configuration content was not tested --- config/backends.go | 10 ++++---- config/config.go | 6 +++++ config/config_test.go | 55 +++++++++++++++++++++++-------------------- 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/config/backends.go b/config/backends.go index 122617a6..a29d6a27 100644 --- a/config/backends.go +++ b/config/backends.go @@ -188,11 +188,11 @@ type RedisSentinel struct { } func (cfg *RedisSentinel) validateAndLog() error { - log.Infof("config.backend.redis-sentinel.sentinel_addrs: %s", cfg.SentinelAddrs) - log.Infof("config.backend.redis-sentinel.master_name: %s", cfg.MasterName) - log.Infof("config.backend.redis-sentinel.db: %d", cfg.Db) - log.Infof("config.backend.redis-sentinel.tls.enabled: %t", cfg.TLS.Enabled) - log.Infof("config.backend.redis-sentinel.tls.insecure_skip_verify: %t", cfg.TLS.InsecureSkipVerify) + log.Infof("config.backend.redis_sentinel.sentinel_addrs: %s", cfg.SentinelAddrs) + log.Infof("config.backend.redis_sentinel.master_name: %s", cfg.MasterName) + log.Infof("config.backend.redis_sentinel.db: %d", cfg.Db) + log.Infof("config.backend.redis_sentinel.tls.enabled: %t", cfg.TLS.Enabled) + log.Infof("config.backend.redis_sentinel.tls.insecure_skip_verify: %t", cfg.TLS.InsecureSkipVerify) return nil } diff --git a/config/config.go b/config/config.go index 59e02080..02bc949f 100644 --- a/config/config.go +++ b/config/config.go @@ -69,6 +69,12 @@ func setConfigDefaults(v *viper.Viper) { v.SetDefault("backend.redis.expiration", utils.REDIS_DEFAULT_EXPIRATION_MINUTES) v.SetDefault("backend.redis.tls.enabled", false) v.SetDefault("backend.redis.tls.insecure_skip_verify", false) + v.SetDefault("backend.redis_sentinel.db", 0) + v.SetDefault("backend.redis_sentinel.master_name", "") + v.SetDefault("backend.redis_sentinel.password", "") + v.SetDefault("backend.redis_sentinel.sentinel_addrs", []string{}) + v.SetDefault("backend.redis_sentinel.tls.enabled", false) + v.SetDefault("backend.redis_sentinel.tls.insecure_skip_verify", false) v.SetDefault("backend.ignite.scheme", "") v.SetDefault("backend.ignite.host", "") v.SetDefault("backend.ignite.port", 0) diff --git a/config/config_test.go b/config/config_test.go index a6957c10..8d75a9ba 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -290,7 +290,7 @@ func TestCheckMetricsEnabled(t *testing.T) { }, } - //Standard elements of the config.Metrics object are set so test cases only modify what's relevant to them + // Standard elements of the config.Metrics object are set so test cases only modify what's relevant to them cfg := &Metrics{ Influx: InfluxMetrics{ Host: "http://fakeurl.com", @@ -307,7 +307,7 @@ func TestCheckMetricsEnabled(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them hook := testLogrus.NewGlobal() - //substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes + // substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes defer func() { logrus.StandardLogger().ExitFunc = nil }() var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } @@ -321,7 +321,7 @@ func TestCheckMetricsEnabled(t *testing.T) { cfg.Influx.Enabled = tc.influxEnabled cfg.Prometheus.Enabled = tc.prometheusEnabled - //run test + // run test cfg.validateAndLog() // Assert logrus expected entries @@ -337,7 +337,7 @@ func TestCheckMetricsEnabled(t *testing.T) { // Assert log.Fatalf() was called or not assert.Equal(t, tc.expectedError, fatal, "Test case %d failed.", i+1) - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -447,7 +447,7 @@ func TestEnabledFlagGetsModified(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them hook := testLogrus.NewGlobal() - //substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes + // substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} @@ -468,14 +468,14 @@ func TestEnabledFlagGetsModified(t *testing.T) { }, } - //run test + // run test metricsCfg.validateAndLog() // Assert `Enabled` flags value assert.Equal(t, tc.out.expectedInfluxEnabled, metricsCfg.Influx.Enabled, "Test case %d failed. `cfg.Influx.Enabled` carries wrong value.", i+1) assert.Equal(t, tc.out.expectedprometheusEnabled, metricsCfg.Prometheus.Enabled, "Test case %d failed. `cfg.Prometheus.Enabled` carries wrong value.", i+1) - //Reset log after every test + // Reset log after every test hook.Reset() } } @@ -493,7 +493,7 @@ func TestInfluxValidateAndLog(t *testing.T) { description string // In influxConfig *InfluxMetrics - //out + // out expectError bool expectedLogInfo []logComponents } @@ -598,7 +598,7 @@ func TestInfluxValidateAndLog(t *testing.T) { }, } - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } @@ -607,7 +607,7 @@ func TestInfluxValidateAndLog(t *testing.T) { // Reset the fatal flag to false every test fatal = false - //run test + // run test tc.influxConfig.validateAndLog() // Assert logrus expected entries @@ -623,7 +623,7 @@ func TestInfluxValidateAndLog(t *testing.T) { // Assert log.Fatalf() was called or not assert.Equal(t, tc.expectError, fatal) - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -639,7 +639,7 @@ func TestPrometheusValidateAndLog(t *testing.T) { description string // In prometheusConfig *PrometheusMetrics - //out + // out expectError bool expectedLogInfo []logComponents } @@ -768,7 +768,7 @@ func TestPrometheusValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them hook := testLogrus.NewGlobal() - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } @@ -777,7 +777,7 @@ func TestPrometheusValidateAndLog(t *testing.T) { // Reset the fatal flag to false every test fatal = false - //run test + // run test tc.prometheusConfig.validateAndLog() // Assert logrus expected entries @@ -793,7 +793,7 @@ func TestPrometheusValidateAndLog(t *testing.T) { // Assert log.Fatalf() was called or not assert.Equal(t, tc.expectError, fatal) - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -869,7 +869,7 @@ func TestRequestLimitsValidateAndLog(t *testing.T) { }, } - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } @@ -897,7 +897,7 @@ func TestRequestLimitsValidateAndLog(t *testing.T) { } assert.Len(t, tc.expectedLogInfo, logEntryCount, tc.description) - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -953,7 +953,7 @@ func TestCompressionValidateAndLog(t *testing.T) { }, } - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} @@ -969,7 +969,7 @@ func TestCompressionValidateAndLog(t *testing.T) { } } - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -1029,12 +1029,12 @@ func TestNewConfigFromFile(t *testing.T) { }, } - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} for _, tc := range testCases { - //run test + // run test actualCfg := NewConfig(tc.inConfigFileName) // Assert logrus expected entries @@ -1047,7 +1047,7 @@ func TestNewConfigFromFile(t *testing.T) { assert.Equal(t, tc.expectedConfig, actualCfg, "Expected Configuration instance does not match. Test desc:%s", tc.description) - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -1056,7 +1056,7 @@ func TestNewConfigFromFile(t *testing.T) { func TestConfigurationValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them hook := testLogrus.NewGlobal() - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} @@ -1094,7 +1094,7 @@ func TestConfigurationValidateAndLog(t *testing.T) { } } - //Reset log + // Reset log hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -1137,7 +1137,7 @@ func TestRoutesValidateAndLog(t *testing.T) { }, } - //substitute logger exit function so execution doesn't get interrupted + // substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} @@ -1153,7 +1153,7 @@ func TestRoutesValidateAndLog(t *testing.T) { } } - //Reset log after every test and assert successful reset + // Reset log after every test and assert successful reset hook.Reset() assert.Nil(t, hook.LastEntry()) } @@ -1204,6 +1204,9 @@ func getExpectedDefaultConfig() Configuration { Redis: Redis{ ExpirationMinutes: utils.REDIS_DEFAULT_EXPIRATION_MINUTES, }, + RedisSentinel: RedisSentinel{ + SentinelAddrs: []string{}, + }, Ignite: Ignite{ Headers: map[string]string{}, }, From 541c2371f4a79efaa4db96ece91bed4cad377f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Millet?= Date: Tue, 3 Sep 2024 16:28:57 +0200 Subject: [PATCH 4/4] Fix redis_sentinel password usage When using a password in Redis Sentinel, it should be provided at both levels --- backends/redis-sentinel.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/backends/redis-sentinel.go b/backends/redis-sentinel.go index 025eb51d..b26fed48 100644 --- a/backends/redis-sentinel.go +++ b/backends/redis-sentinel.go @@ -46,10 +46,11 @@ type RedisSentinelBackend struct { // NewRedisSentinelBackend initializes the Redis Sentinel client and pings to make sure connection was successful func NewRedisSentinelBackend(cfg config.RedisSentinel, ctx context.Context) *RedisSentinelBackend { options := &redis.FailoverOptions{ - MasterName: cfg.MasterName, - SentinelAddrs: cfg.SentinelAddrs, - Password: cfg.Password, - DB: cfg.Db, + MasterName: cfg.MasterName, + SentinelAddrs: cfg.SentinelAddrs, + SentinelPassword: cfg.Password, + Password: cfg.Password, + DB: cfg.Db, } if cfg.TLS.Enabled {