Enable gocritic and don't ignore globally (woodpecker-ci#3159)

Use `nolint` directives instead.

From woodpecker-ci#2960

.golangci.yml

@@ -161,29 +161,8 @@ linters:
     - unparam
     - wastedassign
     - whitespace
+    - gocritic
     - nolintlint
 
 run:
   timeout: 15m
-
-issues:
-  exclude-rules:
-    # gin force us to use string as context key
-    - path: server/store/context.go
-      linters:
-        - staticcheck
-        - revive
-
-    # let cli use print and panic and log.Fatal()
-    - path: 'cmd/*|cli/*'
-      linters:
-        - forbidigo
-
-    # allow some setup functions to use log.Fatal()
-    - path: 'server/web/web.go|server/plugins/encryption/tink_keyset_watcher.go|shared/logger/logger.go'
-      linters:
-        - forbidigo
-
-    - path: '_test.go'
-      linters:
-        - forcetypeassert

agent/runner.go

@@ -160,13 +160,14 @@ func (r *Runner) Run(runnerCtx context.Context) error {
 		state.ExitCode = 137
 	} else if err != nil {
 		pExitError := &pipeline.ExitError{}
-		if errors.As(err, &pExitError) {
+		switch {
+		case errors.As(err, &pExitError):
 			state.ExitCode = pExitError.Code
-		} else if errors.Is(err, pipeline.ErrCancel) {
+		case errors.Is(err, pipeline.ErrCancel):
 			state.Error = ""
 			state.ExitCode = 137
 			canceled.SetTo(true)
-		} else {
+		default:
 			state.ExitCode = 1
 			state.Error = err.Error()
 		}

cli/common/zerologger.go

@@ -21,6 +21,5 @@ import (
 )
 
 func SetupGlobalLogger(c *cli.Context) error {
-	logger.SetupGlobalLogger(c, false)
-	return nil
+	return logger.SetupGlobalLogger(c, false)
 }

cli/secret/secret_info.go

@@ -61,17 +61,18 @@ func secretInfo(c *cli.Context) error {
 	}
 
 	var secret *woodpecker.Secret
-	if global {
+	switch {
+	case global:
 		secret, err = client.GlobalSecret(secretName)
 		if err != nil {
 			return err
 		}
-	} else if orgID != -1 {
+	case orgID != -1:
 		secret, err = client.OrgSecret(orgID, secretName)
 		if err != nil {
 			return err
 		}
-	} else {
+	default:
 		secret, err = client.Secret(repoID, secretName)
 		if err != nil {
 			return err

cli/secret/secret_list.go

@@ -56,17 +56,18 @@ func secretList(c *cli.Context) error {
 	}
 
 	var list []*woodpecker.Secret
-	if global {
+	switch {
+	case global:
 		list, err = client.GlobalSecretList()
 		if err != nil {
 			return err
 		}
-	} else if orgID != -1 {
+	case orgID != -1:
 		list, err = client.OrgSecretList(orgID)
 		if err != nil {
 			return err
 		}
-	} else {
+	default:
 		list, err = client.SecretList(repoID)
 		if err != nil {
 			return err

cmd/agent/agent.go

@@ -50,8 +50,6 @@ import (
 )
 
 func run(c *cli.Context) error {
-	logger.SetupGlobalLogger(c, true)
-
 	agentConfigPath := c.String("agent-config")
 	hostname := c.String("hostname")
 	if len(hostname) == 0 {
@@ -271,6 +269,10 @@ func getBackendEngine(backendCtx context.Context, backendName string, addons []s
 }
 
 func runWithRetry(context *cli.Context) error {
+	if err := logger.SetupGlobalLogger(context, true); err != nil {
+		return err
+	}
+
 	initHealth()
 
 	retryCount := context.Int("connect-retry-count")

cmd/agent/main.go

@@ -15,10 +15,10 @@
 package main
 
 import (
-	"fmt"
 	"os"
 
 	_ "github.com/joho/godotenv/autoload"
+	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli/v2"
 
 	"go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/docker"
@@ -45,7 +45,6 @@ func main() {
 	app.Flags = utils.MergeSlices(flags, logger.GlobalLoggerFlags, docker.Flags, kubernetes.Flags, local.Flags)
 
 	if err := app.Run(os.Args); err != nil {
-		fmt.Fprintln(os.Stderr, err)
-		os.Exit(1)
+		log.Fatal().Err(err).Msg("error running agent") //nolint:forbidigo
 	}
 }

cmd/cli/main.go

@@ -24,6 +24,6 @@ import (
 func main() {
 	app := newApp()
 	if err := app.Run(os.Args); err != nil {
-		log.Fatal().Err(err).Msg("error running cli")
+		log.Fatal().Err(err).Msg("error running cli") //nolint:forbidigo
 	}
 }

cmd/server/main.go

@@ -15,11 +15,12 @@
 package main
 
 import (
-	"fmt"
 	"os"
 
 	_ "github.com/joho/godotenv/autoload"
+	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli/v2"
+
 	_ "go.woodpecker-ci.org/woodpecker/v2/cmd/server/docs"
 
 	"go.woodpecker-ci.org/woodpecker/v2/version"
@@ -43,7 +44,6 @@ func main() {
 	setupSwaggerStaticConfig()
 
 	if err := app.Run(os.Args); err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, err)
-		os.Exit(1)
+		log.Fatal().Err(err).Msgf("error running server") //nolint:forbidigo
 	}
 }

cmd/server/server.go

@@ -17,6 +17,7 @@ package main
 import (
 	"crypto/tls"
 	"errors"
+	"fmt"
 	"net"
 	"net/http"
 	"net/http/httputil"
@@ -55,25 +56,25 @@ import (
 )
 
 func run(c *cli.Context) error {
-	logger.SetupGlobalLogger(c, true)
+	if err := logger.SetupGlobalLogger(c, true); err != nil {
+		return err
+	}
 
 	// set gin mode based on log level
 	if zerolog.GlobalLevel() > zerolog.DebugLevel {
 		gin.SetMode(gin.ReleaseMode)
 	}
 
 	if c.String("server-host") == "" {
-		log.Fatal().Msg("WOODPECKER_HOST is not properly configured")
+		return fmt.Errorf("WOODPECKER_HOST is not properly configured")
 	}
 
 	if !strings.Contains(c.String("server-host"), "://") {
-		log.Fatal().Msg(
-			"WOODPECKER_HOST must be <scheme>://<hostname> format",
-		)
+		return fmt.Errorf("WOODPECKER_HOST must be <scheme>://<hostname> format")
 	}
 
 	if _, err := url.Parse(c.String("server-host")); err != nil {
-		log.Fatal().Err(err).Msg("could not parse WOODPECKER_HOST")
+		return fmt.Errorf("could not parse WOODPECKER_HOST: %w", err)
 	}
 
 	if strings.Contains(c.String("server-host"), "://localhost") {
@@ -84,10 +85,13 @@ func run(c *cli.Context) error {
 
 	_forge, err := setupForge(c)
 	if err != nil {
-		log.Fatal().Err(err).Msg("can't setup forge")
+		return fmt.Errorf("can't setup forge: %w", err)
 	}
 
-	_store := setupStore(c)
+	_store, err := setupStore(c)
+	if err != nil {
+		return fmt.Errorf("can't setup store: %w", err)
+	}
 	defer func() {
 		if err := _store.Close(); err != nil {
 			log.Error().Err(err).Msg("could not close store")
@@ -96,7 +100,7 @@ func run(c *cli.Context) error {
 
 	err = setupEvilGlobals(c, _store, _forge)
 	if err != nil {
-		log.Fatal().Err(err).Msg("can't setup globals")
+		return fmt.Errorf("can't setup globals: %w", err)
 	}
 
 	var g errgroup.Group
@@ -111,7 +115,7 @@ func run(c *cli.Context) error {
 	g.Go(func() error {
 		lis, err := net.Listen("tcp", c.String("grpc-addr"))
 		if err != nil {
-			log.Fatal().Err(err).Msg("failed to listen on grpc-addr")
+			log.Fatal().Err(err).Msg("failed to listen on grpc-addr") //nolint:forbidigo
 		}
 
 		jwtSecret := c.String("grpc-secret")
@@ -144,7 +148,7 @@ func run(c *cli.Context) error {
 
 		err = grpcServer.Serve(lis)
 		if err != nil {
-			log.Fatal().Err(err).Msg("failed to serve grpc server")
+			log.Fatal().Err(err).Msg("failed to serve grpc server") //nolint:forbidigo
 		}
 		return nil
 	})
@@ -155,7 +159,8 @@ func run(c *cli.Context) error {
 	if proxyWebUI == "" {
 		webEngine, err := web.New()
 		if err != nil {
-			log.Fatal().Err(err).Msg("failed to create web engine")
+			log.Error().Err(err).Msg("failed to create web engine")
+			return err
 		}
 		webUIServe = webEngine.ServeHTTP
 	} else {
@@ -180,7 +185,8 @@ func run(c *cli.Context) error {
 		middleware.Store(c, _store),
 	)
 
-	if c.String("server-cert") != "" {
+	switch {
+	case c.String("server-cert") != "":
 		// start the server with tls enabled
 		g.Go(func() error {
 			serve := &http.Server{
@@ -195,7 +201,7 @@ func run(c *cli.Context) error {
 				c.String("server-key"),
 			)
 			if err != nil && !errors.Is(err, http.ErrServerClosed) {
-				log.Fatal().Err(err).Msg("failed to start server with tls")
+				log.Fatal().Err(err).Msg("failed to start server with tls") //nolint:forbidigo
 			}
 			return err
 		})
@@ -214,11 +220,11 @@ func run(c *cli.Context) error {
 		g.Go(func() error {
 			err := http.ListenAndServe(server.Config.Server.Port, http.HandlerFunc(redirect))
 			if err != nil && !errors.Is(err, http.ErrServerClosed) {
-				log.Fatal().Err(err).Msg("unable to start server to redirect from http to https")
+				log.Fatal().Err(err).Msg("unable to start server to redirect from http to https") //nolint:forbidigo
 			}
 			return err
 		})
-	} else if c.Bool("lets-encrypt") {
+	case c.Bool("lets-encrypt"):
 		// start the server with lets-encrypt
 		certmagic.DefaultACME.Email = c.String("lets-encrypt-email")
 		certmagic.DefaultACME.Agreed = true
@@ -230,19 +236,19 @@ func run(c *cli.Context) error {
 
 		g.Go(func() error {
 			if err := certmagic.HTTPS([]string{address.Host}, handler); err != nil {
-				log.Fatal().Err(err).Msg("certmagic does not work")
+				log.Fatal().Err(err).Msg("certmagic does not work") //nolint:forbidigo
 			}
 			return nil
 		})
-	} else {
+	default:
 		// start the server without tls
 		g.Go(func() error {
 			err := http.ListenAndServe(
 				c.String("server-addr"),
 				handler,
 			)
 			if err != nil && !errors.Is(err, http.ErrServerClosed) {
-				log.Fatal().Err(err).Msg("could not start server")
+				log.Fatal().Err(err).Msg("could not start server") //nolint:forbidigo
 			}
 			return err
 		})
@@ -254,7 +260,7 @@ func run(c *cli.Context) error {
 			metricsRouter.GET("/metrics", gin.WrapH(promhttp.Handler()))
 			err := http.ListenAndServe(metricsServerAddr, metricsRouter)
 			if err != nil && !errors.Is(err, http.ErrServerClosed) {
-				log.Fatal().Err(err).Msg("could not start metrics server")
+				log.Fatal().Err(err).Msg("could not start metrics server") //nolint:forbidigo
 			}
 			return err
 		})
@@ -298,7 +304,10 @@ func setupEvilGlobals(c *cli.Context, v store.Store, f forge.Forge) error {
 	}
 	server.Config.Services.Membership = setupMembershipService(c, f)
 
-	server.Config.Services.SignaturePrivateKey, server.Config.Services.SignaturePublicKey = setupSignatureKeys(v)
+	server.Config.Services.SignaturePrivateKey, server.Config.Services.SignaturePublicKey, err = setupSignatureKeys(v)
+	if err != nil {
+		return err
+	}
 
 	server.Config.Services.ConfigService, err = setupConfigService(c)
 	if err != nil {