add golangci lint to github action workflow (#1068)

* add golangci lint to github action workflow

* add golang ci

* update code based on golangci lint

* update workflow

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update based on githubci lint

* update golang ci lint

* update grpc related change

.github/workflows/golangci.lint.yml

@@ -0,0 +1,32 @@
+name: CI Lint
+
+on:
+ pull_request:
+ branches:
+ - main
+ paths:
+ - "**/*.go"
+ - ".github/workflows/golangci.lint.yml"
+
+env:
+ GO_VERSION: "1.20"
+
+jobs:
+
+ analysis:
+ runs-on: ubuntu-latest
+ steps:
+
+ - name: Set up Go
+ uses: actions/setup-go@v4
+ with:
+ go-version: ${{ env.GO_VERSION }}
+
+ - name: Check out code into the Go module directory
+ uses: actions/checkout@v3
+
+ - name: golangci-lint
+ uses: golangci/golangci-lint-action@v3
+ with:
+ version: v1.54
+ args: --timeout=30m --disable=errcheck

cmd/server.go

@@ -20,10 +20,8 @@ package cmd
 
 import (
  "log"
- "math/rand"
  "os"
  "sync"
- "time"
 
  "github.com/megaease/easegress/v2/pkg/api"
  "github.com/megaease/easegress/v2/pkg/cluster"
@@ -40,8 +38,6 @@ import (
 
 // RunServer runs Easegress server.
 func RunServer() {
- rand.Seed(time.Now().UnixNano())
-
  opt := option.New()
  if err := opt.Parse(); err != nil {
  common.Exit(1, err.Error())

pkg/cluster/cluster_test.go

@@ -18,8 +18,8 @@
 package cluster
 
 import (
+ "crypto/rand"
  "fmt"
- "math/rand"
  "os"
  "path"
  "path/filepath"
@@ -48,7 +48,6 @@ func getRandomString(n int) string {
 }
 
 func TestMain(m *testing.M) {
- rand.Seed(time.Now().UnixNano())
  logger.InitNop()
  // logger.InitMock()
  tempDir = path.Join(tempDir, getRandomString(6))

pkg/filters/headerlookup/headerlookup_test.go

@@ -21,7 +21,6 @@ import (
  "net/http"
  "os"
  "sort"
- "sync"
  "testing"
  "time"
 
@@ -79,9 +78,8 @@ func TestValidate(t *testing.T) {
  assert := assert.New(t)
  clusterInstance, _ := createClusterAndSyncer()
 
- var mockMap sync.Map
  supervisor := supervisor.NewMock(
- nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil)
+ nil, clusterInstance, nil, nil, false, nil, nil)
 
  const validYaml = `
 name: headerLookup
@@ -191,9 +189,8 @@ headerSetters:
 
  clusterInstance, syncerChannel := createClusterAndSyncer()
 
- var mockMap sync.Map
  supervisor := supervisor.NewMock(
- nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil)
+ nil, clusterInstance, nil, nil, false, nil, nil)
 
  // let's put data to 'foobar'
  foobar := `
@@ -225,6 +222,7 @@ extra-entry: "extra"
  assert.Equal("123456789", hdr1)
 
  hl, err = createHeaderLookup(config, hl, supervisor)
+ assert.Nil(err)
  ctx, header = prepareCtxAndHeader(t)
 
  // update key-value store
@@ -245,6 +243,7 @@ extra-entry: "extra"
  assert.Equal("77341", header.Get("user-ext-id"))
 
  hl, err = createHeaderLookup(config, hl, supervisor)
+ assert.Nil(err)
  ctx, header = prepareCtxAndHeader(t)
  header.Set("X-AUTH-USER", "foobar")
  // delete foobar completely
@@ -274,9 +273,8 @@ headerSetters:
  headerKey: "user-ext-id"
 `
  clusterInstance, _ := createClusterAndSyncer()
- var mockMap sync.Map
  supervisor := supervisor.NewMock(
- nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil)
+ nil, clusterInstance, nil, nil, false, nil, nil)
  bobbanana := `
 ext-id: "333"
 extra-entry: "extra"

pkg/filters/meshadaptor/meshadaptor.go

@@ -24,7 +24,6 @@ import (
  proxy "github.com/megaease/easegress/v2/pkg/filters/proxies/httpproxy"
  "github.com/megaease/easegress/v2/pkg/protocols/httpprot"
  "github.com/megaease/easegress/v2/pkg/protocols/httpprot/httpheader"
- "github.com/megaease/easegress/v2/pkg/util/pathadaptor"
 )
 
 const (
@@ -52,8 +51,6 @@ type (
  // MeshAdaptor is filter MeshAdaptor.
  MeshAdaptor struct {
  spec *Spec
-
- pa *pathadaptor.PathAdaptor
  }
 
  // Spec is HTTPAdaptor Spec.

pkg/filters/oidcadaptor/oidcadaptor.go

@@ -253,7 +253,7 @@ func (o *OIDCAdaptor) handleOIDCCallback(ctx *context.Context) string {
  if err != nil {
  return filterResp(rw, http.StatusForbidden, err.Error())
  }
- oidcToken, err := o.fetchOIDCToken(authCode, state, spec, err, rw, req)
+ oidcToken, err := o.fetchOIDCToken(authCode, state, spec, rw, req)
  if err != nil {
  return errorResp(rw, "fetch OIDC token error: "+err.Error())
  }
@@ -293,7 +293,7 @@ func (o *OIDCAdaptor) handleOIDCCallback(ctx *context.Context) string {
  return ""
 }
 
-func (o *OIDCAdaptor) fetchOIDCToken(authCode string, state string, spec *Spec, err error, rw *httpprot.Response, req *httpprot.Request) (*oidcIDToken, error) {
+func (o *OIDCAdaptor) fetchOIDCToken(authCode string, state string, spec *Spec, rw *httpprot.Response, req *httpprot.Request) (*oidcIDToken, error) {
  // client_secret_post || client_secret_basic
  tokenFormData := url.Values{
  "client_id": {o.spec.ClientID},

pkg/filters/opafilter/opafilter_test.go

@@ -55,14 +55,13 @@ func createOPAFilter(yamlConfig string, prev *OPAFilter, supervisor *supervisor.
 }
 
 type testCase struct {
- req func() *http.Request
- status int
- shouldHandlerError bool
- shouldRegoError bool
- readBody bool
- policy string
- defaultStatus int
- includedHeaders string
+ req func() *http.Request
+ status int
+ shouldRegoError bool
+ readBody bool
+ policy string
+ defaultStatus int
+ includedHeaders string
 }
 
 func TestOpaPolicyInFilter(t *testing.T) {

pkg/filters/proxies/grpcproxy/codec.go

@@ -52,7 +52,7 @@ func (GrpcCodec) Unmarshal(data []byte, v interface{}) error {
  return proto.Unmarshal(data, v.(proto.Message))
 }
 
-// String return codec name
-func (GrpcCodec) String() string {
+// Name return codec name
+func (GrpcCodec) Name() string {
  return codecName
 }

pkg/filters/proxies/grpcproxy/codec_test.go

@@ -25,7 +25,7 @@ import (
 
 func TestGrpcCodecString(t *testing.T) {
  grpcCodec := GrpcCodec{}
- assert.Equal(t, codecName, grpcCodec.String())
+ assert.Equal(t, codecName, grpcCodec.Name())
 }
 
 func TestCodecMarshalUnmarshal(t *testing.T) {

pkg/filters/proxies/grpcproxy/pool.go

@@ -345,8 +345,8 @@ func (sp *ServerPool) biTransport(ctx *serverPoolContext, proxyAsClientStream gr
  // Explicitly *do not Close* c2sErrChan and c2sErrChan, otherwise the select below will not terminate.
  // Channels do not have to be closed, it is just a control flow mechanism, see
  // https://groups.google.com/forum/#!msg/golang-nuts/pZwdYRGxCIk/qpbHxRRPJdUJ
- c2sErrChan := sp.forwardE2E(ctx.stdr, proxyAsClientStream, nil)
- s2cErrChan := sp.forwardE2E(proxyAsClientStream, ctx.stdw, ctx.resp.RawHeader())
+ c2sErrChan := sp.forwardS2C(ctx.stdr, proxyAsClientStream, nil)
+ s2cErrChan := sp.forwardC2S(proxyAsClientStream, ctx.stdw, ctx.resp.RawHeader())
  // We don't know which side is going to stop sending first, so we need a select between the two.
  for {
  select {
@@ -381,7 +381,7 @@ func (sp *ServerPool) buildOutputResponse(spCtx *serverPoolContext, s *status.St
  spCtx.SetOutputResponse(spCtx.resp)
 }
 
-func (sp *ServerPool) forwardE2E(src grpc.Stream, dst grpc.Stream, header *grpcprot.Header) chan error {
+func (sp *ServerPool) forwardC2S(src grpc.ClientStream, dst grpc.ServerStream, header *grpcprot.Header) chan error {
  ret := make(chan error, 1)
  go func() {
  f := &emptypb.Empty{}
@@ -391,24 +391,40 @@ func (sp *ServerPool) forwardE2E(src grpc.Stream, dst grpc.Stream, header *grpcp
  return
  }
  if i == 0 {
- if cs, ok := src.(grpc.ClientStream); ok {
- // This is a bit of a hack, but client to server headers are only readable after first client msg is
- // received but must be written to server stream before the first msg is flushed.
- // This is the only place to do it nicely.
- md, err := cs.Header()
- if err != nil {
- ret <- err
- return
- }
- if header != nil {
- md = metadata.Join(header.GetMD(), md)
- }
-
- if err = dst.(grpc.ServerStream).SendHeader(md); err != nil {
- ret <- err
- return
- }
+ // This is a bit of a hack, but client to server headers are only readable after first client msg is
+ // received but must be written to server stream before the first msg is flushed.
+ // This is the only place to do it nicely.
+ md, err := src.Header()
+ if err != nil {
+ ret <- err
+ return
  }
+ if header != nil {
+ md = metadata.Join(header.GetMD(), md)
+ }
+
+ if err = dst.SendHeader(md); err != nil {
+ ret <- err
+ return
+ }
+ }
+ if err := dst.SendMsg(f); err != nil {
+ ret <- err
+ return
+ }
+ }
+ }()
+ return ret
+}
+
+func (sp *ServerPool) forwardS2C(src grpc.ServerStream, dst grpc.ClientStream, header *grpcprot.Header) chan error {
+ ret := make(chan error, 1)
+ go func() {
+ f := &emptypb.Empty{}
+ for i := 0; ; i++ {
+ if err := src.RecvMsg(f); err != nil {
+ ret <- err // this can be io.EOF which is happy case
+ return
  }
  if err := dst.SendMsg(f); err != nil {
  ret <- err

pkg/filters/proxies/grpcproxy/proxy.go

@@ -70,8 +70,9 @@ var (
  }
  defaultDialOpts = []grpc.DialOption{
  grpc.WithTransportCredentials(insecure.NewCredentials()),
- grpc.WithCodec(&GrpcCodec{}),
- grpc.WithBlock()}
+ grpc.WithDefaultCallOptions(grpc.ForceCodec(&GrpcCodec{})),
+ grpc.WithBlock(),
+ }
 )
 
 var _ filters.Filter = (*Proxy)(nil)

pkg/filters/proxies/httpproxy/pool_test.go

@@ -19,7 +19,6 @@ package httpproxy
 
 import (
  "net/http"
- "sync"
  "testing"
 
  "github.com/megaease/easegress/v2/pkg/context"
@@ -93,7 +92,7 @@ servers:
  assert.NoError(spec.Validate())
 
  p := &Proxy{}
- p.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
+ p.super = supervisor.NewMock(option.New(), nil, nil,
  nil, false, nil, nil)
  sp := NewServerPool(p, spec, "test")
  policies := map[string]resilience.Policy{}
@@ -135,7 +134,7 @@ servers:
  assert.NoError(spec.Validate())
 
  p := &Proxy{}
- p.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
+ p.super = supervisor.NewMock(option.New(), nil, nil,
  nil, false, nil, nil)
  sp := NewServerPool(p, spec, "test")
  spCtx := &serverPoolContext{
@@ -178,7 +177,7 @@ func TestCopyCORSHeaders(t *testing.T) {
  dst.Add("X-Dst", "dst")
 
  p := &Proxy{}
- p.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
+ p.super = supervisor.NewMock(option.New(), nil, nil,
  nil, false, nil, nil)
  sp := NewServerPool(p, &ServerPoolSpec{}, "test")
  dst = sp.mergeResponseHeader(dst, src)

pkg/filters/proxies/httpproxy/proxy_test.go

@@ -24,7 +24,6 @@ import (
  "net/http/httptest"
  "os"
  "strings"
- "sync"
  "sync/atomic"
  "testing"
  "time"
@@ -58,7 +57,7 @@ func newTestProxy(yamlConfig string, assert *assert.Assertions) *Proxy {
 
  proxy := kind.CreateInstance(spec).(*Proxy)
 
- proxy.super = supervisor.NewMock(option.New(), nil, sync.Map{}, sync.Map{}, nil,
+ proxy.super = supervisor.NewMock(option.New(), nil, nil,
  nil, false, nil, nil)
 
  proxy.Init()

pkg/filters/proxies/loadbalance_test.go

@@ -19,7 +19,6 @@ package proxies
 
 import (
  "fmt"
- "math/rand"
  "net/http"
  "os"
  "sync"
@@ -85,8 +84,6 @@ func TestGeneralLoadBalancer(t *testing.T) {
 }
 
 func TestRandomLoadBalancePolicy(t *testing.T) {
- rand.Seed(0)
-
  counter := [10]int{}
  servers := prepareServers(10)
 
@@ -126,23 +123,21 @@ func TestRoundRobinLoadBalancePolicy(t *testing.T) {
 }
 
 func TestWeightedRandomLoadBalancePolicy(t *testing.T) {
- rand.Seed(0)
-
  counter := [10]int{}
  servers := prepareServers(10)
 
  lb := NewGeneralLoadBalancer(&LoadBalanceSpec{Policy: LoadBalancePolicyWeightedRandom}, servers)
  lb.Init(nil, nil, nil)
 
- for i := 0; i < 1000; i++ {
+ for i := 0; i < 50000; i++ {
  svr := lb.ChooseServer(nil)
  counter[svr.Weight-1]++
  }
 
  v := 0
  for i := 0; i < 10; i++ {
  if v >= counter[i] {
- t.Error("possibility is not weighted even")
+ t.Errorf("possibility is not weighted even %v", counter)
  }
  v = counter[i]
  }