From 1ff6a02b8ef777bf20d0ba2dda1c64b77fa425b3 Mon Sep 17 00:00:00 2001 From: Daniel Butler <16944132+dbut023@users.noreply.github.com> Date: Thu, 15 Sep 2022 16:35:47 +0100 Subject: [PATCH 1/3] Updated TLS support to the redis client based on PR #5312. Closes issue #5310 Signed-off-by: Daniel Butler <16944132+dbut023@users.noreply.github.com> --- pkg/cacheutil/redis_client.go | 43 +++++++++++++++- pkg/cacheutil/redis_client_test.go | 83 ++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/pkg/cacheutil/redis_client.go b/pkg/cacheutil/redis_client.go index 962c57712a..a12c8d5e83 100644 --- a/pkg/cacheutil/redis_client.go +++ b/pkg/cacheutil/redis_client.go @@ -20,6 +20,7 @@ import ( "github.com/thanos-io/thanos/pkg/extprom" "github.com/thanos-io/thanos/pkg/gate" + thanos_tls "github.com/thanos-io/thanos/pkg/tls" ) var ( @@ -35,9 +36,25 @@ var ( GetMultiBatchSize: 100, MaxSetMultiConcurrency: 100, SetMultiBatchSize: 100, + TLSEnabled: false, + TLSConfig: TLSConfig{}, } ) +// TLSConfig configures TLS connections. +type TLSConfig struct { + // The CA cert to use for the targets. + CAFile string `yaml:"ca_file"` + // The client cert file for the targets. + CertFile string `yaml:"cert_file"` + // The client key file for the targets. + KeyFile string `yaml:"key_file"` + // Used to verify the hostname for the targets. See https://tools.ietf.org/html/rfc4366#section-3.1 + ServerName string `yaml:"server_name"` + // Disable target certificate validation. + InsecureSkipVerify bool `yaml:"insecure_skip_verify"` +} + // RedisClientConfig is the config accepted by RedisClient. type RedisClientConfig struct { // Addr specifies the addresses of redis server. @@ -94,12 +111,19 @@ type RedisClientConfig struct { // SetMultiBatchSize specifies the maximum size per batch for pipeline set. SetMultiBatchSize int `yaml:"set_multi_batch_size"` + + // TLSEnabled enable tls for redis connection. + TLSEnabled bool `yaml:"tls_enabled"` + + // TLSConfig to use to connect to the redis server. + TLSConfig TLSConfig `yaml:"tls_config"` } func (c *RedisClientConfig) validate() error { if c.Addr == "" { return errors.New("no redis addr provided") } + return nil } @@ -136,7 +160,8 @@ func NewRedisClientWithConfig(logger log.Logger, name string, config RedisClient if err := config.validate(); err != nil { return nil, err } - redisClient := redis.NewClient(&redis.Options{ + + opts := &redis.Options{ Addr: config.Addr, Username: config.Username, Password: config.Password, @@ -147,8 +172,22 @@ func NewRedisClientWithConfig(logger log.Logger, name string, config RedisClient MinIdleConns: config.MinIdleConns, MaxConnAge: config.MaxConnAge, IdleTimeout: config.IdleTimeout, - }) + } + + if config.TLSEnabled { + tlsConfig := config.TLSConfig + + tlsClientConfig, err := thanos_tls.NewClientConfig(logger, tlsConfig.CertFile, tlsConfig.KeyFile, + tlsConfig.CAFile, tlsConfig.ServerName, tlsConfig.InsecureSkipVerify) + + if err != nil { + return nil, err + } + + opts.TLSConfig = tlsClientConfig + } + redisClient := redis.NewClient(opts) if reg != nil { reg = prometheus.WrapRegistererWith(prometheus.Labels{"name": name}, reg) } diff --git a/pkg/cacheutil/redis_client_test.go b/pkg/cacheutil/redis_client_test.go index 1fb611c019..8dfc1cafdc 100644 --- a/pkg/cacheutil/redis_client_test.go +++ b/pkg/cacheutil/redis_client_test.go @@ -136,3 +136,86 @@ func TestRedisClient(t *testing.T) { }) } } + +func TestValidateRedisConfig(t *testing.T) { + tests := []struct { + name string + config func() RedisClientConfig + expect_err bool // func(*testing.T, interface{}, error) + }{ + { + name: "simpleConfig", + config: func() RedisClientConfig { + cfg := DefaultRedisClientConfig + cfg.Addr = "127.0.0.1:6789" + cfg.Username = "user" + cfg.Password = "1234" + return cfg + }, + expect_err: false, + }, + { + name: "tlsConfigDefaults", + config: func() RedisClientConfig { + cfg := DefaultRedisClientConfig + cfg.Addr = "127.0.0.1:6789" + cfg.Username = "user" + cfg.Password = "1234" + cfg.TLSEnabled = true + return cfg + }, + expect_err: false, + }, + { + name: "tlsClientCertConfig", + config: func() RedisClientConfig { + cfg := DefaultRedisClientConfig + cfg.Addr = "127.0.0.1:6789" + cfg.Username = "user" + cfg.Password = "1234" + cfg.TLSEnabled = true + cfg.TLSConfig = TLSConfig{ + CertFile: "cert/client.pem", + KeyFile: "cert/client.key", + } + return cfg + }, + expect_err: false, + }, + { + name: "tlsInvalidClientCertConfig", + config: func() RedisClientConfig { + cfg := DefaultRedisClientConfig + cfg.Addr = "127.0.0.1:6789" + cfg.Username = "user" + cfg.Password = "1234" + cfg.TLSEnabled = true + cfg.TLSConfig = TLSConfig{ + CertFile: "cert/client.pem", + } + return cfg + }, + expect_err: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := tt.config() + + logger := log.NewLogfmtLogger(os.Stderr) + reg := prometheus.NewRegistry() + val, err := NewRedisClientWithConfig(logger, tt.name, cfg, reg) + if val != nil { + defer val.Stop() + } + + if tt.expect_err { + testutil.NotOk(t, err, val) + } else { + testutil.Ok(t, err, val) + } + }) + } + +} From 7e3ff487f1eef7065c487444bbed02ad2695ca3c Mon Sep 17 00:00:00 2001 From: Daniel Butler <16944132+dbut023@users.noreply.github.com> Date: Mon, 12 Sep 2022 16:58:18 +0100 Subject: [PATCH 2/3] Changelog Signed-off-by: Daniel Butler <16944132+dbut023@users.noreply.github.com> --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f559818a..77890eb787 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#5658](https://github.com/thanos-io/thanos/pull/5658) Query Frontend: Introduce new optional parameters (`query-range.min-split-interval`, `query-range.max-split-interval`, `query-range.horizontal-shards`) to implement more dynamic horizontal query splitting. - [#5721](https://github.com/thanos-io/thanos/pull/5721) Store: Add metric `thanos_bucket_store_empty_postings_total` for number of empty postings when fetching series. - [#5723](https://github.com/thanos-io/thanos/pull/5723) Compactor: Support disable block viewer UI. +- [#5674](https://github.com/thanos-io/thanos/pull/5674) Query Frontend/Store: Add support connecting to redis using TLS. ### Changed From e8e8e7b9eede81c9df1f50f7ffcc3748801de863 Mon Sep 17 00:00:00 2001 From: Daniel Butler <16944132+dbut023@users.noreply.github.com> Date: Thu, 15 Sep 2022 16:22:57 +0100 Subject: [PATCH 3/3] documentation Signed-off-by: Daniel Butler <16944132+dbut023@users.noreply.github.com> --- docs/components/query-frontend.md | 7 +++++++ docs/components/store.md | 18 ++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/components/query-frontend.md b/docs/components/query-frontend.md index bc150cb40f..44be77ec31 100644 --- a/docs/components/query-frontend.md +++ b/docs/components/query-frontend.md @@ -125,6 +125,13 @@ config: get_multi_batch_size: 100 max_set_multi_concurrency: 100 set_multi_batch_size: 100 + tls_enabled: false + tls_config: + ca_file: "" + cert_file: "" + key_file: "" + server_name: "" + insecure_skip_verify: false expiration: 24h0m0s ``` diff --git a/docs/components/store.md b/docs/components/store.md index 3071e7b928..2f4e6a5bbb 100644 --- a/docs/components/store.md +++ b/docs/components/store.md @@ -328,6 +328,13 @@ config: get_multi_batch_size: 100 max_set_multi_concurrency: 100 set_multi_batch_size: 100 + tls_enabled: false + tls_config: + ca_file: "" + cert_file: "" + key_file: "" + server_name: "" + insecure_skip_verify: false ``` The **required** settings are: @@ -336,8 +343,8 @@ The **required** settings are: While the remaining settings are **optional**: -- `username`: the username to connect redis, only redis 6.0 and grater need this field. -- `password`: the password to connect redis. +- `username`: the username to connect to redis, only redis 6.0 and grater need this field. +- `password`: the password to connect to redis. - `db`: the database to be selected after connecting to the server. - `dial_timeout`: the redis dial timeout. - `read_timeout`: the redis read timeout. @@ -350,6 +357,13 @@ While the remaining settings are **optional**: - `get_multi_batch_size`: specifies the maximum size per batch for mget. - `max_set_multi_concurrency`: specifies the maximum number of concurrent SetMulti() operations. - `set_multi_batch_size`: specifies the maximum size per batch for pipeline set. +- `tls_enabled`: enables the use of TLS to connect to redis +- `tls_config`: TLS connection configuration: + - `ca_file`: path to Root CA certificate file to use + - `cert_file`: path to Client Certificate file to use + - `key_file`: path to the Key file for cert_file (NOTE: Both this and `cert_file` must be set if used) + - `servername`: Override the server name used to validate the server certificate + - `insecure_skip_verify`: Disable certificate verification ## Caching Bucket