Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable all linters for ct-go #1064

Merged
merged 4 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 1 addition & 21 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,11 @@ run:

linters-settings:
gocyclo:
min-complexity: 40
min-complexity: 25
depguard:
list-type: blacklist
packages:
- ^golang.org/x/net/context$
- github.com/gogo/protobuf/proto
- encoding/asn1
- crypto/x509

linters:
disable-all: true
enable:
- depguard
- gocyclo
- gofmt
- goimports
- govet
- ineffassign
- megacheck
- misspell
- revive
- unused
# TODO(gbelvin): write license linter and commit to upstream.
# ./scripts/check_license.sh is run by ./scripts/presubmit.sh

issues:
# Don't turn off any checks by default. We can do this explicitly if needed.
exclude-use-default: false
8 changes: 6 additions & 2 deletions client/multilog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,14 @@ func TestTemporalAddChain(t *testing.T) {
switch r.URL.Path {
case "/ct/v1/add-chain":
data, _ := sctToJSON(testdata.TestCertProof)
w.Write(data)
if _, err := w.Write(data); err != nil {
t.Error(err)
}
case "/ct/v1/add-pre-chain":
data, _ := sctToJSON(testdata.TestPreCertProof)
w.Write(data)
if _, err := w.Write(data); err != nil {
t.Error(err)
}
default:
t.Fatalf("Incorrect URL path: %s", r.URL.Path)
}
Expand Down
8 changes: 6 additions & 2 deletions fixchain/chainfix/chainfix.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,18 @@ func contentStore(baseDir string, subDir string, content []byte) {
r := sha256.Sum256(content)
h := base64.URLEncoding.EncodeToString(r[:])
d := baseDir + "/" + subDir
os.MkdirAll(d, 0777)
if err := os.MkdirAll(d, 0777); err != nil {
log.Fatalf("Can't create directories %q: %v", d, err)
}
fn := d + "/" + h
f, err := os.Create(fn)
if err != nil {
log.Fatalf("Can't create %q: %s", fn, err)
}
defer f.Close()
f.Write(content)
if _, err := f.Write(content); err != nil {
log.Fatalf("Can't write to %q: %v", fn, err)
}
}

func logStringErrors(wg *sync.WaitGroup, errors chan *fixchain.FixError, baseDir string) {
Expand Down
4 changes: 3 additions & 1 deletion fixchain/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ func (l *Logger) postChain(p *toPost) {
derChain = append(derChain, ct.ASN1Cert{Data: cert.Raw})
}

l.limiter.Wait(l.ctx)
if err := l.limiter.Wait(l.ctx); err != nil {
log.Println(err)
}
atomic.AddUint32(&l.posted, 1)
_, err := l.client.AddChain(l.ctx, derChain)
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion internal/witness/cmd/witness/impl/witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ func Main(ctx context.Context, opts ServerOpts) error {
}()
<-ctx.Done()
klog.Info("Server shutting down")
hServer.Shutdown(ctx)
if err := hServer.Shutdown(ctx); err != nil {
klog.Errorf("Shutdown(): %v", err)
}
return <-e
}
13 changes: 10 additions & 3 deletions internal/witness/cmd/witness/internal/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/gorilla/mux"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/klog/v2"
)

// Server is the core handler implementation of the witness.
Expand Down Expand Up @@ -76,7 +77,9 @@ func (s *Server) update(w http.ResponseWriter, r *http.Request) {
}
}
w.Header().Set("Content-Type", "text/plain")
w.Write(sth)
if _, err := w.Write(sth); err != nil {
klog.Errorf("Write(): %v", err)
}
}

// getSTH returns an STH stored for a given log.
Expand All @@ -93,7 +96,9 @@ func (s *Server) getSTH(w http.ResponseWriter, r *http.Request) {
return
}
w.Header().Set("Content-Type", "text/plain")
w.Write(sth)
if _, err := w.Write(sth); err != nil {
klog.Errorf("Write(): %v", err)
}
}

// getLogs returns a list of all logs the witness is aware of.
Expand All @@ -109,7 +114,9 @@ func (s *Server) getLogs(w http.ResponseWriter, r *http.Request) {
return
}
w.Header().Set("Content-Type", "text/json")
w.Write(logList)
if _, err := w.Write(logList); err != nil {
klog.Errorf("Write(): %v", err)
}
}

// RegisterHandlers registers HTTP handlers for witness endpoints.
Expand Down
8 changes: 6 additions & 2 deletions internal/witness/cmd/witness/internal/http/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,17 @@ func dh(h string, expLen int) []byte {
return r
}

func mustCreateDB(t *testing.T) (*sql.DB, func() error) {
func mustCreateDB(t *testing.T) (*sql.DB, func()) {
t.Helper()
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("failed to open temporary in-memory DB: %v", err)
}
return db, db.Close
return db, func() {
if err := db.Close(); err != nil {
t.Error(err)
}
}
}

