chore(lint): add golangci-lint v2 and fix all lint issues (#803)

## Summary
- Replace old `.golangci.yml` (v1 format) with v2 format, aligned with gitea's lint config
- Add `lint-go`, `lint-go-fix`, and `lint` Makefile targets using golangci-lint v2.10.1
- Replace `make vet` with `make lint` in CI workflow (lint includes vet)
- Fix all 35 lint issues: modernize (maps.Copy, range over int, any), perfsprint (errors.New), unparam (remove unused parameters), revive (var naming), staticcheck, forbidigo exclusion for cmd/
- Make `security-check` non-fatal (apply https://github.com/go-gitea/gitea/pull/36681)
- Remove dead gocritic exclusion rules (commentFormatting, exitAfterDefer)
- Remove dead linter exclusions and disabled checks (singleCaseSwitch, ST1003, QF1001, QF1006, QF1008, testifylint go-require/require-error, test file exclusions for dupl/errcheck/staticcheck/unparam)

## Test plan
- [x] `golangci-lint run` passes
- [x] `go build ./...` passes
- [x] `go test ./...` passes

---------

Co-authored-by: ChristopherHX <christopher.homberger@web.de>
Co-authored-by: Christopher Homberger <christopher.homberger@web.de>
Reviewed-on: https://gitea.com/gitea/act_runner/pulls/803
Reviewed-by: ChristopherHX <christopherhx@noreply.gitea.com>
This commit is contained in:
silverwind
2026-02-22 17:35:08 +00:00
parent f0f9f0c8ab
commit 658101d9cb
14 changed files with 181 additions and 227 deletions

View File

@@ -12,8 +12,8 @@ jobs:
- uses: actions/setup-go@v6 - uses: actions/setup-go@v6
with: with:
go-version-file: 'go.mod' go-version-file: 'go.mod'
- name: vet checks - name: lint
run: make vet run: make lint
- name: build - name: build
run: make build run: make build
- name: test - name: test

View File

@@ -1,165 +1,112 @@
version: "2"
output:
sort-order:
- file
linters: linters:
default: none
enable: enable:
- gosimple
- typecheck
- govet
- errcheck
- staticcheck
- unused
- dupl
#- gocyclo # The cyclomatic complexety of a lot of functions is too high, we should refactor those another time.
- gofmt
- misspell
- gocritic
- bidichk - bidichk
- ineffassign - bodyclose
- revive
- gofumpt
- depguard - depguard
- dupl
- errcheck
- forbidigo
- gocheckcompilerdirectives
- gocritic
- govet
- ineffassign
- mirror
- modernize
- nakedret - nakedret
- unconvert - nilnil
- wastedassign
- nolintlint - nolintlint
- stylecheck - perfsprint
enable-all: false - revive
disable-all: true - staticcheck
fast: false - testifylint
- unconvert
run: - unparam
go: 1.18 - unused
timeout: 10m - usestdlibvars
skip-dirs: - usetesting
- node_modules - wastedassign
- public settings:
- web_src depguard:
rules:
linters-settings: main:
stylecheck: deny:
checks: ["all", "-ST1005", "-ST1003"] - pkg: io/ioutil
nakedret: desc: use os or io instead
max-func-lines: 0 - pkg: golang.org/x/exp
gocritic: desc: it's experimental and unreliable
disabled-checks: - pkg: github.com/pkg/errors
- ifElseChain desc: use builtin errors package instead
- singleCaseSwitch # Every time this occurred in the code, there was no other way. nolintlint:
revive: allow-unused: false
ignore-generated-header: false require-explanation: true
severity: warning require-specific: true
confidence: 0.8 gocritic:
errorCode: 1 enabled-checks:
warningCode: 1 - equalFold
disabled-checks:
- ifElseChain
revive:
severity: error
rules:
- name: blank-imports
- name: constant-logical-expr
- name: context-as-argument
- name: context-keys-type
- name: dot-imports
- name: empty-lines
- name: error-return
- name: error-strings
- name: exported
- name: identical-branches
- name: if-return
- name: increment-decrement
- name: modifies-value-receiver
- name: package-comments
- name: redefines-builtin-id
- name: superfluous-else
- name: time-naming
- name: unexported-return
- name: var-declaration
- name: var-naming
staticcheck:
checks:
- all
- -ST1005
usetesting:
os-temp-dir: true
perfsprint:
concat-loop: false
govet:
enable:
- nilness
- unusedwrite
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
rules: rules:
- name: blank-imports - linters:
- name: context-as-argument - forbidigo
- name: context-keys-type path: cmd
- name: dot-imports
- name: error-return
- name: error-strings
- name: error-naming
- name: exported
- name: if-return
- name: increment-decrement
- name: var-naming
- name: var-declaration
- name: package-comments
- name: range
- name: receiver-naming
- name: time-naming
- name: unexported-return
- name: indent-error-flow
- name: errorf
- name: duplicated-imports
- name: modifies-value-receiver
gofumpt:
extra-rules: true
lang-version: "1.18"
depguard:
# TODO: use depguard to replace import checks in gitea-vet
list-type: denylist
# Check the list against standard lib.
include-go-root: true
packages-with-error-message:
- github.com/unknwon/com: "use gitea's util and replacements"
issues: issues:
exclude-rules: max-issues-per-linter: 0
# Exclude some linters from running on tests files. max-same-issues: 0
- path: _test\.go formatters:
linters: enable:
- gocyclo - gofmt
- errcheck - gofumpt
- dupl settings:
- gosec gofumpt:
- unparam extra-rules: true
- staticcheck exclusions:
- path: models/migrations/v generated: lax
linters: run:
- gocyclo timeout: 10m
- errcheck
- dupl
- gosec
- linters:
- dupl
text: "webhook"
- linters:
- gocritic
text: "`ID' should not be capitalized"
- path: modules/templates/helper.go
linters:
- gocritic
- linters:
- unused
text: "swagger"
- path: contrib/pr/checkout.go
linters:
- errcheck
- path: models/issue.go
linters:
- errcheck
- path: models/migrations/
linters:
- errcheck
- path: modules/log/
linters:
- errcheck
- path: routers/api/v1/repo/issue_subscription.go
linters:
- dupl
- path: routers/repo/view.go
linters:
- dupl
- path: models/migrations/
linters:
- unused
- linters:
- staticcheck
text: "argument x is overwritten before first use"
- path: modules/httplib/httplib.go
linters:
- staticcheck
# Enabling this would require refactoring the methods and how they are called.
- path: models/issue_comment_list.go
linters:
- dupl
- linters:
- misspell
text: '`Unknwon` is a misspelling of `Unknown`'
- path: models/update.go
linters:
- unused
- path: cmd/dump.go
linters:
- dupl
- text: "commentFormatting: put a space between `//` and comment text"
linters:
- gocritic
- text: "exitAfterDefer:"
linters:
- gocritic
- path: modules/graceful/manager_windows.go
linters:
- staticcheck
text: "svc.IsAnInteractiveSession is deprecated: Use IsWindowsService instead."
- path: models/user/openid.go
linters:
- golint

View File

@@ -20,6 +20,7 @@ DOCKER_TAG ?= nightly
DOCKER_REF := $(DOCKER_IMAGE):$(DOCKER_TAG) DOCKER_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)
DOCKER_ROOTLESS_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)-dind-rootless DOCKER_ROOTLESS_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)-dind-rootless
GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.10.1
GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1
ifneq ($(shell uname), Darwin) ifneq ($(shell uname), Darwin)
@@ -107,9 +108,20 @@ fmt-check:
deps-tools: ## install tool dependencies deps-tools: ## install tool dependencies
$(GO) install $(GOVULNCHECK_PACKAGE) $(GO) install $(GOVULNCHECK_PACKAGE)
.PHONY: lint
lint: lint-go vet
.PHONY: lint-go
lint-go: ## lint go files
$(GO) run $(GOLANGCI_LINT_PACKAGE) run
.PHONY: lint-go-fix
lint-go-fix: ## lint go files and fix issues
$(GO) run $(GOLANGCI_LINT_PACKAGE) run --fix
.PHONY: security-check .PHONY: security-check
security-check: deps-tools security-check: deps-tools
GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... || true
.PHONY: tidy .PHONY: tidy
tidy: tidy:

