From 8ac84bd4c470dcfd2d14c11b0250e147b17ca85f Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 8 Feb 2023 10:16:01 +0100 Subject: [PATCH] refactor: add more linters and fix findings (#27) --- .drone.yml | 8 ++-- .golangci.yml | 93 ++++++++++++++++++++++++++++++++++++++------- Makefile | 2 +- drone/network.go | 1 + drone/types.go | 20 +++++----- drone/types_test.go | 1 + go.mod | 2 +- trace/http.go | 15 ++++---- urfave/network.go | 30 +++++++++------ urfave/repo.go | 2 +- urfave/urfave.go | 22 +++++------ 11 files changed, 138 insertions(+), 58 deletions(-) diff --git a/.drone.yml b/.drone.yml index 8e0fd14..5d806dd 100644 --- a/.drone.yml +++ b/.drone.yml @@ -8,7 +8,7 @@ platform: steps: - name: deps - image: golang:1.19 + image: golang:1.20 commands: - make deps volumes: @@ -16,7 +16,7 @@ steps: path: /go - name: lint - image: golang:1.19 + image: golang:1.20 commands: - make lint volumes: @@ -24,7 +24,7 @@ steps: path: /go - name: test - image: golang:1.19 + image: golang:1.20 commands: - make test volumes: @@ -156,6 +156,6 @@ depends_on: --- kind: signature -hmac: 9cadf4dc495cf5ea9e54b4964eab9fd7a545d6b419c7cd78bc6846108428a926 +hmac: ebcc6ff0a66ff606330d4e9fc8a2015e36b7c592e6278a36303e4970aa195575 ... diff --git a/.golangci.yml b/.golangci.yml index 7bb18ea..2faa799 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,25 +1,92 @@ linters: + enable-all: false + disable-all: true enable: + - errcheck - gosimple - - deadcode - - typecheck - govet - - errcheck + - ineffassign - staticcheck + - typecheck - unused - - structcheck - - varcheck + - asasalint + - asciicheck + - bidichk + - bodyclose + - containedctx + - contextcheck + - decorder + - depguard + - dogsled - dupl + - dupword + - durationcheck + - errchkjson + - errname + - errorlint + - execinquery + - exhaustive + - exportloopref + - forcetypeassert + - ginkgolinter + - gocheckcompilerdirectives + - gochecknoglobals + - gochecknoinits + - gocognit + - goconst + - gocritic + - gocyclo + - godot + - godox + - goerr113 - gofmt + - gofumpt + - goheader + - goimports + - gomnd + - gomoddirectives + - gomodguard + - goprintffuncname + - gosec + - grouper + - importas + - interfacebloat + - ireturn + - lll + - loggercheck + - maintidx + - makezero - misspell - - gocritic - - bidichk - - ineffassign + - musttag + - nakedret + - nestif + - nilerr + - nilnil + - nlreturn + - noctx + - nolintlint + - nonamedreturns + - nosprintfhostport + - prealloc + - predeclared + - promlinter + - reassign - revive - - gofumpt - - depguard - enable-all: false - disable-all: true + # - rowserrcheck + # - sqlclosecheck + # - structcheck + - stylecheck + - tagliatelle + - tenv + - testableexamples + - thelper + - tparallel + - unconvert + - unparam + - usestdlibvars + # - wastedassign + - whitespace + - wsl fast: false run: @@ -28,4 +95,4 @@ run: linters-settings: gofumpt: extra-rules: true - lang-version: "1.18" + lang-version: "1.20" diff --git a/Makefile b/Makefile index fc9ef12..187d05a 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@$(G XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest GENERATE ?= -XGO_VERSION := go-1.19.x +XGO_VERSION := go-1.20.x XGO_TARGETS ?= linux/amd64,linux/arm64,linux/arm-6,linux/arm-7 TARGETOS ?= linux diff --git a/drone/network.go b/drone/network.go index fb560a3..4938864 100644 --- a/drone/network.go +++ b/drone/network.go @@ -17,6 +17,7 @@ type Network struct { // // If `trace` logging is requested the context will use `httptrace` to // capture all network requests. + //nolint:containedctx Context context.Context /// Whether SSL verification is skipped diff --git a/drone/types.go b/drone/types.go index 3b3741b..c1a5b5a 100644 --- a/drone/types.go +++ b/drone/types.go @@ -4,7 +4,7 @@ import ( "strings" ) -// StringSliceFlag is a flag type which support comma separated values and escaping to not split at unwanted lines +// StringSliceFlag is a flag type which support comma separated values and escaping to not split at unwanted lines. type StringSliceFlag struct { slice []string } @@ -15,6 +15,7 @@ func (s *StringSliceFlag) String() string { func (s *StringSliceFlag) Set(value string) error { s.slice = splitWithEscaping(value, ",", "\\") + return nil } @@ -22,19 +23,20 @@ func (s *StringSliceFlag) Get() []string { return s.slice } -func splitWithEscaping(s, separator, escapeString string) []string { - if len(s) == 0 { +func splitWithEscaping(in, separator, escapeString string) []string { + if len(in) == 0 { return []string{} } - a := strings.Split(s, separator) + out := strings.Split(in, separator) - for i := len(a) - 2; i >= 0; i-- { - if strings.HasSuffix(a[i], escapeString) { - a[i] = a[i][:len(a[i])-len(escapeString)] + separator + a[i+1] - a = append(a[:i+1], a[i+2:]...) + //nolint:gomnd + for i := len(out) - 2; i >= 0; i-- { + if strings.HasSuffix(out[i], escapeString) { + out[i] = out[i][:len(out[i])-len(escapeString)] + separator + out[i+1] + out = append(out[:i+1], out[i+2:]...) } } - return a + return out } diff --git a/drone/types_test.go b/drone/types_test.go index f5279f3..eacca0b 100644 --- a/drone/types_test.go +++ b/drone/types_test.go @@ -20,6 +20,7 @@ func TestSplitWithEscaping(t *testing.T) { for _, test := range tests { strings := splitWithEscaping(test.Input, ",", "\\") got, want := strings, test.Output + if !reflect.DeepEqual(got, want) { t.Errorf("Got tag %v, want %v", got, want) } diff --git a/go.mod b/go.mod index 0bad209..02dc153 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/thegeeklab/drone-plugin-lib/v2 -go 1.19 +go 1.20 require ( github.com/sirupsen/logrus v1.9.0 diff --git a/trace/http.go b/trace/http.go index 6d2b682..10d6139 100644 --- a/trace/http.go +++ b/trace/http.go @@ -49,6 +49,7 @@ func HTTP(ctx context.Context) context.Context { "code": code, "header": header, }).Trace("ClientTrace.Got1xxxResponse") + return nil }, @@ -83,14 +84,14 @@ func HTTP(ctx context.Context) context.Context { logrus.Trace("ClientTrace.TLSHandshakeStart") }, - TLSHandshakeDone: func(cs tls.ConnectionState, err error) { + TLSHandshakeDone: func(connState tls.ConnectionState, err error) { logrus.WithFields(logrus.Fields{ - "version": cs.Version, - "handshake-complete": cs.HandshakeComplete, - "did-resume": cs.DidResume, - "cipher-suite": cs.CipherSuite, - "negotiated-protocol": cs.NegotiatedProtocol, - "server-name": cs.ServerName, + "version": connState.Version, + "handshake-complete": connState.HandshakeComplete, + "did-resume": connState.DidResume, + "cipher-suite": connState.CipherSuite, + "negotiated-protocol": connState.NegotiatedProtocol, + "server-name": connState.ServerName, "error": err, }).Trace("ClientTrace.TLSHandshakeDone") }, diff --git a/urfave/network.go b/urfave/network.go index 5e528e4..39653d7 100644 --- a/urfave/network.go +++ b/urfave/network.go @@ -19,6 +19,13 @@ import ( "github.com/urfave/cli/v2" ) +const ( + NetDailerTimeout = 30 * time.Second + HTTPTransportIdleTimeout = 90 * time.Second + HTTPTransportTLSHandshakeTimeout = 10 * time.Second + HTTPTransportMaxIdleConns = 100 +) + // networkFlags has the cli.Flags for the drone.Network. func networkFlags(category string) []cli.Flag { return []cli.Flag{ @@ -32,35 +39,36 @@ func networkFlags(category string) []cli.Flag { } // NetworkFromContext creates a drone.Network from the cli.Context. -func NetworkFromContext(c *cli.Context) drone.Network { +func NetworkFromContext(ctx *cli.Context) drone.Network { dialer := &net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, + Timeout: NetDailerTimeout, + KeepAlive: NetDailerTimeout, DualStack: true, } transport := &http.Transport{ Proxy: http.ProxyFromEnvironment, DialContext: dialer.DialContext, - MaxIdleConns: 100, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, + MaxIdleConns: HTTPTransportMaxIdleConns, + IdleConnTimeout: HTTPTransportIdleTimeout, + TLSHandshakeTimeout: HTTPTransportTLSHandshakeTimeout, ExpectContinueTimeout: 1 * time.Second, } - ctx := context.Background() - skipVerify := c.Bool("transport.skip-verify") + context := context.Background() + skipVerify := ctx.Bool("transport.skip-verify") if skipVerify { logrus.Warning("ssl verification is turned off") transport.TLSClientConfig = &tls.Config{ + //nolint:gosec InsecureSkipVerify: true, } } - if c.String("log-level") == logrus.TraceLevel.String() { - ctx = trace.HTTP(ctx) + if ctx.String("log-level") == logrus.TraceLevel.String() { + context = trace.HTTP(context) } client := &http.Client{ @@ -68,7 +76,7 @@ func NetworkFromContext(c *cli.Context) drone.Network { } return drone.Network{ - Context: ctx, + Context: context, SkipVerify: skipVerify, Client: client, } diff --git a/urfave/repo.go b/urfave/repo.go index 338767d..b19e2c7 100644 --- a/urfave/repo.go +++ b/urfave/repo.go @@ -11,7 +11,7 @@ import ( "github.com/urfave/cli/v2" ) -// repoFlags has the cli.Flags for the drone.Repo +// repoFlags has the cli.Flags for the drone.Repo. func repoFlags(category string) []cli.Flag { return []cli.Flag{ &cli.StringFlag{ diff --git a/urfave/urfave.go b/urfave/urfave.go index ae8b033..622abc3 100644 --- a/urfave/urfave.go +++ b/urfave/urfave.go @@ -11,19 +11,19 @@ import ( "github.com/urfave/cli/v2" ) -var ( - FlagsBuildCategory = "Drone Build Flags" - FlagsRepoCategory = "Drone Repo Flags" - FlagsCommitCategory = "Drone Commit Flags" - FlagsStageCategory = "Drone Stage Flags" - FlagsStepCategory = "Drone Step Flags" - FlagsVersioningCategory = "Drone Versioning Flags" - FlagsSystemCategory = "Drone System Flags" - FlagsPluginCategory = "Plugin Flags" -) - // Flags has the cli.Flags for the Drone plugin. func Flags() []cli.Flag { + var ( + FlagsBuildCategory = "Drone Build Flags" + FlagsRepoCategory = "Drone Repo Flags" + FlagsCommitCategory = "Drone Commit Flags" + FlagsStageCategory = "Drone Stage Flags" + FlagsStepCategory = "Drone Step Flags" + FlagsVersioningCategory = "Drone Versioning Flags" + FlagsSystemCategory = "Drone System Flags" + FlagsPluginCategory = "Plugin Flags" + ) + flags := []cli.Flag{} flags = append(flags, buildFlags(FlagsBuildCategory)...)