diff --git a/.github/workflows/common.yml b/.github/workflows/common.yml index a1b35db5..1e826ce1 100644 --- a/.github/workflows/common.yml +++ b/.github/workflows/common.yml @@ -58,7 +58,9 @@ jobs: key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} restore-keys: | ${{ runner.os }}-go- - - run: make ${{ inputs.target }} + - name: make ${{ inputs.target }} + if: ${{ inputs.target != '' }} + run: make ${{ inputs.target }} - name: "set up tmate session if necessary" if: ${{ failure() && inputs.debug }} uses: mxschmitt/action-tmate@v3 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9ed681b2..c4af4896 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -28,6 +28,7 @@ concurrency: jobs: cmd: + if: ${{ github.event_name != 'push' }} uses: ./.github/workflows/common.yml with: debug: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug }} @@ -36,6 +37,7 @@ jobs: all_platform: true build: + if: ${{ github.event_name != 'push' }} needs: cmd uses: ./.github/workflows/common.yml with: @@ -43,10 +45,28 @@ jobs: ref: ${{ inputs.ref || github.ref }} target: "build" + lint: + if: ${{ github.event_name != 'push' }} + needs: build + uses: ./.github/workflows/common.yml + with: + debug: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug }} + ref: ${{ inputs.ref || github.ref }} + target: "lint" + test: + if: ${{ github.event_name != 'push' }} needs: build uses: ./.github/workflows/common.yml with: debug: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.debug }} ref: ${{ inputs.ref || github.ref }} target: "test" + + cache: + if: ${{ github.event_name == 'push' }} + uses: ./.github/workflows/common.yml + with: + ref: ${{ inputs.ref || github.ref }} + debug: false + target: "build lint test" diff --git a/.golangci.yaml b/.golangci.yaml index 1180ed4e..f3e54ad1 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,99 +1,30 @@ -# options for analysis running run: - # default concurrency is a available CPU number - concurrency: 4 - - # timeout for analysis, e.g. 30s, 5m, default is 1m - timeout: 20m - - # exit code when at least one issue was found, default is 1 + timeout: 10m issues-exit-code: 1 - # include test files or not, default is true - tests: false - - # list of build tags, all linters use it. Default is empty list. - build-tags: - - # default is true. Enables skipping of directories: - # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ - skip-dirs-use-default: true - - # which dirs to skip: they won't be analyzed; - # can use regexp here: generated.*, regexp is applied on full path; - # default value is empty list, but next dirs are always skipped independently - # from this option's value: - # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ - # skip-dirs: - # - ^test.* - - # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": - # If invoked with -mod=readonly, the go command is disallowed from the implicit - # automatic updating of go.mod described above. Instead, it fails when any changes - # to go.mod are needed. This setting is most useful to check that go.mod does - # not need updates, such as in a continuous integration and testing system. - # If invoked with -mod=vendor, the go command assumes that the vendor - # directory holds the correct copies of dependencies and ignores - # the dependency descriptions in go.mod. - modules-download-mode: readonly - - # which files to skip: they will be analyzed, but issues from them - # won't be reported. Default value is empty list, but there is - # no need to include all autogenerated files, we confidently recognize - # autogenerated files. If it's not please let us know. - skip-files: - # - ".*\\.my\\.go$" - # - lib/bad.go - -# all available settings of specific linters linters-settings: - misspell: - # Correct spellings using locale preferences for US or UK. - # Default is to use a neutral variety of English. - # Setting locale to US will correct the British spelling of 'colour' to 'color'. - # locale: US - ignore-words: - - rela # This is for elf.SHT_RELA + errcheck: + exclude-functions: + - io.WriteString + - (*go.uber.org/zap/buffer.Buffer).WriteString + - (*go.uber.org/zap/buffer.Buffer).WriteByte issues: - # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: - - linters: [staticcheck] - text: "SA1019" # this is rule for deprecated method - - linters: [staticcheck] - text: "SA9003: empty branch" - - linters: [staticcheck] - text: "SA2001: empty critical section" - - linters: [goerr113] - text: "do not define dynamic errors, use wrapped static errors instead" # This rule to avoid opinionated check fmt.Errorf("text") - - path: _test\.go # Skip 1.13 errors check for test files + - path: _test\.go + linters: + - gosec + - path: util/security/tls.go linters: - - goerr113 + - gosec + text: "G402:" + - linters: + - unused + source: "updateAuthInfoFromSessionStates" + linters: - disable-all: true enable: - - deadcode - - goerr113 + - gosec - gofmt - goimports - - gosimple - - govet - - ineffassign - misspell - - staticcheck - - stylecheck - - varcheck - -# To enable later if makes sense -# - errcheck -# - gocyclo -# - golint -# - gosec -# - gosimple -# - lll -# - maligned -# - misspell -# - prealloc -# - structcheck -# - typecheck -# - unused diff --git a/Makefile b/Makefile index d59caf24..452bb31d 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,7 @@ endif IMAGE_TAG ?= latest EXECUTABLE_TARGETS := $(patsubst cmd/%,cmd_%,$(wildcard cmd/*)) -.PHONY: cmd_% build test docker +.PHONY: cmd_% build test docker ./bin/golangci-lint ./bin/gocovmerge default: cmd @@ -44,14 +44,21 @@ cmd_%: SOURCE=$(patsubst cmd_%,./cmd/%,$@) cmd_%: go build $(BUILDFLAGS) -o $(OUTPUT) $(SOURCE) +lint: ./bin/golangci-lint + $(GOBIN)/golangci-lint run + cd lib && $(GOBIN)/golangci-lint run + test: ./bin/gocovmerge rm -f .cover.* go test -coverprofile=.cover.pkg ./... cd lib && go test -coverprofile=../.cover.lib ./... - ./bin/gocovmerge .cover.* > .cover + $(GOBIN)/gocovmerge .cover.* > .cover rm -f .cover.* go tool cover -html=.cover -o .cover.html +./bin/golangci-lint: + GOBIN=$(GOBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest + ./bin/gocovmerge: GOBIN=$(GOBIN) go install github.com/wadey/gocovmerge@master diff --git a/cmd/tiproxy/main.go b/cmd/tiproxy/main.go index a65a53e2..c407aa8a 100644 --- a/cmd/tiproxy/main.go +++ b/cmd/tiproxy/main.go @@ -16,7 +16,6 @@ package main import ( - "io/ioutil" "os" "github.com/pingcap/TiProxy/lib/config" @@ -39,7 +38,7 @@ func main() { logLevel := rootCmd.PersistentFlags().String("log_level", "", "log level") rootCmd.RunE = func(cmd *cobra.Command, _ []string) error { - proxyConfigData, err := ioutil.ReadFile(*configFile) + proxyConfigData, err := os.ReadFile(*configFile) if err != nil { return err } diff --git a/lib/cli/main.go b/lib/cli/main.go index fb0d453e..33a41c1c 100644 --- a/lib/cli/main.go +++ b/lib/cli/main.go @@ -19,9 +19,9 @@ import ( "net/http" "github.com/pingcap/TiProxy/lib/config" + "github.com/pingcap/TiProxy/lib/util/cmd" "github.com/pingcap/TiProxy/lib/util/security" "github.com/spf13/cobra" - "go.uber.org/zap" ) func GetRootCmd(tlsConfig *tls.Config) *cobra.Command { @@ -42,12 +42,12 @@ func GetRootCmd(tlsConfig *tls.Config) *cobra.Command { keyPath := rootCmd.PersistentFlags().String("key", "", "key for server-side client authentication") rootCmd.PersistentFlags().Bool("indent", true, "whether indent the returned json") rootCmd.PersistentPreRunE = func(_ *cobra.Command, _ []string) error { - zapcfg := zap.NewDevelopmentConfig() - zapcfg.Encoding = *logEncoder - if level, err := zap.ParseAtomicLevel(*logLevel); err == nil { - zapcfg.Level = level - } - logger, err := zapcfg.Build() + logger, _, _, err := cmd.BuildLogger(&config.Log{ + Encoder: *logEncoder, + LogOnline: config.LogOnline{ + Level: *logLevel, + }, + }) if err != nil { return err } diff --git a/lib/cli/namespace.go b/lib/cli/namespace.go index 48b30aa5..46a24bf5 100644 --- a/lib/cli/namespace.go +++ b/lib/cli/namespace.go @@ -18,7 +18,6 @@ import ( "bytes" "encoding/json" "fmt" - "io/ioutil" "net/http" "os" "path/filepath" @@ -34,7 +33,7 @@ const ( ) func listAllFiles(dir, ext string) ([]string, error) { - infos, err := ioutil.ReadDir(dir) + infos, err := os.ReadDir(dir) if err != nil { return nil, err } @@ -178,7 +177,7 @@ func GetNamespaceCmd(ctx *Context) *cobra.Command { } for _, nFile := range nFiles { - fileData, err := ioutil.ReadFile(nFile) + fileData, err := os.ReadFile(nFile) if err != nil { return err } diff --git a/lib/cli/util.go b/lib/cli/util.go index 62b397f2..a9b959f5 100644 --- a/lib/cli/util.go +++ b/lib/cli/util.go @@ -18,7 +18,6 @@ import ( "context" "fmt" "io" - "io/ioutil" "math/rand" "net/http" @@ -70,7 +69,7 @@ func doRequest(ctx context.Context, bctx *Context, method string, url string, rd return "", err } } - resb, _ := ioutil.ReadAll(res.Body) + resb, _ := io.ReadAll(res.Body) res.Body.Close() switch res.StatusCode { diff --git a/lib/go.mod b/lib/go.mod index a5366d08..b68a1091 100644 --- a/lib/go.mod +++ b/lib/go.mod @@ -18,4 +18,5 @@ require ( github.com/pkg/errors v0.9.1 // indirect golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect + gopkg.in/natefinch/lumberjack.v2 v2.0.0 ) diff --git a/lib/util/cmd/cmd.go b/lib/util/cmd/cmd.go index 5f4ac6f2..736b990c 100644 --- a/lib/util/cmd/cmd.go +++ b/lib/util/cmd/cmd.go @@ -19,27 +19,12 @@ import ( "fmt" "os" "os/signal" - "sync" "syscall" - "github.com/pingcap/log" "github.com/spf13/cobra" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" ) -var registerEncoders sync.Once - func RunRootCommand(rootCmd *cobra.Command) { - registerEncoders.Do(func() { - zap.RegisterEncoder("tidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) { - return log.NewTextEncoder(&log.Config{}) - }) - zap.RegisterEncoder("newtidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) { - return NewTiDBEncoder(cfg), nil - }) - }) - ctx, cancel := context.WithCancel(context.Background()) go func() { sc := make(chan os.Signal, 1) diff --git a/lib/util/cmd/encoder.go b/lib/util/cmd/encoder.go index 8ebe738f..29943b42 100644 --- a/lib/util/cmd/encoder.go +++ b/lib/util/cmd/encoder.go @@ -148,20 +148,6 @@ func (e *tidbEncoder) EncodeEntry(ent zapcore.Entry, fields []zapcore.Field) (*b } /* map encoder part */ -func (f *tidbEncoder) needDoubleQuotes(s string) bool { - for i := 0; i < len(s); { - b := s[i] - if b <= 0x20 { - return true - } - switch b { - case '\\', '"', '[', ']', '=': - return true - } - i++ - } - return false -} func (f *tidbEncoder) safeAddString(s string) { needQuotes := false outerloop: diff --git a/pkg/manager/logger/builder.go b/lib/util/cmd/logger.go similarity index 61% rename from pkg/manager/logger/builder.go rename to lib/util/cmd/logger.go index 55cfba0d..2021b257 100644 --- a/pkg/manager/logger/builder.go +++ b/lib/util/cmd/logger.go @@ -12,37 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -package logger +package cmd import ( - "sync" - "github.com/pingcap/TiProxy/lib/config" - "github.com/pingcap/TiProxy/lib/util/cmd" "github.com/pingcap/log" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) -var registerEncoders sync.Once - func buildEncoder(cfg *config.Log) (zapcore.Encoder, error) { - zapcfg := zap.NewDevelopmentConfig() - zapcfg.Encoding = cfg.Encoder - zapcfg.DisableStacktrace = true - encoder, err := log.NewTextEncoder(&log.Config{}) - if err != nil { - return nil, err + encfg := zap.NewProductionEncoderConfig() + switch cfg.Encoder { + case "tidb": + return log.NewTextEncoder(&log.Config{}) + case "newtidb": + fallthrough + default: + return NewTiDBEncoder(encfg), nil } - registerEncoders.Do(func() { - zap.RegisterEncoder("tidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) { - return encoder, nil - }) - zap.RegisterEncoder("newtidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) { - return cmd.NewTiDBEncoder(cfg), nil - }) - }) - return encoder, nil } func buildLevel(cfg *config.Log) (zap.AtomicLevel, error) { @@ -56,3 +44,19 @@ func buildSyncer(cfg *config.Log) (*AtomicWriteSyncer, error) { } return syncer, nil } + +func BuildLogger(cfg *config.Log) (*zap.Logger, *AtomicWriteSyncer, zap.AtomicLevel, error) { + level, err := buildLevel(cfg) + if err != nil { + return nil, nil, level, err + } + encoder, err := buildEncoder(cfg) + if err != nil { + return nil, nil, level, err + } + syncer, err := buildSyncer(cfg) + if err != nil { + return nil, nil, level, err + } + return zap.New(zapcore.NewCore(encoder, syncer, level), zap.ErrorOutput(syncer), zap.AddStacktrace(zapcore.FatalLevel)), syncer, level, nil +} diff --git a/pkg/manager/logger/syncer.go b/lib/util/cmd/syncer.go similarity index 99% rename from pkg/manager/logger/syncer.go rename to lib/util/cmd/syncer.go index 0ce1b7c8..bb2e6bdb 100644 --- a/pkg/manager/logger/syncer.go +++ b/lib/util/cmd/syncer.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package logger +package cmd import ( "os" diff --git a/lib/util/errors/stacktrace.go b/lib/util/errors/stacktrace.go index f17a89e0..6cda9742 100644 --- a/lib/util/errors/stacktrace.go +++ b/lib/util/errors/stacktrace.go @@ -53,7 +53,7 @@ func formatFrame(s fmt.State, fr runtime.Frame, verb rune) { } } -// stacktrace only stores the pointers, infomation will be queried as need. +// stacktrace only stores the pointers, information will be queried as need. type stacktrace []uintptr // Format formats the stack of Frames according to the fmt.Formatter interface. diff --git a/lib/util/security/tls.go b/lib/util/security/tls.go index 60ad1bc0..f3ea9b17 100644 --- a/lib/util/security/tls.go +++ b/lib/util/security/tls.go @@ -22,7 +22,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" - "io/ioutil" "math/big" "net" "os" @@ -70,14 +69,14 @@ func createTLSConfigificates(logger *zap.Logger, certpath, keypath, capath strin return err } - if err := ioutil.WriteFile(certpath, certPEM.Bytes(), 0600); err != nil { + if err := os.WriteFile(certpath, certPEM.Bytes(), 0600); err != nil { return err } - if err := ioutil.WriteFile(keypath, keyPEM.Bytes(), 0600); err != nil { + if err := os.WriteFile(keypath, keyPEM.Bytes(), 0600); err != nil { return err } if capath != "" { - if err := ioutil.WriteFile(capath, caPEM.Bytes(), 0600); err != nil { + if err := os.WriteFile(capath, caPEM.Bytes(), 0600); err != nil { return err } } @@ -204,14 +203,16 @@ func CreateTLSConfigForTest() (serverTLSConf *tls.Config, clientTLSConf *tls.Con } serverTLSConf = &tls.Config{ + MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{serverCert}, } certpool := x509.NewCertPool() certpool.AppendCertsFromPEM(caPEM.Bytes()) clientTLSConf = &tls.Config{ + MinVersion: tls.VersionTLS12, InsecureSkipVerify: true, - RootCAs: certpool, + RootCAs: certpool, } return @@ -224,7 +225,9 @@ func BuildServerTLSConfig(logger *zap.Logger, cfg config.TLSConfig) (*tls.Config return nil, nil } - tcfg := &tls.Config{} + tcfg := &tls.Config{ + MinVersion: tls.VersionTLS12, + } cert, err := tls.LoadX509KeyPair(cfg.Cert, cfg.Key) if err != nil { return nil, errors.Errorf("failed to load certs: %w", err) @@ -238,7 +241,7 @@ func BuildServerTLSConfig(logger *zap.Logger, cfg config.TLSConfig) (*tls.Config tcfg.ClientAuth = tls.RequireAndVerifyClientCert tcfg.ClientCAs = x509.NewCertPool() - certBytes, err := ioutil.ReadFile(cfg.CA) + certBytes, err := os.ReadFile(cfg.CA) if err != nil { return nil, errors.Errorf("failed to read CA: %w", err) } @@ -253,15 +256,20 @@ func BuildClientTLSConfig(logger *zap.Logger, cfg config.TLSConfig) (*tls.Config if !cfg.HasCA() { if cfg.SkipCA { // still enable TLS without verify server certs - return &tls.Config{InsecureSkipVerify: true}, nil + return &tls.Config{ + InsecureSkipVerify: true, + MinVersion: tls.VersionTLS12, + }, nil } logger.Warn("no CA to verify server connections, disable TLS") return nil, nil } - tcfg := &tls.Config{} + tcfg := &tls.Config{ + MinVersion: tls.VersionTLS12, + } tcfg.RootCAs = x509.NewCertPool() - certBytes, err := ioutil.ReadFile(cfg.CA) + certBytes, err := os.ReadFile(cfg.CA) if err != nil { return nil, errors.Errorf("failed to read CA: %w", err) } diff --git a/pkg/manager/config/utils.go b/pkg/manager/config/utils.go index 6214c39c..3a525949 100644 --- a/pkg/manager/config/utils.go +++ b/pkg/manager/config/utils.go @@ -1,4 +1,3 @@ - // Copyright 2022 PingCAP, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/manager/logger/manager.go b/pkg/manager/logger/manager.go index 3f5f008d..08b8fdb2 100644 --- a/pkg/manager/logger/manager.go +++ b/pkg/manager/logger/manager.go @@ -19,6 +19,7 @@ import ( "encoding/json" "github.com/pingcap/TiProxy/lib/config" + "github.com/pingcap/TiProxy/lib/util/cmd" "github.com/pingcap/TiProxy/lib/util/waitgroup" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -27,39 +28,28 @@ import ( // LoggerManager updates log configurations online. type LoggerManager struct { // The logger used by LoggerManager itself to log. - logger *zap.Logger - encoder zapcore.Encoder - syncer *AtomicWriteSyncer - level zap.AtomicLevel - cancel context.CancelFunc - wg waitgroup.WaitGroup + logger *zap.Logger + syncer *cmd.AtomicWriteSyncer + level zap.AtomicLevel + cancel context.CancelFunc + wg waitgroup.WaitGroup } // NewLoggerManager creates a new LoggerManager. func NewLoggerManager(cfg *config.Log) (*LoggerManager, *zap.Logger, error) { lm := &LoggerManager{} var err error - if lm.encoder, err = buildEncoder(cfg); err != nil { + mainLogger, syncer, level, err := cmd.BuildLogger(cfg) + if err != nil { return nil, nil, err } - if lm.syncer, err = buildSyncer(cfg); err != nil { - return nil, nil, err - } - if lm.level, err = buildLevel(cfg); err != nil { - return nil, nil, err - } - mainLogger := lm.buildLogger().Named("main") + lm.syncer = syncer + lm.level = level + mainLogger = mainLogger.Named("main") lm.logger = mainLogger.Named("lgmgr") return lm, mainLogger, nil } -// buildLogger returns a new logger with the same syncer. -func (lm *LoggerManager) buildLogger() *zap.Logger { - return zap.New(zapcore.NewCore(lm.encoder, lm.syncer, lm.level), - zap.ErrorOutput(lm.syncer), - zap.AddStacktrace(zapcore.FatalLevel)) -} - // Init starts a goroutine to watch configuration. func (lm *LoggerManager) Init(cfgCh <-chan *config.LogOnline) { ctx, cancel := context.WithCancel(context.Background()) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 2467697e..bd8c5875 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -110,7 +110,7 @@ func (mm *MetricsManager) pushMetric(ctx context.Context, addr string, interval // registerProxyMetrics registers metrics. func registerProxyMetrics() { - prometheus.DefaultRegisterer.Unregister(prometheus.NewGoCollector()) + prometheus.DefaultRegisterer.Unregister(collectors.NewGoCollector()) prometheus.MustRegister(collectors.NewGoCollector(collectors.WithGoCollections(collectors.GoRuntimeMetricsCollection | collectors.GoRuntimeMemStatsCollection))) prometheus.MustRegister(ConnGauge) diff --git a/pkg/proxy/backend/authenticator.go b/pkg/proxy/backend/authenticator.go index ea223266..69930d88 100644 --- a/pkg/proxy/backend/authenticator.go +++ b/pkg/proxy/backend/authenticator.go @@ -96,7 +96,7 @@ func (auth *Authenticator) handshakeFirstTime(clientIO, backendIO *pnet.PacketIO addr := backendIO.RemoteAddr().String() if auth.serverAddr != "" { // NOTE: should use DNS name as much as possible - // Usally certs are signed with domain instead of IP addrs + // Usually certs are signed with domain instead of IP addrs // And `RemoteAddr()` will return IP addr addr = auth.serverAddr } diff --git a/pkg/proxy/backend/backend_conn.go b/pkg/proxy/backend/backend_conn.go index 8df2a035..dce69d2c 100644 --- a/pkg/proxy/backend/backend_conn.go +++ b/pkg/proxy/backend/backend_conn.go @@ -18,8 +18,8 @@ import ( "net" "time" - pnet "github.com/pingcap/TiProxy/pkg/proxy/net" "github.com/pingcap/TiProxy/lib/util/errors" + pnet "github.com/pingcap/TiProxy/pkg/proxy/net" ) const ( diff --git a/pkg/proxy/backend/cmd_processor_exec.go b/pkg/proxy/backend/cmd_processor_exec.go index 0b6d76f0..1695aab8 100644 --- a/pkg/proxy/backend/cmd_processor_exec.go +++ b/pkg/proxy/backend/cmd_processor_exec.go @@ -18,8 +18,8 @@ import ( "encoding/binary" "strings" - pnet "github.com/pingcap/TiProxy/pkg/proxy/net" "github.com/pingcap/TiProxy/lib/util/errors" + pnet "github.com/pingcap/TiProxy/pkg/proxy/net" "github.com/pingcap/tidb/parser" "github.com/pingcap/tidb/parser/mysql" "github.com/siddontang/go/hack" @@ -196,7 +196,7 @@ func (cp *CmdProcessor) forwardQueryCmd(clientIO, backendIO *pnet.PacketIO, requ case mysql.LocalInFileHeader: serverStatus, err = cp.forwardLoadInFile(clientIO, backendIO, request) default: - serverStatus, err = cp.forwardResultSet(clientIO, backendIO, request, response) + serverStatus, err = cp.forwardResultSet(clientIO, backendIO, request) } if err != nil { return err @@ -239,8 +239,9 @@ func (cp *CmdProcessor) forwardLoadInFile(clientIO, backendIO *pnet.PacketIO, re return serverStatus, errors.Errorf("unexpected response, cmd:%d resp:%d", mysql.ComQuery, response[0]) } -func (cp *CmdProcessor) forwardResultSet(clientIO, backendIO *pnet.PacketIO, request, response []byte) (uint16, error) { +func (cp *CmdProcessor) forwardResultSet(clientIO, backendIO *pnet.PacketIO, request []byte) (uint16, error) { if cp.capability&mysql.ClientDeprecateEOF == 0 { + var response []byte // read columns for { var err error diff --git a/pkg/proxy/backend/cmd_processor_query.go b/pkg/proxy/backend/cmd_processor_query.go index 61faa3ad..a2e111b3 100644 --- a/pkg/proxy/backend/cmd_processor_query.go +++ b/pkg/proxy/backend/cmd_processor_query.go @@ -18,8 +18,8 @@ import ( "encoding/binary" gomysql "github.com/go-mysql-org/go-mysql/mysql" - pnet "github.com/pingcap/TiProxy/pkg/proxy/net" "github.com/pingcap/TiProxy/lib/util/errors" + pnet "github.com/pingcap/TiProxy/pkg/proxy/net" "github.com/pingcap/tidb/parser/mysql" "github.com/siddontang/go/hack" ) diff --git a/pkg/proxy/backend/mock_backend_test.go b/pkg/proxy/backend/mock_backend_test.go index ef2e44b0..4480d130 100644 --- a/pkg/proxy/backend/mock_backend_test.go +++ b/pkg/proxy/backend/mock_backend_test.go @@ -25,19 +25,19 @@ import ( type backendConfig struct { // for auth - tlsConfig *tls.Config - authPlugin string - salt []byte - columns int - loops int - params int - rows int - respondType respondType // for cmd - stmtNum int - capability uint32 - status uint16 - authSucceed bool - switchAuth bool + tlsConfig *tls.Config + authPlugin string + salt []byte + columns int + loops int + params int + rows int + respondType respondType // for cmd + stmtNum int + capability uint32 + status uint16 + authSucceed bool + switchAuth bool // for both auth and cmd abnormalExit bool } diff --git a/pkg/proxy/backend/mock_client_test.go b/pkg/proxy/backend/mock_client_test.go index 08040337..52c8a52a 100644 --- a/pkg/proxy/backend/mock_client_test.go +++ b/pkg/proxy/backend/mock_client_test.go @@ -24,20 +24,20 @@ import ( type clientConfig struct { // for auth - tlsConfig *tls.Config - sql string - username string - dbName string - authPlugin string - dataBytes []byte - attrs []byte - authData []byte - filePkts int - prepStmtID int - capability uint32 - collation uint8 + tlsConfig *tls.Config + sql string + username string + dbName string + authPlugin string + dataBytes []byte + attrs []byte + authData []byte + filePkts int + prepStmtID int + capability uint32 + collation uint8 // for cmd - cmd byte + cmd byte // for both auth and cmd abnormalExit bool } @@ -206,7 +206,7 @@ func (mc *mockClient) requestPrepare(packetIO *pnet.PacketIO) error { } } for i := 0; i < expectedPacketNum; i++ { - if response, err = packetIO.ReadPacket(); err != nil { + if _, err = packetIO.ReadPacket(); err != nil { return err } } diff --git a/pkg/proxy/backend/testsuite_test.go b/pkg/proxy/backend/testsuite_test.go index 11695b79..9f79721f 100644 --- a/pkg/proxy/backend/testsuite_test.go +++ b/pkg/proxy/backend/testsuite_test.go @@ -75,9 +75,6 @@ func getCfgCombinations(cfgs [][]cfgOverrider) [][]cfgOverrider { // Append the cfg to each of the existing overrider list. for _, cfg := range cfgList { for _, o := range cfgOverriders { - newOverrider := make([]cfgOverrider, 0, len(o)+1) - newOverrider = append(newOverrider, o...) - newOverrider = append(newOverrider, cfg) newOverriders = append(newOverriders, append(o, cfg)) } } diff --git a/pkg/proxy/net/packetio.go b/pkg/proxy/net/packetio.go index 1a7d29ec..914b3d5d 100644 --- a/pkg/proxy/net/packetio.go +++ b/pkg/proxy/net/packetio.go @@ -147,7 +147,7 @@ func (p *PacketIO) readOnePacket() ([]byte, bool, error) { // probe proxy V2 refill := false if !p.proxyInited.Load() { - if bytes.Compare(header[:], proxyV2Magic[:4]) == 0 { + if bytes.Equal(header[:], proxyV2Magic[:4]) { proxyHeader, err := p.parseProxyV2() if err != nil { return nil, false, errors.Wrap(ErrReadConn, err) diff --git a/pkg/proxy/net/proxy.go b/pkg/proxy/net/proxy.go index bd8ea19b..2ba39a65 100644 --- a/pkg/proxy/net/proxy.go +++ b/pkg/proxy/net/proxy.go @@ -151,7 +151,7 @@ func (p *PacketIO) parseProxyV2() (*Proxy, error) { if err != nil { return nil, errors.WithStack(errors.Wrap(ErrReadConn, err)) } - if bytes.Compare(rem, proxyV2Magic[4:]) != 0 { + if !bytes.Equal(rem, proxyV2Magic[4:]) { return nil, nil } diff --git a/pkg/server/api/namespace.go b/pkg/server/api/namespace.go index 95ccf944..eaa37985 100644 --- a/pkg/server/api/namespace.go +++ b/pkg/server/api/namespace.go @@ -93,6 +93,10 @@ func (h *namespaceHttpHandler) HandleCommit(c *gin.Context) { if len(ns_names) == 0 { nss, err = h.cfgmgr.ListAllNamespace(c) if err != nil { + errMsg := "failed to list all namespaces" + h.logger.Error(errMsg, zap.Error(err), zap.Any("namespaces", nss)) + c.JSON(http.StatusInternalServerError, errMsg) + return } } else { nss = make([]*config.Namespace, len(ns_names)) @@ -100,6 +104,10 @@ func (h *namespaceHttpHandler) HandleCommit(c *gin.Context) { for i, ns_name := range ns_names { ns, err := h.cfgmgr.GetNamespace(c, ns_name) if err != nil { + errMsg := "failed to get namespace" + h.logger.Error(errMsg, zap.Error(err), zap.Any("namespaces", nss)) + c.JSON(http.StatusInternalServerError, errMsg) + return } nss[i] = ns nss_delete[i] = false