View File

@@ -4,7 +4,6 @@
package cmd package cmd
import ( import (
"context"
"fmt" "fmt"
"os" "os"
"os/signal" "os/signal"
@@ -22,7 +21,7 @@ type cacheServerArgs struct {
Port uint16 Port uint16
} }
func runCacheServer(ctx context.Context, configFile *string, cacheArgs *cacheServerArgs) func(cmd *cobra.Command, args []string) error { func runCacheServer(configFile *string, cacheArgs *cacheServerArgs) func(cmd *cobra.Command, args []string) error {
return func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error {
cfg, err := config.LoadDefault(*configFile) cfg, err := config.LoadDefault(*configFile)
if err != nil { if err != nil {

View File

@@ -72,7 +72,7 @@ func Execute(ctx context.Context) {
Use: "cache-server", Use: "cache-server",
Short: "Start a cache server for the cache action", Short: "Start a cache server for the cache action",
Args: cobra.MaximumNArgs(0), Args: cobra.MaximumNArgs(0),
RunE: runCacheServer(ctx, &configFile, &cacheArgs), RunE: runCacheServer(&configFile, &cacheArgs),
} }
cacheCmd.Flags().StringVarP(&cacheArgs.Dir, "dir", "d", "", "Cache directory") cacheCmd.Flags().StringVarP(&cacheArgs.Dir, "dir", "d", "", "Cache directory")
cacheCmd.Flags().StringVarP(&cacheArgs.Host, "host", "s", "", "Host of the cache server") cacheCmd.Flags().StringVarP(&cacheArgs.Host, "host", "s", "", "Host of the cache server")

View File

@@ -262,5 +262,5 @@ func getDockerSocketPath(configDockerHost string) (string, error) {
} }
} }
return "", fmt.Errorf("daemon Docker Engine socket not found and docker_host config was invalid") return "", errors.New("daemon Docker Engine socket not found and docker_host config was invalid")
} }

