Enable golangci linter gomnd (#3171)

.golangci.yaml

@@ -166,6 +166,13 @@ linters:
     - contextcheck
     - forcetypeassert
     - gci
+    - gomnd
+
+issues:
+  exclude-rules:
+    - path: 'fixtures|cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go'
+      linters:
+        - gomnd
 
 run:
   timeout: 15m

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{

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
 }
 

agent/runner.go

@@ -158,15 +158,15 @@ func (r *Runner) Run(runnerCtx context.Context) error {
 
 	if canceled.IsSet() {
 		state.Error = ""
-		state.ExitCode = 137
+		state.ExitCode = pipeline.ExitCodeKilled
 	} else if err != nil {
 		pExitError := &pipeline.ExitError{}
 		switch {
 		case errors.As(err, &pExitError):
 			state.ExitCode = pExitError.Code
 		case errors.Is(err, pipeline.ErrCancel):
 			state.Error = ""
-			state.ExitCode = 137
+			state.ExitCode = pipeline.ExitCodeKilled
 			canceled.SetTo(true)
 		default:
 			state.ExitCode = 1

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 (i.e. production)")
 	}

cli/exec/exec.go

@@ -123,13 +123,13 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error
 
 	pipelineEnv := make(map[string]string)
 	for _, env := range c.StringSlice("env") {
-		envs := strings.SplitN(env, "=", 2)
-		pipelineEnv[envs[0]] = envs[1]
-		if oldVar, exists := environ[envs[0]]; exists {
+		before, after, _ := strings.Cut(env, "=")
+		pipelineEnv[before] = after
+		if oldVar, exists := environ[before]; exists {
 			// override existing values, but print a warning
-			log.Warn().Msgf("environment variable '%s' had value '%s', but got overwritten", envs[0], oldVar)
+			log.Warn().Msgf("environment variable '%s' had value '%s', but got overwritten", before, oldVar)
 		}
-		environ[envs[0]] = envs[1]
+		environ[before] = after
 	}
 
 	tmpl, err := envsubst.ParseFile(file)
@@ -244,8 +244,16 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error
 	).Run(c.Context)
 }
 
+// convertPathForWindows converts a path to use slash separators
+// for Windows. If the path is a Windows volume name like C:, it
+// converts it to an absolute root path starting with slash (e.g.
+// C: -> /c). Otherwise it just converts backslash separators to
+// slashes.
 func convertPathForWindows(path string) string {
 	base := filepath.VolumeName(path)
+
+	// Check if path is volume name like C:
+	//nolint: gomnd
 	if len(base) == 2 {
 		path = strings.TrimPrefix(path, base)
 		base = strings.ToLower(strings.TrimSuffix(base, ":"))

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 {
+		before, after, ok := strings.Cut(i, "=")
+		if !ok || before == "" {
 			continue
 		}
-		params[parts[0]] = parts[1]
+		params[before] = after
 	}
 	return params
 }

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 {
-			variables[sp[0]] = sp[1]
+		before, after, _ := strings.Cut(vaz, "=")
+		if before != "" && after != "" {
+			variables[before] = after
 		}
 	}
 

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",

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
 	}

cli/setup/token_fetcher.go

@@ -61,7 +61,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine {
 		c.Writer.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS, GET, PUT")
 
 		if c.Request.Method == "OPTIONS" {
-			c.AbortWithStatus(204)
+			c.AbortWithStatus(http.StatusNoContent)
 			return
 		}
 
@@ -76,15 +76,15 @@ func setupRouter(tokenReceived chan string) *gin.Engine {
 		err := c.BindJSON(&data)
 		if err != nil {
 			log.Debug().Err(err).Msg("Failed to bind JSON")
-			c.JSON(400, gin.H{
+			c.JSON(http.StatusBadRequest, gin.H{
 				"error": "invalid request",
 			})
 			return
 		}
 
 		tokenReceived <- data.Token
 
-		c.JSON(200, gin.H{
+		c.JSON(http.StatusOK, gin.H{
 			"ok": "true",
 		})
 	})
@@ -111,7 +111,10 @@ func openBrowser(url string) error {
 }
 
 func randomPort() int {
-	s1 := rand.NewSource(time.Now().UnixNano())
-	r1 := rand.New(s1)
-	return r1.Intn(10000) + 20000
+	const minPort = 10000
+	const maxPort = 65535
+
+	source := rand.NewSource(time.Now().UnixNano())
+	rand := rand.New(source)
+	return rand.Intn(maxPort-minPort+1) + minPort
 }

cmd/agent/core/agent.go

@@ -89,7 +89,7 @@ func run(c *cli.Context, backends []types.Backend) 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
 	}
@@ -276,12 +276,12 @@ func stringSliceAddToMap(sl []string, m map[string]string) error {
 		m = make(map[string]string)
 	}
 	for _, v := range utils.StringSliceDeleteEmpty(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 before != "":
+			return fmt.Errorf("key '%s' does not have a value assigned", before)
 		default:
 			return fmt.Errorf("empty string in slice")
 		}

cmd/agent/core/flags.go

@@ -22,6 +22,7 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
+//nolint:gomnd
 var flags = []cli.Flag{
 	&cli.StringFlag{
 		EnvVars: []string{"WOODPECKER_SERVER"},

cmd/agent/core/health.go

@@ -39,14 +39,14 @@ func initHealth() {
 
 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")
 	err := json.NewEncoder(w).Encode(versionResp{
 		Source:  "https://github.com/woodpecker-ci/woodpecker",
@@ -59,9 +59,9 @@ func handleVersion(w http.ResponseWriter, _ *http.Request) {
 
 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 {
@@ -92,8 +92,8 @@ func pinger(c *cli.Context) error {
 		return err
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode != 200 {
-		return fmt.Errorf("agent returned non-200 status code")
+	if resp.StatusCode != http.StatusOK {
+		return fmt.Errorf("agent returned non-http.StatusOK status code")
 	}
 	return nil
 }

pipeline/backend/docker/convert.go

@@ -27,6 +27,9 @@ import (
 	"go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types"
 )
 
+// Valid container volumes must have at least two components, source and destination.
+const minVolumeComponents = 2
+
 // returns a container configuration.
 func (e *docker) toConfig(step *types.Step) *container.Config {
 	config := &container.Config{
@@ -131,7 +134,7 @@ func toVol(paths []string) map[string]struct{} {
 		if err != nil {
 			continue
 		}
-		if len(parts) < 2 {
+		if len(parts) < minVolumeComponents {
 			continue
 		}
 		set[parts[1]] = struct{}{}
@@ -149,16 +152,18 @@ func toEnv(env map[string]string) []string {
 	return envs
 }
 
-// helper function that converts a slice of device paths to a slice of
-// container.DeviceMapping.
+// toDev converts a slice of volume paths to a set of device mappings for
+// use in a Docker container config. It handles splitting the volume paths
+// into host and container paths, and setting default permissions.
 func toDev(paths []string) []container.DeviceMapping {
 	var devices []container.DeviceMapping
+
 	for _, path := range paths {
 		parts, err := splitVolumeParts(path)
 		if err != nil {
 			continue
 		}
-		if len(parts) < 2 {
+		if len(parts) < minVolumeComponents {
 			continue
 		}
 		if strings.HasSuffix(parts[1], ":ro") || strings.HasSuffix(parts[1], ":rw") {
@@ -183,7 +188,15 @@ func encodeAuthToBase64(authConfig types.Auth) (string, error) {
 	return base64.URLEncoding.EncodeToString(buf), nil
 }
 
-// helper function that split volume path
+// splitVolumeParts splits a volume string into its constituent parts.
+//
+// The parts are:
+//
+//  1. The path on the host machine
+//  2. The path inside the container
+//  3. The read/write mode
+//
+// It handles Windows and Linux style volume paths.
 func splitVolumeParts(volumeParts string) ([]string, error) {
 	pattern := `^((?:[\w]\:)?[^\:]*)\:((?:[\w]\:)?[^\:]*)(?:\:([rwom]*))?`
 	r, err := regexp.Compile(pattern)

pipeline/backend/kubernetes/kubernetes.go

@@ -42,6 +42,8 @@ import (
 
 const (
 	EngineName = "kubernetes"
+	// TODO 5 seconds is against best practice, k3s didn't work otherwise
+	defaultResyncDuration = 5 * time.Second
 )
 
 var defaultDeleteOptions = newDefaultDeleteOptions()
@@ -249,8 +251,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, defaultResyncDuration, informers.WithNamespace(e.config.Namespace))
 	if _, err := si.Core().V1().Pods().Informer().AddEventHandler(
 		cache.ResourceEventHandlerFuncs{
 			UpdateFunc: podUpdated,
@@ -322,8 +323,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, defaultResyncDuration, informers.WithNamespace(e.config.Namespace))
 	if _, err := si.Core().V1().Pods().Informer().AddEventHandler(
 		cache.ResourceEventHandlerFuncs{
 			UpdateFunc: podUpdated,

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

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
 }

pipeline/frontend/metadata/environment.go

@@ -38,7 +38,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]
 	}