Update CI following latest plugindemo version (#34)

* Update CI following latest plugindemo version

* disable some linter

* Disable some linter

* Fix linter in redis, add error logger

* 🚨 Fix lint errors in private packages

* 🚨 Update lints in package and core code

* 🚨 exclude unfixable lint errors

* 🚨 update lint at package level and ignore necessary global variable

* 🍱 fix lint qnd merge

* 🚨 Fix Lint for string

* 🚨 Fix lint for bouncer

* 🚨 Fix error

* Fix weird linter error

Co-authored-by: Max Lerebourg <maxlerebourg@gmail.com>

.github/workflows/go-cross.yml

@@ -11,7 +11,7 @@ jobs:
 
     strategy:
       matrix:
-        go-version: [ 1.17, 1.x ]
+        go-version: [ 1.18, 1.x ]
         os: [ubuntu-latest, macos-latest, windows-latest]
 
     steps:

.github/workflows/main.yml

@@ -12,9 +12,9 @@ jobs:
     name: Main Process
     runs-on: ubuntu-latest
     env:
-      GO_VERSION: 1.17
-      GOLANGCI_LINT_VERSION: v1.46.2
-      YAEGI_VERSION: v0.13.0
+      GO_VERSION: 1.18
+      GOLANGCI_LINT_VERSION: v1.50.0
+      YAEGI_VERSION: v0.14.2
       CGO_ENABLED: 0
     defaults:
       run:

.golangci.yml

@@ -29,11 +29,20 @@ linters-settings:
 linters:
   enable-all: true
   disable:
+    - deadcode # deprecated
+    - exhaustivestruct # deprecated
+    - golint # deprecated
+    - ifshort # deprecated
     - interfacer # deprecated
     - maligned # deprecated
+    - nosnakecase # deprecated
     - scopelint # deprecated
-    - golint # deprecated
-    - exhaustivestruct # deprecated
+    - scopelint # deprecated
+    - structcheck # deprecated
+    - varcheck # deprecated
+    - sqlclosecheck # not relevant (SQL)
+    - rowserrcheck # not relevant (SQL)
+    - execinquery # not relevant (SQL)
     - cyclop # duplicate of gocyclo
     - bodyclose # Too many false positives: https://github.com/timakin/bodyclose/issues/30
     - dupl
@@ -52,6 +61,9 @@ linters:
     - gomnd
     - forbidigo
     - varnamelen
+    - wastedassign # is disabled because of generics
+    - gofumpt
+    - gci
 
 issues:
   exclude-use-default: false

bouncer.go

@@ -1,4 +1,6 @@
-package crowdsec_bouncer_traefik_plugin
+// Package crowdsec_bouncer_traefik_plugin implements a middleware that communicates with crowdsec.
+// It can cache results to filesystem or redis, or even ask crowdsec for every requests.
+package crowdsec_bouncer_traefik_plugin //nolint:revive,stylecheck
 
 import (
 	"bytes"
@@ -15,7 +17,7 @@ import (
 	cache "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/cache"
 	ip "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/ip"
 	logger "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/logger"
-	simpleredis "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/redis"
+	simpleredis "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/simpleredis"
 )
 
 const (
@@ -28,6 +30,7 @@ const (
 	cacheTimeoutKey         = "updated"
 )
 
+//nolint:gochecknoglobals
 var (
 	crowdsecStreamHealthy = false
 	ticker                chan bool
@@ -61,8 +64,8 @@ func CreateConfig() *Config {
 		CrowdsecLapiKey:            "",
 		UpdateIntervalSeconds:      60,
 		DefaultDecisionSeconds:     60,
-		ForwardedHeadersTrustedIPs: []string{},
 		ClientTrustedIPs:           []string{},
+		ForwardedHeadersTrustedIPs: []string{},
 		ForwardedHeadersCustomName: "X-Forwarded-For",
 		RedisCacheEnabled:          false,
 		RedisCacheHost:             "redis:6379",
@@ -83,8 +86,8 @@ type Bouncer struct {
 	updateInterval         int64
 	defaultDecisionTimeout int64
 	customHeader           string
-	serverPoolStrategy     *ip.PoolStrategy
 	clientPoolStrategy     *ip.PoolStrategy
+	serverPoolStrategy     *ip.PoolStrategy
 	client                 *http.Client
 }
 
@@ -141,41 +144,44 @@ func New(ctx context.Context, next http.Handler, config *Config, name string) (h
 }
 
 // ServeHTTP principal function of plugin.
+//
+//nolint:nestif
 func (bouncer *Bouncer) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	if !bouncer.enabled {
 		bouncer.next.ServeHTTP(rw, req)
 		return
 	}
+
 	// Here we check for the trusted IPs in the customHeader
-	remoteHost, err := ip.GetRemoteIP(req, bouncer.serverPoolStrategy, bouncer.customHeader)
+	remoteIP, err := ip.GetRemoteIP(req, bouncer.serverPoolStrategy, bouncer.customHeader)
 	if err != nil {
-		logger.Info(err.Error())
-		bouncer.next.ServeHTTP(rw, req)
+		logger.Error(fmt.Sprintf("ServeHTTP ip:%s %s", remoteIP, err.Error()))
+		rw.WriteHeader(http.StatusForbidden)
 		return
 	}
-
-	trusted, err := bouncer.clientPoolStrategy.Checker.Contains(remoteHost)
+	trusted, err := bouncer.clientPoolStrategy.Checker.Contains(remoteIP)
 	if err != nil {
 		logger.Info(err.Error())
 		return
 	}
 	// if our IP is in the trusted list we bypass the next checks
-	logger.Debug(fmt.Sprintf("ServeHTTP ip:%v isTrusted:%v", remoteHost, trusted))
+	logger.Debug(fmt.Sprintf("ServeHTTP ip:%s isTrusted:%v", remoteIP, trusted))
 	if trusted {
 		bouncer.next.ServeHTTP(rw, req)
 		return
 	}
 
+	// TODO This should be simplified
 	healthy := crowdsecStreamHealthy
 	if bouncer.crowdsecMode != noneMode {
-		isBanned, err := cache.GetDecision(remoteHost)
+		isBanned, err := cache.GetDecision(remoteIP)
 		if err != nil {
-			logger.Debug(err.Error())
+			logger.Error(err.Error())
 			if err.Error() == simpleredis.RedisUnreachable {
 				healthy = false
 			}
 		} else {
-			logger.Debug(fmt.Sprintf("ServeHTTP cache:hit isBanned:%v", isBanned))
+			logger.Debug(fmt.Sprintf("ServeHTTP ip:%s cache:hit isBanned:%v", remoteIP, isBanned))
 			if isBanned {
 				rw.WriteHeader(http.StatusForbidden)
 			} else {
@@ -193,7 +199,7 @@ func (bouncer *Bouncer) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 			rw.WriteHeader(http.StatusForbidden)
 		}
 	} else {
-		handleNoStreamCache(bouncer, rw, req, remoteHost)
+		handleNoStreamCache(bouncer, rw, req, remoteIP)
 	}
 }
 
@@ -245,12 +251,12 @@ func startTicker(config *Config, work func()) chan bool {
 }
 
 // We are now in none or live mode.
-func handleNoStreamCache(bouncer *Bouncer, rw http.ResponseWriter, req *http.Request, remoteHost string) {
+func handleNoStreamCache(bouncer *Bouncer, rw http.ResponseWriter, req *http.Request, remoteIP string) {
 	routeURL := url.URL{
 		Scheme:   bouncer.crowdsecScheme,
 		Host:     bouncer.crowdsecHost,
 		Path:     crowdsecLapiRoute,
-		RawQuery: fmt.Sprintf("ip=%v&banned=true", remoteHost),
+		RawQuery: fmt.Sprintf("ip=%v&banned=true", remoteIP),
 	}
 	body, err := crowdsecQuery(bouncer, routeURL.String())
 	if err != nil {
@@ -261,7 +267,7 @@ func handleNoStreamCache(bouncer *Bouncer, rw http.ResponseWriter, req *http.Req
 
 	if bytes.Equal(body, []byte("null")) {
 		if bouncer.crowdsecMode == liveMode {
-			cache.SetDecision(remoteHost, false, bouncer.defaultDecisionTimeout)
+			cache.SetDecision(remoteIP, false, bouncer.defaultDecisionTimeout)
 		}
 		bouncer.next.ServeHTTP(rw, req)
 		return
@@ -276,7 +282,7 @@ func handleNoStreamCache(bouncer *Bouncer, rw http.ResponseWriter, req *http.Req
 	}
 	if len(decisions) == 0 {
 		if bouncer.crowdsecMode == liveMode {
-			cache.SetDecision(remoteHost, false, bouncer.defaultDecisionTimeout)
+			cache.SetDecision(remoteIP, false, bouncer.defaultDecisionTimeout)
 		}
 		bouncer.next.ServeHTTP(rw, req)
 		return
@@ -288,7 +294,7 @@ func handleNoStreamCache(bouncer *Bouncer, rw http.ResponseWriter, req *http.Req
 		return
 	}
 	if bouncer.crowdsecMode == liveMode {
-		cache.SetDecision(remoteHost, true, int64(duration.Seconds()))
+		cache.SetDecision(remoteIP, true, int64(duration.Seconds()))
 	}
 }
 
@@ -340,7 +346,7 @@ func crowdsecQuery(bouncer *Bouncer, stringURL string) ([]byte, error) {
 	req.Header.Add(crowdsecLapiHeader, bouncer.crowdsecKey)
 	res, err := bouncer.client.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("error while fetching %v: %s", stringURL, err)
+		return nil, fmt.Errorf("error while fetching %v: %w", stringURL, err)
 	}
 	if res.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("error while fetching %v, status code: %d", stringURL, res.StatusCode)
@@ -348,12 +354,12 @@ func crowdsecQuery(bouncer *Bouncer, stringURL string) ([]byte, error) {
 	defer func(body io.ReadCloser) {
 		err = body.Close()
 		if err != nil {
-			logger.Info(fmt.Sprintf("failed to close body reader: %s", err))
+			logger.Error(fmt.Sprintf("failed to close body reader: %s", err.Error()))
 		}
 	}(res.Body)
 	body, err := ioutil.ReadAll(res.Body)
 	if err != nil {
-		return nil, fmt.Errorf("error while reading body: %s", err)
+		return nil, fmt.Errorf("error while reading body: %w", err)
 	}
 	return body, nil
 }

bouncer_test.go

@@ -1,4 +1,4 @@
-package crowdsec_bouncer_traefik_plugin
+package crowdsec_bouncer_traefik_plugin //nolint:revive,stylecheck
 
 import (
 	"context"

pkg/cache/cache.go

@@ -1,3 +1,5 @@
+// Package cache implements utility routines for manipulating cache.
+// It supports currently local file and redis cache.
 package cache
 
 import (
@@ -6,7 +8,7 @@ import (
 	ttl_map "github.com/leprosus/golang-ttl-map"
 
 	logger "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/logger"
-	simpleredis "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/redis"
+	simpleredis "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/simpleredis"
 )
 
 const (
@@ -14,12 +16,14 @@ const (
 	cacheNoBannedValue = "f"
 )
 
-var cache = ttl_map.New()
-var redis simpleredis.SimpleRedis
+//nolint:gochecknoglobals
+var (
+	cache        = ttl_map.New()
+	redis        simpleredis.SimpleRedis
+	redisEnabled = false
+)
 
-var redisEnabled = false
-
-// CLASSIC
+// FileSystem Cache
 
 func getDecisionLocalCache(clientIP string) (bool, error) {
 	banned, isCached := cache.Get(clientIP)
@@ -38,7 +42,7 @@ func deleteDecisionLocalCache(clientIP string) {
 	cache.Del(clientIP)
 }
 
-// REDIS
+// Redis Cache
 
 func getDecisionRedisCache(clientIP string) (bool, error) {
 	banned, err := redis.Get(clientIP)
@@ -50,14 +54,18 @@ func getDecisionRedisCache(clientIP string) (bool, error) {
 }
 
 func setDecisionRedisCache(clientIP string, value string, duration int64) {
-	redis.Set(clientIP, []byte(value), duration)
+	if err := redis.Set(clientIP, []byte(value), duration); err != nil {
+		logger.Error(fmt.Sprintf("cache:setDecisionRedisCache %s", err.Error()))
+	}
 }
 
 func deleteDecisionRedisCache(clientIP string) {
-	redis.Del(clientIP)
+	if err := redis.Del(clientIP); err != nil {
+		logger.Error(fmt.Sprintf("cache:deleteDecisionRedisCache %s", err.Error()))
+	}
 }
 
-// DeleteDecision delete decision in cache
+// DeleteDecision delete decision in cache.
 func DeleteDecision(clientIP string) {
 	if redisEnabled {
 		deleteDecisionRedisCache(clientIP)
@@ -71,15 +79,15 @@ func DeleteDecision(clientIP string) {
 func GetDecision(clientIP string) (bool, error) {
 	if redisEnabled {
 		return getDecisionRedisCache(clientIP)
-	} else {
-		return getDecisionLocalCache(clientIP)
 	}
+	return getDecisionLocalCache(clientIP)
 }
 
+// SetDecision update the cache with the IP as key and the value banned / not banned.
 func SetDecision(clientIP string, isBanned bool, duration int64) {
 	var value string
 	if isBanned {
-		logger.Debug(fmt.Sprintf("%v banned", clientIP))
+		logger.Debug(fmt.Sprintf("cache:SetDecision ip:%v banned", clientIP))
 		value = cacheBannedValue
 	} else {
 		value = cacheNoBannedValue
@@ -91,8 +99,9 @@ func SetDecision(clientIP string, isBanned bool, duration int64) {
 	}
 }
 
+// InitRedisClient loads variables.
 func InitRedisClient(host string) {
 	redisEnabled = true
 	redis.Init(host)
-	logger.Debug("Redis initialized")
+	logger.Debug("cache:InitRedisClient redis:initialized")
 }

pkg/ip/ip.go

@@ -1,3 +1,5 @@
+// Package ip implements utility routines for to manipulates IP and CIDR.
+// It allows to find on IP on a list, and find if an IP is part of a list of CIDR.
 package ip
 
 import (

pkg/logger/logger.go

@@ -1,3 +1,5 @@
+// Package logger implements utility routines to write to stdout and stderr.
+// It supports debug, info and error level
 package logger
 
 import (
@@ -7,29 +9,31 @@ import (
 )
 
 var (
-	loggerInfo  = log.New(io.Discard, "INFO: CrowdsecBouncerTraefikPlugin: ", log.Ldate|log.Ltime)
-	loggerDebug = log.New(io.Discard, "DEBUG: CrowdsecBouncerTraefikPlugin: ", log.Ldate|log.Ltime)
+	loggerInfo  = log.New(io.Discard, "INFO: CrowdsecBouncerTraefikPlugin: ", log.Ldate|log.Ltime)  //nolint:gochecknoglobals
+	loggerDebug = log.New(io.Discard, "DEBUG: CrowdsecBouncerTraefikPlugin: ", log.Ldate|log.Ltime) //nolint:gochecknoglobals
+	loggerError = log.New(io.Discard, "ERROR: CrowdsecBouncerTraefikPlugin: ", log.Ldate|log.Ltime) //nolint:gochecknoglobals
 )
 
-// Init Set Default log level to info in case log level to defined
+// Init Set Default log level to info in case log level to defined.
 func Init(logLevel string) {
-	switch logLevel {
-	case "INFO":
-		loggerInfo.SetOutput(os.Stdout)
-	case "DEBUG":
+	loggerError.SetOutput(os.Stderr)
 	loggerInfo.SetOutput(os.Stdout)
+	if logLevel == "DEBUG" {
 		loggerDebug.SetOutput(os.Stdout)
-	default:
-		loggerInfo.SetOutput(os.Stdout)
 	}
 }
 
-// Info Log info
+// Info log to Stdout.
 func Info(str string) {
 	loggerInfo.Printf(str)
 }
 
-// Info Log debug
+// Debug log to Stdout.
 func Debug(str string) {
 	loggerDebug.Printf(str)
 }
+
+// Error log to Stderr.
+func Error(str string) {
+	loggerError.Printf(str)
+}

pkg/simpleredis/simpleredis.go

@@ -1,3 +1,6 @@
+// Package simpleredis implements utility routines for interacting.
+// It supports currently the following operations: GET, SET, DELETE,
+// and support timetoleave for keys.
 package simpleredis
 
 import (
@@ -11,12 +14,14 @@ import (
 	logger "github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/pkg/logger"
 )
 
+// Error strings for redis.
 const (
 	RedisUnreachable = "redis:unreachable"
 	RedisMiss        = "redis:miss"
 	RedisTimeout     = "redis:timeout"
 )
 
+// A RedisCmd is used to communicate with redis at low level using commands.
 type RedisCmd struct {
 	Command  string
 	Name     string
@@ -25,6 +30,7 @@ type RedisCmd struct {
 	Error    error
 }
 
+// A SimpleRedis is used to communicate with redis.
 type SimpleRedis struct {
 	redisHost string
 }
@@ -39,6 +45,14 @@ func genRedisArray(params ...[]byte) []byte {
 	return []byte(MSG)
 }
 
+func send(wr *textproto.Writer, method string, data []byte) {
+	if err := wr.PrintfLine(string(data)); err != nil {
+		logger.Error(fmt.Sprintf("redis:%s  %s", method, err.Error()))
+	} else {
+		logger.Debug(fmt.Sprintf("redis:%s", method))
+	}
+}
+
 func askRedis(hostnamePort string, cmd RedisCmd, channel chan RedisCmd) {
 	dialer := net.Dialer{Timeout: 2 * time.Second}
 	conn, err := dialer.Dial("tcp", hostnamePort)
@@ -46,24 +60,25 @@ func askRedis(hostnamePort string, cmd RedisCmd, channel chan RedisCmd) {
 		channel <- RedisCmd{Error: fmt.Errorf(RedisUnreachable)}
 		return
 	}
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			logger.Error(fmt.Sprintf("redis:connClose %s", err.Error()))
+		}
+	}()
 
 	writer := textproto.NewWriter(bufio.NewWriter(conn))
 	reader := textproto.NewReader(bufio.NewReader(conn))
 
 	switch cmd.Command {
 	case "SET":
-		data := genRedisArray([]byte("SET"), []byte(cmd.Name), []byte(cmd.Data), []byte("EX"), []byte(fmt.Sprintf("%v", cmd.Duration)))
-		writer.PrintfLine(string(data))
-		logger.Debug("redis:set")
+		data := genRedisArray([]byte("SET"), []byte(cmd.Name), cmd.Data, []byte("EX"), []byte(fmt.Sprintf("%d", cmd.Duration)))
+		send(writer, "set", data)
 	case "DEL":
 		data := genRedisArray([]byte("DEL"), []byte(cmd.Name))
-		writer.PrintfLine(string(data))
-		logger.Debug("redis:del")
+		send(writer, "del", data)
 	case "GET":
 		data := genRedisArray([]byte("GET"), []byte(cmd.Name))
-		writer.PrintfLine(string(data))
-		logger.Debug("redis:get")
+		send(writer, "get", data)
 		for {
 			select {
 			case <-time.After(time.Second * 1):
@@ -83,10 +98,12 @@ func askRedis(hostnamePort string, cmd RedisCmd, channel chan RedisCmd) {
 	}
 }
 
+// Init sets the redisHost used to connect to redis.
 func (sr *SimpleRedis) Init(redisHost string) {
 	sr.redisHost = redisHost
 }
 
+// Get fetches the value for key name in redis.
 func (sr *SimpleRedis) Get(name string) ([]byte, error) {
 	redisCmd := RedisCmd{
 		Command: "GET",
@@ -101,6 +118,7 @@ func (sr *SimpleRedis) Get(name string) ([]byte, error) {
 	return resp.Data, nil
 }
 
+// Set update the value for key name in redis with value data for duration.
 func (sr *SimpleRedis) Set(name string, data []byte, duration int64) error {
 	redisCmd := RedisCmd{
 		Command:  "SET",
@@ -112,6 +130,7 @@ func (sr *SimpleRedis) Set(name string, data []byte, duration int64) error {
 	return nil
 }
 
+// Del remove the key name in redis.
 func (sr *SimpleRedis) Del(name string) error {
 	redisCmd := RedisCmd{
 		Command: "DEL",