chore(lint): add lint to the CI and make it happy (#685)

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/685
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

.golangci.yml

@@ -26,12 +26,7 @@ linters:
   fast: false
 
 run:
-  go: 1.18
   timeout: 10m
-  skip-dirs:
-    - node_modules
-    - public
-    - web_src
 
 linters-settings:
   stylecheck:
@@ -72,10 +67,12 @@ linters-settings:
       - name: modifies-value-receiver
   gofumpt:
     extra-rules: true
-    lang-version: "1.18"
   depguard:
-    list-type: denylist
+    rules:
-    include-go-root: true
+      main:
+        deny:
+          - pkg: io/ioutil
+            desc: use os or io instead
 
 issues:
   exclude-rules:

Makefile

@@ -14,6 +14,7 @@ GO_FMT_FILES := $(shell find . -type f -name "*.go" ! -name "generated.*")
 GOFILES := $(shell find . -type f -name "*.go" -o -name "go.mod" ! -name "generated.*")
 
 MOCKERY_PACKAGE ?= github.com/vektra/mockery/v2@v2.53.4 # renovate: datasource=go
+GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.2 # renovate: datasource=go
 
 DOCKER_IMAGE ?= gitea/act_runner
 DOCKER_TAG ?= nightly
@@ -63,12 +64,19 @@ endif
 
 GO_PACKAGES_TO_VET ?= $(filter-out runner.forgejo.org/internal/pkg/client/mocks,$(shell $(GO) list ./...))
 
-
 TAGS ?=
 LDFLAGS ?= -X "runner.forgejo.org/internal/pkg/ver.version=v$(RELEASE_VERSION)"
 
 all: build
 
+.PHONY: lint
+lint-check:
+	$(GO) run $(GOLANGCI_LINT_PACKAGE) run $(GOLANGCI_LINT_ARGS)
+
+.PHONY: lint-fix
+lint:
+	$(GO) run $(GOLANGCI_LINT_PACKAGE) run $(GOLANGCI_LINT_ARGS) --fix
+
 fmt:
 	@hash gofumpt > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
 		$(GO) install mvdan.cc/gofumpt@latest; \
@@ -97,7 +105,7 @@ fmt-check:
 		exit 1; \
 	fi;
 
-test: fmt-check
+test: lint-check fmt-check
 	@$(GO) test -v -cover -coverprofile coverage.txt ./... && echo "\n==>\033[32m Ok\033[m\n" || exit 1
 
 .PHONY: vet

internal/app/cmd/create-runner-file.go

@@ -37,9 +37,9 @@ func createRunnerFileCmd(ctx context.Context, configFile *string) *cobra.Command
 	}
 	cmd.Flags().BoolVar(&argsVar.Connect, "connect", false, "tries to connect to the instance using the secret (Forgejo v1.21 instance or greater)")
 	cmd.Flags().StringVar(&argsVar.InstanceAddr, "instance", "", "Forgejo instance address")
-	cmd.MarkFlagRequired("instance")
+	_ = cmd.MarkFlagRequired("instance")
 	cmd.Flags().StringVar(&argsVar.Secret, "secret", "", "secret shared with the Forgejo instance via forgejo-cli actions register")
-	cmd.MarkFlagRequired("secret")
+	_ = cmd.MarkFlagRequired("secret")
 	cmd.Flags().StringVar(&argsVar.Name, "name", "", "Runner name")
 
 	return cmd

internal/app/cmd/create-runner-file_test.go

@@ -70,7 +70,7 @@ func Test_runCreateRunnerFile(t *testing.T) {
 	dir := t.TempDir()
 	configFile := dir + "/config.yml"
 	runnerFile := dir + "/.runner"
-	cfg, err := config.LoadDefault("")
+	cfg, _ := config.LoadDefault("")
 	cfg.Runner.File = runnerFile
 	yamlData, err := yaml.Marshal(cfg)
 	assert.NoError(t, err)

internal/app/run/runner.go

@@ -98,7 +98,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client)
 }
 
 func setupCache(cfg *config.Config, envs map[string]string) *cacheproxy.Handler {
-	var cacheUrl string
+	var cacheURL string
 	var cacheSecret string
 
 	if cfg.Cache.ExternalServer == "" {
@@ -127,7 +127,7 @@ func setupCache(cfg *config.Config, envs map[string]string) *cacheproxy.Handler
 			return nil
 		}
 
-		cacheUrl = cacheServer.ExternalURL()
+		cacheURL = cacheServer.ExternalURL()
 	} else {
 		// An external cache server was specified, use its url
 		cacheSecret = cfg.Cache.Secret
@@ -137,11 +137,11 @@ func setupCache(cfg *config.Config, envs map[string]string) *cacheproxy.Handler
 			return nil
 		}
 
-		cacheUrl = strings.TrimSuffix(cfg.Cache.ExternalServer, "/")
+		cacheURL = strings.TrimSuffix(cfg.Cache.ExternalServer, "/")
 	}
 
 	cacheProxy, err := cacheproxy.StartHandler(
-		cacheUrl,
+		cacheURL,
 		cfg.Cache.Host,
 		cfg.Cache.ProxyPort,
 		cacheSecret,
@@ -152,8 +152,8 @@ func setupCache(cfg *config.Config, envs map[string]string) *cacheproxy.Handler
 	}
 
 	envs["ACTIONS_CACHE_URL"] = cacheProxy.ExternalURL()
