From f4be1a1596ceea5ad6da3a6219a7ba5b843ef70a Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Tue, 31 Mar 2020 05:30:34 +0900 Subject: [PATCH 01/10] 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. --- go.mod | 3 +- go.sum | 15 +++++--- http.go | 42 +++++++++++++++------- http_test.go | 29 ++++++++++++++++ pkg/sessions/redis/client.go | 58 +++++++++++++++++++++++++++++++ pkg/sessions/redis/redis_store.go | 37 +++++++++++--------- 6 files changed, 149 insertions(+), 35 deletions(-) create mode 100644 pkg/sessions/redis/client.go diff --git a/go.mod b/go.mod index 6fa871c32b..965d44f692 100644 --- a/go.mod +++ b/go.mod @@ -9,8 +9,7 @@ require ( github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 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/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 diff --git a/go.sum b/go.sum index b519ac9e55..32973c981d 100644 --- a/go.sum +++ b/go.sum @@ -27,8 +27,8 @@ 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/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 +36,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 +53,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 +64,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= @@ -101,6 +103,7 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn 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-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= 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/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -121,6 +124,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h 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-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= @@ -154,6 +158,8 @@ 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= @@ -164,6 +170,7 @@ 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 afc8ba378e..8f6965280a 100644 --- a/http.go +++ b/http.go @@ -1,10 +1,15 @@ package main import ( + "context" "crypto/tls" + "errors" "net" "net/http" + "os" + "os/signal" "strings" + "syscall" "time" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" @@ -90,13 +95,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,14 +124,33 @@ func (s *Server) ServeHTTPS() { logger.Printf("HTTPS: listening on %s", ln.Addr()) tlsListener := tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, config) + s.serve(tlsListener) + logger.Printf("HTTPS: closing %s", tlsListener.Addr()) +} + +func (s *Server) serve(listener net.Listener) { 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) - } + // See https://golang.org/pkg/net/http/#Server.Shutdown + idleConnsClosed := make(chan struct{}) + go func() { + sigint := make(chan os.Signal, 1) + signal.Notify(sigint, os.Interrupt, syscall.SIGTERM) + s := <-sigint + logger.Printf("caught signal: %s", s) + + // 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) + }() - logger.Printf("HTTPS: closing %s", tlsListener.Addr()) + err := srv.Serve(listener) + if err != nil && !errors.Is(err, http.ErrServerClosed) { + logger.Printf("ERROR: http.Serve() - %s", err) + } } // tcpKeepAliveListener sets TCP keep-alive timeouts on accepted diff --git a/http_test.go b/http_test.go index 400213a01b..f7d2b73488 100644 --- a/http_test.go +++ b/http_test.go @@ -1,9 +1,14 @@ package main import ( + "fmt" "net/http" "net/http/httptest" + "os" + "sync" + "syscall" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -156,3 +161,27 @@ 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) { + signals := []syscall.Signal{syscall.SIGINT, syscall.SIGTERM} + + for i, signal := range signals { + name := fmt.Sprintf("%s", signal) + t.Run(name, func(t *testing.T) { + opts := NewOptions() + opts.HTTPAddress = fmt.Sprintf(":%d", 4180+i) + srv := Server{Handler: http.DefaultServeMux, Opts: opts} + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + srv.ServeHTTP() + }() + time.Sleep(500 * time.Millisecond) + if err := syscall.Kill(os.Getpid(), signal); err != nil { + t.Fatal(err) + } + wg.Wait() + }) + } +} diff --git a/pkg/sessions/redis/client.go b/pkg/sessions/redis/client.go new file mode 100644 index 0000000000..4d4e174be0 --- /dev/null +++ b/pkg/sessions/redis/client.go @@ -0,0 +1,58 @@ +package redis + +import ( + "context" + "time" + + "github.com/go-redis/redis/v7" +) + +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 f4169398e3..3f9271e17b 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 { From 0d81995afac89202e934dbd358839b784b503b18 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Tue, 31 Mar 2020 05:49:22 +0900 Subject: [PATCH 02/10] 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 --- go.mod | 10 +++++----- go.sum | 14 ++++---------- providers/keycloak_test.go | 3 ++- providers/oidc_test.go | 2 +- watcher.go | 3 ++- 5 files changed, 14 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index 965d44f692..e38fbe25e6 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,10 @@ 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/fsnotify/fsnotify v1.4.7 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 @@ -18,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 32973c981d..6184fbd68c 100644 --- a/go.sum +++ b/go.sum @@ -85,9 +85,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= @@ -100,12 +99,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-20190923162816-aa69164e4478 h1:l5EDrHhldLYb3ZRHDUhXF7Om7MvYXnkV9/iQNo1lX6g= golang.org/x/net v0.0.0-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -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/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= @@ -121,7 +118,6 @@ 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-20191010194322-b09406accb47/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -141,8 +137,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= @@ -162,8 +158,6 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogR 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= diff --git a/providers/keycloak_test.go b/providers/keycloak_test.go index e00fb0452c..2ac9d67f5c 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 675f8fda22..7a0581a9c7 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 cfddcdf614..c1ca8b3d85 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 From 21e7f60199191d8dd267e15f7bd6175a71c07a84 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Tue, 31 Mar 2020 06:07:03 +0900 Subject: [PATCH 03/10] add doc for wrapper interface --- pkg/sessions/redis/client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/sessions/redis/client.go b/pkg/sessions/redis/client.go index 4d4e174be0..6037ce470d 100644 --- a/pkg/sessions/redis/client.go +++ b/pkg/sessions/redis/client.go @@ -7,6 +7,7 @@ import ( "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 From 686756f33be86cb630c8453ac3afbb31464143e6 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Tue, 31 Mar 2020 06:11:43 +0900 Subject: [PATCH 04/10] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 946c0717eb..c383f8d251 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` From c28a948271652414b6b68e45d1273a6c35698499 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Thu, 2 Apr 2020 04:33:52 +0900 Subject: [PATCH 05/10] fix: upgrade fsnotify to v1.4.9 --- go.mod | 2 +- go.sum | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index e38fbe25e6..2170dc9bc7 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( 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/fsnotify/fsnotify v1.4.7 + 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 diff --git a/go.sum b/go.sum index 6184fbd68c..5d4662da5d 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,8 @@ 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/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= @@ -120,6 +122,7 @@ golang.org/x/sys v0.0.0-20190204203706-41f3e6584952/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= 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= From 610ec1237779d1f961f1f9adf42307f789fa450e Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Thu, 2 Apr 2020 04:45:14 +0900 Subject: [PATCH 06/10] fix: remove unnessary logging --- http.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/http.go b/http.go index 8f6965280a..b3e15ecea0 100644 --- a/http.go +++ b/http.go @@ -136,8 +136,7 @@ func (s *Server) serve(listener net.Listener) { go func() { sigint := make(chan os.Signal, 1) signal.Notify(sigint, os.Interrupt, syscall.SIGTERM) - s := <-sigint - logger.Printf("caught signal: %s", s) + <-sigint // We received an interrupt signal, shut down. if err := srv.Shutdown(context.Background()); err != nil { From a4e531abed21d6f9fd7ce2dd48027f7e67ba3ea0 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Thu, 2 Apr 2020 04:59:47 +0900 Subject: [PATCH 07/10] fix: wait until all connections have been closed --- http.go | 1 + 1 file changed, 1 insertion(+) diff --git a/http.go b/http.go index b3e15ecea0..1ba8e704a8 100644 --- a/http.go +++ b/http.go @@ -150,6 +150,7 @@ func (s *Server) serve(listener net.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 From 44ed2ffe6e11564b0e81d1e57e39c8a22b2d984b Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Thu, 2 Apr 2020 05:27:49 +0900 Subject: [PATCH 08/10] refactor: move chan to main for testing --- http.go | 8 ++------ http_test.go | 37 ++++++++++++------------------------- main.go | 10 ++++++++++ 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/http.go b/http.go index 1ba8e704a8..228364e37d 100644 --- a/http.go +++ b/http.go @@ -6,10 +6,7 @@ import ( "errors" "net" "net/http" - "os" - "os/signal" "strings" - "syscall" "time" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" @@ -19,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 @@ -134,9 +132,7 @@ func (s *Server) serve(listener net.Listener) { // See https://golang.org/pkg/net/http/#Server.Shutdown idleConnsClosed := make(chan struct{}) go func() { - sigint := make(chan os.Signal, 1) - signal.Notify(sigint, os.Interrupt, syscall.SIGTERM) - <-sigint + <-s.stop // wait notification for stopping server // We received an interrupt signal, shut down. if err := srv.Shutdown(context.Background()); err != nil { diff --git a/http_test.go b/http_test.go index f7d2b73488..6bf8de1c24 100644 --- a/http_test.go +++ b/http_test.go @@ -1,14 +1,10 @@ package main import ( - "fmt" "net/http" "net/http/httptest" - "os" "sync" - "syscall" "testing" - "time" "github.com/stretchr/testify/assert" ) @@ -163,25 +159,16 @@ func TestRedirectNotWhenHTTPS(t *testing.T) { } func TestGracefulShutdown(t *testing.T) { - signals := []syscall.Signal{syscall.SIGINT, syscall.SIGTERM} - - for i, signal := range signals { - name := fmt.Sprintf("%s", signal) - t.Run(name, func(t *testing.T) { - opts := NewOptions() - opts.HTTPAddress = fmt.Sprintf(":%d", 4180+i) - srv := Server{Handler: http.DefaultServeMux, Opts: opts} - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - srv.ServeHTTP() - }() - time.Sleep(500 * time.Millisecond) - if err := syscall.Kill(os.Getpid(), signal); err != nil { - t.Fatal(err) - } - wg.Wait() - }) - } + 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 + wg.Wait() } diff --git a/main.go b/main.go index 2512e0646d..1af017c5ff 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() } From 7c2ea540c90d0349fa18ebcc5251c97529428acf Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Sat, 4 Apr 2020 23:48:55 +0900 Subject: [PATCH 09/10] add assert to check if stop chan is empty --- http_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/http_test.go b/http_test.go index 6bf8de1c24..934ed0cc3b 100644 --- a/http_test.go +++ b/http_test.go @@ -171,4 +171,6 @@ func TestGracefulShutdown(t *testing.T) { stop <- struct{}{} // emulate catching signals wg.Wait() + + assert.Len(t, stop, 0) // check if stop chan is empty } From b620f6eb0223ded84ca34eff463f528c536038e9 Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Sun, 5 Apr 2020 00:03:51 +0900 Subject: [PATCH 10/10] add an idiomatic for sync.WaitGroup with timeout --- http_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/http_test.go b/http_test.go index 934ed0cc3b..f5165c5e2d 100644 --- a/http_test.go +++ b/http_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "sync" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -170,7 +171,18 @@ func TestGracefulShutdown(t *testing.T) { }() stop <- struct{}{} // emulate catching signals - wg.Wait() + + // 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 }