From bbe77529670664acee12780fa33ff3c0f52bf885 Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:14:56 +0100 Subject: [PATCH] update golangci-lint to 1.62 (#3332) - ensure consistent pointer/value receivers - testify: json assertions with dedicated methods --- .github/workflows/go-tests-windows.yml | 2 +- .github/workflows/go-tests.yml | 2 +- .golangci.yml | 13 ++++++++-- .../clinotifications/notifications.go | 2 +- cmd/crowdsec/main.go | 4 ++-- pkg/apiserver/alerts_test.go | 24 +++++++++---------- pkg/apiserver/decisions_test.go | 16 ++++++------- pkg/apiserver/jwt_test.go | 10 ++++---- pkg/apiserver/machines_test.go | 6 ++--- pkg/appsec/coraza_logger.go | 2 +- pkg/cache/cache_test.go | 13 +++++----- 11 files changed, 52 insertions(+), 42 deletions(-) diff --git a/.github/workflows/go-tests-windows.yml b/.github/workflows/go-tests-windows.yml index 2966b999a4a..3276dbb1bfd 100644 --- a/.github/workflows/go-tests-windows.yml +++ b/.github/workflows/go-tests-windows.yml @@ -61,6 +61,6 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.61 + version: v1.62 args: --issues-exit-code=1 --timeout 10m only-new-issues: false diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 3f4aa67e139..3638696b4f6 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -190,6 +190,6 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.61 + version: v1.62 args: --issues-exit-code=1 --timeout 10m only-new-issues: false diff --git a/.golangci.yml b/.golangci.yml index acde901dbe6..fd595994e7c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -211,9 +211,7 @@ linters: # # DEPRECATED by golangi-lint # - - execinquery - exportloopref - - gomnd # # Redundant @@ -456,3 +454,14 @@ issues: - revive path: "cmd/crowdsec/win_service.go" text: "deep-exit: .*" + + - linters: + - recvcheck + path: "pkg/csplugin/hclog_adapter.go" + text: 'the methods of "HCLogAdapter" use pointer receiver and non-pointer receiver.' + + # encoding to json/yaml requires value receivers + - linters: + - recvcheck + path: "pkg/cwhub/item.go" + text: 'the methods of "Item" use pointer receiver and non-pointer receiver.' diff --git a/cmd/crowdsec-cli/clinotifications/notifications.go b/cmd/crowdsec-cli/clinotifications/notifications.go index baf899c10cf..80ffebeaa23 100644 --- a/cmd/crowdsec-cli/clinotifications/notifications.go +++ b/cmd/crowdsec-cli/clinotifications/notifications.go @@ -260,7 +260,7 @@ func (cli *cliNotifications) notificationConfigFilter(cmd *cobra.Command, args [ return ret, cobra.ShellCompDirectiveNoFileComp } -func (cli cliNotifications) newTestCmd() *cobra.Command { +func (cli *cliNotifications) newTestCmd() *cobra.Command { var ( pluginBroker csplugin.PluginBroker pluginTomb tomb.Tomb diff --git a/cmd/crowdsec/main.go b/cmd/crowdsec/main.go index 6d8ca24c335..e414f59f3e2 100644 --- a/cmd/crowdsec/main.go +++ b/cmd/crowdsec/main.go @@ -148,14 +148,14 @@ func (l *labelsMap) String() string { return "labels" } -func (l labelsMap) Set(label string) error { +func (l *labelsMap) Set(label string) error { for _, pair := range strings.Split(label, ",") { split := strings.Split(pair, ":") if len(split) != 2 { return fmt.Errorf("invalid format for label '%s', must be key:value", pair) } - l[split[0]] = split[1] + (*l)[split[0]] = split[1] } return nil diff --git a/pkg/apiserver/alerts_test.go b/pkg/apiserver/alerts_test.go index d86234e4813..346619bf691 100644 --- a/pkg/apiserver/alerts_test.go +++ b/pkg/apiserver/alerts_test.go @@ -121,14 +121,14 @@ func TestCreateAlert(t *testing.T) { w := lapi.RecordResponse(t, ctx, http.MethodPost, "/v1/alerts", strings.NewReader("test"), "password") assert.Equal(t, 400, w.Code) - assert.Equal(t, `{"message":"invalid character 'e' in literal true (expecting 'r')"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"invalid character 'e' in literal true (expecting 'r')"}`, w.Body.String()) // Create Alert with invalid input alertContent := GetAlertReaderFromFile(t, "./tests/invalidAlert_sample.json") w = lapi.RecordResponse(t, ctx, http.MethodPost, "/v1/alerts", alertContent, "password") assert.Equal(t, 500, w.Code) - assert.Equal(t, + assert.JSONEq(t, `{"message":"validation failure list:\n0.scenario in body is required\n0.scenario_hash in body is required\n0.scenario_version in body is required\n0.simulated in body is required\n0.source in body is required"}`, w.Body.String()) @@ -176,7 +176,7 @@ func TestAlertListFilters(t *testing.T) { w := lapi.RecordResponse(t, ctx, "GET", "/v1/alerts?test=test", alertContent, "password") assert.Equal(t, 500, w.Code) - assert.Equal(t, `{"message":"Filter parameter 'test' is unknown (=test): invalid filter"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"Filter parameter 'test' is unknown (=test): invalid filter"}`, w.Body.String()) // get without filters @@ -242,7 +242,7 @@ func TestAlertListFilters(t *testing.T) { w = lapi.RecordResponse(t, ctx, "GET", "/v1/alerts?ip=gruueq", emptyBody, "password") assert.Equal(t, 500, w.Code) - assert.Equal(t, `{"message":"unable to convert 'gruueq' to int: invalid address: invalid ip address / range"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"unable to convert 'gruueq' to int: invalid address: invalid ip address / range"}`, w.Body.String()) // test range (ok) @@ -261,7 +261,7 @@ func TestAlertListFilters(t *testing.T) { w = lapi.RecordResponse(t, ctx, "GET", "/v1/alerts?range=ratata", emptyBody, "password") assert.Equal(t, 500, w.Code) - assert.Equal(t, `{"message":"unable to convert 'ratata' to int: invalid address: invalid ip address / range"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"unable to convert 'ratata' to int: invalid address: invalid ip address / range"}`, w.Body.String()) // test since (ok) @@ -332,7 +332,7 @@ func TestAlertListFilters(t *testing.T) { w = lapi.RecordResponse(t, ctx, "GET", "/v1/alerts?has_active_decision=ratatqata", emptyBody, "password") assert.Equal(t, 500, w.Code) - assert.Equal(t, `{"message":"'ratatqata' is not a boolean: strconv.ParseBool: parsing \"ratatqata\": invalid syntax: unable to parse type"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"'ratatqata' is not a boolean: strconv.ParseBool: parsing \"ratatqata\": invalid syntax: unable to parse type"}`, w.Body.String()) } func TestAlertBulkInsert(t *testing.T) { @@ -354,7 +354,7 @@ func TestListAlert(t *testing.T) { w := lapi.RecordResponse(t, ctx, "GET", "/v1/alerts?test=test", emptyBody, "password") assert.Equal(t, 500, w.Code) - assert.Equal(t, `{"message":"Filter parameter 'test' is unknown (=test): invalid filter"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"Filter parameter 'test' is unknown (=test): invalid filter"}`, w.Body.String()) // List Alert @@ -397,7 +397,7 @@ func TestDeleteAlert(t *testing.T) { req.RemoteAddr = "127.0.0.2:4242" lapi.router.ServeHTTP(w, req) assert.Equal(t, 403, w.Code) - assert.Equal(t, `{"message":"access forbidden from this IP (127.0.0.2)"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"access forbidden from this IP (127.0.0.2)"}`, w.Body.String()) // Delete Alert w = httptest.NewRecorder() @@ -406,7 +406,7 @@ func TestDeleteAlert(t *testing.T) { req.RemoteAddr = "127.0.0.1:4242" lapi.router.ServeHTTP(w, req) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"1"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"1"}`, w.Body.String()) } func TestDeleteAlertByID(t *testing.T) { @@ -421,7 +421,7 @@ func TestDeleteAlertByID(t *testing.T) { req.RemoteAddr = "127.0.0.2:4242" lapi.router.ServeHTTP(w, req) assert.Equal(t, 403, w.Code) - assert.Equal(t, `{"message":"access forbidden from this IP (127.0.0.2)"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"access forbidden from this IP (127.0.0.2)"}`, w.Body.String()) // Delete Alert w = httptest.NewRecorder() @@ -430,7 +430,7 @@ func TestDeleteAlertByID(t *testing.T) { req.RemoteAddr = "127.0.0.1:4242" lapi.router.ServeHTTP(w, req) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"1"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"1"}`, w.Body.String()) } func TestDeleteAlertTrustedIPS(t *testing.T) { @@ -475,7 +475,7 @@ func TestDeleteAlertTrustedIPS(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"1"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"1"}`, w.Body.String()) } lapi.InsertAlertFromFile(t, ctx, "./tests/alert_sample.json") diff --git a/pkg/apiserver/decisions_test.go b/pkg/apiserver/decisions_test.go index a0af6956443..cb5d2e1c4f1 100644 --- a/pkg/apiserver/decisions_test.go +++ b/pkg/apiserver/decisions_test.go @@ -22,19 +22,19 @@ func TestDeleteDecisionRange(t *testing.T) { // delete by ip wrong w := lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?range=1.2.3.0/24", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"0"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"0"}`, w.Body.String()) // delete by range w = lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?range=91.121.79.0/24&contains=false", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"2"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"2"}`, w.Body.String()) // delete by range : ensure it was already deleted w = lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?range=91.121.79.0/24", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"0"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"0"}`, w.Body.String()) } func TestDeleteDecisionFilter(t *testing.T) { @@ -48,19 +48,19 @@ func TestDeleteDecisionFilter(t *testing.T) { w := lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?ip=1.2.3.4", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"0"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"0"}`, w.Body.String()) // delete by ip good w = lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?ip=91.121.79.179", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"1"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"1"}`, w.Body.String()) // delete by scope/value w = lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?scopes=Ip&value=91.121.79.178", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"1"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"1"}`, w.Body.String()) } func TestDeleteDecisionFilterByScenario(t *testing.T) { @@ -74,13 +74,13 @@ func TestDeleteDecisionFilterByScenario(t *testing.T) { w := lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?scenario=crowdsecurity/ssh-bff", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"0"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"0"}`, w.Body.String()) // delete by scenario good w = lapi.RecordResponse(t, ctx, "DELETE", "/v1/decisions?scenario=crowdsecurity/ssh-bf", emptyBody, PASSWORD) assert.Equal(t, 200, w.Code) - assert.Equal(t, `{"nbDeleted":"2"}`, w.Body.String()) + assert.JSONEq(t, `{"nbDeleted":"2"}`, w.Body.String()) } func TestGetDecisionFilters(t *testing.T) { diff --git a/pkg/apiserver/jwt_test.go b/pkg/apiserver/jwt_test.go index f6f51763975..72ae0302ae4 100644 --- a/pkg/apiserver/jwt_test.go +++ b/pkg/apiserver/jwt_test.go @@ -23,7 +23,7 @@ func TestLogin(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, 401, w.Code) - assert.Equal(t, `{"code":401,"message":"machine test not validated"}`, w.Body.String()) + assert.JSONEq(t, `{"code":401,"message":"machine test not validated"}`, w.Body.String()) // Login with machine not exist w = httptest.NewRecorder() @@ -32,7 +32,7 @@ func TestLogin(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, 401, w.Code) - assert.Equal(t, `{"code":401,"message":"ent: machine not found"}`, w.Body.String()) + assert.JSONEq(t, `{"code":401,"message":"ent: machine not found"}`, w.Body.String()) // Login with invalid body w = httptest.NewRecorder() @@ -41,7 +41,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.JSONEq(t, `{"code":401,"message":"missing: invalid character 'e' in literal true (expecting 'r')"}`, w.Body.String()) // Login with invalid format w = httptest.NewRecorder() @@ -50,7 +50,7 @@ func TestLogin(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, 401, w.Code) - assert.Equal(t, `{"code":401,"message":"validation failure list:\npassword in body is required"}`, w.Body.String()) + assert.JSONEq(t, `{"code":401,"message":"validation failure list:\npassword in body is required"}`, w.Body.String()) // Validate machine ValidateMachine(t, ctx, "test", config.API.Server.DbConfig) @@ -62,7 +62,7 @@ func TestLogin(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, 401, w.Code) - assert.Equal(t, `{"code":401,"message":"incorrect Username or Password"}`, w.Body.String()) + assert.JSONEq(t, `{"code":401,"message":"incorrect Username or Password"}`, w.Body.String()) // Login with valid machine w = httptest.NewRecorder() diff --git a/pkg/apiserver/machines_test.go b/pkg/apiserver/machines_test.go index 969f75707d6..57b96f54ddd 100644 --- a/pkg/apiserver/machines_test.go +++ b/pkg/apiserver/machines_test.go @@ -25,7 +25,7 @@ func TestCreateMachine(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Equal(t, `{"message":"invalid character 'e' in literal true (expecting 'r')"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"invalid character 'e' in literal true (expecting 'r')"}`, w.Body.String()) // Create machine with invalid input w = httptest.NewRecorder() @@ -34,7 +34,7 @@ func TestCreateMachine(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnprocessableEntity, w.Code) - assert.Equal(t, `{"message":"validation failure list:\nmachine_id in body is required\npassword in body is required"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"validation failure list:\nmachine_id in body is required\npassword in body is required"}`, w.Body.String()) // Create machine b, err := json.Marshal(MachineTest) @@ -144,7 +144,7 @@ func TestCreateMachineAlreadyExist(t *testing.T) { router.ServeHTTP(w, req) assert.Equal(t, http.StatusForbidden, w.Code) - assert.Equal(t, `{"message":"user 'test': user already exist"}`, w.Body.String()) + assert.JSONEq(t, `{"message":"user 'test': user already exist"}`, w.Body.String()) } func TestAutoRegistration(t *testing.T) { diff --git a/pkg/appsec/coraza_logger.go b/pkg/appsec/coraza_logger.go index d2c1612cbd7..93e31be5876 100644 --- a/pkg/appsec/coraza_logger.go +++ b/pkg/appsec/coraza_logger.go @@ -124,7 +124,7 @@ func (e *crzLogEvent) Stringer(key string, val fmt.Stringer) dbg.Event { return e } -func (e crzLogEvent) IsEnabled() bool { +func (e *crzLogEvent) IsEnabled() bool { return !e.muted } diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index a4e0bd0127a..4da9fd5bf7b 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -5,26 +5,27 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCreateSetGet(t *testing.T) { err := CacheInit(CacheCfg{Name: "test", Size: 100, TTL: 1 * time.Second}) - assert.Empty(t, err) + require.NoError(t, err) //set & get err = SetKey("test", "testkey0", "testvalue1", nil) - assert.Empty(t, err) + require.NoError(t, err) ret, err := GetKey("test", "testkey0") assert.Equal(t, "testvalue1", ret) - assert.Empty(t, err) + require.NoError(t, err) //re-set err = SetKey("test", "testkey0", "testvalue2", nil) - assert.Empty(t, err) + require.NoError(t, err) assert.Equal(t, "testvalue1", ret) - assert.Empty(t, err) + require.NoError(t, err) //expire time.Sleep(1500 * time.Millisecond) ret, err = GetKey("test", "testkey0") assert.Equal(t, "", ret) - assert.Empty(t, err) + require.NoError(t, err) }