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

Bump golangci to get the latest lints & fix lints #984

Merged
merged 2 commits into from
Aug 26, 2021
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
12 changes: 10 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ linters-settings:
golint:
min-confidence: 0

issues:
include:
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to always include all?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it still makes sense for some low risk issues to be excluded. Is it possible to get the security team to audit low risk issues instead of add gosec tag?

- EXC0006
- EXC0007
- EXC0008
- EXC0009
- EXC0010

linters:
disable-all: true
enable:
Expand All @@ -23,11 +31,11 @@ linters:
- ineffassign
- deadcode
- typecheck
- golint
- revive
- gosec
- unconvert
- goimports
- depguard
- prealloc
- scopelint
- exportloopref
- whitespace
2 changes: 1 addition & 1 deletion cmd/tidb-dashboard/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"fmt"
"net"
"net/http"
_ "net/http/pprof" //nolint:gosec
_ "net/http/pprof" // #nosec
"os"
"os/signal"
"strings"
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/debugapi/endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (m *APIModel) NewRequest(host string, port int, data map[string]string) (*R
return req, nil
}

var paramRegexp *regexp.Regexp = regexp.MustCompile(`\{(\w+)\}`)
var paramRegexp = regexp.MustCompile(`\{(\w+)\}`)

func populatePath(path string, values Values) (string, error) {
var returnErr error
Expand Down
14 changes: 5 additions & 9 deletions pkg/apiserver/diagnose/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ func GetTiKVErrorTable(startTime, endTime string, db *gorm.DB) (TableDef, error)
}

