enable linter: revive (early-return) (#3051)

* enable linter: revive (early-return)

* lint

.golangci.yml

@@ -102,7 +102,6 @@ linters-settings:
           - "!**/pkg/csplugin/broker.go"
           - "!**/pkg/leakybucket/buckets_test.go"
           - "!**/pkg/leakybucket/manager_load.go"
-          - "!**/pkg/metabase/metabase.go"
           - "!**/pkg/parser/node.go"
           - "!**/pkg/parser/node_test.go"
           - "!**/pkg/parser/parsing_test.go"
@@ -139,8 +138,6 @@ linters-settings:
         disabled: true
       - name: defer
         disabled: true
-      - name: early-return
-        disabled: true
       - name: empty-block
         disabled: true
       - name: empty-lines
@@ -382,6 +379,7 @@ issues:
 
   exclude-dirs:
     - pkg/time/rate
+    - pkg/metabase
 
   exclude-files:
     - pkg/yamlpatch/merge.go

pkg/acquisition/modules/s3/s3.go

@@ -131,7 +131,6 @@ func (s *S3Source) newS3Client() error {
 	}
 
 	sess, err := session.NewSessionWithOptions(options)
-
 	if err != nil {
 		return fmt.Errorf("failed to create aws session: %w", err)
 	}
@@ -146,7 +145,7 @@ func (s *S3Source) newS3Client() error {
 
 	s.s3Client = s3.New(sess, config)
 	if s.s3Client == nil {
-		return fmt.Errorf("failed to create S3 client")
+		return errors.New("failed to create S3 client")
 	}
 
 	return nil
@@ -167,7 +166,7 @@ func (s *S3Source) newSQSClient() error {
 	}
 
 	if sess == nil {
-		return fmt.Errorf("failed to create aws session")
+		return errors.New("failed to create aws session")
 	}
 	config := aws.NewConfig()
 	if s.Config.AwsRegion != "" {
@@ -178,7 +177,7 @@ func (s *S3Source) newSQSClient() error {
 	}
 	s.sqsClient = sqs.New(sess, config)
 	if s.sqsClient == nil {
-		return fmt.Errorf("failed to create SQS client")
+		return errors.New("failed to create SQS client")
 	}
 	return nil
 }
@@ -251,16 +250,15 @@ func (s *S3Source) listPoll() error {
 				continue
 			}
 			for i := len(bucketObjects) - 1; i >= 0; i-- {
-				if bucketObjects[i].LastModified.After(lastObjectDate) {
-					newObject = true
-					logger.Debugf("Found new object %s", *bucketObjects[i].Key)
-					s.readerChan <- S3Object{
-						Bucket: s.Config.BucketName,
-						Key:    *bucketObjects[i].Key,
-					}
-				} else {
+				if !bucketObjects[i].LastModified.After(lastObjectDate) {
 					break
 				}
+				newObject = true
+				logger.Debugf("Found new object %s", *bucketObjects[i].Key)
+				s.readerChan <- S3Object{
+					Bucket: s.Config.BucketName,
+					Key:    *bucketObjects[i].Key,
+				}
 			}
 			if newObject {
 				lastObjectDate = *bucketObjects[len(bucketObjects)-1].LastModified

pkg/apiserver/papi.go

@@ -141,17 +141,18 @@ func (p *Papi) handleEvent(event longpollclient.Event, sync bool) error {
 		return errors.New("no source user in header message, skipping")
 	}
 
-	if operationFunc, ok := operationMap[message.Header.OperationType]; ok {
-		logger.Debugf("Calling operation '%s'", message.Header.OperationType)
-
-		err := operationFunc(message, p, sync)
-		if err != nil {
-			return fmt.Errorf("'%s %s failed: %w", message.Header.OperationType, message.Header.OperationCmd, err)
-		}
-	} else {
+	operationFunc, ok := operationMap[message.Header.OperationType]
+	if !ok {
 		return fmt.Errorf("operation '%s' unknown, continue", message.Header.OperationType)
 	}
 
+	logger.Debugf("Calling operation '%s'", message.Header.OperationType)
+
+	err := operationFunc(message, p, sync)
+	if err != nil {
+		return fmt.Errorf("'%s %s failed: %w", message.Header.OperationType, message.Header.OperationCmd, err)
+	}
+
 	return nil
 }
 

pkg/appsec/appsec_rule/modsecurity.go

@@ -1,6 +1,7 @@
 package appsec_rule
 
 import (
+	"errors"
 	"fmt"
 	"hash/fnv"
 	"strings"
@@ -67,9 +68,7 @@ var bodyTypeMatch map[string]string = map[string]string{
 }
 
 func (m *ModsecurityRule) Build(rule *CustomRule, appsecRuleName string) (string, []uint32, error) {
-
 	rules, err := m.buildRules(rule, appsecRuleName, false, 0, 0)
-
 	if err != nil {
 		return "", nil, err
 	}
@@ -99,7 +98,7 @@ func (m *ModsecurityRule) buildRules(rule *CustomRule, appsecRuleName string, an
 	ret := make([]string, 0)
 
 	if len(rule.And) != 0 && len(rule.Or) != 0 {
-		return nil, fmt.Errorf("cannot have both 'and' and 'or' in the same rule")
+		return nil, errors.New("cannot have both 'and' and 'or' in the same rule")
 	}
 
 	if rule.And != nil {
@@ -166,15 +165,15 @@ func (m *ModsecurityRule) buildRules(rule *CustomRule, appsecRuleName string, an
 	r.WriteByte(' ')
 
 	if rule.Match.Type != "" {
-		if match, ok := matchMap[rule.Match.Type]; ok {
-			prefix := ""
-			if rule.Match.Not {
-				prefix = "!"
-			}
-			r.WriteString(fmt.Sprintf(`"%s%s %s"`, prefix, match, rule.Match.Value))
-		} else {
+		match, ok := matchMap[rule.Match.Type]
+		if !ok {
 			return nil, fmt.Errorf("unknown match type '%s'", rule.Match.Type)
 		}
+		prefix := ""
+		if rule.Match.Not {
+			prefix = "!"
+		}
+		r.WriteString(fmt.Sprintf(`"%s%s %s"`, prefix, match, rule.Match.Value))
 	}
 
 	//Should phase:2 be configurable?
@@ -186,20 +185,20 @@ func (m *ModsecurityRule) buildRules(rule *CustomRule, appsecRuleName string, an
 				continue
 			}
 			r.WriteByte(',')
-			if mappedTransform, ok := transformMap[transform]; ok {
-				r.WriteString(mappedTransform)
-			} else {
+			mappedTransform, ok := transformMap[transform]
+			if !ok {
 				return nil, fmt.Errorf("unknown transform '%s'", transform)
 			}
+			r.WriteString(mappedTransform)
 		}
 	}
 
 	if rule.BodyType != "" {
-		if mappedBodyType, ok := bodyTypeMatch[rule.BodyType]; ok {
-			r.WriteString(fmt.Sprintf(",ctl:requestBodyProcessor=%s", mappedBodyType))
-		} else {
+		mappedBodyType, ok := bodyTypeMatch[rule.BodyType]
+		if !ok {
 			return nil, fmt.Errorf("unknown body type '%s'", rule.BodyType)
 		}
+		r.WriteString(fmt.Sprintf(",ctl:requestBodyProcessor=%s", mappedBodyType))
 	}
 
 	if and {

pkg/appsec/coraza_logger.go

@@ -90,14 +90,13 @@ func (e *crzLogEvent) Bool(key string, b bool) dbg.Event {
 
 func (e *crzLogEvent) Int(key string, i int) dbg.Event {
 	if e.muted {
-		// this allows us to have per-rule debug logging
-		if key == "rule_id" && GetRuleDebug(i) {
-			e.muted = false
-			e.fields = map[string]interface{}{}
-			e.level = log.DebugLevel
-		} else {
+		if key != "rule_id" || !GetRuleDebug(i) {
 			return e
 		}
+		// this allows us to have per-rule debug logging
+		e.muted = false
+		e.fields = map[string]interface{}{}
+		e.level = log.DebugLevel
 	}
 
 	e.fields[key] = i

pkg/csplugin/hclog_adapter.go

@@ -221,11 +221,10 @@ func merge(dst map[string]interface{}, k, v interface{}) {
 func safeString(str fmt.Stringer) (s string) {
 	defer func() {
 		if panicVal := recover(); panicVal != nil {
-			if v := reflect.ValueOf(str); v.Kind() == reflect.Ptr && v.IsNil() {
-				s = "NULL"
-			} else {
+			if v := reflect.ValueOf(str); v.Kind() != reflect.Ptr || !v.IsNil() {
 				panic(panicVal)
 			}
+			s = "NULL"
 		}
 	}()
 

pkg/exprhelpers/crowdsec_cti.go

@@ -86,12 +86,11 @@ func CrowdsecCTI(params ...any) (any, error) {
 	if val, err := CTICache.Get(ip); err == nil && val != nil {
 		ctiClient.Logger.Debugf("cti cache fetch for %s", ip)
 		ret, ok := val.(*cticlient.SmokeItem)
-		if !ok {
-			ctiClient.Logger.Warningf("CrowdsecCTI: invalid type in cache, removing")
-			CTICache.Remove(ip)
-		} else {
+		if ok {
 			return ret, nil
 		}
+		ctiClient.Logger.Warningf("CrowdsecCTI: invalid type in cache, removing")
+		CTICache.Remove(ip)
 	}
 
 	if !CTIBackOffUntil.IsZero() && time.Now().Before(CTIBackOffUntil) {

pkg/parser/parsing_test.go

@@ -278,26 +278,25 @@ func matchEvent(expected types.Event, out types.Event, debug bool) ([]string, bo
 
 	for mapIdx := 0; mapIdx < len(expectMaps); mapIdx++ {
 		for expKey, expVal := range expectMaps[mapIdx] {
-			if outVal, ok := outMaps[mapIdx][expKey]; ok {
-				if outVal == expVal { //ok entry
-					if debug {
-						retInfo = append(retInfo, fmt.Sprintf("ok %s[%s] %s == %s", outLabels[mapIdx], expKey, expVal, outVal))
-					}
-					valid = true
-				} else { //mismatch entry
-					if debug {
-						retInfo = append(retInfo, fmt.Sprintf("mismatch %s[%s] %s != %s", outLabels[mapIdx], expKey, expVal, outVal))
-					}
-					valid = false
-					goto checkFinished
-				}
-			} else { //missing entry
+			outVal, ok := outMaps[mapIdx][expKey]
+			if !ok {
 				if debug {
 					retInfo = append(retInfo, fmt.Sprintf("missing entry %s[%s]", outLabels[mapIdx], expKey))
 				}
 				valid = false
 				goto checkFinished
 			}
+			if outVal != expVal { //ok entry
+				if debug {
+					retInfo = append(retInfo, fmt.Sprintf("mismatch %s[%s] %s != %s", outLabels[mapIdx], expKey, expVal, outVal))
+				}
+				valid = false
+				goto checkFinished
+			}
+			if debug {
+				retInfo = append(retInfo, fmt.Sprintf("ok %s[%s] %s == %s", outLabels[mapIdx], expKey, expVal, outVal))
+			}
+			valid = true
 		}
 	}
 checkFinished: