Преглед изворни кода

chore: modernize old go bits and pieces

jamesread пре 1 дан
родитељ
комит
3ad42630cc

+ 6 - 0
renovate.json

@@ -0,0 +1,6 @@
+{
+  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
+  "extends": ["config:recommended", ":semanticCommits", "group:allNonMajor"],
+  "schedule": ["before 6am on monday"],
+  "postUpdateOptions": ["gomodTidy"]
+}

+ 3 - 3
service/.golangci.yml

@@ -28,15 +28,15 @@ linters:
     - govet
     - ineffassign
     - misspell
-#    - modernize
+    - modernize
     - nilerr
     - noctx
 #    - promlinter
     - staticcheck
-#    - testifylint
+    - testifylint
     - thelper
     - unconvert
-#    - unparam
+    - unparam
     - unused
     - usestdlibvars
   settings:

+ 3 - 2
service/internal/api/api.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -806,8 +807,8 @@ func (api *oliveTinAPI) buildActionLogsResponse(filtered []*executor.InternalLog
 	startIdx, endIdx := calculateReversedIndices(page, len(filtered))
 	ret := &apiv1.GetActionLogsResponse{}
 	chunk := filtered[int(startIdx):int(endIdx)]
-	for i := len(chunk) - 1; i >= 0; i-- {
-		ret.Logs = append(ret.Logs, api.internalLogEntryToPb(chunk[i], user))
+	for _, c := range slices.Backward(chunk) {
+		ret.Logs = append(ret.Logs, api.internalLogEntryToPb(c, user))
 	}
 	ret.CountRemaining = page.start
 	ret.PageSize = page.size

+ 1 - 4
service/internal/api/api_entities_list.go

@@ -156,10 +156,7 @@ func paginateEntityInstances(instances []*apiv1.Entity, page, pageSize int32) []
 		return []*apiv1.Entity{}
 	}
 
-	end := start + int64(pageSize)
-	if end > count {
-		end = count
-	}
+	end := min(start+int64(pageSize), count)
 
 	return instances[int(start):int(end)]
 }

+ 12 - 8
service/internal/api/api_entity_related_actions_test.go

@@ -92,9 +92,11 @@ func buildRelatedActionsTestConfig(t *testing.T) (*config.Config, *authpublic.Au
 	return cfg, lowUser, adminUser
 }
 
