review: add context handling

Tom Moulard · Tom Moulard · commit 26378aa04b0a · 2026-01-23T10:28:29.000+01:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -88,6 +88,10 @@ issues:
     - text: 'use of `fmt.Println` forbidden' # FIXME: add revert this change ASAP
       linters:
         - forbidigo
+    - text: 'Magic number: 2, in <condition> detected'
+      file: 'pkg/notifications/webhook.go'
+      linters:
+        - mnd
 
 output:
   show-stats: true
diff --git a/fail2ban.go b/fail2ban.go
@@ -79,7 +79,7 @@ func ImportIP(list List) ([]string, error) {
 
 // New instantiates and returns the required components used to handle a HTTP
 // request.
-func New(_ context.Context, next http.Handler, config *Config, _ string) (http.Handler, error) {
+func New(ctx context.Context, next http.Handler, config *Config, _ string) (http.Handler, error) {
 	if !config.Rules.Enabled {
 		log.Println("Plugin: FailToBan is disabled")
 
@@ -140,7 +140,7 @@ func New(_ context.Context, next http.Handler, config *Config, _ string) (http.H
 
 	notifSrvc := notifications.NewService(config.Notifications)
 	if notifSrvc != nil {
-		go notifSrvc.Run()
+		go notifSrvc.Run(context.WithoutCancel(ctx))
 	}
 
 	log.Println("Plugin: FailToBan is up and running")
diff --git a/pkg/fail2ban/fail2ban.go b/pkg/fail2ban/fail2ban.go
@@ -77,7 +77,6 @@ func (u *Fail2Ban) ShouldAllow(remoteIP string) bool {
 		}
 
 		fmt.Println(remoteIP + " is no longer banned")
-		u.notify(notifications.UnbanEvent(remoteIP, fmt.Sprintf("%q is no longer banned", remoteIP)))
 
 		return true
 	}
@@ -92,7 +91,6 @@ func (u *Fail2Ban) ShouldAllow(remoteIP string) bool {
 			msg := fmt.Sprintf("%q is banned for %d>=%d request",
 				remoteIP, ip.Count+1, u.rules.MaxRetry)
 			fmt.Printf("%s", msg)
-			u.notify(notifications.BanEvent(remoteIP, msg, u.rules.Bantime))
 
 			return false
 		}
diff --git a/pkg/notifications/discord.go b/pkg/notifications/discord.go
@@ -2,6 +2,7 @@ package notifications
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -47,8 +48,7 @@ func NewDiscordNotifier(cfg DiscordConfig, httpCli *http.Client) *DiscordNotifie
 	}
 }
 
-//nolint:noctx
-func (d *DiscordNotifier) Send(event Event) error {
+func (d *DiscordNotifier) Send(ctx context.Context, event Event) error {
 	var color int
 
 	switch event.Type {
@@ -90,7 +90,7 @@ func (d *DiscordNotifier) Send(event Event) error {
 		return fmt.Errorf("failed to marshal discord payload: %w", err)
 	}
 
-	req, err := http.NewRequest(http.MethodPost, d.webhookURL, bytes.NewReader(jsonPayload))
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, d.webhookURL, bytes.NewReader(jsonPayload))
 	if err != nil {
 		return fmt.Errorf("failed to create discord request: %w", err)
 	}
diff --git a/pkg/notifications/discord_test.go b/pkg/notifications/discord_test.go
@@ -67,7 +67,9 @@ func TestDiscordNotifier(t *testing.T) {
 			test.config.WebhookURL = server.URL
 			n := NewDiscordNotifier(test.config, server.Client())
 
-			err := n.Send(Event{
+			ctx := t.Context()
+
+			err := n.Send(ctx, Event{
 				Type:      EventTypeBan,
 				IP:        "192.0.2.1",
 				Message:   "test",
diff --git a/pkg/notifications/email.go b/pkg/notifications/email.go
@@ -1,11 +1,20 @@
 package notifications
 
 import (
+	"context"
 	"crypto/tls"
 	"fmt"
 	"io"
+	"net"
 	"net/smtp"
+	"strconv"
 	"sync"
+	"time"
+)
+
+const (
+	// dialerTimeout defines the timeout for establishing SMTP connections.
+	dialerTimeout = 5 * time.Second
 )
 
 type mailer interface {
@@ -45,7 +54,7 @@ func NewEmailNotifier(cfg EmailConfig, templates *TemplateHandler) *EmailNotifie
 	return n
 }
 
-func (e *EmailNotifier) ensureConnected() error {
+func (e *EmailNotifier) ensureConnected(ctx context.Context) error {
 	e.clientMux.Lock()
 	defer e.clientMux.Unlock()
 
@@ -58,7 +67,7 @@ func (e *EmailNotifier) ensureConnected() error {
 		_ = e.client.Close()
 	}
 
-	client, err := createSMTPClient(e.server, e.port, e.username, e.password)
+	client, err := createSMTPClient(ctx, e.server, e.port, e.username, e.password)
 	if err != nil {
 		return fmt.Errorf("failed to reconnect SMTP client: %w", err)
 	}
@@ -68,9 +77,9 @@ func (e *EmailNotifier) ensureConnected() error {
 	return nil
 }
 
-func (e *EmailNotifier) Send(event Event) error {
+func (e *EmailNotifier) Send(ctx context.Context, event Event) error {
 	// Ensure we have a valid connection
-	if err := e.ensureConnected(); err != nil {
+	if err := e.ensureConnected(ctx); err != nil {
 		return fmt.Errorf("failed to ensure SMTP connection: %w", err)
 	}
 
@@ -114,12 +123,21 @@ func (e *EmailNotifier) Send(event Event) error {
 	return nil
 }
 
-func createSMTPClient(host string, port int, username, password string) (*smtp.Client, error) {
-	addr := fmt.Sprintf("%s:%d", host, port)
+func createSMTPClient(ctx context.Context, host string, port int, username, password string) (*smtp.Client, error) {
+	addr := net.JoinHostPort(host, strconv.Itoa(port))
+
+	dialer := &net.Dialer{
+		Timeout: dialerTimeout,
+	}
+
+	conn, err := dialer.DialContext(ctx, "tcp", addr)
+	if err != nil {
+		return nil, fmt.Errorf("dial SMTP server: %w", err)
+	}
 
-	client, err := smtp.Dial(addr)
+	client, err := smtp.NewClient(conn, host)
 	if err != nil {
-		return nil, fmt.Errorf("failed to dial SMTP server: %w", err)
+		return nil, fmt.Errorf("create SMTP client: %w", err)
 	}
 
 	tlsConfig := &tls.Config{
diff --git a/pkg/notifications/email_test.go b/pkg/notifications/email_test.go
@@ -98,11 +98,14 @@ func TestEmailNotifier(t *testing.T) {
 			},
 			validate: func(t *testing.T, n *EmailNotifier) {
 				t.Helper()
+
+				ctx := t.Context()
+
 				// Send multiple emails concurrently
 				errCh := make(chan error, 3)
 				for range 3 {
 					go func() {
-						errCh <- n.Send(Event{
+						errCh <- n.Send(ctx, Event{
 							Type:      EventTypeBan,
 							IP:        "192.0.2.1",
 							Message:   "test ban",
@@ -157,7 +160,8 @@ func TestEmailNotifier(t *testing.T) {
 			}
 
 			// Send notification
-			err := n.Send(test.event)
+			ctx := t.Context()
+			err := n.Send(ctx, test.event)
 
 			// Validate results
 			if test.expectError {
diff --git a/pkg/notifications/service.go b/pkg/notifications/service.go
@@ -2,13 +2,14 @@
 package notifications
 
 import (
+	"context"
 	"log"
 	"net/http"
 	"time"
 )
 
 type notifier interface {
-	Send(event Event) error
+	Send(ctx context.Context, event Event) error
 }
 type Service struct {
 	allowedTypes []string
@@ -30,10 +31,10 @@ func (s *Service) Notify(event Event) {
 	}
 }
 
-func (s *Service) Run() {
+func (s *Service) Run(ctx context.Context) {
 	for event := range s.ch {
 		for _, n := range s.notifiers {
-			if err := n.Send(event); err != nil {
+			if err := n.Send(ctx, event); err != nil {
 				log.Printf("failed to send notification: %v", err)
 			}
 		}
diff --git a/pkg/notifications/service_test.go b/pkg/notifications/service_test.go
@@ -1,6 +1,7 @@
 package notifications
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -174,7 +175,7 @@ func TestServiceNotify(t *testing.T) {
 				notifiers:    []notifier{n},
 				ch:           make(chan Event, 1),
 			}
-			go svc.Run()
+			go svc.Run(t.Context())
 			svc.Notify(test.event)
 
 			if test.shouldNotify {
@@ -202,6 +203,6 @@ type mockNotifier struct {
 	fn func(event Event) error
 }
 
-func (m *mockNotifier) Send(event Event) error {
+func (m *mockNotifier) Send(_ context.Context, event Event) error {
 	return m.fn(event)
 }
diff --git a/pkg/notifications/telegram.go b/pkg/notifications/telegram.go
@@ -2,6 +2,7 @@ package notifications
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -31,8 +32,7 @@ func NewTelegramNotifier(cfg TelegramConfig, templates *TemplateHandler, httpCli
 	return &tn
 }
 
-//nolint:noctx
-func (t *TelegramNotifier) Send(event Event) error {
+func (t *TelegramNotifier) Send(ctx context.Context, event Event) error {
 	msg, err := t.templates.RenderTemplate(event)
 	if err != nil {
 		return fmt.Errorf("failed to render telegram template: %w", err)
@@ -50,7 +50,7 @@ func (t *TelegramNotifier) Send(event Event) error {
 		return fmt.Errorf("failed to marshal telegram payload: %w", err)
 	}
 
-	req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(jsonData))
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(jsonData))
 	if err != nil {
 		return fmt.Errorf("failed to create telegram request: %w", err)
 	}
diff --git a/pkg/notifications/telegram_test.go b/pkg/notifications/telegram_test.go
@@ -43,7 +43,8 @@ func TestTelegramNotifier(t *testing.T) {
 				BaseURL:  server.URL,
 			}, NewTemplateHandler(TemplateConfig{}), server.Client())
 
-			err := n.Send(Event{
+			ctx := t.Context()
+			err := n.Send(ctx, Event{
 				Type:      EventTypeBan,
 				IP:        "192.0.2.1",
 				Message:   "test",
diff --git a/pkg/notifications/templates.go b/pkg/notifications/templates.go
@@ -56,9 +56,11 @@ func makeTmplOrDefault(tp EventType, tmpl string) *template.Template {
 	t, err := template.New(string(tp)).Parse(tmpl)
 	if err != nil {
 		log.Printf("failed to parse %s template: %v, falling back to default template", tp, err)
+
 		t, err = template.New(string(tp)).Parse(defaultTemplatesMapping[tp])
 		if err != nil {
 			log.Printf("failed to parse default template for %s: %v", tp, err)
+
 			return nil
 		}
 	}
diff --git a/pkg/notifications/webhook.go b/pkg/notifications/webhook.go
@@ -1,6 +1,7 @@
 package notifications
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"strings"
@@ -24,14 +25,13 @@ func NewWebhookNotifier(cfg WebhookConfig, templates *TemplateHandler, httpCli *
 	}
 }
 
-//nolint:noctx
-func (w *WebhookNotifier) Send(event Event) error {
+func (w *WebhookNotifier) Send(ctx context.Context, event Event) error {
 	payload, err := w.templates.RenderTemplate(event)
 	if err != nil {
 		return fmt.Errorf("failed to render webhook template: %w", err)
 	}
 
-	req, err := http.NewRequest(w.method, w.url, strings.NewReader(payload))
+	req, err := http.NewRequestWithContext(ctx, w.method, w.url, strings.NewReader(payload))
 	if err != nil {
 		return fmt.Errorf("failed to create webhook request: %w", err)
 	}
diff --git a/pkg/notifications/webhook_test.go b/pkg/notifications/webhook_test.go
@@ -94,7 +94,7 @@ func TestWebhookNotifier(t *testing.T) {
 			test.config.URL = server.URL
 			notifier := NewWebhookNotifier(test.config, NewTemplateHandler(TemplateConfig{}), server.Client())
 
-			err := notifier.Send(Event{
+			err := notifier.Send(t.Context(), Event{
 				Type:      EventTypeBan,
 				IP:        "192.0.2.1",
 				Message:   "test",
diff --git a/pkg/response/status/status_test.go b/pkg/response/status/status_test.go
@@ -113,7 +113,7 @@ func TestStatus(t *testing.T) {
 				MaxRetry: 1,
 				Findtime: 300 * time.Second,
 				Bantime:  300 * time.Second,
-			}, nil)
+			}, nil, nil)
 			f2b.IPs = test.ips
 			d, err := New(next, test.codeRanges, f2b)
 			require.NoError(t, err)
@@ -230,7 +230,7 @@ func TestHeaderCopying(t *testing.T) {
 			rulesTransformed, err := rules.TransformRule(rulesObj)
 			require.NoError(t, err)
 
-			f2b := fail2ban.New(rulesTransformed, nil)
+			f2b := fail2ban.New(rulesTransformed, nil, nil)
 			// Set IP viewed state
 			for ip, viewed := range test.ips {
 				f2b.IPs[ip] = viewed
diff --git a/pkg/url/deny/deny_test.go b/pkg/url/deny/deny_test.go
@@ -49,7 +49,7 @@ func TestDeny(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 
-			f2b := fail2ban.New(rules.RulesTransformed{}, nil)
+			f2b := fail2ban.New(rules.RulesTransformed{}, nil, nil)
 			d := New(test.regs, f2b)
 
 			recorder := &httptest.ResponseRecorder{}

Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,6 @@ func (u *Fail2Ban) ShouldAllow(remoteIP string) bool {`
`77`	`77`	`}`
`78`	`78`
`79`	`79`	`fmt.Println(remoteIP + " is no longer banned")`
`80`		`- u.notify(notifications.UnbanEvent(remoteIP, fmt.Sprintf("%q is no longer banned", remoteIP)))`
`81`	`80`
`82`	`81`	`return true`
`83`	`82`	`}`
`@@ -92,7 +91,6 @@ func (u *Fail2Ban) ShouldAllow(remoteIP string) bool {`
`92`	`91`	`msg := fmt.Sprintf("%q is banned for %d>=%d request",`
`93`	`92`	`remoteIP, ip.Count+1, u.rules.MaxRetry)`
`94`	`93`	`fmt.Printf("%s", msg)`
`95`		`- u.notify(notifications.BanEvent(remoteIP, msg, u.rules.Bantime))`
`96`	`94`
`97`	`95`	`return false`
`98`	`96`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ package notifications`
`2`	`2`
`3`	`3`	`import (`
`4`	`4`	`"bytes"`
	`5`	`+ "context"`
`5`	`6`	`"encoding/json"`
`6`	`7`	`"fmt"`
`7`	`8`	`"net/http"`
`@@ -47,8 +48,7 @@ func NewDiscordNotifier(cfg DiscordConfig, httpCli http.Client) DiscordNotifie`
`47`	`48`	`}`
`48`	`49`	`}`
`49`	`50`
`50`		`-//nolint:noctx`
`51`		`-func (d *DiscordNotifier) Send(event Event) error {`
	`51`	`+func (d *DiscordNotifier) Send(ctx context.Context, event Event) error {`
`52`	`52`	`var color int`
`53`	`53`
`54`	`54`	`switch event.Type {`
`@@ -90,7 +90,7 @@ func (d *DiscordNotifier) Send(event Event) error {`
`90`	`90`	`return fmt.Errorf("failed to marshal discord payload: %w", err)`
`91`	`91`	`}`
`92`	`92`
`93`		`- req, err := http.NewRequest(http.MethodPost, d.webhookURL, bytes.NewReader(jsonPayload))`
	`93`	`+ req, err := http.NewRequestWithContext(ctx, http.MethodPost, d.webhookURL, bytes.NewReader(jsonPayload))`
`94`	`94`	`if err != nil {`
`95`	`95`	`return fmt.Errorf("failed to create discord request: %w", err)`
`96`	`96`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,13 +2,14 @@`
`2`	`2`	`package notifications`
`3`	`3`
`4`	`4`	`import (`
	`5`	`+ "context"`
`5`	`6`	`"log"`
`6`	`7`	`"net/http"`
`7`	`8`	`"time"`
`8`	`9`	`)`
`9`	`10`
`10`	`11`	`type notifier interface {`
`11`		`- Send(event Event) error`
	`12`	`+ Send(ctx context.Context, event Event) error`
`12`	`13`	`}`
`13`	`14`	`type Service struct {`
`14`	`15`	`allowedTypes []string`
`@@ -30,10 +31,10 @@ func (s *Service) Notify(event Event) {`
`30`	`31`	`}`
`31`	`32`	`}`
`32`	`33`
`33`		`-func (s *Service) Run() {`
	`34`	`+func (s *Service) Run(ctx context.Context) {`
`34`	`35`	`for event := range s.ch {`
`35`	`36`	`for _, n := range s.notifiers {`
`36`		`- if err := n.Send(event); err != nil {`
	`37`	`+ if err := n.Send(ctx, event); err != nil {`
`37`	`38`	`log.Printf("failed to send notification: %v", err)`
`38`	`39`	`}`
`39`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`package notifications`
`2`	`2`
`3`	`3`	`import (`
	`4`	`+ "context"`
`4`	`5`	`"testing"`
`5`	`6`	`"time"`
`6`	`7`
`@@ -174,7 +175,7 @@ func TestServiceNotify(t *testing.T) {`
`174`	`175`	`notifiers: []notifier{n},`
`175`	`176`	`ch: make(chan Event, 1),`
`176`	`177`	`}`
`177`		`- go svc.Run()`
	`178`	`+ go svc.Run(t.Context())`
`178`	`179`	`svc.Notify(test.event)`
`179`	`180`
`180`	`181`	`if test.shouldNotify {`
`@@ -202,6 +203,6 @@ type mockNotifier struct {`
`202`	`203`	`fn func(event Event) error`
`203`	`204`	`}`
`204`	`205`
`205`		`-func (m *mockNotifier) Send(event Event) error {`
	`206`	`+func (m *mockNotifier) Send(_ context.Context, event Event) error {`
`206`	`207`	`return m.fn(event)`
`207`	`208`	`}`