Merge branch 'develop'

.github/workflows/github-actions.yml

@@ -54,6 +54,12 @@ jobs:
             go get -v -t -d ./...
         fi
 
+    - name: Install go ci lint
+      run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.27.0
+
+    - name: Run Linter
+      run: golangci-lint run --timeout=10m -v
+
     - name: Verify
       run: |
         make verify

.golangci.yml

+linters-settings:
+  govet:
+    check-shadowing: true
+  golint:
+    min-confidence: 0
+  gocyclo:
+    min-complexity: 10
+  maligned:
+    suggest-new: true
+  dupl:
+    threshold: 100
+  goconst:
+    min-len: 2
+    min-occurrences: 2
+  depguard:
+    list-type: blacklist
+    packages:
+      # logging is allowed only by logutils.Log, logrus
+      # is allowed to use only in logutils package
+      - github.com/sirupsen/logrus
+  misspell:
+    locale: US
+  lll:
+    line-length: 140
+  goimports:
+    local-prefixes: github.com/golangci/golangci-lint
+  gocritic:
+    enabled-tags:
+      - performance
+      - style
+      - experimental
+    disabled-checks:
+      - wrapperFunc
+
+linters:
+  disable-all: true
+  enable:
+    - govet
+    - staticcheck
+    - ineffassign
+    - misspell
+
+run:
+  skip-dirs:
+    - test/testdata_etc
+    - pkg/golinters/goanalysis/(checker|passes)
+
+issues:
+  exclude-rules:
+    - text: "weak cryptographic primitive"
+      linters:
+        - gosec
+    - linters:
+        - staticcheck
+      text: "SA1019:"
+
+# golangci.com configuration
+# https://github.com/golangci/golangci/wiki/Configuration
+service:
+  golangci-lint-version: 1.15.x # use the fixed version to not introduce new linters unexpectedly
+  prepare:
+    - echo "here I can run custom commands, but no preparation needed for this repo"
+

.travis.yml

@@ -15,7 +15,7 @@ install: true
 
 # define ci-stage
 script:
-  - go fmt ./... && [[ -z `git status -s` ]]
+  - go fmt ./... && git checkout -- go.mod && [[ -z `git status -s` ]]
   # license-check
   - make verify
   # integrate-test

cluster/cluster_impl/failback_cluster_invoker.go

