diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 735861149..73051431d 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -19,7 +19,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.59.1 + version: v1.60.3 - name: notify failure if: failure() && github.ref == 'refs/heads/main' env: diff --git a/.golangci.yml b/.golangci.yml index 41a679662..4ab7b147c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -116,6 +116,12 @@ linters-settings: - expected-actual go-require: ignore-http-handlers: true + gosec: + excludes: + # Flags for potentially-unsafe casting of ints, seems good, + # but currently is really unstable with no clear way to make the linter pass. + # https://github.com/securego/gosec/issues/1187 + - G115 issues: fix: true @@ -158,7 +164,6 @@ linters: - gocyclo - godot - godox - - goerr113 - gomnd - gomoddirectives - inamedparam @@ -176,13 +181,5 @@ linters: - varnamelen - wsl # Deprecated - - deadcode - - exhaustivestruct - - golint - - ifshort - - interfacer - - maligned - - nosnakecase - - structcheck - - scopelint - - varcheck + - goerr113 + - execinquery diff --git a/.pre-commit/run_linter.sh b/.pre-commit/run_linter.sh index 46a738ec7..86d167724 100755 --- a/.pre-commit/run_linter.sh +++ b/.pre-commit/run_linter.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -VERSION="1.59.1" +VERSION="1.60.3" if ! command -v golangci-lint &> /dev/null then diff --git a/app/eth2wrap/httpwrap.go b/app/eth2wrap/httpwrap.go index d257fa5d1..747ab546d 100644 --- a/app/eth2wrap/httpwrap.go +++ b/app/eth2wrap/httpwrap.go @@ -296,7 +296,6 @@ func httpPost(ctx context.Context, base string, endpoint string, body io.Reader, return nil, errors.Wrap(err, "failed to read POST response") } - //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable if res.StatusCode/100 != 2 { return nil, errors.New("post failed", z.Int("status", res.StatusCode), z.Str("body", string(data))) } diff --git a/app/expbackoff/expbackoff.go b/app/expbackoff/expbackoff.go index f76c647a8..863c37816 100644 --- a/app/expbackoff/expbackoff.go +++ b/app/expbackoff/expbackoff.go @@ -148,14 +148,14 @@ func Backoff(config Config, retries int) time.Duration { } backoff := float64(config.BaseDelay) - max := float64(config.MaxDelay) + maxVal := float64(config.MaxDelay) - for backoff < max && retries > 0 { + for backoff < maxVal && retries > 0 { backoff *= config.Multiplier retries-- } - if backoff > max { - backoff = max + if backoff > maxVal { + backoff = maxVal } // Randomize backoff delays so that if a cluster of requests start at // the same time, they won't operate in lockstep. diff --git a/app/health/checks.go b/app/health/checks.go index 0af5b4e61..ef1603829 100644 --- a/app/health/checks.go +++ b/app/health/checks.go @@ -70,12 +70,12 @@ var checks = []check{ Description: "Beacon Node in syncing state.", Severity: severityCritical, Func: func(q query, _ Metadata) (bool, error) { - max, err := q("app_monitoring_beacon_node_syncing", noLabels, gaugeMax) + maxVal, err := q("app_monitoring_beacon_node_syncing", noLabels, gaugeMax) if err != nil { return false, err } - return max == 1, nil + return maxVal == 1, nil }, }, { @@ -83,14 +83,14 @@ var checks = []check{ Description: "Not connected to at least quorum peers. Check logs for networking issue or coordinate with peers.", Severity: severityCritical, Func: func(q query, m Metadata) (bool, error) { - max, err := q("p2p_ping_success", countNonZeroLabels, gaugeMax) + maxVal, err := q("p2p_ping_success", countNonZeroLabels, gaugeMax) if err != nil { return false, err } required := float64(m.QuorumPeers) - 1 // Exclude self - return max < required, nil + return maxVal < required, nil }, }, { @@ -98,14 +98,14 @@ var checks = []check{ Description: "Pending validators detected. Activate them to start validating.", Severity: severityInfo, Func: func(q query, _ Metadata) (bool, error) { - max, err := q("core_scheduler_validator_status", + maxVal, err := q("core_scheduler_validator_status", countLabels(l("status", "pending")), gaugeMax) if err != nil { return false, err } - return max > 0, nil + return maxVal > 0, nil }, }, { @@ -140,12 +140,12 @@ var checks = []check{ Description: "Metrics reached high cardinality threshold. Please check metrics reported by app_health_metrics_high_cardinality.", Severity: severityWarning, Func: func(q query, _ Metadata) (bool, error) { - max, err := q("app_health_metrics_high_cardinality", sumLabels(), gaugeMax) + maxVal, err := q("app_health_metrics_high_cardinality", sumLabels(), gaugeMax) if err != nil { return false, err } - return max > 0, nil + return maxVal > 0, nil }, }, } diff --git a/app/health/checks_internal_test.go b/app/health/checks_internal_test.go index 2e3b17aa0..332bc6bba 100644 --- a/app/health/checks_internal_test.go +++ b/app/health/checks_internal_test.go @@ -407,19 +407,19 @@ func testCheck(t *testing.T, m Metadata, checkName string, expect bool, metrics genGauge(genLabels("bar", "bar2"), 1, 1, 1), ) - var max int - if len(metrics) > max { - max = len(metrics) + var maxVal int + if len(metrics) > maxVal { + maxVal = len(metrics) } - if len(randomFamFoo) > max { - max = len(randomFamFoo) + if len(randomFamFoo) > maxVal { + maxVal = len(randomFamFoo) } - if len(randomFamBar) > max { - max = len(randomFamBar) + if len(randomFamBar) > maxVal { + maxVal = len(randomFamBar) } - multiFams := make([][]*pb.MetricFamily, max) - for i := range max { + multiFams := make([][]*pb.MetricFamily, maxVal) + for i := range maxVal { var fam []*pb.MetricFamily if i < len(metrics) { fam = append(fam, metrics[i]) @@ -455,14 +455,14 @@ func genFam(name string, metrics ...[]*pb.Metric) []*pb.MetricFamily { typ = pb.MetricType_GAUGE } - var max int + var maxVal int for _, series := range metrics { - if len(series) > max { - max = len(series) + if len(series) > maxVal { + maxVal = len(series) } } - resp := make([]*pb.MetricFamily, max) + resp := make([]*pb.MetricFamily, maxVal) for _, series := range metrics { for i, metric := range series { if resp[i] == nil { diff --git a/app/health/reducers.go b/app/health/reducers.go index 52507cb7f..a81ab5428 100644 --- a/app/health/reducers.go +++ b/app/health/reducers.go @@ -29,16 +29,16 @@ func increase(samples []*pb.Metric) (float64, error) { // gaugeMax returns the maximum value in a time series of gauge metrics. func gaugeMax(samples []*pb.Metric) (float64, error) { - var max float64 + var maxVal float64 for _, sample := range samples { if sample.GetGauge() == nil { return 0, errors.New("bug: non-gauge metric passed") } - if sample.GetGauge().GetValue() > max { - max = sample.GetGauge().GetValue() + if sample.GetGauge().GetValue() > maxVal { + maxVal = sample.GetGauge().GetValue() } } - return max, nil + return maxVal, nil } diff --git a/app/health/select.go b/app/health/select.go index 3d42f5b68..a1c17d62e 100644 --- a/app/health/select.go +++ b/app/health/select.go @@ -15,8 +15,8 @@ type labelSelector func(*pb.MetricFamily) (*pb.Metric, error) // maxLabel returns the metric with the highest value. func maxLabel(metricsFam *pb.MetricFamily) *pb.Metric { //nolint: unused // This is used in the future. var ( - max float64 - resp *pb.Metric + maxVal float64 + resp *pb.Metric ) for _, metric := range metricsFam.GetMetric() { var val float64 @@ -29,8 +29,8 @@ func maxLabel(metricsFam *pb.MetricFamily) *pb.Metric { //nolint: unused // This panic("invalid metric type for simple value labelSelector") } - if max == 0 || val > max { - max = val + if maxVal == 0 || val > maxVal { + maxVal = val resp = metric } } diff --git a/app/log/loki/client.go b/app/log/loki/client.go index e1b3c18c9..327fd72be 100644 --- a/app/log/loki/client.go +++ b/app/log/loki/client.go @@ -210,7 +210,6 @@ func send(ctx context.Context, client *http.Client, endpoint string, batch *batc } defer resp.Body.Close() - //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable if resp.StatusCode/100 != 2 { scanner := bufio.NewScanner(io.LimitReader(resp.Body, maxErrMsgLen)) line := "" diff --git a/app/obolapi/api.go b/app/obolapi/api.go index 0c58a67b4..4ce60a1a8 100644 --- a/app/obolapi/api.go +++ b/app/obolapi/api.go @@ -123,7 +123,6 @@ func httpPost(ctx context.Context, url *url.URL, b []byte) error { return errors.Wrap(err, "failed to read POST response") } - //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable if res.StatusCode/100 != 2 { return errors.New("post failed", z.Int("status", res.StatusCode), z.Str("body", string(data))) } diff --git a/cluster/helpers.go b/cluster/helpers.go index 788bdc696..8ff3ee1ef 100644 --- a/cluster/helpers.go +++ b/cluster/helpers.go @@ -40,7 +40,6 @@ func FetchDefinition(ctx context.Context, url string) (Definition, error) { return Definition{}, errors.Wrap(err, "fetch file") } - //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable if resp.StatusCode/100 != 2 { return Definition{}, errors.New("http error", z.Int("status_code", resp.StatusCode)) } diff --git a/cmd/markdown_internal_test.go b/cmd/markdown_internal_test.go index b46a95a47..98815d0e3 100644 --- a/cmd/markdown_internal_test.go +++ b/cmd/markdown_internal_test.go @@ -205,6 +205,7 @@ func writeMarkdown(t *testing.T, file string, tpl *template.Template, data any) content, err := os.ReadFile(file) require.NoError(t, err) + //nolint:testifylint // don't remove fmt.Sprintf, it's not unnecessary require.Equal(t, string(content), result, fmt.Sprintf("%s doesn't contain latest metrics.\n"+ "To fix, run: go test github.com/obolnetwork/charon/cmd -update-markdown", file)) diff --git a/cmd/testperformance.go b/cmd/testperformance.go index 7acd4b839..69e5980d4 100644 --- a/cmd/testperformance.go +++ b/cmd/testperformance.go @@ -656,7 +656,6 @@ func fetchOoklaServer(_ context.Context, conf *testPerformanceConfig) (speedtest } if len(conf.InternetTestServersExclude) != 0 { - var targets speedtest.Servers for _, server := range serverList { if !slices.Contains(conf.InternetTestServersExclude, server.Name) { targets = append(targets, server) diff --git a/core/aggsigdb/memory_v2_internal_test.go b/core/aggsigdb/memory_v2_internal_test.go index 1b15cb946..6bd30a5be 100644 --- a/core/aggsigdb/memory_v2_internal_test.go +++ b/core/aggsigdb/memory_v2_internal_test.go @@ -35,8 +35,8 @@ func TestDutyExpirationV2(t *testing.T) { deadliner.Expire() - require.Zero(t, len(db.data)) - require.Zero(t, len(db.keysByDuty)) + require.Empty(t, db.data) + require.Empty(t, db.keysByDuty) } func TestCancelledQueryV2(t *testing.T) { diff --git a/core/tracker/tracker.go b/core/tracker/tracker.go index f9bf3d695..728c0e6f7 100644 --- a/core/tracker/tracker.go +++ b/core/tracker/tracker.go @@ -196,6 +196,7 @@ func dutyFailedStep(es []event) (bool, step, error) { } // Final step was successful. + //nolint:gosec // false positive slice index out of range if lastEvent.step == lastStep(es[0].duty.Type) && lastEvent.stepErr == nil { return false, zero, nil } diff --git a/core/validatorapi/router.go b/core/validatorapi/router.go index 6de62dbb9..8abd8af58 100644 --- a/core/validatorapi/router.go +++ b/core/validatorapi/router.go @@ -1144,7 +1144,6 @@ func writeError(ctx context.Context, w http.ResponseWriter, endpoint string, err } } - //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable if aerr.StatusCode/100 == 4 { // 4xx status codes are client errors (not server), so log as debug only. log.Debug(ctx, "Validator api 4xx response", diff --git a/eth2util/keymanager/keymanager.go b/eth2util/keymanager/keymanager.go index fdfaed76b..124f0dfab 100644 --- a/eth2util/keymanager/keymanager.go +++ b/eth2util/keymanager/keymanager.go @@ -110,7 +110,6 @@ func postKeys(ctx context.Context, addr, authToken string, reqBody keymanagerReq } _ = resp.Body.Close() - //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable if resp.StatusCode/100 != 2 { return errors.New("failed posting keys", z.Int("status", resp.StatusCode), z.Str("body", string(data))) } diff --git a/p2p/bootnode.go b/p2p/bootnode.go index 53d7f3380..4ebd4495b 100644 --- a/p2p/bootnode.go +++ b/p2p/bootnode.go @@ -153,7 +153,7 @@ func queryRelayAddrs(ctx context.Context, relayURL string, backoff func(), lockH if err != nil { log.Warn(ctx, "Failure querying relay addresses (will try again)", err) continue - } else if resp.StatusCode/100 != 2 { //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable + } else if resp.StatusCode/100 != 2 { log.Warn(ctx, "Non-200 response querying relay addresses (will try again)", nil, z.Int("status_code", resp.StatusCode)) continue } diff --git a/testutil/promrated/rated.go b/testutil/promrated/rated.go index bc2e08155..ed14c384f 100644 --- a/testutil/promrated/rated.go +++ b/testutil/promrated/rated.go @@ -106,7 +106,7 @@ func queryRatedAPI(ctx context.Context, url *url.URL, ratedAuth string, network backoff() continue - } else if res.StatusCode/100 != 2 { //nolint:usestdlibvars // we should not replace 100 with http.StatusContinue, it makes it less readable + } else if res.StatusCode/100 != 2 { incRatedErrors(res.StatusCode) return nil, errors.New("not ok http response", z.Str("body", string(body)))