-func getEntityRelatedActionTitles(t *testing.T, api *oliveTinAPI, user *authpublic.AuthenticatedUser, entityType, entityKey string) []string {
+func getEntityRelatedActionTitles(t *testing.T, api *oliveTinAPI, user *authpublic.AuthenticatedUser, entityKey string) []string {
 	t.Helper()
 
+	entityType := "host"
+
 	entity, ok := entities.GetEntityInstances(entityType)[entityKey]
 	require.True(t, ok, "entity %s/%s must exist", entityType, entityKey)
 
@@ -108,9 +110,11 @@ func getEntityRelatedActionTitles(t *testing.T, api *oliveTinAPI, user *authpubl
 	return titles
 }
 
-func getEntityRelatedBindingIDs(t *testing.T, api *oliveTinAPI, user *authpublic.AuthenticatedUser, entityType, entityKey string) []string {
+func getEntityRelatedBindingIDs(t *testing.T, api *oliveTinAPI, user *authpublic.AuthenticatedUser, entityKey string) []string {
 	t.Helper()
 
+	entityType := "host"
+
 	entity, ok := entities.GetEntityInstances(entityType)[entityKey]
 	require.True(t, ok, "entity %s/%s must exist", entityType, entityKey)
 
@@ -132,10 +136,10 @@ func TestGetEntityRelatedActionsDeniesRestrictedView(t *testing.T) {
 	ex.RebuildActionMap()
 	api := newServer(ex)
 
-	ids := getEntityRelatedBindingIDs(t, api, lowUser, "host", "0")
+	ids := getEntityRelatedBindingIDs(t, api, lowUser, "0")
 	assert.Empty(t, ids)
 
-	titles := getEntityRelatedActionTitles(t, api, lowUser, "host", "0")
+	titles := getEntityRelatedActionTitles(t, api, lowUser, "0")
 	assert.Empty(t, titles)
 }
 
@@ -147,7 +151,7 @@ func TestGetEntityRelatedActionsAllowsAdminView(t *testing.T) {
 	ex.RebuildActionMap()
 	api := newServer(ex)
 
-	titles := getEntityRelatedActionTitles(t, api, adminUser, "host", "0")
+	titles := getEntityRelatedActionTitles(t, api, adminUser, "0")
 	assert.Contains(t, titles, "Run Automation Playbook")
 	assert.Contains(t, titles, "Public Host Action")
 	assert.Contains(t, titles, "Secret Entity Action")
@@ -161,7 +165,7 @@ func TestGetEntityRelatedActionsExcludesHiddenActions(t *testing.T) {
 	ex.RebuildActionMap()
 	api := newServer(ex)
 
-	titles := getEntityRelatedActionTitles(t, api, adminUser, "host", "0")
+	titles := getEntityRelatedActionTitles(t, api, adminUser, "0")
 	assert.NotContains(t, titles, "Hidden Host Action")
 }
 
@@ -179,11 +183,11 @@ func TestGetEntityRelatedActionsEntityBoundMatchesInstanceOnly(t *testing.T) {
 	api := newServer(ex)
 	user := &authpublic.AuthenticatedUser{Username: "guest", Provider: "system"}
 
-	titlesHost0 := getEntityRelatedActionTitles(t, api, user, "host", "0")
+	titlesHost0 := getEntityRelatedActionTitles(t, api, user, "0")
 	assert.Len(t, titlesHost0, 1)
 	assert.Equal(t, "stuffbox Wake", titlesHost0[0])
 
-	titlesHost1 := getEntityRelatedActionTitles(t, api, user, "host", "1")
+	titlesHost1 := getEntityRelatedActionTitles(t, api, user, "1")
 	assert.Len(t, titlesHost1, 1)
 	assert.Equal(t, "lurker Wake", titlesHost1[0])
 }

+ 3 - 3
service/internal/api/api_init_search_hints_test.go

@@ -190,7 +190,7 @@ func TestBuildSearchHintsCapsActionsAndEntitiesPerType(t *testing.T) {
 		entities.ClearEntitiesOfType("cap_container")
 	})
 
-	for entityIndex := 0; entityIndex < maxSearchHintEntitiesPerType+5; entityIndex++ {
+	for entityIndex := range maxSearchHintEntitiesPerType + 5 {
 		entities.AddEntity("cap_host", fmt.Sprintf("%03d", entityIndex), map[string]any{"name": fmt.Sprintf("host-%03d", entityIndex)})
 		entities.AddEntity("cap_container", fmt.Sprintf("%03d", entityIndex), map[string]any{"name": fmt.Sprintf("ctr-%03d", entityIndex)})
 	}
@@ -201,7 +201,7 @@ func TestBuildSearchHintsCapsActionsAndEntitiesPerType(t *testing.T) {
 		{Name: "cap_container", File: "cap_container.yaml"},
 	}
 	cfg.Actions = make([]*config.Action, 0, maxSearchHintActions+5)
-	for actionIndex := 0; actionIndex < maxSearchHintActions+5; actionIndex++ {
+	for actionIndex := range maxSearchHintActions + 5 {
 		cfg.Actions = append(cfg.Actions, &config.Action{
 			ID:    fmt.Sprintf("action-%03d", actionIndex),
 			Title: fmt.Sprintf("Action %03d", actionIndex),
@@ -240,7 +240,7 @@ func TestBuildSearchHintsPrefersNonEntityActions(t *testing.T) {
 		entities.ClearEntitiesOfType("host")
 	})
 
-	for i := 0; i < 10; i++ {
+	for i := range 10 {
 		entities.AddEntity("host", fmt.Sprintf("%d", i), map[string]any{"name": fmt.Sprintf("host-%d", i)})
 	}
 

+ 6 - 6
service/internal/api/api_log_arguments_test.go

@@ -14,10 +14,10 @@ import (
 	"github.com/OliveTin/OliveTin/internal/executor"
 )
 
-func argumentAction(title, shell string, args []config.ActionArgument) *config.Action {
+func argumentAction(title string, args []config.ActionArgument) *config.Action {
 	return &config.Action{
 		Title:         title,
-		Shell:         shell,
+		Shell:         "echo {{ host }}",
 		MaxConcurrent: 1,
 		Arguments:     args,
 	}
@@ -75,7 +75,7 @@ func waitForLogJustification(t *testing.T, ex *executor.Executor, trackingID, ex
 func TestExecutionStatusIncludesStoredArguments(t *testing.T) {
 	cfg := config.DefaultConfig()
 	cfg.Actions = []*config.Action{
-		argumentAction("Ping host with stored args", "echo {{ host }}", []config.ActionArgument{
+		argumentAction("Ping host with stored args", []config.ActionArgument{
 			{Name: "host", Type: "ascii_identifier"},
 		}),
 	}
@@ -159,7 +159,7 @@ func TestExecutionStatusOmitsPasswordArguments(t *testing.T) {
 func TestRestartActionReusesStoredArguments(t *testing.T) {
 	cfg := config.DefaultConfig()
 	cfg.Actions = []*config.Action{
-		argumentAction("Ping host", "echo {{ host }}", []config.ActionArgument{
+		argumentAction("Ping host", []config.ActionArgument{
 			{Name: "host", Type: "ascii_identifier"},
 		}),
 	}
@@ -236,7 +236,7 @@ func TestRestartActionRejectsIncompleteStoredArguments(t *testing.T) {
 func TestRestartActionRejectsMissingRequiredStoredArguments(t *testing.T) {
 	cfg := config.DefaultConfig()
 	cfg.Actions = []*config.Action{
-		argumentAction("Ping host - reject missing required stored arg", "echo {{ host }}", []config.ActionArgument{
+		argumentAction("Ping host - reject missing required stored arg", []config.ActionArgument{
 			{Name: "host", Type: "ascii_identifier"},
 		}),
 	}
@@ -368,7 +368,7 @@ func TestRestartActionReusesStoredJustificationViaStartActionPath(t *testing.T)
 func TestGetLogsIncludesStoredArguments(t *testing.T) {
 	cfg := config.DefaultConfig()
 	cfg.Actions = []*config.Action{
-		argumentAction("Ping host", "echo {{ host }}", []config.ActionArgument{
+		argumentAction("Ping host", []config.ActionArgument{
 			{Name: "host", Type: "ascii_identifier"},
 		}),
 	}

+ 2 - 2
service/internal/api/api_test.go

@@ -864,7 +864,7 @@ func TestRegisterStreamingClientEnforcesLimit(t *testing.T) {
 	user := &authpublic.AuthenticatedUser{Username: "limit-test"}
 
 	clients := make([]*streamingClient, 0, maxEventStreamClients)
-	for i := 0; i < maxEventStreamClients; i++ {
+	for range maxEventStreamClients {
 		client := &streamingClient{
 			channel:           make(chan *apiv1.EventStreamResponse, 1),
 			AuthenticatedUser: user,
@@ -912,7 +912,7 @@ func TestEventStreamReleasesSlotWhenClientDisconnects(t *testing.T) {
 	server := ensureExecutorListener(ex)
 
 	cancels := make([]context.CancelFunc, 0, maxEventStreamClients)
-	for i := 0; i < maxEventStreamClients; i++ {
+	for range maxEventStreamClients {
 		cancels = append(cancels, openEventStreamAndReadHeartbeat(t, client))
 	}
 	require.Equal(t, maxEventStreamClients, countStreamingClients(server))

+ 1 - 1
service/internal/api/dashboard_acl_test.go

@@ -100,7 +100,7 @@ func TestRootDashboardEntriesIncludeCategory(t *testing.T) {
 	entries := api.buildRootDashboardEntries(user, cfg.Dashboards)
 	require.Len(t, entries, 4)
 	assert.Equal(t, []string{"Misc Tools", "My Servers", "Status Board", "My Containers"}, rootDashboardTitles(entries))
-	assert.Equal(t, "", entries[0].Category)
+	assert.Empty(t, entries[0].Category)
 	assert.Equal(t, "Infrastructure", entries[1].Category)
 	assert.Equal(t, "Monitoring", entries[2].Category)
 	assert.Equal(t, "Infrastructure", entries[3].Category)

+ 2 - 2
service/internal/api/dashboards_race_test.go

@@ -28,14 +28,14 @@ func TestBuildDefaultDashboardConcurrentWithRebuild(t *testing.T) {
 
 	go func() {
 		defer wg.Done()
-		for i := 0; i < 200; i++ {
+		for range 200 {
 			ex.RebuildActionMap()
 		}
 	}()
 
 	go func() {
 		defer wg.Done()
-		for i := 0; i < 200; i++ {
+		for range 200 {
 			buildDefaultDashboard(rr)
 		}
 	}()

+ 1 - 1
service/internal/auth/authpublic/authenticateduser.go

@@ -26,7 +26,7 @@ func (u *AuthenticatedUser) parseUsergroupLine(sep string) []string {
 	ret := []string{}
 
 	if sep != "" {
-		for _, v := range strings.Split(u.UsergroupLine, sep) {
+		for v := range strings.SplitSeq(u.UsergroupLine, sep) {
 			trimmed := strings.TrimSpace(v)
 
 			if trimmed != "" {

+ 6 - 6
service/internal/auth/otoauth2/restapi_auth_oauth2_test.go

@@ -43,13 +43,13 @@ func TestGetGroupFieldString(t *testing.T) {
 func TestGetGroupFieldMissing(t *testing.T) {
 	data := map[string]any{}
 
-	assert.Equal(t, "", getGroupField(data, "olivetin_group", ""))
+	assert.Empty(t, getGroupField(data, "olivetin_group", ""))
 }
 
 func TestGetGroupFieldEmptyFieldName(t *testing.T) {
 	data := map[string]any{"olivetin_group": "admins"}
 
-	assert.Equal(t, "", getGroupField(data, "", ""))
+	assert.Empty(t, getGroupField(data, "", ""))
 }
 
 func TestGetGroupFieldArrayDefaultSeparator(t *testing.T) {
@@ -73,13 +73,13 @@ func TestGetGroupFieldArraySkipsNonStringElements(t *testing.T) {
 func TestGetGroupFieldArrayAllNonStringElements(t *testing.T) {
 	data := map[string]any{"groups": []any{float64(1), true}}
 
-	assert.Equal(t, "", getGroupField(data, "groups", ""))
+	assert.Empty(t, getGroupField(data, "groups", ""))
 }
 
 func TestGetGroupFieldNotStringOrArray(t *testing.T) {
 	data := map[string]any{"groups": map[string]any{"nested": "value"}}
 
-	assert.Equal(t, "", getGroupField(data, "groups", ""))
+	assert.Empty(t, getGroupField(data, "groups", ""))
 }
 
 func TestGetUserInfoWithArrayGroupsClaim(t *testing.T) {
@@ -141,7 +141,7 @@ func TestHandleOAuthLoginRejectsWhenStateMapFull(t *testing.T) {
 
 	h := NewOAuth2Handler(cfg)
 	h.registeredStates = make(map[string]*oauth2State, oauthStateMaxEntries)
-	for i := 0; i < oauthStateMaxEntries; i++ {
+	for i := range oauthStateMaxEntries {
 		h.registeredStates[strconv.Itoa(i)] = &oauth2State{
 			providerConfig: &oauth2.Config{},
 			providerName:   "test",
@@ -155,5 +155,5 @@ func TestHandleOAuthLoginRejectsWhenStateMapFull(t *testing.T) {
 	h.HandleOAuthLogin(rec, req)
 
 	assert.Equal(t, http.StatusServiceUnavailable, rec.Code)
-	assert.Equal(t, oauthStateMaxEntries, len(h.registeredStates))
+	assert.Len(t, h.registeredStates, oauthStateMaxEntries)
 }

+ 8 - 8
service/internal/config/config_reloader_user_test.go

@@ -42,7 +42,7 @@ actions:
 	// Load config using koanf
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
-	assert.NoError(t, err, "Should load config file")
+	require.NoError(t, err, "Should load config file")
 
 	// Create config struct and load it
 	cfg := &Config{}
@@ -50,7 +50,7 @@ actions:
 
 	// Test that authLocalUsers was loaded correctly
 	assert.True(t, cfg.AuthLocalUsers.Enabled, "AuthLocalUsers should be enabled")
-	assert.Equal(t, 2, len(cfg.AuthLocalUsers.Users), "Should load 2 users")
+	assert.Len(t, cfg.AuthLocalUsers.Users, 2, "Should load 2 users")
 
 	// Test individual users
 	user1 := cfg.FindUserByUsername("testuser1")
@@ -91,13 +91,13 @@ actions:
 
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
-	assert.NoError(t, err, "Should load config file")
+	require.NoError(t, err, "Should load config file")
 
 	cfg := &Config{}
 	AppendSource(cfg, k, tmpFile.Name())
 
 	assert.True(t, cfg.AuthLocalUsers.Enabled, "AuthLocalUsers should be enabled")
-	assert.Equal(t, 0, len(cfg.AuthLocalUsers.Users), "Should have 0 users")
+	assert.Empty(t, cfg.AuthLocalUsers.Users, "Should have 0 users")
 	assert.Nil(t, cfg.FindUserByUsername("anyuser"), "Should return nil for any user")
 }
 
@@ -126,13 +126,13 @@ actions:
 
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
-	assert.NoError(t, err, "Should load config file")
+	require.NoError(t, err, "Should load config file")
 
 	cfg := &Config{}
 	AppendSource(cfg, k, tmpFile.Name())
 
 	assert.False(t, cfg.AuthLocalUsers.Enabled, "AuthLocalUsers should be disabled")
-	assert.Equal(t, 1, len(cfg.AuthLocalUsers.Users), "Should still load users even when disabled")
+	assert.Len(t, cfg.AuthLocalUsers.Users, 1, "Should still load users even when disabled")
 
 	// User should still be findable even when auth is disabled
 	user := cfg.FindUserByUsername("testuser")
@@ -157,13 +157,13 @@ actions:
 
 	k := koanf.New(".")
 	err = k.Load(file.Provider(tmpFile.Name()), yaml.Parser())
-	assert.NoError(t, err, "Should load config file")
+	require.NoError(t, err, "Should load config file")
 
 	cfg := &Config{}
 	AppendSource(cfg, k, tmpFile.Name())
 
 	// Should have default values
 	assert.False(t, cfg.AuthLocalUsers.Enabled, "AuthLocalUsers should be disabled by default")
-	assert.Equal(t, 0, len(cfg.AuthLocalUsers.Users), "Should have 0 users by default")
+	assert.Empty(t, cfg.AuthLocalUsers.Users, "Should have 0 users by default")
 	assert.Nil(t, cfg.FindUserByUsername("anyuser"), "Should return nil for any user")
 }

+ 4 - 3
service/internal/config/entity_argument_choices_test.go

@@ -4,10 +4,11 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestValidateEntityArgumentChoicesForArgument(t *testing.T) {
-	assert.NoError(t, validateEntityArgumentChoicesForArgument("Reboot", ActionArgument{
+	require.NoError(t, validateEntityArgumentChoicesForArgument("Reboot", ActionArgument{
 		Name:   "target",
 		Entity: "servers",
 		Choices: []ActionArgumentChoice{
@@ -15,7 +16,7 @@ func TestValidateEntityArgumentChoicesForArgument(t *testing.T) {
 		},
 	}))
 
-	assert.Error(t, validateEntityArgumentChoicesForArgument("Reboot", ActionArgument{
+	require.Error(t, validateEntityArgumentChoicesForArgument("Reboot", ActionArgument{
 		Name:   "target",
 		Entity: "servers",
 		Choices: []ActionArgumentChoice{
@@ -24,7 +25,7 @@ func TestValidateEntityArgumentChoicesForArgument(t *testing.T) {
 		},
 	}))
 
-	assert.Error(t, validateEntityArgumentChoicesForArgument("Reboot", ActionArgument{
+	require.Error(t, validateEntityArgumentChoicesForArgument("Reboot", ActionArgument{
 		Name:    "target",
 		Entity:  "servers",
 		Choices: nil,

+ 4 - 4
service/internal/config/loaded_sources.go

@@ -1,5 +1,7 @@
 package config
 
+import "slices"
+
 import "sync"
 
 var (
@@ -23,10 +25,8 @@ func RecordLoadedSource(path string) {
 	loadedSourcesMu.Lock()
 	defer loadedSourcesMu.Unlock()
 
-	for _, existing := range loadedSources {
-		if existing == path {
-			return
-		}
+	if slices.Contains(loadedSources, path) {
+		return
 	}
 
 	loadedSources = append(loadedSources, path)

+ 2 - 2
service/internal/entities/entities_test.go

@@ -58,8 +58,8 @@ func TestDisplayNameFieldKey(t *testing.T) {
 	assert.Equal(t, "title", DisplayNameFieldKey(map[string]any{"title": "Car"}))
 	assert.Equal(t, "name", DisplayNameFieldKey(map[string]any{"name": "Car"}))
 	assert.Equal(t, "Name", DisplayNameFieldKey(map[string]any{"Name": "Car"}))
-	assert.Equal(t, "", DisplayNameFieldKey(map[string]any{"status": "running"}))
-	assert.Equal(t, "", DisplayNameFieldKey("not a map"))
+	assert.Empty(t, DisplayNameFieldKey(map[string]any{"status": "running"}))
+	assert.Empty(t, DisplayNameFieldKey("not a map"))
 }
 
 func TestDisplayNameFieldKey_caseCollisionIsDeterministic(t *testing.T) {

+ 3 - 3
service/internal/executor/executor.go

@@ -1313,8 +1313,8 @@ func substituteShellAfterCompletedEnvRefs(command string) string {
 
 func replaceShellAfterEnvRef(command string, pattern *regexp.Regexp, envRef string) string {
 	matches := pattern.FindAllStringIndex(command, -1)
-	for i := len(matches) - 1; i >= 0; i-- {
-		start, end := matches[i][0], matches[i][1]
+	for _, match := range slices.Backward(matches) {
+		start, end := match[0], match[1]
 		replacement := `"` + envRef + `"`
 		if shellPosInsideSingleQuotes(command, start) {
 			// Break out of single quotes so the env ref can expand at runtime.
@@ -1528,7 +1528,7 @@ func sanitizeLogFilename(title string) string {
 	}
 
 	// NUL and other C0 controls plus DEL are invalid or problematic in filenames.
-	for i := 0; i < 32; i++ {
+	for i := range 32 {
 		oldnew = append(oldnew, string(rune(i)), "_")
 	}
 	oldnew = append(oldnew, "\x7f", "_")

+ 34 - 34
service/internal/executor/executor_test.go

@@ -158,7 +158,7 @@ func TestArgumentNameCamelCase(t *testing.T) {
 	out, err := parseActionArguments(req)
 
 	assert.Equal(t, "echo 'Tickling Fred'", out)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 }
 
 func TestArgumentNameSnakeCase(t *testing.T) {
@@ -181,7 +181,7 @@ func TestArgumentNameSnakeCase(t *testing.T) {
 	out, err := parseActionArguments(req)
 
 	assert.Equal(t, "echo 'Tickling Fred'", out)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 }
 
 func TestGetLogsEmpty(t *testing.T) {
@@ -192,7 +192,7 @@ func TestGetLogsEmpty(t *testing.T) {
 	logs, paging := e.GetLogTrackingIds(0, 10)
 
 	assert.NotNil(t, logs, "Logs should not be nil")
-	assert.Equal(t, 0, len(logs), "No logs yet")
+	assert.Empty(t, logs, "No logs yet")
 	assert.Equal(t, int64(0), paging.CountRemaining, "There should be no remaining logs")
 }
 
@@ -212,35 +212,35 @@ func TestGetLogsLessThanPageSize(t *testing.T) {
 
 	logEntries, paging := e.GetLogTrackingIds(0, 10)
 
-	assert.Equal(t, 0, len(logEntries), "There should be 0 logs")
+	assert.Empty(t, logEntries, "There should be 0 logs")
 	assert.Zero(t, paging.CountRemaining, "There should be no remaining logs")
 
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
 
 	logEntries, paging = e.GetLogTrackingIds(0, 10)
 
-	assert.Equal(t, 7, len(logEntries), "There should be 7 logs")
+	assert.Len(t, logEntries, 7, "There should be 7 logs")
 	assert.Zero(t, paging.CountRemaining, "There should be no remaining logs")
 
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
-	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
+	execNewReqAndWait(e, cfg)
 
 	logEntries, paging = e.GetLogTrackingIds(0, 10)
 
-	assert.Equal(t, 10, len(logEntries), "There should be 10 logs")
+	assert.Len(t, logEntries, 10, "There should be 10 logs")
 	assert.Equal(t, int64(2), paging.CountRemaining, "There should be 1 remaining logs")
 }
 
-func execNewReqAndWait(e *Executor, title string, cfg *config.Config) {
+func execNewReqAndWait(e *Executor, cfg *config.Config) {
 	req := &ExecutionRequest{
 		//		ActionTitle: title,
 		Cfg: cfg,
@@ -250,7 +250,7 @@ func execNewReqAndWait(e *Executor, title string, cfg *config.Config) {
 	e.RebuildActionMap()
 	var action *config.Action
 	for _, a := range cfg.Actions {
-		if a.Title == title {
+		if a.Title == "blat" {
 			action = a
 			break
 		}
@@ -288,8 +288,8 @@ func TestUnsetRequiredArgument(t *testing.T) {
 
 	out, err := parseActionArguments(req)
 
-	assert.Equal(t, "", out)
-	assert.NotNil(t, err)
+	assert.Empty(t, out)
+	assert.Error(t, err)
 }
 
 func TestUnusedArgumentStillPassesTypeSafetyCheck(t *testing.T) {
@@ -316,8 +316,8 @@ func TestUnusedArgumentStillPassesTypeSafetyCheck(t *testing.T) {
 
 	out, err := parseActionArguments(req)
 
-	assert.Equal(t, "", out)
-	assert.NotNil(t, err)
+	assert.Empty(t, out)
+	assert.Error(t, err)
 }
 
 // https://github.com/OliveTin/OliveTin/issues/564
@@ -357,7 +357,7 @@ func TestMangleInvalidArgumentValues(t *testing.T) {
 	wg.Wait()
 
 	assert.NotNil(t, req.logEntry, "Log entry should not be nil")
-	assert.Equal(t, req.logEntry.Output, "The date is: 1990-01-10T12:00:00\n", "Date should be mangled to a valid format")
+	assert.Equal(t, "The date is: 1990-01-10T12:00:00\n", req.logEntry.Output, "Date should be mangled to a valid format")
 
 }
 
@@ -726,7 +726,7 @@ func TestValidatedSystemArgsMatchesSystemArgumentDefinitions(t *testing.T) {
 
 	args, err := validatedSystemArgs(req)
 
-	assert.Nil(t, err)
+	require.NoError(t, err)
 	assert.Len(t, args, len(systemArgumentDefinitions))
 	for _, arg := range systemArgumentDefinitions {
 		assert.Contains(t, args, arg.Name)
@@ -745,7 +745,7 @@ func TestBuildShellAfterArgsOnlyAddsExpectedNonSystemArgs(t *testing.T) {
 
 	args, err := buildShellAfterArgs(req)
 
-	assert.Nil(t, err)
+	require.NoError(t, err)
 	assert.Len(t, args, len(systemArgumentDefinitions)+2)
 	assert.Contains(t, args, "output")
 	assert.Contains(t, args, "exitCode")
@@ -816,7 +816,7 @@ func TestBuildShellAfterArgsUsesValidatedSystemArgs(t *testing.T) {
 
 	args, err := buildShellAfterArgs(req)
 
-	assert.Nil(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "alice@example.com", args["ot_username"])
 	assert.Equal(t, "server-track-456", args["ot_executionTrackingId"])
 	assert.Equal(t, "hello", args["output"])
@@ -836,7 +836,7 @@ func TestBuildShellAfterArgsFailsWhenSystemArgIsInvalid(t *testing.T) {
 	args, err := buildShellAfterArgs(req)
 
 	assert.Nil(t, args)
-	assert.NotNil(t, err)
+	require.Error(t, err)
 	assert.Contains(t, err.Error(), `system argument "ot_username" failed validation`)
 }
 
@@ -879,7 +879,7 @@ func TestTriggerExecutesTriggeredAction(t *testing.T) {
 	wg.Wait()
 
 	var got []string
-	for i := 0; i < 2; i++ {
+	for i := range 2 {
 		select {
 		case title := <-finishedTitles:
 			got = append(got, title)
@@ -995,15 +995,15 @@ func TestStepSaveLogSanitizesSlashInTitle(t *testing.T) {
 	assert.FileExists(t, outputPath)
 
 	resultsEntries, err := os.ReadDir(resultsDir)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Len(t, resultsEntries, 1, "results file must be flat under resultsDirectory, not a subdirectory")
 
 	data, err := os.ReadFile(resultsPath)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Contains(t, string(data), title, "YAML content keeps the original action title")
 
 	output, err := os.ReadFile(outputPath)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "report ok", string(output))
 }
 
@@ -1071,7 +1071,7 @@ func TestStepSaveLogSanitizesNULInTitle(t *testing.T) {
 	assert.NotContains(t, outputPath, "\x00")
 
 	output, err := os.ReadFile(outputPath)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "nul ok", string(output))
 }
 

+ 1 - 1
service/internal/executor/group_concurrency_test.go

@@ -359,7 +359,7 @@ func TestGroupQueuesThirdAndBlocksWhenQueueFull(t *testing.T) {
 	trackings := []string{tracking1, tracking2}
 	waitGroups := []*sync.WaitGroup{wg1, wg2}
 
-	for idx := 0; idx < 3; idx++ {
+	for range 3 {
 		wg, tracking := e.ExecRequest(&ExecutionRequest{
 			Binding:           binding,
 			Cfg:               cfg,

+ 6 - 6
service/internal/executor/loadlogs.go

@@ -18,12 +18,12 @@ func (e *Executor) LoadLogsFromDisk() {
 		return
 	}
 
-	entries, skippedCount := e.readLogDirectory(resultsDir)
+	entries := e.readLogDirectory(resultsDir)
 	if entries == nil {
 		return
 	}
 
-	loadedLogs, skippedCount := e.parseLogFiles(resultsDir, entries, skippedCount)
+	loadedLogs, skippedCount := e.parseLogFiles(resultsDir, entries, 0)
 
 	sort.Slice(loadedLogs, func(i, j int) bool {
 		return loadedLogs[i].DatetimeStarted.Before(loadedLogs[j].DatetimeStarted)
@@ -37,12 +37,12 @@ func (e *Executor) LoadLogsFromDisk() {
 	}).Info("Finished loading persisted logs from disk")
 }
 
-func (e *Executor) readLogDirectory(resultsDir string) ([]os.DirEntry, int) {
+func (e *Executor) readLogDirectory(resultsDir string) []os.DirEntry {
 	if _, err := os.Stat(resultsDir); os.IsNotExist(err) {
 		log.WithFields(log.Fields{
 			"directory": resultsDir,
 		}).Debug("Logs directory does not exist, skipping log loading")
-		return nil, 0
+		return nil
 	}
 
 	log.WithFields(log.Fields{
@@ -55,10 +55,10 @@ func (e *Executor) readLogDirectory(resultsDir string) ([]os.DirEntry, int) {
 			"directory": resultsDir,
 			"error":     err,
 		}).Warnf("Failed to read logs directory")
-		return nil, 0
+		return nil
 	}
 
-	return entries, 0
+	return entries
 }
 
 func (e *Executor) parseLogFiles(resultsDir string, entries []os.DirEntry, skippedCount int) ([]*InternalLogEntry, int) {

+ 5 - 10
service/internal/logfilter/filter.go

@@ -38,12 +38,7 @@ func Compile(expression string) (*vm.Program, error) {
 		return nil, fmt.Errorf("filter expression exceeds maximum length of %d characters", maxFilterLength)
 	}
 
-	normalized, err := normalizeExpression(trimmed)
-	if err != nil {
-		return nil, err
-	}
-
-	return compileNormalized(normalized)
+	return compileNormalized(normalizeExpression(trimmed))
 }
 
 func compileNormalized(normalized string) (*vm.Program, error) {
@@ -114,19 +109,19 @@ func Matches(program *vm.Program, record Record) (bool, error) {
 	return matched, nil
 }
 
-func normalizeExpression(expression string) (string, error) {
+func normalizeExpression(expression string) string {
 	if isNegatedSearchTerm(expression) {
 		term := quoteLiteral(strings.TrimPrefix(expression, "!"))
-		return negatedSearchExpression(term), nil
+		return negatedSearchExpression(term)
 	}
 
 	if isPositiveSearchTerm(expression) {
-		return positiveSearchExpression(quoteLiteral(expression)), nil
+		return positiveSearchExpression(quoteLiteral(expression))
 	}
 
 	normalized := replaceContainsOperators(expression)
 	normalized = replaceComparisons(normalized)
-	return replaceBooleanWords(normalized), nil
+	return replaceBooleanWords(normalized)
 }
 
 func isNegatedSearchTerm(expression string) bool {

+ 2 - 2
service/internal/tpl/templates.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"regexp"
+	"slices"
 	"strings"
 	"text/template"
 
@@ -112,8 +113,7 @@ func migrateLegacyEntityProperties(rawShellCommand string) string {
 func migrateLegacyArgumentNames(rawShellCommand string) string {
 	matches := legacyArgumentRegex.FindAllStringSubmatchIndex(rawShellCommand, -1)
 
-	for i := len(matches) - 1; i >= 0; i-- {
-		match := matches[i]
+	for _, match := range slices.Backward(matches) {
 		fullMatchStart := match[0]
 		fullMatchEnd := match[1]
 		argNameStart := match[2]

+ 3 - 2
service/internal/tpl/templates_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/OliveTin/OliveTin/internal/entities"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestParseTemplateWithActionContext_Json(t *testing.T) {
@@ -60,7 +61,7 @@ func TestParseTemplateWithActionContext_Json(t *testing.T) {
 				assert.Error(t, err)
 				return
 			}
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			if tt.checkJsonOnly {
 				assertJsonOutput(t, output, tt.expectedOutput, tt.args)
 			} else {
@@ -78,7 +79,7 @@ func assertJsonOutput(t *testing.T, output, expectedPrefix string, args map[stri
 	jsonPart = strings.TrimSpace(jsonPart)
 	var decoded map[string]string
 	err := json.Unmarshal([]byte(jsonPart), &decoded)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	for k, v := range args {
 		assert.Equal(t, v, decoded[k], "decoded JSON should contain %s=%s", k, v)
 	}

+ 7 - 7
service/internal/webhooks/github.go

@@ -53,16 +53,16 @@ func setExtractIfMissing(cfg *config.WebhookConfig, key, value string) {
 	}
 }
 
-func setMatchHeaderIfMissing(cfg *config.WebhookConfig, key, value string) {
-	if _, exists := cfg.MatchHeaders[key]; !exists {
-		cfg.MatchHeaders[key] = value
+func setGitHubEventHeaderIfMissing(cfg *config.WebhookConfig, event string) {
+	if _, exists := cfg.MatchHeaders["X-GitHub-Event"]; !exists {
+		cfg.MatchHeaders["X-GitHub-Event"] = event
 	}
 }
 
 func applyGitHubPushTemplate(cfg *config.WebhookConfig) {
 	setDefaultAuth(cfg)
 	ensureMatchHeaders(cfg)
-	setMatchHeaderIfMissing(cfg, "X-GitHub-Event", "push")
+	setGitHubEventHeaderIfMissing(cfg, "push")
 	ensureExtract(cfg)
 	setExtractIfMissing(cfg, "git_repository", "$.repository.full_name")
 	setExtractIfMissing(cfg, "git_ref", "$.ref")
@@ -75,7 +75,7 @@ func applyGitHubPushTemplate(cfg *config.WebhookConfig) {
 func applyGitHubPRTemplate(cfg *config.WebhookConfig) {
 	setDefaultAuth(cfg)
 	ensureMatchHeaders(cfg)
-	setMatchHeaderIfMissing(cfg, "X-GitHub-Event", "pull_request")
+	setGitHubEventHeaderIfMissing(cfg, "pull_request")
 	ensureExtract(cfg)
 	setExtractIfMissing(cfg, "pr_number", "$.number")
 	setExtractIfMissing(cfg, "pr_title", "$.pull_request.title")
@@ -89,7 +89,7 @@ func applyGitHubPRTemplate(cfg *config.WebhookConfig) {
 func applyGitHubReleaseTemplate(cfg *config.WebhookConfig) {
 	setDefaultAuth(cfg)
 	ensureMatchHeaders(cfg)
-	setMatchHeaderIfMissing(cfg, "X-GitHub-Event", "release")
+	setGitHubEventHeaderIfMissing(cfg, "release")
 	ensureExtract(cfg)
 	setExtractIfMissing(cfg, "release_action", "$.action")
 	setExtractIfMissing(cfg, "release_tag", "$.release.tag_name")
@@ -101,7 +101,7 @@ func applyGitHubReleaseTemplate(cfg *config.WebhookConfig) {
 func applyGitHubWorkflowTemplate(cfg *config.WebhookConfig) {
 	setDefaultAuth(cfg)
 	ensureMatchHeaders(cfg)
-	setMatchHeaderIfMissing(cfg, "X-GitHub-Event", "workflow_run")
+	setGitHubEventHeaderIfMissing(cfg, "workflow_run")
 	ensureExtract(cfg)
 	setExtractIfMissing(cfg, "workflow_name", "$.workflow_run.name")
 	setExtractIfMissing(cfg, "workflow_status", "$.workflow_run.status")