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

br: Enable lint gosec in br #30895

Merged
merged 6 commits into from
Dec 27, 2021
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
9 changes: 8 additions & 1 deletion .golangci_br.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ linters:
- exhaustivestruct
- exhaustive
- godot
- gosec
- errorlint
- wrapcheck
- gomoddirectives
Expand Down Expand Up @@ -81,3 +80,11 @@ linters-settings:

issues:
exclude-rules:
- path: br/tests/
linters:
- gosec
- errcheck
- path: _test\.go
linters:
- gosec

4 changes: 2 additions & 2 deletions br/pkg/lightning/backend/kv/sql2kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ func NewTableKVEncoder(tbl table.Table, options *SessionOptions) (Encoder, error
for _, col := range cols {
if mysql.HasPriKeyFlag(col.Flag) {
incrementalBits := autoRandomIncrementBits(col, int(meta.AutoRandomBits))
autoRandomBits := rand.New(rand.NewSource(options.AutoRandomSeed)).Int63n(1<<meta.AutoRandomBits) << incrementalBits
autoRandomBits := rand.New(rand.NewSource(options.AutoRandomSeed)).Int63n(1<<meta.AutoRandomBits) << incrementalBits // nolint:gosec
autoIDFn = func(id int64) int64 {
return autoRandomBits | id
}
break
}
}
} else if meta.ShardRowIDBits > 0 {
rd := rand.New(rand.NewSource(options.AutoRandomSeed))
rd := rand.New(rand.NewSource(options.AutoRandomSeed)) // nolint:gosec
mask := int64(1)<<meta.ShardRowIDBits - 1
shift := autoid.RowIDBitLength - meta.ShardRowIDBits - 1
autoIDFn = func(id int64) int64 {
Expand Down
6 changes: 4 additions & 2 deletions br/pkg/lightning/checkpoints/checkpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ func (cpdb *FileCheckpointsDB) save() error {
// because `os.WriteFile` is not atomic, directly write into it may reset the file
// to an empty file if write is not finished.
tmpPath := cpdb.path + ".tmp"
if err := os.WriteFile(tmpPath, serialized, 0o644); err != nil {
if err := os.WriteFile(tmpPath, serialized, 0o644); err != nil { // nolint:gosec
return errors.Trace(err)
}
if err := os.Rename(tmpPath, cpdb.path); err != nil {
Expand Down Expand Up @@ -1301,6 +1301,8 @@ func (cpdb *MySQLCheckpointsDB) GetLocalStoringTables(ctx context.Context) (map[
// 1. table status is earlier than CheckpointStatusIndexImported, and
// 2. engine status is earlier than CheckpointStatusImported, and
// 3. chunk has been read

// nolint:gosec
query := fmt.Sprintf(`
SELECT DISTINCT t.table_name, c.engine_id
FROM %s.%s t, %s.%s c, %s.%s e
Expand Down Expand Up @@ -1386,7 +1388,7 @@ func (cpdb *MySQLCheckpointsDB) DestroyErrorCheckpoint(ctx context.Context, tabl
colName = columnTableName
aliasedColName = "t.table_name"
}

// nolint:gosec
selectQuery := fmt.Sprintf(`
SELECT
t.table_name,
Expand Down
2 changes: 1 addition & 1 deletion br/pkg/lightning/common/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func ToTLSConfig(caPath, certPath, keyPath string) (*tls.Config, error) {
return nil, errors.New("failed to append ca certs")
}

return &tls.Config{
return &tls.Config{ // nolint:gosec
Certificates: certificates,
RootCAs: certPool,
NextProtos: []string{"h2", "http/1.1"}, // specify `h2` to let Go use HTTP/2.
Expand Down
2 changes: 1 addition & 1 deletion br/pkg/lightning/lightning.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ func CleanupMetas(ctx context.Context, cfg *config.Config, tableName string) err
func UnsafeCloseEngine(ctx context.Context, importer backend.Backend, engine string) (*backend.ClosedEngine, error) {
if index := strings.LastIndexByte(engine, ':'); index >= 0 {
tableName := engine[:index]
engineID, err := strconv.Atoi(engine[index+1:])
engineID, err := strconv.Atoi(engine[index+1:]) // nolint:gosec
if err != nil {
return nil, errors.Trace(err)
}
Expand Down
11 changes: 6 additions & 5 deletions br/pkg/lightning/restore/meta_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (m *dbTableMetaMgr) AllocTableRowIDs(ctx context.Context, rawRowIDMax int64
}
needAutoID := common.TableHasAutoRowID(m.tr.tableInfo.Core) || m.tr.tableInfo.Core.GetAutoIncrementColInfo() != nil || m.tr.tableInfo.Core.ContainsAutoRandomBits()
err = exec.Transact(ctx, "init table allocator base", func(ctx context.Context, tx *sql.Tx) error {
query := fmt.Sprintf("SELECT task_id, row_id_base, row_id_max, total_kvs_base, total_bytes_base, checksum_base, status from %s WHERE table_id = ? FOR UPDATE", m.tableName)
query := fmt.Sprintf("SELECT task_id, row_id_base, row_id_max, total_kvs_base, total_bytes_base, checksum_base, status from %s WHERE table_id = ? FOR UPDATE", m.tableName) // nolint:gosec
rows, err := tx.QueryContext(ctx, query, m.tr.tableInfo.ID)
if err != nil {
return errors.Trace(err)
Expand Down Expand Up @@ -381,6 +381,7 @@ func (m *dbTableMetaMgr) CheckAndUpdateLocalChecksum(ctx context.Context, checks
needChecksum = true
needRemoteDupe = true
err = exec.Transact(ctx, "checksum pre-check", func(ctx context.Context, tx *sql.Tx) error {
// nolint:gosec
query := fmt.Sprintf("SELECT task_id, total_kvs_base, total_bytes_base, checksum_base, total_kvs, total_bytes, checksum, status, has_duplicates from %s WHERE table_id = ? FOR UPDATE", m.tableName)
rows, err := tx.QueryContext(ctx, query, m.tr.tableInfo.ID)
if err != nil {
Expand Down Expand Up @@ -593,7 +594,7 @@ func (m *dbTaskMetaMgr) CheckTaskExist(ctx context.Context) (bool, error) {
// avoid override existing metadata if the meta is already inserted.
exist := false
err := exec.Transact(ctx, "check whether this task has started before", func(ctx context.Context, tx *sql.Tx) error {
query := fmt.Sprintf("SELECT task_id from %s WHERE task_id = %d", m.tableName, m.taskID)
query := fmt.Sprintf("SELECT task_id from %s WHERE task_id = %d", m.tableName, m.taskID) // nolint:gosec
rows, err := tx.QueryContext(ctx, query)
if err != nil {
return errors.Annotate(err, "fetch task meta failed")
Expand Down Expand Up @@ -635,7 +636,7 @@ func (m *dbTaskMetaMgr) CheckTasksExclusively(ctx context.Context, action func(t
return errors.Annotate(err, "enable pessimistic transaction failed")
}
return exec.Transact(ctx, "check tasks exclusively", func(ctx context.Context, tx *sql.Tx) error {
query := fmt.Sprintf("SELECT task_id, pd_cfgs, status, state, source_bytes, cluster_avail from %s FOR UPDATE", m.tableName)
query := fmt.Sprintf("SELECT task_id, pd_cfgs, status, state, source_bytes, cluster_avail from %s FOR UPDATE", m.tableName) // nolint:gosec
rows, err := tx.QueryContext(ctx, query)
if err != nil {
return errors.Annotate(err, "fetch task metas failed")
Expand Down Expand Up @@ -695,7 +696,7 @@ func (m *dbTaskMetaMgr) CheckAndPausePdSchedulers(ctx context.Context) (pdutil.U
paused := false
var pausedCfg storedCfgs
err = exec.Transact(ctx, "check and pause schedulers", func(ctx context.Context, tx *sql.Tx) error {
query := fmt.Sprintf("SELECT task_id, pd_cfgs, status, state from %s FOR UPDATE", m.tableName)
query := fmt.Sprintf("SELECT task_id, pd_cfgs, status, state from %s FOR UPDATE", m.tableName) // nolint:gosec
rows, err := tx.QueryContext(ctx, query)
if err != nil {
return errors.Annotate(err, "fetch task meta failed")
Expand Down Expand Up @@ -821,7 +822,7 @@ func (m *dbTaskMetaMgr) CheckAndFinishRestore(ctx context.Context, finished bool
switchBack := true
allFinished := finished
err = exec.Transact(ctx, "check and finish schedulers", func(ctx context.Context, tx *sql.Tx) error {
query := fmt.Sprintf("SELECT task_id, status, state from %s FOR UPDATE", m.tableName)
query := fmt.Sprintf("SELECT task_id, status, state from %s FOR UPDATE", m.tableName) // nolint:gosec
rows, err := tx.QueryContext(ctx, query)
if err != nil {
return errors.Annotate(err, "fetch task meta failed")
Expand Down
2 changes: 1 addition & 1 deletion br/pkg/mock/mock_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func waitUntilServerOnline(addr string, statusPort uint) string {
// connect http status
statusURL := fmt.Sprintf("http://127.0.0.1:%d/status", statusPort)
for retry = 0; retry < retryTime; retry++ {
resp, err := http.Get(statusURL) // nolint:noctx
resp, err := http.Get(statusURL) // nolint:gosec
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the noctx should still be kept?

Suggested change
resp, err := http.Get(statusURL) // nolint:gosec
resp, err := http.Get(statusURL) //nolint:noctx,gosec

if err == nil {
// Ignore errors.
_, _ = io.ReadAll(resp.Body)
Expand Down
1 change: 1 addition & 0 deletions br/pkg/storage/hdfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func dfsCommand(args ...string) (*exec.Cmd, error) {
}
cmd = append(cmd, bin, "dfs")
cmd = append(cmd, args...)
//nolint:gosec
return exec.Command(cmd[0], cmd[1:]...), nil
}

Expand Down
2 changes: 1 addition & 1 deletion br/pkg/utils/pprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// #nosec
// register HTTP handler for /debug/pprof
"net/http"
_ "net/http/pprof"
_ "net/http/pprof" // nolint:gosec

"github.com/pingcap/errors"
"github.com/pingcap/failpoint"
Expand Down