From cb8647267ff98b73182578a208ceb4717b80d06d Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sun, 17 Dec 2023 23:24:49 +0100 Subject: [PATCH 01/24] Enable more go linters --- .golangci.yml | 46 +++++++++++ agent/runner.go | 7 +- agent/state.go | 6 +- cli/common/flags.go | 2 +- cli/deploy/deploy.go | 4 +- cli/exec/exec.go | 4 +- cli/internal/util.go | 4 +- cli/secret/secret_info.go | 7 +- cli/secret/secret_list.go | 7 +- cli/user/user_info.go | 2 +- cmd/agent/agent.go | 1 - cmd/agent/health.go | 20 ++--- cmd/agent/main.go | 2 + cmd/server/server.go | 14 ++-- cmd/server/setup.go | 4 +- pipeline/backend/backend.go | 4 +- pipeline/backend/common/script_win.go | 2 +- pipeline/backend/docker/docker.go | 18 ++--- pipeline/backend/kubernetes/kubernetes.go | 29 +++---- pipeline/backend/kubernetes/utils.go | 2 +- pipeline/backend/local/clone.go | 8 +- pipeline/backend/local/local.go | 4 +- pipeline/frontend/yaml/compiler/cacher.go | 4 +- .../frontend/yaml/constraint/constraint.go | 7 +- .../frontend/yaml/linter/schema/schema.go | 10 +-- pipeline/frontend/yaml/matrix/matrix.go | 2 +- pipeline/frontend/yaml/parse.go | 7 +- pipeline/frontend/yaml/types/base/int.go | 4 +- pipeline/frontend/yaml/types/base/map.go | 8 +- pipeline/frontend/yaml/types/base/slice.go | 4 +- pipeline/frontend/yaml/types/network.go | 17 ++-- pipeline/frontend/yaml/types/volume.go | 4 +- pipeline/pipeline.go | 2 +- pipeline/stepBuilder.go | 5 +- server/api/agent.go | 8 +- server/api/badge.go | 4 +- server/api/cron.go | 8 +- server/api/global_secret.go | 6 +- server/api/helper.go | 11 +-- server/api/hook.go | 2 +- server/api/login.go | 2 +- server/api/metrics/prometheus.go | 2 +- server/api/org.go | 4 +- server/api/org_secret.go | 6 +- server/api/orgs.go | 2 +- server/api/pipeline.go | 24 +++--- server/api/registry.go | 6 +- server/api/repo.go | 2 +- server/api/repo_secret.go | 6 +- server/api/user.go | 8 +- server/api/users.go | 8 +- server/badges/badges_test.go | 1 + server/forge/bitbucket/bitbucket.go | 5 +- server/forge/bitbucket/fixtures/handler.go | 79 +++++++++---------- server/forge/bitbucket/internal/client.go | 5 +- server/forge/common/status.go | 3 +- server/forge/gitea/fixtures/handler.go | 40 +++++----- server/forge/gitea/gitea.go | 8 +- server/forge/gitea/helper.go | 4 +- server/forge/github/convert_test.go | 15 ++-- server/forge/github/fixtures/handler.go | 14 ++-- server/forge/github/github.go | 5 +- server/forge/github/parse.go | 22 +++--- server/forge/gitlab/convert.go | 9 ++- server/forge/gitlab/gitlab.go | 2 +- server/forge/types/errors.go | 2 +- server/grpc/auth_server.go | 2 +- server/grpc/filter.go | 4 +- server/grpc/filter_test.go | 5 +- server/grpc/rpc.go | 27 ++++--- server/grpc/server.go | 2 +- server/model/environ.go | 4 +- server/model/registry.go | 6 +- server/model/repo.go | 2 +- server/model/secret.go | 8 +- server/model/user.go | 4 +- server/pipeline/cancel.go | 3 +- server/pipeline/create.go | 13 ++- server/pipeline/decline.go | 4 +- server/pipeline/errors.go | 8 +- server/pipeline/pipelineStatus_test.go | 25 +++--- server/pipeline/queue.go | 10 ++- server/pipeline/start.go | 15 +++- server/pipeline/stepStatus_test.go | 71 +++++++++-------- server/pipeline/topic.go | 10 ++- server/plugins/config/http.go | 8 +- .../plugins/encryption/encryption_builder.go | 14 ++-- server/plugins/registry/filesystem.go | 4 +- server/plugins/utils/http.go | 4 +- server/queue/fifo.go | 16 ++-- server/queue/fifo_test.go | 29 +++---- server/router/middleware/header/header.go | 2 +- server/router/middleware/session/agent.go | 17 ++-- server/router/middleware/session/user.go | 10 +-- server/store/context.go | 3 +- server/store/datastore/agent.go | 2 +- server/store/datastore/migration/migration.go | 2 +- server/web/config.go | 5 +- server/web/web.go | 7 +- 99 files changed, 517 insertions(+), 423 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0e15e61450..08d12dea17 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,6 +16,11 @@ linters-settings: - ^log.Fatal().*$ errorlint: errorf-multi: true + gci: + sections: + - standard + - default + - prefix(go.woodpecker-ci.org/woodpecker) linters: disable-all: true @@ -37,6 +42,34 @@ linters: - errorlint - forbidigo - zerologlint + - asciicheck + - bodyclose + - contextcheck + # - depguard + - dogsled + - durationcheck + - errchkjson + - forcetypeassert + # - gci + - gochecknoinits + # - goconst + - gocritic + - goheader + - gomnd + - gomoddirectives + - gomodguard + - goprintffuncname + - importas + - makezero + - nolintlint + - rowserrcheck + - sqlclosecheck + - stylecheck + - tenv + - unconvert + - unparam + - wastedassign + - whitespace run: timeout: 5m @@ -57,3 +90,16 @@ issues: - path: 'server/web/web.go|server/plugins/encryption/tink_keyset_watcher.go' linters: - forbidigo + + - path: agent/runner.go + linters: + - contextcheck + - errorlint + + - path: 'server/forge/bitbucket/bitbucket_test.go|server/forge/gitea/gitea_test.go|server/forge/github/github_test.go' + linters: + - forcetypeassert + + - path: server/store/datastore/migration/common.go|pipeline/frontend/yaml/constraint/constraint.go + linters: + - gocritic 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/agent/state.go b/agent/state.go index 8d02d928a0..7131cf34bf 100644 --- a/agent/state.go +++ b/agent/state.go @@ -74,7 +74,11 @@ func (s *State) Healthy() bool { func (s *State) WriteTo(w io.Writer) (int64, error) { s.Lock() - out, _ := json.Marshal(s) + out, err := json.Marshal(s) + if err != nil { + s.Unlock() + return 0, err + } s.Unlock() ret, err := w.Write(out) return int64(ret), err diff --git a/cli/common/flags.go b/cli/common/flags.go index 08f2288662..d9bfd00091 100644 --- a/cli/common/flags.go +++ b/cli/common/flags.go @@ -64,7 +64,7 @@ func FormatFlag(tmpl string, hidden ...bool) *cli.StringFlag { } } -// specify repository +// RepoFlag specifies repository var RepoFlag = &cli.StringFlag{ Name: "repository", Aliases: []string{"repo"}, diff --git a/cli/deploy/deploy.go b/cli/deploy/deploy.go index 0ed963e620..6ce9b5502e 100644 --- a/cli/deploy/deploy.go +++ b/cli/deploy/deploy.go @@ -97,7 +97,7 @@ func deploy(c *cli.Context) error { } } if number == 0 { - return fmt.Errorf("Cannot deploy failure pipeline") + return fmt.Errorf("cannot deploy failure pipeline") } } else { number, err = strconv.ParseInt(pipelineArg, 10, 64) @@ -108,7 +108,7 @@ func deploy(c *cli.Context) error { env := c.Args().Get(2) if env == "" { - return fmt.Errorf("Please specify the target environment (ie production)") + return fmt.Errorf("please specify the target environment (ie production)") } params := internal.ParseKeyPair(c.StringSlice("param")) diff --git a/cli/exec/exec.go b/cli/exec/exec.go index 7e8c7fc668..c9b7ab32c1 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -94,7 +94,7 @@ func runExec(c *cli.Context, file, repoPath string) error { axes, err := matrix.ParseString(string(dat)) if err != nil { - return fmt.Errorf("Parse matrix fail") + return fmt.Errorf("parse matrix fail") } if len(axes) == 0 { @@ -213,7 +213,7 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error } backendCtx := context.WithValue(c.Context, backendTypes.CliContext, c) - backend.Init(backendCtx) + backend.Init() backendEngine, err := backend.FindBackend(backendCtx, c.String("backend-engine")) if err != nil { diff --git a/cli/internal/util.go b/cli/internal/util.go index e71b7558dd..e4a3717c34 100644 --- a/cli/internal/util.go +++ b/cli/internal/util.go @@ -44,10 +44,10 @@ func NewClient(c *cli.Context) (woodpecker.Client, error) { // if no server url is provided we can default // to the hosted Woodpecker service. if len(server) == 0 { - return nil, fmt.Errorf("Error: you must provide the Woodpecker server address") + return nil, fmt.Errorf("you must provide the Woodpecker server address") } if len(token) == 0 { - return nil, fmt.Errorf("Error: you must provide your Woodpecker access token") + return nil, fmt.Errorf("you must provide your Woodpecker access token") } // attempt to find system CA certs 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/cli/user/user_info.go b/cli/user/user_info.go index f7c92abca2..62abe27130 100644 --- a/cli/user/user_info.go +++ b/cli/user/user_info.go @@ -41,7 +41,7 @@ func userInfo(c *cli.Context) error { login := c.Args().First() if len(login) == 0 { - return fmt.Errorf("Missing or invalid user login") + return fmt.Errorf("missing or invalid user login") } user, err := client.User(login) diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index 1ff953d73c..c8ce918ab7 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -261,7 +261,6 @@ func getBackendEngine(backendCtx context.Context, backendName string, addons []s return addonBackend.Value, nil } - backend.Init(backendCtx) engine, err := backend.FindBackend(backendCtx, backendName) if err != nil { log.Error().Err(err).Msgf("cannot find backend engine '%s'", backendName) diff --git a/cmd/agent/health.go b/cmd/agent/health.go index 0a508f07c5..a7a52d37d3 100644 --- a/cmd/agent/health.go +++ b/cmd/agent/health.go @@ -31,7 +31,7 @@ import ( // following specification: // https://github.com/mozilla-services/Dockerflow -func init() { +func initHealth() { http.HandleFunc("/varz", handleStats) http.HandleFunc("/healthz", handleHeartbeat) http.HandleFunc("/version", handleVersion) @@ -39,26 +39,28 @@ func init() { func handleHeartbeat(w http.ResponseWriter, _ *http.Request) { if counter.Healthy() { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } } func handleVersion(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Header().Add("Content-Type", "text/json") - _ = json.NewEncoder(w).Encode(versionResp{ + if err := json.NewEncoder(w).Encode(versionResp{ Source: "https://github.com/woodpecker-ci/woodpecker", Version: version.String(), - }) + }); err != nil { + log.Err(err).Msg("handleVersion could not encode") + } } func handleStats(w http.ResponseWriter, _ *http.Request) { if counter.Healthy() { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } w.Header().Add("Content-Type", "text/json") if _, err := counter.WriteTo(w); err != nil { @@ -89,7 +91,7 @@ func pinger(c *cli.Context) error { return err } defer resp.Body.Close() - if resp.StatusCode != 200 { + if resp.StatusCode != http.StatusOK { return fmt.Errorf("agent returned non-200 status code") } return nil diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 8cbd4e0549..87acced2b9 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -30,6 +30,8 @@ import ( ) func main() { + initHealth() + app := cli.NewApp() app.Name = "woodpecker-agent" app.Version = version.String() diff --git a/cmd/server/server.go b/cmd/server/server.go index 37723c5b49..3956427a5a 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -88,10 +88,7 @@ func run(c *cli.Context) error { log.Fatal().Err(err).Msg("can't setup forge") } - _store, err := setupStore(c) - if err != nil { - log.Fatal().Err(err).Msg("cant't setup database store") - } + _store := setupStore(c) defer func() { if err := _store.Close(); err != nil { log.Error().Err(err).Msg("could not close store") @@ -181,7 +178,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{ @@ -219,7 +217,7 @@ func run(c *cli.Context) error { } 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 @@ -235,7 +233,7 @@ func run(c *cli.Context) error { } return nil }) - } else { + default: // start the server without tls g.Go(func() error { err := http.ListenAndServe( @@ -305,7 +303,7 @@ func setupEvilGlobals(c *cli.Context, v store.Store, f forge.Forge) { // Execution _events := c.StringSlice("default-cancel-previous-pipeline-events") - events := make([]model.WebhookEvent, len(_events)) + events := make([]model.WebhookEvent, len(_events), 0) for _, v := range _events { events = append(events, model.WebhookEvent(v)) } diff --git a/cmd/server/setup.go b/cmd/server/setup.go index df036510f4..6c8fdfee63 100644 --- a/cmd/server/setup.go +++ b/cmd/server/setup.go @@ -53,7 +53,7 @@ import ( addonTypes "go.woodpecker-ci.org/woodpecker/v2/shared/addon/types" ) -func setupStore(c *cli.Context) (store.Store, error) { +func setupStore(c *cli.Context) store.Store { datasource := c.String("datasource") driver := c.String("driver") xorm := store.XORM{ @@ -94,7 +94,7 @@ func setupStore(c *cli.Context) (store.Store, error) { log.Fatal().Err(err).Msg("could not migrate datastore") } - return store, nil + return store } func checkSqliteFileExist(path string) error { diff --git a/pipeline/backend/backend.go b/pipeline/backend/backend.go index 56d3291d75..99ecb69134 100644 --- a/pipeline/backend/backend.go +++ b/pipeline/backend/backend.go @@ -29,11 +29,11 @@ var ( backends []types.Backend ) -func Init(ctx context.Context) { +func Init() { backends = []types.Backend{ docker.New(), local.New(), - kubernetes.New(ctx), + kubernetes.New(), } backendsByName = make(map[string]types.Backend) 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/docker/docker.go b/pipeline/backend/docker/docker.go index bd8dd73cec..98249b4c0e 100644 --- a/pipeline/backend/docker/docker.go +++ b/pipeline/backend/docker/docker.go @@ -147,11 +147,11 @@ func (e *docker) Load(ctx context.Context) (*backend.BackendInfo, error) { }, nil } -func (e *docker) SetupWorkflow(_ context.Context, conf *backend.Config, taskUUID string) error { +func (e *docker) SetupWorkflow(ctx context.Context, conf *backend.Config, taskUUID string) error { log.Trace().Str("taskUUID", taskUUID).Msg("create workflow environment") for _, vol := range conf.Volumes { - _, err := e.client.VolumeCreate(noContext, volume.CreateOptions{ + _, err := e.client.VolumeCreate(ctx, volume.CreateOptions{ Name: vol.Name, Driver: volumeDriver, }) @@ -165,7 +165,7 @@ func (e *docker) SetupWorkflow(_ context.Context, conf *backend.Config, taskUUID networkDriver = networkDriverNAT } for _, n := range conf.Networks { - _, err := e.client.NetworkCreate(noContext, n.Name, types.NetworkCreate{ + _, err := e.client.NetworkCreate(ctx, n.Name, types.NetworkCreate{ Driver: networkDriver, EnableIPv6: e.enableIPv6, }) @@ -311,27 +311,27 @@ func (e *docker) DestroyStep(ctx context.Context, step *backend.Step, taskUUID s return nil } -func (e *docker) DestroyWorkflow(_ context.Context, conf *backend.Config, taskUUID string) error { +func (e *docker) DestroyWorkflow(ctx context.Context, conf *backend.Config, taskUUID string) error { log.Trace().Str("taskUUID", taskUUID).Msgf("delete workflow environment") for _, stage := range conf.Stages { for _, step := range stage.Steps { containerName := toContainerName(step) - if err := e.client.ContainerKill(noContext, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) { + if err := e.client.ContainerKill(ctx, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) { log.Error().Err(err).Msgf("could not kill container '%s'", stage.Name) } - if err := e.client.ContainerRemove(noContext, containerName, removeOpts); err != nil && !isErrContainerNotFoundOrNotRunning(err) { + if err := e.client.ContainerRemove(ctx, containerName, removeOpts); err != nil && !isErrContainerNotFoundOrNotRunning(err) { log.Error().Err(err).Msgf("could not remove container '%s'", stage.Name) } } } for _, v := range conf.Volumes { - if err := e.client.VolumeRemove(noContext, v.Name, true); err != nil { + if err := e.client.VolumeRemove(ctx, v.Name, true); err != nil { log.Error().Err(err).Msgf("could not remove volume '%s'", v.Name) } } for _, n := range conf.Networks { - if err := e.client.NetworkRemove(noContext, n.Name); err != nil { + if err := e.client.NetworkRemove(ctx, n.Name); err != nil { log.Error().Err(err).Msgf("could not remove network '%s'", n.Name) } } @@ -339,8 +339,6 @@ func (e *docker) DestroyWorkflow(_ context.Context, conf *backend.Config, taskUU } var ( - noContext = context.Background() - startOpts = types.ContainerStartOptions{} removeOpts = types.ContainerRemoveOptions{ diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index c8accfb698..6e9c55b528 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -48,7 +48,6 @@ const ( var defaultDeleteOptions = newDefaultDeleteOptions() type kube struct { - ctx context.Context client kubernetes.Interface config *config goos string @@ -112,10 +111,8 @@ func configFromCliContext(ctx context.Context) (*config, error) { } // New returns a new Kubernetes Backend. -func New(ctx context.Context) types.Backend { - return &kube{ - ctx: ctx, - } +func New() types.Backend { + return &kube{} } func (e *kube) Name() string { @@ -127,8 +124,8 @@ func (e *kube) IsAvailable(context.Context) bool { return len(host) > 0 } -func (e *kube) Load(context.Context) (*types.BackendInfo, error) { - config, err := configFromCliContext(e.ctx) +func (e *kube) Load(ctx context.Context) (*types.BackendInfo, error) { + config, err := configFromCliContext(ctx) if err != nil { return nil, err } @@ -210,7 +207,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) finished := make(chan bool) podUpdated := func(old, new any) { - pod := new.(*v1.Pod) + pod, _ := new.(*v1.Pod) if pod.Name == podName { if isImagePullBackOffState(pod) { finished <- true @@ -246,7 +243,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } if isImagePullBackOffState(pod) { - return nil, fmt.Errorf("Could not pull image for pod %s", pod.Name) + return nil, fmt.Errorf("could not pull image for pod %s", pod.Name) } bs := &types.State{ @@ -270,7 +267,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) up := make(chan bool) podUpdated := func(old, new any) { - pod := new.(*v1.Pod) + pod, _ := new.(*v1.Pod) if pod.Name == podName { switch pod.Status.Phase { case v1.PodRunning, v1.PodSucceeded, v1.PodFailed: @@ -329,26 +326,26 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) // return rc, nil } -func (e *kube) DestroyStep(_ context.Context, step *types.Step, taskUUID string) error { +func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID string) error { log.Trace().Str("taskUUID", taskUUID).Msgf("Stopping step: %s", step.Name) - err := stopPod(e.ctx, e, step, defaultDeleteOptions) + err := stopPod(ctx, e, step, defaultDeleteOptions) return err } // Destroy the pipeline environment. -func (e *kube) DestroyWorkflow(_ context.Context, conf *types.Config, taskUUID string) error { +func (e *kube) DestroyWorkflow(ctx context.Context, conf *types.Config, taskUUID string) error { log.Trace().Str("taskUUID", taskUUID).Msg("Deleting Kubernetes primitives") // Use noContext because the ctx sent to this function will be canceled/done in case of error or canceled by user. for _, stage := range conf.Stages { for _, step := range stage.Steps { - err := stopPod(e.ctx, e, step, defaultDeleteOptions) + err := stopPod(ctx, e, step, defaultDeleteOptions) if err != nil { return err } if step.Type == types.StepTypeService { - err := stopService(e.ctx, e, step, defaultDeleteOptions) + err := stopService(ctx, e, step, defaultDeleteOptions) if err != nil { return err } @@ -357,7 +354,7 @@ func (e *kube) DestroyWorkflow(_ context.Context, conf *types.Config, taskUUID s } for _, vol := range conf.Volumes { - err := stopVolume(e.ctx, e, vol.Name, defaultDeleteOptions) + err := stopVolume(ctx, e, vol.Name, defaultDeleteOptions) if err != nil { return err } 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/backend/local/clone.go b/pipeline/backend/local/clone.go index 083c018207..5642ed2c72 100644 --- a/pipeline/backend/local/clone.go +++ b/pipeline/backend/local/clone.go @@ -29,6 +29,8 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" ) +const osWindows = "windows" + // checkGitCloneCap check if we have the git binary on hand func checkGitCloneCap() error { _, err := exec.LookPath("git") @@ -54,7 +56,7 @@ func (e *local) setupClone(state *workflowState) error { log.Info().Msg("no global 'plugin-git' installed, try to download for current workflow") state.pluginGitBinary = filepath.Join(state.homeDir, "plugin-git") - if e.os == "windows" { + if e.os == osWindows { state.pluginGitBinary += ".exe" } return e.downloadLatestGitPluginBinary(state.pluginGitBinary) @@ -87,7 +89,7 @@ func (e *local) execClone(ctx context.Context, step *types.Step, state *workflow var cmd *exec.Cmd if rmCmd != "" { // if we have a netrc injected we have to make sure it's deleted in any case after clone was attempted - if e.os == "windows" { + if e.os == osWindows { pwsh, err := exec.LookPath("powershell.exe") if err != nil { return err @@ -121,7 +123,7 @@ func (e *local) writeNetRC(step *types.Step, state *workflowState) (string, erro file := filepath.Join(state.homeDir, ".netrc") rmCmd := fmt.Sprintf("rm \"%s\"", file) - if e.os == "windows" { + if e.os == osWindows { file = filepath.Join(state.homeDir, "_netrc") rmCmd = fmt.Sprintf("del \"%s\"", file) } diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index 8b3cd52312..568e203d74 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -257,7 +257,9 @@ func (e *local) getState(taskUUID string) (*workflowState, error) { if !ok { return nil, ErrWorkflowStateNotFound } - return state.(*workflowState), nil + + workflowState, _ := state.(*workflowState) + return workflowState, nil } func (e *local) saveState(taskUUID string, state *workflowState) { 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..9502919f26 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/pipeline/frontend/yaml/constraint/constraint.go @@ -195,7 +195,8 @@ func (c *Constraint) Match(m metadata.Metadata, global bool, env map[string]stri if err != nil { return false, err } - match = match && result.(bool) + bresult, _ := result.(bool) + match = match && bresult } return match, nil @@ -261,7 +262,7 @@ func (c *List) UnmarshalYAML(value *yaml.Node) error { if err1 != nil && err2 != nil { y, _ := yaml.Marshal(value) - return fmt.Errorf("Could not parse condition: %s: %w", y, multierr.Append(err1, err2)) + return fmt.Errorf("could not parse condition: %s: %w", y, multierr.Append(err1, err2)) } return nil @@ -342,7 +343,7 @@ func (c *Path) UnmarshalYAML(value *yaml.Node) error { if err1 != nil && err2 != nil { y, _ := yaml.Marshal(value) - return fmt.Errorf("Could not parse condition: %s", y) + return fmt.Errorf("could not parse condition: %s", y) } return nil diff --git a/pipeline/frontend/yaml/linter/schema/schema.go b/pipeline/frontend/yaml/linter/schema/schema.go index da24cab37b..fae8636279 100644 --- a/pipeline/frontend/yaml/linter/schema/schema.go +++ b/pipeline/frontend/yaml/linter/schema/schema.go @@ -36,29 +36,29 @@ func Lint(r io.Reader) ([]gojsonschema.ResultError, error) { // read yaml config rBytes, err := io.ReadAll(r) if err != nil { - return nil, fmt.Errorf("Failed to load yml file %w", err) + return nil, fmt.Errorf("failed to load yml file %w", err) } // resolve sequence merges yamlDoc := new(yaml.Node) if err := xyaml.Unmarshal(rBytes, yamlDoc); err != nil { - return nil, fmt.Errorf("Failed to parse yml file %w", err) + return nil, fmt.Errorf("failed to parse yml file %w", err) } // convert to json jsonDoc, err := yaml2json.ConvertNode(yamlDoc) if err != nil { - return nil, fmt.Errorf("Failed to convert yaml %w", err) + return nil, fmt.Errorf("failed to convert yaml %w", err) } documentLoader := gojsonschema.NewBytesLoader(jsonDoc) result, err := gojsonschema.Validate(schemaLoader, documentLoader) if err != nil { - return nil, fmt.Errorf("Validation failed %w", err) + return nil, fmt.Errorf("validation failed %w", err) } if !result.Valid() { - return result.Errors(), fmt.Errorf("Config not valid") + return result.Errors(), fmt.Errorf("config not valid") } return nil, nil 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 f919b58141..49ddc0b8a2 100644 --- a/pipeline/frontend/yaml/parse.go +++ b/pipeline/frontend/yaml/parse.go @@ -46,11 +46,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/pipeline/frontend/yaml/types/base/int.go b/pipeline/frontend/yaml/types/base/int.go index e4ff93b792..26565f2d2e 100644 --- a/pipeline/frontend/yaml/types/base/int.go +++ b/pipeline/frontend/yaml/types/base/int.go @@ -42,7 +42,7 @@ func (s *StringOrInt) UnmarshalYAML(unmarshal func(any) error) error { return nil } - return errors.New("Failed to unmarshal StringOrInt") + return errors.New("failed to unmarshal StringOrInt") } // MemStringOrInt represents a string or an integer @@ -67,5 +67,5 @@ func (s *MemStringOrInt) UnmarshalYAML(unmarshal func(any) error) error { return nil } - return errors.New("Failed to unmarshal MemStringOrInt") + return errors.New("failed to unmarshal MemStringOrInt") } diff --git a/pipeline/frontend/yaml/types/base/map.go b/pipeline/frontend/yaml/types/base/map.go index 3102ef1b30..e25a65b04e 100644 --- a/pipeline/frontend/yaml/types/base/map.go +++ b/pipeline/frontend/yaml/types/base/map.go @@ -40,7 +40,7 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { } parts[key] = val } else { - return fmt.Errorf("Cannot unmarshal '%v' of type %T into a string value", s, s) + return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", s, s) } } *s = parts @@ -55,15 +55,15 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { if sv, ok := v.(string); ok { parts[sk] = sv } else { - return fmt.Errorf("Cannot unmarshal '%v' of type %T into a string value", v, v) + return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", v, v) } } else { - return fmt.Errorf("Cannot unmarshal '%v' of type %T into a string value", k, k) + return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", k, k) } } *s = parts return nil } - return errors.New("Failed to unmarshal SliceOrMap") + return errors.New("failed to unmarshal SliceOrMap") } diff --git a/pipeline/frontend/yaml/types/base/slice.go b/pipeline/frontend/yaml/types/base/slice.go index 7c964ace68..90f33fb925 100644 --- a/pipeline/frontend/yaml/types/base/slice.go +++ b/pipeline/frontend/yaml/types/base/slice.go @@ -41,7 +41,7 @@ func (s *StringOrSlice) UnmarshalYAML(unmarshal func(any) error) error { return nil } - return errors.New("Failed to unmarshal StringOrSlice") + return errors.New("failed to unmarshal StringOrSlice") } func toStrings(s []any) ([]string, error) { @@ -53,7 +53,7 @@ func toStrings(s []any) ([]string, error) { if sv, ok := v.(string); ok { r[k] = sv } else { - return nil, fmt.Errorf("Cannot unmarshal '%v' of type %T into a string value", v, v) + return nil, fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", v, v) } } return r, nil diff --git a/pipeline/frontend/yaml/types/network.go b/pipeline/frontend/yaml/types/network.go index fe2798f996..8e21ed0a9b 100644 --- a/pipeline/frontend/yaml/types/network.go +++ b/pipeline/frontend/yaml/types/network.go @@ -50,7 +50,7 @@ func (n *Networks) UnmarshalYAML(unmarshal func(any) error) error { for _, network := range sliceType { name, ok := network.(string) if !ok { - return fmt.Errorf("Cannot unmarshal '%v' to type %T into a string value", name, name) + return fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", name, name) } n.Networks = append(n.Networks, &Network{ Name: name, @@ -65,7 +65,7 @@ func (n *Networks) UnmarshalYAML(unmarshal func(any) error) error { for mapKey, mapValue := range mapType { name, ok := mapKey.(string) if !ok { - return fmt.Errorf("Cannot unmarshal '%v' to type %T into a string value", name, name) + return fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", name, name) } network, err := handleNetwork(name, mapValue) if err != nil { @@ -76,7 +76,7 @@ func (n *Networks) UnmarshalYAML(unmarshal func(any) error) error { return nil } - return errors.New("Failed to unmarshal Networks") + return errors.New("failed to unmarshal networks") } func handleNetwork(name string, value any) (*Network, error) { @@ -95,16 +95,17 @@ func handleNetwork(name string, value any) (*Network, error) { case "aliases": aliases, ok := mapValue.([]any) if !ok { - return &Network{}, fmt.Errorf("Cannot unmarshal '%v' to type %T into a string value", aliases, aliases) + return &Network{}, fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", aliases, aliases) } network.Aliases = []string{} for _, alias := range aliases { - network.Aliases = append(network.Aliases, alias.(string)) + a, _ := alias.(string) + network.Aliases = append(network.Aliases, a) } case "ipv4_address": - network.IPv4Address = mapValue.(string) + network.IPv4Address, _ = mapValue.(string) case "ipv6_address": - network.IPv6Address = mapValue.(string) + network.IPv6Address, _ = mapValue.(string) default: // Ignorer unknown keys ? continue @@ -112,6 +113,6 @@ func handleNetwork(name string, value any) (*Network, error) { } return network, nil default: - return &Network{}, fmt.Errorf("Failed to unmarshal Network: %#v", value) + return &Network{}, fmt.Errorf("failed to unmarshal Network: %#v", value) } } diff --git a/pipeline/frontend/yaml/types/volume.go b/pipeline/frontend/yaml/types/volume.go index 7dc7ec2323..d07a3eec60 100644 --- a/pipeline/frontend/yaml/types/volume.go +++ b/pipeline/frontend/yaml/types/volume.go @@ -64,7 +64,7 @@ func (v *Volumes) UnmarshalYAML(unmarshal func(any) error) error { for _, volume := range sliceType { name, ok := volume.(string) if !ok { - return fmt.Errorf("Cannot unmarshal '%v' to type %T into a string value", name, name) + return fmt.Errorf("cannot unmarshal '%v' to type %T into a string value", name, name) } elts := strings.SplitN(name, ":", 3) var vol *Volume @@ -93,5 +93,5 @@ func (v *Volumes) UnmarshalYAML(unmarshal func(any) error) error { return nil } - return errors.New("Failed to unmarshal Volumes") + return errors.New("failed to unmarshal Volumes") } diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 4c0ef2106f..5c7184fc60 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -112,7 +112,7 @@ func (r *Runtime) Run(runnerCtx context.Context) error { }() r.started = time.Now().Unix() - if err := r.engine.SetupWorkflow(r.ctx, r.spec, r.taskUUID); err != nil { + if err := r.engine.SetupWorkflow(runnerCtx, r.spec, r.taskUUID); err != nil { return err } diff --git a/pipeline/stepBuilder.go b/pipeline/stepBuilder.go index 84c5284753..7d654757f2 100644 --- a/pipeline/stepBuilder.go +++ b/pipeline/stepBuilder.go @@ -26,7 +26,6 @@ import ( backend_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" - pipeline_errors "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" yaml_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/types" forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" @@ -89,7 +88,7 @@ func (b *StepBuilder) Build() (items []*Item, errorsAndWarnings error) { workflow.AxisID = i + 1 } item, err := b.genItemForWorkflow(workflow, axis, string(y.Data)) - if err != nil && pipeline_errors.HasBlockingErrors(err) { + if err != nil && errors.HasBlockingErrors(err) { return nil, err } else if err != nil { errorsAndWarnings = multierr.Append(errorsAndWarnings, err) @@ -149,7 +148,7 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A File: workflow.Name, RawConfig: data, }})) - if pipeline_errors.HasBlockingErrors(errorsAndWarnings) { + if errors.HasBlockingErrors(errorsAndWarnings) { return nil, errorsAndWarnings } diff --git a/server/api/agent.go b/server/api/agent.go index 36bbb04290..3800ff5763 100644 --- a/server/api/agent.go +++ b/server/api/agent.go @@ -65,7 +65,7 @@ func GetAgent(c *gin.Context) { agent, err := store.FromContext(c).AgentFind(agentID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(http.StatusOK, agent) @@ -89,7 +89,7 @@ func GetAgentTasks(c *gin.Context) { agent, err := store.FromContext(c).AgentFind(agentID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -132,7 +132,7 @@ func PatchAgent(c *gin.Context) { agent, err := _store.AgentFind(agentID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } agent.Name = in.Name @@ -201,7 +201,7 @@ func DeleteAgent(c *gin.Context) { agent, err := _store.AgentFind(agentID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if err = _store.AgentDelete(agent); err != nil { diff --git a/server/api/badge.go b/server/api/badge.go index 152fbef6c5..4f3c1f1ef5 100644 --- a/server/api/badge.go +++ b/server/api/badge.go @@ -62,7 +62,7 @@ func GetBadge(c *gin.Context) { } if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -117,7 +117,7 @@ func GetCC(c *gin.Context) { } if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } diff --git a/server/api/cron.go b/server/api/cron.go index f1f1a48a31..c10327de24 100644 --- a/server/api/cron.go +++ b/server/api/cron.go @@ -49,7 +49,7 @@ func GetCron(c *gin.Context) { cron, err := store.FromContext(c).CronFind(repo, id) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(http.StatusOK, cron) @@ -76,7 +76,7 @@ func RunCron(c *gin.Context) { cron, err := _store.CronFind(repo, id) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -183,7 +183,7 @@ func PatchCron(c *gin.Context) { cron, err := _store.CronFind(repo, id) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if in.Branch != "" { @@ -259,7 +259,7 @@ func DeleteCron(c *gin.Context) { return } if err := store.FromContext(c).CronDelete(repo, id); err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.Status(http.StatusNoContent) diff --git a/server/api/global_secret.go b/server/api/global_secret.go index d6f992811b..ed26785007 100644 --- a/server/api/global_secret.go +++ b/server/api/global_secret.go @@ -62,7 +62,7 @@ func GetGlobalSecret(c *gin.Context) { name := c.Param("secret") secret, err := server.Config.Services.Secrets.GlobalSecretFind(name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(http.StatusOK, secret.Copy()) @@ -122,7 +122,7 @@ func PatchGlobalSecret(c *gin.Context) { secret, err := server.Config.Services.Secrets.GlobalSecretFind(name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if in.Value != "" { @@ -158,7 +158,7 @@ func PatchGlobalSecret(c *gin.Context) { func DeleteGlobalSecret(c *gin.Context) { name := c.Param("secret") if err := server.Config.Services.Secrets.GlobalSecretDelete(name); err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.Status(http.StatusNoContent) diff --git a/server/api/helper.go b/server/api/helper.go index cfe409b9b6..51646e370d 100644 --- a/server/api/helper.go +++ b/server/api/helper.go @@ -29,20 +29,21 @@ 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) } } -func handleDbError(c *gin.Context, err error) { +func handleDBError(c *gin.Context, err error) { if errors.Is(err, types.RecordNotExist) { c.AbortWithStatus(http.StatusNotFound) return diff --git a/server/api/hook.go b/server/api/hook.go index 0d3a9b53d8..0c67ab35c2 100644 --- a/server/api/hook.go +++ b/server/api/hook.go @@ -144,7 +144,7 @@ func PostHook(c *gin.Context) { repo, err := _store.GetRepoNameFallback(tmpRepo.ForgeRemoteID, tmpRepo.FullName) if err != nil { log.Error().Err(err).Msgf("failure to get repo %s from store", tmpRepo.FullName) - handleDbError(c, err) + handleDBError(c, err) return } if !repo.IsActive { diff --git a/server/api/login.go b/server/api/login.go index 29d25d7b1a..4a0e53cc26 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -208,7 +208,7 @@ func GetLoginToken(c *gin.Context) { user, err := _store.GetUserLogin(login) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } diff --git a/server/api/metrics/prometheus.go b/server/api/metrics/prometheus.go index a7a7793b5e..1b68f7151f 100644 --- a/server/api/metrics/prometheus.go +++ b/server/api/metrics/prometheus.go @@ -26,7 +26,7 @@ import ( ) // errInvalidToken is returned when the api request token is invalid. -var errInvalidToken = errors.New("Invalid or missing token") +var errInvalidToken = errors.New("invalid or missing token") // PromHandler will pass the call from /api/metrics/prometheus to prometheus func PromHandler() gin.HandlerFunc { diff --git a/server/api/org.go b/server/api/org.go index e3b5999990..b304c5c038 100644 --- a/server/api/org.go +++ b/server/api/org.go @@ -49,7 +49,7 @@ func GetOrg(c *gin.Context) { org, err := _store.OrgGet(orgID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -122,7 +122,7 @@ func LookupOrg(c *gin.Context) { org, err := _store.OrgFindByName(orgFullName) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } diff --git a/server/api/org_secret.go b/server/api/org_secret.go index dbda4c4a50..2a8a219586 100644 --- a/server/api/org_secret.go +++ b/server/api/org_secret.go @@ -47,7 +47,7 @@ func GetOrgSecret(c *gin.Context) { secret, err := server.Config.Services.Secrets.OrgSecretFind(orgID, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(http.StatusOK, secret.Copy()) @@ -152,7 +152,7 @@ func PatchOrgSecret(c *gin.Context) { secret, err := server.Config.Services.Secrets.OrgSecretFind(orgID, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if in.Value != "" { @@ -195,7 +195,7 @@ func DeleteOrgSecret(c *gin.Context) { } if err := server.Config.Services.Secrets.OrgSecretDelete(orgID, name); err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.Status(http.StatusNoContent) diff --git a/server/api/orgs.go b/server/api/orgs.go index e54f77eb15..334c91c989 100644 --- a/server/api/orgs.go +++ b/server/api/orgs.go @@ -65,7 +65,7 @@ func DeleteOrg(c *gin.Context) { err = _store.OrgDelete(orgID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } diff --git a/server/api/pipeline.go b/server/api/pipeline.go index 6277fdf4b0..dd2824f9fc 100644 --- a/server/api/pipeline.go +++ b/server/api/pipeline.go @@ -140,7 +140,7 @@ func GetPipeline(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if pl.Workflows, err = _store.WorkflowGetTree(pl); err != nil { @@ -158,7 +158,7 @@ func GetPipelineLast(c *gin.Context) { pl, err := _store.GetPipelineLast(repo, branch) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -194,7 +194,7 @@ func GetStepLogs(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -206,7 +206,7 @@ func GetStepLogs(c *gin.Context) { step, err := _store.StepLoad(stepID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -218,7 +218,7 @@ func GetStepLogs(c *gin.Context) { logs, err := _store.LogFind(step) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -246,7 +246,7 @@ func GetPipelineConfig(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -277,7 +277,7 @@ func CancelPipeline(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -308,7 +308,7 @@ func PostApproval(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -340,7 +340,7 @@ func PostDecline(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -395,13 +395,13 @@ func PostPipeline(c *gin.Context) { user, err := _store.GetUser(repo.UserID) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -468,7 +468,7 @@ func DeletePipelineLogs(c *gin.Context) { pl, err := _store.GetPipelineNumber(repo, num) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } diff --git a/server/api/registry.go b/server/api/registry.go index 731084ab72..38b01d2c68 100644 --- a/server/api/registry.go +++ b/server/api/registry.go @@ -41,7 +41,7 @@ func GetRegistry(c *gin.Context) { ) registry, err := server.Config.Services.Registries.RegistryFind(repo, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(200, registry.Copy()) @@ -110,7 +110,7 @@ func PatchRegistry(c *gin.Context) { registry, err := server.Config.Services.Registries.RegistryFind(repo, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if in.Username != "" { @@ -180,7 +180,7 @@ func DeleteRegistry(c *gin.Context) { ) err := server.Config.Services.Registries.RegistryDelete(repo, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.Status(http.StatusNoContent) diff --git a/server/api/repo.go b/server/api/repo.go index 42232faec2..74f2bb2c69 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -401,7 +401,7 @@ func DeleteRepo(c *gin.Context) { if remove { if err := _store.DeleteRepo(repo); err != nil { - handleDbError(c, err) + handleDBError(c, err) return } } diff --git a/server/api/repo_secret.go b/server/api/repo_secret.go index 84fe4ca7b8..235e6ae05d 100644 --- a/server/api/repo_secret.go +++ b/server/api/repo_secret.go @@ -42,7 +42,7 @@ func GetSecret(c *gin.Context) { ) secret, err := server.Config.Services.Secrets.SecretFind(repo, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(http.StatusOK, secret.Copy()) @@ -110,7 +110,7 @@ func PatchSecret(c *gin.Context) { secret, err := server.Config.Services.Secrets.SecretFind(repo, name) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if in.Value != "" { @@ -176,7 +176,7 @@ func DeleteSecret(c *gin.Context) { name = c.Param("secret") ) if err := server.Config.Services.Secrets.SecretDelete(repo, name); err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.Status(http.StatusNoContent) diff --git a/server/api/user.go b/server/api/user.go index d9c06d43fd..c3f540f7e0 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/api/users.go b/server/api/users.go index 5769d4c027..86ec1debfe 100644 --- a/server/api/users.go +++ b/server/api/users.go @@ -59,7 +59,7 @@ func GetUsers(c *gin.Context) { func GetUser(c *gin.Context) { user, err := store.FromContext(c).GetUserLogin(c.Param("login")) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.JSON(http.StatusOK, user) @@ -89,7 +89,7 @@ func PatchUser(c *gin.Context) { user, err := _store.GetUserLogin(c.Param("login")) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } @@ -159,11 +159,11 @@ func DeleteUser(c *gin.Context) { user, err := _store.GetUserLogin(c.Param("login")) if err != nil { - handleDbError(c, err) + handleDBError(c, err) return } if err = _store.DeleteUser(user); err != nil { - handleDbError(c, err) + handleDBError(c, err) return } c.Status(http.StatusNoContent) diff --git a/server/badges/badges_test.go b/server/badges/badges_test.go index d8953c37de..fdbefe1957 100644 --- a/server/badges/badges_test.go +++ b/server/badges/badges_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index c1df1efad6..8976d9602a 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -38,6 +38,7 @@ import ( const ( DefaultAPI = "https://api.bitbucket.org" DefaultURL = "https://bitbucket.org" + pageSize = 100 ) // Opts are forge options for bitbucket @@ -142,7 +143,7 @@ func (c *config) Refresh(ctx context.Context, user *model.User) (bool, error) { func (c *config) Teams(ctx context.Context, u *model.User) ([]*model.Team, error) { return shared_utils.Paginate(func(page int) ([]*model.Team, error) { opts := &internal.ListWorkspacesOpts{ - PageLen: 100, + PageLen: pageSize, Page: page, Role: "member", } @@ -191,7 +192,7 @@ func (c *config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error workspaces, err := shared_utils.Paginate(func(page int) ([]*internal.Workspace, error) { resp, err := client.ListWorkspaces(&internal.ListWorkspacesOpts{ Page: page, - PageLen: 100, + PageLen: pageSize, Role: "member", }) if err != nil { diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go index 707a6dd732..dffa9a5891 100644 --- a/server/forge/bitbucket/fixtures/handler.go +++ b/server/forge/bitbucket/fixtures/handler.go @@ -44,28 +44,27 @@ func Handler() http.Handler { } func getOauth(c *gin.Context) { - switch c.PostForm("error") { - case "invalid_scope": - c.String(500, "") + if c.PostForm("error") == "invalid_scope" { + c.String(http.StatusInternalServerError, "") } switch c.PostForm("code") { case "code_bad_request": - c.String(500, "") + c.String(http.StatusInternalServerError, "") return case "code_user_not_found": - c.String(200, tokenNotFoundPayload) + c.String(http.StatusOK, tokenNotFoundPayload) return } switch c.PostForm("refresh_token") { case "refresh_token_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "refresh_token_is_empty": c.Header("Content-Type", "application/json") - c.String(200, "{}") + c.String(http.StatusOK, "{}") default: c.Header("Content-Type", "application/json") - c.String(200, tokenPayload) + c.String(http.StatusOK, tokenPayload) } } @@ -75,12 +74,12 @@ func getWorkspaces(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer teams_not_found", "Bearer c81e728d": - c.String(404, "") + c.String(http.StatusNotFound, "") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, workspacesPayload) + c.String(http.StatusOK, workspacesPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -88,25 +87,25 @@ func getWorkspaces(c *gin.Context) { func getRepo(c *gin.Context) { switch c.Param("name") { case "not_found", "repo_unknown", "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "permission_read", "permission_write", "permission_admin": - c.String(200, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) + c.String(http.StatusOK, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoHooks(c *gin.Context) { switch c.Param("name") { case "hooks_not_found", "repo_no_hooks": - c.String(404, "") + c.String(http.StatusNotFound, "") case "hook_empty": - c.String(200, "{}") + c.String(http.StatusOK, "{}") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, repoHookPayload) + c.String(http.StatusOK, repoHookPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -114,74 +113,74 @@ func getRepoHooks(c *gin.Context) { func getRepoFile(c *gin.Context) { switch c.Param("file") { case "dir": - c.String(200, repoDirPayload) + c.String(http.StatusOK, repoDirPayload) case "dir_not_found/": - c.String(404, "") + c.String(http.StatusNotFound, "") case "file_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoFilePayload) + c.String(http.StatusOK, repoFilePayload) } } func getBranchHead(c *gin.Context) { switch c.Param("commit") { case "branch_name": - c.String(200, branchCommitsPayload) + c.String(http.StatusOK, branchCommitsPayload) default: - c.String(404, "") + c.String(http.StatusNotFound, "") } } func getPullRequests(c *gin.Context) { switch c.Param("name") { case "repo_name": - c.String(200, pullRequestsPayload) + c.String(http.StatusOK, pullRequestsPayload) default: - c.String(404, "") + c.String(http.StatusNotFound, "") } } func createRepoStatus(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, "") + c.String(http.StatusOK, "") } } func createRepoHook(c *gin.Context) { - c.String(200, "") + c.String(http.StatusOK, "") } func deleteRepoHook(c *gin.Context) { switch c.Param("name") { case "hook_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, "") + c.String(http.StatusOK, "") } } func getUser(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer user_not_found", "Bearer a87ff679": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, userPayload) + c.String(http.StatusOK, userPayload) } } func getUserRepos(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer repos_not_found", "Bearer 70efdf2e": - c.String(404, "") + c.String(http.StatusNotFound, "") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, userRepoPayload) + c.String(http.StatusOK, userRepoPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -194,13 +193,13 @@ func getPermissions(c *gin.Context) { query := c.Request.URL.Query()["q"][0] switch query { case `repository.full_name="test_name/permission_read"`: - c.String(200, permission("read")) + c.String(http.StatusOK, permission("read")) case `repository.full_name="test_name/permission_write"`: - c.String(200, permission("write")) + c.String(http.StatusOK, permission("write")) case `repository.full_name="test_name/permission_admin"`: - c.String(200, permission("admin")) + c.String(http.StatusOK, permission("admin")) default: - c.String(200, permission("read")) + c.String(http.StatusOK, permission("read")) } } diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go index 8d832e8c16..e9dda42eea 100644 --- a/server/forge/bitbucket/internal/client.go +++ b/server/forge/bitbucket/internal/client.go @@ -52,6 +52,7 @@ const ( pathPullRequests = "%s/2.0/repositories/%s/%s/pullrequests" pathBranchCommits = "%s/2.0/repositories/%s/%s/commits/%s" pathDir = "%s/2.0/repositories/%s/%s/src/%s%s" + pageSize = 100 ) type Client struct { @@ -114,7 +115,7 @@ func (c *Client) ListRepos(workspace string, opts *ListOpts) (*RepoResp, error) func (c *Client) ListReposAll(workspace string) ([]*Repo, error) { return shared_utils.Paginate(func(page int) ([]*Repo, error) { - resp, err := c.ListRepos(workspace, &ListOpts{Page: page, PageLen: 100}) + resp, err := c.ListRepos(workspace, &ListOpts{Page: page, PageLen: pageSize}) if err != nil { return nil, err } @@ -195,7 +196,7 @@ func (c *Client) GetBranchHead(owner, name, branch string) (string, error) { func (c *Client) GetUserWorkspaceMembership(workspace, user string) (string, error) { out := new(WorkspaceMembershipResp) - opts := &ListOpts{Page: 1, PageLen: 100} + opts := &ListOpts{Page: 1, PageLen: pageSize} for { uri := fmt.Sprintf(pathOrgPerms, c.base, workspace, opts.Encode()) _, err := c.do(uri, get, nil, out) 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/fixtures/handler.go b/server/forge/gitea/fixtures/handler.go index d6ff48fa1a..7bc2bbb42a 100644 --- a/server/forge/gitea/fixtures/handler.go +++ b/server/forge/gitea/fixtures/handler.go @@ -43,35 +43,35 @@ func Handler() http.Handler { func listRepoHooks(c *gin.Context) { page := c.Query("page") if page != "" && page != "1" { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } else { - c.String(200, listRepoHookPayloads) + c.String(http.StatusOK, listRepoHookPayloads) } } func getRepo(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoByID(c *gin.Context) { switch c.Param("id") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func createRepoCommitStatus(c *gin.Context) { if c.Param("commit") == "v1.0.0" || c.Param("commit") == "9ecad50" { - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } - c.String(404, "") + c.String(http.StatusNotFound, "") } func getRepoFile(c *gin.Context) { @@ -79,12 +79,12 @@ func getRepoFile(c *gin.Context) { ref := c.Query("ref") if file == "file_not_found" { - c.String(404, "") + c.String(http.StatusNotFound, "") } if ref == "v1.0.0" || ref == "9ecad50" { - c.String(200, repoFilePayload) + c.String(http.StatusOK, repoFilePayload) } - c.String(404, "") + c.String(http.StatusNotFound, "") } func createRepoHook(c *gin.Context) { @@ -99,41 +99,41 @@ func createRepoHook(c *gin.Context) { if in.Type != "gitea" || in.Conf.Type != "json" || in.Conf.URL != "http://localhost" { - c.String(500, "") + c.String(http.StatusInternalServerError, "") return } - c.String(200, "{}") + c.String(http.StatusOK, "{}") } func deleteRepoHook(c *gin.Context) { - c.String(200, "{}") + c.String(http.StatusOK, "{}") } func getUserRepos(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "token repos_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: page := c.Query("page") if page != "" && page != "1" { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } else { - c.String(200, userRepoPayload) + c.String(http.StatusOK, userRepoPayload) } } } func getVersion(c *gin.Context) { - c.JSON(200, map[string]any{"version": "1.18.0"}) + c.JSON(http.StatusOK, map[string]any{"version": "1.18.0"}) } func getPRFiles(c *gin.Context) { page := c.Query("page") if page == "1" { - c.String(200, prFilesPayload) + c.String(http.StatusOK, prFilesPayload) } else { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } } diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index ae6a6c1d07..de739b01b8 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -401,11 +401,11 @@ func (c *Gitea) Activate(ctx context.Context, u *model.User, r *model.Repo, link _, response, err := client.CreateRepoHook(r.Owner, r.Name, hook) if err != nil { if response != nil { - if response.StatusCode == 404 { - return fmt.Errorf("Could not find repository") + if response.StatusCode == http.StatusNotFound { + return fmt.Errorf("could not find repository") } - if response.StatusCode == 200 { - return fmt.Errorf("Could not find repository, repository was probably renamed") + if response.StatusCode == http.StatusOK { + return fmt.Errorf("could not find repository, repository was probably renamed") } } return err diff --git a/server/forge/gitea/helper.go b/server/forge/gitea/helper.go index ce4709a5f7..d3a366684f 100644 --- a/server/forge/gitea/helper.go +++ b/server/forge/gitea/helper.go @@ -76,7 +76,7 @@ func pipelineFromPush(hook *pushHook) *model.Pipeline { fixMalformedAvatar(hook.Sender.AvatarURL), ) - message := "" + var message string link := hook.Compare if len(hook.Commits) > 0 { message = hook.Commits[0].Message @@ -189,7 +189,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/convert_test.go b/server/forge/github/convert_test.go index d98d8fedfd..e95704ef07 100644 --- a/server/forge/github/convert_test.go +++ b/server/forge/github/convert_test.go @@ -232,8 +232,7 @@ func Test_helper(t *testing.T) { from.Sender.Login = github.String("octocat") from.Sender.AvatarURL = github.String("https://avatars1.githubusercontent.com/u/583231") - _, pipeline, err := parseDeployHook(from) - g.Assert(err).IsNil() + _, pipeline := parseDeployHook(from) g.Assert(pipeline.Event).Equal(model.EventDeploy) g.Assert(pipeline.Branch).Equal("main") g.Assert(pipeline.Ref).Equal("refs/heads/main") @@ -255,8 +254,7 @@ func Test_helper(t *testing.T) { from.HeadCommit.ID = github.String("f72fc19") from.Ref = github.String("refs/heads/main") - _, pipeline, err := parsePushHook(from) - g.Assert(err).IsNil() + _, pipeline := parsePushHook(from) g.Assert(pipeline.Event).Equal(model.EventPush) g.Assert(pipeline.Branch).Equal("main") g.Assert(pipeline.Ref).Equal("refs/heads/main") @@ -273,8 +271,7 @@ func Test_helper(t *testing.T) { from := &github.PushEvent{} from.Ref = github.String("refs/tags/v1.0.0") - _, pipeline, err := parsePushHook(from) - g.Assert(err).IsNil() + _, pipeline := parsePushHook(from) g.Assert(pipeline.Event).Equal(model.EventTag) g.Assert(pipeline.Ref).Equal("refs/tags/v1.0.0") }) @@ -284,8 +281,7 @@ func Test_helper(t *testing.T) { from.Ref = github.String("refs/tags/v1.0.0") from.BaseRef = github.String("refs/heads/main") - _, pipeline, err := parsePushHook(from) - g.Assert(err).IsNil() + _, pipeline := parsePushHook(from) g.Assert(pipeline.Event).Equal(model.EventTag) g.Assert(pipeline.Branch).Equal("main") }) @@ -295,8 +291,7 @@ func Test_helper(t *testing.T) { from.Ref = github.String("refs/tags/v1.0.0") from.BaseRef = github.String("refs/refs/main") - _, pipeline, err := parsePushHook(from) - g.Assert(err).IsNil() + _, pipeline := parsePushHook(from) g.Assert(pipeline.Event).Equal(model.EventTag) g.Assert(pipeline.Branch).Equal("refs/tags/v1.0.0") }) diff --git a/server/forge/github/fixtures/handler.go b/server/forge/github/fixtures/handler.go index 320805a517..0f02557053 100644 --- a/server/forge/github/fixtures/handler.go +++ b/server/forge/github/fixtures/handler.go @@ -37,29 +37,29 @@ func Handler() http.Handler { func getRepo(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoByID(c *gin.Context) { switch c.Param("id") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getMembership(c *gin.Context) { switch c.Param("org") { case "org_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "github": - c.String(200, membershipIsMemberPayload) + c.String(http.StatusOK, membershipIsMemberPayload) default: - c.String(200, membershipIsOwnerPayload) + c.String(http.StatusOK, membershipIsOwnerPayload) } } diff --git a/server/forge/github/github.go b/server/forge/github/github.go index 197afc73b4..de30a01e51 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -132,7 +132,7 @@ func (c *client) Login(ctx context.Context, res http.ResponseWriter, req *http.R } email := matchingEmail(emails, c.API) if email == nil { - return nil, fmt.Errorf("No verified Email address for GitHub account") + return nil, fmt.Errorf("no verified Email address for GitHub account") } return &model.User{ @@ -435,7 +435,8 @@ func (c *client) newClientToken(ctx context.Context, token string) *github.Clien ) tc := oauth2.NewClient(ctx, ts) if c.SkipVerify { - tc.Transport.(*oauth2.Transport).Base = &http.Transport{ + tp, _ := tc.Transport.(*oauth2.Transport) + tp.Base = &http.Transport{ Proxy: http.ProxyFromEnvironment, TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, diff --git a/server/forge/github/parse.go b/server/forge/github/parse.go index e1937b4d3d..11eb56e5c6 100644 --- a/server/forge/github/parse.go +++ b/server/forge/github/parse.go @@ -59,11 +59,11 @@ func parseHook(r *http.Request, merge bool) (*github.PullRequest, *model.Repo, * switch hook := payload.(type) { case *github.PushEvent: - repo, pipeline, err := parsePushHook(hook) - return nil, repo, pipeline, err + repo, pipeline := parsePushHook(hook) + return nil, repo, pipeline, nil case *github.DeploymentEvent: - repo, pipeline, err := parseDeployHook(hook) - return nil, repo, pipeline, err + repo, pipeline := parseDeployHook(hook) + return nil, repo, pipeline, nil case *github.PullRequestEvent: return parsePullHook(hook, merge) default: @@ -73,9 +73,9 @@ func parseHook(r *http.Request, merge bool) (*github.PullRequest, *model.Repo, * // parsePushHook parses a push hook and returns the Repo and Pipeline details. // If the commit type is unsupported nil values are returned. -func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline, error) { +func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline) { if hook.Deleted != nil && *hook.Deleted { - return nil, nil, nil + return nil, nil } pipeline := &model.Pipeline{ @@ -83,7 +83,7 @@ func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline, error) 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(), @@ -104,16 +104,16 @@ func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline, error) // 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/", "") } } - return convertRepoHook(hook.GetRepo()), pipeline, nil + return convertRepoHook(hook.GetRepo()), pipeline } // parseDeployHook parses a deployment and returns the Repo and Pipeline details. // If the commit type is unsupported nil values are returned. -func parseDeployHook(hook *github.DeploymentEvent) (*model.Repo, *model.Pipeline, error) { +func parseDeployHook(hook *github.DeploymentEvent) (*model.Repo, *model.Pipeline) { pipeline := &model.Pipeline{ Event: model.EventDeploy, Commit: hook.GetDeployment().GetSHA(), @@ -136,7 +136,7 @@ func parseDeployHook(hook *github.DeploymentEvent) (*model.Repo, *model.Pipeline pipeline.Ref = fmt.Sprintf("refs/heads/%s", pipeline.Branch) } - return convertRepo(hook.GetRepo()), pipeline, nil + return convertRepo(hook.GetRepo()), pipeline } // parsePullHook parses a pull request hook and returns the Repo and Pipeline diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index f93b3d6cd4..7dc88b8700 100644 --- a/server/forge/gitlab/convert.go +++ b/server/forge/gitlab/convert.go @@ -69,11 +69,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") } @@ -251,7 +252,7 @@ func getUserAvatar(email string) string { func extractFromPath(str string) (string, string, error) { s := strings.Split(str, "/") if len(s) < 2 { - return "", "", fmt.Errorf("Minimum match not found") + return "", "", fmt.Errorf("minimum match not found") } return s[0], s[1], nil } diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index 649f1c978c..8c5c8eb319 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -127,7 +127,7 @@ func (g *GitLab) Login(ctx context.Context, res http.ResponseWriter, req *http.R token, err := config.Exchange(oauth2Ctx, code) if err != nil { - return nil, fmt.Errorf("Error exchanging token. %w", err) + return nil, fmt.Errorf("error exchanging token: %w", err) } client, err := newClient(g.url, token.AccessToken, g.SkipVerify) diff --git a/server/forge/types/errors.go b/server/forge/types/errors.go index 957e30432c..fbb1fda938 100644 --- a/server/forge/types/errors.go +++ b/server/forge/types/errors.go @@ -54,7 +54,7 @@ func (err *ErrIgnoreEvent) Error() string { } func (*ErrIgnoreEvent) Is(target error) bool { - _, ok := target.(*ErrIgnoreEvent) //nolint:errorlint + _, ok := target.(*ErrIgnoreEvent) return ok } diff --git a/server/grpc/auth_server.go b/server/grpc/auth_server.go index 362e74b4c2..88ef145313 100644 --- a/server/grpc/auth_server.go +++ b/server/grpc/auth_server.go @@ -41,7 +41,7 @@ func NewWoodpeckerAuthServer(jwtManager *JWTManager, agentMasterToken string, st func (s *WoodpeckerAuthServer) Auth(_ context.Context, req *proto.AuthRequest) (*proto.AuthResponse, error) { agent, err := s.getAgent(req.AgentId, req.AgentToken) if err != nil { - return nil, fmt.Errorf("Agent could not auth: %w", err) + return nil, fmt.Errorf("agent could not auth: %w", err) } accessToken, err := s.jwtManager.Generate(agent.ID) diff --git a/server/grpc/filter.go b/server/grpc/filter.go index 30b34605cf..9cf2d87fae 100644 --- a/server/grpc/filter.go +++ b/server/grpc/filter.go @@ -20,7 +20,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/queue" ) -func createFilterFunc(agentFilter rpc.Filter) (queue.FilterFn, error) { +func createFilterFunc(agentFilter rpc.Filter) queue.FilterFn { return func(task *model.Task) bool { for taskLabel, taskLabelValue := range task.Labels { // if a task label is empty it will be ignored @@ -44,5 +44,5 @@ func createFilterFunc(agentFilter rpc.Filter) (queue.FilterFn, error) { } } return true - }, nil + } } diff --git a/server/grpc/filter_test.go b/server/grpc/filter_test.go index d3b06e91f0..5fbacc4acf 100644 --- a/server/grpc/filter_test.go +++ b/server/grpc/filter_test.go @@ -84,10 +84,7 @@ func TestCreateFilterFunc(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - fn, err := createFilterFunc(rpc.Filter{Labels: test.agentLabels}) - if !assert.NoError(t, err) { - t.Fail() - } + fn := createFilterFunc(rpc.Filter{Labels: test.agentLabels}) assert.EqualValues(t, test.exp, fn(&test.task)) }) diff --git a/server/grpc/rpc.go b/server/grpc/rpc.go index cf507c396e..f36c0a3d70 100644 --- a/server/grpc/rpc.go +++ b/server/grpc/rpc.go @@ -56,10 +56,7 @@ func (s *RPC) Next(c context.Context, agentFilter rpc.Filter) (*rpc.Workflow, er log.Debug().Msgf("agent connected: %s: polling", hostname) } - fn, err := createFilterFunc(agentFilter) - if err != nil { - return nil, err - } + fn := createFilterFunc(agentFilter) for { agent, err := s.getAgentFromContext(c) if err != nil { @@ -142,10 +139,12 @@ func (s *RPC) Update(_ context.Context, id string, state rpc.State) error { "private": strconv.FormatBool(repo.IsSCMPrivate), }, } - message.Data, _ = json.Marshal(model.Event{ + if message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: *currentPipeline, - }) + }); err != nil { + log.Error().Err(err).Msg("can not marshal json to publish to proc list") + } s.pubsub.Publish(message) return nil @@ -198,10 +197,12 @@ func (s *RPC) Init(c context.Context, id string, state rpc.State) error { "private": strconv.FormatBool(repo.IsSCMPrivate), }, } - message.Data, _ = json.Marshal(model.Event{ + if message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: *currentPipeline, - }) + }); err != nil { + log.Error().Err(err).Msg("can not marshal event to publish") + } s.pubsub.Publish(message) }() @@ -256,7 +257,7 @@ func (s *RPC) Done(c context.Context, id string, state rpc.State) error { var queueErr error if workflow.Failing() { - queueErr = s.queue.Error(c, id, fmt.Errorf("Step finished with exit code %d, %s", state.ExitCode, state.Error)) + queueErr = s.queue.Error(c, id, fmt.Errorf("step finished with exit code %d, %s", state.ExitCode, state.Error)) } else { queueErr = s.queue.Done(c, id, workflow.State) } @@ -375,7 +376,7 @@ func (s *RPC) ReportHealth(ctx context.Context, status string) error { } if status != "I am alive!" { - return errors.New("Are you alive?") + return errors.New("are you alive?") } agent.LastContact = time.Now().Unix() @@ -418,10 +419,12 @@ func (s *RPC) notify(repo *model.Repo, pipeline *model.Pipeline) (err error) { "private": strconv.FormatBool(repo.IsSCMPrivate), }, } - message.Data, _ = json.Marshal(model.Event{ + if message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: *pipeline, - }) + }); err != nil { + log.Error().Err(err).Msg("grpc could not marshal event") + } s.pubsub.Publish(message) return nil } diff --git a/server/grpc/server.go b/server/grpc/server.go index aa20ddce26..bc6a37e99e 100644 --- a/server/grpc/server.go +++ b/server/grpc/server.go @@ -84,7 +84,7 @@ func (s *WoodpeckerServer) Next(c context.Context, req *proto.NextRequest) (*pro res.Workflow = new(proto.Workflow) res.Workflow.Id = pipeline.ID res.Workflow.Timeout = pipeline.Timeout - res.Workflow.Payload, _ = json.Marshal(pipeline.Config) + res.Workflow.Payload, err = json.Marshal(pipeline.Config) return res, err } diff --git a/server/model/environ.go b/server/model/environ.go index 115e833818..6b8b59796c 100644 --- a/server/model/environ.go +++ b/server/model/environ.go @@ -20,8 +20,8 @@ import ( ) var ( - errEnvironNameInvalid = errors.New("Invalid Environment Variable Name") - errEnvironValueInvalid = errors.New("Invalid Environment Variable Value") + errEnvironNameInvalid = errors.New("invalid environment variable name") + errEnvironValueInvalid = errors.New("invalid environment variable value") ) // EnvironService defines a service for managing environment variables. diff --git a/server/model/registry.go b/server/model/registry.go index cd93dc7dfd..875cd2d9c0 100644 --- a/server/model/registry.go +++ b/server/model/registry.go @@ -21,9 +21,9 @@ import ( ) var ( - errRegistryAddressInvalid = errors.New("Invalid Registry Address") - errRegistryUsernameInvalid = errors.New("Invalid Registry Username") - errRegistryPasswordInvalid = errors.New("Invalid Registry Password") + errRegistryAddressInvalid = errors.New("invalid registry address") + errRegistryUsernameInvalid = errors.New("invalid registry username") + errRegistryPasswordInvalid = errors.New("invalid registry password") ) // RegistryService defines a service for managing registries. diff --git a/server/model/repo.go b/server/model/repo.go index 03f3dd7d33..9d508834a4 100644 --- a/server/model/repo.go +++ b/server/model/repo.go @@ -66,7 +66,7 @@ func (r *Repo) ResetVisibility() { func ParseRepo(str string) (user, repo string, err error) { parts := strings.Split(str, "/") if len(parts) != 2 { - err = fmt.Errorf("Error: Invalid or missing repository. eg octocat/hello-world") + err = fmt.Errorf("invalid or missing repository. eg octocat/hello-world") return } user = parts[0] diff --git a/server/model/secret.go b/server/model/secret.go index 7dbb087e97..2747945589 100644 --- a/server/model/secret.go +++ b/server/model/secret.go @@ -24,10 +24,10 @@ import ( ) var ( - ErrSecretNameInvalid = errors.New("Invalid Secret Name") - ErrSecretImageInvalid = errors.New("Invalid Secret Image") - ErrSecretValueInvalid = errors.New("Invalid Secret Value") - ErrSecretEventInvalid = errors.New("Invalid Secret Event") + ErrSecretNameInvalid = errors.New("invalid secret name") + ErrSecretImageInvalid = errors.New("invalid secret image") + ErrSecretValueInvalid = errors.New("invalid secret value") + ErrSecretEventInvalid = errors.New("invalid secret event") ) // SecretService defines a service for managing secrets. diff --git a/server/model/user.go b/server/model/user.go index 73eca72634..46e6e5ed7f 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -20,6 +20,8 @@ import ( "regexp" ) +const maxLoginLen = 250 + // validate a username (e.g. from github) var reUsername = regexp.MustCompile("^[a-zA-Z0-9-_.]+$") @@ -79,7 +81,7 @@ func (u *User) Validate() error { switch { case len(u.Login) == 0: return errUserLoginInvalid - case len(u.Login) > 250: + case len(u.Login) > maxLoginLen: return errUserLoginInvalid case !reUsername.MatchString(u.Login): return errUserLoginInvalid diff --git a/server/pipeline/cancel.go b/server/pipeline/cancel.go index 71df7dd068..be7e1c732d 100644 --- a/server/pipeline/cancel.go +++ b/server/pipeline/cancel.go @@ -93,9 +93,8 @@ func Cancel(ctx context.Context, store store.Store, repo *model.Repo, user *mode if killedPipeline.Workflows, err = store.WorkflowGetTree(killedPipeline); err != nil { return err } - publishToTopic(killedPipeline, repo) - return nil + return publishToTopic(killedPipeline, repo) } func cancelPreviousPipelines( diff --git a/server/pipeline/create.go b/server/pipeline/create.go index 5182a4cbf4..2c31174291 100644 --- a/server/pipeline/create.go +++ b/server/pipeline/create.go @@ -116,7 +116,10 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline } if pipeline.Status == model.StatusBlocked { - publishPipeline(ctx, pipeline, repo, repoUser) + if err := publishPipeline(ctx, pipeline, repo, repoUser); err != nil { + return nil, err + } + return pipeline, nil } @@ -142,9 +145,7 @@ func updatePipelineWithErr(ctx context.Context, _store store.Store, pipeline *mo // update value in ref *pipeline = *_pipeline - publishPipeline(ctx, pipeline, repo, repoUser) - - return nil + return publishPipeline(ctx, pipeline, repo, repoUser) } func updatePipelinePending(ctx context.Context, _store store.Store, pipeline *model.Pipeline, repo *model.Repo, repoUser *model.User) error { @@ -155,7 +156,5 @@ func updatePipelinePending(ctx context.Context, _store store.Store, pipeline *mo // update value in ref *pipeline = *_pipeline - publishPipeline(ctx, pipeline, repo, repoUser) - - return nil + return publishPipeline(ctx, pipeline, repo, repoUser) } diff --git a/server/pipeline/decline.go b/server/pipeline/decline.go index 7ddadfa5f6..24bc953502 100644 --- a/server/pipeline/decline.go +++ b/server/pipeline/decline.go @@ -41,7 +41,9 @@ func Decline(ctx context.Context, store store.Store, pipeline *model.Pipeline, u updatePipelineStatus(ctx, pipeline, repo, user) - publishToTopic(pipeline, repo) + if err := publishToTopic(pipeline, repo); err != nil { + return nil, err + } return pipeline, nil } diff --git a/server/pipeline/errors.go b/server/pipeline/errors.go index d887904b9a..4b7887a381 100644 --- a/server/pipeline/errors.go +++ b/server/pipeline/errors.go @@ -25,9 +25,9 @@ func (e ErrNotFound) Error() string { } func (e ErrNotFound) Is(target error) bool { - _, ok := target.(ErrNotFound) //nolint:errorlint + _, ok := target.(ErrNotFound) if !ok { - _, ok = target.(*ErrNotFound) //nolint:errorlint + _, ok = target.(*ErrNotFound) } return ok } @@ -41,9 +41,9 @@ func (e ErrBadRequest) Error() string { } func (e ErrBadRequest) Is(target error) bool { - _, ok := target.(ErrBadRequest) //nolint:errorlint + _, ok := target.(ErrBadRequest) if !ok { - _, ok = target.(*ErrBadRequest) //nolint:errorlint + _, ok = target.(*ErrBadRequest) } return ok } 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/queue.go b/server/pipeline/queue.go index 657355d320..26b81ae32f 100644 --- a/server/pipeline/queue.go +++ b/server/pipeline/queue.go @@ -25,7 +25,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/model" ) -func queuePipeline(repo *model.Repo, pipelineItems []*pipeline.Item) error { +func queuePipeline(ctx context.Context, repo *model.Repo, pipelineItems []*pipeline.Item) error { var tasks []*model.Task for _, item := range pipelineItems { if item.Workflow.State == model.StatusSkipped { @@ -42,15 +42,19 @@ func queuePipeline(repo *model.Repo, pipelineItems []*pipeline.Item) error { task.RunOn = item.RunsOn task.DepStatus = make(map[string]model.StatusValue) - task.Data, _ = json.Marshal(rpc.Workflow{ + var err error + task.Data, err = json.Marshal(rpc.Workflow{ ID: fmt.Sprint(item.Workflow.ID), Config: item.Config, Timeout: repo.Timeout, }) + if err != nil { + return err + } tasks = append(tasks, task) } - return server.Config.Services.Queue.PushAtOnce(context.Background(), tasks) + return server.Config.Services.Queue.PushAtOnce(ctx, tasks) } func taskIds(dependsOn []string, pipelineItems []*pipeline.Item) (taskIds []string) { diff --git a/server/pipeline/start.go b/server/pipeline/start.go index 99d1d6a562..fce38db879 100644 --- a/server/pipeline/start.go +++ b/server/pipeline/start.go @@ -37,9 +37,11 @@ func start(ctx context.Context, store store.Store, activePipeline *model.Pipelin return nil, err } - publishPipeline(ctx, activePipeline, repo, user) + if err := publishPipeline(ctx, activePipeline, repo, user); err != nil { + return nil, err + } - if err := queuePipeline(repo, pipelineItems); err != nil { + if err := queuePipeline(ctx, repo, pipelineItems); err != nil { log.Error().Err(err).Msg("queuePipeline") return nil, err } @@ -47,7 +49,12 @@ func start(ctx context.Context, store store.Store, activePipeline *model.Pipelin return activePipeline, nil } -func publishPipeline(ctx context.Context, pipeline *model.Pipeline, repo *model.Repo, repoUser *model.User) { - publishToTopic(pipeline, repo) +func publishPipeline(ctx context.Context, pipeline *model.Pipeline, repo *model.Repo, repoUser *model.User) error { + if err := publishToTopic(pipeline, repo); err != nil { + return err + } + updatePipelineStatus(ctx, pipeline, repo, repoUser) + + return nil } diff --git a/server/pipeline/stepStatus_test.go b/server/pipeline/stepStatus_test.go index ffb2a7fa80..08e893cb04 100644 --- a/server/pipeline/stepStatus_test.go +++ b/server/pipeline/stepStatus_test.go @@ -46,15 +46,16 @@ func TestUpdateStepStatusNotExited(t *testing.T) { err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(1)) assert.NoError(t, err) - if step.State != model.StatusRunning { + switch { + case step.State != model.StatusRunning: t.Errorf("Step status not equals '%s' != '%s'", model.StatusRunning, step.State) - } else if step.Started != int64(42) { + case step.Started != int64(42): t.Errorf("Step started not equals 42 != %d", step.Started) - } else if step.Stopped != int64(0) { + case step.Stopped != int64(0): t.Errorf("Step stopped not equals 0 != %d", step.Stopped) - } else if step.ExitCode != 0 { + case step.ExitCode != 0: t.Errorf("Step exit code not equals 0 != %d", step.ExitCode) - } else if step.Error != "" { + case step.Error != "": t.Errorf("Step error not equals '' != '%s'", step.Error) } } @@ -74,15 +75,16 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) { err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42)) assert.NoError(t, err) - if step.State != model.StatusRunning { + switch { + case step.State != model.StatusRunning: t.Errorf("Step status not equals '%s' != '%s'", model.StatusRunning, step.State) - } else if step.Started != int64(42) { + case step.Started != int64(42): t.Errorf("Step started not equals 42 != %d", step.Started) - } else if step.Stopped != int64(64) { + case step.Stopped != int64(64): t.Errorf("Step stopped not equals 64 != %d", step.Stopped) - } else if step.ExitCode != 0 { + case step.ExitCode != 0: t.Errorf("Step exit code not equals 0 != %d", step.ExitCode) - } else if step.Error != "" { + case step.Error != "": t.Errorf("Step error not equals '' != '%s'", step.Error) } } @@ -102,15 +104,16 @@ func TestUpdateStepStatusExited(t *testing.T) { err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42)) assert.NoError(t, err) - 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.Started != int64(42) { + case step.Started != int64(42): t.Errorf("Step started not equals 42 != %d", step.Started) - } else if step.Stopped != int64(34) { + case step.Stopped != int64(34): t.Errorf("Step stopped not equals 34 != %d", step.Stopped) - } else if step.ExitCode != 137 { + case step.ExitCode != 137: t.Errorf("Step exit code not equals 137 != %d", step.ExitCode) - } else if step.Error != "an error" { + case step.Error != "an error": t.Errorf("Step error not equals 'an error' != '%s'", step.Error) } } @@ -128,15 +131,16 @@ func TestUpdateStepStatusExitedButNot137(t *testing.T) { err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42)) assert.NoError(t, err) - if step.State != model.StatusFailure { + switch { + case step.State != model.StatusFailure: t.Errorf("Step status not equals '%s' != '%s'", model.StatusFailure, step.State) - } else if step.Started != int64(42) { + case step.Started != int64(42): t.Errorf("Step started not equals 42 != %d", step.Started) - } else if step.Stopped != int64(34) { + case step.Stopped != int64(34): t.Errorf("Step stopped not equals 34 != %d", step.Stopped) - } else if step.ExitCode != 0 { + case step.ExitCode != 0: t.Errorf("Step exit code not equals 0 != %d", step.ExitCode) - } else if step.Error != "an error" { + case step.Error != "an error": t.Errorf("Step error not equals 'an error' != '%s'", step.Error) } } @@ -212,13 +216,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) } } @@ -233,13 +238,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) } } @@ -275,13 +281,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/pipeline/topic.go b/server/pipeline/topic.go index 61dd58295f..7d977846c4 100644 --- a/server/pipeline/topic.go +++ b/server/pipeline/topic.go @@ -24,7 +24,7 @@ import ( ) // publishToTopic publishes message to UI clients -func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) { +func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) (err error) { message := pubsub.Message{ Labels: map[string]string{ "repo": repo.FullName, @@ -33,9 +33,13 @@ func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) { } pipelineCopy := *pipeline - message.Data, _ = json.Marshal(model.Event{ + if message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: pipelineCopy, - }) + }); err != nil { + return err + } server.Config.Services.Pubsub.Publish(message) + + return nil } diff --git a/server/plugins/config/http.go b/server/plugins/config/http.go index ab3e662c22..e3a393ff7d 100644 --- a/server/plugins/config/http.go +++ b/server/plugins/config/http.go @@ -19,6 +19,8 @@ import ( "crypto" "fmt" + httpStatus "net/http" + forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/plugins/utils" @@ -66,11 +68,11 @@ func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *mod status, err := utils.Send(ctx, "POST", cp.endpoint, cp.privateKey, body, response) if err != nil && status != 204 { - return nil, false, fmt.Errorf("Failed to fetch config via http (%d) %w", status, err) + return nil, false, fmt.Errorf("failed to fetch config via http (%d) %w", status, err) } var newFileMeta []*forge_types.FileMeta - if status != 200 { + if status != httpStatus.StatusOK { newFileMeta = make([]*forge_types.FileMeta, 0) } else { newFileMeta = make([]*forge_types.FileMeta, len(response.Configs)) @@ -79,5 +81,5 @@ func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *mod } } - return newFileMeta, status == 204, nil + return newFileMeta, status == httpStatus.StatusNoContent, nil } 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/registry/filesystem.go b/server/plugins/registry/filesystem.go index 2b889f49d3..db9e24a0d3 100644 --- a/server/plugins/registry/filesystem.go +++ b/server/plugins/registry/filesystem.go @@ -111,11 +111,11 @@ func decodeAuth(authStr string) (string, string, error) { return "", "", err } if n > decLen { - return "", "", fmt.Errorf("Something went wrong decoding auth config") + return "", "", fmt.Errorf("something went wrong decoding auth config") } arr := strings.SplitN(string(decoded), ":", 2) if len(arr) != 2 { - return "", "", fmt.Errorf("Invalid auth configuration file") + return "", "", fmt.Errorf("invalid auth configuration file") } password := strings.Trim(arr[1], "\x00") return arr[0], password, nil diff --git a/server/plugins/utils/http.go b/server/plugins/utils/http.go index 95e504a54f..f3a0891b00 100644 --- a/server/plugins/utils/http.go +++ b/server/plugins/utils/http.go @@ -65,13 +65,13 @@ func Send(ctx context.Context, method, path string, privateKey crypto.PrivateKey } defer resp.Body.Close() - if resp.StatusCode != 200 { + if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { return resp.StatusCode, err } - return resp.StatusCode, fmt.Errorf("Response: %s", string(body)) + return resp.StatusCode, fmt.Errorf("response: %s", string(body)) } // if no other errors parse and return the json response. diff --git a/server/queue/fifo.go b/server/queue/fifo.go index f86c2a8664..4409974063 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -204,10 +204,12 @@ func (q *fifo) Info(_ context.Context) InfoT { stats.Stats.Complete = 0 // TODO: implement this for e := q.pending.Front(); e != nil; e = e.Next() { - stats.Pending = append(stats.Pending, e.Value.(*model.Task)) + task, _ := e.Value.(*model.Task) + stats.Pending = append(stats.Pending, task) } for e := q.waitingOnDeps.Front(); e != nil; e = e.Next() { - stats.WaitingOnDeps = append(stats.WaitingOnDeps, e.Value.(*model.Task)) + task, _ := e.Value.(*model.Task) + stats.WaitingOnDeps = append(stats.WaitingOnDeps, task) } for _, entry := range q.running { stats.Running = append(stats.Running, entry.item) @@ -255,7 +257,7 @@ func (q *fifo) process() { q.resubmitExpiredPipelines() q.filterWaiting() for pending, worker := q.assignToWorker(); pending != nil && worker != nil; pending, worker = q.assignToWorker() { - task := pending.Value.(*model.Task) + task, _ := pending.Value.(*model.Task) task.AgentID = worker.agentID delete(q.workers, worker) q.pending.Remove(pending) @@ -273,7 +275,7 @@ func (q *fifo) filterWaiting() { var nextWaiting *list.Element for e := q.waitingOnDeps.Front(); e != nil; e = nextWaiting { nextWaiting = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) q.pending.PushBack(task) } @@ -283,7 +285,7 @@ func (q *fifo) filterWaiting() { var nextPending *list.Element for e := q.pending.Front(); e != nil; e = nextPending { nextPending = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) if q.depsInQueue(task) { log.Debug().Msgf("queue: waiting due to unmet dependencies %v", task.ID) q.waitingOnDeps.PushBack(task) @@ -301,7 +303,7 @@ func (q *fifo) assignToWorker() (*list.Element, *worker) { var next *list.Element for e := q.pending.Front(); e != nil; e = next { next = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) log.Debug().Msgf("queue: trying to assign task: %v with deps %v", task.ID, task.Dependencies) for w := range q.workers { @@ -384,7 +386,7 @@ func (q *fifo) removeFromPending(taskID string) { var next *list.Element for e := q.pending.Front(); e != nil; e = next { next = e.Next() - task := e.Value.(*model.Task) + task, _ := e.Value.(*model.Task) if task.ID == taskID { log.Debug().Msgf("queue: %s is removed from pending", taskID) q.pending.Remove(e) diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index e36b417ed7..49d191e3ad 100644 --- a/server/queue/fifo_test.go +++ b/server/queue/fifo_test.go @@ -70,7 +70,7 @@ func TestFifo(t *testing.T) { func TestFifoExpire(t *testing.T) { want := &model.Task{ID: "1"} - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) q.extension = 0 assert.NoError(t, q.Push(noContext, want)) info := q.Info(noContext) @@ -95,7 +95,7 @@ func TestFifoExpire(t *testing.T) { func TestFifoWait(t *testing.T) { want := &model.Task{ID: "1"} - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.Push(noContext, want)) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -148,7 +148,7 @@ func TestFifoDependencies(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -184,7 +184,7 @@ func TestFifoErrors(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -233,7 +233,7 @@ func TestFifoErrors2(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) for i := 0; i < 2; i++ { @@ -280,7 +280,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) obtainedWorkCh := make(chan *model.Task) @@ -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 @@ -370,7 +371,7 @@ func TestFifoTransitiveErrors(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -420,7 +421,7 @@ func TestFifoCancel(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) _, _ = q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -440,7 +441,7 @@ func TestFifoPause(t *testing.T) { ID: "1", } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) var wg sync.WaitGroup wg.Add(1) go func() { @@ -472,7 +473,7 @@ func TestFifoPauseResume(t *testing.T) { ID: "1", } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) q.Pause() assert.NoError(t, q.Push(noContext, task1)) q.Resume() @@ -498,7 +499,7 @@ func TestWaitingVsPending(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New(context.Background()).(*fifo) + q, _ := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) diff --git a/server/router/middleware/header/header.go b/server/router/middleware/header/header.go index 571354604f..663bf451db 100644 --- a/server/router/middleware/header/header.go +++ b/server/router/middleware/header/header.go @@ -42,7 +42,7 @@ func Options(c *gin.Context) { c.Header("Access-Control-Allow-Headers", "authorization, origin, content-type, accept") c.Header("Allow", "HEAD,GET,POST,PUT,PATCH,DELETE,OPTIONS") c.Header("Content-Type", "application/json") - c.AbortWithStatus(200) + c.AbortWithStatus(http.StatusOK) } } diff --git a/server/router/middleware/session/agent.go b/server/router/middleware/session/agent.go index 3f8dda0603..c9014e5710 100644 --- a/server/router/middleware/session/agent.go +++ b/server/router/middleware/session/agent.go @@ -16,6 +16,8 @@ package session import ( + "net/http" + "github.com/gin-gonic/gin" "go.woodpecker-ci.org/woodpecker/v2/shared/token" @@ -23,22 +25,23 @@ import ( // AuthorizeAgent authorizes requests from agent to access the queue. func AuthorizeAgent(c *gin.Context) { - secret := c.MustGet("agent").(string) + secret, _ := c.MustGet("agent").(string) if secret == "" { - c.String(401, "invalid or empty token.") + c.String(http.StatusUnauthorized, "invalid or empty token.") return } parsed, err := token.ParseRequest(c.Request, func(t *token.Token) (string, error) { return secret, nil }) - if err != nil { - c.String(500, "invalid or empty token. %s", err) + switch { + case err != nil: + c.String(http.StatusInternalServerError, "invalid or empty token. %s", err) c.Abort() - } else if parsed.Kind != token.AgentToken { - c.String(403, "invalid token. please use an agent token") + case parsed.Kind != token.AgentToken: + c.String(http.StatusForbidden, "invalid token. please use an agent token") c.Abort() - } else { + default: c.Next() } } diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index 0e40bffff5..9ed94bae96 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -75,10 +75,10 @@ func MustAdmin() gin.HandlerFunc { user := User(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() case !user.Admin: - c.String(403, "User not authorized") + c.String(http.StatusForbidden, "User not authorized") c.Abort() default: c.Next() @@ -92,10 +92,10 @@ func MustRepoAdmin() gin.HandlerFunc { perm := Perm(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() case !perm.Admin: - c.String(403, "User not authorized") + c.String(http.StatusForbidden, "User not authorized") c.Abort() default: c.Next() @@ -108,7 +108,7 @@ func MustUser() gin.HandlerFunc { user := User(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() default: c.Next() diff --git a/server/store/context.go b/server/store/context.go index 02381e5136..96214a04c1 100644 --- a/server/store/context.go +++ b/server/store/context.go @@ -27,7 +27,8 @@ type Setter interface { // FromContext returns the Store associated with this context. func FromContext(c context.Context) Store { - return c.Value(key).(Store) + store, _ := c.Value(key).(Store) + return store } // TryFromContext try to return the Store associated with this context. diff --git a/server/store/datastore/agent.go b/server/store/datastore/agent.go index 6c3051d861..383692289d 100644 --- a/server/store/datastore/agent.go +++ b/server/store/datastore/agent.go @@ -20,7 +20,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/model" ) -var ErrNoTokenProvided = errors.New("Please provide a token") +var ErrNoTokenProvided = errors.New("please provide a token") func (s storage) AgentList(p *model.ListOptions) ([]*model.Agent, error) { var agents []*model.Agent diff --git a/server/store/datastore/migration/migration.go b/server/store/datastore/migration/migration.go index 218f7e4042..0da773bcfe 100644 --- a/server/store/datastore/migration/migration.go +++ b/server/store/datastore/migration/migration.go @@ -113,7 +113,7 @@ func Migrate(e *xorm.Engine, allowLong bool) error { func syncAll(sess *xorm.Engine) error { for _, bean := range allBeans { if err := sess.Sync(bean); err != nil { - return fmt.Errorf("Sync error '%s': %w", reflect.TypeOf(bean), err) + return fmt.Errorf("sync error '%s': %w", reflect.TypeOf(bean), err) } } return nil diff --git a/server/web/config.go b/server/web/config.go index 4344dea7fb..1fb2f7d8a8 100644 --- a/server/web/config.go +++ b/server/web/config.go @@ -51,7 +51,10 @@ func Config(c *gin.Context) { // default func map with json parser. funcMap := template.FuncMap{ "json": func(v any) string { - a, _ := json.Marshal(v) + a, err := json.Marshal(v) + if err != nil { + log.Err(err).Msg("") + } return string(a) }, } diff --git a/server/web/web.go b/server/web/web.go index 7199c4b649..49c5af23c7 100644 --- a/server/web/web.go +++ b/server/web/web.go @@ -72,11 +72,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) } } From ec0bbea5392eee6a46a4b67f2ad5fba4e29007b6 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 10:01:51 +0100 Subject: [PATCH 02/24] fix gomnd --- .golangci.yml | 23 ++++++++++++++++--- agent/rpc/auth_client_grpc.go | 2 +- agent/rpc/client_grpc.go | 4 ++-- agent/runner.go | 5 ++-- cli/deploy/deploy.go | 3 ++- cli/internal/util.go | 2 +- cli/pipeline/create.go | 2 +- cli/pipeline/list.go | 1 + cli/pipeline/logs.go | 6 +++-- cmd/agent/agent.go | 3 ++- pipeline/backend/kubernetes/kubernetes.go | 4 ++-- pipeline/frontend/metadata/environment.go | 2 +- .../yaml/types/base/base_types_test.go | 2 +- pipeline/frontend/yaml/types/base/map.go | 2 +- pipeline/frontend/yaml/types/volume.go | 1 + server/api/login.go | 2 +- server/api/registry.go | 2 +- server/cache/membership.go | 1 + server/forge/gitlab/convert.go | 2 +- server/forge/gitlab/gitlab.go | 6 ++--- server/forge/refresh.go | 8 ++++--- server/model/const.go | 2 ++ server/pipeline/stepStatus.go | 4 ++-- server/pipeline/stepStatus_test.go | 14 +++++------ server/plugins/registry/filesystem.go | 2 +- server/queue/fifo.go | 1 + shared/httputil/httputil.go | 5 +++- 27 files changed, 72 insertions(+), 39 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 08d12dea17..da8bdb4fc7 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,6 +21,19 @@ linters-settings: - standard - default - prefix(go.woodpecker-ci.org/woodpecker) + gomnd: + ignored-numbers: + - '0o600' + - '0o660' + - '0o644' + - '0o755' + - '0o700' + ignored-functions: + - make + - time.* + - strings.Split + - callerName + - securecookie.GenerateRandomKey linters: disable-all: true @@ -77,7 +90,7 @@ run: issues: exclude-rules: # gin force us to use string as context key - - path: server/store/context.go + - path: 'server/store/context.go' linters: - staticcheck - revive @@ -91,7 +104,7 @@ issues: linters: - forbidigo - - path: agent/runner.go + - path: 'agent/runner.go' linters: - contextcheck - errorlint @@ -100,6 +113,10 @@ issues: linters: - forcetypeassert - - path: server/store/datastore/migration/common.go|pipeline/frontend/yaml/constraint/constraint.go + - path: 'server/store/datastore/migration/common.go|pipeline/frontend/yaml/constraint/constraint.go' linters: - gocritic + + - path: 'cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go' + linters: + - gomnd diff --git a/agent/rpc/auth_client_grpc.go b/agent/rpc/auth_client_grpc.go index 5f31e2803f..e10b2f3313 100644 --- a/agent/rpc/auth_client_grpc.go +++ b/agent/rpc/auth_client_grpc.go @@ -40,7 +40,7 @@ func NewAuthGrpcClient(conn *grpc.ClientConn, agentToken string, agentID int64) } func (c *AuthClient) Auth() (string, int64, error) { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint: gomnd defer cancel() req := &proto.AuthRequest{ diff --git a/agent/rpc/client_grpc.go b/agent/rpc/client_grpc.go index 531c5c7ad7..972b1065ff 100644 --- a/agent/rpc/client_grpc.go +++ b/agent/rpc/client_grpc.go @@ -53,8 +53,8 @@ func (c *client) Close() error { func (c *client) newBackOff() backoff.BackOff { b := backoff.NewExponentialBackOff() - b.MaxInterval = 10 * time.Second - b.InitialInterval = 10 * time.Millisecond + b.MaxInterval = 10 * time.Second //nolint: gomnd + b.InitialInterval = 10 * time.Millisecond //nolint: gomnd return b } diff --git a/agent/runner.go b/agent/runner.go index df353f60f3..8c1a2f0e31 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -29,6 +29,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline" backend "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" + "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/shared/utils" ) @@ -157,7 +158,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { if canceled.IsSet() { state.Error = "" - state.ExitCode = 137 + state.ExitCode = model.ExitCodeKilled } else if err != nil { pExitError := &pipeline.ExitError{} switch { @@ -165,7 +166,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { state.ExitCode = pExitError.Code case errors.Is(err, pipeline.ErrCancel): state.Error = "" - state.ExitCode = 137 + state.ExitCode = model.ExitCodeKilled canceled.SetTo(true) default: state.ExitCode = 1 diff --git a/cli/deploy/deploy.go b/cli/deploy/deploy.go index 6ce9b5502e..d4cdf5c878 100644 --- a/cli/deploy/deploy.go +++ b/cli/deploy/deploy.go @@ -106,7 +106,8 @@ func deploy(c *cli.Context) error { } } - env := c.Args().Get(2) + envArgIndex := 2 + env := c.Args().Get(envArgIndex) if env == "" { return fmt.Errorf("please specify the target environment (ie production)") } diff --git a/cli/internal/util.go b/cli/internal/util.go index e4a3717c34..019619c353 100644 --- a/cli/internal/util.go +++ b/cli/internal/util.go @@ -108,7 +108,7 @@ func ParseKeyPair(p []string) map[string]string { params := map[string]string{} for _, i := range p { parts := strings.SplitN(i, "=", 2) - if len(parts) != 2 { + if len(parts) != 2 { //nolint:gomnd continue } params[parts[0]] = parts[1] diff --git a/cli/pipeline/create.go b/cli/pipeline/create.go index b268622eb7..eecbbaa3d6 100644 --- a/cli/pipeline/create.go +++ b/cli/pipeline/create.go @@ -62,7 +62,7 @@ func pipelineCreate(c *cli.Context) error { for _, vaz := range c.StringSlice("var") { sp := strings.SplitN(vaz, "=", 2) - if len(sp) == 2 { + if len(sp) == 2 { //nolint:gomnd variables[sp[0]] = sp[1] } } diff --git a/cli/pipeline/list.go b/cli/pipeline/list.go index 5a2fbaf92d..5cac3aa4d8 100644 --- a/cli/pipeline/list.go +++ b/cli/pipeline/list.go @@ -24,6 +24,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/cli/internal" ) +//nolint:gomnd var pipelineListCmd = &cli.Command{ Name: "ls", Usage: "show pipeline history", diff --git a/cli/pipeline/logs.go b/cli/pipeline/logs.go index 8951b20521..116d68a4d2 100644 --- a/cli/pipeline/logs.go +++ b/cli/pipeline/logs.go @@ -41,12 +41,14 @@ func pipelineLogs(c *cli.Context) error { return err } - number, err := strconv.ParseInt(c.Args().Get(1), 10, 64) + numberArgIndex := 1 + number, err := strconv.ParseInt(c.Args().Get(numberArgIndex), 10, 64) if err != nil { return err } - step, err := strconv.ParseInt(c.Args().Get(2), 10, 64) + stepArgIndex := 2 + step, err := strconv.ParseInt(c.Args().Get(stepArgIndex), 10, 64) if err != nil { return err } diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index c8ce918ab7..7181e59e3f 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -93,7 +93,7 @@ func run(c *cli.Context) error { agentToken := c.String("grpc-token") authClient := agentRpc.NewAuthGrpcClient(authConn, agentToken, agentConfig.AgentID) - authInterceptor, err := agentRpc.NewAuthInterceptor(authClient, 30*time.Minute) + authInterceptor, err := agentRpc.NewAuthInterceptor(authClient, 30*time.Minute) //nolint: gomnd if err != nil { return err } @@ -288,6 +288,7 @@ func stringSliceAddToMap(sl []string, m map[string]string) error { if m == nil { m = make(map[string]string) } + //nolint: gomnd for _, v := range sl { parts := strings.SplitN(v, "=", 2) switch len(parts) { diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 6e9c55b528..28baf28201 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -221,7 +221,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) + si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) //nolint: gomnd if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, @@ -277,7 +277,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) } // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) + si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) //nolint: gomnd if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, diff --git a/pipeline/frontend/metadata/environment.go b/pipeline/frontend/metadata/environment.go index ff84c172d2..1b2aca9320 100644 --- a/pipeline/frontend/metadata/environment.go +++ b/pipeline/frontend/metadata/environment.go @@ -36,7 +36,7 @@ func (m *Metadata) Environ() map[string]string { ) branchParts := strings.Split(m.Curr.Commit.Refspec, ":") - if len(branchParts) == 2 { + if len(branchParts) == 2 { //nolint: gomnd sourceBranch = branchParts[0] targetBranch = branchParts[1] } diff --git a/pipeline/frontend/yaml/types/base/base_types_test.go b/pipeline/frontend/yaml/types/base/base_types_test.go index 98accdfabf..3813036472 100644 --- a/pipeline/frontend/yaml/types/base/base_types_test.go +++ b/pipeline/frontend/yaml/types/base/base_types_test.go @@ -97,7 +97,7 @@ bars: [] func TestUnmarshalSliceOrMap(t *testing.T) { s := StructSliceorMap{} err := yaml.Unmarshal([]byte(sampleStructSliceorMap), &s) - assert.Equal(t, fmt.Errorf("Cannot unmarshal 'true' of type bool into a string value"), err) + assert.Equal(t, fmt.Errorf("cannot unmarshal 'true' of type bool into a string value"), err) } func TestStr2SliceOrMapPtrMap(t *testing.T) { diff --git a/pipeline/frontend/yaml/types/base/map.go b/pipeline/frontend/yaml/types/base/map.go index e25a65b04e..cead91b188 100644 --- a/pipeline/frontend/yaml/types/base/map.go +++ b/pipeline/frontend/yaml/types/base/map.go @@ -35,7 +35,7 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { key := keyValueSlice[0] val := "" - if len(keyValueSlice) == 2 { + if len(keyValueSlice) == 2 { //nolint: gomnd val = keyValueSlice[1] } parts[key] = val diff --git a/pipeline/frontend/yaml/types/volume.go b/pipeline/frontend/yaml/types/volume.go index d07a3eec60..3d925e25b3 100644 --- a/pipeline/frontend/yaml/types/volume.go +++ b/pipeline/frontend/yaml/types/volume.go @@ -68,6 +68,7 @@ func (v *Volumes) UnmarshalYAML(unmarshal func(any) error) error { } elts := strings.SplitN(name, ":", 3) var vol *Volume + //nolint: gomnd switch { case len(elts) == 1: vol = &Volume{ diff --git a/server/api/login.go b/server/api/login.go index 4a0e53cc26..5442f0fe6b 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -81,7 +81,7 @@ func HandleAuth(c *gin.Context) { teams, terr := _forge.Teams(c, tmpuser) if terr != nil || !server.Config.Permissions.Orgs.IsMember(teams) { log.Error().Err(terr).Msgf("cannot verify team membership for %s.", u.Login) - c.Redirect(303, server.Config.Server.RootPath+"/login?error=access_denied") + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied") return } } diff --git a/server/api/registry.go b/server/api/registry.go index 38b01d2c68..714837931e 100644 --- a/server/api/registry.go +++ b/server/api/registry.go @@ -44,7 +44,7 @@ func GetRegistry(c *gin.Context) { handleDBError(c, err) return } - c.JSON(200, registry.Copy()) + c.JSON(http.StatusOK, registry.Copy()) } // PostRegistry diff --git a/server/cache/membership.go b/server/cache/membership.go index 233d7fd080..467a5a2a81 100644 --- a/server/cache/membership.go +++ b/server/cache/membership.go @@ -39,6 +39,7 @@ type membershipCache struct { // NewMembershipService creates a new membership service. func NewMembershipService(f forge.Forge) MembershipService { + //nolint: gomnd return &membershipCache{ ttl: 10 * time.Minute, forge: f, diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index 7dc88b8700..46c06e1a3b 100644 --- a/server/forge/gitlab/convert.go +++ b/server/forge/gitlab/convert.go @@ -251,7 +251,7 @@ func getUserAvatar(email string) string { func extractFromPath(str string) (string, string, error) { s := strings.Split(str, "/") - if len(s) < 2 { + if len(s) < 2 { //nolint: gomnd return "", "", fmt.Errorf("minimum match not found") } return s[0], s[1], nil diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index 8c5c8eb319..50e23bb948 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -523,7 +523,7 @@ func (g *GitLab) Deactivate(ctx context.Context, user *model.User, repo *model.R hookID := -1 listProjectHooksOptions := &gitlab.ListProjectHooksOptions{ - PerPage: 10, + PerPage: perPage, Page: 1, } for { @@ -652,7 +652,7 @@ func (g *GitLab) OrgMembership(ctx context.Context, u *model.User, owner string) groups, _, err := client.Groups.ListGroups(&gitlab.ListGroupsOptions{ ListOptions: gitlab.ListOptions{ Page: 1, - PerPage: 100, + PerPage: perPage, }, Search: gitlab.Ptr(owner), }, gitlab.WithContext(ctx)) @@ -673,7 +673,7 @@ func (g *GitLab) OrgMembership(ctx context.Context, u *model.User, owner string) opts := &gitlab.ListGroupMembersOptions{ ListOptions: gitlab.ListOptions{ Page: 1, - PerPage: 100, + PerPage: perPage, }, } diff --git a/server/forge/refresh.go b/server/forge/refresh.go index dd904c91f5..f650eb1af2 100644 --- a/server/forge/refresh.go +++ b/server/forge/refresh.go @@ -32,11 +32,13 @@ type Refresher interface { } func Refresh(c context.Context, forge Forge, _store store.Store, user *model.User) { + // Remaining ttl of 30 minutes (1800 seconds) until a token is refreshed. + const tokenMinTTL = 1800 + if refresher, ok := forge.(Refresher); ok { - // Check to see if the user token is expired or - // will expire within the next 30 minutes (1800 seconds). + // Check to see if the user token is expired or will expire soon. // If not, there is nothing we really need to do here. - if time.Now().UTC().Unix() < (user.Expiry - 1800) { + if time.Now().UTC().Unix() < (user.Expiry - tokenMinTTL) { return } diff --git a/server/model/const.go b/server/model/const.go index 89760d730d..4823786e2e 100644 --- a/server/model/const.go +++ b/server/model/const.go @@ -62,6 +62,8 @@ const ( StatusBlocked StatusValue = "blocked" // waiting for approval StatusDeclined StatusValue = "declined" // blocked and declined StatusCreated StatusValue = "created" // created / internal use only + + ExitCodeKilled int = 137 ) // SCMKind represent different version control systems diff --git a/server/pipeline/stepStatus.go b/server/pipeline/stepStatus.go index 2258b4c22a..122b79dfe1 100644 --- a/server/pipeline/stepStatus.go +++ b/server/pipeline/stepStatus.go @@ -31,7 +31,7 @@ func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.S if state.ExitCode != 0 || state.Error != "" { step.State = model.StatusFailure } - if state.ExitCode == 137 { + if state.ExitCode == model.ExitCodeKilled { step.State = model.StatusKilled } } else { @@ -81,6 +81,6 @@ func UpdateStepToStatusKilled(store model.UpdateStepStore, step model.Step) (*mo if step.Started == 0 { step.Started = step.Stopped } - step.ExitCode = 137 + step.ExitCode = model.ExitCodeKilled return &step, store.StepUpdate(&step) } diff --git a/server/pipeline/stepStatus_test.go b/server/pipeline/stepStatus_test.go index 08e893cb04..d7b1292a40 100644 --- a/server/pipeline/stepStatus_test.go +++ b/server/pipeline/stepStatus_test.go @@ -39,7 +39,7 @@ func TestUpdateStepStatusNotExited(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: 137, + ExitCode: model.ExitCodeKilled, Error: "not an error", } step := &model.Step{} @@ -69,7 +69,7 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: 137, + ExitCode: model.ExitCodeKilled, Error: "not an error", } err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42)) @@ -96,7 +96,7 @@ func TestUpdateStepStatusExited(t *testing.T) { Started: int64(42), Exited: true, Finished: int64(34), - ExitCode: 137, + ExitCode: model.ExitCodeKilled, Error: "an error", } @@ -111,8 +111,8 @@ func TestUpdateStepStatusExited(t *testing.T) { t.Errorf("Step started not equals 42 != %d", step.Started) case step.Stopped != int64(34): t.Errorf("Step stopped not equals 34 != %d", step.Stopped) - case step.ExitCode != 137: - t.Errorf("Step exit code not equals 137 != %d", step.ExitCode) + case step.ExitCode != model.ExitCodeKilled: + t.Errorf("Step exit code not equals %d != %d", model.ExitCodeKilled, step.ExitCode) case step.Error != "an error": t.Errorf("Step error not equals 'an error' != '%s'", step.Error) } @@ -288,8 +288,8 @@ func TestUpdateStepToStatusKilledStarted(t *testing.T) { t.Errorf("Step stopped not equals %d < %d", now, step.Stopped) case step.Started != step.Stopped: t.Errorf("Step started not equals %d != %d", step.Stopped, step.Started) - case step.ExitCode != 137: - t.Errorf("Step exit code not equals 137 != %d", step.ExitCode) + case step.ExitCode != model.ExitCodeKilled: + t.Errorf("Step exit code not equals %d != %d", model.ExitCodeKilled, step.ExitCode) } } diff --git a/server/plugins/registry/filesystem.go b/server/plugins/registry/filesystem.go index db9e24a0d3..26d8ca164d 100644 --- a/server/plugins/registry/filesystem.go +++ b/server/plugins/registry/filesystem.go @@ -114,7 +114,7 @@ func decodeAuth(authStr string) (string, string, error) { return "", "", fmt.Errorf("something went wrong decoding auth config") } arr := strings.SplitN(string(decoded), ":", 2) - if len(arr) != 2 { + if len(arr) != 2 { //nolint:gomnd return "", "", fmt.Errorf("invalid auth configuration file") } password := strings.Trim(arr[1], "\x00") diff --git a/server/queue/fifo.go b/server/queue/fifo.go index 4409974063..110f6d5db8 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -52,6 +52,7 @@ type fifo struct { // New returns a new fifo queue. func New(_ context.Context) Queue { + //nolint: gomnd return &fifo{ workers: map[*worker]struct{}{}, running: map[string]*entry{}, diff --git a/shared/httputil/httputil.go b/shared/httputil/httputil.go index 11c9a3fe9c..a2328b7ce7 100644 --- a/shared/httputil/httputil.go +++ b/shared/httputil/httputil.go @@ -40,6 +40,9 @@ func IsHTTPS(r *http.Request) bool { // SetCookie writes the cookie value. func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { + // the cookie value (token) is responsible for expiration + cookieMaxAgeSeconds := 2147483647 + cookie := http.Cookie{ Name: name, Value: value, @@ -47,7 +50,7 @@ func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { Domain: r.URL.Host, HttpOnly: true, Secure: IsHTTPS(r), - MaxAge: 2147483647, // the cooke value (token) is responsible for expiration + MaxAge: cookieMaxAgeSeconds, } http.SetCookie(w, &cookie) From 57a896c18380beac7b7465880c68b5075e98716a Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 10:18:02 +0100 Subject: [PATCH 03/24] fix goconst and cleanup golangci excludes --- .golangci.yml | 17 ++++++----------- agent/runner.go | 5 +++-- cli/pipeline/info.go | 2 +- pipeline/frontend/yaml/compiler/compiler.go | 2 +- pipeline/frontend/yaml/constraint/constraint.go | 2 ++ pipeline/tracer.go | 10 ++++++---- .../datastore/migration/008_secrets_add_user.go | 10 ++++++---- .../migration/013_rename_remote_to_forge.go | 6 ++++-- server/store/datastore/migration/common.go | 2 +- 9 files changed, 30 insertions(+), 26 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index da8bdb4fc7..4599655967 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,7 +65,7 @@ linters: - forcetypeassert # - gci - gochecknoinits - # - goconst + - goconst - gocritic - goheader - gomnd @@ -104,19 +104,14 @@ issues: linters: - forbidigo - - path: 'agent/runner.go' - linters: - - contextcheck - - errorlint - - - path: 'server/forge/bitbucket/bitbucket_test.go|server/forge/gitea/gitea_test.go|server/forge/github/github_test.go' + - path: '_test.go' linters: - forcetypeassert - - path: 'server/store/datastore/migration/common.go|pipeline/frontend/yaml/constraint/constraint.go' - linters: - - gocritic - - path: 'cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go' linters: - gomnd + + - path: '_test.go' + linters: + - goconst diff --git a/agent/runner.go b/agent/runner.go index 8c1a2f0e31..8d8e549342 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -134,12 +134,13 @@ func (r *Runner) Run(runnerCtx context.Context) error { state := rpc.State{} state.Started = time.Now().Unix() - err = r.client.Init(ctxmeta, work.ID, state) + err = r.client.Init(ctxmeta, work.ID, state) //nolint:contextcheck if err != nil { logger.Error().Err(err).Msg("pipeline initialization failed") } var uploads sync.WaitGroup + //nolint:contextcheck err = pipeline.New(work.Config, pipeline.WithContext(workflowCtx), pipeline.WithTaskUUID(fmt.Sprint(work.ID)), @@ -189,7 +190,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { Int("exit_code", state.ExitCode). Msg("updating pipeline status") - if err := r.client.Done(ctxmeta, work.ID, state); err != nil { + if err := r.client.Done(ctxmeta, work.ID, state); err != nil { //nolint:contextcheck logger.Error().Err(err).Msg("updating pipeline status failed") } else { logger.Debug().Msg("updating pipeline status complete") diff --git a/cli/pipeline/info.go b/cli/pipeline/info.go index 2b1e9103fc..51d7554438 100644 --- a/cli/pipeline/info.go +++ b/cli/pipeline/info.go @@ -46,7 +46,7 @@ func pipelineInfo(c *cli.Context) error { pipelineArg := c.Args().Get(1) var number int64 - if pipelineArg == "last" || len(pipelineArg) == 0 { + if pipelineArg == "last" || len(pipelineArg) == 0 { //nolint:goconst // Fetch the pipeline number from the last pipeline pipeline, err := client.PipelineLast(repoID, "") if err != nil { diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index 3617391f61..34b526b2de 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -153,7 +153,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er if !c.local && len(conf.Clone.ContainerList) == 0 && !conf.SkipClone { cloneSettings := map[string]any{"depth": "0"} if c.metadata.Curr.Event == metadata.EventTag { - cloneSettings["tags"] = "true" + cloneSettings["tags"] = "true" //nolint:goconst } container := &yaml_types.Container{ Name: defaultCloneName, diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go index 9502919f26..684fd0857b 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/pipeline/frontend/yaml/constraint/constraint.go @@ -255,6 +255,7 @@ func (c *List) UnmarshalYAML(value *yaml.Node) error { err2 := value.Decode(&out2) c.Exclude = out1.Exclude + //nolint:gocritic c.Include = append( out1.Include, out2..., @@ -336,6 +337,7 @@ func (c *Path) UnmarshalYAML(value *yaml.Node) error { c.Exclude = out1.Exclude c.IgnoreMessage = out1.IgnoreMessage + //nolint:gocritic c.Include = append( out1.Include, out2..., diff --git a/pipeline/tracer.go b/pipeline/tracer.go index 88db835868..f276162bf3 100644 --- a/pipeline/tracer.go +++ b/pipeline/tracer.go @@ -17,6 +17,8 @@ package pipeline import ( "strconv" "time" + + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) // Tracer handles process tracing. @@ -43,17 +45,17 @@ var DefaultTracer = TraceFunc(func(state *State) error { if state.Pipeline.Step.Environment == nil { return nil } - state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = "success" + state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(model.StatusSuccess) state.Pipeline.Step.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Pipeline.Time, 10) state.Pipeline.Step.Environment["CI_PIPELINE_FINISHED"] = strconv.FormatInt(time.Now().Unix(), 10) - state.Pipeline.Step.Environment["CI_STEP_STATUS"] = "success" + state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(model.StatusSuccess) state.Pipeline.Step.Environment["CI_STEP_STARTED"] = strconv.FormatInt(state.Pipeline.Time, 10) state.Pipeline.Step.Environment["CI_STEP_FINISHED"] = strconv.FormatInt(time.Now().Unix(), 10) if state.Pipeline.Error != nil { - state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = "failure" - state.Pipeline.Step.Environment["CI_STEP_STATUS"] = "failure" + state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(model.StatusFailure) + state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(model.StatusFailure) } return nil }) diff --git a/server/store/datastore/migration/008_secrets_add_user.go b/server/store/datastore/migration/008_secrets_add_user.go index b77208f750..0a5320e68a 100644 --- a/server/store/datastore/migration/008_secrets_add_user.go +++ b/server/store/datastore/migration/008_secrets_add_user.go @@ -19,6 +19,8 @@ import ( "xorm.io/xorm" ) +const secretsTableName = "secrets" + type SecretV008 struct { Owner string `json:"-" xorm:"NOT NULL DEFAULT '' UNIQUE(s) INDEX 'secret_owner'"` RepoID int64 `json:"-" xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_repo_id'"` @@ -27,7 +29,7 @@ type SecretV008 struct { // TableName return database table name for xorm func (SecretV008) TableName() string { - return "secrets" + return secretsTableName } var alterTableSecretsAddUserCol = xormigrate.Migration{ @@ -36,12 +38,12 @@ var alterTableSecretsAddUserCol = xormigrate.Migration{ if err := sess.Sync(new(SecretV008)); err != nil { return err } - if err := alterColumnDefault(sess, "secrets", "secret_repo_id", "0"); err != nil { + if err := alterColumnDefault(sess, secretsTableName, "secret_repo_id", "0"); err != nil { return err } - if err := alterColumnNull(sess, "secrets", "secret_repo_id", false); err != nil { + if err := alterColumnNull(sess, secretsTableName, "secret_repo_id", false); err != nil { return err } - return alterColumnNull(sess, "secrets", "secret_name", false) + return alterColumnNull(sess, secretsTableName, "secret_name", false) }, } diff --git a/server/store/datastore/migration/013_rename_remote_to_forge.go b/server/store/datastore/migration/013_rename_remote_to_forge.go index 78eff328be..870f6000ea 100644 --- a/server/store/datastore/migration/013_rename_remote_to_forge.go +++ b/server/store/datastore/migration/013_rename_remote_to_forge.go @@ -19,13 +19,15 @@ import ( "xorm.io/xorm" ) +const reposTableName = "repos" + type oldRepo013 struct { ID int64 `xorm:"pk autoincr 'repo_id'"` RemoteID string `xorm:"remote_id"` } func (oldRepo013) TableName() string { - return "repos" + return reposTableName } var renameRemoteToForge = xormigrate.Migration{ @@ -40,6 +42,6 @@ var renameRemoteToForge = xormigrate.Migration{ return err } - return renameColumn(sess, "repos", "remote_id", "forge_id") + return renameColumn(sess, reposTableName, "remote_id", "forge_id") }, } diff --git a/server/store/datastore/migration/common.go b/server/store/datastore/migration/common.go index c0d0df2f03..e79e8979aa 100644 --- a/server/store/datastore/migration/common.go +++ b/server/store/datastore/migration/common.go @@ -85,7 +85,7 @@ 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, "("):] + tableSQL = tableSQL[strings.Index(tableSQL, "("):] //nolint:gocritic // Remove the required columnNames tableSQL = removeColumnFromSQLITETableSchema(tableSQL, columnNames...) From 7bba645aef2f429e14b5f6d9c768ae61bc0c2e83 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 10:21:11 +0100 Subject: [PATCH 04/24] fix gci --- .golangci.yml | 2 +- agent/rpc/auth_client_grpc.go | 4 ++-- cli/pipeline/create.go | 3 +-- cmd/server/main.go | 2 +- cmd/server/server.go | 2 -- pipeline/backend/kubernetes/kubernetes.go | 10 ++++------ pipeline/backend/local/clone.go | 1 + pipeline/frontend/metadata/drone_compatibility_test.go | 1 + pipeline/frontend/yaml/matrix/matrix.go | 4 ++-- pipeline/frontend/yaml/types/base/base_types_test.go | 3 +-- pipeline/frontend/yaml/types/network_test.go | 3 +-- pipeline/frontend/yaml/types/volume_test.go | 3 +-- pipeline/stepBuilder.go | 5 ++--- server/api/global_secret.go | 3 +-- server/api/org.go | 3 +-- server/api/org_secret.go | 3 +-- server/api/signature_public_key.go | 1 + server/cache/membership.go | 4 ++-- server/forge/bitbucket/internal/client.go | 4 ++-- server/logging/log_test.go | 1 + server/plugins/config/http.go | 1 - server/plugins/encryption/aes_encryption.go | 4 ++-- server/plugins/permissions/repo_owners_test.go | 1 + server/plugins/secrets/builtin_test.go | 1 + server/plugins/utils/http_test.go | 1 + server/queue/fifo.go | 4 ++-- server/queue/fifo_test.go | 1 + server/router/middleware/session/pagination.go | 1 + server/router/middleware/session/repo.go | 2 +- server/router/middleware/session/user.go | 6 +++--- server/store/datastore/cron.go | 4 ++-- server/store/datastore/engine.go | 5 ++--- server/store/datastore/engine_test.go | 3 +-- server/store/datastore/helper_test.go | 1 + server/store/datastore/log_test.go | 1 + server/store/datastore/migration/migration_test.go | 7 +++---- server/web/web_test.go | 1 + 37 files changed, 51 insertions(+), 55 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4599655967..8fc31e871b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -63,7 +63,7 @@ linters: - durationcheck - errchkjson - forcetypeassert - # - gci + - gci - gochecknoinits - goconst - gocritic diff --git a/agent/rpc/auth_client_grpc.go b/agent/rpc/auth_client_grpc.go index e10b2f3313..94a01151e2 100644 --- a/agent/rpc/auth_client_grpc.go +++ b/agent/rpc/auth_client_grpc.go @@ -18,9 +18,9 @@ import ( "context" "time" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc/proto" - "google.golang.org/grpc" + + "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc/proto" ) type AuthClient struct { diff --git a/cli/pipeline/create.go b/cli/pipeline/create.go index eecbbaa3d6..0c2cc2bed2 100644 --- a/cli/pipeline/create.go +++ b/cli/pipeline/create.go @@ -19,12 +19,11 @@ import ( "strings" "text/template" - "go.woodpecker-ci.org/woodpecker/v2/woodpecker-go/woodpecker" - "github.com/urfave/cli/v2" "go.woodpecker-ci.org/woodpecker/v2/cli/common" "go.woodpecker-ci.org/woodpecker/v2/cli/internal" + "go.woodpecker-ci.org/woodpecker/v2/woodpecker-go/woodpecker" ) var pipelineCreateCmd = &cli.Command{ diff --git a/cmd/server/main.go b/cmd/server/main.go index 72c1806c1a..4222e1911e 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -20,8 +20,8 @@ import ( _ "github.com/joho/godotenv/autoload" "github.com/urfave/cli/v2" - _ "go.woodpecker-ci.org/woodpecker/v2/cmd/server/docs" + _ "go.woodpecker-ci.org/woodpecker/v2/cmd/server/docs" "go.woodpecker-ci.org/woodpecker/v2/version" ) diff --git a/cmd/server/server.go b/cmd/server/server.go index 3956427a5a..20a7e4ed2d 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -51,8 +51,6 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/web" "go.woodpecker-ci.org/woodpecker/v2/shared/constant" "go.woodpecker-ci.org/woodpecker/v2/version" - // "go.woodpecker-ci.org/woodpecker/v2/server/plugins/encryption" - // encryptedStore "go.woodpecker-ci.org/woodpecker/v2/server/plugins/encryption/wrapper/store" ) func run(c *cli.Context) error { diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 28baf28201..cd3a1b636b 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -24,21 +24,19 @@ import ( "time" "github.com/rs/zerolog/log" - "gopkg.in/yaml.v3" - - "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" - "github.com/urfave/cli/v2" + "gopkg.in/yaml.v3" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" + // To authenticate to GCP K8s clusters + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - // To authenticate to GCP K8s clusters - _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" + "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" ) const ( diff --git a/pipeline/backend/local/clone.go b/pipeline/backend/local/clone.go index 5642ed2c72..0e49afb652 100644 --- a/pipeline/backend/local/clone.go +++ b/pipeline/backend/local/clone.go @@ -26,6 +26,7 @@ import ( "strings" "github.com/rs/zerolog/log" + "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" ) diff --git a/pipeline/frontend/metadata/drone_compatibility_test.go b/pipeline/frontend/metadata/drone_compatibility_test.go index ff77135267..292d45e679 100644 --- a/pipeline/frontend/metadata/drone_compatibility_test.go +++ b/pipeline/frontend/metadata/drone_compatibility_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/metadata" ) diff --git a/pipeline/frontend/yaml/matrix/matrix.go b/pipeline/frontend/yaml/matrix/matrix.go index d9c6729ee6..b89dfc5d93 100644 --- a/pipeline/frontend/yaml/matrix/matrix.go +++ b/pipeline/frontend/yaml/matrix/matrix.go @@ -17,9 +17,9 @@ package matrix import ( "strings" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" - "codeberg.org/6543/xyaml" + + "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" ) const ( diff --git a/pipeline/frontend/yaml/types/base/base_types_test.go b/pipeline/frontend/yaml/types/base/base_types_test.go index 3813036472..c3c5cdf44a 100644 --- a/pipeline/frontend/yaml/types/base/base_types_test.go +++ b/pipeline/frontend/yaml/types/base/base_types_test.go @@ -18,9 +18,8 @@ import ( "fmt" "testing" - "gopkg.in/yaml.v3" - "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) type StructStringorInt struct { diff --git a/pipeline/frontend/yaml/types/network_test.go b/pipeline/frontend/yaml/types/network_test.go index bdb09bf9e9..2eb95ccda3 100644 --- a/pipeline/frontend/yaml/types/network_test.go +++ b/pipeline/frontend/yaml/types/network_test.go @@ -17,9 +17,8 @@ package types import ( "testing" - "gopkg.in/yaml.v3" - "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) func TestMarshalNetworks(t *testing.T) { diff --git a/pipeline/frontend/yaml/types/volume_test.go b/pipeline/frontend/yaml/types/volume_test.go index 07270b1a42..a6e27a0fe3 100644 --- a/pipeline/frontend/yaml/types/volume_test.go +++ b/pipeline/frontend/yaml/types/volume_test.go @@ -17,9 +17,8 @@ package types import ( "testing" - "gopkg.in/yaml.v3" - "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) func TestMarshalVolumes(t *testing.T) { diff --git a/pipeline/stepBuilder.go b/pipeline/stepBuilder.go index 7d654757f2..ffae30c441 100644 --- a/pipeline/stepBuilder.go +++ b/pipeline/stepBuilder.go @@ -26,16 +26,15 @@ import ( backend_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" - yaml_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/types" - forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/metadata" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/compiler" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/linter" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/matrix" + yaml_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/types" "go.woodpecker-ci.org/woodpecker/v2/server" + forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/api/global_secret.go b/server/api/global_secret.go index ed26785007..d36cb5d27e 100644 --- a/server/api/global_secret.go +++ b/server/api/global_secret.go @@ -19,10 +19,9 @@ import ( "github.com/gin-gonic/gin" - "go.woodpecker-ci.org/woodpecker/v2/server/router/middleware/session" - "go.woodpecker-ci.org/woodpecker/v2/server" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/router/middleware/session" ) // GetGlobalSecretList diff --git a/server/api/org.go b/server/api/org.go index b304c5c038..4474b7193a 100644 --- a/server/api/org.go +++ b/server/api/org.go @@ -19,9 +19,8 @@ import ( "strconv" "strings" - "github.com/rs/zerolog/log" - "github.com/gin-gonic/gin" + "github.com/rs/zerolog/log" "go.woodpecker-ci.org/woodpecker/v2/server" "go.woodpecker-ci.org/woodpecker/v2/server/model" diff --git a/server/api/org_secret.go b/server/api/org_secret.go index 2a8a219586..341baa94db 100644 --- a/server/api/org_secret.go +++ b/server/api/org_secret.go @@ -20,10 +20,9 @@ import ( "github.com/gin-gonic/gin" - "go.woodpecker-ci.org/woodpecker/v2/server/router/middleware/session" - "go.woodpecker-ci.org/woodpecker/v2/server" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/router/middleware/session" ) // GetOrgSecret diff --git a/server/api/signature_public_key.go b/server/api/signature_public_key.go index 0846f0c968..937f900247 100644 --- a/server/api/signature_public_key.go +++ b/server/api/signature_public_key.go @@ -21,6 +21,7 @@ import ( "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" + "go.woodpecker-ci.org/woodpecker/v2/server" ) diff --git a/server/cache/membership.go b/server/cache/membership.go index 467a5a2a81..0dd1eb3fa4 100644 --- a/server/cache/membership.go +++ b/server/cache/membership.go @@ -19,10 +19,10 @@ import ( "fmt" "time" + "github.com/jellydator/ttlcache/v3" + "go.woodpecker-ci.org/woodpecker/v2/server/forge" "go.woodpecker-ci.org/woodpecker/v2/server/model" - - "github.com/jellydator/ttlcache/v3" ) // MembershipService is a service to check for user membership. diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go index e9dda42eea..949334dcb3 100644 --- a/server/forge/bitbucket/internal/client.go +++ b/server/forge/bitbucket/internal/client.go @@ -23,10 +23,10 @@ import ( "net/http" "net/url" - shared_utils "go.woodpecker-ci.org/woodpecker/v2/shared/utils" - "golang.org/x/oauth2" "golang.org/x/oauth2/bitbucket" + + shared_utils "go.woodpecker-ci.org/woodpecker/v2/shared/utils" ) const ( diff --git a/server/logging/log_test.go b/server/logging/log_test.go index ffa4166d3f..3ca442e2a3 100644 --- a/server/logging/log_test.go +++ b/server/logging/log_test.go @@ -21,6 +21,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/plugins/config/http.go b/server/plugins/config/http.go index e3a393ff7d..9af77e80ce 100644 --- a/server/plugins/config/http.go +++ b/server/plugins/config/http.go @@ -18,7 +18,6 @@ import ( "context" "crypto" "fmt" - httpStatus "net/http" forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" diff --git a/server/plugins/encryption/aes_encryption.go b/server/plugins/encryption/aes_encryption.go index 7bf2dae522..b7b5016ffb 100644 --- a/server/plugins/encryption/aes_encryption.go +++ b/server/plugins/encryption/aes_encryption.go @@ -20,10 +20,10 @@ import ( "errors" "fmt" - "go.woodpecker-ci.org/woodpecker/v2/server/store/types" "golang.org/x/crypto/bcrypt" - "golang.org/x/crypto/sha3" + + "go.woodpecker-ci.org/woodpecker/v2/server/store/types" ) func (svc *aesEncryptionService) loadCipher(password string) error { diff --git a/server/plugins/permissions/repo_owners_test.go b/server/plugins/permissions/repo_owners_test.go index 4ecd363219..c348212552 100644 --- a/server/plugins/permissions/repo_owners_test.go +++ b/server/plugins/permissions/repo_owners_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/plugins/secrets/builtin_test.go b/server/plugins/secrets/builtin_test.go index c2f2509ac9..6c35aebfd7 100644 --- a/server/plugins/secrets/builtin_test.go +++ b/server/plugins/secrets/builtin_test.go @@ -20,6 +20,7 @@ import ( "github.com/franela/goblin" "github.com/stretchr/testify/mock" + "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/plugins/secrets" mocks_store "go.woodpecker-ci.org/woodpecker/v2/server/store/mocks" diff --git a/server/plugins/utils/http_test.go b/server/plugins/utils/http_test.go index 4426e82ffa..150538b56e 100644 --- a/server/plugins/utils/http_test.go +++ b/server/plugins/utils/http_test.go @@ -24,6 +24,7 @@ import ( "github.com/go-ap/httpsig" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/plugins/utils" ) diff --git a/server/queue/fifo.go b/server/queue/fifo.go index 110f6d5db8..f690352b12 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -21,9 +21,9 @@ import ( "sync" "time" - "go.woodpecker-ci.org/woodpecker/v2/server/model" - "github.com/rs/zerolog/log" + + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) type entry struct { diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index 49d191e3ad..873ffeffe6 100644 --- a/server/queue/fifo_test.go +++ b/server/queue/fifo_test.go @@ -23,6 +23,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/router/middleware/session/pagination.go b/server/router/middleware/session/pagination.go index a9021100c4..9bf4455c10 100644 --- a/server/router/middleware/session/pagination.go +++ b/server/router/middleware/session/pagination.go @@ -18,6 +18,7 @@ import ( "strconv" "github.com/gin-gonic/gin" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/router/middleware/session/repo.go b/server/router/middleware/session/repo.go index 93366887e3..e61120b641 100644 --- a/server/router/middleware/session/repo.go +++ b/server/router/middleware/session/repo.go @@ -23,8 +23,8 @@ import ( "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" - "go.woodpecker-ci.org/woodpecker/v2/server" + "go.woodpecker-ci.org/woodpecker/v2/server" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/store" "go.woodpecker-ci.org/woodpecker/v2/server/store/types" diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index 9ed94bae96..520fa40526 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -18,13 +18,13 @@ import ( "net/http" "strconv" + "github.com/gin-gonic/gin" + "github.com/rs/zerolog/log" + "go.woodpecker-ci.org/woodpecker/v2/server" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/store" "go.woodpecker-ci.org/woodpecker/v2/shared/token" - - "github.com/gin-gonic/gin" - "github.com/rs/zerolog/log" ) func User(c *gin.Context) *model.User { diff --git a/server/store/datastore/cron.go b/server/store/datastore/cron.go index ed3e23f9be..c6c7713cb5 100644 --- a/server/store/datastore/cron.go +++ b/server/store/datastore/cron.go @@ -15,9 +15,9 @@ package datastore import ( - "go.woodpecker-ci.org/woodpecker/v2/server/model" - "xorm.io/builder" + + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) func (s storage) CronCreate(cron *model.Cron) error { diff --git a/server/store/datastore/engine.go b/server/store/datastore/engine.go index fb5a5c082e..20b1b9b546 100644 --- a/server/store/datastore/engine.go +++ b/server/store/datastore/engine.go @@ -16,12 +16,11 @@ package datastore import ( "github.com/rs/zerolog" + "xorm.io/xorm" + xlog "xorm.io/xorm/log" "go.woodpecker-ci.org/woodpecker/v2/server/store" "go.woodpecker-ci.org/woodpecker/v2/server/store/datastore/migration" - - "xorm.io/xorm" - xlog "xorm.io/xorm/log" ) type storage struct { diff --git a/server/store/datastore/engine_test.go b/server/store/datastore/engine_test.go index a9f4222243..b8e16fadb6 100644 --- a/server/store/datastore/engine_test.go +++ b/server/store/datastore/engine_test.go @@ -19,10 +19,9 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "xorm.io/xorm" "xorm.io/xorm/schemas" - - "github.com/stretchr/testify/assert" ) func testDriverConfig() (driver, config string) { diff --git a/server/store/datastore/helper_test.go b/server/store/datastore/helper_test.go index 7beff99f33..be8b835e6e 100644 --- a/server/store/datastore/helper_test.go +++ b/server/store/datastore/helper_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/store/types" ) diff --git a/server/store/datastore/log_test.go b/server/store/datastore/log_test.go index 1aa02d2854..3e6729844d 100644 --- a/server/store/datastore/log_test.go +++ b/server/store/datastore/log_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/store/datastore/migration/migration_test.go b/server/store/datastore/migration/migration_test.go index d8985d5906..858b5f7cdc 100644 --- a/server/store/datastore/migration/migration_test.go +++ b/server/store/datastore/migration/migration_test.go @@ -19,14 +19,13 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" - "xorm.io/xorm" - "xorm.io/xorm/schemas" - // blank imports to register the sql drivers _ "github.com/go-sql-driver/mysql" _ "github.com/lib/pq" _ "github.com/mattn/go-sqlite3" + "github.com/stretchr/testify/assert" + "xorm.io/xorm" + "xorm.io/xorm/schemas" ) const ( diff --git a/server/web/web_test.go b/server/web/web_test.go index e09df0e710..52e7b255ed 100644 --- a/server/web/web_test.go +++ b/server/web/web_test.go @@ -22,6 +22,7 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/server" ) From 28c14500f70fda4e6603fb8e65cc1ae294a62347 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 10:49:15 +0100 Subject: [PATCH 05/24] cleanup --- .golangci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 8fc31e871b..b7fd34379b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -58,7 +58,6 @@ linters: - asciicheck - bodyclose - contextcheck - # - depguard - dogsled - durationcheck - errchkjson From bf40adb833e9f40b09a590ba622f9020e1fc0e55 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 11:12:05 +0100 Subject: [PATCH 06/24] temp disable gci --- .golangci.yml | 2 +- pipeline/backend/kubernetes/kubernetes.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index b7fd34379b..db9c040c93 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -62,7 +62,7 @@ linters: - durationcheck - errchkjson - forcetypeassert - - gci + # - gci - gochecknoinits - goconst - gocritic diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index cd3a1b636b..26c9ad011c 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -31,6 +31,7 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" + // To authenticate to GCP K8s clusters _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "k8s.io/client-go/rest" From 239b6461b6143e5669b470ade67e77282cb5ac64 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 11:20:20 +0100 Subject: [PATCH 07/24] cleanup --- pipeline/frontend/yaml/types/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/frontend/yaml/types/network.go b/pipeline/frontend/yaml/types/network.go index 8e21ed0a9b..4d7660a4c2 100644 --- a/pipeline/frontend/yaml/types/network.go +++ b/pipeline/frontend/yaml/types/network.go @@ -76,7 +76,7 @@ func (n *Networks) UnmarshalYAML(unmarshal func(any) error) error { return nil } - return errors.New("failed to unmarshal networks") + return errors.New("failed to unmarshal Networks") } func handleNetwork(name string, value any) (*Network, error) { From 8427a2e13032f6b93c3cabed418b65660f7a09f9 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 11:30:35 +0100 Subject: [PATCH 08/24] cleanup cookieMaxAgeSeconds --- shared/httputil/httputil.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/shared/httputil/httputil.go b/shared/httputil/httputil.go index a2328b7ce7..3f3fdf7f7f 100644 --- a/shared/httputil/httputil.go +++ b/shared/httputil/httputil.go @@ -15,6 +15,7 @@ package httputil import ( + "math" "net/http" "strings" ) @@ -40,9 +41,6 @@ func IsHTTPS(r *http.Request) bool { // SetCookie writes the cookie value. func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { - // the cookie value (token) is responsible for expiration - cookieMaxAgeSeconds := 2147483647 - cookie := http.Cookie{ Name: name, Value: value, @@ -50,7 +48,7 @@ func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { Domain: r.URL.Host, HttpOnly: true, Secure: IsHTTPS(r), - MaxAge: cookieMaxAgeSeconds, + MaxAge: math.MaxInt32, } http.SetCookie(w, &cookie) From 9b6eb03bc5c19c57a10a15cf6de8090561cafbc2 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 11:31:42 +0100 Subject: [PATCH 09/24] add back comment --- shared/httputil/httputil.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/httputil/httputil.go b/shared/httputil/httputil.go index 3f3fdf7f7f..ce44152d53 100644 --- a/shared/httputil/httputil.go +++ b/shared/httputil/httputil.go @@ -48,7 +48,7 @@ func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { Domain: r.URL.Host, HttpOnly: true, Secure: IsHTTPS(r), - MaxAge: math.MaxInt32, + MaxAge: math.MaxInt32, // the cookie value (token) is responsible for expiration } http.SetCookie(w, &cookie) From 1d39a1d7e9f2e4c4ccddfb7eacf7e9100814b059 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Mon, 18 Dec 2023 21:19:52 +0100 Subject: [PATCH 10/24] replace strings.SplitN(..., 2) by strings.Cut --- cli/exec/exec.go | 8 ++++---- cli/internal/util.go | 6 +++--- cli/pipeline/create.go | 6 +++--- cmd/agent/agent.go | 13 ++++++------- .../frontend/metadata/drone_compatibility_test.go | 6 +++--- pipeline/frontend/yaml/types/base/map.go | 11 ++--------- server/plugins/environments/parse.go | 8 ++++---- server/plugins/registry/filesystem.go | 8 ++++---- 8 files changed, 29 insertions(+), 37 deletions(-) diff --git a/cli/exec/exec.go b/cli/exec/exec.go index c9b7ab32c1..029072916b 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -123,13 +123,13 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error droneEnv := make(map[string]string) for _, env := range c.StringSlice("env") { - envs := strings.SplitN(env, "=", 2) - droneEnv[envs[0]] = envs[1] - if _, exists := environ[envs[0]]; exists { + before, after, _ := strings.Cut(env, "=") + droneEnv[before] = after + if _, exists := environ[before]; exists { // don't override existing values continue } - environ[envs[0]] = envs[1] + environ[before] = before } tmpl, err := envsubst.ParseFile(file) diff --git a/cli/internal/util.go b/cli/internal/util.go index 019619c353..921c1e7ec9 100644 --- a/cli/internal/util.go +++ b/cli/internal/util.go @@ -107,11 +107,11 @@ func ParseRepo(client woodpecker.Client, str string) (repoID int64, err error) { func ParseKeyPair(p []string) map[string]string { params := map[string]string{} for _, i := range p { - parts := strings.SplitN(i, "=", 2) - if len(parts) != 2 { //nolint:gomnd + before, after, ok := strings.Cut(i, "=") + if !ok || before == "" { continue } - params[parts[0]] = parts[1] + params[before] = after } return params } diff --git a/cli/pipeline/create.go b/cli/pipeline/create.go index 0c2cc2bed2..3db896a9f1 100644 --- a/cli/pipeline/create.go +++ b/cli/pipeline/create.go @@ -60,9 +60,9 @@ func pipelineCreate(c *cli.Context) error { variables := make(map[string]string) for _, vaz := range c.StringSlice("var") { - sp := strings.SplitN(vaz, "=", 2) - if len(sp) == 2 { //nolint:gomnd - variables[sp[0]] = sp[1] + before, after, _ := strings.Cut(vaz, "=") + if before != "" && after != "" { + variables[before] = after } } diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index 7181e59e3f..ce6858a862 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -288,14 +288,13 @@ func stringSliceAddToMap(sl []string, m map[string]string) error { if m == nil { m = make(map[string]string) } - //nolint: gomnd for _, v := range sl { - parts := strings.SplitN(v, "=", 2) - switch len(parts) { - case 2: - m[parts[0]] = parts[1] - case 1: - return fmt.Errorf("key '%s' does not have a value assigned", parts[0]) + before, after, _ := strings.Cut(v, "=") + switch { + case before != "" && after != "": + m[before] = after + case after != "": + return fmt.Errorf("key '%s' does not have a value assigned", before) default: return fmt.Errorf("empty string in slice") } diff --git a/pipeline/frontend/metadata/drone_compatibility_test.go b/pipeline/frontend/metadata/drone_compatibility_test.go index 292d45e679..c2334585c2 100644 --- a/pipeline/frontend/metadata/drone_compatibility_test.go +++ b/pipeline/frontend/metadata/drone_compatibility_test.go @@ -113,11 +113,11 @@ DRONE_TARGET_BRANCH=main` func convertListToEnvMap(t *testing.T, list string) map[string]string { result := make(map[string]string) for _, s := range strings.Split(list, "\n") { - ss := strings.SplitN(strings.TrimSpace(s), "=", 2) - if len(ss) != 2 { + before, after, _ := strings.Cut(strings.TrimSpace(s), "=") + if before == "" || after == "" { t.Fatal("helper function got invalid test data") } - result[ss[0]] = ss[1] + result[before] = after } return result } diff --git a/pipeline/frontend/yaml/types/base/map.go b/pipeline/frontend/yaml/types/base/map.go index cead91b188..8d91be9914 100644 --- a/pipeline/frontend/yaml/types/base/map.go +++ b/pipeline/frontend/yaml/types/base/map.go @@ -30,15 +30,8 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { parts := map[string]string{} for _, s := range sliceType { if str, ok := s.(string); ok { - str := strings.TrimSpace(str) - keyValueSlice := strings.SplitN(str, "=", 2) - - key := keyValueSlice[0] - val := "" - if len(keyValueSlice) == 2 { //nolint: gomnd - val = keyValueSlice[1] - } - parts[key] = val + before, after, _ := strings.Cut(strings.TrimSpace(str), "=") + parts[before] = after } else { return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", s, s) } diff --git a/server/plugins/environments/parse.go b/server/plugins/environments/parse.go index 09935a5a13..c7836721c0 100644 --- a/server/plugins/environments/parse.go +++ b/server/plugins/environments/parse.go @@ -31,13 +31,13 @@ func Parse(params []string) model.EnvironService { var globals []*model.Environ for _, item := range params { - kvPair := strings.SplitN(item, ":", 2) - if len(kvPair) != 2 { + before, after, _ := strings.Cut(item, ":") + if after != "" { // ignore items only containing a key and no value - log.Warn().Msgf("key '%s' has no value, will be ignored", kvPair[0]) + log.Warn().Msgf("key '%s' has no value, will be ignored", before) continue } - globals = append(globals, &model.Environ{Name: kvPair[0], Value: kvPair[1]}) + globals = append(globals, &model.Environ{Name: before, Value: after}) } return &builtin{globals} } diff --git a/server/plugins/registry/filesystem.go b/server/plugins/registry/filesystem.go index 26d8ca164d..dab4e17f6a 100644 --- a/server/plugins/registry/filesystem.go +++ b/server/plugins/registry/filesystem.go @@ -113,10 +113,10 @@ func decodeAuth(authStr string) (string, string, error) { if n > decLen { return "", "", fmt.Errorf("something went wrong decoding auth config") } - arr := strings.SplitN(string(decoded), ":", 2) - if len(arr) != 2 { //nolint:gomnd + before, after, _ := strings.Cut(string(decoded), ":") + if before == "" || after == "" { return "", "", fmt.Errorf("invalid auth configuration file") } - password := strings.Trim(arr[1], "\x00") - return arr[0], password, nil + password := strings.Trim(after, "\x00") + return before, password, nil } From bf377377d07042d11c31521670d2c5988b2599ba Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 20 Dec 2023 14:47:43 +0100 Subject: [PATCH 11/24] rebase --- pipeline/backend/kubernetes/service.go | 9 +++------ pipeline/backend/kubernetes/service_test.go | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index 2fa66a462b..6930f743ea 100644 --- a/pipeline/backend/kubernetes/service.go +++ b/pipeline/backend/kubernetes/service.go @@ -26,7 +26,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" ) -func mkService(namespace, name string, ports []uint16, selector map[string]string) (*v1.Service, error) { +func mkService(namespace, name string, ports []uint16, selector map[string]string) *v1.Service { log.Trace().Str("name", name).Interface("selector", selector).Interface("ports", ports).Msg("Creating service") var svcPorts []v1.ServicePort @@ -48,7 +48,7 @@ func mkService(namespace, name string, ports []uint16, selector map[string]strin Selector: selector, Ports: svcPorts, }, - }, nil + } } func serviceName(step *types.Step) (string, error) { @@ -69,10 +69,7 @@ func startService(ctx context.Context, engine *kube, step *types.Step) (*v1.Serv StepLabel: podName, } - svc, err := mkService(engine.config.Namespace, name, step.Ports, selector) - if err != nil { - return nil, err - } + svc := mkService(engine.config.Namespace, name, step.Ports, selector) return engine.client.CoreV1().Services(engine.config.Namespace).Create(ctx, svc, metav1.CreateOptions{}) } diff --git a/pipeline/backend/kubernetes/service_test.go b/pipeline/backend/kubernetes/service_test.go index 5dc06509ff..f4b479f60f 100644 --- a/pipeline/backend/kubernetes/service_test.go +++ b/pipeline/backend/kubernetes/service_test.go @@ -71,7 +71,7 @@ func TestService(t *testing.T) { } }` - s, _ := mkService("foo", "bar", []uint16{1, 2, 3}, map[string]string{"step": "baz"}) + s := mkService("foo", "bar", []uint16{1, 2, 3}, map[string]string{"step": "baz"}) j, err := json.Marshal(s) assert.NoError(t, err) assert.JSONEq(t, expected, string(j)) From f69e4dc01c7a8c9615afb71cbad64c828c789061 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 20 Dec 2023 15:04:22 +0100 Subject: [PATCH 12/24] cleanup --- cli/exec/exec.go | 2 +- cmd/agent/agent.go | 1 + pipeline/backend/backend.go | 4 ++-- pipeline/backend/kubernetes/kubernetes.go | 7 +++++-- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cli/exec/exec.go b/cli/exec/exec.go index 029072916b..300d452703 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -213,7 +213,7 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error } backendCtx := context.WithValue(c.Context, backendTypes.CliContext, c) - backend.Init() + backend.Init(backendCtx) backendEngine, err := backend.FindBackend(backendCtx, c.String("backend-engine")) if err != nil { diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index ce6858a862..6cf98039c0 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -261,6 +261,7 @@ func getBackendEngine(backendCtx context.Context, backendName string, addons []s return addonBackend.Value, nil } + backend.Init(backendCtx) engine, err := backend.FindBackend(backendCtx, backendName) if err != nil { log.Error().Err(err).Msgf("cannot find backend engine '%s'", backendName) diff --git a/pipeline/backend/backend.go b/pipeline/backend/backend.go index 99ecb69134..56d3291d75 100644 --- a/pipeline/backend/backend.go +++ b/pipeline/backend/backend.go @@ -29,11 +29,11 @@ var ( backends []types.Backend ) -func Init() { +func Init(ctx context.Context) { backends = []types.Backend{ docker.New(), local.New(), - kubernetes.New(), + kubernetes.New(ctx), } backendsByName = make(map[string]types.Backend) diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 26c9ad011c..842ced54f0 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -47,6 +47,7 @@ const ( var defaultDeleteOptions = newDefaultDeleteOptions() type kube struct { + ctx context.Context client kubernetes.Interface config *config goos string @@ -110,8 +111,10 @@ func configFromCliContext(ctx context.Context) (*config, error) { } // New returns a new Kubernetes Backend. -func New() types.Backend { - return &kube{} +func New(ctx context.Context) types.Backend { + return &kube{ + ctx: ctx, + } } func (e *kube) Name() string { From ac03e5dfb1dfac372d316cf6ae2ed9db1acf16c3 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 20 Dec 2023 15:21:17 +0100 Subject: [PATCH 13/24] move ExitCodeKilled const into pipeline package --- agent/runner.go | 5 ++--- pipeline/const.go | 17 +++++++++++++++++ server/model/const.go | 2 -- server/pipeline/stepStatus.go | 5 +++-- server/pipeline/stepStatus_test.go | 15 ++++++++------- 5 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 pipeline/const.go diff --git a/agent/runner.go b/agent/runner.go index 8d8e549342..b640d55803 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -29,7 +29,6 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline" backend "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" - "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/shared/utils" ) @@ -159,7 +158,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { if canceled.IsSet() { state.Error = "" - state.ExitCode = model.ExitCodeKilled + state.ExitCode = pipeline.ExitCodeKilled } else if err != nil { pExitError := &pipeline.ExitError{} switch { @@ -167,7 +166,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { state.ExitCode = pExitError.Code case errors.Is(err, pipeline.ErrCancel): state.Error = "" - state.ExitCode = model.ExitCodeKilled + state.ExitCode = pipeline.ExitCodeKilled canceled.SetTo(true) default: state.ExitCode = 1 diff --git a/pipeline/const.go b/pipeline/const.go new file mode 100644 index 0000000000..a4bec9789b --- /dev/null +++ b/pipeline/const.go @@ -0,0 +1,17 @@ +// Copyright 2023 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pipeline + +const ExitCodeKilled int = 137 diff --git a/server/model/const.go b/server/model/const.go index 4823786e2e..89760d730d 100644 --- a/server/model/const.go +++ b/server/model/const.go @@ -62,8 +62,6 @@ const ( StatusBlocked StatusValue = "blocked" // waiting for approval StatusDeclined StatusValue = "declined" // blocked and declined StatusCreated StatusValue = "created" // created / internal use only - - ExitCodeKilled int = 137 ) // SCMKind represent different version control systems diff --git a/server/pipeline/stepStatus.go b/server/pipeline/stepStatus.go index 122b79dfe1..5b47240339 100644 --- a/server/pipeline/stepStatus.go +++ b/server/pipeline/stepStatus.go @@ -18,6 +18,7 @@ package pipeline import ( "time" + "go.woodpecker-ci.org/woodpecker/v2/pipeline" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" ) @@ -31,7 +32,7 @@ func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.S if state.ExitCode != 0 || state.Error != "" { step.State = model.StatusFailure } - if state.ExitCode == model.ExitCodeKilled { + if state.ExitCode == pipeline.ExitCodeKilled { step.State = model.StatusKilled } } else { @@ -81,6 +82,6 @@ func UpdateStepToStatusKilled(store model.UpdateStepStore, step model.Step) (*mo if step.Started == 0 { step.Started = step.Stopped } - step.ExitCode = model.ExitCodeKilled + step.ExitCode = pipeline.ExitCodeKilled return &step, store.StepUpdate(&step) } diff --git a/server/pipeline/stepStatus_test.go b/server/pipeline/stepStatus_test.go index d7b1292a40..6121c0515a 100644 --- a/server/pipeline/stepStatus_test.go +++ b/server/pipeline/stepStatus_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" + "go.woodpecker-ci.org/woodpecker/v2/pipeline" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" ) @@ -39,7 +40,7 @@ func TestUpdateStepStatusNotExited(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: model.ExitCodeKilled, + ExitCode: pipeline.ExitCodeKilled, Error: "not an error", } step := &model.Step{} @@ -69,7 +70,7 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: model.ExitCodeKilled, + ExitCode: pipeline.ExitCodeKilled, Error: "not an error", } err := UpdateStepStatus(&mockUpdateStepStore{}, step, state, int64(42)) @@ -96,7 +97,7 @@ func TestUpdateStepStatusExited(t *testing.T) { Started: int64(42), Exited: true, Finished: int64(34), - ExitCode: model.ExitCodeKilled, + ExitCode: pipeline.ExitCodeKilled, Error: "an error", } @@ -111,8 +112,8 @@ func TestUpdateStepStatusExited(t *testing.T) { t.Errorf("Step started not equals 42 != %d", step.Started) case step.Stopped != int64(34): t.Errorf("Step stopped not equals 34 != %d", step.Stopped) - case step.ExitCode != model.ExitCodeKilled: - t.Errorf("Step exit code not equals %d != %d", model.ExitCodeKilled, step.ExitCode) + case step.ExitCode != pipeline.ExitCodeKilled: + t.Errorf("Step exit code not equals %d != %d", pipeline.ExitCodeKilled, step.ExitCode) case step.Error != "an error": t.Errorf("Step error not equals 'an error' != '%s'", step.Error) } @@ -288,8 +289,8 @@ func TestUpdateStepToStatusKilledStarted(t *testing.T) { t.Errorf("Step stopped not equals %d < %d", now, step.Stopped) case step.Started != step.Stopped: t.Errorf("Step started not equals %d != %d", step.Stopped, step.Started) - case step.ExitCode != model.ExitCodeKilled: - t.Errorf("Step exit code not equals %d != %d", model.ExitCodeKilled, step.ExitCode) + case step.ExitCode != pipeline.ExitCodeKilled: + t.Errorf("Step exit code not equals %d != %d", pipeline.ExitCodeKilled, step.ExitCode) } } From e30fdc2c488571d6a5135dea691a263ed46617d4 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 20 Dec 2023 16:13:41 +0100 Subject: [PATCH 14/24] use pipeline_errors --- pipeline/stepBuilder.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pipeline/stepBuilder.go b/pipeline/stepBuilder.go index ffae30c441..f4f22bf38e 100644 --- a/pipeline/stepBuilder.go +++ b/pipeline/stepBuilder.go @@ -25,7 +25,7 @@ import ( "go.uber.org/multierr" backend_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" + pipeline_errors "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/metadata" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml" @@ -87,7 +87,7 @@ func (b *StepBuilder) Build() (items []*Item, errorsAndWarnings error) { workflow.AxisID = i + 1 } item, err := b.genItemForWorkflow(workflow, axis, string(y.Data)) - if err != nil && errors.HasBlockingErrors(err) { + if err != nil && pipeline_errors.HasBlockingErrors(err) { return nil, err } else if err != nil { errorsAndWarnings = multierr.Append(errorsAndWarnings, err) @@ -136,7 +136,7 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A // parse yaml pipeline parsed, err := yaml.ParseString(substituted) if err != nil { - return nil, &errors.PipelineError{Message: err.Error(), Type: errors.PipelineErrorTypeCompiler} + return nil, &pipeline_errors.PipelineError{Message: err.Error(), Type: pipeline_errors.PipelineErrorTypeCompiler} } // lint pipeline @@ -147,7 +147,7 @@ func (b *StepBuilder) genItemForWorkflow(workflow *model.Workflow, axis matrix.A File: workflow.Name, RawConfig: data, }})) - if errors.HasBlockingErrors(errorsAndWarnings) { + if pipeline_errors.HasBlockingErrors(errorsAndWarnings) { return nil, errorsAndWarnings } From 2789e3c7d903837292b52bf64a7d40498d70ee0e Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 20 Dec 2023 16:54:43 +0100 Subject: [PATCH 15/24] add error handling for pods UpdateFunc --- pipeline/backend/kubernetes/kubernetes.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 842ced54f0..59cb08edbe 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -209,7 +209,12 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) finished := make(chan bool) podUpdated := func(old, new any) { - pod, _ := new.(*v1.Pod) + pod, ok := new.(*v1.Pod) + if !ok { + log.Error().Msgf("Unexpected object while awaiting Pod: %+v", pod) + return + } + if pod.Name == podName { if isImagePullBackOffState(pod) { finished <- true @@ -269,7 +274,12 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) up := make(chan bool) podUpdated := func(old, new any) { - pod, _ := new.(*v1.Pod) + pod, ok := new.(*v1.Pod) + if !ok { + log.Error().Msgf("Unexpected object while awaiting Pod: %+v", pod) + return + } + if pod.Name == podName { switch pod.Status.Phase { case v1.PodRunning, v1.PodSucceeded, v1.PodFailed: From abf5366b353b87c95bc1afe0cfabdc0766ffe388 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 21 Dec 2023 12:34:14 +0100 Subject: [PATCH 16/24] add comment to refactor server import in pipeline --- pipeline/stepBuilder.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pipeline/stepBuilder.go b/pipeline/stepBuilder.go index f4f22bf38e..e51c7b6748 100644 --- a/pipeline/stepBuilder.go +++ b/pipeline/stepBuilder.go @@ -34,6 +34,8 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/matrix" yaml_types "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/types" "go.woodpecker-ci.org/woodpecker/v2/server" + + // TODO: Import of the server package should not be allowed and must be refactored forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" "go.woodpecker-ci.org/woodpecker/v2/server/model" ) From 3be90d0476989210d1b70a54b724af7a5362342f Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 21 Dec 2023 12:41:49 +0100 Subject: [PATCH 17/24] copy status enums to pipeline package --- agent/tracer.go | 6 +++--- pipeline/const.go | 16 ++++++++++++++++ pipeline/tracer.go | 10 ++++------ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/agent/tracer.go b/agent/tracer.go index 36d210b723..c40adc78df 100644 --- a/agent/tracer.go +++ b/agent/tracer.go @@ -68,7 +68,7 @@ func (r *Runner) createTracer(ctxmeta context.Context, logger zerolog.Logger, wo // TODO: find better way to update this state and move it to pipeline to have the same env in cli-exec state.Pipeline.Step.Environment["CI_MACHINE"] = r.hostname - state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = "success" + state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(pipeline.StatusSuccess) state.Pipeline.Step.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Pipeline.Time, 10) state.Pipeline.Step.Environment["CI_PIPELINE_FINISHED"] = strconv.FormatInt(time.Now().Unix(), 10) @@ -79,8 +79,8 @@ func (r *Runner) createTracer(ctxmeta context.Context, logger zerolog.Logger, wo state.Pipeline.Step.Environment["CI_SYSTEM_PLATFORM"] = runtime.GOOS + "/" + runtime.GOARCH if state.Pipeline.Error != nil { - state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = "failure" - state.Pipeline.Step.Environment["CI_STEP_STATUS"] = "failure" + state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(pipeline.StatusFailure) + state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(pipeline.StatusFailure) } return nil diff --git a/pipeline/const.go b/pipeline/const.go index a4bec9789b..be0592f01f 100644 --- a/pipeline/const.go +++ b/pipeline/const.go @@ -14,4 +14,20 @@ package pipeline +// StatusValue represent pipeline states woodpecker know +type StatusValue string // @name StatusValue + +const ( + StatusSkipped StatusValue = "skipped" // skipped as another step failed + StatusPending StatusValue = "pending" // pending to be executed + StatusRunning StatusValue = "running" // currently running + StatusSuccess StatusValue = "success" // successfully finished + StatusFailure StatusValue = "failure" // failed to finish (exit code != 0) + StatusKilled StatusValue = "killed" // killed by user + StatusError StatusValue = "error" // error with the config / while parsing / some other system problem + StatusBlocked StatusValue = "blocked" // waiting for approval + StatusDeclined StatusValue = "declined" // blocked and declined + StatusCreated StatusValue = "created" // created / internal use only +) + const ExitCodeKilled int = 137 diff --git a/pipeline/tracer.go b/pipeline/tracer.go index f276162bf3..e9ab17f730 100644 --- a/pipeline/tracer.go +++ b/pipeline/tracer.go @@ -17,8 +17,6 @@ package pipeline import ( "strconv" "time" - - "go.woodpecker-ci.org/woodpecker/v2/server/model" ) // Tracer handles process tracing. @@ -45,17 +43,17 @@ var DefaultTracer = TraceFunc(func(state *State) error { if state.Pipeline.Step.Environment == nil { return nil } - state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(model.StatusSuccess) + state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(StatusSuccess) state.Pipeline.Step.Environment["CI_PIPELINE_STARTED"] = strconv.FormatInt(state.Pipeline.Time, 10) state.Pipeline.Step.Environment["CI_PIPELINE_FINISHED"] = strconv.FormatInt(time.Now().Unix(), 10) - state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(model.StatusSuccess) + state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(StatusSuccess) state.Pipeline.Step.Environment["CI_STEP_STARTED"] = strconv.FormatInt(state.Pipeline.Time, 10) state.Pipeline.Step.Environment["CI_STEP_FINISHED"] = strconv.FormatInt(time.Now().Unix(), 10) if state.Pipeline.Error != nil { - state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(model.StatusFailure) - state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(model.StatusFailure) + state.Pipeline.Step.Environment["CI_PIPELINE_STATUS"] = string(StatusFailure) + state.Pipeline.Step.Environment["CI_STEP_STATUS"] = string(StatusFailure) } return nil }) From a6bef8b35261d3d9fe9a0275226ad8f0de1b3388 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 21 Dec 2023 14:35:54 +0100 Subject: [PATCH 18/24] add workflowState type assertion error handling --- pipeline/backend/local/const.go | 5 +++-- pipeline/backend/local/local.go | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pipeline/backend/local/const.go b/pipeline/backend/local/const.go index 70594a7621..e84f5ed86d 100644 --- a/pipeline/backend/local/const.go +++ b/pipeline/backend/local/const.go @@ -31,8 +31,9 @@ var notAllowedEnvVarOverwrites = []string{ } var ( - ErrUnsupportedStepType = errors.New("unsupported step type") - ErrWorkflowStateNotFound = errors.New("workflow state not found") + ErrUnsupportedStepType = errors.New("unsupported step type") + ErrWorkflowStateNotFound = errors.New("workflow state not found") + ErrWorkflowStateMalformed = errors.New("workflow state malformed") ) const netrcFile = ` diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index 568e203d74..c40df5c1ac 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -258,7 +258,10 @@ func (e *local) getState(taskUUID string) (*workflowState, error) { return nil, ErrWorkflowStateNotFound } - workflowState, _ := state.(*workflowState) + workflowState, ok := state.(*workflowState) + if !ok { + return nil, ErrWorkflowStateMalformed + } return workflowState, nil } From 134d61f57440e7a07c37d43b91e2125c27616281 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 21 Dec 2023 14:44:09 +0100 Subject: [PATCH 19/24] add type assertion error handling --- pipeline/frontend/yaml/constraint/constraint.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go index 684fd0857b..2fa89e7bd0 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/pipeline/frontend/yaml/constraint/constraint.go @@ -195,7 +195,10 @@ func (c *Constraint) Match(m metadata.Metadata, global bool, env map[string]stri if err != nil { return false, err } - bresult, _ := result.(bool) + bresult, ok := result.(bool) + if !ok { + return false, fmt.Errorf("could not parse result: %v", result) + } match = match && bresult } From 9e5711af231b638c829f64ef10158bebebe56605 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 9 Jan 2024 21:46:08 +0100 Subject: [PATCH 20/24] make it build again --- pipeline/backend/docker/convert.go | 1 - pipeline/backend/docker/docker.go | 4 ++-- pipeline/backend/kubernetes/kubernetes.go | 2 +- server/pipeline/queue.go | 3 ++- server/pipeline/topic.go | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index b67e1693b7..5b55a84abd 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -79,7 +79,6 @@ func toHostConfig(step *types.Step) *container.HostConfig { }, Privileged: step.Privileged, ShmSize: step.ShmSize, - Sysctls: step.Sysctls, } if len(step.NetworkMode) != 0 { diff --git a/pipeline/backend/docker/docker.go b/pipeline/backend/docker/docker.go index 1809852973..2406910627 100644 --- a/pipeline/backend/docker/docker.go +++ b/pipeline/backend/docker/docker.go @@ -317,10 +317,10 @@ func (e *docker) DestroyWorkflow(ctx context.Context, conf *backend.Config, task for _, stage := range conf.Stages { for _, step := range stage.Steps { containerName := toContainerName(step) - if err := e.client.ContainerKill(noContext, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) { + if err := e.client.ContainerKill(ctx, containerName, "9"); err != nil && !isErrContainerNotFoundOrNotRunning(err) { log.Error().Err(err).Msgf("could not kill container '%s'", step.Name) } - if err := e.client.ContainerRemove(noContext, containerName, removeOpts); err != nil && !isErrContainerNotFoundOrNotRunning(err) { + if err := e.client.ContainerRemove(ctx, containerName, removeOpts); err != nil && !isErrContainerNotFoundOrNotRunning(err) { log.Error().Err(err).Msgf("could not remove container '%s'", step.Name) } } diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 56039c5b4b..89bc1b0d08 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -343,7 +343,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) return rc, nil } -func (e *kube) DestroyStep(_ context.Context, step *types.Step, taskUUID string) error { +func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID string) error { if step.Type == types.StepTypeService { // a service should be stopped by DestroyWorkflow so we can ignore it log.Trace().Msgf("DestroyStep got service '%s', ignoring it.", step.Name) diff --git a/server/pipeline/queue.go b/server/pipeline/queue.go index 81b732a1c4..c8b1fc77ee 100644 --- a/server/pipeline/queue.go +++ b/server/pipeline/queue.go @@ -15,6 +15,7 @@ package pipeline import ( + "context" "encoding/json" "fmt" @@ -24,7 +25,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/pipeline/stepbuilder" ) -func queuePipeline(repo *model.Repo, pipelineItems []*stepbuilder.Item) error { +func queuePipeline(ctx context.Context, repo *model.Repo, pipelineItems []*stepbuilder.Item) error { var tasks []*model.Task for _, item := range pipelineItems { if item.Workflow.State == model.StatusSkipped { diff --git a/server/pipeline/topic.go b/server/pipeline/topic.go index 636070b8ec..d8df0060b7 100644 --- a/server/pipeline/topic.go +++ b/server/pipeline/topic.go @@ -25,7 +25,7 @@ import ( ) // publishToTopic publishes message to UI clients -func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) (err error) { +func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) error { message := pubsub.Message{ Labels: map[string]string{ "repo": repo.FullName, @@ -41,7 +41,7 @@ func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) (err error) { }) if err != nil { log.Error().Err(err).Msg("can't marshal JSON") - return + return err } server.Config.Services.Pubsub.Publish(message) From 8e1dd4967499ffd1bf348bb58de4a524e3cfdb7c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 9 Jan 2024 21:48:08 +0100 Subject: [PATCH 21/24] part reset test files --- .../metadata/drone_compatibility_test.go | 6 ++-- .../yaml/types/base/base_types_test.go | 5 ++-- pipeline/frontend/yaml/types/network_test.go | 3 +- pipeline/frontend/yaml/types/volume_test.go | 3 +- server/pipeline/pipelineStatus_test.go | 25 +++++++--------- server/pipeline/step_status_test.go | 29 +++++++++---------- server/queue/fifo_test.go | 29 +++++++++---------- server/store/datastore/engine_test.go | 3 +- server/store/datastore/helper_test.go | 1 - .../datastore/migration/migration_test.go | 7 +++-- 10 files changed, 54 insertions(+), 57 deletions(-) diff --git a/pipeline/frontend/metadata/drone_compatibility_test.go b/pipeline/frontend/metadata/drone_compatibility_test.go index c2334585c2..292d45e679 100644 --- a/pipeline/frontend/metadata/drone_compatibility_test.go +++ b/pipeline/frontend/metadata/drone_compatibility_test.go @@ -113,11 +113,11 @@ DRONE_TARGET_BRANCH=main` func convertListToEnvMap(t *testing.T, list string) map[string]string { result := make(map[string]string) for _, s := range strings.Split(list, "\n") { - before, after, _ := strings.Cut(strings.TrimSpace(s), "=") - if before == "" || after == "" { + ss := strings.SplitN(strings.TrimSpace(s), "=", 2) + if len(ss) != 2 { t.Fatal("helper function got invalid test data") } - result[before] = after + result[ss[0]] = ss[1] } return result } diff --git a/pipeline/frontend/yaml/types/base/base_types_test.go b/pipeline/frontend/yaml/types/base/base_types_test.go index 676be9fafc..e21d68cdae 100644 --- a/pipeline/frontend/yaml/types/base/base_types_test.go +++ b/pipeline/frontend/yaml/types/base/base_types_test.go @@ -18,8 +18,9 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" + + "github.com/stretchr/testify/assert" ) type StructStringorInt struct { @@ -103,7 +104,7 @@ bars: [] func TestUnmarshalSliceOrMap(t *testing.T) { s := StructSliceorMap{} err := yaml.Unmarshal([]byte(sampleStructSliceorMap), &s) - assert.Equal(t, fmt.Errorf("cannot unmarshal 'true' of type bool into a string value"), err) + assert.Equal(t, fmt.Errorf("Cannot unmarshal 'true' of type bool into a string value"), err) } func TestStr2SliceOrMapPtrMap(t *testing.T) { diff --git a/pipeline/frontend/yaml/types/network_test.go b/pipeline/frontend/yaml/types/network_test.go index 2eb95ccda3..bdb09bf9e9 100644 --- a/pipeline/frontend/yaml/types/network_test.go +++ b/pipeline/frontend/yaml/types/network_test.go @@ -17,8 +17,9 @@ package types import ( "testing" - "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" + + "github.com/stretchr/testify/assert" ) func TestMarshalNetworks(t *testing.T) { diff --git a/pipeline/frontend/yaml/types/volume_test.go b/pipeline/frontend/yaml/types/volume_test.go index a6e27a0fe3..07270b1a42 100644 --- a/pipeline/frontend/yaml/types/volume_test.go +++ b/pipeline/frontend/yaml/types/volume_test.go @@ -17,8 +17,9 @@ package types import ( "testing" - "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" + + "github.com/stretchr/testify/assert" ) func TestMarshalVolumes(t *testing.T) { diff --git a/server/pipeline/pipelineStatus_test.go b/server/pipeline/pipelineStatus_test.go index ff0c6b8689..a46a831c8e 100644 --- a/server/pipeline/pipelineStatus_test.go +++ b/server/pipeline/pipelineStatus_test.go @@ -48,12 +48,11 @@ func TestUpdateToStatusPending(t *testing.T) { pipeline, _ := UpdateToStatusPending(&mockUpdatePipelineStore{}, model.Pipeline{}, "Reviewer") - switch { - case model.StatusPending != pipeline.Status: + if model.StatusPending != pipeline.Status { t.Errorf("Pipeline status not equals '%s' != '%s'", model.StatusPending, pipeline.Status) - case pipeline.Reviewer != "Reviewer": + } else if pipeline.Reviewer != "Reviewer" { t.Errorf("Reviewer not equals 'Reviewer' != '%s'", pipeline.Reviewer) - case now > pipeline.Reviewed: + } else if now > pipeline.Reviewed { t.Errorf("Reviewed not updated %d !< %d", now, pipeline.Reviewed) } } @@ -65,12 +64,11 @@ func TestUpdateToStatusDeclined(t *testing.T) { pipeline, _ := UpdateToStatusDeclined(&mockUpdatePipelineStore{}, model.Pipeline{}, "Reviewer") - switch { - case model.StatusDeclined != pipeline.Status: + if model.StatusDeclined != pipeline.Status { t.Errorf("Pipeline status not equals '%s' != '%s'", model.StatusDeclined, pipeline.Status) - case pipeline.Reviewer != "Reviewer": + } else if pipeline.Reviewer != "Reviewer" { t.Errorf("Reviewer not equals 'Reviewer' != '%s'", pipeline.Reviewer) - case now > pipeline.Reviewed: + } else if now > pipeline.Reviewed { t.Errorf("Reviewed not updated %d !< %d", now, pipeline.Reviewed) } } @@ -94,16 +92,15 @@ func TestUpdateToStatusError(t *testing.T) { pipeline, _ := UpdateToStatusError(&mockUpdatePipelineStore{}, model.Pipeline{}, errors.New("this is an error")) - switch { - case len(pipeline.Errors) != 1: + if len(pipeline.Errors) != 1 { t.Errorf("Expected one error, got %d", len(pipeline.Errors)) - case pipeline.Errors[0].Error() != "[generic] this is an error": + } else if 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()) - case model.StatusError != pipeline.Status: + } else if model.StatusError != pipeline.Status { t.Errorf("Pipeline status not equals '%s' != '%s'", model.StatusError, pipeline.Status) - case now > pipeline.Started: + } else if now > pipeline.Started { t.Errorf("Started not updated %d !< %d", now, pipeline.Started) - case pipeline.Started != pipeline.Finished: + } else if 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 b54b3a4d37..de05f218e2 100644 --- a/server/pipeline/step_status_test.go +++ b/server/pipeline/step_status_test.go @@ -199,14 +199,13 @@ func TestUpdateStepStatusToDoneSkipped(t *testing.T) { step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state) - switch { - case step.State != model.StatusSkipped: + if step.State != model.StatusSkipped { t.Errorf("Step status not equals '%s' != '%s'", model.StatusSkipped, step.State) - case step.Stopped != int64(34): + } else if step.Stopped != int64(34) { t.Errorf("Step stopped not equals 34 != %d", step.Stopped) - case step.Error != "": + } else if step.Error != "" { t.Errorf("Step error not equals '' != '%s'", step.Error) - case step.ExitCode != 0: + } else if step.ExitCode != 0 { t.Errorf("Step exit code not equals 0 != %d", step.ExitCode) } } @@ -221,14 +220,13 @@ func TestUpdateStepStatusToDoneSuccess(t *testing.T) { step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state) - switch { - case step.State != model.StatusSuccess: + if step.State != model.StatusSuccess { t.Errorf("Step status not equals '%s' != '%s'", model.StatusSuccess, step.State) - case step.Stopped != int64(34): + } else if step.Stopped != int64(34) { t.Errorf("Step stopped not equals 34 != %d", step.Stopped) - case step.Error != "": + } else if step.Error != "" { t.Errorf("Step error not equals '' != '%s'", step.Error) - case step.ExitCode != 0: + } else if step.ExitCode != 0 { t.Errorf("Step exit code not equals 0 != %d", step.ExitCode) } } @@ -264,15 +262,14 @@ func TestUpdateStepToStatusKilledStarted(t *testing.T) { step, _ := UpdateStepToStatusKilled(&mockUpdateStepStore{}, model.Step{}) - switch { - case step.State != model.StatusKilled: + if step.State != model.StatusKilled { t.Errorf("Step status not equals '%s' != '%s'", model.StatusKilled, step.State) - case step.Stopped < now: + } else if step.Stopped < now { t.Errorf("Step stopped not equals %d < %d", now, step.Stopped) - case step.Started != step.Stopped: + } else if step.Started != step.Stopped { t.Errorf("Step started not equals %d != %d", step.Stopped, step.Started) - case step.ExitCode != pipeline.ExitCodeKilled: - t.Errorf("Step exit code not equals %d != %d", pipeline.ExitCodeKilled, step.ExitCode) + } else if step.ExitCode != 137 { + t.Errorf("Step exit code not equals 137 != %d", step.ExitCode) } } diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index 873ffeffe6..d06d4148de 100644 --- a/server/queue/fifo_test.go +++ b/server/queue/fifo_test.go @@ -71,7 +71,7 @@ func TestFifo(t *testing.T) { func TestFifoExpire(t *testing.T) { want := &model.Task{ID: "1"} - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) q.extension = 0 assert.NoError(t, q.Push(noContext, want)) info := q.Info(noContext) @@ -96,7 +96,7 @@ func TestFifoExpire(t *testing.T) { func TestFifoWait(t *testing.T) { want := &model.Task{ID: "1"} - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.Push(noContext, want)) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -149,7 +149,7 @@ func TestFifoDependencies(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -185,7 +185,7 @@ func TestFifoErrors(t *testing.T) { RunOn: []string{"success", "failure"}, } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -234,7 +234,7 @@ func TestFifoErrors2(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) for i := 0; i < 2; i++ { @@ -281,7 +281,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) obtainedWorkCh := make(chan *model.Task) @@ -304,8 +304,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { case got := <-obtainedWorkCh: fmt.Println(got.ID) - switch { - case !task1Processed: + if !task1Processed { if got != task1 { t.Errorf("expect task1 returned from queue as task2 and task3 depends on it") return @@ -319,7 +318,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { obtainedWorkCh <- got } }() - case !task2Processed: + } else if !task2Processed { if got != task2 { t.Errorf("expect task2 returned from queue") return @@ -333,7 +332,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { obtainedWorkCh <- got } }() - default: + } else { if got != task3 { t.Errorf("expect task3 returned from queue") return @@ -372,7 +371,7 @@ func TestFifoTransitiveErrors(t *testing.T) { DepStatus: make(map[string]model.StatusValue), } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -422,7 +421,7 @@ func TestFifoCancel(t *testing.T) { RunOn: []string{"success", "failure"}, } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) _, _ = q.Poll(noContext, 1, func(*model.Task) bool { return true }) @@ -442,7 +441,7 @@ func TestFifoPause(t *testing.T) { ID: "1", } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) var wg sync.WaitGroup wg.Add(1) go func() { @@ -474,7 +473,7 @@ func TestFifoPauseResume(t *testing.T) { ID: "1", } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) q.Pause() assert.NoError(t, q.Push(noContext, task1)) q.Resume() @@ -500,7 +499,7 @@ func TestWaitingVsPending(t *testing.T) { RunOn: []string{"success", "failure"}, } - q, _ := New(context.Background()).(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*model.Task{task2, task3, task1})) got, _ := q.Poll(noContext, 1, func(*model.Task) bool { return true }) diff --git a/server/store/datastore/engine_test.go b/server/store/datastore/engine_test.go index b8e16fadb6..a9f4222243 100644 --- a/server/store/datastore/engine_test.go +++ b/server/store/datastore/engine_test.go @@ -19,9 +19,10 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" "xorm.io/xorm" "xorm.io/xorm/schemas" + + "github.com/stretchr/testify/assert" ) func testDriverConfig() (driver, config string) { diff --git a/server/store/datastore/helper_test.go b/server/store/datastore/helper_test.go index be8b835e6e..7beff99f33 100644 --- a/server/store/datastore/helper_test.go +++ b/server/store/datastore/helper_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "go.woodpecker-ci.org/woodpecker/v2/server/store/types" ) diff --git a/server/store/datastore/migration/migration_test.go b/server/store/datastore/migration/migration_test.go index 858b5f7cdc..d8985d5906 100644 --- a/server/store/datastore/migration/migration_test.go +++ b/server/store/datastore/migration/migration_test.go @@ -19,13 +19,14 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "xorm.io/xorm" + "xorm.io/xorm/schemas" + // blank imports to register the sql drivers _ "github.com/go-sql-driver/mysql" _ "github.com/lib/pq" _ "github.com/mattn/go-sqlite3" - "github.com/stretchr/testify/assert" - "xorm.io/xorm" - "xorm.io/xorm/schemas" ) const ( From 40aa1c4b0391a9cbbe50e1f5efa001ce210bf975 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 9 Jan 2024 21:48:55 +0100 Subject: [PATCH 22/24] fix --- server/plugins/environments/parse.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/plugins/environments/parse.go b/server/plugins/environments/parse.go index 2db49e878b..b06294c2e6 100644 --- a/server/plugins/environments/parse.go +++ b/server/plugins/environments/parse.go @@ -31,13 +31,13 @@ func Parse(params []string) model.EnvironService { var globals []*model.Environ for _, item := range params { - before, after, _ := strings.Cut(item, ":") - if after != "" { + kvPair := strings.SplitN(item, ":", 2) + if len(kvPair) != 2 { // ignore items only containing a key and no value - log.Warn().Msgf("key '%s' has no value, will be ignored", before) + log.Warn().Msgf("key '%s' has no value, will be ignored", kvPair[0]) continue } - globals = append(globals, &model.Environ{Name: before, Value: after}) + globals = append(globals, &model.Environ{Name: kvPair[0], Value: kvPair[1]}) } return &builtin{globals} } From ee064243a98ed9a4d8e7e3e127dc30186eaaa5ca Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 9 Jan 2024 21:51:28 +0100 Subject: [PATCH 23/24] fix --- pipeline/frontend/yaml/types/base/base_types_test.go | 2 +- pipeline/frontend/yaml/types/base/map.go | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pipeline/frontend/yaml/types/base/base_types_test.go b/pipeline/frontend/yaml/types/base/base_types_test.go index e21d68cdae..428b44f06b 100644 --- a/pipeline/frontend/yaml/types/base/base_types_test.go +++ b/pipeline/frontend/yaml/types/base/base_types_test.go @@ -104,7 +104,7 @@ bars: [] func TestUnmarshalSliceOrMap(t *testing.T) { s := StructSliceorMap{} err := yaml.Unmarshal([]byte(sampleStructSliceorMap), &s) - assert.Equal(t, fmt.Errorf("Cannot unmarshal 'true' of type bool into a string value"), err) + assert.Equal(t, fmt.Errorf("cannot unmarshal 'true' of type bool into a string value"), err) } func TestStr2SliceOrMapPtrMap(t *testing.T) { diff --git a/pipeline/frontend/yaml/types/base/map.go b/pipeline/frontend/yaml/types/base/map.go index 8d91be9914..e25a65b04e 100644 --- a/pipeline/frontend/yaml/types/base/map.go +++ b/pipeline/frontend/yaml/types/base/map.go @@ -30,8 +30,15 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { parts := map[string]string{} for _, s := range sliceType { if str, ok := s.(string); ok { - before, after, _ := strings.Cut(strings.TrimSpace(str), "=") - parts[before] = after + str := strings.TrimSpace(str) + keyValueSlice := strings.SplitN(str, "=", 2) + + key := keyValueSlice[0] + val := "" + if len(keyValueSlice) == 2 { + val = keyValueSlice[1] + } + parts[key] = val } else { return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", s, s) } From bc7ef10566b12bc535d2a8eb0d3b349e16cb8137 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 9 Jan 2024 21:59:38 +0100 Subject: [PATCH 24/24] cleanup --- cmd/agent/main.go | 2 - pipeline/backend/docker/convert.go | 1 + pipeline/backend/kubernetes/kubernetes.go | 47 +++++++++------------- pipeline/pipeline.go | 2 +- server/badges/badges_test.go | 1 - server/cache/membership.go | 1 - server/forge/gitlab/convert.go | 2 +- server/forge/types/errors.go | 2 +- server/pipeline/cancel.go | 3 +- server/pipeline/create.go | 13 +++--- server/pipeline/decline.go | 4 +- server/pipeline/errors.go | 8 ++-- server/pipeline/stepbuilder/stepBuilder.go | 2 - server/pipeline/topic.go | 6 +-- 14 files changed, 39 insertions(+), 55 deletions(-) diff --git a/cmd/agent/main.go b/cmd/agent/main.go index d3f669b108..a96167b8b1 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -30,8 +30,6 @@ import ( ) func main() { - initHealth() - app := cli.NewApp() app.Name = "woodpecker-agent" app.Version = version.String() diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index 5b55a84abd..b67e1693b7 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -79,6 +79,7 @@ func toHostConfig(step *types.Step) *container.HostConfig { }, Privileged: step.Privileged, ShmSize: step.ShmSize, + Sysctls: step.Sysctls, } if len(step.NetworkMode) != 0 { diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 89bc1b0d08..81aaf24e1c 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -23,20 +23,21 @@ import ( "time" "github.com/rs/zerolog/log" - "github.com/urfave/cli/v2" "gopkg.in/yaml.v3" + + "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" + + "github.com/urfave/cli/v2" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" - - // To authenticate to GCP K8s clusters - _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" + // To authenticate to GCP K8s clusters + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" ) const ( @@ -131,8 +132,8 @@ func (e *kube) IsAvailable(context.Context) bool { return len(host) > 0 } -func (e *kube) Load(ctx context.Context) (*types.BackendInfo, error) { - config, err := configFromCliContext(ctx) +func (e *kube) Load(context.Context) (*types.BackendInfo, error) { + config, err := configFromCliContext(e.ctx) if err != nil { return nil, err } @@ -218,12 +219,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) finished := make(chan bool) podUpdated := func(old, new any) { - pod, ok := new.(*v1.Pod) - if !ok { - log.Error().Msgf("Unexpected object while awaiting Pod: %+v", pod) - return - } - + pod := new.(*v1.Pod) if pod.Name == podName { if isImagePullBackOffState(pod) { finished <- true @@ -237,7 +233,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) //nolint: gomnd + si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, @@ -259,7 +255,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } if isImagePullBackOffState(pod) { - return nil, fmt.Errorf("could not pull image for pod %s", pod.Name) + return nil, fmt.Errorf("Could not pull image for pod %s", pod.Name) } bs := &types.State{ @@ -283,12 +279,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) up := make(chan bool) podUpdated := func(old, new any) { - pod, ok := new.(*v1.Pod) - if !ok { - log.Error().Msgf("Unexpected object while awaiting Pod: %+v", pod) - return - } - + pod := new.(*v1.Pod) if pod.Name == podName { switch pod.Status.Phase { case v1.PodRunning, v1.PodSucceeded, v1.PodFailed: @@ -298,7 +289,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) } // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) //nolint: gomnd + si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, @@ -343,31 +334,31 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) return rc, nil } -func (e *kube) DestroyStep(ctx context.Context, step *types.Step, taskUUID string) error { +func (e *kube) DestroyStep(_ context.Context, step *types.Step, taskUUID string) error { if step.Type == types.StepTypeService { // a service should be stopped by DestroyWorkflow so we can ignore it log.Trace().Msgf("DestroyStep got service '%s', ignoring it.", step.Name) return nil } log.Trace().Str("taskUUID", taskUUID).Msgf("Stopping step: %s", step.Name) - err := stopPod(ctx, e, step, defaultDeleteOptions) + err := stopPod(e.ctx, e, step, defaultDeleteOptions) return err } // Destroy the pipeline environment. -func (e *kube) DestroyWorkflow(ctx context.Context, conf *types.Config, taskUUID string) error { +func (e *kube) DestroyWorkflow(_ context.Context, conf *types.Config, taskUUID string) error { log.Trace().Str("taskUUID", taskUUID).Msg("Deleting Kubernetes primitives") // Use noContext because the ctx sent to this function will be canceled/done in case of error or canceled by user. for _, stage := range conf.Stages { for _, step := range stage.Steps { - err := stopPod(ctx, e, step, defaultDeleteOptions) + err := stopPod(e.ctx, e, step, defaultDeleteOptions) if err != nil { return err } if step.Type == types.StepTypeService { - err := stopService(ctx, e, step, defaultDeleteOptions) + err := stopService(e.ctx, e, step, defaultDeleteOptions) if err != nil { return err } @@ -376,7 +367,7 @@ func (e *kube) DestroyWorkflow(ctx context.Context, conf *types.Config, taskUUID } for _, vol := range conf.Volumes { - err := stopVolume(ctx, e, vol.Name, defaultDeleteOptions) + err := stopVolume(e.ctx, e, vol.Name, defaultDeleteOptions) if err != nil { return err } diff --git a/pipeline/pipeline.go b/pipeline/pipeline.go index 09681003d4..48694562e5 100644 --- a/pipeline/pipeline.go +++ b/pipeline/pipeline.go @@ -112,7 +112,7 @@ func (r *Runtime) Run(runnerCtx context.Context) error { }() r.started = time.Now().Unix() - if err := r.engine.SetupWorkflow(runnerCtx, r.spec, r.taskUUID); err != nil { + if err := r.engine.SetupWorkflow(r.ctx, r.spec, r.taskUUID); err != nil { return err } diff --git a/server/badges/badges_test.go b/server/badges/badges_test.go index fdbefe1957..d8953c37de 100644 --- a/server/badges/badges_test.go +++ b/server/badges/badges_test.go @@ -18,7 +18,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/cache/membership.go b/server/cache/membership.go index 0dd1eb3fa4..3c73c12e03 100644 --- a/server/cache/membership.go +++ b/server/cache/membership.go @@ -39,7 +39,6 @@ type membershipCache struct { // NewMembershipService creates a new membership service. func NewMembershipService(f forge.Forge) MembershipService { - //nolint: gomnd return &membershipCache{ ttl: 10 * time.Minute, forge: f, diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index 502e537822..d376e26229 100644 --- a/server/forge/gitlab/convert.go +++ b/server/forge/gitlab/convert.go @@ -255,7 +255,7 @@ func getUserAvatar(email string) string { func extractFromPath(str string) (string, string, error) { s := strings.Split(str, "/") - if len(s) < 2 { //nolint: gomnd + if len(s) < 2 { return "", "", fmt.Errorf("minimum match not found") } return s[0], s[1], nil diff --git a/server/forge/types/errors.go b/server/forge/types/errors.go index fbb1fda938..957e30432c 100644 --- a/server/forge/types/errors.go +++ b/server/forge/types/errors.go @@ -54,7 +54,7 @@ func (err *ErrIgnoreEvent) Error() string { } func (*ErrIgnoreEvent) Is(target error) bool { - _, ok := target.(*ErrIgnoreEvent) + _, ok := target.(*ErrIgnoreEvent) //nolint:errorlint return ok } diff --git a/server/pipeline/cancel.go b/server/pipeline/cancel.go index be7e1c732d..71df7dd068 100644 --- a/server/pipeline/cancel.go +++ b/server/pipeline/cancel.go @@ -93,8 +93,9 @@ func Cancel(ctx context.Context, store store.Store, repo *model.Repo, user *mode if killedPipeline.Workflows, err = store.WorkflowGetTree(killedPipeline); err != nil { return err } + publishToTopic(killedPipeline, repo) - return publishToTopic(killedPipeline, repo) + return nil } func cancelPreviousPipelines( diff --git a/server/pipeline/create.go b/server/pipeline/create.go index 2c31174291..5182a4cbf4 100644 --- a/server/pipeline/create.go +++ b/server/pipeline/create.go @@ -116,10 +116,7 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline } if pipeline.Status == model.StatusBlocked { - if err := publishPipeline(ctx, pipeline, repo, repoUser); err != nil { - return nil, err - } - + publishPipeline(ctx, pipeline, repo, repoUser) return pipeline, nil } @@ -145,7 +142,9 @@ func updatePipelineWithErr(ctx context.Context, _store store.Store, pipeline *mo // update value in ref *pipeline = *_pipeline - return publishPipeline(ctx, pipeline, repo, repoUser) + publishPipeline(ctx, pipeline, repo, repoUser) + + return nil } func updatePipelinePending(ctx context.Context, _store store.Store, pipeline *model.Pipeline, repo *model.Repo, repoUser *model.User) error { @@ -156,5 +155,7 @@ func updatePipelinePending(ctx context.Context, _store store.Store, pipeline *mo // update value in ref *pipeline = *_pipeline - return publishPipeline(ctx, pipeline, repo, repoUser) + publishPipeline(ctx, pipeline, repo, repoUser) + + return nil } diff --git a/server/pipeline/decline.go b/server/pipeline/decline.go index 24bc953502..7ddadfa5f6 100644 --- a/server/pipeline/decline.go +++ b/server/pipeline/decline.go @@ -41,9 +41,7 @@ func Decline(ctx context.Context, store store.Store, pipeline *model.Pipeline, u updatePipelineStatus(ctx, pipeline, repo, user) - if err := publishToTopic(pipeline, repo); err != nil { - return nil, err - } + publishToTopic(pipeline, repo) return pipeline, nil } diff --git a/server/pipeline/errors.go b/server/pipeline/errors.go index 4b7887a381..d887904b9a 100644 --- a/server/pipeline/errors.go +++ b/server/pipeline/errors.go @@ -25,9 +25,9 @@ func (e ErrNotFound) Error() string { } func (e ErrNotFound) Is(target error) bool { - _, ok := target.(ErrNotFound) + _, ok := target.(ErrNotFound) //nolint:errorlint if !ok { - _, ok = target.(*ErrNotFound) + _, ok = target.(*ErrNotFound) //nolint:errorlint } return ok } @@ -41,9 +41,9 @@ func (e ErrBadRequest) Error() string { } func (e ErrBadRequest) Is(target error) bool { - _, ok := target.(ErrBadRequest) + _, ok := target.(ErrBadRequest) //nolint:errorlint if !ok { - _, ok = target.(*ErrBadRequest) + _, ok = target.(*ErrBadRequest) //nolint:errorlint } return ok } diff --git a/server/pipeline/stepbuilder/stepBuilder.go b/server/pipeline/stepbuilder/stepBuilder.go index 93f1b6332a..78ceb65c03 100644 --- a/server/pipeline/stepbuilder/stepBuilder.go +++ b/server/pipeline/stepbuilder/stepBuilder.go @@ -35,8 +35,6 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/compiler" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/linter" "go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/matrix" - - // TODO: Import of the server package should not be allowed and must be refactored "go.woodpecker-ci.org/woodpecker/v2/server" "go.woodpecker-ci.org/woodpecker/v2/server/model" ) diff --git a/server/pipeline/topic.go b/server/pipeline/topic.go index d8df0060b7..f0c1d72f46 100644 --- a/server/pipeline/topic.go +++ b/server/pipeline/topic.go @@ -25,7 +25,7 @@ import ( ) // publishToTopic publishes message to UI clients -func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) error { +func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) { message := pubsub.Message{ Labels: map[string]string{ "repo": repo.FullName, @@ -41,9 +41,7 @@ func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) error { }) if err != nil { log.Error().Err(err).Msg("can't marshal JSON") - return err + return } server.Config.Services.Pubsub.Publish(message) - - return nil }