View File

@@ -6,7 +6,9 @@ package cmd
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"maps"
"os" "os"
"path/filepath" "path/filepath"
"strconv" "strconv"
@@ -77,7 +79,7 @@ func (i *executeArgs) LoadSecrets() map[string]string {
for _, secretPair := range i.secrets { for _, secretPair := range i.secrets {
secretPairParts := strings.SplitN(secretPair, "=", 2) secretPairParts := strings.SplitN(secretPair, "=", 2)
secretPairParts[0] = strings.ToUpper(secretPairParts[0]) secretPairParts[0] = strings.ToUpper(secretPairParts[0])
if strings.ToUpper(s[secretPairParts[0]]) == secretPairParts[0] { if strings.EqualFold(s[secretPairParts[0]], secretPairParts[0]) {
log.Errorf("Secret %s is already defined (secrets are case insensitive)", secretPairParts[0]) log.Errorf("Secret %s is already defined (secrets are case insensitive)", secretPairParts[0])
} }
if len(secretPairParts) == 2 { if len(secretPairParts) == 2 {
@@ -104,9 +106,7 @@ func readEnvs(path string, envs map[string]string) bool {
if err != nil { if err != nil {
log.Fatalf("Error loading from %s: %v", path, err) log.Fatalf("Error loading from %s: %v", path, err)
} }
for k, v := range env { maps.Copy(envs, env)
envs[k] = v
}
return true return true
} }
return false return false
@@ -166,7 +166,7 @@ func (i *executeArgs) resolve(path string) string {
return path return path
} }
func printList(plan *model.Plan) error { func printList(plan *model.Plan) {
type lineInfoDef struct { type lineInfoDef struct {
jobID string jobID string
jobName string jobName string
@@ -261,10 +261,9 @@ func printList(plan *model.Plan) error {
if duplicateJobIDs { if duplicateJobIDs {
fmt.Print("\nDetected multiple jobs with the same job name, use `-W` to specify the path to the specific workflow.\n") fmt.Print("\nDetected multiple jobs with the same job name, use `-W` to specify the path to the specific workflow.\n")
} }
return nil
} }
func runExecList(ctx context.Context, planner model.WorkflowPlanner, execArgs *executeArgs) error { func runExecList(planner model.WorkflowPlanner, execArgs *executeArgs) error {
// plan with filtered jobs - to be used for filtering only // plan with filtered jobs - to be used for filtering only
var filterPlan *model.Plan var filterPlan *model.Plan
@@ -306,7 +305,7 @@ func runExecList(ctx context.Context, planner model.WorkflowPlanner, execArgs *e
} }
} }
_ = printList(filterPlan) printList(filterPlan)
return nil return nil
} }
@@ -319,7 +318,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command
} }
if execArgs.runList { if execArgs.runList {
return runExecList(ctx, planner, execArgs) return runExecList(planner, execArgs)
} }
// plan with triggered jobs // plan with triggered jobs
@@ -378,7 +377,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command
if len(execArgs.artifactServerAddr) == 0 { if len(execArgs.artifactServerAddr) == 0 {
ip := common.GetOutboundIP() ip := common.GetOutboundIP()
if ip == nil { if ip == nil {
return fmt.Errorf("unable to determine outbound IP address") return errors.New("unable to determine outbound IP address")
} }
execArgs.artifactServerAddr = ip.String() execArgs.artifactServerAddr = ip.String()
} }
@@ -422,7 +421,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command
NoSkipCheckout: execArgs.noSkipCheckout, NoSkipCheckout: execArgs.noSkipCheckout,
// PresetGitHubContext: preset, // PresetGitHubContext: preset,
// EventJSON: string(eventJSON), // EventJSON: string(eventJSON),
ContainerNamePrefix: fmt.Sprintf("GITEA-ACTIONS-TASK-%s", eventName), ContainerNamePrefix: "GITEA-ACTIONS-TASK-" + eventName,
ContainerMaxLifetime: maxLifetime, ContainerMaxLifetime: maxLifetime,
ContainerNetworkMode: container.NetworkMode(execArgs.network), ContainerNetworkMode: container.NetworkMode(execArgs.network),
DefaultActionInstance: execArgs.defaultActionsURL, DefaultActionInstance: execArgs.defaultActionsURL,

View File

@@ -6,6 +6,7 @@ package cmd
import ( import (
"bufio" "bufio"
"context" "context"
"errors"
"fmt" "fmt"
"os" "os"
"os/signal" "os/signal"
@@ -107,10 +108,10 @@ type registerInputs struct {
func (r *registerInputs) validate() error { func (r *registerInputs) validate() error {
if r.InstanceAddr == "" { if r.InstanceAddr == "" {
return fmt.Errorf("instance address is empty") return errors.New("instance address is empty")
} }
if r.Token == "" { if r.Token == "" {
return fmt.Errorf("token is empty") return errors.New("token is empty")
} }
if len(r.Labels) > 0 { if len(r.Labels) > 0 {
return validateLabels(r.Labels) return validateLabels(r.Labels)

View File

@@ -102,7 +102,7 @@ func (p *Poller) Shutdown(ctx context.Context) error {
p.shutdownJobs() p.shutdownJobs()
// wait for running jobs to report their status to Gitea // wait for running jobs to report their status to Gitea
_, _ = <-p.done <-p.done
return ctx.Err() return ctx.Err()
} }

View File

@@ -7,6 +7,7 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"maps"
"path/filepath" "path/filepath"
"strings" "strings"
"sync" "sync"
@@ -49,9 +50,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client)
} }
} }
envs := make(map[string]string, len(cfg.Runner.Envs)) envs := make(map[string]string, len(cfg.Runner.Envs))
for k, v := range cfg.Runner.Envs { maps.Copy(envs, cfg.Runner.Envs)
envs[k] = v
}
if cfg.Cache.Enabled == nil || *cfg.Cache.Enabled { if cfg.Cache.Enabled == nil || *cfg.Cache.Enabled {
if cfg.Cache.ExternalServer != "" { if cfg.Cache.ExternalServer != "" {
envs["ACTIONS_CACHE_URL"] = cfg.Cache.ExternalServer envs["ACTIONS_CACHE_URL"] = cfg.Cache.ExternalServer
@@ -116,7 +115,7 @@ func (r *Runner) Run(ctx context.Context, task *runnerv1.Task) error {
// getDefaultActionsURL // getDefaultActionsURL
// when DEFAULT_ACTIONS_URL == "https://github.com" and GithubMirror is not blank, // when DEFAULT_ACTIONS_URL == "https://github.com" and GithubMirror is not blank,
// it should be set to GithubMirror first. // it should be set to GithubMirror first.
func (r *Runner) getDefaultActionsURL(ctx context.Context, task *runnerv1.Task) string { func (r *Runner) getDefaultActionsURL(task *runnerv1.Task) string {
giteaDefaultActionsURL := task.Context.Fields["gitea_default_actions_url"].GetStringValue() giteaDefaultActionsURL := task.Context.Fields["gitea_default_actions_url"].GetStringValue()
if giteaDefaultActionsURL == "https://github.com" && r.cfg.Runner.GithubMirror != "" { if giteaDefaultActionsURL == "https://github.com" && r.cfg.Runner.GithubMirror != "" {
return r.cfg.Runner.GithubMirror return r.cfg.Runner.GithubMirror
@@ -148,7 +147,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
taskContext := task.Context.Fields taskContext := task.Context.Fields
log.Infof("task %v repo is %v %v %v", task.Id, taskContext["repository"].GetStringValue(), log.Infof("task %v repo is %v %v %v", task.Id, taskContext["repository"].GetStringValue(),
r.getDefaultActionsURL(ctx, task), r.getDefaultActionsURL(task),
r.client.Address()) r.client.Address())
preset := &model.GithubContext{ preset := &model.GithubContext{
@@ -174,8 +173,8 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
preset.Token = t preset.Token = t
} }
if actionsIdTokenRequestUrl := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIdTokenRequestUrl != "" { if actionsIDTokenRequestURL := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIDTokenRequestURL != "" {
r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIdTokenRequestUrl r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIDTokenRequestURL
r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = taskContext["actions_id_token_request_token"].GetStringValue() r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = taskContext["actions_id_token_request_token"].GetStringValue()
task.Secrets["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] task.Secrets["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"]
} }
@@ -222,7 +221,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
ContainerOptions: r.cfg.Container.Options, ContainerOptions: r.cfg.Container.Options,
ContainerDaemonSocket: r.cfg.Container.DockerHost, ContainerDaemonSocket: r.cfg.Container.DockerHost,
Privileged: r.cfg.Container.Privileged, Privileged: r.cfg.Container.Privileged,
DefaultActionInstance: r.getDefaultActionsURL(ctx, task), DefaultActionInstance: r.getDefaultActionsURL(task),
PlatformPicker: r.labels.PickPlatform, PlatformPicker: r.labels.PickPlatform,
Vars: task.Vars, Vars: task.Vars,
ValidVolumes: r.cfg.Container.ValidVolumes, ValidVolumes: r.cfg.Container.ValidVolumes,

View File

@@ -307,7 +307,7 @@ func Test_yamlV4NodeRoundTrip(t *testing.T) {
t.Run("unmarshal and re-marshal workflow", func(t *testing.T) { t.Run("unmarshal and re-marshal workflow", func(t *testing.T) {
input := []byte("name: test\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo hello\n") input := []byte("name: test\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo hello\n")
var wf map[string]interface{} var wf map[string]any
err := yaml.Unmarshal(input, &wf) err := yaml.Unmarshal(input, &wf)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, wf["name"], "test") assert.Equal(t, wf["name"], "test")
@@ -315,7 +315,7 @@ func Test_yamlV4NodeRoundTrip(t *testing.T) {
out, err := yaml.Marshal(wf) out, err := yaml.Marshal(wf)
require.NoError(t, err) require.NoError(t, err)
var wf2 map[string]interface{} var wf2 map[string]any
err = yaml.Unmarshal(out, &wf2) err = yaml.Unmarshal(out, &wf2)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, wf2["name"], "test") assert.Equal(t, wf2["name"], "test")

View File

@@ -5,6 +5,7 @@ package config
import ( import (
"fmt" "fmt"
"maps"
"os" "os"
"path/filepath" "path/filepath"
"time" "time"
@@ -96,9 +97,7 @@ func LoadDefault(file string) (*Config, error) {
if cfg.Runner.Envs == nil { if cfg.Runner.Envs == nil {
cfg.Runner.Envs = map[string]string{} cfg.Runner.Envs = map[string]string{}
} }
for k, v := range envs { maps.Copy(cfg.Runner.Envs, envs)
cfg.Runner.Envs[k] = v
}
} }
} }

View File

@@ -78,7 +78,7 @@ func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.C
func (r *Reporter) ResetSteps(l int) { func (r *Reporter) ResetSteps(l int) {
r.stateMu.Lock() r.stateMu.Lock()
defer r.stateMu.Unlock() defer r.stateMu.Unlock()
for i := 0; i < l; i++ { for i := range l {
r.state.Steps = append(r.state.Steps, &runnerv1.StepState{ r.state.Steps = append(r.state.Steps, &runnerv1.StepState{
Id: int64(i), Id: int64(i),
}) })
@@ -207,14 +207,14 @@ func (r *Reporter) RunDaemon() {
time.AfterFunc(time.Second, r.RunDaemon) time.AfterFunc(time.Second, r.RunDaemon)
} }
func (r *Reporter) Logf(format string, a ...interface{}) { func (r *Reporter) Logf(format string, a ...any) {
r.stateMu.Lock() r.stateMu.Lock()
defer r.stateMu.Unlock() defer r.stateMu.Unlock()
r.logf(format, a...) r.logf(format, a...)
} }
func (r *Reporter) logf(format string, a ...interface{}) { func (r *Reporter) logf(format string, a ...any) {
if !r.duringSteps() { if !r.duringSteps() {
r.logRows = append(r.logRows, &runnerv1.LogRow{ r.logRows = append(r.logRows, &runnerv1.LogRow{
Time: timestamppb.Now(), Time: timestamppb.Now(),
@@ -307,7 +307,7 @@ func (r *Reporter) ReportLog(noMore bool) error {
ack := int(resp.Msg.AckIndex) ack := int(resp.Msg.AckIndex)
if ack < r.logOffset { if ack < r.logOffset {
return fmt.Errorf("submitted logs are lost") return errors.New("submitted logs are lost")
} }
r.stateMu.Lock() r.stateMu.Lock()
@@ -317,7 +317,7 @@ func (r *Reporter) ReportLog(noMore bool) error {
r.stateMu.Unlock() r.stateMu.Unlock()
if noMore && ack < submitted { if noMore && ack < submitted {
return fmt.Errorf("not all logs are submitted") return errors.New("not all logs are submitted")
} }
return nil return nil
@@ -339,7 +339,7 @@ func (r *Reporter) ReportState(reportResult bool) error {
} }
outputs := make(map[string]string) outputs := make(map[string]string)
r.outputs.Range(func(k, v interface{}) bool { r.outputs.Range(func(k, v any) bool {
if val, ok := v.(string); ok { if val, ok := v.(string); ok {
outputs[k.(string)] = val outputs[k.(string)] = val
} }
@@ -363,7 +363,7 @@ func (r *Reporter) ReportState(reportResult bool) error {
} }
var noSent []string var noSent []string
r.outputs.Range(func(k, v interface{}) bool { r.outputs.Range(func(k, v any) bool {
if _, ok := v.(string); ok { if _, ok := v.(string); ok {
noSent = append(noSent, k.(string)) noSent = append(noSent, k.(string))
} }
@@ -394,7 +394,7 @@ var stringToResult = map[string]runnerv1.Result{
"cancelled": runnerv1.Result_RESULT_CANCELLED, "cancelled": runnerv1.Result_RESULT_CANCELLED,
} }
func (r *Reporter) parseResult(result interface{}) (runnerv1.Result, bool) { func (r *Reporter) parseResult(result any) (runnerv1.Result, bool) {
str := "" str := ""
if v, ok := result.(string); ok { // for jobResult if v, ok := result.(string); ok { // for jobResult
str = v str = v
@@ -408,7 +408,7 @@ func (r *Reporter) parseResult(result interface{}) (runnerv1.Result, bool) {
var cmdRegex = regexp.MustCompile(`^::([^ :]+)( .*)?::(.*)$`) var cmdRegex = regexp.MustCompile(`^::([^ :]+)( .*)?::(.*)$`)
func (r *Reporter) handleCommand(originalContent, command, parameters, value string) *string { func (r *Reporter) handleCommand(originalContent, command, value string) *string {
if r.stopCommandEndToken != "" && command != r.stopCommandEndToken { if r.stopCommandEndToken != "" && command != r.stopCommandEndToken {
return &originalContent return &originalContent
} }
@@ -454,7 +454,7 @@ func (r *Reporter) parseLogRow(entry *log.Entry) *runnerv1.LogRow {
matches := cmdRegex.FindStringSubmatch(content) matches := cmdRegex.FindStringSubmatch(content)
if matches != nil { if matches != nil {
if output := r.handleCommand(content, matches[1], matches[2], matches[3]); output != nil { if output := r.handleCommand(content, matches[1], matches[3]); output != nil {
content = *output content = *output
} else { } else {
return nil return nil

View File

@@ -5,7 +5,7 @@ package report
import ( import (
"context" "context"
"fmt" "errors"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -173,30 +173,30 @@ func TestReporter_Fire(t *testing.T) {
return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
}) })
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
taskCtx, err := structpb.NewStruct(map[string]interface{}{}) taskCtx, err := structpb.NewStruct(map[string]any{})
require.NoError(t, err) require.NoError(t, err)
reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{ reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{
Context: taskCtx, Context: taskCtx,
}) })
reporter.RunDaemon() reporter.RunDaemon()
defer func() { defer func() {
assert.NoError(t, reporter.Close("")) require.NoError(t, reporter.Close(""))
}() }()
reporter.ResetSteps(5) reporter.ResetSteps(5)
dataStep0 := map[string]interface{}{ dataStep0 := map[string]any{
"stage": "Main", "stage": "Main",
"stepNumber": 0, "stepNumber": 0,
"raw_output": true, "raw_output": true,
} }
assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0})) require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0})) require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
assert.NoError(t, reporter.Fire(&log.Entry{Message: "composite step result", Data: map[string]interface{}{ require.NoError(t, reporter.Fire(&log.Entry{Message: "composite step result", Data: map[string]any{
"stage": "Main", "stage": "Main",
"stepID": []string{"0", "0"}, "stepID": []string{"0", "0"},
"stepNumber": 0, "stepNumber": 0,
@@ -204,7 +204,7 @@ func TestReporter_Fire(t *testing.T) {
"stepResult": "failure", "stepResult": "failure",
}})) }}))
assert.Equal(t, runnerv1.Result_RESULT_UNSPECIFIED, reporter.state.Steps[0].Result) assert.Equal(t, runnerv1.Result_RESULT_UNSPECIFIED, reporter.state.Steps[0].Result)
assert.NoError(t, reporter.Fire(&log.Entry{Message: "step result", Data: map[string]interface{}{ require.NoError(t, reporter.Fire(&log.Entry{Message: "step result", Data: map[string]any{
"stage": "Main", "stage": "Main",
"stepNumber": 0, "stepNumber": 0,
"raw_output": true, "raw_output": true,
@@ -231,7 +231,7 @@ func TestReporter_EphemeralRunnerDeletion(t *testing.T) {
client.On("UpdateLog", mock.Anything, mock.Anything).Return( client.On("UpdateLog", mock.Anything, mock.Anything).Return(
func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) { func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) {
if runnerDeleted { if runnerDeleted {
return nil, fmt.Errorf("runner has been deleted") return nil, errors.New("runner has been deleted")
} }
return connect_go.NewResponse(&runnerv1.UpdateLogResponse{ return connect_go.NewResponse(&runnerv1.UpdateLogResponse{
AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)), AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)),
@@ -250,19 +250,19 @@ func TestReporter_EphemeralRunnerDeletion(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
taskCtx, err := structpb.NewStruct(map[string]interface{}{}) taskCtx, err := structpb.NewStruct(map[string]any{})
require.NoError(t, err) require.NoError(t, err)
reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{Context: taskCtx}) reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{Context: taskCtx})
reporter.ResetSteps(1) reporter.ResetSteps(1)
// Fire a log entry to create pending data // Fire a log entry to create pending data
assert.NoError(t, reporter.Fire(&log.Entry{ require.NoError(t, reporter.Fire(&log.Entry{
Message: "build output", Message: "build output",
Data: log.Fields{"stage": "Main", "stepNumber": 0, "raw_output": true}, Data: log.Fields{"stage": "Main", "stepNumber": 0, "raw_output": true},
})) }))
// Step 1: RunDaemon calls ReportLog(false) — runner is still alive // Step 1: RunDaemon calls ReportLog(false) — runner is still alive
assert.NoError(t, reporter.ReportLog(false)) require.NoError(t, reporter.ReportLog(false))
// Step 2: Close() updates state — sets Result=FAILURE and marks steps cancelled. // Step 2: Close() updates state — sets Result=FAILURE and marks steps cancelled.
// In the real race, this happens while RunDaemon is between ReportLog and ReportState. // In the real race, this happens while RunDaemon is between ReportLog and ReportState.
@@ -283,18 +283,18 @@ func TestReporter_EphemeralRunnerDeletion(t *testing.T) {
// Step 3: RunDaemon's ReportState() — with the fix, this returns early // Step 3: RunDaemon's ReportState() — with the fix, this returns early
// because closed=true, preventing the server from deleting the runner. // because closed=true, preventing the server from deleting the runner.
assert.NoError(t, reporter.ReportState(false)) require.NoError(t, reporter.ReportState(false))
assert.False(t, runnerDeleted, "runner must not be deleted by RunDaemon's ReportState") assert.False(t, runnerDeleted, "runner must not be deleted by RunDaemon's ReportState")
// Step 4: Close's final log upload succeeds because the runner is still alive. // Step 4: Close's final log upload succeeds because the runner is still alive.
// Flush pending rows first, then send the noMore signal (matching Close's retry behavior). // Flush pending rows first, then send the noMore signal (matching Close's retry behavior).
assert.NoError(t, reporter.ReportLog(false)) require.NoError(t, reporter.ReportLog(false))
// Acknowledge Close as done in daemon // Acknowledge Close as done in daemon
close(reporter.daemon) close(reporter.daemon)
err = reporter.ReportLog(true) err = reporter.ReportLog(true)
assert.NoError(t, err, "final log upload must not fail: runner should not be deleted before Close finishes sending logs") require.NoError(t, err, "final log upload must not fail: runner should not be deleted before Close finishes sending logs")
err = reporter.ReportState(true) err = reporter.ReportState(true)
assert.NoError(t, err, "final state update should work: runner should not be deleted before Close finishes sending logs") require.NoError(t, err, "final state update should work: runner should not be deleted before Close finishes sending logs")
} }
func TestReporter_RunDaemonClose_Race(t *testing.T) { func TestReporter_RunDaemonClose_Race(t *testing.T) {
@@ -313,7 +313,7 @@ func TestReporter_RunDaemonClose_Race(t *testing.T) {
) )
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
taskCtx, err := structpb.NewStruct(map[string]interface{}{}) taskCtx, err := structpb.NewStruct(map[string]any{})
require.NoError(t, err) require.NoError(t, err)
reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{ reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{
Context: taskCtx, Context: taskCtx,
@@ -323,14 +323,12 @@ func TestReporter_RunDaemonClose_Race(t *testing.T) {
// Start the daemon loop in a separate goroutine. // Start the daemon loop in a separate goroutine.
// RunDaemon reads r.closed and reschedules itself via time.AfterFunc. // RunDaemon reads r.closed and reschedules itself via time.AfterFunc.
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Go(func() {
go func() {
defer wg.Done()
reporter.RunDaemon() reporter.RunDaemon()
}() })
// Close concurrently — this races with RunDaemon on r.closed. // Close concurrently — this races with RunDaemon on r.closed.
assert.NoError(t, reporter.Close("")) require.NoError(t, reporter.Close(""))
// Cancel context so pending AfterFunc callbacks exit quickly. // Cancel context so pending AfterFunc callbacks exit quickly.
cancel() cancel()