From 0cfb9c6da0c30feb3aceefd0e8537ad4a9927b0f Mon Sep 17 00:00:00 2001 From: Dmitry Kartsev Date: Tue, 9 Aug 2022 23:57:13 +0300 Subject: [PATCH] =?UTF-8?q?adding=20IdleTimeout=20with=20the=20redis-conne?= =?UTF-8?q?ction-idle-timeout=20flag,=20to=20ke=E2=80=A6=20(#1691)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * adding IdleTimeout with the redis-connection-idle-timeout flag, to keep redis connections in valid state, when Redis option is set * docs update - add redis idle timeout configurations * changelog update for #1691 fix --- CHANGELOG.md | 2 ++ docs/docs/configuration/overview.md | 3 ++- docs/docs/configuration/sessions.md | 4 ++++ pkg/apis/options/options.go | 2 +- pkg/apis/options/sessions.go | 1 + pkg/sessions/redis/redis_store.go | 10 +++++++--- 6 files changed, 17 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 028e0931..f016f86f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ ## Breaking Changes ## Changes since v7.3.0 +- [#1691](https://github.com/oauth2-proxy/oauth2-proxy/pull/1691) Fix Redis IdleTimeout when Redis timeout option is set to non-zero (@dimss) + - [#1669](https://github.com/oauth2-proxy/oauth2-proxy/pull/1699) Fix method deprecated error in lint (@t-katsumura) - [#1709](https://github.com/oauth2-proxy/oauth2-proxy/pull/1709) Show an alert message when basic auth credentials are invalid (@aiciobanu) diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index 00c59ca5..bf36bd59 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -167,6 +167,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--redis-sentinel-connection-urls` | string \| list | List of Redis sentinel connection URLs (e.g. `redis://HOST[:PORT]`). Used in conjunction with `--redis-use-sentinel` | | | `--redis-use-cluster` | bool | Connect to redis cluster. Must set `--redis-cluster-connection-urls` to use this feature | false | | `--redis-use-sentinel` | bool | Connect to redis via sentinels. Must set `--redis-sentinel-master-name` and `--redis-sentinel-connection-urls` to use this feature | false | +| `--redis-connection-idle-timeout` | int | Redis connection idle timeout seconds. If Redis [timeout](https://redis.io/docs/reference/clients/#client-timeouts) option is set to non-zero, the `--redis-connection-idle-timeout` must be less than Redis timeout option. Exmpale: if either redis.conf includes `timeout 15` or using `CONFIG SET timeout 15` the `--redis-connection-idle-timeout` must be at least `--redis-connection-idle-timeout=14` | 0 | | `--request-id-header` | string | Request header to use as the request ID in logging | X-Request-Id | | `--request-logging` | bool | Log requests | true | | `--request-logging-format` | string | Template for request log lines | see [Logging Configuration](#logging-configuration) | @@ -594,4 +595,4 @@ http: :::note If you set up your OAuth2 provider to rotate your client secret, you can use the `client-secret-file` option to reload the secret when it is updated. -::: +::: \ No newline at end of file diff --git a/docs/docs/configuration/sessions.md b/docs/docs/configuration/sessions.md index 174164ba..8a4b640c 100644 --- a/docs/docs/configuration/sessions.md +++ b/docs/docs/configuration/sessions.md @@ -65,3 +65,7 @@ Redis Cluster is available to be the backend store as well. To leverage it, you `--redis-use-cluster=true` flag, and configure the flags `--redis-cluster-connection-urls` appropriately. Note that flags `--redis-use-sentinel=true` and `--redis-use-cluster=true` are mutually exclusive. + +Note, if Redis timeout option is set to non-zero, the `--redis-connection-idle-timeout` +must be less than [Redis timeout option](https://redis.io/docs/reference/clients/#client-timeouts). For example: if either redis.conf includes +`timeout 15` or using `CONFIG SET timeout 15` the `--redis-connection-idle-timeout` must be at least `--redis-connection-idle-timeout=14` \ No newline at end of file diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 8641a430..eac42ead 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -143,7 +143,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.StringSlice("redis-sentinel-connection-urls", []string{}, "List of Redis sentinel connection URLs (eg redis://HOST[:PORT]). Used in conjunction with --redis-use-sentinel") flagSet.Bool("redis-use-cluster", false, "Connect to redis cluster. Must set --redis-cluster-connection-urls to use this feature") flagSet.StringSlice("redis-cluster-connection-urls", []string{}, "List of Redis cluster connection URLs (eg redis://HOST[:PORT]). Used in conjunction with --redis-use-cluster") - + flagSet.Int("redis-connection-idle-timeout", 0, "Redis connection idle timeout seconds, if Redis timeout option is non-zero, the --redis-connection-idle-timeout must be less then Redis timeout option") flagSet.String("signature-key", "", "GAP-Signature request signature key (algorithm:secretkey)") flagSet.Bool("gcp-healthchecks", false, "Enable GCP/GKE healthcheck endpoints") diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go index 9b36d79e..188f0e45 100644 --- a/pkg/apis/options/sessions.go +++ b/pkg/apis/options/sessions.go @@ -32,6 +32,7 @@ type RedisStoreOptions struct { ClusterConnectionURLs []string `flag:"redis-cluster-connection-urls" cfg:"redis_cluster_connection_urls"` CAPath string `flag:"redis-ca-path" cfg:"redis_ca_path"` InsecureSkipTLSVerify bool `flag:"redis-insecure-skip-tls-verify" cfg:"redis_insecure_skip_tls_verify"` + IdleTimeout int `flag:"redis-connection-idle-timeout" cfg:"redis_connection_idle_timeout"` } func sessionOptionsDefaults() SessionOptions { diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index f1c798fa..1243c7c5 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -104,6 +104,7 @@ func buildSentinelClient(opts options.RedisStoreOptions) (Client, error) { SentinelPassword: opts.SentinelPassword, Password: opts.Password, TLSConfig: opt.TLSConfig, + IdleTimeout: time.Duration(opts.IdleTimeout) * time.Second, }) return newClient(client), nil } @@ -120,9 +121,10 @@ func buildClusterClient(opts options.RedisStoreOptions) (Client, error) { } client := redis.NewClusterClient(&redis.ClusterOptions{ - Addrs: addrs, - Password: opts.Password, - TLSConfig: opt.TLSConfig, + Addrs: addrs, + Password: opts.Password, + TLSConfig: opt.TLSConfig, + IdleTimeout: time.Duration(opts.IdleTimeout) * time.Second, }) return newClusterClient(client), nil } @@ -143,6 +145,8 @@ func buildStandaloneClient(opts options.RedisStoreOptions) (Client, error) { return nil, err } + opt.IdleTimeout = time.Duration(opts.IdleTimeout) * time.Second + client := redis.NewClient(opt) return newClient(client), nil }