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

Use `nolint` directives instead.

From #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,7 +56,9 @@ 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 {
@@ -63,17 +66,15 @@ func run(c *cli.Context) error {
 	}
 
 	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,11 +236,11 @@ 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(
@@ -242,7 +248,7 @@ func run(c *cli.Context) error {
 				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 {

cmd/server/setup.go

@@ -54,7 +54,7 @@ import (
 	addonTypes "go.woodpecker-ci.org/woodpecker/v2/shared/addon/types"
 )
 
-func setupStore(c *cli.Context) store.Store {
+func setupStore(c *cli.Context) (store.Store, error) {
 	datasource := c.String("datasource")
 	driver := c.String("driver")
 	xorm := store.XORM{
@@ -71,12 +71,12 @@ func setupStore(c *cli.Context) store.Store {
 	}
 
 	if !datastore.SupportedDriver(driver) {
-		log.Fatal().Msgf("database driver '%s' not supported", driver)
+		return nil, fmt.Errorf("database driver '%s' not supported", driver)
 	}
 
 	if driver == "sqlite3" {
 		if err := checkSqliteFileExist(datasource); err != nil {
-			log.Fatal().Err(err).Msg("check sqlite file")
+			return nil, fmt.Errorf("check sqlite file: %w", err)
 		}
 	}
 
@@ -88,14 +88,14 @@ func setupStore(c *cli.Context) store.Store {
 	log.Trace().Msgf("setup datastore: %#v", *opts)
 	store, err := datastore.NewEngine(opts)
 	if err != nil {
-		log.Fatal().Err(err).Msg("could not open datastore")
+		return nil, fmt.Errorf("could not open datastore: %w", err)
 	}
 
 	if err := store.Migrate(c.Bool("migrations-allow-long")); err != nil {
-		log.Fatal().Err(err).Msg("could not migrate datastore")
+		return nil, fmt.Errorf("could not migrate datastore: %w", err)
 	}
 
-	return store
+	return store, nil
 }
 
 func checkSqliteFileExist(path string) error {
@@ -204,7 +204,7 @@ func setupGitea(c *cli.Context) (forge.Forge, error) {
 		SkipVerify: c.Bool("gitea-skip-verify"),
 	}
 	if len(opts.URL) == 0 {
-		log.Fatal().Msg("WOODPECKER_GITEA_URL must be set")
+		return nil, fmt.Errorf("WOODPECKER_GITEA_URL must be set")
 	}
 	log.Trace().Msgf("Forge (gitea) opts: %#v", opts)
 	return gitea.New(opts)
@@ -294,34 +294,30 @@ func setupMetrics(g *errgroup.Group, _store store.Store) {
 }
 
 // setupSignatureKeys generate or load key pair to sign webhooks requests (i.e. used for extensions)
-func setupSignatureKeys(_store store.Store) (crypto.PrivateKey, crypto.PublicKey) {
+func setupSignatureKeys(_store store.Store) (crypto.PrivateKey, crypto.PublicKey, error) {
 	privKeyID := "signature-private-key"
 
 	privKey, err := _store.ServerConfigGet(privKeyID)
 	if errors.Is(err, types.RecordNotExist) {
 		_, privKey, err := ed25519.GenerateKey(rand.Reader)
 		if err != nil {
-			log.Fatal().Err(err).Msgf("Failed to generate private key")
-			return nil, nil
+			return nil, nil, fmt.Errorf("failed to generate private key: %w", err)
 		}
 		err = _store.ServerConfigSet(privKeyID, hex.EncodeToString(privKey))
 		if err != nil {
-			log.Fatal().Err(err).Msgf("Failed to generate private key")
-			return nil, nil
+			return nil, nil, fmt.Errorf("failed to store private key: %w", err)
 		}
 		log.Debug().Msg("Created private key")
-		return privKey, privKey.Public()
+		return privKey, privKey.Public(), nil
 	} else if err != nil {
-		log.Fatal().Err(err).Msgf("Failed to load private key")
-		return nil, nil
+		return nil, nil, fmt.Errorf("failed to load private key: %w", err)
 	}
 	privKeyStr, err := hex.DecodeString(privKey)
 	if err != nil {
-		log.Fatal().Err(err).Msgf("Failed to decode private key")
-		return nil, nil
+		return nil, nil, fmt.Errorf("failed to decode private key: %w", err)
 	}
 	privateKey := ed25519.PrivateKey(privKeyStr)
-	return privateKey, privateKey.Public()
+	return privateKey, privateKey.Public(), nil
 }
 
 func setupConfigService(c *cli.Context) (config.Extension, error) {

pipeline/backend/common/script_win.go

@@ -24,7 +24,7 @@ func generateScriptWindows(commands []string) string {
 	var buf bytes.Buffer
 	for _, command := range commands {
 		escaped := fmt.Sprintf("%q", command)
-		escaped = strings.Replace(escaped, "$", `\$`, -1)
+		escaped = strings.ReplaceAll(escaped, "$", `\$`)
 		buf.WriteString(fmt.Sprintf(
 			traceScriptWin,
 			escaped,

pipeline/backend/kubernetes/utils.go

@@ -32,7 +32,7 @@ var (
 )
 
 func dnsName(i string) (string, error) {
-	res := strings.Replace(i, "_", "-", -1)
+	res := strings.ReplaceAll(i, "_", "-")
 
 	if found := dnsPattern.FindStringIndex(res); found == nil {
 		return "", ErrDNSPatternInvalid

pipeline/frontend/yaml/compiler/cacher.go

@@ -40,7 +40,7 @@ func (c *volumeCacher) Restore(repo, branch string, mounts []string) *yaml_types
 			"mount":       mounts,
 			"path":        "/cache",
 			"restore":     true,
-			"file":        strings.Replace(branch, "/", "_", -1) + ".tar",
+			"file":        strings.ReplaceAll(branch, "/", "_") + ".tar",
 			"fallback_to": "main.tar",
 		},
 		Volumes: yaml_types.Volumes{
@@ -64,7 +64,7 @@ func (c *volumeCacher) Rebuild(repo, branch string, mounts []string) *yaml_types
 			"path":    "/cache",
 			"rebuild": true,
 			"flush":   true,
-			"file":    strings.Replace(branch, "/", "_", -1) + ".tar",
+			"file":    strings.ReplaceAll(branch, "/", "_") + ".tar",
 		},
 		Volumes: yaml_types.Volumes{
 			Volumes: []*yaml_types.Volume{

pipeline/frontend/yaml/constraint/constraint.go

@@ -254,7 +254,7 @@ func (c *List) UnmarshalYAML(value *yaml.Node) error {
 	err2 := value.Decode(&out2)
 
 	c.Exclude = out1.Exclude
-	c.Include = append(
+	c.Include = append( //nolint:gocritic
 		out1.Include,
 		out2...,
 	)
@@ -335,7 +335,7 @@ func (c *Path) UnmarshalYAML(value *yaml.Node) error {
 
 	c.Exclude = out1.Exclude
 	c.IgnoreMessage = out1.IgnoreMessage
-	c.Include = append(
+	c.Include = append( //nolint:gocritic
 		out1.Include,
 		out2...,
 	)

pipeline/frontend/yaml/matrix/matrix.go

@@ -89,7 +89,7 @@ func calc(matrix Matrix) []Axis {
 		decr := perm
 		for i, tag := range tags {
 			elems := matrix[tag]
-			decr = decr / len(elems)
+			decr /= len(elems)
 			elem := p / decr % len(elems)
 			axis[tag] = elems[elem]
 

pipeline/frontend/yaml/parse.go

@@ -34,11 +34,12 @@ func ParseBytes(b []byte) (*types.Workflow, error) {
 
 	// support deprecated branch filter
 	if out.BranchesDontUseIt != nil {
-		if out.When.Constraints == nil {
+		switch {
+		case out.When.Constraints == nil:
 			out.When.Constraints = []constraint.Constraint{{Branch: *out.BranchesDontUseIt}}
-		} else if len(out.When.Constraints) == 1 && out.When.Constraints[0].Branch.IsEmpty() {
+		case len(out.When.Constraints) == 1 && out.When.Constraints[0].Branch.IsEmpty():
 			out.When.Constraints[0].Branch = *out.BranchesDontUseIt
-		} else {
+		default:
 			return nil, fmt.Errorf("could not apply deprecated branches filter into global when filter")
 		}
 		out.BranchesDontUseIt = nil

server/api/helper.go

@@ -29,15 +29,16 @@ import (
 )
 
 func handlePipelineErr(c *gin.Context, err error) {
-	if errors.Is(err, &pipeline.ErrNotFound{}) {
+	switch {
+	case errors.Is(err, &pipeline.ErrNotFound{}):
 		c.String(http.StatusNotFound, "%s", err)
-	} else if errors.Is(err, &pipeline.ErrBadRequest{}) {
+	case errors.Is(err, &pipeline.ErrBadRequest{}):
 		c.String(http.StatusBadRequest, "%s", err)
-	} else if errors.Is(err, pipeline.ErrFiltered) {
+	case errors.Is(err, pipeline.ErrFiltered):
 		// for debugging purpose we add a header
 		c.Writer.Header().Add("Pipeline-Filtered", "true")
 		c.Status(http.StatusNoContent)
-	} else {
+	default:
 		_ = c.AbortWithError(http.StatusInternalServerError, err)
 	}
 }

server/api/user.go

@@ -117,11 +117,9 @@ func GetRepos(c *gin.Context) {
 					existingRepo.Update(r)
 					existingRepo.IsActive = active[r.ForgeRemoteID].IsActive
 					repos = append(repos, existingRepo)
-				} else {
-					if r.Perm.Admin {
-						// you must be admin to enable the repo
-						repos = append(repos, r)
-					}
+				} else if r.Perm.Admin {
+					// you must be admin to enable the repo
+					repos = append(repos, r)
 				}
 			}
 		}

server/forge/bitbucket/fixtures/handler.go

@@ -44,8 +44,7 @@ func Handler() http.Handler {
 }
 
 func getOauth(c *gin.Context) {
-	switch c.PostForm("error") {
-	case "invalid_scope":
+	if c.PostForm("error") == "invalid_scope" {
 		c.String(500, "")
 	}
 

server/forge/common/status.go

@@ -27,8 +27,7 @@ import (
 
 func GetPipelineStatusContext(repo *model.Repo, pipeline *model.Pipeline, workflow *model.Workflow) string {
 	event := string(pipeline.Event)
-	switch pipeline.Event {
-	case model.EventPull:
+	if pipeline.Event == model.EventPull {
 		event = "pr"
 	}
 

server/forge/gitea/helper.go

@@ -197,7 +197,7 @@ func fixMalformedAvatar(url string) string {
 	}
 	index = strings.Index(url, "//avatars/")
 	if index != -1 {
-		return strings.Replace(url, "//avatars/", "/avatars/", -1)
+		return strings.ReplaceAll(url, "//avatars/", "/avatars/")
 	}
 	return url
 }

server/forge/github/parse.go

@@ -85,7 +85,7 @@ func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline) {
 		Commit:       hook.GetHeadCommit().GetID(),
 		Ref:          hook.GetRef(),
 		ForgeURL:     hook.GetHeadCommit().GetURL(),
-		Branch:       strings.Replace(hook.GetRef(), "refs/heads/", "", -1),
+		Branch:       strings.ReplaceAll(hook.GetRef(), "refs/heads/", ""),
 		Message:      hook.GetHeadCommit().GetMessage(),
 		Email:        hook.GetHeadCommit().GetAuthor().GetEmail(),
 		Avatar:       hook.GetSender().GetAvatarURL(),
@@ -106,7 +106,7 @@ func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline) {
 		// For tags, if the base_ref (tag's base branch) is set, we're using it
 		// as pipeline's branch so that we can filter events base on it
 		if strings.HasPrefix(hook.GetBaseRef(), "refs/heads/") {
-			pipeline.Branch = strings.Replace(hook.GetBaseRef(), "refs/heads/", "", -1)
+			pipeline.Branch = strings.ReplaceAll(hook.GetBaseRef(), "refs/heads/", "")
 		}
 	}
 

server/forge/gitlab/convert.go

@@ -70,11 +70,12 @@ func convertMergeRequestHook(hook *gitlab.MergeEvent, req *http.Request) (int, *
 	source := hook.ObjectAttributes.Source
 	obj := hook.ObjectAttributes
 
-	if target == nil && source == nil {
+	switch {
+	case target == nil && source == nil:
 		return 0, nil, nil, fmt.Errorf("target and source keys expected in merge request hook")
-	} else if target == nil {
+	case target == nil:
 		return 0, nil, nil, fmt.Errorf("target key expected in merge request hook")
-	} else if source == nil {
+	case source == nil:
 		return 0, nil, nil, fmt.Errorf("source key expected in merge request hook")
 	}
 

server/pipeline/pipelineStatus_test.go

@@ -48,11 +48,12 @@ func TestUpdateToStatusPending(t *testing.T) {
 
 	pipeline, _ := UpdateToStatusPending(&mockUpdatePipelineStore{}, model.Pipeline{}, "Reviewer")
 
-	if model.StatusPending != pipeline.Status {
+	switch {
+	case model.StatusPending != pipeline.Status:
 		t.Errorf("Pipeline status not equals '%s' != '%s'", model.StatusPending, pipeline.Status)
-	} else if pipeline.Reviewer != "Reviewer" {
+	case pipeline.Reviewer != "Reviewer":
 		t.Errorf("Reviewer not equals 'Reviewer' != '%s'", pipeline.Reviewer)
-	} else if now > pipeline.Reviewed {
+	case now > pipeline.Reviewed:
 		t.Errorf("Reviewed not updated %d !< %d", now, pipeline.Reviewed)
 	}
 }
@@ -64,11 +65,12 @@ func TestUpdateToStatusDeclined(t *testing.T) {
 
 	pipeline, _ := UpdateToStatusDeclined(&mockUpdatePipelineStore{}, model.Pipeline{}, "Reviewer")
 
-	if model.StatusDeclined != pipeline.Status {
+	switch {
+	case model.StatusDeclined != pipeline.Status:
 		t.Errorf("Pipeline status not equals '%s' != '%s'", model.StatusDeclined, pipeline.Status)
-	} else if pipeline.Reviewer != "Reviewer" {
+	case pipeline.Reviewer != "Reviewer":
 		t.Errorf("Reviewer not equals 'Reviewer' != '%s'", pipeline.Reviewer)
-	} else if now > pipeline.Reviewed {
+	case now > pipeline.Reviewed:
 		t.Errorf("Reviewed not updated %d !< %d", now, pipeline.Reviewed)
 	}
 }
@@ -92,15 +94,16 @@ func TestUpdateToStatusError(t *testing.T) {
 
 	pipeline, _ := UpdateToStatusError(&mockUpdatePipelineStore{}, model.Pipeline{}, errors.New("this is an error"))
 
-	if len(pipeline.Errors) != 1 {
+	switch {
+	case len(pipeline.Errors) != 1:
 		t.Errorf("Expected one error, got %d", len(pipeline.Errors))
-	} else if pipeline.Errors[0].Error() != "[generic] this is an error" {
+	case pipeline.Errors[0].Error() != "[generic] this is an error":
 		t.Errorf("Pipeline error not equals '[generic] this is an error' != '%s'", pipeline.Errors[0].Error())
-	} else if model.StatusError != pipeline.Status {
+	case model.StatusError != pipeline.Status:
 		t.Errorf("Pipeline status not equals '%s' != '%s'", model.StatusError, pipeline.Status)
-	} else if now > pipeline.Started {
+	case now > pipeline.Started:
 		t.Errorf("Started not updated %d !< %d", now, pipeline.Started)
-	} else if pipeline.Started != pipeline.Finished {
+	case pipeline.Started != pipeline.Finished:
 		t.Errorf("Pipeline started and finished not equals %d != %d", pipeline.Started, pipeline.Finished)
 	}
 }

server/pipeline/step_status_test.go

@@ -198,13 +198,14 @@ func TestUpdateStepStatusToDoneSkipped(t *testing.T) {
 
 	step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state)
 
-	if step.State != model.StatusSkipped {
+	switch {
+	case step.State != model.StatusSkipped:
 		t.Errorf("Step status not equals '%s' != '%s'", model.StatusSkipped, step.State)
-	} else if step.Stopped != int64(34) {
+	case step.Stopped != int64(34):
 		t.Errorf("Step stopped not equals 34 != %d", step.Stopped)
-	} else if step.Error != "" {
+	case step.Error != "":
 		t.Errorf("Step error not equals '' != '%s'", step.Error)
-	} else if step.ExitCode != 0 {
+	case step.ExitCode != 0:
 		t.Errorf("Step exit code not equals 0 != %d", step.ExitCode)
 	}
 }
@@ -219,13 +220,14 @@ func TestUpdateStepStatusToDoneSuccess(t *testing.T) {
 
 	step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state)
 
-	if step.State != model.StatusSuccess {
+	switch {
+	case step.State != model.StatusSuccess:
 		t.Errorf("Step status not equals '%s' != '%s'", model.StatusSuccess, step.State)
-	} else if step.Stopped != int64(34) {
+	case step.Stopped != int64(34):
 		t.Errorf("Step stopped not equals 34 != %d", step.Stopped)
-	} else if step.Error != "" {
+	case step.Error != "":
 		t.Errorf("Step error not equals '' != '%s'", step.Error)
-	} else if step.ExitCode != 0 {
+	case step.ExitCode != 0:
 		t.Errorf("Step exit code not equals 0 != %d", step.ExitCode)
 	}
 }
@@ -261,13 +263,14 @@ func TestUpdateStepToStatusKilledStarted(t *testing.T) {
 
 	step, _ := UpdateStepToStatusKilled(&mockUpdateStepStore{}, model.Step{})
 
-	if step.State != model.StatusKilled {
+	switch {
+	case step.State != model.StatusKilled:
 		t.Errorf("Step status not equals '%s' != '%s'", model.StatusKilled, step.State)
-	} else if step.Stopped < now {
+	case step.Stopped < now:
 		t.Errorf("Step stopped not equals %d < %d", now, step.Stopped)
-	} else if step.Started != step.Stopped {
+	case step.Started != step.Stopped:
 		t.Errorf("Step started not equals %d != %d", step.Stopped, step.Started)
-	} else if step.ExitCode != 137 {
+	case step.ExitCode != 137:
 		t.Errorf("Step exit code not equals 137 != %d", step.ExitCode)
 	}
 }

server/plugins/encryption/encryption_builder.go

@@ -50,22 +50,24 @@ func (b builder) isEnabled() (bool, error) {
 func (b builder) detectKeyType() (string, error) {
 	rawKeyPresent := b.ctx.IsSet(rawKeyConfigFlag)
 	tinkKeysetPresent := b.ctx.IsSet(tinkKeysetFilepathConfigFlag)
-	if rawKeyPresent && tinkKeysetPresent {
+	switch {
+	case rawKeyPresent && tinkKeysetPresent:
 		return "", errors.New(errMessageCantUseBothServices)
-	} else if rawKeyPresent {
+	case rawKeyPresent:
 		return keyTypeRaw, nil
-	} else if tinkKeysetPresent {
+	case tinkKeysetPresent:
 		return keyTypeTink, nil
 	}
 	return keyTypeNone, nil
 }
 
 func (b builder) serviceBuilder(keyType string) (model.EncryptionServiceBuilder, error) {
-	if keyType == keyTypeTink {
+	switch {
+	case keyType == keyTypeTink:
 		return newTink(b.ctx, b.store), nil
-	} else if keyType == keyTypeRaw {
+	case keyType == keyTypeRaw:
 		return newAES(b.ctx, b.store), nil
-	} else if keyType == keyTypeNone {
+	case keyType == keyTypeNone:
 		return &noEncryptionBuilder{}, nil
 	}
 	return nil, fmt.Errorf(errMessageTemplateUnsupportedKeyType, keyType)

server/plugins/encryption/tink_keyset_watcher.go

@@ -42,19 +42,19 @@ func (svc *tinkEncryptionService) handleFileEvents() {
 		select {
 		case event, ok := <-svc.keysetFileWatcher.Events:
 			if !ok {
-				log.Fatal().Msg(errMessageTinkKeysetFileWatchFailed)
+				log.Fatal().Msg(errMessageTinkKeysetFileWatchFailed) //nolint:forbidigo
 			}
 			if (event.Op == fsnotify.Write) || (event.Op == fsnotify.Create) {
 				log.Warn().Msgf(logTemplateTinkKeysetFileChanged, event.Name)
 				err := svc.rotate()
 				if err != nil {
-					log.Fatal().Err(err).Msgf(errMessageFailedRotatingEncryption)
+					log.Fatal().Err(err).Msgf(errMessageFailedRotatingEncryption) //nolint:forbidigo
 				}
 				return
 			}
 		case err, ok := <-svc.keysetFileWatcher.Errors:
 			if !ok {
-				log.Fatal().Err(err).Msgf(errMessageTinkKeysetFileWatchFailed)
+				log.Fatal().Err(err).Msgf(errMessageTinkKeysetFileWatchFailed) //nolint:forbidigo
 			}
 		}
 	}

server/queue/fifo_test.go

@@ -303,7 +303,8 @@ func TestFifoErrorsMultiThread(t *testing.T) {
 		case got := <-obtainedWorkCh:
 			fmt.Println(got.ID)
 
-			if !task1Processed {
+			switch {
+			case !task1Processed:
 				if got != task1 {
 					t.Errorf("expect task1 returned from queue as task2 and task3 depends on it")
 					return
@@ -317,7 +318,7 @@ func TestFifoErrorsMultiThread(t *testing.T) {
 						obtainedWorkCh <- got
 					}
 				}()
-			} else if !task2Processed {
+			case !task2Processed:
 				if got != task2 {
 					t.Errorf("expect task2 returned from queue")
 					return
@@ -331,7 +332,7 @@ func TestFifoErrorsMultiThread(t *testing.T) {
 						obtainedWorkCh <- got
 					}
 				}()
-			} else {
+			default:
 				if got != task3 {
 					t.Errorf("expect task3 returned from queue")
 					return

server/router/middleware/session/agent.go

@@ -32,13 +32,14 @@ func AuthorizeAgent(c *gin.Context) {
 	parsed, err := token.ParseRequest(c.Request, func(t *token.Token) (string, error) {
 		return secret, nil
 	})
-	if err != nil {
+	switch {
+	case err != nil:
 		c.String(500, "invalid or empty token. %s", err)
 		c.Abort()
-	} else if parsed.Kind != token.AgentToken {
+	case parsed.Kind != token.AgentToken:
 		c.String(403, "invalid token. please use an agent token")
 		c.Abort()
-	} else {
+	default:
 		c.Next()
 	}
 }

server/store/context.go

@@ -43,5 +43,5 @@ func ToContext(c Setter, store Store) {
 }
 
 func InjectToContext(ctx context.Context, store Store) context.Context {
-	return context.WithValue(ctx, key, store)
+	return context.WithValue(ctx, key, store) //nolint:revive,staticcheck
 }

server/store/datastore/migration/common.go

@@ -85,7 +85,11 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin
 		tableSQL := normalizeSQLiteTableSchema(string(res[0]["sql"]))
 
 		// Separate out the column definitions
-		tableSQL = tableSQL[strings.Index(tableSQL, "("):]
+		sqlIndex := strings.Index(tableSQL, "(")
+		if sqlIndex < 0 {
+			return fmt.Errorf("could not separate column definitions")
+		}
+		tableSQL = tableSQL[sqlIndex:]
 
 		// Remove the required columnNames
 		tableSQL = removeColumnFromSQLITETableSchema(tableSQL, columnNames...)

server/web/web.go

@@ -17,6 +17,7 @@ package web
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 	"io/fs"
 	"net/http"
@@ -44,7 +45,11 @@ func (f *prefixFS) Open(name string) (http.File, error) {
 // New returns a gin engine to serve the web frontend.
 func New() (*gin.Engine, error) {
 	e := gin.New()
-	indexHTML = parseIndex()
+	var err error
+	indexHTML, err = parseIndex()
+	if err != nil {
+		return nil, err
+	}
 
 	rootPath := server.Config.Server.RootPath
 
@@ -72,11 +77,12 @@ func handleCustomFilesAndAssets(fs *prefixFS) func(ctx *gin.Context) {
 		}
 	}
 	return func(ctx *gin.Context) {
-		if strings.HasSuffix(ctx.Request.RequestURI, "/assets/custom.js") {
+		switch {
+		case strings.HasSuffix(ctx.Request.RequestURI, "/assets/custom.js"):
 			serveFileOrEmptyContent(ctx.Writer, ctx.Request, server.Config.Server.CustomJsFile, "file.js")
-		} else if strings.HasSuffix(ctx.Request.RequestURI, "/assets/custom.css") {
+		case strings.HasSuffix(ctx.Request.RequestURI, "/assets/custom.css"):
 			serveFileOrEmptyContent(ctx.Writer, ctx.Request, server.Config.Server.CustomCSSFile, "file.css")
-		} else {
+		default:
 			serveFile(fs)(ctx)
 		}
 	}
@@ -155,13 +161,13 @@ func replaceBytes(data []byte) []byte {
 	return bytes.ReplaceAll(data, []byte("/BASE_PATH"), []byte(server.Config.Server.RootPath))
 }
 
-func parseIndex() []byte {
+func parseIndex() ([]byte, error) {
 	data, err := loadFile("index.html")
 	if err != nil {
-		log.Fatal().Err(err).Msg("can not find index.html")
+		return nil, fmt.Errorf("can not find index.html: %w", err)
 	}
 	data = bytes.ReplaceAll(data, []byte("/web-config.js"), []byte(server.Config.Server.RootPath+"/web-config.js"))
 	data = bytes.ReplaceAll(data, []byte("/assets/custom.css"), []byte(server.Config.Server.RootPath+"/assets/custom.css"))
 	data = bytes.ReplaceAll(data, []byte("/assets/custom.js"), []byte(server.Config.Server.RootPath+"/assets/custom.js"))
-	return data
+	return data, nil
 }

shared/logger/logger.go

@@ -15,6 +15,7 @@
 package logger
 
 import (
+	"fmt"
 	"io"
 	"os"
 
@@ -51,7 +52,7 @@ var GlobalLoggerFlags = []cli.Flag{
 	},
 }
 
-func SetupGlobalLogger(c *cli.Context, printLvl bool) {
+func SetupGlobalLogger(c *cli.Context, outputLvl bool) error {
 	logLevel := c.String("log-level")
 	pretty := c.Bool("pretty")
 	noColor := c.Bool("nocolor")
@@ -66,7 +67,7 @@ func SetupGlobalLogger(c *cli.Context, printLvl bool) {
 	default: // a file was set
 		openFile, err := logfile.OpenFileWithContext(c.Context, logFile, 0o660)
 		if err != nil {
-			log.Fatal().Err(err).Msgf("could not open log file '%s'", logFile)
+			return fmt.Errorf("could not open log file '%s': %w", logFile, err)
 		}
 		file = openFile
 		noColor = true
@@ -87,7 +88,7 @@ func SetupGlobalLogger(c *cli.Context, printLvl bool) {
 
 	lvl, err := zerolog.ParseLevel(logLevel)
 	if err != nil {
-		log.Fatal().Msgf("unknown logging level: %s", logLevel)
+		return fmt.Errorf("unknown logging level: %s", logLevel)
 	}
 	zerolog.SetGlobalLevel(lvl)
 
@@ -96,7 +97,9 @@ func SetupGlobalLogger(c *cli.Context, printLvl bool) {
 		log.Logger = log.With().Caller().Logger()
 	}
 
-	if printLvl {
+	if outputLvl {
 		log.Info().Msgf("LogLevel = %s", zerolog.GlobalLevel().String())
 	}
+
+	return nil
 }