Enable some linters (#3129)

Mostly those that did not require much work.

From #2960

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

.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

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
 }

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

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) {

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))
 	}

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 {

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 <https://pkg.go.dev/time#ParseDuration> for syntax reference.
 
 ### `WOODPECKER_ENABLE_SWAGGER`
 

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{})
 }

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))

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)
 	}
 

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)
 		}

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

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")
 		})

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

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
+	}
 }

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))
 		})
 	}