func mustCreatePK(pkPem string) crypto.PublicKey {
Expand Down
7 changes: 6 additions & 1 deletion internal/witness/cmd/witness/internal/witness/witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/transparency-dev/merkle/rfc6962"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/klog/v2"
)

// Opts is the options passed to a witness.
Expand Down Expand Up @@ -174,7 +175,11 @@ func (w *Witness) Update(ctx context.Context, logID string, nextRaw []byte, pf [
if err != nil {
return nil, fmt.Errorf("couldn't create db tx: %v", err)
}
defer tx.Rollback()
defer func() {
if err := tx.Rollback(); err != nil {
klog.Errorf("Rollback(): %v", err)
}
}()

// Get the latest STH (if one exists).
prevRaw, err := w.getLatestSTH(tx.QueryRow, logID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,17 @@ func dh(h string, expLen int) []byte {
return r
}

func mustCreateDB(t *testing.T) (*sql.DB, func() error) {
func mustCreateDB(t *testing.T) (*sql.DB, func()) {
t.Helper()
db, err := sql.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("failed to open temporary in-memory DB: %v", err)
}
return db, db.Close
return db, func() {
if err := db.Close(); err != nil {
t.Error(err)
}
}
}

func TestGetLogs(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion preload/preloader/preloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,9 @@ func main() {
}
precerts <- entry
}
s.Scan(ctx, addChainFunc, addPreChainFunc)
if err := s.Scan(ctx, addChainFunc, addPreChainFunc); err != nil {
klog.Errorf("Scan(): %v", err)
}

close(certs)
close(precerts)
Expand Down
8 changes: 6 additions & 2 deletions scanner/scanlog/scanlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,12 @@ func main() {

ctx := context.Background()
if *printChains {
s.Scan(ctx, logFullChain, logFullChain)
if err := s.Scan(ctx, logFullChain, logFullChain); err != nil {
log.Fatal(err)
}
} else {
s.Scan(ctx, logCertInfo, logPrecertInfo)
if err := s.Scan(ctx, logCertInfo, logPrecertInfo); err != nil {
log.Fatal(err)
}
}
}
8 changes: 6 additions & 2 deletions submission/distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,9 @@ func (d *Distributor) addSomeChain(ctx context.Context, rawChain [][]byte, loadP
if err != nil {
return
}
GetSCTs(ctx, d, chain, asPreChain, pendingGroup)
if _, err := GetSCTs(ctx, d, chain, asPreChain, pendingGroup); err != nil {
klog.Errorf("GetSCTs(): %v", err)
}
}()
}
return GetSCTs(ctx, d, chain, asPreChain, groups)
Expand Down Expand Up @@ -369,7 +371,9 @@ func NewDistributor(ll *loglist3.LogList, plc ctpolicy.CTPolicy, lcBuilder LogCl
if err := d.buildLogClients(lcBuilder, d.usableLl); err != nil {
return nil, err
}
d.buildLogClients(lcBuilder, d.pendingQualifiedLl)
if err := d.buildLogClients(lcBuilder, d.pendingQualifiedLl); err != nil {
return nil, err
}

if mf == nil {
mf = monitoring.InertMetricFactory{}
Expand Down
5 changes: 4 additions & 1 deletion submission/proxy_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,8 @@ func (s *ProxyServer) HandleInfo(w http.ResponseWriter, r *http.Request) {
return
}

t.Execute(w, data)
if err := t.Execute(w, data); err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
}
4 changes: 3 additions & 1 deletion submission/races_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import (

func testdataSCT() *ct.SignedCertificateTimestamp {
var sct ct.SignedCertificateTimestamp
tls.Unmarshal(testdata.TestPreCertProof, &sct)
if _, err := tls.Unmarshal(testdata.TestPreCertProof, &sct); err != nil {
panic(err)
}
return &sct
}

Expand Down
24 changes: 18 additions & 6 deletions trillian/ctfe/ct_server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,17 +148,25 @@ func main() {

etcdHTTPKey := fmt.Sprintf("%s/%s", *etcdHTTPService, *httpEndpoint)
klog.Infof("Announcing our presence at %v with %+v", etcdHTTPKey, *httpEndpoint)
httpManager.AddEndpoint(ctx, etcdHTTPKey, endpoints.Endpoint{Addr: *httpEndpoint})
if err := httpManager.AddEndpoint(ctx, etcdHTTPKey, endpoints.Endpoint{Addr: *httpEndpoint}); err != nil {
klog.Exitf("AddEndpoint(): %v", err)
}

etcdMetricsKey := fmt.Sprintf("%s/%s", *etcdMetricsService, metricsAt)
klog.Infof("Announcing our presence in %v with %+v", *etcdMetricsService, metricsAt)
metricsManager.AddEndpoint(ctx, etcdMetricsKey, endpoints.Endpoint{Addr: metricsAt})
if err := metricsManager.AddEndpoint(ctx, etcdMetricsKey, endpoints.Endpoint{Addr: metricsAt}); err != nil {
klog.Exitf("AddEndpoint(): %v", err)
}

defer func() {
klog.Infof("Removing our presence in %v", etcdHTTPKey)
httpManager.DeleteEndpoint(ctx, etcdHTTPKey)
if err := httpManager.DeleteEndpoint(ctx, etcdHTTPKey); err != nil {
klog.Errorf("DeleteEndpoint(): %v", err)
}
klog.Infof("Removing our presence in %v", etcdMetricsKey)
metricsManager.DeleteEndpoint(ctx, etcdMetricsKey)
if err := metricsManager.DeleteEndpoint(ctx, etcdMetricsKey); err != nil {
klog.Errorf("DeleteEndpoint(): %v", err)
}
}()
} else if strings.Contains(*rpcBackend, ",") {
// This should probably not be used in production. Either use etcd or a gRPC
Expand Down Expand Up @@ -249,7 +257,9 @@ func main() {
// Export a healthz target.
corsMux.HandleFunc("/healthz", func(resp http.ResponseWriter, req *http.Request) {
// TODO(al): Wire this up to tell the truth.
resp.Write([]byte("ok"))
if _, err := resp.Write([]byte("ok")); err != nil {
klog.Errorf("resp.Write(): %v", err)
}
})

if metricsAt != *httpEndpoint {
Expand Down Expand Up @@ -285,7 +295,9 @@ func main() {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*60)
defer cancel()
klog.Info("Shutting down HTTP server...")
srv.Shutdown(ctx)
if err := srv.Shutdown(ctx); err != nil {
klog.Errorf("srv.Shutdown(): %v", err)
}
klog.Info("HTTP server shutdown")
})

Expand Down
20 changes: 14 additions & 6 deletions trillian/integration/copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,22 @@ func NewCopyChainGeneratorFromOpts(ctx context.Context, client *client.LogClient
StartIndex: startIndex,
}
certFetcher := scanner.NewFetcher(client, &fetchOpts)
go certFetcher.Run(ctx, func(batch scanner.EntryBatch) {
generator.processBatch(batch, generator.certs, ct.X509LogEntryType)
})
go func() {
mhutchinson marked this conversation as resolved.
Show resolved Hide resolved
if err := certFetcher.Run(ctx, func(batch scanner.EntryBatch) {
generator.processBatch(batch, generator.certs, ct.X509LogEntryType)
}); err != nil {
klog.Errorf("processBatch(): %v", err)
}
}()

precertFetcher := scanner.NewFetcher(client, &fetchOpts)
go precertFetcher.Run(ctx, func(batch scanner.EntryBatch) {
generator.processBatch(batch, generator.precerts, ct.PrecertLogEntryType)
})
go func() {
if err := precertFetcher.Run(ctx, func(batch scanner.EntryBatch) {
generator.processBatch(batch, generator.precerts, ct.PrecertLogEntryType)
}); err != nil {
klog.Errorf("processBatch(): %v", err)
}
}()

return generator, nil
}
Expand Down
4 changes: 3 additions & 1 deletion trillian/integration/ct_hammer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,9 @@ func main() {
mcData, _ := base64.StdEncoding.DecodeString(mc)
b := bytes.NewReader(mcData)
r, _ := gzip.NewReader(b)
io.Copy(os.Stdout, r)
if _, err := io.Copy(os.Stdout, r); err != nil {
klog.Exitf("Failed to print banner!")
}
r.Close()
fmt.Print("\n\nHammer Time\n\n")
}
Expand Down
2 changes: 0 additions & 2 deletions trillian/integration/ct_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"crypto/sha256"
"encoding/pem"
"fmt"
"io"
"math/rand"
"net"
"net/http"
Expand Down Expand Up @@ -913,7 +912,6 @@ func (ls *logStats) fromServer(ctx context.Context, servers string) (*logStats,
return nil, fmt.Errorf("getting stats failed: %v", err)
}
defer httpRsp.Body.Close()
defer io.ReadAll(httpRsp.Body)
if httpRsp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("got HTTP Status %q", httpRsp.Status)
}
Expand Down
8 changes: 6 additions & 2 deletions trillian/integration/hammer.go
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,9 @@ func (s *hammerState) String() string {
}

func (s *hammerState) performOp(ctx context.Context, ep ctfe.EntrypointName) (int, error) {
s.cfg.Limiter.Wait(ctx)
if err := s.cfg.Limiter.Wait(ctx); err != nil {
return http.StatusRequestTimeout, fmt.Errorf("Limiter.Wait(): %v", err)
}

s.mu.Lock()
defer s.mu.Unlock()
Expand Down Expand Up @@ -1014,7 +1016,9 @@ func (s *hammerState) performOp(ctx context.Context, ep ctfe.EntrypointName) (in
}

func (s *hammerState) performInvalidOp(ctx context.Context, ep ctfe.EntrypointName) error {
s.cfg.Limiter.Wait(ctx)
if err := s.cfg.Limiter.Wait(ctx); err != nil {
return fmt.Errorf("Limiter.Wait(): %v", err)
}
switch ep {
case ctfe.AddChainName:
return s.addChainInvalid(ctx)
Expand Down
Loading