Upgrade golangci-lint to v1.31.0 (dapr#2175)

Co-authored-by: Yaron Schneider <[email protected]>

.github/workflows/dapr.yml

@@ -22,7 +22,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     env:
       GOVER: 1.15
-      GOLANGCILINT_VER: 1.26.0
+      GOLANGCILINT_VER: 1.31.0
       GOOS: ${{ matrix.target_os }}
       GOARCH: ${{ matrix.target_arch }}
       GOPROXY: https://proxy.golang.org

.golangci.yml

@@ -175,6 +175,7 @@ linters-settings:
       - hugeParam
       - ifElseChain
       - singleCaseSwitch
+      - exitAfterDefer
 
     # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
     # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
@@ -237,3 +238,8 @@ linters:
     - testpackage
     - goerr113
     - nestif
+    - nlreturn
+    - exhaustive
+    - noctx
+    - gci
+    - gofumpt

pkg/actors/actor_test.go

@@ -53,7 +53,7 @@ func TestBusyChannel(t *testing.T) {
 	testActor := newActor("testType", "testID")
 	testActor.lock()
 
-	var channelClosed = false
+	channelClosed := false
 	go func() {
 		select {
 		case <-time.After(10 * time.Second):

pkg/actors/actors.go

@@ -305,7 +305,6 @@ func (a *actorsRuntime) callLocalActor(ctx context.Context, req *invokev1.Invoke
 		req.Message().HttpExtension.Verb = commonv1pb.HTTPExtension_PUT
 	}
 	resp, err := a.appChannel.InvokeMethod(ctx, req)
-
 	if err != nil {
 		return nil, err
 	}
@@ -698,7 +697,8 @@ func (a *actorsRuntime) evaluateReminders() {
 			go func(wg *sync.WaitGroup, reminders []Reminder) {
 				defer wg.Done()
 
-				for _, r := range reminders {
+				for i := range reminders {
+					r := reminders[i] // Make a copy since we will refer to this as a reference in this loop.
 					targetActorAddress, _ := a.lookupActorAddress(r.ActorType, r.ActorID)
 					if targetActorAddress == "" {
 						continue
@@ -1212,15 +1212,15 @@ func (a *actorsRuntime) DeleteTimer(ctx context.Context, req *DeleteTimerRequest
 }
 
 func (a *actorsRuntime) GetActiveActorsCount(ctx context.Context) []ActiveActorsCount {
-	var actorCountMap = map[string]int{}
+	actorCountMap := map[string]int{}
 	a.actorsTable.Range(func(key, value interface{}) bool {
 		actorType, _ := a.getActorTypeAndIDFromKey(key.(string))
 		actorCountMap[actorType]++
 
 		return true
 	})
 
-	var activeActorsCount = []ActiveActorsCount{}
+	activeActorsCount := []ActiveActorsCount{}
 	for actorType, count := range actorCountMap {
 		activeActorsCount = append(activeActorsCount, ActiveActorsCount{Type: actorType, Count: count})
 	}

pkg/credentials/grpc.go

@@ -23,6 +23,7 @@ func GetServerOptions(certChain *CertChain) ([]grpc.ServerOption, error) {
 		return opts, nil
 	}
 
+	// nolint:gosec
 	config := &tls.Config{
 		ClientCAs: cp,
 		// Require cert verification

pkg/credentials/tls.go

@@ -12,6 +12,7 @@ func TLSConfigFromCertAndKey(certPem, keyPem []byte, serverName string, rootCA *
 		return nil, err
 	}
 
+	// nolint:gosec
 	config := &tls.Config{
 		InsecureSkipVerify: false,
 		RootCAs:            rootCA,

pkg/diagnostics/utils/metrics_utils_test.go

@@ -14,37 +14,37 @@ import (
 
 func TestWithTags(t *testing.T) {
 	t.Run("one tag", func(t *testing.T) {
-		var appKey = tag.MustNewKey("app_id")
+		appKey := tag.MustNewKey("app_id")
 		mutators := WithTags(appKey, "test")
 		assert.Equal(t, 1, len(mutators))
 	})
 
 	t.Run("two tags", func(t *testing.T) {
-		var appKey = tag.MustNewKey("app_id")
-		var operationKey = tag.MustNewKey("operation")
+		appKey := tag.MustNewKey("app_id")
+		operationKey := tag.MustNewKey("operation")
 		mutators := WithTags(appKey, "test", operationKey, "op")
 		assert.Equal(t, 2, len(mutators))
 	})
 
 	t.Run("three tags", func(t *testing.T) {
-		var appKey = tag.MustNewKey("app_id")
-		var operationKey = tag.MustNewKey("operation")
-		var methodKey = tag.MustNewKey("method")
+		appKey := tag.MustNewKey("app_id")
+		operationKey := tag.MustNewKey("operation")
+		methodKey := tag.MustNewKey("method")
 		mutators := WithTags(appKey, "test", operationKey, "op", methodKey, "method")
 		assert.Equal(t, 3, len(mutators))
 	})
 
 	t.Run("two tags with wrong value type", func(t *testing.T) {
-		var appKey = tag.MustNewKey("app_id")
-		var operationKey = tag.MustNewKey("operation")
+		appKey := tag.MustNewKey("app_id")
+		operationKey := tag.MustNewKey("operation")
 		mutators := WithTags(appKey, "test", operationKey, 1)
 		assert.Equal(t, 1, len(mutators))
 	})
 
 	t.Run("skip empty value key", func(t *testing.T) {
-		var appKey = tag.MustNewKey("app_id")
-		var operationKey = tag.MustNewKey("operation")
-		var methodKey = tag.MustNewKey("method")
+		appKey := tag.MustNewKey("app_id")
+		operationKey := tag.MustNewKey("operation")
+		methodKey := tag.MustNewKey("method")
 		mutators := WithTags(appKey, "", operationKey, "op", methodKey, "method")
 		assert.Equal(t, 2, len(mutators))
 	})

pkg/grpc/api.go

@@ -18,12 +18,10 @@ import (
 	"github.com/dapr/dapr/pkg/channel"
 	"github.com/dapr/dapr/pkg/concurrency"
 	"github.com/dapr/dapr/pkg/config"
-	"github.com/dapr/dapr/pkg/diagnostics"
 	diag "github.com/dapr/dapr/pkg/diagnostics"
 	diag_utils "github.com/dapr/dapr/pkg/diagnostics/utils"
 	"github.com/dapr/dapr/pkg/messaging"
 	invokev1 "github.com/dapr/dapr/pkg/messaging/v1"
-	"github.com/dapr/dapr/pkg/proto/common/v1"
 	commonv1pb "github.com/dapr/dapr/pkg/proto/common/v1"
 	internalv1pb "github.com/dapr/dapr/pkg/proto/internals/v1"
 	runtimev1pb "github.com/dapr/dapr/pkg/proto/runtime/v1"
@@ -118,7 +116,7 @@ func (a *api) CallLocal(ctx context.Context, in *internalv1pb.InternalInvokeRequ
 	if a.accessControlList != nil {
 		// An access control policy has been specified for the app. Apply the policies.
 		operation := req.Message().Method
-		var httpVerb common.HTTPExtension_Verb
+		var httpVerb commonv1pb.HTTPExtension_Verb
 		// Get the http verb in case the application protocol is http
 		if a.appProtocol == config.HTTPProtocol && req.Metadata() != nil && len(req.Metadata()) > 0 {
 			httpExt := req.Message().GetHttpExtension()
@@ -141,7 +139,7 @@ func (a *api) CallLocal(ctx context.Context, in *internalv1pb.InternalInvokeRequ
 	return resp.Proto(), err
 }
 
-func (a *api) applyAccessControlPolicies(ctx context.Context, operation string, httpVerb common.HTTPExtension_Verb, appProtocol string) (bool, string) {
+func (a *api) applyAccessControlPolicies(ctx context.Context, operation string, httpVerb commonv1pb.HTTPExtension_Verb, appProtocol string) (bool, string) {
 	// Apply access control list filter
 	spiffeID, err := config.GetAndParseSpiffeID(ctx)
 	if err != nil {
@@ -580,16 +578,16 @@ func emitACLMetrics(actionPolicy, appID, trustDomain, namespace, operation, verb
 	if action {
 		switch actionPolicy {
 		case config.ActionPolicyApp:
-			diagnostics.DefaultMonitoring.RequestAllowedByAppAction(appID, trustDomain, namespace, operation, verb, action)
+			diag.DefaultMonitoring.RequestAllowedByAppAction(appID, trustDomain, namespace, operation, verb, action)
 		case config.ActionPolicyGlobal:
-			diagnostics.DefaultMonitoring.RequestAllowedByGlobalAction(appID, trustDomain, namespace, operation, verb, action)
+			diag.DefaultMonitoring.RequestAllowedByGlobalAction(appID, trustDomain, namespace, operation, verb, action)
 		}
 	} else {
 		switch actionPolicy {
 		case config.ActionPolicyApp:
-			diagnostics.DefaultMonitoring.RequestBlockedByAppAction(appID, trustDomain, namespace, operation, verb, action)
+			diag.DefaultMonitoring.RequestBlockedByAppAction(appID, trustDomain, namespace, operation, verb, action)
 		case config.ActionPolicyGlobal:
-			diagnostics.DefaultMonitoring.RequestBlockedByGlobalAction(appID, trustDomain, namespace, operation, verb, action)
+			diag.DefaultMonitoring.RequestBlockedByGlobalAction(appID, trustDomain, namespace, operation, verb, action)
 		}
 	}
 }

pkg/grpc/api_test.go

@@ -39,7 +39,6 @@ import (
 	"go.opencensus.io/trace"
 	epb "google.golang.org/genproto/googleapis/rpc/errdetails"
 	"google.golang.org/grpc"
-	grpc_go "google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
@@ -113,7 +112,7 @@ func configureTestTraceExporter(meta exporters.Metadata) {
 	exporter.Init("fakeID", "fakeAddress", meta)
 }
 
-func startTestServerWithTracing(port int) (*grpc_go.Server, *string) {
+func startTestServerWithTracing(port int) (*grpc.Server, *string) {
 	lis, _ := net.Listen("tcp", fmt.Sprintf(":%d", port))
 
 	var buffer = ""
@@ -125,8 +124,8 @@ func startTestServerWithTracing(port int) (*grpc_go.Server, *string) {
 	})
 
 	spec := config.TracingSpec{SamplingRate: "1"}
-	server := grpc_go.NewServer(
-		grpc_go.UnaryInterceptor(grpc_middleware.ChainUnaryServer(diag.GRPCTraceUnaryServerInterceptor("id", spec))),
+	server := grpc.NewServer(
+		grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer(diag.GRPCTraceUnaryServerInterceptor("id", spec))),
 	)
 
 	go func() {
@@ -142,14 +141,14 @@ func startTestServerWithTracing(port int) (*grpc_go.Server, *string) {
 	return server, &buffer
 }
 
-func startTestServer(port int) *grpc_go.Server {
+func startTestServer(port int) *grpc.Server {
 	return startTestServerAPI(port, &mockGRPCAPI{})
 }
 
-func startTestServerAPI(port int, srv runtimev1pb.DaprServer) *grpc_go.Server {
+func startTestServerAPI(port int, srv runtimev1pb.DaprServer) *grpc.Server {
 	lis, _ := net.Listen("tcp", fmt.Sprintf(":%d", port))
 
-	server := grpc_go.NewServer()
+	server := grpc.NewServer()
 	go func() {
 		runtimev1pb.RegisterDaprServer(server, srv)
 		if err := server.Serve(lis); err != nil {
@@ -163,10 +162,10 @@ func startTestServerAPI(port int, srv runtimev1pb.DaprServer) *grpc_go.Server {
 	return server
 }
 
-func startInternalServer(port int, testAPIServer *api) *grpc_go.Server {
+func startInternalServer(port int, testAPIServer *api) *grpc.Server {
 	lis, _ := net.Listen("tcp", fmt.Sprintf(":%d", port))
 
-	server := grpc_go.NewServer()
+	server := grpc.NewServer()
 	go func() {
 		internalv1pb.RegisterServiceInvocationServer(server, testAPIServer)
 		if err := server.Serve(lis); err != nil {
@@ -180,17 +179,17 @@ func startInternalServer(port int, testAPIServer *api) *grpc_go.Server {
 	return server
 }
 
-func startDaprAPIServer(port int, testAPIServer *api, token string) *grpc_go.Server {
+func startDaprAPIServer(port int, testAPIServer *api, token string) *grpc.Server {
 	lis, _ := net.Listen("tcp", fmt.Sprintf(":%d", port))
 
-	opts := []grpc_go.ServerOption{}
+	opts := []grpc.ServerOption{}
 	if token != "" {
 		opts = append(opts,
-			grpc_go.UnaryInterceptor(setAPIAuthenticationMiddlewareUnary(token, "dapr-api-token")),
+			grpc.UnaryInterceptor(setAPIAuthenticationMiddlewareUnary(token, "dapr-api-token")),
 		)
 	}
 
-	server := grpc_go.NewServer(opts...)
+	server := grpc.NewServer(opts...)
 	go func() {
 		runtimev1pb.RegisterDaprServer(server, testAPIServer)
 		if err := server.Serve(lis); err != nil {
@@ -204,10 +203,8 @@ func startDaprAPIServer(port int, testAPIServer *api, token string) *grpc_go.Ser
 	return server
 }
 
-func createTestClient(port int) *grpc_go.ClientConn {
-	var opts []grpc_go.DialOption
-	opts = append(opts, grpc_go.WithInsecure())
-	conn, err := grpc_go.Dial(fmt.Sprintf("localhost:%d", port), opts...)
+func createTestClient(port int) *grpc.ClientConn {
+	conn, err := grpc.Dial(fmt.Sprintf("localhost:%d", port), grpc.WithInsecure())
 	if err != nil {
 		panic(err)
 	}

pkg/grpc/grpc.go

@@ -97,6 +97,7 @@ func (g *Manager) GetGRPCConnection(address, id string, namespace string, skipTL
 			serverName = fmt.Sprintf("%s.%s.svc.cluster.local", id, namespace)
 		}
 
+		// nolint:gosec
 		ta := credentials.NewTLS(&tls.Config{
 			ServerName:   serverName,
 			Certificates: []tls.Certificate{cert},

pkg/grpc/server.go

@@ -179,6 +179,7 @@ func (s *server) getGRPCServer() (*grpc_go.Server, error) {
 			return nil, err
 		}
 
+		// nolint:gosec
 		tlsConfig := tls.Config{
 			ClientCAs:  s.signedCert.TrustChain,
 			ClientAuth: tls.RequireAndVerifyClientCert,

pkg/http/api_test.go

@@ -1319,7 +1319,8 @@ type fakeStateStore struct {
 }
 
 func (c fakeStateStore) BulkDelete(req []state.DeleteRequest) error {
-	for _, r := range req {
+	for i := range req {
+		r := req[i] // Make a copy since we will refer to this as a reference in this loop.
 		err := c.Delete(&r)
 		if err != nil {
 			return err
@@ -1330,7 +1331,8 @@ func (c fakeStateStore) BulkDelete(req []state.DeleteRequest) error {
 }
 
 func (c fakeStateStore) BulkSet(req []state.SetRequest) error {
-	for _, s := range req {
+	for i := range req {
+		s := req[i] // Make a copy since we will refer to this as a reference in this loop.
 		err := c.Set(&s)
 		if err != nil {
 			return err

pkg/injector/pod_patch.go

@@ -78,7 +78,7 @@ const (
 	defaultMtlsEnabled                = true
 	trueString                        = "true"
 
-	// Deprecated, remove in v1.0
+	// Deprecated: remove in v1.0
 	idKey                 = "dapr.io/id"
 	daprPortKey           = "dapr.io/port"
 	daprProfilingKey      = "dapr.io/profiling"

pkg/injector/pod_patch_test.go

@@ -20,30 +20,30 @@ import (
 
 func TestLogAsJSONEnabled(t *testing.T) {
 	t.Run("dapr.io/log-as-json is true", func(t *testing.T) {
-		var fakeAnnotation = map[string]string{
+		fakeAnnotation := map[string]string{
 			daprLogAsJSON: "true",
 		}
 
 		assert.Equal(t, true, logAsJSONEnabled(fakeAnnotation))
 	})
 
 	t.Run("dapr.io/log-as-json is false", func(t *testing.T) {
-		var fakeAnnotation = map[string]string{
+		fakeAnnotation := map[string]string{
 			daprLogAsJSON: "false",
 		}
 
 		assert.Equal(t, false, logAsJSONEnabled(fakeAnnotation))
 	})
 
 	t.Run("dapr.io/log-as-json is not given", func(t *testing.T) {
-		var fakeAnnotation = map[string]string{}
+		fakeAnnotation := map[string]string{}
 
 		assert.Equal(t, false, logAsJSONEnabled(fakeAnnotation))
 	})
 }
 
 func TestFormatProbePath(t *testing.T) {
-	var testCases = []struct {
+	testCases := []struct {
 		given    []string
 		expected string
 	}{
@@ -96,7 +96,7 @@ func TestGetSideCarContainer(t *testing.T) {
 
 	container, _ := getSidecarContainer(annotations, "app_id", "darpio/dapr", "dapr-system", "controlplane:9000", "placement:50000", nil, "", "", "", "sentry:50000", true, "pod_identity")
 
-	var expectedArgs = []string{
+	expectedArgs := []string{
 		"--mode", "kubernetes",
 		"--dapr-http-port", "3500",
 		"--dapr-grpc-port", "50001",

pkg/logger/options_test.go

@@ -60,7 +60,7 @@ func TestApplyOptionsToLoggers(t *testing.T) {
 	}
 
 	// Create two loggers
-	var testLoggers = []Logger{
+	testLoggers := []Logger{
 		NewLogger("testLogger0"),
 		NewLogger("testLogger1"),
 	}

pkg/messaging/direct_messaging.go

@@ -27,7 +27,6 @@ import (
 	"google.golang.org/grpc/status"
 
 	invokev1 "github.com/dapr/dapr/pkg/messaging/v1"
-	v1 "github.com/dapr/dapr/pkg/messaging/v1"
 	internalv1pb "github.com/dapr/dapr/pkg/proto/internals/v1"
 )
 
@@ -174,7 +173,7 @@ func (d *directMessaging) invokeRemote(ctx context.Context, appID, namespace, ap
 }
 
 func (d *directMessaging) addDestinationAppIDHeaderToMetadata(appID string, req *invokev1.InvokeMethodRequest) {
-	req.Metadata()[v1.DestinationIDHeader] = &internalv1pb.ListStringValue{
+	req.Metadata()[invokev1.DestinationIDHeader] = &internalv1pb.ListStringValue{
 		Values: []string{appID},
 	}
 }