func GetTiDBCurrentConfig(startTime, endTime string, db *gorm.DB) (TableDef, error) {
sql := fmt.Sprintf("select `key`,`value` from information_schema.CLUSTER_CONFIG where type='tidb' group by `key`,`value` order by `key`;")
sql := "select `key`,`value` from information_schema.CLUSTER_CONFIG where type='tidb' group by `key`,`value` order by `key`;"
table := TableDef{
Category: []string{CategoryConfig},
Title: "tidb_current_config",
Expand All @@ -1507,7 +1507,7 @@ func GetTiDBCurrentConfig(startTime, endTime string, db *gorm.DB) (TableDef, err
}

func GetPDCurrentConfig(startTime, endTime string, db *gorm.DB) (TableDef, error) {
sql := fmt.Sprintf("select `key`,`value` from information_schema.CLUSTER_CONFIG where type='pd' group by `key`,`value` order by `key`;")
sql := "select `key`,`value` from information_schema.CLUSTER_CONFIG where type='pd' group by `key`,`value` order by `key`;"
table := TableDef{
Category: []string{CategoryConfig},
Title: "pd_current_config",
Expand All @@ -1523,7 +1523,7 @@ func GetPDCurrentConfig(startTime, endTime string, db *gorm.DB) (TableDef, error
}

func GetTiKVCurrentConfig(startTime, endTime string, db *gorm.DB) (TableDef, error) {
sql := fmt.Sprintf("select `key`,`value` from information_schema.CLUSTER_CONFIG where type='tikv' group by `key`,`value` order by `key`;")
sql := "select `key`,`value` from information_schema.CLUSTER_CONFIG where type='tikv' group by `key`,`value` order by `key`;"
table := TableDef{
Category: []string{CategoryConfig},
Title: "tikv_current_config",
Expand Down Expand Up @@ -2050,7 +2050,7 @@ func GetPDEtcdStatusTable(startTime, endTime string, db *gorm.DB) (TableDef, err
}

func GetClusterInfoTable(startTime, endTime string, db *gorm.DB) (TableDef, error) {
sql := fmt.Sprintf("select * from information_schema.cluster_info order by type,start_time desc")
sql := "select * from information_schema.cluster_info order by type,start_time desc"
table := TableDef{
Category: []string{CategoryHeader},
Title: "cluster_info",
Expand Down Expand Up @@ -2150,11 +2150,7 @@ func GetClusterHardwareInfoTable(startTime, endTime string, db *gorm.DB) (TableD
if !ok {
m[s] = &hardWare{s, map[string]int{row[1]: 1}, make(map[string]int), 0, make(map[string]float64), ""}
}
if _, ok := m[s].Type[row[1]]; ok {
Copy link
Member

Choose a reason for hiding this comment

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

Smart linter...

Copy link
Contributor

Choose a reason for hiding this comment

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

すげー😲

m[s].Type[row[1]]++
} else {
m[s].Type[row[1]] = 1
}
m[s].Type[row[1]]++
if _, ok := m[s].cpu[row[2]]; !ok {
m[s].cpu[row[2]] = cpuCnt
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/info/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func RegisterRouter(r *gin.RouterGroup, auth *user.AuthService, s *Service) {
endpoint.GET("/tables", s.tablesHandler)
}

type InfoResponse struct { //nolint:golint
type InfoResponse struct { //nolint
Version *version.Info `json:"version"`
EnableTelemetry bool `json:"enable_telemetry"`
EnableExperimental bool `json:"enable_experimental"`
Expand Down
7 changes: 4 additions & 3 deletions pkg/apiserver/logsearch/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ func (tg *TaskGroup) SyncRun() {

// Create log directory
dir := path.Join(tg.service.logStoreDirectory, strconv.Itoa(int(tg.model.ID)))
if err := os.MkdirAll(dir, 0777); err == nil {
err := os.MkdirAll(dir, 0777) // #nosec
breezewish marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
tg.model.LogStoreDir = &dir
tg.service.db.Save(tg.model)
}
Expand Down Expand Up @@ -223,7 +224,7 @@ func (t *Task) searchLog(client diagnosticspb.DiagnosticsClient, targetType diag
t.setError(err)
return
}
defer f.Close()
defer f.Close() // #nosec

// TODO: Could we use a memory buffer for this and flush the writer regularly to avoid OOM.
// This might perform an faster processing. This could also avoid creating an empty .zip
Expand Down Expand Up @@ -260,7 +261,7 @@ func (t *Task) searchLog(client diagnosticspb.DiagnosticsClient, targetType diag
}
for _, msg := range res.Messages {
line := logMessageToString(msg)
_, err := bufWriter.Write(*(*[]byte)(unsafe.Pointer(&line)))
_, err := bufWriter.Write(*(*[]byte)(unsafe.Pointer(&line))) // #nosec
Copy link
Member

Choose a reason for hiding this comment

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

ditto, this error is checked and why do we need nosec?

Copy link
Member Author

Choose a reason for hiding this comment

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

Every unsafe api invoke should use nosec tag... I'm wondering whether we need to turn on all gosec rules. Obviously, we know what we are doing when we use a specific api.

if err != nil {
t.setError(err)
return
Expand Down
7 changes: 4 additions & 3 deletions pkg/apiserver/profiling/pprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"io"
"io/ioutil"
"os"
"path/filepath"
"strconv"
"sync"
"time"
Expand Down Expand Up @@ -50,7 +51,7 @@ func fetchPprofSVG(op *pprofOptions) (string, error) {
return "", fmt.Errorf("failed to get DOT output from file: %v", err)
}

b, err := ioutil.ReadFile(f)
b, err := ioutil.ReadFile(filepath.Clean(f))
if err != nil {
return "", fmt.Errorf("failed to get DOT output from file: %v", err)
}
Expand All @@ -59,7 +60,7 @@ func fetchPprofSVG(op *pprofOptions) (string, error) {
if err != nil {
return "", fmt.Errorf("failed to create temp file: %v", err)
}
defer tmpfile.Close()
defer tmpfile.Close() // #nosec
tmpPath := fmt.Sprintf("%s.%s", tmpfile.Name(), "svg")

g := graphviz.New()
Expand Down Expand Up @@ -87,7 +88,7 @@ func fetchPprof(op *pprofOptions, format string) (string, error) {
if err != nil {
return "", fmt.Errorf("failed to create temp file: %v", err)
}
defer tmpfile.Close()
defer tmpfile.Close() // #nosec
tmpPath := fmt.Sprintf("%s.%s", tmpfile.Name(), format)
format = "-" + format
args := []string{
Expand Down
4 changes: 2 additions & 2 deletions pkg/apiserver/statement/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func buildGlobalConfigProjectionSelectSQL(config interface{}) string {
column := utils.GetGormColumnName(gormTag)
return fmt.Sprintf("@@GLOBAL.%s AS %s", column, column), true
}, ", ")
return "SELECT " + str //nolint:gosec
return "SELECT " + str // #nosec
}

// sql will be built like this,
Expand All @@ -55,7 +55,7 @@ func buildGlobalConfigNamedArgsUpdateSQL(config interface{}, allowedFields ...st
column := utils.GetGormColumnName(gormTag)
return fmt.Sprintf("@@GLOBAL.%s = @%s", column, f.Name), true
}, ", ")
return "SET " + str //nolint:gosec
return "SET " + str // #nosec
}

func buildStringByStructField(i interface{}, buildFunc func(f reflect.StructField) (string, bool), sep string) string {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/user/sso/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const (
ImpersonateStatusAuthFail ImpersonateStatus = "auth_fail"
)

type SSOImpersonationModel struct { //nolint:golint
type SSOImpersonationModel struct { //nolint
SQLUser string `gorm:"primary_key;size:128" json:"sql_user"`
// The encryption key is placed somewhere else in the FS, to avoid being collected by diagnostics collecting tools.
EncryptedPass string `gorm:"type:text" json:"-"`
Expand Down
3 changes: 2 additions & 1 deletion pkg/apiserver/user/sso/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,9 @@ func (s *Service) createImpersonation(user string, password string) (*SSOImperso
}

func (s *Service) revokeAllImpersonations() error {
sqlStr := fmt.Sprintf("DELETE FROM `%s`", SSOImpersonationModel{}.TableName()) // #nosec
return s.params.LocalStore.
Exec(fmt.Sprintf("DELETE FROM `%s`", SSOImpersonationModel{}.TableName())). //nolint:gosec
Exec(sqlStr).
Error
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/apiserver/utils/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"io/ioutil"
"os"
"path/filepath"
"reflect"
"strings"
"time"
Expand Down Expand Up @@ -75,7 +76,7 @@ func ExportCSV(data [][]string, filename, tokenNamespace string) (token string,
if err != nil {
return
}
defer csvFile.Close()
defer csvFile.Close() // #nosec

// generate encryption key
secretKey := *cryptopasta.NewEncryptionKey()
Expand All @@ -84,7 +85,7 @@ func ExportCSV(data [][]string, filename, tokenNamespace string) (token string,
go func() {
csvwriter := csv.NewWriter(pw)
_ = csvwriter.WriteAll(data)
pw.Close()
_ = pw.Close()
}()
err = aesctr.Encrypt(pr, csvFile, secretKey[0:16], secretKey[16:])
if err != nil {
Expand Down Expand Up @@ -121,7 +122,7 @@ func DownloadByToken(token, tokenNamespace string, c *gin.Context) {
_ = c.Error(err)
return
}
f, err := os.Open(filePath)
f, err := os.Open(filepath.Clean(filePath))
if err != nil {
_ = c.Error(err)
return
Expand All @@ -134,6 +135,6 @@ func DownloadByToken(token, tokenNamespace string, c *gin.Context) {
log.Error("decrypt csv failed", zap.Error(err))
}
// delete it anyway
f.Close()
_ = f.Close()
_ = os.Remove(filePath)
}
4 changes: 2 additions & 2 deletions pkg/apiserver/utils/fs_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ func FSServe(c *gin.Context, token string, requiredIssuer string) {
}
return
}
defer file.Close() //nolint:errcheck
defer os.Remove(tokenBody.TempFileName) //nolint:errcheck
defer file.Close() // #nosec
defer os.Remove(tokenBody.TempFileName) // #nosec

key, err := hex.DecodeString(tokenBody.TempFileKeyInHex)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions pkg/apiserver/utils/zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"archive/zip"
"io"
"os"
"path/filepath"
)

func StreamZipPack(w io.Writer, files []string, needCompress bool) error {
Expand All @@ -34,11 +35,11 @@ func StreamZipPack(w io.Writer, files []string, needCompress bool) error {
}

func streamZipFile(zipPack *zip.Writer, file string, needCompress bool) error {
f, err := os.Open(file)
f, err := os.Open(filepath.Clean(file))
if err != nil {
return err
}
defer f.Close()
defer f.Close() // #nosec

fileInfo, err := f.Stat()
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/dbstore/dbstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ type DB struct {
}

func NewDBStore(lc fx.Lifecycle, config *config.Config) (*DB, error) {
if err := os.MkdirAll(config.DataDir, 0777); err != nil {
err := os.MkdirAll(config.DataDir, 0777) // #nosec
if err != nil {
log.Error("Failed to create Dashboard storage directory", zap.Error(err))
return nil, err
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/keyvisual/input/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"context"
"io/ioutil"
"os"
"path/filepath"
"time"

"github.com/pingcap/log"
Expand Down Expand Up @@ -60,11 +61,11 @@ func (input *fileInput) Background(ctx context.Context, stat *storage.Stat) {

func readFile(fileTime time.Time) (*RegionsInfo, error) {
fileName := fileTime.Format("./data/20060102-15-04.json")
file, err := os.Open(fileName)
file, err := os.Open(filepath.Clean(fileName))
if err != nil {
return nil, ErrInvalidData.Wrap(err, "%s regions API unmarshal failed, from file %s", distro.Data("pd"), fileName)
}
defer file.Close()
defer file.Close() // #nosec
data, err := ioutil.ReadAll(file)
if err != nil {
return nil, ErrInvalidData.Wrap(err, "%s regions API unmarshal failed, from file %s", distro.Data("pd"), fileName)
Expand Down
8 changes: 4 additions & 4 deletions pkg/keyvisual/matrix/axis.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (c *chunk) Reduce(newKeys []string) chunk {
// GetFocusRows estimates the number of rows generated by executing a Focus with a specified threshold
func (c *chunk) GetFocusRows(threshold uint64) (count int) {
start := 0
var bucketSum uint64 = 0
var bucketSum uint64
generateBucket := func(end int) {
if end > start {
count++
Expand Down Expand Up @@ -226,7 +226,7 @@ func (c *chunk) Focus(labeler decorator.Labeler, threshold uint64, ratio int, ta
newKeys = append(newKeys, c.Keys[0])

start := 0
var bucketSum uint64 = 0
var bucketSum uint64
generateBucket := func(end int) {
if end > start {
newKeys = append(newKeys, c.Keys[end])
Expand Down Expand Up @@ -263,7 +263,7 @@ func (c *chunk) MergeColdLogicalRange(labeler decorator.Labeler, threshold uint6

coldStart := 0
coldEnd := 0
var coldRangeSum uint64 = 0
var coldRangeSum uint64
mergeColdRange := func() {
if coldEnd <= coldStart {
return
Expand All @@ -277,7 +277,7 @@ func (c *chunk) MergeColdLogicalRange(labeler decorator.Labeler, threshold uint6
if end <= coldEnd {
return
}
var rangeSum uint64 = 0
var rangeSum uint64
for i := coldEnd; i < end; i++ {
rangeSum += c.Values[i]
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/keyvisual/matrix/distance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ func BenchmarkGenerateScale(b *testing.B) {
var data testDisData
fin, err := os.Open("../testdata/dis.json.gzip")
perr(err)
defer fin.Close()
defer func() {
_ = fin.Close()
}()
ifs, err := gzip.NewReader(fin)
perr(err)
err = json.NewDecoder(ifs).Decode(&data)
Expand Down
4 changes: 2 additions & 2 deletions pkg/keyvisual/matrix/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (km *KeyMap) SaveKeys(keys []string) {
}

func equal(keyA, keyB string) bool {
pA := (*reflect.StringHeader)(unsafe.Pointer(&keyA))
pB := (*reflect.StringHeader)(unsafe.Pointer(&keyB))
pA := (*reflect.StringHeader)(unsafe.Pointer(&keyA)) // #nosec
pB := (*reflect.StringHeader)(unsafe.Pointer(&keyB)) // #nosec
return pA.Data == pB.Data && pA.Len == pB.Len
}
8 changes: 4 additions & 4 deletions pkg/keyvisual/region/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ func String(b []byte) (s string) {
if len(b) == 0 {
return ""
}
pbytes := (*reflect.SliceHeader)(unsafe.Pointer(&b))
pstring := (*reflect.StringHeader)(unsafe.Pointer(&s))
pbytes := (*reflect.SliceHeader)(unsafe.Pointer(&b)) // #nosec
pstring := (*reflect.StringHeader)(unsafe.Pointer(&s)) // #nosec
pstring.Data = pbytes.Data
pstring.Len = pbytes.Len
return
Expand All @@ -35,8 +35,8 @@ func Bytes(s string) (b []byte) {
if len(s) == 0 {
return
}
pbytes := (*reflect.SliceHeader)(unsafe.Pointer(&b))
pstring := (*reflect.StringHeader)(unsafe.Pointer(&s))
pbytes := (*reflect.SliceHeader)(unsafe.Pointer(&b)) // #nosec
pstring := (*reflect.StringHeader)(unsafe.Pointer(&s)) // #nosec
pbytes.Data = pstring.Data
pbytes.Len = pstring.Len
pbytes.Cap = pstring.Len
Expand Down
Loading