refact: don't call defer in loops, enable linter (#3907)

Author: mmetc

.golangci.yml

@@ -157,7 +157,6 @@ linters:
         - sloppyReassign
         - appendCombine
         - commentFormatting
-        - deferInLoop             #
         - whyNoLint
         - equalFold               #
         - unnecessaryBlock        #
@@ -220,8 +219,6 @@ linters:
           arguments:
             # lower this after refactoring
             - 39
-        - name: defer
-          disabled: true
         - name: empty-block
           disabled: true
         - name: empty-lines

pkg/apiserver/apic.go

@@ -742,102 +742,102 @@ func (a *apic) PullAllowlist(ctx context.Context, allowlist *modelscapi.Allowlis
 	return nil
 }
 
-func (a *apic) UpdateAllowlists(ctx context.Context, allowlistsLinks []*modelscapi.AllowlistLink, forcePull bool) error {
-	if len(allowlistsLinks) == 0 {
+func (a *apic) updateOneAllowlist(ctx context.Context, client *apiclient.ApiClient, link *modelscapi.AllowlistLink) error {
+	if log.IsLevelEnabled(log.TraceLevel) {
+		log.Tracef("allowlist body: %+v", spew.Sdump(link))
+	}
+
+	if link.Name == nil {
+		log.Warn("allowlist has no name")
 		return nil
 	}
 
-	defaultClient, err := apiclient.NewDefaultClient(a.apiClient.BaseURL, "", "", nil)
+	if link.URL == nil {
+		log.Warnf("allowlist %s has no URL", *link.Name)
+		return nil
+	}
+
+	if link.ID == nil {
+		return fmt.Errorf("allowlist %s has no ID", *link.Name)
+	}
+
+	description := ""
+	if link.Description != nil {
+		description = *link.Description
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, *link.URL, http.NoBody)
 	if err != nil {
-		return fmt.Errorf("while creating default client: %w", err)
+		return fmt.Errorf("while pulling allowlist: %s", err)
 	}
 
-	for _, link := range allowlistsLinks {
-		if log.IsLevelEnabled(log.TraceLevel) {
-			log.Tracef("allowlist body: %+v", spew.Sdump(link))
-		}
+	resp, err := client.GetClient().Do(req)
+	if err != nil {
+		return fmt.Errorf("while pulling allowlist: %s", err)
+	}
+	defer resp.Body.Close()
 
-		if link.Name == nil {
-			log.Warningf("allowlist has no name")
-			continue
-		}
+	scanner := bufio.NewScanner(resp.Body)
+	items := make([]*models.AllowlistItem, 0)
 
-		if link.URL == nil {
-			log.Warningf("allowlist %s has no URL", *link.Name)
-			continue
-		}
+	for scanner.Scan() {
+		item := scanner.Text()
+		j := &models.AllowlistItem{}
 
-		if link.ID == nil {
-			log.Warningf("allowlist %s has no ID", *link.Name)
-			continue
+		if err := json.Unmarshal([]byte(item), j); err != nil {
+			return fmt.Errorf("while unmarshalling allowlist item: %s", err)
 		}
 
-		description := ""
-		if link.Description != nil {
-			description = *link.Description
-		}
+		items = append(items, j)
+	}
 
-		req, err := http.NewRequestWithContext(ctx, http.MethodGet, *link.URL, http.NoBody)
-		if err != nil {
-			log.Errorf("while pulling allowlist: %s", err)
-			continue
+	list, err := a.dbClient.GetAllowListByID(ctx, *link.ID, false)
+	if err != nil {
+		if !ent.IsNotFound(err) {
+			return fmt.Errorf("while getting allowlist %s: %s", *link.Name, err)
 		}
+	}
 
-		resp, err := defaultClient.GetClient().Do(req)
+	if list == nil {
+		list, err = a.dbClient.CreateAllowList(ctx, *link.Name, description, *link.ID, true)
 		if err != nil {
-			log.Errorf("while pulling allowlist: %s", err)
-			continue
+			return fmt.Errorf("while creating allowlist %s: %s", *link.Name, err)
 		}
-		defer resp.Body.Close()
-
-		scanner := bufio.NewScanner(resp.Body)
-		items := make([]*models.AllowlistItem, 0)
-
-		for scanner.Scan() {
-			item := scanner.Text()
-			j := &models.AllowlistItem{}
+	}
 
-			if err := json.Unmarshal([]byte(item), j); err != nil {
-				log.Errorf("while unmarshalling allowlist item: %s", err)
-				continue
-			}
+	added, err := a.dbClient.ReplaceAllowlist(ctx, list, items, true)
+	if err != nil {
+		return fmt.Errorf("while replacing allowlist %s: %s", *link.Name, err)
+	}
 
-			items = append(items, j)
-		}
+	log.Infof("added %d values to allowlist %s", added, list.Name)
 
-		list, err := a.dbClient.GetAllowListByID(ctx, *link.ID, false)
+	if list.Name != *link.Name || list.Description != description {
+		err = a.dbClient.UpdateAllowlistMeta(ctx, *link.ID, *link.Name, description)
 		if err != nil {
-			if !ent.IsNotFound(err) {
-				log.Errorf("while getting allowlist %s: %s", *link.Name, err)
-				continue
-			}
+			return fmt.Errorf("while updating allowlist meta %s: %s", *link.Name, err)
 		}
+	}
 
-		if list == nil {
-			list, err = a.dbClient.CreateAllowList(ctx, *link.Name, description, *link.ID, true)
-			if err != nil {
-				log.Errorf("while creating allowlist %s: %s", *link.Name, err)
-				continue
-			}
-		}
+	log.Infof("Allowlist %s updated", *link.Name)
 
-		added, err := a.dbClient.ReplaceAllowlist(ctx, list, items, true)
-		if err != nil {
-			log.Errorf("while replacing allowlist %s: %s", *link.Name, err)
-			continue
-		}
+	return nil
+}
 
-		log.Infof("added %d values to allowlist %s", added, list.Name)
+func (a *apic) UpdateAllowlists(ctx context.Context, allowlistsLinks []*modelscapi.AllowlistLink, forcePull bool) error {
+	if len(allowlistsLinks) == 0 {
+		return nil
+	}
 
-		if list.Name != *link.Name || list.Description != description {
-			err = a.dbClient.UpdateAllowlistMeta(ctx, *link.ID, *link.Name, description)
-			if err != nil {
-				log.Errorf("while updating allowlist meta %s: %s", *link.Name, err)
-				continue
-			}
-		}
+	client, err := apiclient.NewDefaultClient(a.apiClient.BaseURL, "", "", nil)
+	if err != nil {
+		return fmt.Errorf("while creating default client: %w", err)
+	}
 
-		log.Infof("Allowlist %s updated", *link.Name)
+	for _, link := range allowlistsLinks {
+		if err := a.updateOneAllowlist(ctx, client, link); err != nil {
+			log.Errorf("updating allowlists from CAPI: %s", err)
+		}
 	}
 
 	return nil

pkg/apiserver/apiserver.go

@@ -84,7 +84,7 @@ func isBrokenConnection(maybeError any) bool {
 }
 
 func recoverFromPanic(c *gin.Context) {
-	err := recover()
+	err := recover() //nolint:revive
 	if err == nil {
 		return
 	}

pkg/leakybucket/bucket.go

@@ -163,6 +163,12 @@ func LeakRoutine(leaky *Leaky) error {
 		firstEvent         = true
 	)
 
+	defer func() {
+		if durationTicker != nil {
+			durationTicker.Stop()
+		}
+	}()
+
 	defer trace.CatchPanic(fmt.Sprintf("crowdsec/LeakRoutine/%s", leaky.Name))
 
 	metrics.BucketsCurrentCount.With(prometheus.Labels{"name": leaky.Name}).Inc()
@@ -232,7 +238,6 @@ func LeakRoutine(leaky *Leaky) error {
 				if firstEvent {
 					durationTicker = time.NewTicker(leaky.Duration)
 					durationTickerChan = durationTicker.C
-					defer durationTicker.Stop()
 				} else {
 					durationTicker.Reset(leaky.Duration)
 				}