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 lint checks in trillian repo #2979

Merged
merged 1 commit into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 0 additions & 16 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,6 @@ linters-settings:
- golang.org/x/net/context
- github.com/gogo/protobuf/proto

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
7 changes: 6 additions & 1 deletion storage/admin_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"github.com/google/trillian"
"github.com/google/trillian/monitoring"
"k8s.io/klog/v2"
)

const traceSpanRoot = "/trillian/storage"
Expand Down Expand Up @@ -131,7 +132,11 @@ func RunInAdminSnapshot(ctx context.Context, admin AdminStorage, fn func(tx Read
if err != nil {
return err
}
defer tx.Close()
defer func() {
if err := tx.Close(); err != nil {
klog.Errorf("tx.Close(): %v", err)
}
}()
if err := fn(tx); err != nil {
return err
}
Expand Down
6 changes: 5 additions & 1 deletion storage/cache/subtree_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/transparency-dev/merkle"
"github.com/transparency-dev/merkle/compact"
"google.golang.org/protobuf/proto"
"k8s.io/klog/v2"
)

// TODO(al): move this up the stack
Expand Down Expand Up @@ -116,7 +117,10 @@ func (s *SubtreeCache) preload(ids []compact.NodeID, getSubtrees GetSubtreesFunc
// return it when done
defer func() { workTokens <- true }()

PopulateLogTile(t, s.hasher)
if err := PopulateLogTile(t, s.hasher); err != nil {
// TODO(mhutchinson): This error should be propagated.
klog.Errorf("PopulateLogTile(): %v", err)
}
ch <- t // Note: This never blocks because len(ch) == len(subtrees).
}()
}
Expand Down
6 changes: 5 additions & 1 deletion storage/cloudspanner/getdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ func inMemClient(ctx context.Context, t testing.TB, dbName string, statements []
}
client, err := spanner.NewClient(ctx, dbName, option.WithGRPCConn(conn))
if err != nil {
conn.Close()
defer func() {
if err := conn.Close(); err != nil {
t.Errorf("conn.Close(): %v", err)
}
}()
t.Fatalf("Connecting to in-memory fake: %v", err)
}
t.Cleanup(client.Close)
Expand Down
6 changes: 5 additions & 1 deletion storage/cloudspanner/log_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,11 @@ func (ls *logStorage) begin(ctx context.Context, tree *trillian.Tree, readonly b
if err := ltx.getLatestRoot(ctx); err == storage.ErrTreeNeedsInit {
return ltx, err
} else if err != nil {
tx.Close()
defer func() {
if err := tx.Close(); err != nil {
klog.Errorf("conn.Close(): %v", err)
}
}()
return nil, err
}
return ltx, nil
Expand Down
4 changes: 3 additions & 1 deletion storage/crdb/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ func TestMain(m *testing.M) {
dburl.Path = "/"

// Set the environment variable for the test server
os.Setenv(testdb.CockroachDBURIEnv, dburl.String())
if err := os.Setenv(testdb.CockroachDBURIEnv, dburl.String()); err != nil {
klog.Exitf("Failed to SetEnv CockroachDBURIEnv: %v", err)
}

if !testdb.CockroachDBAvailable() {
klog.Errorf("CockroachDB not available, skipping all CockroachDB storage tests")
Expand Down
62 changes: 51 additions & 11 deletions storage/crdb/log_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ func (m *crdbLogStorage) GetActiveLogIDs(ctx context.Context) ([]int64, error) {
if err != nil {
return nil, err
}
defer rows.Close()
defer func() {
if err := rows.Close(); err != nil {
klog.Errorf("rows.Close(): %v", err)
}
}()
ids := []int64{}
for rows.Next() {
var treeID int64
Expand Down Expand Up @@ -204,12 +208,16 @@ func (m *crdbLogStorage) beginInternal(ctx context.Context, tree *trillian.Tree)
ltx.treeTX.writeRevision = 0
return ltx, err
} else if err != nil {
ttx.Close()
if err := ttx.Close(); err != nil {
klog.Errorf("ttx.Close(): %v", err)
}
return nil, err
}

if err := ltx.root.UnmarshalBinary(ltx.slr.LogRoot); err != nil {
ttx.Close()
if err := ttx.Close(); err != nil {
klog.Errorf("ttx.Close(): %v", err)
}
return nil, err
}

Expand All @@ -226,7 +234,11 @@ func (m *crdbLogStorage) ReadWriteTransaction(ctx context.Context, tree *trillia
if err != nil && err != storage.ErrTreeNeedsInit {
return err
}
defer tx.Close()
defer func() {
if err := tx.Close(); err != nil {
klog.Errorf("tx.Close(): %v", err)
}
}()
if err := f(ctx, tx); err != nil {
return err
}
Expand All @@ -239,7 +251,11 @@ func (m *crdbLogStorage) AddSequencedLeaves(ctx context.Context, tree *trillian.
// Ensure we don't leak the transaction. For example if we get an
// ErrTreeNeedsInit from beginInternal() or if AddSequencedLeaves fails
// below.
defer tx.Close()
defer func() {
if err := tx.Close(); err != nil {
klog.Errorf("tx.Close(): %v", err)
}
}()
}
if err != nil {
return nil, err
Expand Down Expand Up @@ -268,7 +284,11 @@ func (m *crdbLogStorage) QueueLeaves(ctx context.Context, tree *trillian.Tree, l
// Ensure we don't leak the transaction. For example if we get an
// ErrTreeNeedsInit from beginInternal() or if QueueLeaves fails
// below.
defer tx.Close()
defer func() {
if err := tx.Close(); err != nil {
klog.Errorf("tx.Close(): %v", err)
}
}()
}
if err != nil {
return nil, err
Expand Down Expand Up @@ -328,15 +348,23 @@ func (t *logTreeTX) DequeueLeaves(ctx context.Context, limit int, cutoffTime tim
klog.Warningf("Failed to prepare dequeue select: %s", err)
return nil, err
}
defer stx.Close()
defer func() {
if err := stx.Close(); err != nil {
klog.Errorf("stx.Close(): %v", err)
}
}()

leaves := make([]*trillian.LogLeaf, 0, limit)
rows, err := stx.QueryContext(ctx, t.treeID, cutoffTime.UnixNano(), limit)
if err != nil {
klog.Warningf("Failed to select rows for work: %s", err)
return nil, err
}
defer rows.Close()
defer func() {
if err := rows.Close(); err != nil {
klog.Errorf("rows.Close(): %v", err)
}
}()

for rows.Next() {
leaf, dqInfo, err := t.dequeueLeaf(rows)
Expand Down Expand Up @@ -639,7 +667,11 @@ func (t *logTreeTX) getLeavesByRangeInternal(ctx context.Context, start, count i
klog.Warningf("Failed to get leaves by range: %s", err)
return nil, err
}
defer rows.Close()
defer func() {
if err := rows.Close(); err != nil {
klog.Errorf("rows.Close(): %v", err)
}
}()

ret := make([]*trillian.LogLeaf, 0, count)
for wantIndex := start; rows.Next(); wantIndex++ {
Expand Down Expand Up @@ -770,7 +802,11 @@ func (t *logTreeTX) StoreSignedLogRoot(ctx context.Context, root *trillian.Signe

func (t *logTreeTX) getLeavesByHashInternal(ctx context.Context, leafHashes [][]byte, tmpl *sql.Stmt, desc string) ([]*trillian.LogLeaf, error) {
stx := t.tx.StmtContext(ctx, tmpl)
defer stx.Close()
defer func() {
if err := stx.Close(); err != nil {
klog.Errorf("stx.Close(): %v", err)
}
}()

var args []interface{}
for _, hash := range leafHashes {
Expand All @@ -782,7 +818,11 @@ func (t *logTreeTX) getLeavesByHashInternal(ctx context.Context, leafHashes [][]
klog.Warningf("Query() %s hash = %v", desc, err)
return nil, err
}
defer rows.Close()
defer func() {
if err := rows.Close(); err != nil {
klog.Errorf("rows.Close(): %v", err)
}
}()

// The tree could include duplicates so we don't know how many results will be returned
var ret []*trillian.LogLeaf
Expand Down
6 changes: 5 additions & 1 deletion storage/crdb/log_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,11 @@ func TestGetActiveLogIDs(t *testing.T) {
if err != nil {
t.Fatalf("PrepareContext() returned err = %v", err)
}
defer updateDeletedStmt.Close()
defer func() {
if err := updateDeletedStmt.Close(); err != nil {
t.Errorf("updateDeletedStmt.Close(): %v", err)
}
}()
for _, treeID := range []int64{deletedLog.TreeId} {
if _, err := updateDeletedStmt.ExecContext(ctx, true, treeID); err != nil {
t.Fatalf("ExecContext(%v) returned err = %v", treeID, err)
Expand Down
6 changes: 5 additions & 1 deletion storage/crdb/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,11 @@ func (t *logTreeTX) removeSequencedLeaves(ctx context.Context, leaves []dequeued
klog.Warningf("Failed to prep delete statement for sequenced work: %v", err)
return err
}
defer stx.Close()
defer func() {
if err := stx.Close(); err != nil {
klog.Errorf("stx.Close(): %v", err)
}
}()
for _, dql := range leaves {
result, err := stx.ExecContext(ctx, t.treeID, dql.queueTimestampNanos, dql.leafIdentityHash)
err = checkResultOkAndRowCountIs(result, err, int64(1))
Expand Down
43 changes: 36 additions & 7 deletions storage/crdb/sqladminstorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/timestamppb"
"k8s.io/klog/v2"
)

const (
Expand Down Expand Up @@ -88,7 +89,11 @@ func (s *sqlAdminStorage) ReadWriteTransaction(ctx context.Context, f storage.Ad
if err != nil {
return err
}
defer tx.Close()
defer func() {
if err := tx.Close(); err != nil {
klog.Errorf("tx.Close(): %v", err)
}
}()
if err := f(ctx, tx); err != nil {
return err
}
Expand Down Expand Up @@ -132,7 +137,11 @@ func (t *adminTX) GetTree(ctx context.Context, treeID int64) (*trillian.Tree, er
if err != nil {
return nil, err
}
defer stmt.Close()
defer func() {
if err := stmt.Close(); err != nil {
klog.Errorf("stmt.Close(): %v", err)
}
}()

// GetTree is an entry point for most RPCs, let's provide somewhat nicer error messages.
tree, err := storage.ReadTree(stmt.QueryRowContext(ctx, treeID))
Expand All @@ -158,12 +167,20 @@ func (t *adminTX) ListTrees(ctx context.Context, includeDeleted bool) ([]*trilli
if err != nil {
return nil, err
}
defer stmt.Close()
defer func() {
if err := stmt.Close(); err != nil {
klog.Errorf("stmt.Close(): %v", err)
}
}()
rows, err := stmt.QueryContext(ctx)
if err != nil {
return nil, err
}
defer rows.Close()
defer func() {
if err := rows.Close(); err != nil {
klog.Errorf("rows.Close(): %v", err)
}
}()
trees := []*trillian.Tree{}
for rows.Next() {
tree, err := storage.ReadTree(rows)
Expand Down Expand Up @@ -227,7 +244,11 @@ func (t *adminTX) CreateTree(ctx context.Context, tree *trillian.Tree) (*trillia
if err != nil {
return nil, err
}
defer insertTreeStmt.Close()
defer func() {
if err := insertTreeStmt.Close(); err != nil {
klog.Errorf("insertTreeStmt.Close(): %v", err)
}
}()

_, err = insertTreeStmt.ExecContext(
ctx,
Expand Down Expand Up @@ -269,7 +290,11 @@ func (t *adminTX) CreateTree(ctx context.Context, tree *trillian.Tree) (*trillia
if err != nil {
return nil, err
}
defer insertControlStmt.Close()
defer func() {
if err := insertControlStmt.Close(); err != nil {
klog.Errorf("insertControlStmt.Close(): %v", err)
}
}()
_, err = insertControlStmt.ExecContext(
ctx,
newTree.TreeId,
Expand Down Expand Up @@ -318,7 +343,11 @@ func (t *adminTX) UpdateTree(ctx context.Context, treeID int64, updateFunc func(
if err != nil {
return nil, err
}
defer stmt.Close()
defer func() {
if err := stmt.Close(); err != nil {
klog.Errorf("stmt.Close(): %v", err)
}
}()

if _, err = stmt.ExecContext(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion storage/crdb/sqladminstorage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func setNulls(ctx context.Context, db *sql.DB, treeID int64) error {
if err != nil {
return err
}
defer stmt.Close()
defer func() { _ = stmt.Close() }()
_, err = stmt.ExecContext(ctx, treeID)
return err
}
Loading