diff --git a/.golangci.yml b/.golangci.yml index 2c3a1f50fe..11d0d75996 100644 --- a/.golangci.yml +++ b/.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 diff --git a/agent/runner.go b/agent/runner.go index 753785dcd1..df353f60f3 100644 --- a/agent/runner.go +++ b/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() } diff --git a/cli/common/zerologger.go b/cli/common/zerologger.go index ba0dcd621a..2d0e2aceab 100644 --- a/cli/common/zerologger.go +++ b/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) } diff --git a/cli/secret/secret_info.go b/cli/secret/secret_info.go index 138dbb28e1..02d1a5d5ea 100644 --- a/cli/secret/secret_info.go +++ b/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 diff --git a/cli/secret/secret_list.go b/cli/secret/secret_list.go index 94a794f329..24d61fb0dd 100644 --- a/cli/secret/secret_list.go +++ b/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 diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index fc9c6f0c28..886c0411bb 100644 --- a/cmd/agent/agent.go +++ b/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") diff --git a/cmd/agent/main.go b/cmd/agent/main.go index a96167b8b1..01bb60246c 100644 --- a/cmd/agent/main.go +++ b/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 } } diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 36905d36f9..d0b1558685 100644 --- a/cmd/cli/main.go +++ b/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 } } diff --git a/cmd/server/main.go b/cmd/server/main.go index 72c1806c1a..0a87d39ef8 100644 --- a/cmd/server/main.go +++ b/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 } } diff --git a/cmd/server/server.go b/cmd/server/server.go index 34d385ceaa..4fa776c2f5 100644 --- a/cmd/server/server.go +++ b/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 :// format", - ) + return fmt.Errorf("WOODPECKER_HOST must be :// 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 { diff --git a/cmd/server/setup.go b/cmd/server/setup.go index 5a2c0ebf93..95fb611368 100644 --- a/cmd/server/setup.go +++ b/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) { diff --git a/pipeline/backend/common/script_win.go b/pipeline/backend/common/script_win.go index b311b0acb4..17d3b82797 100644 --- a/pipeline/backend/common/script_win.go +++ b/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, diff --git a/pipeline/backend/kubernetes/utils.go b/pipeline/backend/kubernetes/utils.go index fd924f3570..c5802eab6d 100644 --- a/pipeline/backend/kubernetes/utils.go +++ b/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 diff --git a/pipeline/frontend/yaml/compiler/cacher.go b/pipeline/frontend/yaml/compiler/cacher.go index 8e57f5f0fe..93c279f312 100644 --- a/pipeline/frontend/yaml/compiler/cacher.go +++ b/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{ diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go index 582753a1df..dd9e31bd68 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/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..., ) diff --git a/pipeline/frontend/yaml/matrix/matrix.go b/pipeline/frontend/yaml/matrix/matrix.go index 9abf3bd1af..d9c6729ee6 100644 --- a/pipeline/frontend/yaml/matrix/matrix.go +++ b/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] diff --git a/pipeline/frontend/yaml/parse.go b/pipeline/frontend/yaml/parse.go index e14b1bd1a6..2aaf6d940d 100644 --- a/pipeline/frontend/yaml/parse.go +++ b/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 diff --git a/server/api/helper.go b/server/api/helper.go index cfe409b9b6..f9c21d3373 100644 --- a/server/api/helper.go +++ b/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) } } diff --git a/server/api/user.go b/server/api/user.go index 4bd15ed34e..f33d4d32e6 100644 --- a/server/api/user.go +++ b/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) } } } diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go index 707a6dd732..4a7f8a985a 100644 --- a/server/forge/bitbucket/fixtures/handler.go +++ b/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, "") } diff --git a/server/forge/common/status.go b/server/forge/common/status.go index 27fa3f93fe..845edc2bf9 100644 --- a/server/forge/common/status.go +++ b/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" } diff --git a/server/forge/gitea/helper.go b/server/forge/gitea/helper.go index 1f68bdc62d..44dcde56aa 100644 --- a/server/forge/gitea/helper.go +++ b/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 } diff --git a/server/forge/github/parse.go b/server/forge/github/parse.go index 6df51b242e..df5b97c2d5 100644 --- a/server/forge/github/parse.go +++ b/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/", "") } } diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index 0641d43d48..7502b17127 100644 --- a/server/forge/gitlab/convert.go +++ b/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") } diff --git a/server/pipeline/pipelineStatus_test.go b/server/pipeline/pipelineStatus_test.go index a46a831c8e..ff0c6b8689 100644 --- a/server/pipeline/pipelineStatus_test.go +++ b/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) } } diff --git a/server/pipeline/step_status_test.go b/server/pipeline/step_status_test.go index 7a996cabe6..0f43f19d42 100644 --- a/server/pipeline/step_status_test.go +++ b/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) } } diff --git a/server/plugins/encryption/encryption_builder.go b/server/plugins/encryption/encryption_builder.go index 93b9dff1f6..f14a8824a6 100644 --- a/server/plugins/encryption/encryption_builder.go +++ b/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) diff --git a/server/plugins/encryption/tink_keyset_watcher.go b/server/plugins/encryption/tink_keyset_watcher.go index 15c0e3ec56..595f2daf08 100644 --- a/server/plugins/encryption/tink_keyset_watcher.go +++ b/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 } } } diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index e36b417ed7..ddc77dba32 100644 --- a/server/queue/fifo_test.go +++ b/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 diff --git a/server/router/middleware/session/agent.go b/server/router/middleware/session/agent.go index 3f8dda0603..52d5f5f17f 100644 --- a/server/router/middleware/session/agent.go +++ b/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() } } diff --git a/server/store/context.go b/server/store/context.go index 02381e5136..fc94c53a4c 100644 --- a/server/store/context.go +++ b/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 } diff --git a/server/store/datastore/migration/common.go b/server/store/datastore/migration/common.go index c0d0df2f03..5bc77db7d2 100644 --- a/server/store/datastore/migration/common.go +++ b/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...) diff --git a/server/web/web.go b/server/web/web.go index 2eb9b1aa2e..843fa37b42 100644 --- a/server/web/web.go +++ b/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 } diff --git a/shared/logger/logger.go b/shared/logger/logger.go index e8e71f1715..1fd0a3e957 100644 --- a/shared/logger/logger.go +++ b/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 }