From 7ecc7791d854b1a27616f49afec0263f7cf24df0 Mon Sep 17 00:00:00 2001 From: jamesread Date: Wed, 29 Jul 2026 00:30:57 +0100 Subject: [PATCH] chore: golangci-lint fixes --- service/.golangci.yml | 7 + service/cmd/config-tool/main.go | 21 +-- service/internal/api/api.go | 8 +- service/internal/api/api_log_arguments.go | 6 +- .../internal/api/api_log_arguments_test.go | 4 +- service/internal/api/api_test.go | 38 ++-- service/internal/auth/otjwt/jwt.go | 6 +- service/internal/auth/otjwt/jwt_test.go | 36 +++- .../auth/otoauth2/restapi_auth_oauth2.go | 2 +- service/internal/config/config_reloader.go | 30 ++-- service/internal/config/sanitize.go | 11 +- service/internal/config/source_file.go | 8 +- service/internal/entities/entities.go | 2 +- service/internal/entities/storage.go | 11 +- service/internal/executor/arguments.go | 8 +- service/internal/executor/arguments_test.go | 170 +++++++++--------- service/internal/executor/executor.go | 13 +- service/internal/executor/executor_unix.go | 1 - .../servicehost/log_directory_test.go | 2 +- .../servicehost/servicehost_nonwin.go | 1 - service/internal/tpl/templates.go | 2 +- service/internal/updatecheck/updateCheck.go | 2 +- service/internal/webhooks/jsonpath.go | 6 +- service/internal/webhooks/matcher.go | 3 +- service/main.go | 2 +- 25 files changed, 213 insertions(+), 187 deletions(-) diff --git a/service/.golangci.yml b/service/.golangci.yml index 2e486b8..6f7462d 100644 --- a/service/.golangci.yml +++ b/service/.golangci.yml @@ -9,6 +9,7 @@ linters: enable: - bidichk - bodyclose + - copyloopvar - durationcheck - errcheck - errorlint @@ -18,11 +19,17 @@ linters: - govet - ineffassign - misspell +# - modernize - nilerr - noctx +# - promlinter - staticcheck +# - testifylint + - thelper - unconvert +# - unparam - unused + - usestdlibvars settings: gocyclo: min-complexity: 5 diff --git a/service/cmd/config-tool/main.go b/service/cmd/config-tool/main.go index e714e64..ef9f111 100644 --- a/service/cmd/config-tool/main.go +++ b/service/cmd/config-tool/main.go @@ -3,6 +3,7 @@ package main import ( "flag" "fmt" + "maps" "os" "path/filepath" "strconv" @@ -98,18 +99,18 @@ func userDisplayName(username string, index int) string { return username } -func copyUserMapWithPassword(userMap map[string]interface{}, hashedPassword string) map[string]interface{} { - newUserMap := make(map[string]interface{}, len(userMap)+1) - for key, value := range userMap { - newUserMap[key] = value - } +func copyUserMapWithPassword(userMap map[string]any, hashedPassword string) map[string]any { + newUserMap := make(map[string]any, len(userMap)+1) + + maps.Copy(newUserMap, userMap) + newUserMap["password"] = hashedPassword return newUserMap } -func resetPasswordInUserMap(userValue interface{}, index int, hashedPassword string) interface{} { - userMap, ok := userValue.(map[string]interface{}) +func resetPasswordInUserMap(userValue any, index int, hashedPassword string) any { + userMap, ok := userValue.(map[string]any) if !ok { log.Warnf("User entry at index %d is not a map, skipping", index) return userValue @@ -122,8 +123,8 @@ func resetPasswordInUserMap(userValue interface{}, index int, hashedPassword str return copyUserMapWithPassword(userMap, hashedPassword) } -func resetPasswordsFromSlice(k *koanf.Koanf, usersSliceTyped []interface{}, hashedPassword string) { - newUsersSlice := make([]interface{}, len(usersSliceTyped)) +func resetPasswordsFromSlice(k *koanf.Koanf, usersSliceTyped []any, hashedPassword string) { + newUsersSlice := make([]any, len(usersSliceTyped)) for index, userValue := range usersSliceTyped { newUsersSlice[index] = resetPasswordInUserMap(userValue, index, hashedPassword) } @@ -155,7 +156,7 @@ func hasLocalUsers(cfg *config.Config) bool { } func applyPasswordResets(k *koanf.Koanf, cfg *config.Config, hashedPassword string) { - usersSliceTyped, ok := k.Get("authLocalUsers.users").([]interface{}) + usersSliceTyped, ok := k.Get("authLocalUsers.users").([]any) if ok && len(usersSliceTyped) > 0 { resetPasswordsFromSlice(k, usersSliceTyped, hashedPassword) return diff --git a/service/internal/api/api.go b/service/internal/api/api.go index ab9ae00..6b82e31 100644 --- a/service/internal/api/api.go +++ b/service/internal/api/api.go @@ -807,13 +807,13 @@ func paginate(total int64, size int64, start int64) pageInfo { if start < 0 { start = 0 } + if start >= total { return pageInfo{total: total, size: size, start: start, end: start, empty: true} } - end := start + size - if end > total { - end = total - } + + end := min(start+size, total) + return pageInfo{total: total, size: size, start: start, end: end, empty: false} } diff --git a/service/internal/api/api_log_arguments.go b/service/internal/api/api_log_arguments.go index b33c874..809d898 100644 --- a/service/internal/api/api_log_arguments.go +++ b/service/internal/api/api_log_arguments.go @@ -2,6 +2,7 @@ package api import ( "fmt" + "maps" "sort" "strings" @@ -35,9 +36,8 @@ func logEntryArgumentsToProto(args map[string]string) []*apiv1.StartActionArgume func copyStringMap(source map[string]string) map[string]string { copied := make(map[string]string, len(source)) - for key, value := range source { - copied[key] = value - } + + maps.Copy(copied, source) return copied } diff --git a/service/internal/api/api_log_arguments_test.go b/service/internal/api/api_log_arguments_test.go index 851898e..f5528fd 100644 --- a/service/internal/api/api_log_arguments_test.go +++ b/service/internal/api/api_log_arguments_test.go @@ -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", "echo {{ host }}", []config.ActionArgument{ + argumentAction("Ping host with stored args", "echo {{ 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", "echo {{ host }}", []config.ActionArgument{ + argumentAction("Ping host - reject missing required stored arg", "echo {{ host }}", []config.ActionArgument{ {Name: "host", Type: "ascii_identifier"}, }), } diff --git a/service/internal/api/api_test.go b/service/internal/api/api_test.go index 0ec5a9d..ad86e2b 100644 --- a/service/internal/api/api_test.go +++ b/service/internal/api/api_test.go @@ -29,6 +29,8 @@ func getNewTestServerAndClient(injectedConfig *config.Config) (*httptest.Server, } func getNewTestServerAndClientWithExecutor(injectedConfig *config.Config, ex *executor.Executor) (*httptest.Server, apiv1connect.OliveTinApiServiceClient) { + ex.Cfg = injectedConfig + apiPath, apiHandler := GetNewHandler(ex) mux := http.NewServeMux() @@ -102,8 +104,6 @@ func TestGetActionsAndStart(t *testing.T) { log.Infof("GetReadyz response: %v", respGetReady.Msg) - assert.Equal(t, true, true, "sayHello Failed") - // assert.Equal(t, 1, len(respGb.Msg.Actions), "Got 1 action button back") log.Printf("Response: %+v", respInit) @@ -112,7 +112,7 @@ func TestGetActionsAndStart(t *testing.T) { // ActionId: "blat" })) - assert.NotNil(t, err, "Error 404 after start action") + require.Error(t, err, "Error 404 after start action") assert.Nil(t, respSa, "Nil response for non existing action") defer conn.Close() @@ -137,12 +137,12 @@ func TestGetEntities(t *testing.T) { resp, err := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{})) - assert.NoError(t, err, "GetEntities should not return an error") + require.NoError(t, err, "GetEntities should not return an error") assert.NotNil(t, resp, "GetEntities response should not be nil") assert.NotNil(t, resp.Msg, "GetEntities response message should not be nil") entityDefinitions := resp.Msg.EntityDefinitions - assert.Equal(t, 3, len(entityDefinitions), "Should return 3 entity definitions") + assert.Len(t, entityDefinitions, 3, "Should return 3 entity definitions") validateEntityOrderAndStructure(t, entityDefinitions) validateNoDuplicates(t, entityDefinitions) @@ -151,6 +151,8 @@ func TestGetEntities(t *testing.T) { } func validateEntityListProperties(t *testing.T, client apiv1connect.OliveTinApiServiceClient) { + t.Helper() + resp, err := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{ EntityType: "server", Page: 1, @@ -185,21 +187,25 @@ func setupTestEntities() { } func validateEntityOrderAndStructure(t *testing.T, entityDefinitions []*apiv1.EntityDefinition) { + t.Helper() + assert.Equal(t, "application", entityDefinitions[0].Title, "First entity should be 'application' (alphabetically first)") - assert.Equal(t, 1, len(entityDefinitions[0].Instances), "Application should have 1 instance") + assert.Len(t, entityDefinitions[0].Instances, 1, "Application should have 1 instance") assert.Equal(t, "webapp", entityDefinitions[0].Instances[0].UniqueKey, "Application instance should be 'webapp'") assert.Equal(t, "database", entityDefinitions[1].Title, "Second entity should be 'database' (alphabetically second)") - assert.Equal(t, 2, len(entityDefinitions[1].Instances), "Database should have 2 instances") + assert.Len(t, entityDefinitions[1].Instances, 2, "Database should have 2 instances") assert.Equal(t, "mysql", entityDefinitions[1].Instances[0].UniqueKey, "First database instance should be 'mysql' (alphabetically first)") assert.Equal(t, "postgres", entityDefinitions[1].Instances[1].UniqueKey, "Second database instance should be 'postgres' (alphabetically second)") assert.Equal(t, "server", entityDefinitions[2].Title, "Third entity should be 'server' (alphabetically third)") - assert.Equal(t, 0, len(entityDefinitions[2].Instances), "Server instances should not be included in bulk list response") + assert.Empty(t, entityDefinitions[2].Instances, "Server instances should not be included in bulk list response") assert.Equal(t, int32(3), entityDefinitions[2].TotalInstances, "Server should report total instance count") } func validateNoDuplicates(t *testing.T, entityDefinitions []*apiv1.EntityDefinition) { + t.Helper() + instanceKeys := make(map[string]map[string]bool) for _, def := range entityDefinitions { instanceKeys[def.Title] = make(map[string]bool) @@ -211,13 +217,15 @@ func validateNoDuplicates(t *testing.T, entityDefinitions []*apiv1.EntityDefinit } func validateConsistency(t *testing.T, client apiv1connect.OliveTinApiServiceClient, entityDefinitions []*apiv1.EntityDefinition) { + t.Helper() + resp2, err2 := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{})) - assert.NoError(t, err2, "Second GetEntities call should not return an error") - assert.Equal(t, len(entityDefinitions), len(resp2.Msg.EntityDefinitions), "Second call should return same number of entity definitions") + require.NoError(t, err2, "Second GetEntities call should not return an error") + assert.Len(t, entityDefinitions, len(resp2.Msg.EntityDefinitions), "Second call should return same number of entity definitions") for i, def := range entityDefinitions { assert.Equal(t, def.Title, resp2.Msg.EntityDefinitions[i].Title, "Entity order should be consistent across calls") - assert.Equal(t, len(def.Instances), len(resp2.Msg.EntityDefinitions[i].Instances), "Instance count should be consistent") + assert.Len(t, def.Instances, len(resp2.Msg.EntityDefinitions[i].Instances), "Instance count should be consistent") for j, inst := range def.Instances { assert.Equal(t, inst.UniqueKey, resp2.Msg.EntityDefinitions[i].Instances[j].UniqueKey, "Instance order should be consistent across calls") } @@ -376,6 +384,8 @@ func findBindingByTitle(ex *executor.Executor, title string) *executor.ActionBin } func testWithEntity(t *testing.T, binding *executor.ActionBinding, rr *DashboardRenderRequest, enabled bool, expectedCanExec bool, message string) { + t.Helper() + binding.Entity = &entities.Entity{ UniqueKey: "test-entity", Data: map[string]any{"enabled": enabled}, @@ -809,12 +819,12 @@ func TestRegisterStreamingClientEnforcesLimit(t *testing.T) { } close(overflow.heartbeatDone) err := api.registerStreamingClient(overflow) - assert.ErrorIs(t, err, errEventStreamClientLimit) - assert.Equal(t, maxEventStreamClients, len(api.streamingClients)) + require.ErrorIs(t, err, errEventStreamClientLimit) + assert.Len(t, api.streamingClients, maxEventStreamClients) api.removeClient(clients[0]) require.NoError(t, api.registerStreamingClient(overflow)) - assert.Equal(t, maxEventStreamClients, len(api.streamingClients)) + assert.Len(t, api.streamingClients, maxEventStreamClients) for _, client := range clients[1:] { api.removeClient(client) diff --git a/service/internal/auth/otjwt/jwt.go b/service/internal/auth/otjwt/jwt.go index 2b06da5..e61754b 100644 --- a/service/internal/auth/otjwt/jwt.go +++ b/service/internal/auth/otjwt/jwt.go @@ -156,7 +156,7 @@ func parseJwtTokenWithLocalKey(cfg *config.Config, jwtString string) (*jwt.Token return nil, err } - keyFunc := func(token *jwt.Token) (interface{}, error) { + keyFunc := func(token *jwt.Token) (any, error) { if _, ok := token.Method.(*jwt.SigningMethodRSA); !ok { return nil, fmt.Errorf("parseJwt expected token algorithm RSA but got: %v", token.Header["alg"]) } @@ -170,7 +170,7 @@ func parseJwtTokenWithLocalKey(cfg *config.Config, jwtString string) (*jwt.Token // Hash-based Message Authentication Code func parseJwtTokenWithHMAC(cfg *config.Config, jwtString string) (*jwt.Token, error) { - keyFunc := func(token *jwt.Token) (interface{}, error) { + keyFunc := func(token *jwt.Token) (any, error) { if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { return nil, fmt.Errorf("parseJwt expected token algorithm HMAC but got: %v", token.Header["alg"]) } @@ -237,7 +237,7 @@ func parseJwt(cfg *config.Config, token string) *authTypes.AuthenticatedUser { func parseGroupClaim(groupClaim string, claims jwt.MapClaims) string { usergroup := "" if val, ok := claims[groupClaim]; ok { - if array, ok := val.([]interface{}); ok { + if array, ok := val.([]any); ok { groups := make([]string, len(array)) for i, v := range array { groups[i] = fmt.Sprintf("%s", v) diff --git a/service/internal/auth/otjwt/jwt_test.go b/service/internal/auth/otjwt/jwt_test.go index 219fcb5..ceae685 100644 --- a/service/internal/auth/otjwt/jwt_test.go +++ b/service/internal/auth/otjwt/jwt_test.go @@ -20,6 +20,8 @@ import ( ) func generateRSAKeyPair(t *testing.T) (*rsa.PrivateKey, []byte) { + t.Helper() + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { t.Fatalf("failed to generate RSA key: %v", err) @@ -42,6 +44,8 @@ func generateRSAKeyPair(t *testing.T) (*rsa.PrivateKey, []byte) { } func createKeys(t *testing.T) (*rsa.PrivateKey, string) { + t.Helper() + tmpFile, err := os.CreateTemp(os.TempDir(), "olivetin-jwt-") if err != nil { t.Fatalf("failed to create temp file: %v", err) @@ -66,6 +70,8 @@ func newMux() *http.ServeMux { } func createJWTTokenWithExpirationAndAudience(t *testing.T, privateKey *rsa.PrivateKey, expire int64, audience string) string { + t.Helper() + token := jwt.New(jwt.SigningMethodRS256) claims := token.Claims.(jwt.MapClaims) claims["nbf"] = time.Now().Unix() - 1000 @@ -84,6 +90,8 @@ func createJWTTokenWithExpirationAndAudience(t *testing.T, privateKey *rsa.Priva } func setupJWTTestHandler(t *testing.T, cfg *config.Config) http.Handler { + t.Helper() + mux := newMux() mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { context := &authpublic.AuthCheckingContext{ @@ -93,7 +101,7 @@ func setupJWTTestHandler(t *testing.T, cfg *config.Config) http.Handler { user := CheckUserFromJwtHeader(context) if user == nil { - w.WriteHeader(403) + w.WriteHeader(http.StatusForbidden) return } @@ -104,6 +112,8 @@ func setupJWTTestHandler(t *testing.T, cfg *config.Config) http.Handler { } func verifyJWTResponse(t *testing.T, res *http.Response, expectCode int) { + t.Helper() + defer func() { _ = res.Body.Close() }() assert.Equal(t, expectCode, res.StatusCode) body, _ := io.ReadAll(res.Body) @@ -111,10 +121,14 @@ func verifyJWTResponse(t *testing.T, res *http.Response, expectCode int) { } func testJwkValidation(t *testing.T, expire int64, expectCode int) { + t.Helper() + testJwkValidationWithAudience(t, expire, expectCode, "", "") } func testJwkValidationWithAudience(t *testing.T, expire int64, expectCode int, configAudience, tokenAudience string) { + t.Helper() + privateKey, publicKeyPath := createKeys(t) defer func() { _ = os.Remove(publicKeyPath) }() @@ -143,22 +157,24 @@ func testJwkValidationWithAudience(t *testing.T, expire int64, expectCode int, c } func TestJWTSignatureVerificationSucceeds(t *testing.T) { - testJwkValidation(t, 1000, 200) + testJwkValidation(t, 1000, http.StatusOK) } func TestJWTSignatureVerificationFails(t *testing.T) { - testJwkValidation(t, -500, 403) + testJwkValidation(t, -500, http.StatusForbidden) } func TestJWTAudienceValidationRejectsWrongAudience(t *testing.T) { - testJwkValidationWithAudience(t, 1000, 403, "expected-audience", "wrong-audience") + testJwkValidationWithAudience(t, 1000, http.StatusForbidden, "expected-audience", "wrong-audience") } func TestJWTAudienceValidationAcceptsCorrectAudience(t *testing.T) { - testJwkValidationWithAudience(t, 1000, 200, "expected-audience", "expected-audience") + testJwkValidationWithAudience(t, 1000, http.StatusOK, "expected-audience", "expected-audience") } -func createJWTTokenWithGroups(t *testing.T, privateKey *rsa.PrivateKey, groups interface{}) string { +func createJWTTokenWithGroups(t *testing.T, privateKey *rsa.PrivateKey, groups any) string { + t.Helper() + token := jwt.New(jwt.SigningMethodRS256) claims := token.Claims.(jwt.MapClaims) claims["nbf"] = time.Now().Unix() - 1000 @@ -174,7 +190,9 @@ func createJWTTokenWithGroups(t *testing.T, privateKey *rsa.PrivateKey, groups i } func makeJWTRequest(t *testing.T, srv *httptest.Server, tokenStr string) *http.Response { - req, err := http.NewRequestWithContext(t.Context(), "GET", srv.URL, nil) + t.Helper() + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL, nil) if err != nil { t.Fatalf("failed to create request: %v", err) } @@ -209,7 +227,7 @@ func TestJWTHeader(t *testing.T) { user := CheckUserFromJwtHeader(context) if user == nil { - w.WriteHeader(403) + w.WriteHeader(http.StatusForbidden) return } @@ -223,7 +241,7 @@ func TestJWTHeader(t *testing.T) { res := makeJWTRequest(t, srv, tokenStr) defer func() { _ = res.Body.Close() }() - assert.Equal(t, 200, res.StatusCode) + assert.Equal(t, http.StatusOK, res.StatusCode) body, _ := io.ReadAll(res.Body) t.Logf("Response body: %s", string(body)) } diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2.go b/service/internal/auth/otoauth2/restapi_auth_oauth2.go index 5c38a39..9b87f39 100644 --- a/service/internal/auth/otoauth2/restapi_auth_oauth2.go +++ b/service/internal/auth/otoauth2/restapi_auth_oauth2.go @@ -346,7 +346,7 @@ func getUserInfo(cfg *config.Config, client *http.Client, provider *config.OAuth defer cancel() - req, err := http.NewRequestWithContext(ctx, "GET", provider.WhoamiUrl, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, provider.WhoamiUrl, nil) if err != nil { log.Error("Could not construct user data request", err) diff --git a/service/internal/config/config_reloader.go b/service/internal/config/config_reloader.go index 9b85c6d..4a81a09 100644 --- a/service/internal/config/config_reloader.go +++ b/service/internal/config/config_reloader.go @@ -249,17 +249,17 @@ func loadAndMergeIncludedFile(k *koanf.Koanf, includePath, filename string) { }).Info("Successfully loaded included config file") } -func mergeFuncForSource(sourceFile string) func(src, dest map[string]interface{}) error { - return func(src map[string]interface{}, dest map[string]interface{}) error { +func mergeFuncForSource(sourceFile string) func(src, dest map[string]any) error { + return func(src map[string]any, dest map[string]any) error { return mergeFunc(src, dest, sourceFile) } } // mergeActionsWhenBothExist merges actions when both src and dest have actions. -func mergeActionsWhenBothExist(srcActions interface{}, destActions interface{}, dest map[string]interface{}, sourceFile string) { +func mergeActionsWhenBothExist(srcActions any, destActions any, dest map[string]any, sourceFile string) { stampSourceOnMaps(srcActions, sourceFile) - srcSlice, ok1 := srcActions.([]interface{}) - destSlice, ok2 := destActions.([]interface{}) + srcSlice, ok1 := srcActions.([]any) + destSlice, ok2 := destActions.([]any) if ok1 && ok2 { dest["actions"] = append(destSlice, srcSlice...) } else { @@ -268,7 +268,7 @@ func mergeActionsWhenBothExist(srcActions interface{}, destActions interface{}, } // mergeActionsFromSource merges actions from source into destination. -func mergeActionsFromSource(srcActions interface{}, dest map[string]interface{}, sourceFile string) { +func mergeActionsFromSource(srcActions any, dest map[string]any, sourceFile string) { if destActions, ok := dest["actions"]; ok { mergeActionsWhenBothExist(srcActions, destActions, dest, sourceFile) } else { @@ -278,9 +278,9 @@ func mergeActionsFromSource(srcActions interface{}, dest map[string]interface{}, } // mergeDashboardsWhenBothExist merges dashboards when both src and dest have dashboards. -func mergeDashboardsWhenBothExist(srcDashboards interface{}, destDashboards interface{}, dest map[string]interface{}) { - srcSlice, ok1 := srcDashboards.([]interface{}) - destSlice, ok2 := destDashboards.([]interface{}) +func mergeDashboardsWhenBothExist(srcDashboards any, destDashboards any, dest map[string]any) { + srcSlice, ok1 := srcDashboards.([]any) + destSlice, ok2 := destDashboards.([]any) if ok1 && ok2 { dest["dashboards"] = append(destSlice, srcSlice...) } else { @@ -289,7 +289,7 @@ func mergeDashboardsWhenBothExist(srcDashboards interface{}, destDashboards inte } // mergeDashboardsFromSource merges dashboards from source into destination. -func mergeDashboardsFromSource(srcDashboards interface{}, dest map[string]interface{}) { +func mergeDashboardsFromSource(srcDashboards any, dest map[string]any) { if destDashboards, ok := dest["dashboards"]; ok { mergeDashboardsWhenBothExist(srcDashboards, destDashboards, dest) } else { @@ -298,10 +298,10 @@ func mergeDashboardsFromSource(srcDashboards interface{}, dest map[string]interf } // mergeEntitiesWhenBothExist merges entities when both src and dest have entities. -func mergeEntitiesWhenBothExist(srcEntities interface{}, destEntities interface{}, dest map[string]interface{}, sourceFile string) { +func mergeEntitiesWhenBothExist(srcEntities any, destEntities any, dest map[string]any, sourceFile string) { stampSourceOnMaps(srcEntities, sourceFile) - srcSlice, ok1 := srcEntities.([]interface{}) - destSlice, ok2 := destEntities.([]interface{}) + srcSlice, ok1 := srcEntities.([]any) + destSlice, ok2 := destEntities.([]any) if ok1 && ok2 { dest["entities"] = append(destSlice, srcSlice...) } else { @@ -310,7 +310,7 @@ func mergeEntitiesWhenBothExist(srcEntities interface{}, destEntities interface{ } // mergeEntitiesFromSource merges entities from source into destination. -func mergeEntitiesFromSource(srcEntities interface{}, dest map[string]interface{}, sourceFile string) { +func mergeEntitiesFromSource(srcEntities any, dest map[string]any, sourceFile string) { if destEntities, ok := dest["entities"]; ok { mergeEntitiesWhenBothExist(srcEntities, destEntities, dest, sourceFile) } else { @@ -319,7 +319,7 @@ func mergeEntitiesFromSource(srcEntities interface{}, dest map[string]interface{ } } -func mergeFunc(src map[string]interface{}, dest map[string]interface{}, sourceFile string) error { +func mergeFunc(src map[string]any, dest map[string]any, sourceFile string) error { if srcActions, ok := src["actions"]; ok { mergeActionsFromSource(srcActions, dest, sourceFile) } diff --git a/service/internal/config/sanitize.go b/service/internal/config/sanitize.go index 9b9e3fb..bb48550 100644 --- a/service/internal/config/sanitize.go +++ b/service/internal/config/sanitize.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "slices" "strings" "text/template" @@ -179,13 +180,7 @@ func (cfg *Config) inlineActionExists(action *Action) bool { } func (cfg *Config) inlineActionPointerExists(action *Action) bool { - for _, existingAction := range cfg.Actions { - if existingAction == action { - return true - } - } - - return false + return slices.Contains(cfg.Actions, action) } func (cfg *Config) inlineActionIDExists(action *Action) bool { @@ -400,7 +395,7 @@ func expandEnvTemplate(source string) string { return source } var b strings.Builder - if err := t.Execute(&b, map[string]interface{}{"Env": env.BuildEnvMap()}); err != nil { + if err := t.Execute(&b, map[string]any{"Env": env.BuildEnvMap()}); err != nil { log.WithFields(log.Fields{"error": err}).Debug("Env template execute failed, using literal") return source } diff --git a/service/internal/config/source_file.go b/service/internal/config/source_file.go index 37f1880..80174c4 100644 --- a/service/internal/config/source_file.go +++ b/service/internal/config/source_file.go @@ -15,7 +15,7 @@ func stampSourceOnMaps(raw any, sourceFile string) any { return raw } - items, ok := raw.([]interface{}) + items, ok := raw.([]any) if !ok { return raw } @@ -27,7 +27,7 @@ func stampSourceOnMaps(raw any, sourceFile string) any { } func stampSourceOnMap(item any, sourceFile string) { - m, ok := item.(map[string]interface{}) + m, ok := item.(map[string]any) if !ok { return } @@ -74,7 +74,7 @@ func sourcePathAt(paths []string, index int) string { } func stampedSourcePaths(raw any) []string { - items, ok := raw.([]interface{}) + items, ok := raw.([]any) if !ok { return nil } @@ -87,7 +87,7 @@ func stampedSourcePaths(raw any) []string { } func stampedSourceFromMap(item any) string { - m, ok := item.(map[string]interface{}) + m, ok := item.(map[string]any) if !ok { return "" } diff --git a/service/internal/entities/entities.go b/service/internal/entities/entities.go index 6be2b7d..5ca86b8 100644 --- a/service/internal/entities/entities.go +++ b/service/internal/entities/entities.go @@ -69,7 +69,7 @@ func watchAndLoadEntity(baseDir string, ef *config.EntityFile) { p = filepath.Join(baseDir, p) log.WithFields(log.Fields{"entityFile": p}).Debugf("Adding config dir to entity file path") } - go filehelper.WatchFileWrite(p, func(filename string) { loadEntityFile(p, ef.Name) }, filehelper.WatchMeta{ + go filehelper.WatchFileWrite(p, func(_ string) { loadEntityFile(p, ef.Name) }, filehelper.WatchMeta{ ConfigFile: ef.SourceFile, }) loadEntityFile(p, ef.Name) diff --git a/service/internal/entities/storage.go b/service/internal/entities/storage.go index bd61718..2871e04 100644 --- a/service/internal/entities/storage.go +++ b/service/internal/entities/storage.go @@ -10,6 +10,7 @@ package entities */ import ( + "maps" "sort" "strconv" "strings" @@ -39,9 +40,8 @@ func GetEntities() EntitiesByClass { for entityName, entityInstances := range entities { copiedInstances := make(entityInstancesByKey, len(entityInstances)) - for key, entity := range entityInstances { - copiedInstances[key] = entity - } + maps.Copy(copiedInstances, entityInstances) + copiedEntities[entityName] = copiedInstances } @@ -57,9 +57,8 @@ func GetEntityInstances(entityName string) entityInstancesByKey { if entities, ok := entities[entityName]; ok { copiedInstances := make(entityInstancesByKey, len(entities)) - for key, entity := range entities { - copiedInstances[key] = entity - } + maps.Copy(copiedInstances, entities) + return copiedInstances } diff --git a/service/internal/executor/arguments.go b/service/internal/executor/arguments.go index 53ce43f..084ce6a 100644 --- a/service/internal/executor/arguments.go +++ b/service/internal/executor/arguments.go @@ -67,7 +67,7 @@ func parseExecSegment(arg string, values map[string]string, entity *entities.Ent func validateArguments(values map[string]string, action *config.Action) error { for _, arg := range action.Arguments { - if err := typecheckActionArgument(&arg, values[arg.Name], action); err != nil { + if err := typecheckActionArgument(&arg, values[arg.Name]); err != nil { return err } log.WithFields(log.Fields{"name": arg.Name, "value": values[arg.Name]}).Debugf("Arg assigned") @@ -90,7 +90,7 @@ func parseActionArguments(req *ExecutionRequest) (string, error) { argName := arg.Name argValue := req.Arguments[argName] - err := typecheckActionArgument(&arg, argValue, req.Binding.Action) + err := typecheckActionArgument(&arg, argValue) if err != nil { return "", err @@ -153,7 +153,7 @@ func argumentSkipsValidation(arg *config.ActionArgument) bool { return arg.Type == "html" } -func typecheckActionArgument(arg *config.ActionArgument, value string, action *config.Action) error { +func typecheckActionArgument(arg *config.ActionArgument, value string) error { if argumentSkipsValidation(arg) { return nil } @@ -199,7 +199,7 @@ func ValidateArgument(arg *config.ActionArgument, value string, action *config.A mangledValue := MangleArgumentValue(arg, value, action.Title) // Use the same validation path as the executor - return typecheckActionArgument(arg, mangledValue, action) + return typecheckActionArgument(arg, mangledValue) } func typecheckActionArgumentFound(value string, arg *config.ActionArgument) error { diff --git a/service/internal/executor/arguments_test.go b/service/internal/executor/arguments_test.go index f4c7882..6a77f95 100644 --- a/service/internal/executor/arguments_test.go +++ b/service/internal/executor/arguments_test.go @@ -12,16 +12,17 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSanitizeUnsafe(t *testing.T) { - assert.Nil(t, TypeSafetyCheck("", "_zomg_ c:/ haxxor ' bobby tables && rm -rf ", "very_dangerous_raw_string")) + require.NoError(t, TypeSafetyCheck("", "_zomg_ c:/ haxxor ' bobby tables && rm -rf ", "very_dangerous_raw_string")) } func TestSanitizeUnimplemented(t *testing.T) { err := TypeSafetyCheck("", "I am a happy little argument", "greeting_type") - assert.NotNil(t, err, "Test an argument type that does not exist") + require.Error(t, err, "Test an argument type that does not exist") } func TestValidateArgumentCheckboxDefaultValues(t *testing.T) { @@ -35,10 +36,10 @@ func TestValidateArgumentCheckboxDefaultValues(t *testing.T) { // Default checkbox values without choices should accept "1" and "0" err := ValidateArgument(&arg, "1", &action) - assert.Nil(t, err, "Expected checkbox value \"1\" to be accepted without choices") + require.NoError(t, err, "Expected checkbox value \"1\" to be accepted without choices") err = ValidateArgument(&arg, "0", &action) - assert.Nil(t, err, "Expected checkbox value \"0\" to be accepted without choices") + require.NoError(t, err, "Expected checkbox value \"0\" to be accepted without choices") } func TestMangleCheckboxValueWithChoices(t *testing.T) { @@ -105,14 +106,14 @@ func TestValidateArgumentCheckboxWithChoices(t *testing.T) { // Titles should be accepted once mangled to their values err := ValidateArgument(&arg, "Enabled", &action) - assert.Nil(t, err, "Expected checkbox title \"Enabled\" to be accepted after mangling to choice value") + require.NoError(t, err, "Expected checkbox title \"Enabled\" to be accepted after mangling to choice value") err = ValidateArgument(&arg, "Disabled", &action) - assert.Nil(t, err, "Expected checkbox title \"Disabled\" to be accepted after mangling to choice value") + require.NoError(t, err, "Expected checkbox title \"Disabled\" to be accepted after mangling to choice value") // Unknown titles should be rejected because they do not match any choice value err = ValidateArgument(&arg, "Maybe", &action) - assert.NotNil(t, err, "Expected unknown checkbox title to be rejected against choices") + require.Error(t, err, "Expected unknown checkbox title to be rejected against choices") } func checklistTestArg() config.ActionArgument { @@ -134,13 +135,13 @@ func TestValidateArgumentChecklistSelections(t *testing.T) { action := config.Action{Title: "Test checklist"} err := ValidateArgument(&arg, "documents", &action) - assert.Nil(t, err) + require.NoError(t, err) err = ValidateArgument(&arg, `["documents","photos"]`, &action) - assert.Nil(t, err) + require.NoError(t, err) err = ValidateArgument(&arg, `["documents","unknown"]`, &action) - assert.NotNil(t, err) + require.Error(t, err) } func TestValidateArgumentChecklistTitleMangling(t *testing.T) { @@ -150,7 +151,7 @@ func TestValidateArgumentChecklistTitleMangling(t *testing.T) { action := config.Action{Title: "Test checklist title mangling"} err := ValidateArgument(&arg, `["Documents","Photos"]`, &action) - assert.Nil(t, err) + require.NoError(t, err) } func TestValidateArgumentChecklistEmptySelection(t *testing.T) { @@ -160,11 +161,11 @@ func TestValidateArgumentChecklistEmptySelection(t *testing.T) { action := config.Action{Title: "Test checklist empty"} err := ValidateArgument(&arg, "", &action) - assert.Nil(t, err) + require.NoError(t, err) arg.RejectNull = true err = ValidateArgument(&arg, "", &action) - assert.NotNil(t, err) + require.Error(t, err) } func TestValidateArgumentChecklistWithoutChoices(t *testing.T) { @@ -177,7 +178,7 @@ func TestValidateArgumentChecklistWithoutChoices(t *testing.T) { action := config.Action{Title: "Test checklist without choices"} err := ValidateArgument(&arg, "documents", &action) - assert.NotNil(t, err) + require.Error(t, err) } func TestValidateArgumentChecklistRejectsEmptySegment(t *testing.T) { @@ -187,7 +188,7 @@ func TestValidateArgumentChecklistRejectsEmptySegment(t *testing.T) { action := config.Action{Title: "Test checklist empty segment"} err := ValidateArgument(&arg, `["documents","","photos"]`, &action) - assert.NotNil(t, err) + require.Error(t, err) } func TestMangleArgumentValueChecklist(t *testing.T) { @@ -223,13 +224,13 @@ func TestValidateArgumentChecklistEntitySelections(t *testing.T) { action := config.Action{Title: "Test checklist entity"} err := ValidateArgument(&arg, "attic", &action) - assert.Nil(t, err) + require.NoError(t, err) err = ValidateArgument(&arg, `["attic","basement"]`, &action) - assert.Nil(t, err) + require.NoError(t, err) err = ValidateArgument(&arg, `["attic","unknown"]`, &action) - assert.NotNil(t, err) + require.Error(t, err) } func TestMangleArgumentValueChecklistEntityTitles(t *testing.T) { @@ -274,7 +275,7 @@ func TestParseActionArgumentsChecklistEmptySelection(t *testing.T) { mangleInvalidArgumentValues(req) out, err := parseActionArguments(req) - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, "echo 'Selected segments: '", out) } @@ -307,13 +308,13 @@ func TestArgumentValueNullable(t *testing.T) { out, err := parseActionArguments(req) assert.Equal(t, "echo 'Releasing hounds'", out) - assert.Nil(t, err) + require.NoError(t, err) req.Binding.Action.Arguments[0].RejectNull = true _, err = parseActionArguments(req) - assert.NotNil(t, err) + require.Error(t, err) } func TestArgumentNameNumbers(t *testing.T) { @@ -336,7 +337,7 @@ func TestArgumentNameNumbers(t *testing.T) { out, err := parseActionArguments(req) assert.Equal(t, "echo 'Tickling Fred'", out) - assert.Nil(t, err) + require.NoError(t, err) } func TestArgumentNotProvided(t *testing.T) { @@ -356,8 +357,8 @@ func TestArgumentNotProvided(t *testing.T) { out, err := parseActionArguments(req) - assert.Equal(t, "", out) - assert.Equal(t, err.Error(), "required arg not provided: personName") + assert.Empty(t, out) + assert.Equal(t, "required arg not provided: personName", err.Error()) } func TestExecArrayParsing(t *testing.T) { @@ -372,7 +373,7 @@ func TestExecArrayParsing(t *testing.T) { out, err := parseActionExec(req.Arguments, req.Binding.Action, req.Binding.Entity) - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, []string{"ls", "-alh"}, out) } @@ -394,7 +395,7 @@ func TestExecArrayWithTemplateReplacement(t *testing.T) { out, err := parseActionExec(values, &a1, nil) - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, []string{"ls", "-alh", "tmp"}, out) } @@ -411,7 +412,7 @@ func TestCheckShellArgumentSafetyWithURL(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "unsafe argument type 'url' cannot be used with Shell execution") assert.Contains(t, err.Error(), "https://docs.olivetin.app/action_execution/shellvsexec.html") } @@ -429,7 +430,7 @@ func TestCheckShellArgumentSafetyWithEmail(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "unsafe argument type 'email' cannot be used with Shell execution") } @@ -446,7 +447,7 @@ func TestCheckShellArgumentSafetyWithExec(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.Nil(t, err) + require.NoError(t, err) } func TestCheckShellArgumentSafetyWithSafeTypes(t *testing.T) { @@ -462,7 +463,7 @@ func TestCheckShellArgumentSafetyWithSafeTypes(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.Nil(t, err) + require.NoError(t, err) } func TestCheckShellArgumentSafetyWithPassword(t *testing.T) { @@ -478,7 +479,7 @@ func TestCheckShellArgumentSafetyWithPassword(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "unsafe argument type 'password' cannot be used with Shell execution") assert.Contains(t, err.Error(), "https://docs.olivetin.app/action_execution/shellvsexec.html") } @@ -496,7 +497,7 @@ func TestCheckShellArgumentSafetyWithPasswordAndExec(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.Nil(t, err) + require.NoError(t, err) } func TestCheckShellArgumentSafetyWithHTML(t *testing.T) { @@ -509,7 +510,7 @@ func TestCheckShellArgumentSafetyWithHTML(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "unsafe argument type 'html'") } @@ -523,7 +524,7 @@ func TestCheckShellArgumentSafetyWithConfirmation(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.Nil(t, err, "confirmation is constrained to 0/1 and is safe with shell") + require.NoError(t, err, "confirmation is constrained to 0/1 and is safe with shell") } func TestCheckShellArgumentSafetyWithUnnamedConfirmation(t *testing.T) { @@ -536,7 +537,7 @@ func TestCheckShellArgumentSafetyWithUnnamedConfirmation(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.Nil(t, err) + require.NoError(t, err) } func TestCheckShellArgumentSafetyWithChoicelessCheckbox(t *testing.T) { @@ -549,7 +550,7 @@ func TestCheckShellArgumentSafetyWithChoicelessCheckbox(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "unsafe argument type 'checkbox'") } @@ -563,20 +564,20 @@ func TestCheckShellArgumentSafetyWithCustomRegex(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "unsafe argument type 'regex:[a-zA-Z0-9.-]+'") } func TestTypeSafetyCheckUrl(t *testing.T) { - assert.Nil(t, TypeSafetyCheck("test1", "http://google.com", "url"), "Test URL: google.com") - assert.Nil(t, TypeSafetyCheck("test2", "http://technowax.net:80?foo=bar", "url"), "Test URL: technowax.net with query arguments") - assert.Nil(t, TypeSafetyCheck("test3", "http://localhost:80?foo=bar", "url"), "Test URL: localhost with query arguments") - assert.Nil(t, TypeSafetyCheck("test7", "https://example.com/path", "url"), "Test URL: https scheme") - assert.NotNil(t, TypeSafetyCheck("test4", "http://lo host:80", "url"), "Test a badly formed URL") - assert.NotNil(t, TypeSafetyCheck("test5", "12345", "url"), "Test a badly formed URL") - assert.NotNil(t, TypeSafetyCheck("test6", "_!23;", "url"), "Test a badly formed URL") - assert.NotNil(t, TypeSafetyCheck("test8", "file:///etc/passwd", "url"), "file:// scheme must be rejected") - assert.NotNil(t, TypeSafetyCheck("test9", "gopher://example.com", "url"), "gopher:// scheme must be rejected") + require.NoError(t, TypeSafetyCheck("test1", "http://google.com", "url"), "Test URL: google.com") + require.NoError(t, TypeSafetyCheck("test2", "http://technowax.net:80?foo=bar", "url"), "Test URL: technowax.net with query arguments") + require.NoError(t, TypeSafetyCheck("test3", "http://localhost:80?foo=bar", "url"), "Test URL: localhost with query arguments") + require.NoError(t, TypeSafetyCheck("test7", "https://example.com/path", "url"), "Test URL: https scheme") + require.Error(t, TypeSafetyCheck("test4", "http://lo host:80", "url"), "Test a badly formed URL") + require.Error(t, TypeSafetyCheck("test5", "12345", "url"), "Test a badly formed URL") + require.Error(t, TypeSafetyCheck("test6", "_!23;", "url"), "Test a badly formed URL") + require.Error(t, TypeSafetyCheck("test8", "file:///etc/passwd", "url"), "file:// scheme must be rejected") + require.Error(t, TypeSafetyCheck("test9", "gopher://example.com", "url"), "gopher:// scheme must be rejected") } func TestTypeSafetyCheckRegex(t *testing.T) { @@ -622,9 +623,9 @@ func TestTypeSafetyCheckRegex(t *testing.T) { err := typeSafetyCheckRegex(tt.field, tt.value, tt.pattern) if tt.hasError { - assert.NotNil(t, err, "Expected error for value %s with pattern %s, but got no error", tt.value, tt.pattern) + require.Error(t, err, "Expected error for value %s with pattern %s, but got no error", tt.value, tt.pattern) } else { - assert.Nil(t, err, "Expected no error for value %s with pattern %s, but got error: %v", tt.value, tt.pattern, err) + require.NoError(t, err, "Expected no error for value %s with pattern %s, but got error: %v", tt.value, tt.pattern, err) } }) } @@ -687,9 +688,9 @@ func TestTypeSafetyCheckEmail(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck(tt.field, tt.value, "email") if tt.hasError { - assert.NotNil(t, err, "Expected error for value '%s'", tt.value) + require.Error(t, err, "Expected error for value '%s'", tt.value) } else { - assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) + require.NoError(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) } }) } @@ -716,9 +717,9 @@ func TestTypeSafetyCheckDatetime(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck(tt.field, tt.value, "datetime") if tt.hasError { - assert.NotNil(t, err, "Expected error for value '%s'", tt.value) + require.Error(t, err, "Expected error for value '%s'", tt.value) } else { - assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) + require.NoError(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) } }) } @@ -740,7 +741,7 @@ func TestTypeSafetyCheckRawStringMultiline(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck(tt.field, tt.value, "raw_string_multiline") - assert.Nil(t, err, "raw_string_multiline should accept any value") + require.NoError(t, err, "raw_string_multiline should accept any value") }) } } @@ -772,6 +773,8 @@ func TestTypeSafetyCheckUnicodeIdentifier(t *testing.T) { } func validateTypeSafetyResult(t *testing.T, value string, expectsError bool, err error) { + t.Helper() + if expectsError { assertErrorExpected(t, value, err) } else { @@ -780,6 +783,8 @@ func validateTypeSafetyResult(t *testing.T, value string, expectsError bool, err } func assertErrorExpected(t *testing.T, value string, err error) { + t.Helper() + if err == nil { t.Errorf("Expected error for value '%s', but got none", value) } else { @@ -788,6 +793,8 @@ func assertErrorExpected(t *testing.T, value string, err error) { } func assertNoErrorExpected(t *testing.T, value string, err error) { + t.Helper() + if err != nil { t.Errorf("Expected no error for value '%s', but got: %v", value, err) } else { @@ -816,9 +823,9 @@ func TestTypeSafetyCheckAsciiIdentifier(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck(tt.field, tt.value, "ascii_identifier") if tt.hasError { - assert.NotNil(t, err, "Expected error for value '%s'", tt.value) + require.Error(t, err, "Expected error for value '%s'", tt.value) } else { - assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) + require.NoError(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) } }) } @@ -854,9 +861,9 @@ func TestTypeSafetyCheckDnsName(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck("host", tt.value, "dnsname") if tt.hasError { - assert.NotNil(t, err, "Expected error for value '%s'", tt.value) + require.Error(t, err, "Expected error for value '%s'", tt.value) } else { - assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) + require.NoError(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) } }) } @@ -886,9 +893,9 @@ func TestTypeSafetyCheckShellSafeIdentifier(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck("username", tt.value, "shell_safe_identifier") if tt.hasError { - assert.NotNil(t, err, "Expected error for value '%s'", tt.value) + require.Error(t, err, "Expected error for value '%s'", tt.value) } else { - assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) + require.NoError(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) } }) } @@ -915,9 +922,9 @@ func TestTypeSafetyCheckAsciiSentence(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := TypeSafetyCheck(tt.field, tt.value, "ascii_sentence") if tt.hasError { - assert.NotNil(t, err, "Expected error for value '%s'", tt.value) + require.Error(t, err, "Expected error for value '%s'", tt.value) } else { - assert.Nil(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) + require.NoError(t, err, "Expected no error for value '%s', but got: %v", tt.value, err) } }) } @@ -928,10 +935,9 @@ func TestTypecheckActionArgumentEmptyName(t *testing.T) { Name: "", Type: "ascii", } - action := config.Action{Title: "Test"} - err := typecheckActionArgument(&arg, "test", &action) - assert.NotNil(t, err) + err := typecheckActionArgument(&arg, "test") + require.Error(t, err) assert.Contains(t, err.Error(), "argument name cannot be empty") } @@ -940,17 +946,16 @@ func TestTypecheckActionArgumentConfirmation(t *testing.T) { Name: "confirm", Type: "confirmation", } - action := config.Action{Title: "Test"} - assert.Nil(t, typecheckActionArgument(&arg, "0", &action)) - assert.Nil(t, typecheckActionArgument(&arg, "1", &action)) + require.NoError(t, typecheckActionArgument(&arg, "0")) + require.NoError(t, typecheckActionArgument(&arg, "1")) - err := typecheckActionArgument(&arg, "any_value", &action) - assert.NotNil(t, err) + err := typecheckActionArgument(&arg, "any_value") + require.Error(t, err) assert.Contains(t, err.Error(), "must be \"0\" or \"1\"") - err = typecheckActionArgument(&arg, "", &action) - assert.NotNil(t, err) + err = typecheckActionArgument(&arg, "") + require.Error(t, err) assert.Contains(t, err.Error(), "must be \"0\" or \"1\"") } @@ -959,10 +964,9 @@ func TestTypecheckActionArgumentUnnamedConfirmation(t *testing.T) { Type: "confirmation", Title: "Are you sure?!", } - action := config.Action{Title: "Test"} - assert.Nil(t, typecheckActionArgument(&arg, "", &action)) - assert.Nil(t, typecheckActionArgument(&arg, "ignored", &action)) + require.NoError(t, typecheckActionArgument(&arg, "")) + require.NoError(t, typecheckActionArgument(&arg, "ignored")) } func TestTypecheckActionArgumentHtmlWithoutName(t *testing.T) { @@ -976,7 +980,7 @@ func TestTypecheckActionArgumentHtmlWithoutName(t *testing.T) { } err := validateArguments(map[string]string{}, &action) - assert.NoError(t, err) + require.NoError(t, err) } func TestParseCommandForReplacements(t *testing.T) { @@ -1038,12 +1042,12 @@ func TestParseCommandForReplacements(t *testing.T) { output, err := tpl.ParseTemplateWithActionContext(tt.shellCommand, nil, tt.values) if tt.expectError { - assert.NotNil(t, err, "Expected error but got none") + require.Error(t, err, "Expected error but got none") if tt.errorContains != "" { assert.Contains(t, err.Error(), tt.errorContains) } } else { - assert.Nil(t, err, "Expected no error but got: %v", err) + require.NoError(t, err, "Expected no error but got: %v", err) assert.Equal(t, tt.expectedOutput, output) } }) @@ -1136,10 +1140,10 @@ func TestArgumentChoicesValidation(t *testing.T) { _, err := parseActionArguments(tt.req) if tt.expectError { - assert.NotNil(t, err, tt.description) + require.Error(t, err, tt.description) assert.Contains(t, err.Error(), "predefined choices") } else { - assert.Nil(t, err, tt.description) + require.NoError(t, err, tt.description) } }) } @@ -1161,7 +1165,7 @@ func TestTypeSafetyCheckVeryDangerousRawString(t *testing.T) { for _, value := range tests { t.Run(fmt.Sprintf("Value: %s", value), func(t *testing.T) { err := TypeSafetyCheck("test", value, "very_dangerous_raw_string") - assert.Nil(t, err, "very_dangerous_raw_string should accept any value including: %s", value) + require.NoError(t, err, "very_dangerous_raw_string should accept any value including: %s", value) }) } } @@ -1186,7 +1190,7 @@ func TestParseActionArgumentsWithEntityPrefix(t *testing.T) { // Test with entity prefix output, err := parseActionArguments(req) - assert.Nil(t, err) + require.NoError(t, err) assert.Contains(t, output, "testuser") } @@ -1227,9 +1231,9 @@ func TestComplexRegexPatterns(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := typeSafetyCheckRegex("test", tt.value, tt.pattern) if tt.hasError { - assert.NotNil(t, err) + require.Error(t, err) } else { - assert.Nil(t, err) + require.NoError(t, err) } }) } diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 6b640cb..fcd4c18 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -17,10 +17,12 @@ import ( "context" "errors" "fmt" + "maps" "os" "os/exec" "path" "regexp" + "slices" "strings" "sync" "time" @@ -927,12 +929,7 @@ func keepArgument(name string, definedNames map[string]struct{}) bool { } func hasWebhookTag(req *ExecutionRequest) bool { - for _, tag := range req.Tags { - if tag == "webhook" { - return true - } - } - return false + return slices.Contains(req.Tags, "webhook") } var systemArgumentDefinitions = []config.ActionArgument{ @@ -946,9 +943,7 @@ func injectSystemArgs(req *ExecutionRequest) error { return err } - for name, value := range args { - req.Arguments[name] = value - } + maps.Copy(req.Arguments, args) return nil } diff --git a/service/internal/executor/executor_unix.go b/service/internal/executor/executor_unix.go index 1a54685..f6b9d25 100644 --- a/service/internal/executor/executor_unix.go +++ b/service/internal/executor/executor_unix.go @@ -1,5 +1,4 @@ //go:build !windows -// +build !windows package executor diff --git a/service/internal/servicehost/log_directory_test.go b/service/internal/servicehost/log_directory_test.go index 0c3c16f..9852066 100644 --- a/service/internal/servicehost/log_directory_test.go +++ b/service/internal/servicehost/log_directory_test.go @@ -13,7 +13,7 @@ func TestResolveLogDirectory(t *testing.T) { baseDir := filepath.Join(t.TempDir(), "OliveTin") absoluteDir := t.TempDir() - assert.Equal(t, "", resolveLogDirectory("", baseDir)) + assert.Empty(t, resolveLogDirectory("", baseDir)) assert.Equal(t, absoluteDir, resolveLogDirectory(absoluteDir, baseDir)) assert.Equal(t, filepath.Join(baseDir, "logs", "service"), resolveLogDirectory("./logs/service", baseDir)) assert.Equal(t, "logs/service", resolveLogDirectory("logs/service", "")) diff --git a/service/internal/servicehost/servicehost_nonwin.go b/service/internal/servicehost/servicehost_nonwin.go index d5fb296..0b85dcf 100644 --- a/service/internal/servicehost/servicehost_nonwin.go +++ b/service/internal/servicehost/servicehost_nonwin.go @@ -1,5 +1,4 @@ //go:build !windows -// +build !windows package servicehost diff --git a/service/internal/tpl/templates.go b/service/internal/tpl/templates.go index f2ca256..a37bd75 100644 --- a/service/internal/tpl/templates.go +++ b/service/internal/tpl/templates.go @@ -44,7 +44,7 @@ type generalTemplateContext struct { } type actionTemplateContext struct { - CurrentEntity interface{} + CurrentEntity any Arguments map[string]string // These are deliberately repeated because embedding structs diff --git a/service/internal/updatecheck/updateCheck.go b/service/internal/updatecheck/updateCheck.go index a0c6353..0019faa 100644 --- a/service/internal/updatecheck/updateCheck.go +++ b/service/internal/updatecheck/updateCheck.go @@ -90,7 +90,7 @@ func doRequest() string { defer cancel() - req, err := http.NewRequestWithContext(ctx, "GET", "http://update-check.olivetin.app/versions.json", nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://update-check.olivetin.app/versions.json", nil) if err != nil { log.Errorf("Update check failed %v", err) diff --git a/service/internal/webhooks/jsonpath.go b/service/internal/webhooks/jsonpath.go index bfeac08..ae1bc7b 100644 --- a/service/internal/webhooks/jsonpath.go +++ b/service/internal/webhooks/jsonpath.go @@ -8,11 +8,11 @@ import ( ) type JSONMatcher struct { - payload interface{} + payload any } func NewJSONMatcher(payload []byte) (*JSONMatcher, error) { - var data interface{} + var data any if err := json.Unmarshal(payload, &data); err != nil { return nil, err } @@ -60,6 +60,6 @@ func (m *JSONMatcher) ExtractValue(pathExpr string) (string, error) { return string(jsonBytes), nil } -func (m *JSONMatcher) GetPayload() interface{} { +func (m *JSONMatcher) GetPayload() any { return m.payload } diff --git a/service/internal/webhooks/matcher.go b/service/internal/webhooks/matcher.go index 94c64a8..9d8d512 100644 --- a/service/internal/webhooks/matcher.go +++ b/service/internal/webhooks/matcher.go @@ -123,8 +123,7 @@ func (m *WebhookMatcher) matchPathValue(matcher *JSONMatcher, jsonPath, expected } func (m *WebhookMatcher) compareValues(actual, expected string) bool { - if strings.HasPrefix(expected, "regex:") { - pattern := strings.TrimPrefix(expected, "regex:") + if pattern, hasRegex := strings.CutPrefix(expected, "regex:"); hasRegex { matched, err := regexp.MatchString(pattern, actual) if err != nil { log.WithFields(log.Fields{ diff --git a/service/main.go b/service/main.go index b5c7134..db149cd 100644 --- a/service/main.go +++ b/service/main.go @@ -158,7 +158,7 @@ func configPathExists(configPath string) bool { } func watchConfigFile(k *koanf.Koanf, f *file.File, configPath string) { - err := f.Watch(func(evt interface{}, err error) { + err := f.Watch(func(evt any, err error) { log.Infof("config file changed: %v", evt) errLoad := k.Load(f, yaml.Parser())