Enable all lint checks in trillian repo (#2979)

Fixed all of the remaining lint errors.

.golangci.yaml

@@ -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

storage/admin_helpers.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/google/trillian"
 	"github.com/google/trillian/monitoring"
+	"k8s.io/klog/v2"
 )
 
 const traceSpanRoot = "/trillian/storage"
@@ -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
 	}

storage/cache/subtree_cache.go

@@ -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
@@ -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).
 		}()
 	}

storage/cloudspanner/getdb_test.go

@@ -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)

storage/cloudspanner/log_storage.go

@@ -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

storage/crdb/common_test.go

@@ -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")

storage/crdb/log_storage.go

@@ -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
@@ -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
 	}
 
@@ -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
 	}
@@ -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
@@ -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
@@ -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)
@@ -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++ {
@@ -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 {
@@ -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

storage/crdb/log_storage_test.go

@@ -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)

storage/crdb/queue.go

@@ -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))

storage/crdb/sqladminstorage.go

@@ -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 (
@@ -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
 	}
@@ -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))
@@ -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)
@@ -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,
@@ -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,
@@ -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,

storage/crdb/sqladminstorage_test.go

@@ -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
 }