ci(linter): add go-critic (kubernetes-sigs#5875)

* ci: add go-critic linter

follow go-critic advices when possible

* docs: add a short description about go-critic

Author: PascalBourdier

.golangci.yml

@@ -28,6 +28,7 @@ linters:
     - nilnil # Checks that there is no simultaneous return of nil error and an nil value. ref: https://golangci-lint.run/usage/linters/#nilnil
     - nonamedreturns # Checks that functions with named return values do not return named values. https://golangci-lint.run/usage/linters/#nonamedreturns
     - cyclop # Checks function and package cyclomatic complexity. https://golangci-lint.run/usage/linters/#cyclop
+    - gocritic # Analyze source code for various issues, including bugs, performance hiccups, and non-idiomatic coding practices. https://golangci-lint.run/docs/linters/configuration/#gocritic
 
     # tests
     - testifylint # Checks usage of github.com/stretchr/testify. https://golangci-lint.run/usage/linters/#testifylint

controller/controller.go

@@ -353,7 +353,7 @@ func (c *Controller) Run(ctx context.Context) {
 					consecutiveSoftErrors.Gauge.Set(float64(softErrorCount))
 					log.Errorf("Failed to do run once: %v (consecutive soft errors: %d)", err, softErrorCount)
 				} else {
-					log.Fatalf("Failed to do run once: %v", err)
+					log.Fatalf("Failed to do run once: %v", err) // nolint: gocritic // exitAfterDefer
 				}
 			} else {
 				if softErrorCount > 0 {

controller/execute.go

@@ -104,7 +104,7 @@ func Execute() {
 
 	endpointsSource, err := buildSource(ctx, cfg)
 	if err != nil {
-		log.Fatal(err)
+		log.Fatal(err) // nolint: gocritic // exitAfterDefer
 	}
 
 	domainFilter := createDomainFilter(cfg)

endpoint/domain_filter.go

@@ -122,13 +122,12 @@ func matchFilter(filters []string, domain string, emptyval bool) bool {
 			continue
 		}
 
-		if strings.HasPrefix(filter, ".") && strings.HasSuffix(strippedDomain, filter) {
+		switch {
+		case strings.HasPrefix(filter, ".") && strings.HasSuffix(strippedDomain, filter):
 			return true
-		} else if strings.Count(strippedDomain, ".") == strings.Count(filter, ".") {
-			if strippedDomain == filter {
-				return true
-			}
-		} else if strings.HasSuffix(strippedDomain, "."+filter) {
+		case strings.Count(strippedDomain, ".") == strings.Count(filter, ".") && strippedDomain == filter:
+			return true
+		case strings.HasSuffix(strippedDomain, "."+filter):
 			return true
 		}
 	}

endpoint/endpoint.go

@@ -386,11 +386,12 @@ func FilterEndpointsByOwnerID(ownerID string, eps []*Endpoint) []*Endpoint {
 	filtered := []*Endpoint{}
 	for _, ep := range eps {
 		endpointOwner, ok := ep.Labels[OwnerLabelKey]
-		if !ok {
+		switch {
+		case !ok:
 			log.Debugf(`Skipping endpoint %v because of missing owner label (required: "%s")`, ep, ownerID)
-		} else if endpointOwner != ownerID {
+		case endpointOwner != ownerID:
 			log.Debugf(`Skipping endpoint %v because owner id does not match (found: "%s", required: "%s")`, ep, endpointOwner, ownerID)
-		} else {
+		default:
 			filtered = append(filtered, ep)
 		}
 	}

internal/gen/docs/flags/main.go

@@ -57,7 +57,7 @@ func main() {
 	if err != nil {
 		_ = fmt.Errorf("failed to generate markdown file '%s': %v", path, err.Error())
 	}
-	content = content + "\n"
+	content += "\n"
 	_ = utils.WriteToFile(path, content)
 }
 

internal/gen/docs/flags/main_test.go

@@ -73,7 +73,7 @@ func TestFlagsMdUpToDate(t *testing.T) {
 	flags := computeFlags()
 	actual, err := flags.generateMarkdownTable()
 	assert.NoError(t, err)
-	actual = actual + "\n"
+	actual += "\n"
 	assert.Len(t, actual, len(expected), "expected file '%s' to be up to date. execute 'make generate-flags-documentation", fileName)
 }
 

internal/gen/docs/metrics/main.go

@@ -54,7 +54,7 @@ func main() {
 		_, _ = fmt.Fprintf(os.Stderr, "failed to generate markdown file '%s': %v\n", path, err)
 		os.Exit(1)
 	}
-	content = content + "\n"
+	content += "\n"
 	_ = utils.WriteToFile(path, content)
 }
 
@@ -119,6 +119,7 @@ func getRuntimeMetrics(reg prometheus.Registerer) []string {
 				runtimeMetrics = append(runtimeMetrics, k)
 			}
 		}
+	default:
 	}
 	sort.Strings(runtimeMetrics)
 	return runtimeMetrics

internal/testutils/endpoint_test.go

@@ -84,7 +84,7 @@ func TestExampleSameEndpoints(t *testing.T) {
 	// example.org 0 IN TXT  load-balancer.org []
 }
 
-func makeEndpoint(DNSName string) *endpoint.Endpoint {
+func makeEndpoint(DNSName string) *endpoint.Endpoint { // nolint: gocritic // captLocal
 	return &endpoint.Endpoint{
 		DNSName:       DNSName,
 		Targets:       endpoint.Targets{"target.com"},

pkg/events/types.go

@@ -177,7 +177,7 @@ func sanitize(input string) string {
 
 	// the name should end with an alphanumeric character
 	if len(sanitized) > 0 && !endsWithAlphaNumeric.MatchString(sanitized) {
-		sanitized = sanitized + "z"
+		sanitized += "z"
 	}
 
 	sanitized = invalidChars.ReplaceAllString(sanitized, "-")