lint: update configuration, reserve fmt.Print for debugging (#402)

mmetc · web-flow · commit 57668a6d3e32 · 2025-04-10T11:40:29.000+02:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -3,24 +3,22 @@ version: "2"
 linters:
   default: all
   disable:
-    - cyclop
+    - cyclop    # revive
+    - funlen    # revive
+    - gocognit  # revive
+    - gocyclo   # revive
+    - lll       # revive
+
     - dupl
     - err113
     - exhaustruct
-    - forbidigo
-    - funlen
     - gochecknoglobals
-    - gocognit
     - goconst
-    - gocritic
-    - gocyclo
     - godox
     - gosec
     - ireturn
-    - lll
     - maintidx
     - mnd
-    - nestif
     - nlreturn
     - paralleltest
     - tagliatelle
@@ -38,15 +36,31 @@ linters:
           deny:
             - pkg: github.com/pkg/errors
               desc: errors.Wrap() is deprecated in favor of fmt.Errorf()
+
+    errcheck:
+      check-type-assertions: false
+
+    gocritic:
+      enable-all: true
+      disabled-checks:
+        - appendCombine
+        - commentFormatting
+        - paramTypeCombine
+        - sloppyReassign
+        - unnamedResult
+
     govet:
       disable:
         - fieldalignment
-        - shadow
       enable-all: true
 
     misspell:
       locale: US
 
+    nestif:
+      # lower this after refactoring
+      min-complexity: 13
+
     nlreturn:
       block-size: 4
 
@@ -68,15 +82,15 @@ linters:
         - name: confusing-results
           disabled: true
         - name: cyclomatic
-          disabled: true
+          arguments:
+            # lower this after refactoring
+            - 28
         - name: empty-lines
           disabled: true
         - name: flag-parameter
           disabled: true
         - name: function-length
           disabled: true
-        - name: if-return
-          disabled: true
         - name: import-alias-naming
           disabled: true
         - name: import-shadowing
@@ -85,34 +99,20 @@ linters:
           disabled: true
         - name: nested-structs
           disabled: true
-        - name: redundant-build-tag
-          disabled: true
-        - name: var-declaration
-          disabled: true
         - name: exported
           disabled: true
-        - name: unexported-naming
-          disabled: true
         - name: unexported-return
           disabled: true
         - name: unhandled-error
           arguments:
             - fmt.Print
             - fmt.Printf
             - fmt.Println
-          disabled: true
-        - name: unused-receiver
-          disabled: true
         - name: function-result-limit
           arguments:
             - 5
     staticcheck:
       checks:
-        - -ST1003
-        - -ST1005
-        - -ST1012
-        - -ST1016
-        - -ST1022
         - all
     wsl:
       allow-trailing-comment: true
@@ -124,8 +124,9 @@ linters:
       - std-error-handling
     rules:
       - linters:
-          - errcheck
-        text: Error return value of `.*` is not checked
+          - govet
+        text: 'shadow: declaration of "(err|ctx)" shadows declaration'
+
       - linters:
           - perfsprint
         text: fmt.Sprintf can be replaced .*
diff --git a/cmd/root.go b/cmd/root.go
@@ -147,7 +147,7 @@ func Execute() error {
 	flag.Parse()
 
 	if *bouncerVersion {
-		fmt.Print(version.FullString())
+		fmt.Fprint(os.Stdout, version.FullString())
 		return nil
 	}
 
@@ -161,7 +161,7 @@ func Execute() error {
 	}
 
 	if *showConfig {
-		fmt.Println(string(configMerged))
+		fmt.Fprintln(os.Stdout, string(configMerged))
 		return nil
 	}
 
diff --git a/pkg/dryrun/dryrun.go b/pkg/dryrun/dryrun.go
@@ -16,31 +16,31 @@ func NewDryRun(_ *cfg.BouncerConfig) (types.Backend, error) {
 	return &dryRun{}, nil
 }
 
-func (d *dryRun) Init() error {
+func (*dryRun) Init() error {
 	log.Infof("backend.Init() called")
 	return nil
 }
 
-func (d *dryRun) Commit() error {
+func (*dryRun) Commit() error {
 	log.Infof("backend.Commit() called")
 	return nil
 }
 
-func (d *dryRun) Add(decision *models.Decision) error {
+func (*dryRun) Add(decision *models.Decision) error {
 	log.Infof("backend.Add() called with %s", *decision.Value)
 	return nil
 }
 
-func (d *dryRun) CollectMetrics() {
+func (*dryRun) CollectMetrics() {
 	log.Infof("backend.CollectMetrics() called")
 }
 
-func (d *dryRun) Delete(decision *models.Decision) error {
+func (*dryRun) Delete(decision *models.Decision) error {
 	log.Infof("backend.Delete() called with %s", *decision.Value)
 	return nil
 }
 
-func (d *dryRun) ShutDown() error {
+func (*dryRun) ShutDown() error {
 	log.Infof("backend.ShutDown() called")
 	return nil
 }
diff --git a/pkg/ipsetcmd/ipset.go b/pkg/ipsetcmd/ipset.go
@@ -213,17 +213,21 @@ func (i *IPSet) Len() int {
 	}
 
 	for _, line := range strings.Split(string(out), "\n") {
-		if strings.Contains(strings.ToLower(line), "number of entries:") {
-			fields := strings.Split(line, ":")
-			if len(fields) != 2 {
-				continue
-			}
-			count, err := strconv.Atoi(strings.TrimSpace(fields[1]))
-			if err != nil {
-				return 0
-			}
-			return count
+		if !strings.Contains(strings.ToLower(line), "number of entries:") {
+			continue
 		}
+
+		fields := strings.Split(line, ":")
+		if len(fields) != 2 {
+			continue
+		}
+
+		count, err := strconv.Atoi(strings.TrimSpace(fields[1]))
+		if err != nil {
+			return 0
+		}
+
+		return count
 	}
 
 	return 0
@@ -242,17 +246,21 @@ func GetSetsStartingWith(name string) (map[string]*IPSet, error) {
 	sets := make(map[string]*IPSet, 0)
 
 	for _, line := range strings.Split(string(out), "\n") {
-		if strings.HasPrefix(line, name) {
-			fields := strings.Fields(line)
-			if len(fields) != 1 {
-				continue
-			}
-			set, err := NewIPSet(fields[0])
-			if err != nil {
-				return nil, err
-			}
-			sets[fields[0]] = set
+		if !strings.HasPrefix(line, name) {
+			continue
 		}
+
+		fields := strings.Fields(line)
+		if len(fields) != 1 {
+			continue
+		}
+
+		set, err := NewIPSet(fields[0])
+		if err != nil {
+			return nil, err
+		}
+
+		sets[fields[0]] = set
 	}
 
 	return sets, nil
diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package iptables
 
diff --git a/pkg/iptables/iptables_context.go b/pkg/iptables/iptables_context.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package iptables
 
diff --git a/pkg/iptables/metrics.go b/pkg/iptables/metrics.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package iptables
 
diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go
@@ -50,7 +50,7 @@ func (m metricMap) MustRegisterAll() {
 	}
 }
 
-var Map metricMap = metricMap{
+var Map = metricMap{
 	ActiveBannedIPs: {
 		Name: "active_decisions",
 		Unit: "ip",
diff --git a/pkg/nftables/metrics.go b/pkg/nftables/metrics.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package nftables
 
diff --git a/pkg/nftables/nftables.go b/pkg/nftables/nftables.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package nftables
 
@@ -236,11 +235,7 @@ func (n *nft) commitAddedDecisions() error {
 		return err
 	}
 
-	if err := n.v6.addElements(ip6); err != nil {
-		return err
-	}
-
-	return nil
+	return n.v6.addElements(ip6)
 }
 
 func (n *nft) Commit() error {
@@ -250,11 +245,7 @@ func (n *nft) Commit() error {
 		return err
 	}
 
-	if err := n.commitAddedDecisions(); err != nil {
-		return err
-	}
-
-	return nil
+	return n.commitAddedDecisions()
 }
 
 type tmpDecisions struct {
@@ -311,9 +302,5 @@ func (n *nft) ShutDown() error {
 		return err
 	}
 
-	if err := n.v6.shutDown(); err != nil {
-		return err
-	}
-
-	return nil
+	return n.v6.shutDown()
 }
diff --git a/pkg/nftables/nftables_context.go b/pkg/nftables/nftables_context.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package nftables
 
@@ -397,9 +396,5 @@ func (c *nftContext) shutDown() error {
 		c.conn.DelTable(c.table)
 	}
 
-	if err := c.conn.Flush(); err != nil {
-		return err
-	}
-
-	return nil
+	return c.conn.Flush()
 }
diff --git a/pkg/pf/metrics.go b/pkg/pf/metrics.go
@@ -80,7 +80,7 @@ func parseMetrics(reader *strings.Reader, tables []string) map[string]counter {
 }
 
 // countIPs returns the number of IPs in a table.
-func (pf *pf) countIPs(table string) int {
+func countIPs(table string) int {
 	cmd := execPfctl("", "-T", "show", "-t", table)
 
 	out, err := cmd.Output()
@@ -126,7 +126,7 @@ func (pf *pf) CollectMetrics() {
 
 		droppedPackets := float64(st.packets)
 		droppedBytes := float64(st.bytes)
-		bannedIPs := pf.countIPs(table)
+		bannedIPs := countIPs(table)
 
 		if pf.inet != nil && table == pf.inet.table {
 			metrics.Map[metrics.DroppedPackets].Gauge.With(prometheus.Labels{"ip_type": "ipv4", "origin": ""}).Set(droppedPackets)
diff --git a/pkg/pf/pf.go b/pkg/pf/pf.go
@@ -100,11 +100,7 @@ func (pf *pf) Commit() error {
 		return err
 	}
 
-	if err := pf.commitAddedDecisions(); err != nil {
-		return err
-	}
-
-	return nil
+	return pf.commitAddedDecisions()
 }
 
 func (pf *pf) Add(decision *models.Decision) error {

Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,7 @@ func Execute() error {`
`147`	`147`	`flag.Parse()`
`148`	`148`
`149`	`149`	`if *bouncerVersion {`
`150`		`- fmt.Print(version.FullString())`
	`150`	`+ fmt.Fprint(os.Stdout, version.FullString())`
`151`	`151`	`return nil`
`152`	`152`	`}`
`153`	`153`
`@@ -161,7 +161,7 @@ func Execute() error {`
`161`	`161`	`}`
`162`	`162`
`163`	`163`	`if *showConfig {`
`164`		`- fmt.Println(string(configMerged))`
	`164`	`+ fmt.Fprintln(os.Stdout, string(configMerged))`
`165`	`165`	`return nil`
`166`	`166`	`}`
`167`	`167`
Original file line number	Diff line number	Diff line change
`@@ -16,31 +16,31 @@ func NewDryRun(_ *cfg.BouncerConfig) (types.Backend, error) {`
`16`	`16`	`return &dryRun{}, nil`
`17`	`17`	`}`
`18`	`18`
`19`		`-func (d *dryRun) Init() error {`
	`19`	`+func (*dryRun) Init() error {`
`20`	`20`	`log.Infof("backend.Init() called")`
`21`	`21`	`return nil`
`22`	`22`	`}`
`23`	`23`
`24`		`-func (d *dryRun) Commit() error {`
	`24`	`+func (*dryRun) Commit() error {`
`25`	`25`	`log.Infof("backend.Commit() called")`
`26`	`26`	`return nil`
`27`	`27`	`}`
`28`	`28`
`29`		`-func (d dryRun) Add(decision models.Decision) error {`
	`29`	`+func (dryRun) Add(decision models.Decision) error {`
`30`	`30`	`log.Infof("backend.Add() called with %s", *decision.Value)`
`31`	`31`	`return nil`
`32`	`32`	`}`
`33`	`33`
`34`		`-func (d *dryRun) CollectMetrics() {`
	`34`	`+func (*dryRun) CollectMetrics() {`
`35`	`35`	`log.Infof("backend.CollectMetrics() called")`
`36`	`36`	`}`
`37`	`37`
`38`		`-func (d dryRun) Delete(decision models.Decision) error {`
	`38`	`+func (dryRun) Delete(decision models.Decision) error {`
`39`	`39`	`log.Infof("backend.Delete() called with %s", *decision.Value)`
`40`	`40`	`return nil`
`41`	`41`	`}`
`42`	`42`
`43`		`-func (d *dryRun) ShutDown() error {`
	`43`	`+func (*dryRun) ShutDown() error {`
`44`	`44`	`log.Infof("backend.ShutDown() called")`
`45`	`45`	`return nil`
`46`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`//go:build linux`
`2`		`-// +build linux`
`3`	`2`
`4`	`3`	`package iptables`
`5`	`4`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ func (m metricMap) MustRegisterAll() {`
`50`	`50`	`}`
`51`	`51`	`}`
`52`	`52`
`53`		`-var Map metricMap = metricMap{`
	`53`	`+var Map = metricMap{`
`54`	`54`	`ActiveBannedIPs: {`
`55`	`55`	`Name: "active_decisions",`
`56`	`56`	`Unit: "ip",`