-	if cfg.Cache.ActionsCacheUrlOverride != "" {
+	if cfg.Cache.ActionsCacheURLOverride != "" {
-		envs["ACTIONS_CACHE_URL"] = cfg.Cache.ActionsCacheUrlOverride
+		envs["ACTIONS_CACHE_URL"] = cfg.Cache.ActionsCacheURLOverride
 	}
 
 	return cacheProxy
@@ -256,11 +256,11 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 	if r.cacheProxy != nil {
 		timestamp := strconv.FormatInt(time.Now().Unix(), 10)
 		cacheRunData := r.cacheProxy.CreateRunData(preset.Repository, preset.RunID, timestamp)
-		cacheRunId, err := r.cacheProxy.AddRun(cacheRunData)
+		cacheRunID, err := r.cacheProxy.AddRun(cacheRunData)
 		if err == nil {
-			defer r.cacheProxy.RemoveRun(cacheRunId)
+			defer func() { _ = r.cacheProxy.RemoveRun(cacheRunID) }()
 			baseURL := runEnvs["ACTIONS_CACHE_URL"]
-			runEnvs["ACTIONS_CACHE_URL"] = fmt.Sprintf("%s/%s/", baseURL, cacheRunId)
+			runEnvs["ACTIONS_CACHE_URL"] = fmt.Sprintf("%s/%s/", baseURL, cacheRunID)
 		}
 	}
 
@@ -340,7 +340,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 	}
 
 	execErr := executor(ctx)
-	reporter.SetOutputs(job.Outputs)
+	_ = reporter.SetOutputs(job.Outputs)
 	return execErr
 }
 

internal/pkg/config/config.go

@@ -43,7 +43,7 @@ type Cache struct {
 	Port                    uint16 `yaml:"port"`                       // Port specifies the caching port.
 	ProxyPort               uint16 `yaml:"proxy_port"`                 // ProxyPort specifies the cache proxy port.
 	ExternalServer          string `yaml:"external_server"`            // ExternalServer specifies the URL of external cache server
-	ActionsCacheUrlOverride string `yaml:"actions_cache_url_override"` // Allows the user to override the ACTIONS_CACHE_URL passed to the workflow containers
+	ActionsCacheURLOverride string `yaml:"actions_cache_url_override"` // Allows the user to override the ACTIONS_CACHE_URL passed to the workflow containers
 	Secret                  string `yaml:"secret"`                     // Shared secret to secure caches.
 }
 

internal/pkg/labels/labels_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+
 	"gotest.tools/v3/assert"
 )
 

internal/pkg/report/mask.go

@@ -81,34 +81,34 @@ func (o *masker) trimEOL(s string) string {
 func (o *masker) equalMultiline(multiLine []string, rows []*runnerv1.LogRow) (equal, needMore bool) {
 	if len(rows) < 2 {
 		needMore = true
-		return
+		return equal, needMore
 	}
 
 	lastIndex := len(multiLine) - 1
 	first := multiLine[0]
 	if !strings.HasSuffix(o.trimEOL(rows[0].Content), first) {
-		return // unreachable because the caller checks that already
+		return equal, needMore // unreachable because the caller checks that already
 	}
 	for i, line := range multiLine[1:lastIndex] {
 		rowIndex := i + 1
 		if rowIndex >= len(rows) {
 			needMore = true
-			return
+			return equal, needMore
 		}
 		if o.trimEOL(rows[rowIndex].Content) != line {
-			return
+			return equal, needMore
 		}
 	}
 	last := multiLine[lastIndex]
 	if lastIndex >= len(rows) {
 		needMore = true
-		return
+		return equal, needMore
 	}
 	if !strings.HasPrefix(o.trimEOL(rows[lastIndex].Content), last) {
-		return
+		return equal, needMore
 	}
 	equal = true
-	return
+	return equal, needMore
 }
 
 func (o *masker) replaceMultiline(multiLine []string, rows []*runnerv1.LogRow) {

internal/pkg/report/reporter.go

@@ -280,7 +280,7 @@ func (err ErrRetry) Is(target error) bool {
 	return ok
 }
 
-var (
+const (
 	errRetryNeedMoreRows = "need more rows to figure out if multiline secrets must be masked"
 	errRetrySendAll      = "not all logs are submitted %d remain"
 )

internal/pkg/report/reporter_test.go

@@ -387,10 +387,8 @@ func TestReporterReportLog(t *testing.T) {
 			err := reporter.ReportLog(testCase.noMore)
 			if testCase.err == nil {
 				assert.NoError(t, err)
-			} else {
+			} else if assert.ErrorIs(t, err, testCase.err) {
-				if assert.ErrorIs(t, err, testCase.err) {
+				assert.Equal(t, err.Error(), testCase.err.Error())
-					assert.Equal(t, err.Error(), testCase.err.Error())
-				}
 			}
 			if testCase.sent != "" {
 				assert.Equal(t, testCase.sent, sent)