diff --git a/.github/workflows/ci_golangci-lint.yml b/.github/workflows/ci_golangci-lint.yml index bf0f57f22..4ea56711d 100644 --- a/.github/workflows/ci_golangci-lint.yml +++ b/.github/workflows/ci_golangci-lint.yml @@ -19,14 +19,22 @@ jobs: name: lint runs-on: ubuntu-latest steps: + - uses: actions/setup-go@v3 - uses: actions/checkout@v3 - name: golangci-lint - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v3 with: - # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.45.2 + # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version + version: v1.45 # Optional: golangci-lint command line arguments. - args: --issues-exit-code=0 --timeout 5m - only-new-issues: true - + args: --issues-exit-code=1 --timeout 5m + # Optional: show only new issues if it's a pull request. The default value is `false`. + only-new-issues: false + # Optional: if set to true then the all caching functionality will be complete disabled, + # takes precedence over all other caching options. + skip-cache: false + # Optional: if set to true then the action don't cache or restore ~/go/pkg. + skip-pkg-cache: false + # Optional: if set to true then the action don't cache or restore ~/.cache/go-build. + skip-build-cache: false diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..2f6372487 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,247 @@ +# see https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml + +run: + skip-dirs: + - pkg/time/rate + skip-files: + - pkg/database/ent/generate.go + +linters-settings: + gocyclo: + min-complexity: 30 + + funlen: + # Checks the number of lines in a function. + # If lower than 0, disable the check. + # Default: 60 + lines: -1 + # Checks the number of statements in a function. + # If lower than 0, disable the check. + # Default: 40 + statements: -1 + + govet: + check-shadowing: true + lll: + line-length: 140 + misspell: + locale: US + nolintlint: + allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) + allow-unused: false # report any unused nolint directives + require-explanation: false # don't require an explanation for nolint directives + require-specific: false # don't require nolint directives to be specific about which linter is being skipped + +linters: + enable-all: true + disable: + # + # DEPRECATED by golangi-lint + # + - golint # [deprecated]: Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes + - interfacer # [deprecated]: Linter that suggests narrower interface types + - maligned # [deprecated]: Tool to detect Go structs that would take less memory if their fields were sorted + - scopelint # [deprecated]: Scopelint checks for unpinned variables in go programs + + # + # Enabled + # + # - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers + # - bidichk # Checks for dangerous unicode character sequences + # - decorder # check declaration order and count of types, constants, variables and functions + # - depguard # Go linter that checks if package imports are in a list of acceptable packages + # - durationcheck # check for two durations multiplied together + # - exportloopref # checks for pointers to enclosing loop variables + # - funlen # Tool for detection of long functions + # - gochecknoinits # Checks that no init functions are present in Go code + # - goheader # Checks is file header matches to pattern + # - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. + # - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. + # - goprintffuncname # Checks that printf-like functions are named with `f` at the end + # - grouper # An analyzer to analyze expression groups. + # - importas # Enforces consistent import aliases + # - makezero # Finds slice declarations with non-zero initial length + # - nolintlint # Reports ill-formed or insufficient nolint directives + # - rowserrcheck # checks whether Err of rows is checked successfully + # - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. + # - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 + # - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes + # - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code + # - varcheck # Finds unused global variables and constants + + # + # Enabled by default by golangci (but requires fixing current issues, see at the end of this file) There + # is some redundancy, but never 1 to 1 (staticcheck seems to find more + # cases than ineffassign, deadcore more than unused..). + # + # - deadcode # Finds unused code + # - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases + # - gosimple # (megacheck): Linter for Go source code that specializes in simplifying a code + # - govet # (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + # - ineffassign # Detects when assignments to existing variables are not used + # - staticcheck # (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks + # - structcheck # Finds unused struct fields + # - unused # (megacheck): Checks Go code for unused constants, variables, functions and types + + # + # Recommended? (easy) + # + - errchkjson # Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occations, where the check for the returned error can be omitted. + - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. + - forcetypeassert # finds forced type assertions + - gci # Gci control golang package import order and make it always deterministic. + - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification + - goimports # In addition to fixing imports, goimports also formats your code in the same style as gofmt. + - gosec # (gas): Inspects source code for security problems + - misspell # Finds commonly misspelled English words in comments + - nakedret # Finds naked returns in functions greater than a specified function length + - nilerr # Finds the code that returns nil even if it checks that the error is not nil. + - predeclared # find code that shadows one of Go's predeclared identifiers + - promlinter # Check Prometheus metrics naming via promlint + - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. + - unconvert # Remove unnecessary type conversions + - wastedassign # wastedassign finds wasted assignment statements. + - gocritic # Provides diagnostics that check for bugs, performance and style issues. + - exhaustive # check exhaustiveness of enum switch statements + - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers + - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) + - wrapcheck # Checks that errors returned from external packages are wrapped + - lll # Reports long lines + - ifshort # Checks that your code uses short syntax for if-statements whenever possible + - godot # Check if comments end in a period + + # + # Recommended? (requires some work) + # + + - bodyclose # checks whether HTTP response body is closed successfully + - containedctx # containedctx is a linter that detects struct contained context.Context field + - contextcheck # check the function whether use a non-inherited context + - nilnil # Checks that there is no simultaneous return of `nil` error and an invalid value. + - noctx # noctx finds sending http request without context.Context + - unparam # Reports unused function parameters + - errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. + - gomnd # An analyzer to detect magic numbers. + - ireturn # Accept Interfaces, Return Concrete Types + + # + # Formatting only, useful in IDE but should not be forced on CI? + # + + - gofumpt # Gofumpt checks whether code was gofumpt-ed. + - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity + - whitespace # Tool for detection of leading and trailing whitespace + - wsl # Whitespace Linter - Forces you to use empty lines! + + # + # Well intended, but not ready for this + # + - paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test + - cyclop # checks function and package cyclomatic complexity + - gocognit # Computes and checks the cognitive complexity of functions + - maintidx # maintidx measures the maintainability index of each function. + - goerr113 # Golang linter to check the errors handling expressions + - nestif # Reports deeply nested if statements + - gocyclo # Computes and checks the cyclomatic complexity of functions + - godox # Tool for detection of FIXME, TODO and other comment keywords + - dupl # Tool for code clone detection + - testpackage # linter that makes you use a separate _test package + + # + # Too strict (for now?) + # + - forbidigo # Forbids identifiers + - tagliatelle # Checks the struct tags. + - varnamelen # checks that the length of a variable's name matches its scope + - gochecknoglobals # check that no global variables exist + - exhaustivestruct # Checks if all struct's fields are initialized + - goconst # Finds repeated strings that could be replaced by a constant + - stylecheck # Stylecheck is a replacement for golint + + # + # Under evaluation + # + + - prealloc # Finds slice declarations that could potentially be preallocated + + +issues: + exclude-rules: + - path: go.mod + text: "replacement are not allowed: golang.org/x/time/rate" + + # `err` is often shadowed, we may continue to do it + - linters: + - govet + text: "shadow: declaration of \"err\" shadows declaration" + + # + # govet + # + + - linters: + - govet + text: "shadow: declaration of .* shadows declaration" + - linters: + - govet + text: "copylocks: assignment copies lock value to newStream:" + - linters: + - govet + text: "composites: .* composite literal uses unkeyed fields" + + # + # errcheck + # + + - linters: + - errcheck + text: "Error return value of `.*` is not checked" + + # + # staticcheck + # + + - linters: + - staticcheck + text: "SA4009: argument .* is overwritten before first use" + - linters: + - staticcheck + text: "SA4009.related information.: assignment to .*" + - linters: + - staticcheck + text: "SA4006: this value of .* is never used" + - linters: + - staticcheck + text: "SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead" + + # + # gosimple + # + + - linters: + - gosimple + text: "S1023: redundant .* statement" + - linters: + - gosimple + text: "S1000: should use a simple channel send/receive instead of `select` with a single case" + - linters: + - gosimple + text: "S1028: should use .* instead of .*" + + # + # deadcode + # + + - linters: + - deadcode + - unused + - structcheck + text: ".* is unused" + + # + # ineffassign + # + + - linters: + - ineffassign + text: "ineffectual assignment to .*" diff --git a/cmd/crowdsec-cli/hubtest.go b/cmd/crowdsec-cli/hubtest.go index 9c3a8b372..0ec77fb1d 100644 --- a/cmd/crowdsec-cli/hubtest.go +++ b/cmd/crowdsec-cli/hubtest.go @@ -533,7 +533,7 @@ cscli hubtest create my-scenario-test --parsers crowdsecurity/nginx --scenarios if err != nil { log.Fatalf(err.Error()) } - fmt.Printf(output) + fmt.Print(output) } }, } diff --git a/cmd/crowdsec/serve.go b/cmd/crowdsec/serve.go index 9150b5784..1b50d93fb 100644 --- a/cmd/crowdsec/serve.go +++ b/cmd/crowdsec/serve.go @@ -18,7 +18,7 @@ import ( //"github.com/sevlyar/go-daemon" ) -//debugHandler is kept as a dev convenience : it shuts down and serialize internal state +// debugHandler is kept as a dev convenience : it shuts down and serialize internal state func debugHandler(sig os.Signal, cConfig *csconfig.Config) error { var tmpFile string var err error diff --git a/pkg/acquisition/acquisition_test.go b/pkg/acquisition/acquisition_test.go index 3e80b8979..282aa6841 100644 --- a/pkg/acquisition/acquisition_test.go +++ b/pkg/acquisition/acquisition_test.go @@ -371,10 +371,8 @@ func (f *MockTail) StreamingAcquisition(out chan types.Event, t *tomb.Tomb) erro evt.Line.Src = "test" out <- evt } - select { - case <-t.Dying(): - return nil - } + <-t.Dying() + return nil } func (f *MockTail) CanRun() error { return nil } func (f *MockTail) GetMetrics() []prometheus.Collector { return nil } diff --git a/pkg/apiclient/alerts_service_test.go b/pkg/apiclient/alerts_service_test.go index 888a1cbe4..096268bfe 100644 --- a/pkg/apiclient/alerts_service_test.go +++ b/pkg/apiclient/alerts_service_test.go @@ -399,7 +399,7 @@ func TestAlertsGetAsMachine(t *testing.T) { } //fail - alerts, resp, err = client.Alerts.GetByID(context.Background(), 2) + _, resp, err = client.Alerts.GetByID(context.Background(), 2) assert.Contains(t, fmt.Sprintf("%s", err), "API error: object not found") } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index b8228383f..942cc4c99 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -190,7 +190,6 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) { router.NoRoute(func(c *gin.Context) { c.JSON(http.StatusNotFound, gin.H{"message": "Page or Method not found"}) - return }) router.Use(CustomRecoveryWithWriter()) diff --git a/pkg/apiserver/controllers/v1/decisions.go b/pkg/apiserver/controllers/v1/decisions.go index f8369ab9a..2a1edaebd 100644 --- a/pkg/apiserver/controllers/v1/decisions.go +++ b/pkg/apiserver/controllers/v1/decisions.go @@ -82,7 +82,6 @@ func (c *Controller) DeleteDecisionById(gctx *gin.Context) { } gctx.JSON(http.StatusOK, deleteDecisionResp) - return } func (c *Controller) DeleteDecisions(gctx *gin.Context) { @@ -98,7 +97,6 @@ func (c *Controller) DeleteDecisions(gctx *gin.Context) { } gctx.JSON(http.StatusOK, deleteDecisionResp) - return } func (c *Controller) StreamDecision(gctx *gin.Context) { @@ -210,5 +208,4 @@ func (c *Controller) StreamDecision(gctx *gin.Context) { } gctx.JSON(http.StatusOK, ret) - return } diff --git a/pkg/apiserver/controllers/v1/machines.go b/pkg/apiserver/controllers/v1/machines.go index 307b240a0..1f4b339c0 100644 --- a/pkg/apiserver/controllers/v1/machines.go +++ b/pkg/apiserver/controllers/v1/machines.go @@ -27,5 +27,4 @@ func (c *Controller) CreateMachine(gctx *gin.Context) { } gctx.Status(http.StatusCreated) - return } diff --git a/pkg/apiserver/jwt_test.go b/pkg/apiserver/jwt_test.go index 6d9ba27be..8a83d2289 100644 --- a/pkg/apiserver/jwt_test.go +++ b/pkg/apiserver/jwt_test.go @@ -46,7 +46,7 @@ func TestLogin(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, 401, w.Code) - assert.Equal(t, "{\"code\":401,\"message\":\"missing : invalid character 'e' in literal true (expecting 'r')\"}", w.Body.String()) + assert.Equal(t, "{\"code\":401,\"message\":\"missing: invalid character 'e' in literal true (expecting 'r')\"}", w.Body.String()) // Login with invalid format w = httptest.NewRecorder() diff --git a/pkg/apiserver/middlewares/v1/jwt.go b/pkg/apiserver/middlewares/v1/jwt.go index c531ad6dc..21b178227 100644 --- a/pkg/apiserver/middlewares/v1/jwt.go +++ b/pkg/apiserver/middlewares/v1/jwt.go @@ -7,14 +7,13 @@ import ( "strings" "time" - "errors" - jwt "github.com/appleboy/gin-jwt/v2" "github.com/crowdsecurity/crowdsec/pkg/database" "github.com/crowdsecurity/crowdsec/pkg/database/ent/machine" "github.com/crowdsecurity/crowdsec/pkg/models" "github.com/gin-gonic/gin" "github.com/go-openapi/strfmt" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "golang.org/x/crypto/bcrypt" ) @@ -48,7 +47,7 @@ func (j *JWT) Authenticator(c *gin.Context) (interface{}, error) { var scenarios string var err error if err := c.ShouldBindJSON(&loginInput); err != nil { - return "", errors.New(fmt.Sprintf("missing : %v", err.Error())) + return "", errors.Wrap(err, "missing") } if err := loginInput.Validate(strfmt.Default); err != nil { return "", errors.New("input format error") diff --git a/pkg/metabase/database.go b/pkg/metabase/database.go index 5ba577875..0a7890fac 100644 --- a/pkg/metabase/database.go +++ b/pkg/metabase/database.go @@ -89,7 +89,7 @@ func (d *Database) Update() error { return errors.Wrap(err, "update sqlite db response (unmarshal)") } model.Details = d.Details - success, errormsg, err = d.Client.Do("PUT", routes[databaseEndpoint], model) + _, errormsg, err = d.Client.Do("PUT", routes[databaseEndpoint], model) if err != nil { return err }