refactor: add more linters and fix findings (#41)

.drone.yml

@@ -8,7 +8,7 @@ platform:
 
 steps:
   - name: deps
-    image: golang:1.19
+    image: golang:1.20
     commands:
       - make deps
     volumes:
@@ -16,7 +16,7 @@ steps:
         path: /go
 
   - name: lint
-    image: golang:1.19
+    image: golang:1.20
     commands:
       - make lint
     volumes:
@@ -24,7 +24,7 @@ steps:
         path: /go
 
   - name: test
-    image: golang:1.19
+    image: golang:1.20
     commands:
       - make test
     volumes:
@@ -51,7 +51,7 @@ platform:
 
 steps:
   - name: build
-    image: techknowlogick/xgo:go-1.19.x
+    image: techknowlogick/xgo:go-1.20.x
     commands:
       - ln -s /drone/src /source
       - make release
@@ -292,6 +292,6 @@ depends_on:
 
 ---
 kind: signature
-hmac: ccad191c33e8df2f7c7c51f10e2e2999e5e6f46a8a50de9aa1e8a5ae6c116600
+hmac: ed52cf54615cff7806ee5d08c981a280bdd31d42bd1af00096686a60727a5526
 
 ...

.golangci.yml

@@ -1,25 +1,92 @@
 linters:
-  enable:
-    - gosimple
-    - deadcode
-    - typecheck
-    - govet
-    - errcheck
-    - staticcheck
-    - unused
-    - structcheck
-    - varcheck
-    - dupl
-    - gofmt
-    - misspell
-    - gocritic
-    - bidichk
-    - ineffassign
-    - revive
-    - gofumpt
-    - depguard
   enable-all: false
   disable-all: true
+  enable:
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - typecheck
+    - unused
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - containedctx
+    - contextcheck
+    - decorder
+    - depguard
+    - dogsled
+    - dupl
+    - dupword
+    - durationcheck
+    - errchkjson
+    - errname
+    - errorlint
+    - execinquery
+    - exhaustive
+    - exportloopref
+    - forcetypeassert
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecknoglobals
+    - gochecknoinits
+    - gocognit
+    - goconst
+    - gocritic
+    - gocyclo
+    - godot
+    - godox
+    - goerr113
+    - gofmt
+    - gofumpt
+    - goheader
+    - goimports
+    - gomnd
+    - gomoddirectives
+    - gomodguard
+    - goprintffuncname
+    - gosec
+    - grouper
+    - importas
+    - interfacebloat
+    - ireturn
+    - lll
+    - loggercheck
+    - maintidx
+    - makezero
+    - misspell
+    - musttag
+    - nakedret
+    - nestif
+    - nilerr
+    - nilnil
+    - nlreturn
+    - noctx
+    - nolintlint
+    - nonamedreturns
+    - nosprintfhostport
+    - prealloc
+    - predeclared
+    - promlinter
+    - reassign
+    - revive
+    # - rowserrcheck
+    # - sqlclosecheck
+    # - structcheck
+    - stylecheck
+    - tagliatelle
+    - tenv
+    - testableexamples
+    - thelper
+    - tparallel
+    - unconvert
+    - unparam
+    - usestdlibvars
+    # - wastedassign
+    - whitespace
+    - wsl
   fast: false
 
 run:
@@ -28,4 +95,11 @@ run:
 linters-settings:
   gofumpt:
     extra-rules: true
-    lang-version: "1.18"
+    lang-version: "1.20"
+  ireturn:
+    allow:
+      - anon
+      - error
+      - empty
+      - stdlib
+      - drone.Client

Makefile

@@ -1,7 +1,7 @@
 # renovate: datasource=github-releases depName=mvdan/gofumpt
 GOFUMPT_PACKAGE_VERSION := v0.4.0
 # renovate: datasource=github-releases depName=golangci/golangci-lint
-GOLANGCI_LINT_PACKAGE_VERSION := v1.50.1
+GOLANGCI_LINT_PACKAGE_VERSION := v1.51.1
 
 EXECUTABLE := drone-admin
 
@@ -19,7 +19,7 @@ GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@$(G
 XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest
 
 GENERATE ?=
-XGO_VERSION := go-1.19.x
+XGO_VERSION := go-1.20.x
 XGO_TARGETS ?= linux/amd64,linux/arm64,linux/arm-6,linux/arm-7
 
 TARGETOS ?= linux

admin/autoscaler/autoscaler.go

@@ -2,10 +2,12 @@ package autoscaler
 
 import "github.com/urfave/cli/v2"
 
-var Command = &cli.Command{
-	Name:  "autoscaler",
-	Usage: "manage autoscaler",
-	Subcommands: []*cli.Command{
-		&autoscalerReaperCmd,
-	},
+func GetAutoscalerCmd() *cli.Command {
+	return &cli.Command{
+		Name:  "autoscaler",
+		Usage: "manage autoscaler",
+		Subcommands: []*cli.Command{
+			getReaperCmd(),
+		},
+	}
 }

admin/autoscaler/autoscaler_reaper.go

@@ -11,24 +11,28 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-var autoscalerReaperCmd = cli.Command{
-	Name:   "reaper",
-	Usage:  "find and kill agents in error state",
-	Action: autoscalerReaper,
-	Flags: []cli.Flag{
-		&cli.StringFlag{
-			Name:    "state-file",
-			Usage:   "state file",
-			EnvVars: []string{"DRONE_ADMIN_AUTOSCALER_REAPER_STATE_FILE"},
-			Value:   "/tmp/droneclean.gob",
+func getReaperCmd() *cli.Command {
+	return &cli.Command{
+		Name:   "reaper",
+		Usage:  "find and kill agents in error state",
+		Action: reaper,
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:    "state-file",
+				Usage:   "state file",
+				EnvVars: []string{"DRONE_ADMIN_AUTOSCALER_REAPER_STATE_FILE"},
+				Value:   "/tmp/droneclean.gob",
+			},
 		},
-	},
+	}
 }
 
-func autoscalerReaper(c *cli.Context) error {
-	statefile := c.String("state-file")
-	scaler := c.StringSlice("server")
-	dry := c.Bool("dry-run")
+func reaper(ctx *cli.Context) error {
+	const maxRetries = 3
+
+	statefile := ctx.String("state-file")
+	scaler := ctx.StringSlice("server")
+	dry := ctx.Bool("dry-run")
 	state := map[string]int{}
 	force := false
 
@@ -44,7 +48,7 @@ func autoscalerReaper(c *cli.Context) error {
 	}
 
 	for _, scaler := range scaler {
-		client, err := client.New(scaler, c.String("token"))
+		client, err := client.New(scaler, ctx.String("token"))
 		if err != nil {
 			return err
 		}
@@ -53,6 +57,7 @@ func autoscalerReaper(c *cli.Context) error {
 		if err != nil {
 			return err
 		}
+
 		serversAll := len(servers)
 		servers = util.Filter(servers, func(s *drone.Server) bool {
 			return s.State == "running"
@@ -65,25 +70,26 @@ func autoscalerReaper(c *cli.Context) error {
 		}
 		logrus.WithFields(searchFields).Infof("lookup agents in error state")
 
-		for _, s := range servers {
-			state[s.Name]++
-			triage := state[s.Name]
+		for _, server := range servers {
+			state[server.Name]++
+			triage := state[server.Name]
 
-			if state[s.Name] == 3 {
+			if state[server.Name] == maxRetries {
 				force = true
-				delete(state, s.Name)
 
+				delete(state, server.Name)
 			}
 
 			foundFields := logrus.Fields{
 				"server": scaler,
-				"agent":  s.Name,
+				"agent":  server.Name,
 				"triage": triage,
 				"force":  force,
 			}
 			logrus.WithFields(foundFields).Infof("destroy agent")
+
 			if !dry {
-				err = serverDestroy(client, s.Name, force)
+				err = serverDestroy(client, server.Name, force)
 				if err != nil && !strings.Contains(err.Error(), "client error 404") {
 					return err
 				}

admin/build/build.go

@@ -2,10 +2,12 @@ package build
 
 import "github.com/urfave/cli/v2"
 
-var Command = &cli.Command{
-	Name:  "build",
-	Usage: "manage build",
-	Subcommands: []*cli.Command{
-		&buidPruneCmd,
-	},
+func GetBuildCmd() *cli.Command {
+	return &cli.Command{
+		Name:  "build",
+		Usage: "manage build",
+		Subcommands: []*cli.Command{
+			getPruneCmd(),
+		},
+	}
 }

admin/build/build_prune.go

@@ -11,29 +11,34 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-var buidPruneCmd = cli.Command{
-	Name:      "prune",
-	Usage:     "prune builds",
-	ArgsUsage: "<repo/name>",
-	Action:    buidPrune,
-	Flags: []cli.Flag{
-		&cli.StringFlag{
-			Name:     "older-than",
-			Usage:    "remove builds older than the specified time limit",
-			EnvVars:  []string{"DRONE_ADMIN_BUILD_PRUNE_OLDER_THAN"},
-			Required: true,
+const DroneClientPageSize = 50
+
+func getPruneCmd() *cli.Command {
+	return &cli.Command{
+		Name:      "prune",
+		Usage:     "prune builds",
+		ArgsUsage: "<repo/name>",
+		Action:    prune,
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:     "older-than",
+				Usage:    "remove builds older than the specified time limit",
+				EnvVars:  []string{"DRONE_ADMIN_BUILD_PRUNE_OLDER_THAN"},
+				Required: true,
+			},
+			&cli.IntFlag{
+				Name:    "keep-min",
+				Usage:   "minimum number of builds to keep",
+				EnvVars: []string{"DRONE_ADMIN_BUILD_PRUNE_KEEP_MIN"},
+				//nolint:gomnd
+				Value: 10,
+			},
 		},
-		&cli.IntFlag{
-			Name:    "keep-min",
-			Usage:   "minimum number of builds to keep",
-			EnvVars: []string{"DRONE_ADMIN_BUILD_PRUNE_KEEP_MIN"},
-			Value:   10,
-		},
-	},
+	}
 }
 
-func buidPrune(c *cli.Context) error {
-	client, err := client.New(c.StringSlice("server")[0], c.String("token"))
+func prune(ctx *cli.Context) error {
+	client, err := client.New(ctx.StringSlice("server")[0], ctx.String("token"))
 	if err != nil {
 		return err
 	}
@@ -43,11 +48,11 @@ func buidPrune(c *cli.Context) error {
 		return err
 	}
 
-	dur := c.String("older-than")
-	keep := c.Int("keep-min")
-	dry := c.Bool("dry-run")
+	buildDuration := ctx.String("older-than")
+	buildMin := ctx.Int("keep-min")
+	dry := ctx.Bool("dry-run")
 
-	duration, err := time.ParseDuration(dur)
+	duration, err := time.ParseDuration(buildDuration)
 	if err != nil {
 		return err
 	}
@@ -56,10 +61,10 @@ func buidPrune(c *cli.Context) error {
 		logrus.Info("dry-run enabled, no data will be removed")
 	}
 
-	logrus.Infof("prune builds older than %v, keep min %v", dur, keep)
+	logrus.Infof("prune builds older than %v, keep min %v", buildDuration, buildMin)
 
-	for _, r := range repos {
-		builds, err := getBuilds(client, r)
+	for _, repo := range repos {
+		builds, err := getBuilds(client, repo)
 		if err != nil {
 			return err
 		}
@@ -68,24 +73,23 @@ func buidPrune(c *cli.Context) error {
 			return builds[i].Number > builds[j].Number
 		})
 
-		if bl := len(builds); bl > 0 && bl > keep {
-			builds = builds[keep:]
+		if buildCount := len(builds); buildCount > 0 && buildCount > buildMin {
+			builds = builds[buildMin:]
 			builds = util.Filter(builds, func(b *drone.Build) bool {
 				return time.Since(time.Unix(b.Created, 0)) > duration
 			})
 
-			logrus.Infof("prune %v/%v builds from '%v'", len(builds), bl, r.Slug)
+			logrus.Infof("prune %v/%v builds from '%v'", len(builds), buildCount, repo.Slug)
 
 			if !dry && len(builds) > 0 {
-				err := client.BuildPurge(r.Namespace, r.Name, int(builds[0].Number+1))
+				err := client.BuildPurge(repo.Namespace, repo.Name, int(builds[0].Number+1))
 				if err != nil {
 					return err
 				}
 			}
 		} else {
-			logrus.Infof("skip '%v', number of %v builds lower than min value", r.Slug, len(builds))
+			logrus.Infof("skip '%v', number of %v builds lower than min value", repo.Slug, len(builds))
 		}
-
 	}
 
 	return nil
@@ -96,21 +100,21 @@ func getRepos(client drone.Client) ([]*drone.Repo, error) {
 	repos := make([]*drone.Repo, 0)
 
 	for {
-		r, err := client.RepoListAll(drone.ListOptions{Page: page, Size: 50})
+		repo, err := client.RepoListAll(drone.ListOptions{Page: page, Size: DroneClientPageSize})
 		if err != nil {
 			return nil, err
 		}
 
-		if len(r) == 0 {
+		if len(repo) == 0 {
 			break
 		}
 
-		repos = append(repos, r...)
+		repos = append(repos, repo...)
 		page++
 	}
 
-	repos = util.Filter(repos, func(r *drone.Repo) bool {
-		return r.Active
+	repos = util.Filter(repos, func(repo *drone.Repo) bool {
+		return repo.Active
 	})
 
 	return repos, nil
@@ -121,17 +125,18 @@ func getBuilds(client drone.Client, repo *drone.Repo) ([]*drone.Build, error) {
 	builds := make([]*drone.Build, 0)
 
 	for {
-		b, err := client.BuildList(repo.Namespace, repo.Name, drone.ListOptions{Page: page, Size: 50})
+		build, err := client.BuildList(repo.Namespace, repo.Name, drone.ListOptions{Page: page, Size: DroneClientPageSize})
 		if err != nil {
 			return nil, err
 		}
 
-		if len(b) == 0 {
+		if len(build) == 0 {
 			break
 		}
 
-		builds = append(builds, b...)
+		builds = append(builds, build...)
 		page++
 	}
+
 	return builds, nil
 }

admin/client/client.go

@@ -13,12 +13,13 @@ import (
 )
 
 func New(server, token string) (drone.Client, error) {
-	s, err := url.Parse(server)
+	serverURL, err := url.Parse(server)
 	if err != nil {
 		return nil, err
 	}
-	if len(s.Scheme) == 0 {
-		s.Scheme = "http"
+
+	if len(serverURL.Scheme) == 0 {
+		serverURL.Scheme = "http"
 	}
 
 	// attempt to find system CA certs
@@ -26,6 +27,7 @@ func New(server, token string) (drone.Client, error) {
 	tlsConfig := &tls.Config{
 		RootCAs:            certs,
 		InsecureSkipVerify: false,
+		MinVersion:         tls.VersionTLS12,
 	}
 
 	oauth := new(oauth2.Config)
@@ -44,5 +46,5 @@ func New(server, token string) (drone.Client, error) {
 		Proxy:           http.ProxyFromEnvironment,
 	}
 
-	return drone.NewClient(s.String(), authenticator), nil
+	return drone.NewClient(serverURL.String(), authenticator), nil
 }

admin/util/util.go

@@ -7,11 +7,13 @@ import (
 
 func Filter[T any](vs []T, f func(T) bool) []T {
 	filtered := make([]T, 0)
+
 	for _, v := range vs {
 		if f(v) {
 			filtered = append(filtered, v)
 		}
 	}
+
 	return filtered
 }
 
@@ -21,7 +23,9 @@ func WriteGob(filePath string, object interface{}) error {
 		encoder := gob.NewEncoder(file)
 		err = encoder.Encode(object)
 	}
+
 	file.Close()
+
 	return err
 }
 
@@ -31,6 +35,8 @@ func ReadGob(filePath string, object interface{}) error {
 		decoder := gob.NewDecoder(file)
 		err = decoder.Decode(object)
 	}
+
 	file.Close()
+
 	return err
 }

cmd/drone-admin/main.go

@@ -13,6 +13,7 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
+//nolint:gochecknoglobals
 var (
 	BuildVersion = "devel"
 	BuildDate    = "00000000"
@@ -69,8 +70,8 @@ func main() {
 			return nil
 		},
 		Commands: []*cli.Command{
-			build.Command,
-			autoscaler.Command,
+			build.GetBuildCmd(),
+			autoscaler.GetAutoscalerCmd(),
 		},
 	}
 

go.mod

@@ -1,6 +1,6 @@
 module github.com/thegeeklab/drone-admin
 
-go 1.19
+go 1.20
 
 require (
 	github.com/drone/drone-go v1.7.1