diff --git a/.golangci.yml b/.golangci.yml index b0032961d09..1b426fe75f6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -102,6 +102,24 @@ linters-settings: - pkg: 'go.woodpecker-ci.org/woodpecker/v2/cmd/agent' - pkg: 'go.woodpecker-ci.org/woodpecker/v2/cmd/cli' - pkg: 'go.woodpecker-ci.org/woodpecker/v2/woodpecker-go/woodpecker' + gci: + sections: + - 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 @@ -124,6 +142,25 @@ linters: - forbidigo - zerologlint - depguard + - asciicheck + - bodyclose + - dogsled + - durationcheck + - errchkjson + - gochecknoinits + - goheader + - gomoddirectives + - gomodguard + - goprintffuncname + - importas + - makezero + - rowserrcheck + - sqlclosecheck + - tenv + - unconvert + - unparam + - wastedassign + - whitespace run: timeout: 15m @@ -140,7 +177,12 @@ issues: - path: 'cmd/*|cli/*' linters: - forbidigo + # allow some setup functions to use log.Fatal() - path: 'server/web/web.go|server/plugins/encryption/tink_keyset_watcher.go|shared/logger/logger.go' linters: - forbidigo + + - path: '_test.go' + linters: + - forcetypeassert diff --git a/agent/state.go b/agent/state.go index 8d02d928a0b..72f8b99a161 100644 --- a/agent/state.go +++ b/agent/state.go @@ -74,8 +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) s.Unlock() + if err != nil { + return 0, err + } ret, err := w.Write(out) return int64(ret), err } diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index 44f1bcff877..fc9c6f0c281 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -271,6 +271,8 @@ func getBackendEngine(backendCtx context.Context, backendName string, addons []s } func runWithRetry(context *cli.Context) error { + initHealth() + retryCount := context.Int("connect-retry-count") retryDelay := context.Duration("connect-retry-delay") var err error diff --git a/cmd/agent/health.go b/cmd/agent/health.go index 0a508f07c51..8c11589b61f 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) @@ -48,10 +48,13 @@ func handleHeartbeat(w http.ResponseWriter, _ *http.Request) { func handleVersion(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(200) w.Header().Add("Content-Type", "text/json") - _ = json.NewEncoder(w).Encode(versionResp{ + err := json.NewEncoder(w).Encode(versionResp{ Source: "https://github.com/woodpecker-ci/woodpecker", Version: version.String(), }) + if err != nil { + log.Error().Err(err).Msg("handleVersion") + } } func handleStats(w http.ResponseWriter, _ *http.Request) { diff --git a/cmd/server/server.go b/cmd/server/server.go index cbd318705e9..20478c892cc 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -87,10 +87,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("can't setup database store") - } + _store := setupStore(c) defer func() { if err := _store.Close(); err != nil { log.Error().Err(err).Msg("could not close store") @@ -317,7 +314,7 @@ func setupEvilGlobals(c *cli.Context, v store.Store, f forge.Forge) error { // 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 ae9c57e3f50..5a2c0ebf93d 100644 --- a/cmd/server/setup.go +++ b/cmd/server/setup.go @@ -54,7 +54,7 @@ import ( addonTypes "go.woodpecker-ci.org/woodpecker/v2/shared/addon/types" ) -func setupStore(c *cli.Context) (store.Store, error) { +func setupStore(c *cli.Context) store.Store { datasource := c.String("datasource") driver := c.String("driver") xorm := store.XORM{ @@ -95,7 +95,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/docs/docs/30-administration/10-server-config.md b/docs/docs/30-administration/10-server-config.md index 405d10c56d0..af96f5bccd1 100644 --- a/docs/docs/30-administration/10-server-config.md +++ b/docs/docs/30-administration/10-server-config.md @@ -586,9 +586,9 @@ Specify a configuration service endpoint, see [Configuration Extension](./100-ex ### `WOODPECKER_FORGE_TIMEOUT` -> Default: 3sec +> Default: 3s -Specify how many seconds before timeout when fetching the Woodpecker configuration from a Forge +Specify timeout when fetching the Woodpecker configuration from forge. See for syntax reference. ### `WOODPECKER_ENABLE_SWAGGER` diff --git a/pipeline/backend/kubernetes/service.go b/pipeline/backend/kubernetes/service.go index 2fa66a462b6..6930f743eac 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 5dc06509ff0..f4b479f60fa 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)) diff --git a/pipeline/frontend/metadata/environment.go b/pipeline/frontend/metadata/environment.go index ff84c172d21..b2629a9f7d2 100644 --- a/pipeline/frontend/metadata/environment.go +++ b/pipeline/frontend/metadata/environment.go @@ -21,6 +21,8 @@ import ( "regexp" "strconv" "strings" + + "github.com/rs/zerolog/log" ) var ( @@ -136,7 +138,10 @@ func (m *Metadata) Environ() map[string]string { params["CI_PIPELINE_FILES"] = "[]" } else if len(m.Curr.Commit.ChangedFiles) <= maxChangedFiles { // we have to use json, as other separators like ;, or space are valid filename chars - changedFiles, _ := json.Marshal(m.Curr.Commit.ChangedFiles) + changedFiles, err := json.Marshal(m.Curr.Commit.ChangedFiles) + if err != nil { + log.Error().Err(err).Msg("marshal changed files") + } params["CI_PIPELINE_FILES"] = string(changedFiles) } diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go index 5331469384f..1f041204abe 100644 --- a/server/forge/configFetcher.go +++ b/server/forge/configFetcher.go @@ -61,7 +61,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er // try to fetch 3 times for i := 0; i < 3; i++ { - files, err = cf.fetch(ctx, time.Second*cf.timeout, strings.TrimSpace(cf.configPath)) + files, err = cf.fetch(ctx, cf.timeout, strings.TrimSpace(cf.configPath)) if err != nil { log.Trace().Err(err).Msgf("%d. try failed", i+1) } diff --git a/server/forge/gitea/helper.go b/server/forge/gitea/helper.go index 6972224a7db..1f68bdc62dc 100644 --- a/server/forge/gitea/helper.go +++ b/server/forge/gitea/helper.go @@ -77,7 +77,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 diff --git a/server/forge/github/convert_test.go b/server/forge/github/convert_test.go index d98d8fedfd0..e95704ef07e 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/parse.go b/server/forge/github/parse.go index 293cab3834a..6df51b242e8 100644 --- a/server/forge/github/parse.go +++ b/server/forge/github/parse.go @@ -61,11 +61,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: @@ -75,9 +75,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{ @@ -110,12 +110,12 @@ func parsePushHook(hook *github.PushEvent) (*model.Repo, *model.Pipeline, error) } } - 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(), @@ -138,7 +138,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/grpc/filter.go b/server/grpc/filter.go index 30b34605cf3..9cf2d87fae7 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 d3b06e91f0b..e4c7eeae645 100644 --- a/server/grpc/filter_test.go +++ b/server/grpc/filter_test.go @@ -84,11 +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 6f505425b22..ea1e9ae1514 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 { @@ -151,10 +148,13 @@ func (s *RPC) Update(_ context.Context, id string, state rpc.State) error { "private": strconv.FormatBool(repo.IsSCMPrivate), }, } - message.Data, _ = json.Marshal(model.Event{ + message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: *currentPipeline, }) + if err != nil { + return err + } s.pubsub.Publish(message) return nil @@ -207,10 +207,14 @@ func (s *RPC) Init(c context.Context, id string, state rpc.State) error { "private": strconv.FormatBool(repo.IsSCMPrivate), }, } - message.Data, _ = json.Marshal(model.Event{ + message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: *currentPipeline, }) + if err != nil { + log.Error().Err(err).Msgf("could not marshal JSON") + return + } s.pubsub.Publish(message) }() @@ -427,10 +431,13 @@ func (s *RPC) notify(repo *model.Repo, pipeline *model.Pipeline) (err error) { "private": strconv.FormatBool(repo.IsSCMPrivate), }, } - message.Data, _ = json.Marshal(model.Event{ + message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: *pipeline, }) + if err != nil { + return err + } s.pubsub.Publish(message) return nil } diff --git a/server/grpc/server.go b/server/grpc/server.go index 6f5f88703d5..83a9833510c 100644 --- a/server/grpc/server.go +++ b/server/grpc/server.go @@ -74,17 +74,14 @@ func (s *WoodpeckerServer) Next(c context.Context, req *proto.NextRequest) (*pro res := new(proto.NextResponse) pipeline, err := s.peer.Next(c, filter) - if err != nil { - return res, err - } - if pipeline == nil { + if err != nil || pipeline == nil { return res, err } 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/pipeline/queue.go b/server/pipeline/queue.go index d40d0437ad5..9b845ce91cb 100644 --- a/server/pipeline/queue.go +++ b/server/pipeline/queue.go @@ -42,11 +42,15 @@ func queuePipeline(repo *model.Repo, pipelineItems []*stepbuilder.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) } diff --git a/server/pipeline/topic.go b/server/pipeline/topic.go index 61dd58295f0..f0c1d72f468 100644 --- a/server/pipeline/topic.go +++ b/server/pipeline/topic.go @@ -18,6 +18,7 @@ import ( "encoding/json" "strconv" + "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/pubsub" @@ -33,9 +34,14 @@ func publishToTopic(pipeline *model.Pipeline, repo *model.Repo) { } pipelineCopy := *pipeline - message.Data, _ = json.Marshal(model.Event{ + var err error + message.Data, err = json.Marshal(model.Event{ Repo: *repo, Pipeline: pipelineCopy, }) + if err != nil { + log.Error().Err(err).Msg("can't marshal JSON") + return + } server.Config.Services.Pubsub.Publish(message) } diff --git a/server/web/config.go b/server/web/config.go index 8c58b3b8817..0efbe88cbbc 100644 --- a/server/web/config.go +++ b/server/web/config.go @@ -52,7 +52,11 @@ 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.Error().Err(err).Msgf("could not marshal JSON") + return "" + } return string(a) }, }