refact pkg/apiserver: happy path; nil guard (#4180)

* recoverFromPanic(): drop else

* isBrokenConnection: dry

* shouldAutoRegister(): nil guard

* Authenticator(): replace loop with strings.Join()

* prometheus middleware: happy path

* promehteus middleware: happy path, redundant c.Next()

Author: mmetc

pkg/apiserver/apiserver.go

@@ -54,7 +54,8 @@ func isBrokenConnection(maybeError any) bool {
 	if errors.As(err, &netOpError) {
 		var syscallError *os.SyscallError
 		if errors.As(netOpError.Err, &syscallError) {
-			if strings.Contains(strings.ToLower(syscallError.Error()), "broken pipe") || strings.Contains(strings.ToLower(syscallError.Error()), "connection reset by peer") {
+			s := strings.ToLower(syscallError.Error())
+			if strings.Contains(s, "broken pipe") || strings.Contains(s, "connection reset by peer") {
 				return true
 			}
 		}
@@ -92,17 +93,18 @@ func recoverFromPanic(c *gin.Context) {
 	if isBrokenConnection(err) {
 		log.Warningf("client %s disconnected: %s", c.ClientIP(), err)
 		c.Abort()
-	} else {
-		log.Warningf("client %s error: %s", c.ClientIP(), err)
+		return
+	}
 
-		filename, err := trace.WriteStackTrace(err)
-		if err != nil {
-			log.Errorf("also while writing stacktrace: %s", err)
-		}
+	log.Warningf("client %s error: %s", c.ClientIP(), err)
 
-		log.Warningf("stacktrace written to %s, please join to your issue", filename)
-		c.AbortWithStatus(http.StatusInternalServerError)
+	filename, err := trace.WriteStackTrace(err)
+	if err != nil {
+		log.Errorf("also while writing stacktrace: %s", err)
 	}
+
+	log.Warningf("stacktrace written to %s, please join to your issue", filename)
+	c.AbortWithStatus(http.StatusInternalServerError)
 }
 
 // CustomRecoveryWithWriter returns a middleware for a writer that recovers from any panics and writes a 500 if there was one.

pkg/apiserver/controllers/v1/machines.go

@@ -14,7 +14,7 @@ import (
 )
 
 func (c *Controller) shouldAutoRegister(token string, gctx *gin.Context) (bool, error) {
-	if !*c.AutoRegisterCfg.Enable {
+	if c.AutoRegisterCfg == nil || c.AutoRegisterCfg.Enable == nil || !*c.AutoRegisterCfg.Enable {
 		return false, nil
 	}
 
@@ -26,7 +26,7 @@ func (c *Controller) shouldAutoRegister(token string, gctx *gin.Context) (bool,
 		return false, nil
 	}
 
-	if token == "" || c.AutoRegisterCfg == nil {
+	if token == "" {
 		return false, nil
 	}
 

pkg/apiserver/controllers/v1/metrics.go

@@ -1,6 +1,7 @@
 package v1
 
 import (
+	"cmp"
 	"time"
 
 	"github.com/crowdsecurity/crowdsec/pkg/metrics"
@@ -10,76 +11,71 @@ import (
 
 func PrometheusBouncersHasEmptyDecision(c *gin.Context) {
 	bouncer, _ := getBouncerFromContext(c)
-	if bouncer != nil {
-		metrics.LapiNilDecisions.With(prometheus.Labels{
-			"bouncer": bouncer.Name,
-		}).Inc()
+	if bouncer == nil {
+		return
 	}
+
+	metrics.LapiNilDecisions.With(prometheus.Labels{
+		"bouncer": bouncer.Name,
+	}).Inc()
 }
 
 func PrometheusBouncersHasNonEmptyDecision(c *gin.Context) {
 	bouncer, _ := getBouncerFromContext(c)
-	if bouncer != nil {
-		metrics.LapiNonNilDecisions.With(prometheus.Labels{
-			"bouncer": bouncer.Name,
-		}).Inc()
+	if bouncer == nil {
+		return
 	}
+
+	metrics.LapiNonNilDecisions.With(prometheus.Labels{
+		"bouncer": bouncer.Name,
+	}).Inc()
 }
 
 func PrometheusMachinesMiddleware() gin.HandlerFunc {
 	return func(c *gin.Context) {
 		machineID, _ := getMachineIDFromContext(c)
-		if machineID != "" {
-			fullPath := c.FullPath()
-			if fullPath == "" {
-				fullPath = "invalid-endpoint"
-			}
-			metrics.LapiMachineHits.With(prometheus.Labels{
-				"machine": machineID,
-				"route":   fullPath,
-				"method":  c.Request.Method,
-			}).Inc()
+		if machineID == "" {
+			return
 		}
 
-		c.Next()
+		metrics.LapiMachineHits.With(prometheus.Labels{
+			"machine": machineID,
+			"route":   cmp.Or(c.FullPath(), "invalid-endpoint"),
+			"method":  c.Request.Method,
+		}).Inc()
 	}
 }
 
 func PrometheusBouncersMiddleware() gin.HandlerFunc {
 	return func(c *gin.Context) {
 		bouncer, _ := getBouncerFromContext(c)
-		if bouncer != nil {
-			fullPath := c.FullPath()
-			if fullPath == "" {
-				fullPath = "invalid-endpoint"
-			}
-			metrics.LapiBouncerHits.With(prometheus.Labels{
-				"bouncer": bouncer.Name,
-				"route":   fullPath,
-				"method":  c.Request.Method,
-			}).Inc()
+		if bouncer == nil {
+			return
 		}
 
-		c.Next()
+		metrics.LapiBouncerHits.With(prometheus.Labels{
+			"bouncer": bouncer.Name,
+			"route":   cmp.Or(c.FullPath(), "invalid-endpoint"),
+			"method":  c.Request.Method,
+		}).Inc()
 	}
 }
 
 func PrometheusMiddleware() gin.HandlerFunc {
 	return func(c *gin.Context) {
 		startTime := time.Now()
 
-		fullPath := c.FullPath()
-		if fullPath == "" {
-			fullPath = "invalid-endpoint"
-		}
-
 		metrics.LapiRouteHits.With(prometheus.Labels{
-			"route":  fullPath,
+			"route":   cmp.Or(c.FullPath(), "invalid-endpoint"),
 			"method": c.Request.Method,
 		}).Inc()
 		c.Next()
 
 		elapsed := time.Since(startTime)
-		metrics.LapiResponseTime.With(prometheus.Labels{"method": c.Request.Method, "endpoint": c.FullPath()}).Observe(elapsed.Seconds())
+		metrics.LapiResponseTime.With(
+			prometheus.Labels{
+				"method": c.Request.Method,
+				"endpoint": c.FullPath(),
+			}).Observe(elapsed.Seconds())
 	}
 }

pkg/apiserver/middlewares/v1/jwt.go

@@ -192,19 +192,10 @@ func (j *JWT) Authenticator(c *gin.Context) (any, error) {
 		}
 	}
 
-	var scenarios string
-
-	if len(auth.scenariosInput) > 0 {
-		for _, scenario := range auth.scenariosInput {
-			if scenarios == "" {
-				scenarios = scenario
-			} else {
-				scenarios += "," + scenario
-			}
-		}
+	if len(auth.scenariosInput) != 0 {
+		scenarios := strings.Join(auth.scenariosInput, ",")
 
-		err = j.DbClient.UpdateMachineScenarios(ctx, scenarios, auth.clientMachine.ID)
-		if err != nil {
+		if err = j.DbClient.UpdateMachineScenarios(ctx, scenarios, auth.clientMachine.ID); err != nil {
 			log.Errorf("Failed to update scenarios list for '%s': %s\n", auth.machineID, err)
 			return nil, jwt.ErrFailedAuthentication
 		}