diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 7d810253..aa1821e8 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -14,9 +14,9 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v4 with: - go-version: '^1.22' + go-version: '^1.23' - name: golangci-lint uses: golangci/golangci-lint-action@v4 with: - version: v1.57.1 + version: v1.61.0 args: cli/... discern/... elan/... flair/... grpcutil/... lucidity/... mettle/... purity/... rexclient/... zeal/... diff --git a/.golangci.yml b/.golangci.yml index 52364ac2..79f402bb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,8 +1,5 @@ run: timeout: 5m - skip-dirs: - - plz-out/ - - tests/ issues: exclude: @@ -18,6 +15,9 @@ issues: - unused-parameter # Quite a few of these, some legit, many are fulfilling interfaces - redefines-builtin # Not really a big issue - empty-block # Only came up once and was a false positive. This should be easily handled by review. + exclude-dirs: + - plz-out/ + - tests/ exclude-rules: - path: _test\.go linters: @@ -36,7 +36,7 @@ linters: - bodyclose - dogsled - dupl - - exportloopref + - copyloopvar - gocritic - gofmt - revive diff --git a/elan/rpc/gc.go b/elan/rpc/gc.go index aaf00c3c..3fc06af2 100644 --- a/elan/rpc/gc.go +++ b/elan/rpc/gc.go @@ -20,7 +20,7 @@ import ( func (s *server) List(ctx context.Context, req *ppb.ListRequest) (*ppb.ListResponse, error) { if len(req.Prefix) != 2 { - return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: "+req.Prefix) + return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: %s", req.Prefix) } var g multierror.Group resp := &ppb.ListResponse{} diff --git a/elan/rpc/tree.go b/elan/rpc/tree.go index 9be0a138..a2eb90d8 100644 --- a/elan/rpc/tree.go +++ b/elan/rpc/tree.go @@ -35,7 +35,6 @@ func (s *server) getTree(digest *pb.Digest, stopAtPack bool) ([]*pb.Directory, e var g multierror.Group var mutex sync.Mutex for _, child := range dir.Directories { - child := child g.Go(func() error { dirs, err := s.getTree(child.Digest, stopAtPack) mutex.Lock() diff --git a/flair/rpc/rpc.go b/flair/rpc/rpc.go index dedef4db..c94b6f78 100644 --- a/flair/rpc/rpc.go +++ b/flair/rpc/rpc.go @@ -181,8 +181,6 @@ func (s *server) FindMissingBlobs(ctx context.Context, req *pb.FindMissingBlobsR var g errgroup.Group var mutex sync.Mutex for srv, b := range blobs { - srv := srv - b := b g.Go(func() error { var mutex2 sync.Mutex found := make(map[string]struct{}, len(b)) @@ -235,8 +233,6 @@ func (s *server) BatchUpdateBlobs(ctx context.Context, req *pb.BatchUpdateBlobsR var mutex sync.Mutex m := make(map[string][]*pb.BatchUpdateBlobsResponse_Response, len(blobs)*s.replicator.Replicas) for srv, rs := range blobs { - srv := srv - rs := rs g.Go(func() error { return s.replicator.Parallel(srv.Start, func(srv *trie.Server) error { ctx, cancel := context.WithTimeout(ctx, s.timeout) @@ -297,8 +293,6 @@ func (s *server) BatchReadBlobs(ctx context.Context, req *pb.BatchReadBlobsReque }() for srv, d := range blobs { - srv := srv - d := d g.Go(func() error { return s.replicator.SequentialAck(srv.Start, func(s2 *trie.Server) (bool, error) { ctx, cancel := context.WithTimeout(ctx, s.timeout) @@ -367,7 +361,7 @@ func (s *server) GetTree(req *pb.GetTreeRequest, srv pb.ContentAddressableStorag } else if len(r.Responses) != 1 { return fmt.Errorf("missing blob in response") // shouldn't happen... } else if s := r.Responses[0].Status; s.Code != int32(codes.OK) { - return status.Errorf(codes.Code(s.Code), s.Message) + return status.Errorf(codes.Code(s.Code), "%s", s.Message) } resp = r return nil @@ -610,7 +604,7 @@ func (s *server) forwardMetadata(ctx context.Context) context.Context { func (s *server) List(ctx context.Context, req *ppb.ListRequest) (*ppb.ListResponse, error) { if len(req.Prefix) != 2 { - return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: "+req.Prefix) + return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: %s", req.Prefix) } var mutex sync.Mutex resp := &ppb.ListResponse{} @@ -672,7 +666,7 @@ func (s *server) List(ctx context.Context, req *ppb.ListRequest) (*ppb.ListRespo func (s *server) Delete(ctx context.Context, req *ppb.DeleteRequest) (*ppb.DeleteResponse, error) { if len(req.Prefix) != 2 { - return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: "+req.Prefix) + return nil, status.Errorf(codes.InvalidArgument, "Invalid prefix provided: %s", req.Prefix) } return &ppb.DeleteResponse{}, s.replicator.All(req.Prefix, func(srv *trie.Server) error { ctx, cancel := context.WithTimeout(ctx, 10*s.timeout) // Multiply up timeout since this operation can be expensive. diff --git a/flair/trie/replication_test.go b/flair/trie/replication_test.go index 3b45646b..417dc455 100644 --- a/flair/trie/replication_test.go +++ b/flair/trie/replication_test.go @@ -158,7 +158,7 @@ func TestErrorCode(t *testing.T) { t.Run(fmt.Sprintf("%s_%s", tc.Output, tc.Inputs), func(t *testing.T) { var merr *multierror.Error for _, input := range tc.Inputs { - merr = multierror.Append(merr, status.Errorf(input, input.String())) + merr = multierror.Append(merr, status.Errorf(input, "%s", input)) } assert.Equal(t, tc.Output, errorCode(merr)) }) diff --git a/mettle/worker/download.go b/mettle/worker/download.go index 647783ca..df2890ea 100644 --- a/mettle/worker/download.go +++ b/mettle/worker/download.go @@ -187,8 +187,6 @@ func (w *worker) downloadAllFiles(files map[sdkdigest.Digest][]fileNode, packs m g.Go(func() error { return w.downloadFiles(fileNodes) }) } for dg, paths := range packs { - dg := dg - paths := paths g.Go(func() error { return w.downloadPack(dg, paths) }) diff --git a/mettle/worker/reporting.go b/mettle/worker/reporting.go index 384ed35a..e84cbe99 100644 --- a/mettle/worker/reporting.go +++ b/mettle/worker/reporting.go @@ -173,15 +173,21 @@ func (w *worker) checkLiveConnection() bool { func (w *worker) checkConnectivity(check string) { switch check { case "gstatic": - if resp, err := http.Get("https://connectivitycheck.gstatic.com/generate_204"); err != nil { + resp, err := http.Get("https://connectivitycheck.gstatic.com/generate_204") + if err != nil { log.Fatalf("Failed to complete connectivity check: %s", err) - } else if resp.StatusCode != http.StatusNoContent { + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { log.Fatalf("Connectivity check returned unexpected status: %s", resp.Status) } case "firefox": - if resp, err := http.Get("https://detectportal.firefox.com/canonical.html"); err != nil { + resp, err := http.Get("https://detectportal.firefox.com/canonical.html") + if err != nil { log.Fatalf("Failed to complete connectivity check: %s", err) - } else if resp.StatusCode != http.StatusOK { + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { log.Fatalf("Connectivity check returned unexpected status: %s", resp.Status) } case "": diff --git a/mettle/worker/worker.go b/mettle/worker/worker.go index 3fb7cf69..f88130c9 100644 --- a/mettle/worker/worker.go +++ b/mettle/worker/worker.go @@ -242,7 +242,7 @@ func runForever(instanceName, requestQueue, responseQueue, name, storage, dir, c // our queues or something else bad happened internally so we are basically doomed // and should stop. err = fmt.Errorf("Failed to run task: %s", err) - w.Report(false, false, false, err.Error()) + w.Report(false, false, false, "%s", err) return err } } @@ -533,7 +533,7 @@ func (w *worker) runTask(msg *pubsub.Message) *pb.ExecuteResponse { // forceShutdown sends any shutdown reports and calls log.Fatal() to shut down the worker func (w *worker) forceShutdown(shutdownMsg string) { - w.Report(false, false, false, shutdownMsg) + w.Report(false, false, false, "%s", shutdownMsg) log.Info("Force shutting down worker") if w.currentMsg != nil { if w.actionDigest != nil { @@ -901,7 +901,7 @@ func (w *worker) writeUncachedResult(ar *pb.ActionResult, msg string) string { b, _ := proto.Marshal(&bbcas.UncachedActionResult{ ActionDigest: w.actionDigest, ExecuteResponse: &pb.ExecuteResponse{ - Status: status(nil, codes.Unknown, msg), + Status: status(nil, codes.Unknown, "%s", msg), Result: ar, }, }) @@ -1075,7 +1075,7 @@ func (w *worker) collectOutputs(ar *pb.ActionResult, cmd *pb.Command) error { // update sends an update on the response channel func (w *worker) update(stage pb.ExecutionStage_Value, response *pb.ExecuteResponse) error { - w.Report(true, stage == pb.ExecutionStage_EXECUTING, true, stage.String()) + w.Report(true, stage == pb.ExecutionStage_EXECUTING, true, "%s", stage) body := common.MarshalOperation(stage, w.actionDigest, response, w.metadata) ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) defer cancel() diff --git a/purity/gc/gc.go b/purity/gc/gc.go index bb0ed424..4f05ebfd 100644 --- a/purity/gc/gc.go +++ b/purity/gc/gc.go @@ -182,7 +182,6 @@ func (c *collector) LoadAllBlobs() error { var g multierror.Group var mutex sync.Mutex for i := 0; i < 16; i++ { - i := i g.Go(func() error { for j := 0; j < 16; j++ { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)