From 0a54f7b09f385d7a855095aa87091d25296005a9 Mon Sep 17 00:00:00 2001 From: mathieuHa Date: Sat, 19 Nov 2022 19:35:01 +0100 Subject: [PATCH] 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 * :rotating_light: Fix lint errors in private packages * :rotating_light: Update lints in package and core code * :rotating_light: exclude unfixable lint errors * :rotating_light: update lint at package level and ignore necessary global variable * :bento: fix lint qnd merge * :rotating_light: Fix Lint for string * :rotating_light: Fix lint for bouncer * :rotating_light: Fix error * Fix weird linter error Co-authored-by: Max Lerebourg --- .github/workflows/go-cross.yml | 2 +- .github/workflows/main.yml | 6 +-- .golangci.yml | 16 +++++- bouncer.go | 50 +++++++++++-------- bouncer_test.go | 2 +- pkg/cache/cache.go | 37 ++++++++------ pkg/ip/ip.go | 2 + pkg/logger/logger.go | 28 ++++++----- .../redis.go => simpleredis/simpleredis.go} | 39 +++++++++++---- 9 files changed, 117 insertions(+), 65 deletions(-) rename pkg/{redis/redis.go => simpleredis/simpleredis.go} (66%) diff --git a/.github/workflows/go-cross.yml b/.github/workflows/go-cross.yml index c28832b..c1cbb51 100644 --- a/.github/workflows/go-cross.yml +++ b/.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: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5b388ba..0025179 100644 --- a/.github/workflows/main.yml +++ b/.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: diff --git a/.golangci.yml b/.golangci.yml index c85b21a..953599f 100644 --- a/.golangci.yml +++ b/.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 diff --git a/bouncer.go b/bouncer.go index a2ccde9..da15dfa 100644 --- a/bouncer.go +++ b/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 } diff --git a/bouncer_test.go b/bouncer_test.go index 6babd3f..8e84c89 100644 --- a/bouncer_test.go +++ b/bouncer_test.go @@ -1,4 +1,4 @@ -package crowdsec_bouncer_traefik_plugin +package crowdsec_bouncer_traefik_plugin //nolint:revive,stylecheck import ( "context" diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 7cd3f79..ea3fc4a 100644 --- a/pkg/cache/cache.go +++ b/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") } diff --git a/pkg/ip/ip.go b/pkg/ip/ip.go index c5631b7..f2b167e 100644 --- a/pkg/ip/ip.go +++ b/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 ( diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 5393aa5..30dffd0 100644 --- a/pkg/logger/logger.go +++ b/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": - loggerInfo.SetOutput(os.Stdout) + 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) +} diff --git a/pkg/redis/redis.go b/pkg/simpleredis/simpleredis.go similarity index 66% rename from pkg/redis/redis.go rename to pkg/simpleredis/simpleredis.go index 64ba15c..53c5965 100644 --- a/pkg/redis/redis.go +++ b/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" + 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",