Code clean up and added linter to Makefile

Makefile

@@ -1,10 +1,14 @@
 .DEFAULT_GOAL := release
 VERSION=$(shell cat version)
 LDFLAGS="-X main.Version=$(VERSION)"
+GOLINT = $(GOPATH)/bin/golangci-lint
+
+$(GOLINT):
+	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v1.54.1
 
 .PHONY: lint
-lint:
-	go vet ./...
+lint: $(GOLINT)
+	$(GOLINT) run --out-format tab --path-prefix `pwd`
 
 .PHONY: test
 test:

client.go

@@ -17,6 +17,7 @@ limitations under the License.
 package gubernator
 
 import (
+	crand "crypto/rand"
 	"crypto/tls"
 	"math/rand"
 	"time"
@@ -26,6 +27,7 @@ import (
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 const (
@@ -52,7 +54,7 @@ func DialV1Server(server string, tls *tls.Config) (V1Client, error) {
 	if tls != nil {
 		opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(tls)))
 	} else {
-		opts = append(opts, grpc.WithInsecure())
+		opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	}
 
 	conn, err := grpc.Dial(server, opts...)
@@ -94,11 +96,11 @@ func RandomPeer(peers []PeerInfo) PeerInfo {
 
 // RandomString returns a random alpha string of 'n' length
 func RandomString(n int) string {
-	const alphanum = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+	const alphanumeric = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
 	var bytes = make([]byte, n)
-	rand.Read(bytes)
+	_, _ = crand.Read(bytes)
 	for i, b := range bytes {
-		bytes[i] = alphanum[b%byte(len(alphanum))]
+		bytes[i] = alphanumeric[b%byte(len(alphanumeric))]
 	}
 	return string(bytes)
 }

cluster/cluster.go

@@ -21,7 +21,7 @@ import (
 	"fmt"
 	"math/rand"
 
-	"github.com/mailgun/gubernator/v2"
+	gubernator "github.com/mailgun/gubernator/v2"
 	"github.com/mailgun/holster/v4/clock"
 	"github.com/mailgun/holster/v4/ctxutil"
 	"github.com/mailgun/holster/v4/errors"

cmd/gubernator-cli/main.go

@@ -147,7 +147,7 @@ func main() {
 				req := obj.(*guber.GetRateLimitsReq)
 
 				if reqRate > 0 {
-					limiter.Wait(ctx)
+					_ = limiter.Wait(ctx)
 				}
 
 				sendRequest(ctx, client, req)

cmd/gubernator/main.go

@@ -25,7 +25,7 @@ import (
 	"runtime"
 	"syscall"
 
-	"github.com/mailgun/gubernator/v2"
+	gubernator "github.com/mailgun/gubernator/v2"
 	"github.com/mailgun/holster/v4/clock"
 	"github.com/mailgun/holster/v4/ctxutil"
 	"github.com/mailgun/holster/v4/tracing"
@@ -53,7 +53,7 @@ func main() {
 	// klog (https://github.com/kubernetes/klog), we need to
 	// initialize klog in the way it prints to stderr only.
 	klog.InitFlags(nil)
-	flag.Set("logtostderr", "true")
+	_ = flag.Set("logtostderr", "true")
 
 	res, err := tracing.NewResource("gubernator", Version, resource.NewWithAttributes(
 		semconv.SchemaURL,

config.go

@@ -23,7 +23,7 @@ import (
 	"crypto/x509"
 	"encoding/hex"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net"
 	"os"
 	"runtime"
@@ -511,7 +511,7 @@ func setupEtcdTLS(conf *etcd.Config) error {
 		setter.SetDefault(&conf.TLS, &tls.Config{})
 
 		var certPool *x509.CertPool = nil
-		if pemBytes, err := ioutil.ReadFile(tlsCAFile); err == nil {
+		if pemBytes, err := os.ReadFile(tlsCAFile); err == nil {
 			certPool = x509.NewCertPool()
 			certPool.AppendCertsFromPEM(pemBytes)
 		} else {
@@ -521,7 +521,7 @@ func setupEtcdTLS(conf *etcd.Config) error {
 		conf.TLS.InsecureSkipVerify = false
 	}
 
-	// If the cert and key files are provided attempt to load them
+	// If the cert and key files are provided, attempt to load them
 	if tlsCertFile != "" && tlsKeyFile != "" {
 		tlsCert, err := tls.LoadX509KeyPair(tlsCertFile, tlsKeyFile)
 		if err != nil {
@@ -623,7 +623,7 @@ func fromEnvFile(log logrus.FieldLogger, configFile string) error {
 		return fmt.Errorf("while opening config file: %s", err)
 	}
 
-	contents, err := ioutil.ReadAll(fd)
+	contents, err := io.ReadAll(fd)
 	if err != nil {
 		return fmt.Errorf("while reading config file '%s': %s", configFile, err)
 	}
@@ -649,15 +649,15 @@ func fromEnvFile(log logrus.FieldLogger, configFile string) error {
 
 func validClientAuthTypes(m map[string]tls.ClientAuthType) string {
 	var rs []string
-	for k, _ := range m {
+	for k := range m {
 		rs = append(rs, k)
 	}
 	return strings.Join(rs, ",")
 }
 
 func validHash64Keys(m map[string]HashString64) string {
 	var rs []string
-	for k, _ := range m {
+	for k := range m {
 		rs = append(rs, k)
 	}
 	return strings.Join(rs, ",")

daemon.go

@@ -38,6 +38,7 @@ import (
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/keepalive"
 	"google.golang.org/protobuf/encoding/protojson"
 )
@@ -96,7 +97,7 @@ func (s *Daemon) Start(ctx context.Context) error {
 
 	// Handler to collect duration and API access metrics for GRPC
 	s.statsHandler = NewGRPCStatsHandler()
-	s.promRegister.Register(s.statsHandler)
+	_ = s.promRegister.Register(s.statsHandler)
 
 	var filters []otelgrpc.Option
 	if s.conf.TraceLevel != tracing.DebugLevel {
@@ -151,7 +152,7 @@ func (s *Daemon) Start(ctx context.Context) error {
 	}
 
 	// V1Server instance also implements prometheus.Collector interface
-	s.promRegister.Register(s.V1Server)
+	_ = s.promRegister.Register(s.V1Server)
 
 	l, err := net.Listen("tcp", s.conf.GRPCListenAddress)
 	if err != nil {
@@ -222,7 +223,7 @@ func (s *Daemon) Start(ctx context.Context) error {
 		s.conf.MemberListPoolConf.OnUpdate = s.V1Server.SetPeers
 		s.conf.MemberListPoolConf.Logger = s.log
 
-		// Register peer on member list
+		// Register peer on the member list
 		s.pool, err = NewMemberListPool(ctx, s.conf.MemberListPoolConf)
 		if err != nil {
 			return errors.Wrap(err, "while creating member list pool")
@@ -232,7 +233,7 @@ func (s *Daemon) Start(ctx context.Context) error {
 	// We override the default Marshaller to enable the `UseProtoNames` option.
 	// We do this is because the default JSONPb in 2.5.0 marshals proto structs using
 	// `camelCase`, while all the JSON annotations are `under_score`.
-	// Our protobuf files follow convention described here
+	// Our protobuf files follow the convention described here
 	// https://developers.google.com/protocol-buffers/docs/style#message-and-field-names
 	// Camel case breaks unmarshalling our GRPC gateway responses with protobuf structs.
 	gateway := runtime.NewServeMux(
@@ -247,10 +248,11 @@ func (s *Daemon) Start(ctx context.Context) error {
 		}),
 	)
 
-	// Setup an JSON Gateway API for our GRPC methods
+	// Set up an JSON Gateway API for our GRPC methods
 	var gwCtx context.Context
 	gwCtx, s.gwCancel = context.WithCancel(context.Background())
-	err = RegisterV1HandlerFromEndpoint(gwCtx, gateway, gatewayAddr, []grpc.DialOption{grpc.WithInsecure()})
+	err = RegisterV1HandlerFromEndpoint(gwCtx, gateway, gatewayAddr,
+		[]grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())})
 	if err != nil {
 		return errors.Wrap(err, "while registering GRPC gateway handler")
 	}
@@ -361,10 +363,10 @@ func (s *Daemon) Close() {
 	}
 
 	s.log.Infof("HTTP Gateway close for %s ...", s.conf.HTTPListenAddress)
-	s.httpSrv.Shutdown(context.Background())
+	_ = s.httpSrv.Shutdown(context.Background())
 	if s.httpSrvNoMTLS != nil {
 		s.log.Infof("HTTP Status Gateway close for %s ...", s.conf.HTTPStatusListenAddress)
-		s.httpSrvNoMTLS.Shutdown(context.Background())
+		_ = s.httpSrvNoMTLS.Shutdown(context.Background())
 	}
 	for i, srv := range s.grpcSrvs {
 		s.log.Infof("GRPC close for %s ...", s.GRPCListeners[i].Addr())
@@ -417,25 +419,23 @@ func WaitForConnect(ctx context.Context, addresses []string) error {
 				continue
 			}
 
-			// TODO: golang 15.3 introduces tls.DialContext(). When we are ready to drop
+			// TODO: golang 1.15.3 introduces tls.DialContext(). When we are ready to drop
 			//  support for older versions we can detect tls and use the tls.DialContext to
 			//  avoid the `http: TLS handshake error` we get when using TLS.
 			conn, err := d.DialContext(ctx, "tcp", addr)
 			if err != nil {
 				errs = append(errs, err)
 				continue
 			}
-			conn.Close()
+			_ = conn.Close()
 		}
 
 		if len(errs) == 0 {
 			break
 		}
 
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		}
+		<-ctx.Done()
+		return ctx.Err()
 	}
 
 	if len(errs) != 0 {

etcd.go

@@ -190,7 +190,7 @@ func (e *EtcdPool) watch() error {
 				e.log.Errorf("watch error: %v", err)
 				goto restart
 			}
-			e.collectPeers(&rev)
+			_ = e.collectPeers(&rev)
 		}
 
 	restart:

functional_test.go

@@ -1060,14 +1060,11 @@ func TestHealthCheck(t *testing.T) {
 	})
 	require.Nil(t, err)
 
-	// Stop the rest of the cluster to ensure errors occur on our instance and
-	// collect daemons to restart the stopped peers after the test completes
-	var daemons []*guber.Daemon
+	// Stop the rest of the cluster to ensure errors occur on our instance
 	for i := 1; i < cluster.NumOfDaemons(); i++ {
 		d := cluster.DaemonAt(i)
 		require.NotNil(t, d)
 		d.Close()
-		daemons = append(daemons, d)
 	}
 
 	// Hit the global rate limit again this time causing a connection error
@@ -1191,6 +1188,7 @@ func TestGRPCGateway(t *testing.T) {
 
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	b, err = io.ReadAll(resp.Body)
+	require.NoError(t, err)
 	var r guber.GetRateLimitsResp
 
 	// NOTE: It is important to use 'protojson' instead of the standard 'json' package

gubernator.go

@@ -19,7 +19,6 @@ package gubernator
 import (
 	"context"
 	"fmt"
-	"go.opentelemetry.io/otel/propagation"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -32,6 +31,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -536,28 +536,20 @@ func (s *V1Instance) HealthCheck(ctx context.Context, r *HealthCheckReq) (health
 	// Iterate through local peers and get their last errors
 	localPeers := s.conf.LocalPicker.Peers()
 	for _, peer := range localPeers {
-		lastErr := peer.GetLastErr()
-
-		if lastErr != nil {
-			for _, errMsg := range lastErr {
-				err := fmt.Errorf("Error returned from local peer.GetLastErr: %s", errMsg)
-				span.RecordError(err)
-				errs = append(errs, err.Error())
-			}
+		for _, errMsg := range peer.GetLastErr() {
+			err := fmt.Errorf("Error returned from local peer.GetLastErr: %s", errMsg)
+			span.RecordError(err)
+			errs = append(errs, err.Error())
 		}
 	}
 
 	// Do the same for region peers
 	regionPeers := s.conf.RegionPicker.Peers()
 	for _, peer := range regionPeers {
-		lastErr := peer.GetLastErr()
-
-		if lastErr != nil {
-			for _, errMsg := range lastErr {
-				err := fmt.Errorf("Error returned from region peer.GetLastErr: %s", errMsg)
-				span.RecordError(err)
-				errs = append(errs, err.Error())
-			}
+		for _, errMsg := range peer.GetLastErr() {
+			err := fmt.Errorf("Error returned from region peer.GetLastErr: %s", errMsg)
+			span.RecordError(err)
+			errs = append(errs, err.Error())
 		}
 	}
 

kubernetes.go

@@ -22,7 +22,6 @@ import (
 	"reflect"
 
 	"github.com/mailgun/holster/v4/setter"
-	"github.com/mailgun/holster/v4/syncutil"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	api_v1 "k8s.io/api/core/v1"
@@ -36,7 +35,6 @@ import (
 type K8sPool struct {
 	informer    cache.SharedIndexInformer
 	client      *kubernetes.Clientset
-	wg          syncutil.WaitGroup
 	log         FieldLogger
 	conf        K8sPoolConfig
 	watchCtx    context.Context

lrucache_test.go

@@ -378,7 +378,7 @@ func TestLRUCache(t *testing.T) {
 		<-collChan
 		m := <-collChan // gubernator_unexpired_evictions_count
 		met := new(dto.Metric)
-		m.Write(met)
+		_ = m.Write(met)
 		assert.Contains(t, m.Desc().String(), "gubernator_unexpired_evictions_count")
 		assert.Equal(t, 0, int(*met.Counter.Value))
 	})
@@ -422,7 +422,7 @@ func TestLRUCache(t *testing.T) {
 		<-collChan
 		m := <-collChan // gubernator_unexpired_evictions_count
 		met := new(dto.Metric)
-		m.Write(met)
+		_ = m.Write(met)
 		assert.Contains(t, m.Desc().String(), "gubernator_unexpired_evictions_count")
 		assert.Equal(t, 1, int(*met.Counter.Value))
 	})