From 267d0b7b5a9a3692820319d3c8cb41325c3a45d5 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Fri, 6 Nov 2020 09:26:03 +0100 Subject: [PATCH] chore: update linter. --- .golangci.toml | 5 +++- build.Dockerfile | 2 +- go.sum | 1 + pkg/log/log.go | 2 +- pkg/metrics/prometheus.go | 30 ++++++++++++------- pkg/middlewares/auth/forward.go | 3 +- pkg/middlewares/recovery/recovery.go | 1 + pkg/provider/docker/docker.go | 3 +- pkg/server/server.go | 5 ++-- pkg/server/server_entrypoint_tcp.go | 10 +++++-- pkg/server/server_entrypoint_tcp_test.go | 2 +- .../service/loadbalancer/mirror/mirror.go | 8 ++--- pkg/server/service/loadbalancer/wrr/wrr.go | 3 +- pkg/server/service/proxy.go | 10 ++++--- pkg/server/service/proxy_websocket_test.go | 8 +++-- pkg/tcp/router.go | 6 ++-- pkg/udp/conn_test.go | 7 +++-- 17 files changed, 68 insertions(+), 38 deletions(-) diff --git a/.golangci.toml b/.golangci.toml index 9e939601c..64e06b670 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -54,7 +54,10 @@ "nestif", # Too many false-positive. "noctx", # Too strict "exhaustive", # Too strict - "nlreturn", # Too strict + "nlreturn", # Not relevant + "wrapcheck", # Too strict + "tparallel", # Not relevant + "exhaustivestruct", # Not relevant ] [issues] diff --git a/build.Dockerfile b/build.Dockerfile index e1a0537a8..6df51c544 100644 --- a/build.Dockerfile +++ b/build.Dockerfile @@ -19,7 +19,7 @@ RUN mkdir -p /usr/local/bin \ && chmod +x /usr/local/bin/go-bindata # Download golangci-lint binary to bin folder in $GOPATH -RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | bash -s -- -b $GOPATH/bin v1.31.0 +RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | bash -s -- -b $GOPATH/bin v1.32.2 # Download misspell binary to bin folder in $GOPATH RUN curl -sfL https://raw.githubusercontent.com/client9/misspell/master/install-misspell.sh | bash -s -- -b $GOPATH/bin v0.3.4 diff --git a/go.sum b/go.sum index f97907193..e721061e4 100644 --- a/go.sum +++ b/go.sum @@ -1108,6 +1108,7 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8 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/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw= +gopkg.in/errgo.v2 v2.1.0 h1:0vLT13EuvQ0hNvakwLuFZ/jYrLp5F3kcWHXdRggjCE8= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= diff --git a/pkg/log/log.go b/pkg/log/log.go index dddb80a99..999a5e1fd 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -137,7 +137,7 @@ func RotateFile() error { } if err := OpenFile(logFilePath); err != nil { - return fmt.Errorf("error opening log file: %s", err) + return fmt.Errorf("error opening log file: %w", err) } return nil diff --git a/pkg/metrics/prometheus.go b/pkg/metrics/prometheus.go index 13f9a118c..b627b50b4 100644 --- a/pkg/metrics/prometheus.go +++ b/pkg/metrics/prometheus.go @@ -2,6 +2,7 @@ package metrics import ( "context" + "errors" "net/http" "sort" "strings" @@ -74,12 +75,15 @@ func RegisterPrometheus(ctx context.Context, config *types.Prometheus) Registry standardRegistry := initStandardRegistry(config) if err := promRegistry.Register(stdprometheus.NewProcessCollector(stdprometheus.ProcessCollectorOpts{})); err != nil { - if _, ok := err.(stdprometheus.AlreadyRegisteredError); !ok { + var arErr stdprometheus.AlreadyRegisteredError + if !errors.As(err, &arErr) { log.FromContext(ctx).Warn("ProcessCollector is already registered") } } + if err := promRegistry.Register(stdprometheus.NewGoCollector()); err != nil { - if _, ok := err.(stdprometheus.AlreadyRegisteredError); !ok { + var arErr stdprometheus.AlreadyRegisteredError + if !errors.As(err, &arErr) { log.FromContext(ctx).Warn("GoCollector is already registered") } } @@ -212,15 +216,21 @@ func initStandardRegistry(config *types.Prometheus) Registry { } func registerPromState(ctx context.Context) bool { - if err := promRegistry.Register(promState); err != nil { - logger := log.FromContext(ctx) - if _, ok := err.(stdprometheus.AlreadyRegisteredError); !ok { - logger.Errorf("Unable to register Traefik to Prometheus: %v", err) - return false - } - logger.Debug("Prometheus collector already registered.") + err := promRegistry.Register(promState) + if err == nil { + return true } - return true + + logger := log.FromContext(ctx) + + var arErr stdprometheus.AlreadyRegisteredError + if errors.As(err, &arErr) { + logger.Debug("Prometheus collector already registered.") + return true + } + + logger.Errorf("Unable to register Traefik to Prometheus: %v", err) + return false } // OnConfigurationUpdate receives the current configuration from Traefik. diff --git a/pkg/middlewares/auth/forward.go b/pkg/middlewares/auth/forward.go index e3a0bd067..23327065c 100644 --- a/pkg/middlewares/auth/forward.go +++ b/pkg/middlewares/auth/forward.go @@ -2,6 +2,7 @@ package auth import ( "context" + "errors" "fmt" "io/ioutil" "net" @@ -124,7 +125,7 @@ func (fa *forwardAuth) ServeHTTP(rw http.ResponseWriter, req *http.Request) { redirectURL, err := forwardResponse.Location() if err != nil { - if err != http.ErrNoLocation { + if !errors.Is(err, http.ErrNoLocation) { logMessage := fmt.Sprintf("Error reading response location header %s. Cause: %s", fa.address, err) logger.Debug(logMessage) tracing.SetErrorWithEvent(req, logMessage) diff --git a/pkg/middlewares/recovery/recovery.go b/pkg/middlewares/recovery/recovery.go index dc3fc2a68..7040c91c2 100644 --- a/pkg/middlewares/recovery/recovery.go +++ b/pkg/middlewares/recovery/recovery.go @@ -54,5 +54,6 @@ func recoverFunc(ctx context.Context, rw http.ResponseWriter, r *http.Request) { // https://github.com/golang/go/blob/a0d6420d8be2ae7164797051ec74fa2a2df466a1/src/net/http/server.go#L1761-L1775 // https://github.com/golang/go/blob/c33153f7b416c03983324b3e8f869ce1116d84bc/src/net/http/httputil/reverseproxy.go#L284 func shouldLogPanic(panicValue interface{}) bool { + //nolint:errorlint // false-positive because panicValue is an interface. return panicValue != nil && panicValue != http.ErrAbortHandler } diff --git a/pkg/provider/docker/docker.go b/pkg/provider/docker/docker.go index 1aad0a8a5..a0d199320 100644 --- a/pkg/provider/docker/docker.go +++ b/pkg/provider/docker/docker.go @@ -2,6 +2,7 @@ package docker import ( "context" + "errors" "fmt" "io" "net" @@ -307,7 +308,7 @@ func (p *Provider) Provide(configurationChan chan<- dynamic.Message, pool *safe. startStopHandle(event) } case err := <-errc: - if err == io.EOF { + if errors.Is(err, io.EOF) { logger.Debug("Provider event stream closed") } return err diff --git a/pkg/server/server.go b/pkg/server/server.go index 0df1ba9b3..9c585b7a3 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2,6 +2,7 @@ package server import ( "context" + "errors" "os" "os/signal" "time" @@ -85,9 +86,9 @@ func (s *Server) Close() { go func(ctx context.Context) { <-ctx.Done() - if ctx.Err() == context.Canceled { + if errors.Is(ctx.Err(), context.Canceled) { return - } else if ctx.Err() == context.DeadlineExceeded { + } else if errors.Is(ctx.Err(), context.DeadlineExceeded) { panic("Timeout while stopping traefik, killing instance ✝") } }(ctx) diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index 46e5295db..a9f1fdf94 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -174,11 +174,15 @@ func (e *TCPEntryPoint) Start(ctx context.Context) { conn, err := e.listener.Accept() if err != nil { logger.Error(err) - if netErr, ok := err.(net.Error); ok && netErr.Temporary() { + + var netErr net.Error + if errors.As(err, &netErr) && netErr.Temporary() { continue } + e.httpServer.Forwarder.errChan <- err e.httpsServer.Forwarder.errChan <- err + return } @@ -232,7 +236,7 @@ func (e *TCPEntryPoint) Shutdown(ctx context.Context) { if err == nil { return } - if ctx.Err() == context.DeadlineExceeded { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { logger.Debugf("Server failed to shutdown within deadline because: %s", err) if err = server.Close(); err != nil { logger.Error(err) @@ -263,7 +267,7 @@ func (e *TCPEntryPoint) Shutdown(ctx context.Context) { if err == nil { return } - if ctx.Err() == context.DeadlineExceeded { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { logger.Debugf("Server failed to shutdown before deadline because: %s", err) } e.tracker.Close() diff --git a/pkg/server/server_entrypoint_tcp_test.go b/pkg/server/server_entrypoint_tcp_test.go index 244703946..6c76c768d 100644 --- a/pkg/server/server_entrypoint_tcp_test.go +++ b/pkg/server/server_entrypoint_tcp_test.go @@ -46,7 +46,7 @@ func TestShutdownTCP(t *testing.T) { for { _, err := http.ReadRequest(bufio.NewReader(conn)) - if err == io.EOF || (err != nil && strings.HasSuffix(err.Error(), "use of closed network connection")) { + if errors.Is(err, io.EOF) || (err != nil && strings.HasSuffix(err.Error(), "use of closed network connection")) { return } require.NoError(t, err) diff --git a/pkg/server/service/loadbalancer/mirror/mirror.go b/pkg/server/service/loadbalancer/mirror/mirror.go index d5a6ef1bc..00e2f06cb 100644 --- a/pkg/server/service/loadbalancer/mirror/mirror.go +++ b/pkg/server/service/loadbalancer/mirror/mirror.go @@ -81,13 +81,13 @@ func (m *Mirroring) ServeHTTP(rw http.ResponseWriter, req *http.Request) { logger := log.FromContext(req.Context()) rr, bytesRead, err := newReusableRequest(req, m.maxBodySize) - if err != nil && err != errBodyTooLarge { + if err != nil && !errors.Is(err, errBodyTooLarge) { http.Error(rw, http.StatusText(http.StatusInternalServerError)+ fmt.Sprintf("error creating reusable request: %v", err), http.StatusInternalServerError) return } - if err == errBodyTooLarge { + if errors.Is(err, errBodyTooLarge) { req.Body = ioutil.NopCloser(io.MultiReader(bytes.NewReader(bytesRead), req.Body)) m.handler.ServeHTTP(rw, req) logger.Debugf("no mirroring, request body larger than allowed size") @@ -196,13 +196,13 @@ func newReusableRequest(req *http.Request, maxBodySize int64) (*reusableRequest, // the request body is larger than what we allow for the mirrors. body := make([]byte, maxBodySize+1) n, err := io.ReadFull(req.Body, body) - if err != nil && err != io.ErrUnexpectedEOF { + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) { return nil, nil, err } // we got an ErrUnexpectedEOF, which means there was less than maxBodySize data to read, // which permits us sending also to all the mirrors later. - if err == io.ErrUnexpectedEOF { + if errors.Is(err, io.ErrUnexpectedEOF) { return &reusableRequest{ req: req, body: body[:n], diff --git a/pkg/server/service/loadbalancer/wrr/wrr.go b/pkg/server/service/loadbalancer/wrr/wrr.go index a9ea42a4a..7558f6bca 100644 --- a/pkg/server/service/loadbalancer/wrr/wrr.go +++ b/pkg/server/service/loadbalancer/wrr/wrr.go @@ -2,6 +2,7 @@ package wrr import ( "container/heap" + "errors" "fmt" "net/http" "sync" @@ -105,7 +106,7 @@ func (b *Balancer) ServeHTTP(w http.ResponseWriter, req *http.Request) { if b.stickyCookie != nil { cookie, err := req.Cookie(b.stickyCookie.name) - if err != nil && err != http.ErrNoCookie { + if err != nil && !errors.Is(err, http.ErrNoCookie) { log.WithoutContext().Warnf("Error while reading cookie: %v", err) } diff --git a/pkg/server/service/proxy.go b/pkg/server/service/proxy.go index ea0e04d8b..236a4057d 100644 --- a/pkg/server/service/proxy.go +++ b/pkg/server/service/proxy.go @@ -2,6 +2,7 @@ package service import ( "context" + "errors" "fmt" "io" "net" @@ -83,13 +84,14 @@ func buildProxy(passHostHeader *bool, responseForwarding *dynamic.ResponseForwar statusCode := http.StatusInternalServerError switch { - case err == io.EOF: + case errors.Is(err, io.EOF): statusCode = http.StatusBadGateway - case err == context.Canceled: + case errors.Is(err, context.Canceled): statusCode = StatusClientClosedRequest default: - if e, ok := err.(net.Error); ok { - if e.Timeout() { + var netErr net.Error + if errors.As(err, &netErr) { + if netErr.Timeout() { statusCode = http.StatusGatewayTimeout } else { statusCode = http.StatusBadGateway diff --git a/pkg/server/service/proxy_websocket_test.go b/pkg/server/service/proxy_websocket_test.go index fe169e1c5..a759cf95a 100644 --- a/pkg/server/service/proxy_websocket_test.go +++ b/pkg/server/service/proxy_websocket_test.go @@ -3,6 +3,7 @@ package service import ( "bufio" "crypto/tls" + "errors" "fmt" "net" "net/http" @@ -49,12 +50,13 @@ func TestWebSocketTCPClose(t *testing.T) { withPath("/ws"), ).open() require.NoError(t, err) + conn.Close() serverErr := <-errChan - wsErr, ok := serverErr.(*gorillawebsocket.CloseError) - assert.Equal(t, true, ok) + var wsErr *gorillawebsocket.CloseError + require.True(t, errors.As(serverErr, &wsErr)) assert.Equal(t, 1006, wsErr.Code) } @@ -119,7 +121,7 @@ func TestWebSocketPingPong(t *testing.T) { _, _, err = conn.ReadMessage() - if err != goodErr { + if !errors.Is(err, goodErr) { require.NoError(t, err) } } diff --git a/pkg/tcp/router.go b/pkg/tcp/router.go index 31ff07342..ea0f406e7 100644 --- a/pkg/tcp/router.go +++ b/pkg/tcp/router.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "crypto/tls" + "errors" "io" "net" "net/http" @@ -197,10 +198,11 @@ func (c *Conn) Read(p []byte) (n int, err error) { func clientHelloServerName(br *bufio.Reader) (string, bool, string, error) { hdr, err := br.Peek(1) if err != nil { - opErr, ok := err.(*net.OpError) - if err != io.EOF && (!ok || !opErr.Timeout()) { + var opErr *net.OpError + if !errors.Is(err, io.EOF) && (!errors.As(err, &opErr) || opErr.Timeout()) { log.WithoutContext().Debugf("Error while Peeking first byte: %s", err) } + return "", false, "", err } diff --git a/pkg/udp/conn_test.go b/pkg/udp/conn_test.go index 62c35d04a..f97eb3958 100644 --- a/pkg/udp/conn_test.go +++ b/pkg/udp/conn_test.go @@ -1,6 +1,7 @@ package udp import ( + "errors" "io" "net" "testing" @@ -24,7 +25,7 @@ func TestConsecutiveWrites(t *testing.T) { go func() { for { conn, err := ln.Accept() - if err == errClosedListener { + if errors.Is(err, errClosedListener) { return } require.NoError(t, err) @@ -86,7 +87,7 @@ func TestListenNotBlocking(t *testing.T) { go func() { for { conn, err := ln.Accept() - if err == errClosedListener { + if errors.Is(err, errClosedListener) { return } require.NoError(t, err) @@ -183,7 +184,7 @@ func testTimeout(t *testing.T, withRead bool) { go func() { for { conn, err := ln.Accept() - if err == errClosedListener { + if errors.Is(err, errClosedListener) { return } require.NoError(t, err)