From c7bfbdecef0a724b5bc752607edd4257928ddd40 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Sun, 5 Apr 2020 00:12:38 +0900 Subject: [PATCH] Implement graceful shutdown and propagate request context (#468) * feature: Implement graceful shutdown Propagate the request context to the Redis client. It is possible to propagate a context cancel to Redis client if the connection is closed by the HTTP client. The redis.Cmdable cannot use WithContext, so added the Client interface to handle redis.Client and redis.ClusterClient transparently. Added handling of Unix signals to http server. Upgrade go-redis/redis to v7. * Update dependencies - Upgrade golang/x/* and google-api-go - Migrate fsnotify import from gopkg.in to github.com - Replace bmizerany/assert with stretchr/testify/assert * add doc for wrapper interface * Update CHANGELOG.md * fix: upgrade fsnotify to v1.4.9 * fix: remove unnessary logging * fix: wait until all connections have been closed * refactor: move chan to main for testing * add assert to check if stop chan is empty * add an idiomatic for sync.WaitGroup with timeout --- CHANGELOG.md | 1 + go.mod | 13 ++++--- go.sum | 32 +++++++++-------- http.go | 42 ++++++++++++++-------- http_test.go | 30 ++++++++++++++++ main.go | 10 ++++++ pkg/sessions/redis/client.go | 59 +++++++++++++++++++++++++++++++ pkg/sessions/redis/redis_store.go | 37 ++++++++++--------- providers/keycloak_test.go | 3 +- providers/oidc_test.go | 2 +- watcher.go | 3 +- 11 files changed, 177 insertions(+), 55 deletions(-) create mode 100644 pkg/sessions/redis/client.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 946c0717..c383f8d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ## Changes since v5.1.0 +- [#468](https://github.com/oauth2-proxy/oauth2-proxy/pull/468) Implement graceful shutdown and propagate request context (@johejo) - [#464](https://github.com/oauth2-proxy/oauth2-proxy/pull/464) Migrate to oauth2-proxy/oauth2-proxy (@JoelSpeed) - Project renamed from `pusher/oauth2_proxy` to `oauth2-proxy` - Move Go import path from `github.com/pusher/oauth2_proxy` to `github.com/oauth2-proxy/oauth2-proxy` diff --git a/go.mod b/go.mod index 6fa871c3..2170dc9b 100644 --- a/go.mod +++ b/go.mod @@ -6,11 +6,11 @@ require ( github.com/BurntSushi/toml v0.3.1 github.com/alicebob/miniredis/v2 v2.11.2 github.com/bitly/go-simplejson v0.5.0 - github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 + github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 // indirect github.com/coreos/go-oidc v2.2.1+incompatible github.com/dgrijalva/jwt-go v3.2.0+incompatible - github.com/go-redis/redis v6.15.7+incompatible - github.com/gomodule/redigo v2.0.0+incompatible // indirect + github.com/fsnotify/fsnotify v1.4.9 + github.com/go-redis/redis/v7 v7.2.0 github.com/kr/pretty v0.2.0 // indirect github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa github.com/mreiferson/go-options v1.0.0 @@ -19,11 +19,10 @@ require ( github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect github.com/stretchr/testify v1.5.1 github.com/yhat/wsutil v0.0.0-20170731153501-1d66fa95c997 - golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d - golang.org/x/net v0.0.0-20200226121028-0de0cce0169b + golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 + golang.org/x/net v0.0.0-20190923162816-aa69164e4478 golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d - google.golang.org/api v0.19.0 - gopkg.in/fsnotify/fsnotify.v1 v1.4.7 + google.golang.org/api v0.20.0 gopkg.in/natefinch/lumberjack.v2 v2.0.0 gopkg.in/square/go-jose.v2 v2.4.1 ) diff --git a/go.sum b/go.sum index b519ac9e..5d4662da 100644 --- a/go.sum +++ b/go.sum @@ -27,8 +27,10 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= -github.com/go-redis/redis v6.15.7+incompatible h1:3skhDh95XQMpnqeqNftPkQD9jL9e5e36z/1SUm6dy1U= -github.com/go-redis/redis v6.15.7+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA= +github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4= +github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= +github.com/go-redis/redis/v7 v7.2.0 h1:CrCexy/jYWZjW0AyVoHlcJUeZN19VWlbepTh1Vq6dJs= +github.com/go-redis/redis/v7 v7.2.0/go.mod h1:JDNMw23GTyLNC4GZu9njt15ctBQVn7xjRfnwdHj/Dcg= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= @@ -36,9 +38,8 @@ github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfb github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/gomodule/redigo v1.7.1-0.20190322064113-39e2c31b7ca3 h1:6amM4HsNPOvMLVc2ZnyqrjeQ92YAVWn7T4WBKK87inY= github.com/gomodule/redigo v1.7.1-0.20190322064113-39e2c31b7ca3/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4= -github.com/gomodule/redigo v2.0.0+incompatible h1:K/R+8tc58AaqLkqG2Ol3Qk+DR/TlNuhuh457pBFPtt0= -github.com/gomodule/redigo v2.0.0+incompatible/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY= @@ -54,6 +55,7 @@ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -64,8 +66,10 @@ github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa/go.mod h1:8vxFeeg+ github.com/mreiferson/go-options v1.0.0 h1:RMLidydGlDWpL+lQTXo0bVIf/XT2CTq7AEJMoz5/VWs= github.com/mreiferson/go-options v1.0.0/go.mod h1:zHtCks/HQvOt8ATyfwVe3JJq2PPuImzXINPRTC03+9w= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/ginkgo v1.10.1/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.12.0 h1:Iw5WCbBcaAAd0fpRb1c9r5YCylv4XDoCSigm1zLevwU= github.com/onsi/ginkgo v1.12.0/go.mod h1:oUhWkIvk5aDxtKvDDuw8gItl8pKl42LzjC9KZE0HfGg= +github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.9.0 h1:R1uwffexN6Pr340GtYRIdZmAiN4J+iw6WG4wog1DUXg= github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA= @@ -83,9 +87,8 @@ github.com/yuin/gopher-lua v0.0.0-20191220021717-ab39c6098bdb h1:ZkM6LRnq40pR1Ox github.com/yuin/gopher-lua v0.0.0-20191220021717-ab39c6098bdb/go.mod h1:gqRgreBUhTSL0GeU64rtZ3Uq3wtjOa/TB2YfrtkCbVQ= go.opencensus.io v0.21.0 h1:mU6zScU4U1YAFPHEHYk+3JC4SY7JxgkqS10ZOSyksNg= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d h1:1ZiEyfaQIg3Qh0EoqpwAakHVhecoE5wlSg5GjnafJGw= -golang.org/x/crypto v0.0.0-20200221231518-2aa609cf4a9d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= @@ -98,11 +101,10 @@ golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73r golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190503192946-f4e77d36d62c h1:uOCk1iQW6Vc18bnC13MfzScl+wdKBmM9Y9kU7Z83/lw= golang.org/x/net v0.0.0-20190503192946-f4e77d36d62c/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20200226121028-0de0cce0169b h1:0mm1VjtFUOIlE1SbDlwjYaDxZVDP2S5ou6y0gSgXHu8= -golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20190923162816-aa69164e4478 h1:l5EDrHhldLYb3ZRHDUhXF7Om7MvYXnkV9/iQNo1lX6g= +golang.org/x/net v0.0.0-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= @@ -118,9 +120,10 @@ golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190204203706-41f3e6584952/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b h1:ag/x1USPSsqHud38I9BAC88qdNLDHHtQ4mlgQIZPPNA= golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191010194322-b09406accb47/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e h1:N7DeIrjYszNmSW409R3frPPwglRwMkXSBzwVbkOjLLA= golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -137,8 +140,8 @@ golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= -google.golang.org/api v0.19.0 h1:GwFK8+l5/gdsOYKz5p6M4UK+QT8OvmHWZPJCnf+5DjA= -google.golang.org/api v0.19.0/go.mod h1:BwFmGc8tA3vsd7r/7kR8DY7iEEGSU04BFxCo5jP/sfE= +google.golang.org/api v0.20.0 h1:jz2KixHX7EcCPiQrySzPdnYT7DbINAypCqKZ1Z7GM40= +google.golang.org/api v0.20.0/go.mod h1:BwFmGc8tA3vsd7r/7kR8DY7iEEGSU04BFxCo5jP/sfE= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/appengine v1.5.0 h1:KxkO13IPW4Lslp2bz+KHP2E3gtFlrIGNThxkZQ3g+4c= @@ -154,16 +157,17 @@ google.golang.org/grpc v1.27.0 h1:rRYRFMVgRv6E0D70Skyfsr28tDXIuuPZyWGMPdMcnXg= google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= -gopkg.in/fsnotify/fsnotify.v1 v1.4.7 h1:XNNYLJHt73EyYiCZi6+xjupS9CpvmiDgjPTAjrBlQbo= -gopkg.in/fsnotify/fsnotify.v1 v1.4.7/go.mod h1:Fyux9zXlo4rWoMSIzpn9fDAYjalPqJ/K1qJ27s+7ltE= gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8= gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= gopkg.in/square/go-jose.v2 v2.4.1 h1:H0TmLt7/KmzlrDOpa1F+zr0Tk90PbJYBfsVUmRLrf9Y= gopkg.in/square/go-jose.v2 v2.4.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= +gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= diff --git a/http.go b/http.go index afc8ba37..228364e3 100644 --- a/http.go +++ b/http.go @@ -1,7 +1,9 @@ package main import ( + "context" "crypto/tls" + "errors" "net" "net/http" "strings" @@ -14,6 +16,7 @@ import ( type Server struct { Handler http.Handler Opts *Options + stop chan struct{} // channel for waiting shutdown } // ListenAndServe will serve traffic on HTTP or HTTPS depending on TLS options @@ -90,13 +93,7 @@ func (s *Server) ServeHTTP() { logger.Fatalf("FATAL: listen (%s, %s) failed - %s", networkType, listenAddr, err) } logger.Printf("HTTP: listening on %s", listenAddr) - - server := &http.Server{Handler: s.Handler} - err = server.Serve(listener) - if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { - logger.Printf("ERROR: http.Serve() - %s", err) - } - + s.serve(listener) logger.Printf("HTTP: closing %s", listener.Addr()) } @@ -125,16 +122,33 @@ func (s *Server) ServeHTTPS() { logger.Printf("HTTPS: listening on %s", ln.Addr()) tlsListener := tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, config) - srv := &http.Server{Handler: s.Handler} - err = srv.Serve(tlsListener) - - if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { - logger.Printf("ERROR: https.Serve() - %s", err) - } - + s.serve(tlsListener) logger.Printf("HTTPS: closing %s", tlsListener.Addr()) } +func (s *Server) serve(listener net.Listener) { + srv := &http.Server{Handler: s.Handler} + + // See https://golang.org/pkg/net/http/#Server.Shutdown + idleConnsClosed := make(chan struct{}) + go func() { + <-s.stop // wait notification for stopping server + + // We received an interrupt signal, shut down. + if err := srv.Shutdown(context.Background()); err != nil { + // Error from closing listeners, or context timeout: + logger.Printf("HTTP server Shutdown: %v", err) + } + close(idleConnsClosed) + }() + + err := srv.Serve(listener) + if err != nil && !errors.Is(err, http.ErrServerClosed) { + logger.Printf("ERROR: http.Serve() - %s", err) + } + <-idleConnsClosed +} + // tcpKeepAliveListener sets TCP keep-alive timeouts on accepted // connections. It's used by ListenAndServe and ListenAndServeTLS so // dead TCP connections (e.g. closing laptop mid-download) eventually diff --git a/http_test.go b/http_test.go index 400213a0..f5165c5e 100644 --- a/http_test.go +++ b/http_test.go @@ -3,7 +3,9 @@ package main import ( "net/http" "net/http/httptest" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -156,3 +158,31 @@ func TestRedirectNotWhenHTTPS(t *testing.T) { assert.Equal(t, http.StatusOK, res.StatusCode, "status code should be %d, got: %d", http.StatusOK, res.StatusCode) } + +func TestGracefulShutdown(t *testing.T) { + opts := NewOptions() + stop := make(chan struct{}, 1) + srv := Server{Handler: http.DefaultServeMux, Opts: opts, stop: stop} + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + srv.ServeHTTP() + }() + + stop <- struct{}{} // emulate catching signals + + // An idiomatic for sync.WaitGroup with timeout + c := make(chan struct{}) + go func() { + defer close(c) + wg.Wait() + }() + select { + case <-c: + case <-time.After(1 * time.Second): + t.Fatal("Server should return gracefully but timeout has occurred") + } + + assert.Len(t, stop, 0) // check if stop chan is empty +} diff --git a/main.go b/main.go index 2512e064..1af017c5 100644 --- a/main.go +++ b/main.go @@ -6,8 +6,10 @@ import ( "math/rand" "net/http" "os" + "os/signal" "runtime" "strings" + "syscall" "time" "github.com/BurntSushi/toml" @@ -204,6 +206,14 @@ func main() { s := &Server{ Handler: handler, Opts: opts, + stop: make(chan struct{}, 1), } + // Observe signals in background goroutine. + go func() { + sigint := make(chan os.Signal, 1) + signal.Notify(sigint, os.Interrupt, syscall.SIGTERM) + <-sigint + s.stop <- struct{}{} // notify having caught signal + }() s.ListenAndServe() } diff --git a/pkg/sessions/redis/client.go b/pkg/sessions/redis/client.go new file mode 100644 index 00000000..6037ce47 --- /dev/null +++ b/pkg/sessions/redis/client.go @@ -0,0 +1,59 @@ +package redis + +import ( + "context" + "time" + + "github.com/go-redis/redis/v7" +) + +// Client is wrapper interface for redis.Client and redis.ClusterClient. +type Client interface { + Get(ctx context.Context, key string) ([]byte, error) + Set(ctx context.Context, key string, value []byte, expiration time.Duration) error + Del(ctx context.Context, key string) error +} + +var _ Client = (*client)(nil) + +type client struct { + *redis.Client +} + +func newClient(c *redis.Client) Client { + return &client{Client: c} +} + +func (c *client) Get(ctx context.Context, key string) ([]byte, error) { + return c.WithContext(ctx).Get(key).Bytes() +} + +func (c *client) Set(ctx context.Context, key string, value []byte, expiration time.Duration) error { + return c.WithContext(ctx).Set(key, value, expiration).Err() +} + +func (c *client) Del(ctx context.Context, key string) error { + return c.WithContext(ctx).Del(key).Err() +} + +var _ Client = (*clusterClient)(nil) + +type clusterClient struct { + *redis.ClusterClient +} + +func newClusterClient(c *redis.ClusterClient) Client { + return &clusterClient{ClusterClient: c} +} + +func (c *clusterClient) Get(ctx context.Context, key string) ([]byte, error) { + return c.WithContext(ctx).Get(key).Bytes() +} + +func (c *clusterClient) Set(ctx context.Context, key string, value []byte, expiration time.Duration) error { + return c.WithContext(ctx).Set(key, value, expiration).Err() +} + +func (c *clusterClient) Del(ctx context.Context, key string) error { + return c.WithContext(ctx).Del(key).Err() +} diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index f4169398..3f9271e1 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -1,6 +1,7 @@ package redis import ( + "context" "crypto/aes" "crypto/cipher" "crypto/rand" @@ -14,7 +15,7 @@ import ( "strings" "time" - "github.com/go-redis/redis" + "github.com/go-redis/redis/v7" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/pkg/cookies" @@ -33,19 +34,19 @@ type TicketData struct { type SessionStore struct { CookieCipher *encryption.Cipher CookieOptions *options.CookieOptions - Cmdable redis.Cmdable + Client Client } // NewRedisSessionStore initialises a new instance of the SessionStore from // the configuration given func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) { - cmdable, err := newRedisCmdable(opts.RedisStoreOptions) + client, err := newRedisCmdable(opts.RedisStoreOptions) if err != nil { return nil, fmt.Errorf("error constructing redis client: %v", err) } rs := &SessionStore{ - Cmdable: cmdable, + Client: client, CookieCipher: opts.Cipher, CookieOptions: cookieOpts, } @@ -53,7 +54,7 @@ func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cook } -func newRedisCmdable(opts options.RedisStoreOptions) (redis.Cmdable, error) { +func newRedisCmdable(opts options.RedisStoreOptions) (Client, error) { if opts.UseSentinel && opts.UseCluster { return nil, fmt.Errorf("options redis-use-sentinel and redis-use-cluster are mutually exclusive") } @@ -63,14 +64,14 @@ func newRedisCmdable(opts options.RedisStoreOptions) (redis.Cmdable, error) { MasterName: opts.SentinelMasterName, SentinelAddrs: opts.SentinelConnectionURLs, }) - return client, nil + return newClient(client), nil } if opts.UseCluster { client := redis.NewClusterClient(&redis.ClusterOptions{ Addrs: opts.ClusterConnectionURLs, }) - return client, nil + return newClusterClient(client), nil } opt, err := redis.ParseURL(opts.RedisConnectionURL) @@ -104,7 +105,7 @@ func newRedisCmdable(opts options.RedisStoreOptions) (redis.Cmdable, error) { } client := redis.NewClient(opt) - return client, nil + return newClient(client), nil } // Save takes a sessions.SessionState and stores the information from it @@ -121,7 +122,8 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se if err != nil { return err } - ticketString, err := store.storeValue(value, store.CookieOptions.CookieExpire, requestCookie) + ctx := req.Context() + ticketString, err := store.storeValue(ctx, value, store.CookieOptions.CookieExpire, requestCookie) if err != nil { return err } @@ -149,7 +151,8 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro if !ok { return nil, fmt.Errorf("Cookie Signature not valid") } - session, err := store.loadSessionFromString(val) + ctx := req.Context() + session, err := store.loadSessionFromString(ctx, val) if err != nil { return nil, fmt.Errorf("error loading session: %s", err) } @@ -157,18 +160,17 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro } // loadSessionFromString loads the session based on the ticket value -func (store *SessionStore) loadSessionFromString(value string) (*sessions.SessionState, error) { +func (store *SessionStore) loadSessionFromString(ctx context.Context, value string) (*sessions.SessionState, error) { ticket, err := decodeTicket(store.CookieOptions.CookieName, value) if err != nil { return nil, err } - result, err := store.Cmdable.Get(ticket.asHandle(store.CookieOptions.CookieName)).Result() + resultBytes, err := store.Client.Get(ctx, ticket.asHandle(store.CookieOptions.CookieName)) if err != nil { return nil, err } - resultBytes := []byte(result) block, err := aes.NewCipher(ticket.Secret) if err != nil { return nil, err @@ -214,7 +216,8 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro // If there's an issue decoding the ticket, ignore it ticket, _ := decodeTicket(store.CookieOptions.CookieName, val) if ticket != nil { - _, err := store.Cmdable.Del(ticket.asHandle(store.CookieOptions.CookieName)).Result() + ctx := req.Context() + err := store.Client.Del(ctx, ticket.asHandle(store.CookieOptions.CookieName)) if err != nil { return fmt.Errorf("error clearing cookie from redis: %s", err) } @@ -237,7 +240,7 @@ func (store *SessionStore) makeCookie(req *http.Request, value string, expires t ) } -func (store *SessionStore) storeValue(value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) { +func (store *SessionStore) storeValue(ctx context.Context, value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) { ticket, err := store.getTicket(requestCookie) if err != nil { return "", fmt.Errorf("error getting ticket: %v", err) @@ -254,7 +257,7 @@ func (store *SessionStore) storeValue(value string, expiration time.Duration, re stream.XORKeyStream(ciphertext, []byte(value)) handle := ticket.asHandle(store.CookieOptions.CookieName) - err = store.Cmdable.Set(handle, ciphertext, expiration).Err() + err = store.Client.Set(ctx, handle, ciphertext, expiration) if err != nil { return "", err } @@ -290,7 +293,7 @@ func newTicket() (*TicketData, error) { return nil, fmt.Errorf("failed to create new ticket ID %s", err) } // ticketID is hex encoded - ticketID := fmt.Sprintf("%x", rawID) + ticketID := hex.EncodeToString(rawID) secret := make([]byte, aes.BlockSize) if _, err := io.ReadFull(rand.Reader, secret); err != nil { diff --git a/providers/keycloak_test.go b/providers/keycloak_test.go index e00fb045..2ac9d67f 100644 --- a/providers/keycloak_test.go +++ b/providers/keycloak_test.go @@ -6,7 +6,8 @@ import ( "net/url" "testing" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" + "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" ) diff --git a/providers/oidc_test.go b/providers/oidc_test.go index 675f8fda..7a0581a9 100644 --- a/providers/oidc_test.go +++ b/providers/oidc_test.go @@ -15,9 +15,9 @@ import ( "testing" "time" - "github.com/bmizerany/assert" "github.com/coreos/go-oidc" "github.com/dgrijalva/jwt-go" + "github.com/stretchr/testify/assert" "golang.org/x/oauth2" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" diff --git a/watcher.go b/watcher.go index cfddcdf6..c1ca8b3d 100644 --- a/watcher.go +++ b/watcher.go @@ -7,8 +7,9 @@ import ( "path/filepath" "time" + "github.com/fsnotify/fsnotify" + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" - fsnotify "gopkg.in/fsnotify/fsnotify.v1" ) // WaitForReplacement waits for a file to exist on disk and then starts a watch