@@ -77,8 +77,7 @@ func (invoker *failbackClusterInvoker) tryTimerTaskProc(ctx context.Context, ret
 	invoked = append(invoked, retryTask.lastInvoker)
 
 	retryInvoker := invoker.doSelect(retryTask.loadbalance, retryTask.invocation, retryTask.invokers, invoked)
-	var result protocol.Result
-	result = retryInvoker.Invoke(ctx, retryTask.invocation)
+	result := retryInvoker.Invoke(ctx, retryTask.invocation)
 	if result.Error() != nil {
 		retryTask.lastInvoker = retryInvoker
 		invoker.checkRetry(retryTask, result.Error())
@@ -121,8 +120,11 @@ func (invoker *failbackClusterInvoker) checkRetry(retryTask *retryTimerTask, err
 	if retryTask.retries > invoker.maxRetries {
 		logger.Errorf("Failed retry times exceed threshold (%v), We have to abandon, invocation-> %v.\n",
 			retryTask.retries, retryTask.invocation)
-	} else {
-		invoker.taskList.Put(retryTask)
+		return
+	}
+
+	if err := invoker.taskList.Put(retryTask); err != nil {
+		logger.Errorf("invoker.taskList.Put(retryTask:%#v) = error:%v", retryTask, err)
 	}
 }
 

cluster/loadbalance/random_test.go

@@ -83,6 +83,7 @@ func TestRandomlbSelectWeight(t *testing.T) {
 		}
 		selectedInvoker = append(selectedInvoker, s)
 	}
+	assert.Equal(t, 10000, len(selectedInvoker))
 
 	assert.Condition(t, func() bool {
 		// really is 0.9999999999999
@@ -114,6 +115,8 @@ func TestRandomlbSelectWarmup(t *testing.T) {
 		}
 		selectedInvoker = append(selectedInvoker, s)
 	}
+	assert.Equal(t, 10000, len(selectedInvoker))
+
 	assert.Condition(t, func() bool {
 		return selected/10000 < 0.1
 	})

cluster/router/chain/chain.go

@@ -195,6 +195,9 @@ func (c *RouterChain) copyInvokerIfNecessary(cache *InvokerCache) []protocol.Inv
 func (c *RouterChain) buildCache() {
 	origin := c.loadCache()
 	invokers := c.copyInvokerIfNecessary(origin)
+	if len(invokers) == 0 {
+		return
+	}
 
 	var (
 		mutex sync.Mutex

cluster/router/chain/chain_test.go

@@ -76,8 +76,10 @@ conditions:
 
 	_, err = z.Conn.Set(path, []byte(testyml), 0)
 	assert.NoError(t, err)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		_ = ts.Stop()
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, localIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(zk).GetDynamicConfiguration(zkUrl)
@@ -128,11 +130,15 @@ conditions:
 
 	_, err = z.Conn.Set(path, []byte(testyml), 0)
 	assert.NoError(t, err)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		_ = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, localIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(zk).GetDynamicConfiguration(zkUrl)
+	assert.NoError(t, err)
 	config.GetEnvInstance().SetDynamicConfiguration(configuration)
 
 	chain, err := NewRouterChain(getConditionRouteUrl(applicationKey))
@@ -154,11 +160,15 @@ conditions:
 
 func TestRouterChainRoute(t *testing.T) {
 	ts, z, _, err := zookeeper.NewMockZookeeperClient("test", 15*time.Second)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		err = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, localIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(zk).GetDynamicConfiguration(zkUrl)
+	assert.NoError(t, err)
 	config.GetEnvInstance().SetDynamicConfiguration(configuration)
 
 	chain, err := NewRouterChain(getConditionRouteUrl(applicationKey))
@@ -198,11 +208,15 @@ conditions:
 
 	_, err = z.Conn.Set(path, []byte(testyml), 0)
 	assert.NoError(t, err)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		_ = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, localIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(zk).GetDynamicConfiguration(zkUrl)
+	assert.NoError(t, err)
 	config.GetEnvInstance().SetDynamicConfiguration(configuration)
 
 	chain, err := NewRouterChain(getConditionRouteUrl(applicationKey))
@@ -224,12 +238,17 @@ conditions:
 
 func TestRouterChainRouteNoRoute(t *testing.T) {
 	ts, z, _, err := zookeeper.NewMockZookeeperClient("test", 15*time.Second)
-	defer ts.Stop()
-	defer z.Close()
+	assert.Nil(t, err)
+	defer func() {
+		_ = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, localIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(zk).GetDynamicConfiguration(zkUrl)
 	config.GetEnvInstance().SetDynamicConfiguration(configuration)
+	assert.Nil(t, err)
 
 	chain, err := NewRouterChain(getConditionNoRouteUrl(applicationKey))
 	assert.Nil(t, err)

cluster/router/condition/app_router_test.go

@@ -67,8 +67,11 @@ conditions:
 
 	_, err = z.Conn.Set(routerPath, []byte(testYML), 0)
 	assert.NoError(t, err)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		err = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, routerLocalIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(routerZk).GetDynamicConfiguration(zkUrl)
@@ -115,8 +118,11 @@ conditions:
 
 	_, err = z.Conn.Set(routerPath, []byte(testYML), 0)
 	assert.NoError(t, err)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		err = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, routerLocalIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(routerZk).GetDynamicConfiguration(zkUrl)
@@ -154,8 +160,11 @@ conditions:
 
 	_, err = z.Conn.Set(routerPath, []byte(testYML), 0)
 	assert.NoError(t, err)
-	defer ts.Stop()
-	defer z.Close()
+	defer func() {
+		err = ts.Stop()
+		assert.NoError(t, err)
+		z.Close()
+	}()
 
 	zkUrl, _ := common.NewURL(fmt.Sprintf(zkFormat, routerLocalIP, ts.Servers[0].Port))
 	configuration, err := extension.GetConfigCenterFactory(routerZk).GetDynamicConfiguration(zkUrl)

cluster/router/condition/file_test.go

@@ -89,33 +89,41 @@ func TestParseServiceRouterKey(t *testing.T) {
 	assert.Equal(t, "mock-group", grp)
 	assert.Equal(t, "mock-service", srv)
 	assert.Equal(t, "1.0.0", ver)
+	assert.Nil(t, err)
 
 	testString = "mock-group/mock-service"
 	grp, srv, ver, err = parseServiceRouterKey(testString)
 	assert.Equal(t, "mock-group", grp)
 	assert.Equal(t, "mock-service", srv)
 	assert.Equal(t, "", ver)
+	assert.Nil(t, err)
 
 	testString = "mock-service:1.0.0"
 	grp, srv, ver, err = parseServiceRouterKey(testString)
 	assert.Equal(t, "", grp)
 	assert.Equal(t, "mock-service", srv)
 	assert.Equal(t, "1.0.0", ver)
+	assert.Nil(t, err)
 
 	testString = "mock-service"
 	grp, srv, ver, err = parseServiceRouterKey(testString)
 	assert.Equal(t, "", grp)
 	assert.Equal(t, "mock-service", srv)
 	assert.Equal(t, "", ver)
+	assert.NoError(t, err)
 
 	testString = "/mock-service:"
 	grp, srv, ver, err = parseServiceRouterKey(testString)
 	assert.Equal(t, "", grp)
 	assert.Equal(t, "mock-service", srv)
 	assert.Equal(t, "", ver)
+	assert.NoError(t, err)
 
 	testString = "grp:mock-service:123"
 	grp, srv, ver, err = parseServiceRouterKey(testString)
+	assert.Equal(t, "", grp)
+	assert.Equal(t, "", srv)
+	assert.Equal(t, "", ver)
 	assert.Error(t, err)
 
 	testString = ""
@@ -123,4 +131,5 @@ func TestParseServiceRouterKey(t *testing.T) {
 	assert.Equal(t, "", grp)
 	assert.Equal(t, "", srv)
 	assert.Equal(t, "", ver)
+	assert.NoError(t, err)
 }

cluster/router/condition/listenable_router.go

@@ -47,8 +47,8 @@ type listenableRouter struct {
 	conditionRouters []*ConditionRouter
 	routerRule       *RouterRule
 	url              *common.URL
-	force            bool
-	priority         int64
+	//force            bool
+	priority int64
 }
 
 // RouterRule Get RouterRule instance from listenableRouter

cluster/router/condition/router.go

@@ -46,6 +46,13 @@ var (
 	routerPatternReg = regexp.MustCompile(`([&!=,]*)\s*([^&!=,\s]+)`)
 )
 
+var (
+	emptyMatchPair = MatchPair{
+		Matches:    gxset.NewSet(),
+		Mismatches: gxset.NewSet(),
+	}
+)
+
 // ConditionRouter Condition router struct
 type ConditionRouter struct {
 	Pattern       string
@@ -221,14 +228,14 @@ func parseRule(rule string) (map[string]MatchPair, error) {
 				condition[content] = pair
 			}
 		case "=":
-			if &pair == nil {
+			if pair == emptyMatchPair {
 				var startIndex = getStartIndex(rule)
 				return nil, perrors.Errorf("Illegal route rule \"%s\", The error char '%s' at index %d before \"%d\".", rule, separator, startIndex, startIndex)
 			}
 			values = pair.Matches
 			values.Add(content)
 		case "!=":
-			if &pair == nil {
+			if pair == emptyMatchPair {
 				var startIndex = getStartIndex(rule)
 				return nil, perrors.Errorf("Illegal route rule \"%s\", The error char '%s' at index %d before \"%d\".", rule, separator, startIndex, startIndex)
 			}

cluster/router/condition/router_test.go

@@ -70,10 +70,12 @@ func TestNewConditionRouter(t *testing.T) {
 	assert.EqualValues(t, router.ThenCondition, thenRule)
 
 	router, err = NewConditionRouter(nil)
+	assert.Nil(t, router)
 	assert.Error(t, err)
 
 	url, _ = common.NewURL(`condition://0.0.0.0:?application=mock-app&category=routers&force=true&priority=1&router=condition&rule=YSAmT4gYiAmIGQ%3D`)
 	router, err = NewConditionRouter(url)
+	assert.Nil(t, router)
 	assert.Error(t, err)
 
 	url, _ = common.NewURL(`condition://0.0.0.0:?application=mock-app&category=routers&force=true&router=condition&rule=YSAmIGMgPT4gYiAmIGQ%3D`)

cluster/router/healthcheck/default_health_check_test.go

@@ -56,6 +56,7 @@ func TestDefaultHealthCheckerIsHealthy(t *testing.T) {
 	}
 	hc = NewDefaultHealthChecker(url).(*DefaultHealthChecker)
 	healthy = hc.IsHealthy(invoker)
+	assert.False(t, false, healthy)
 	// the outgoing request is more than OUTSTANDING_REQUEST_COUNT_LIMIT, go to unhealthy
 	assert.False(t, hc.IsHealthy(invoker))
 
@@ -67,11 +68,13 @@ func TestDefaultHealthCheckerIsHealthy(t *testing.T) {
 	url.SetParam(constant.OUTSTANDING_REQUEST_COUNT_LIMIT_KEY, "1000")
 	hc = NewDefaultHealthChecker(url).(*DefaultHealthChecker)
 	healthy = hc.IsHealthy(invoker)
+	assert.False(t, false, healthy)
 	assert.False(t, hc.IsHealthy(invoker))
 
 	// reset successive failed count and go to healthy
 	request(url, healthCheckMethodTest, 0, false, true)
 	healthy = hc.IsHealthy(invoker)
+	assert.False(t, false, healthy)
 	assert.True(t, hc.IsHealthy(invoker))
 }
 

cluster/router/tag/file.go

@@ -41,7 +41,7 @@ type FileTagRouter struct {
 	router     *tagRouter
 	routerRule *RouterRule
 	url        *common.URL
-	force      bool
+	//force      bool
 }
 
 // NewFileTagRouter Create file tag router instance with content (from config file)

cluster/router/tag/router_rule.go

@@ -84,9 +84,9 @@ func (t *RouterRule) getTagNames() []string {
 	return result
 }
 
-func (t *RouterRule) hasTag(tag string) bool {
-	return len(t.TagNameToAddresses[tag]) > 0
-}
+//func (t *RouterRule) hasTag(tag string) bool {
+//	return len(t.TagNameToAddresses[tag]) > 0
+//}
 
 func (t *RouterRule) getAddressToTagNames() map[string][]string {
 	return t.AddressToTagNames
@@ -96,10 +96,10 @@ func (t *RouterRule) getTagNameToAddresses() map[string][]string {
 	return t.TagNameToAddresses
 }
 
-func (t *RouterRule) getTags() []Tag {
-	return t.Tags
-}
+//func (t *RouterRule) getTags() []Tag {
+//	return t.Tags
+//}
 
-func (t *RouterRule) setTags(tags []Tag) {
-	t.Tags = tags
-}
+//func (t *RouterRule) setTags(tags []Tag) {
+//	t.Tags = tags
+//}

cluster/router/tag/tag_router.go

@@ -53,7 +53,7 @@ type addrMetadata struct {
 	// application name
 	application string
 	// is rule a runtime rule
-	ruleRuntime bool
+	//ruleRuntime bool
 	// is rule a force rule
 	ruleForce bool
 	// is rule a valid rule
@@ -227,7 +227,7 @@ func (c *tagRouter) Pool(invokers []protocol.Invoker) (router.AddrPool, router.A
 
 // fetchRuleIfNecessary fetches, parses rule and register listener for the further change
 func (c *tagRouter) fetchRuleIfNecessary(invokers []protocol.Invoker) {
-	if invokers == nil || len(invokers) == 0 {
+	if len(invokers) == 0 {
 		return
 	}
 

cluster/router/tag/tag_router_test.go

@@ -73,8 +73,7 @@ const (
 )
 
 var (
-	zkFormat        = "zookeeper://%s:%d"
-	conditionFormat = "condition://%s/com.foo.BarService"
+	zkFormat = "zookeeper://%s:%d"
 )
 
 // MockInvoker is only mock the Invoker to support test tagRouter
@@ -238,7 +237,7 @@ func TestRouteBeijingInvoker(t *testing.T) {
 
 type DynamicTagRouter struct {
 	suite.Suite
-	rule *RouterRule
+	//rule *RouterRule
 
 	route       *tagRouter
 	zkClient    *zookeeper.ZookeeperClient
@@ -310,7 +309,8 @@ tags:
 
 func (suite *DynamicTagRouter) TearDownTest() {
 	suite.zkClient.Close()
-	suite.testCluster.Stop()
+	err := suite.testCluster.Stop()
+	suite.Nil(err)
 }
 
 func (suite *DynamicTagRouter) TestDynamicTagRouterSetByIPv4() {

common/config/environment.go

@@ -34,8 +34,8 @@ import (
 // We just have config center configuration which can override configuration in consumer.yaml & provider.yaml.
 // But for add these features in future ,I finish the environment struct following Environment class in java.
 type Environment struct {
-	configCenterFirst    bool
-	externalConfigs      sync.Map
+	configCenterFirst bool
+	//externalConfigs      sync.Map
 	externalConfigMap    sync.Map
 	appExternalConfigMap sync.Map
 	dynamicConfiguration config_center.DynamicConfiguration

common/constant/key.go

@@ -17,6 +17,8 @@
 
 package constant
 
+type DubboCtxKey string
+
 const (
 	ASYNC_KEY = "async" // it's value should be "true" or "false" of string type
 )
@@ -47,8 +49,8 @@ const (
 	PORT_KEY                 = "port"
 	PROTOCOL_KEY             = "protocol"
 	PATH_SEPARATOR           = "/"
-	DUBBO_KEY                = "dubbo"
-	SSL_ENABLED_KEY          = "ssl-enabled"
+	//DUBBO_KEY                = "dubbo"
+	SSL_ENABLED_KEY = "ssl-enabled"
 )
 
 const (
@@ -92,7 +94,7 @@ const (
 )
 
 const (
-	DUBBOGO_CTX_KEY = "dubbogo-ctx"
+	DUBBOGO_CTX_KEY = DubboCtxKey("dubbogo-ctx")
 )
 
 const (
@@ -198,7 +200,7 @@ const (
 )
 
 const (
-	TRACING_REMOTE_SPAN_CTX = "tracing.remote.span.ctx"
+	TRACING_REMOTE_SPAN_CTX = DubboCtxKey("tracing.remote.span.ctx")
 )
 
 // Use for router module
@@ -238,7 +240,7 @@ const (
 	Tagkey      = "dubbo.tag"
 
 	// Attachment key in context in invoker
-	AttachmentKey = "attachment"
+	AttachmentKey = DubboCtxKey("attachment")
 )
 
 const (
@@ -265,7 +267,7 @@ const (
 	// signature format
 	SIGNATURE_STRING_FORMAT = "%s#%s#%s#%s"
 	// key whether enable signature
-	PARAMTER_SIGNATURE_ENABLE_KEY = "param.sign"
+	PARAMETER_SIGNATURE_ENABLE_KEY = "param.sign"
 	// consumer
 	CONSUMER = "consumer"
 	// key of access key id
@@ -293,7 +295,7 @@ const (
 	HEALTH_CHECKER = "health.checker"
 	// The name of the default implementation of HealthChecker
 	DEFAULT_HEALTH_CHECKER = "default"
-	// The key of oustanding-request-limit
+	// The key of outstanding-request-limit
 	OUTSTANDING_REQUEST_COUNT_LIMIT_KEY = "outstanding.request.limit"
 	// The key of successive-failed-request's threshold
 	SUCCESSIVE_FAILED_REQUEST_THRESHOLD_KEY = "successive.failed.threshold"

common/logger/logger.go

@@ -163,6 +163,7 @@ type OpsLogger interface {
 // SetLoggerLevel use for set logger level
 func (dl *DubboLogger) SetLoggerLevel(level string) {
 	l := new(zapcore.Level)
-	l.Set(level)
-	dl.dynamicLevel.SetLevel(*l)
+	if err := l.Set(level); err == nil {
+		dl.dynamicLevel.SetLevel(*l)
+	}
 }