From b7868277f14248e21327e560ad04651702aad8ee Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 11 Jan 2024 17:25:56 +0100 Subject: [PATCH 1/4] Enable golangci linter gomnd --- .golangci.yaml | 7 ++ agent/rpc/auth_client_grpc.go | 2 +- agent/rpc/client_grpc.go | 4 +- agent/runner.go | 4 +- cli/deploy/deploy.go | 3 +- cli/exec/exec.go | 10 +-- cli/internal/util.go | 6 +- cli/pipeline/create.go | 6 +- cli/pipeline/list.go | 1 + cli/pipeline/logs.go | 6 +- cmd/agent/core/agent.go | 2 +- cmd/agent/core/health.go | 14 +-- pipeline/backend/kubernetes/kubernetes.go | 8 +- pipeline/const.go | 33 +++++++ .../metadata/drone_compatibility_test.go | 6 +- pipeline/frontend/metadata/environment.go | 2 +- pipeline/frontend/yaml/types/base/map.go | 8 +- pipeline/frontend/yaml/types/volume.go | 1 + server/api/login.go | 4 +- server/api/registry.go | 2 +- server/cache/membership.go | 1 + server/forge/bitbucket/bitbucket.go | 5 +- server/forge/bitbucket/fixtures/handler.go | 68 +++++++-------- server/forge/bitbucket/internal/client.go | 5 +- server/forge/gitea/fixtures/handler.go | 40 ++++----- server/forge/gitea/gitea.go | 4 +- server/forge/github/fixtures/handler.go | 14 +-- server/forge/gitlab/gitlab.go | 6 +- server/forge/refresh.go | 5 +- server/model/user.go | 4 +- server/pipeline/step_status.go | 5 +- server/pipeline/step_status_test.go | 9 +- server/plugins/config/http.go | 85 +++++++++++++++++++ server/queue/fifo.go | 2 + server/router/middleware/header/header.go | 2 +- server/router/middleware/session/agent.go | 8 +- server/router/middleware/session/user.go | 10 +-- server/services/environment/parse.go | 8 +- server/services/registry/filesystem.go | 8 +- server/services/utils/http.go | 2 +- shared/httputil/httputil.go | 3 +- 41 files changed, 281 insertions(+), 142 deletions(-) create mode 100644 pipeline/const.go create mode 100644 server/plugins/config/http.go diff --git a/.golangci.yaml b/.golangci.yaml index f3e00a7060..6070e87e0a 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -166,6 +166,13 @@ linters: - contextcheck - forcetypeassert - gci + - gomnd + +issues: + exclude-rules: + - path: 'cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go' + linters: + - gomnd run: timeout: 15m diff --git a/agent/rpc/auth_client_grpc.go b/agent/rpc/auth_client_grpc.go index 1ef645d431..94a01151e2 100644 --- a/agent/rpc/auth_client_grpc.go +++ b/agent/rpc/auth_client_grpc.go @@ -40,7 +40,7 @@ func NewAuthGrpcClient(conn *grpc.ClientConn, agentToken string, agentID int64) } func (c *AuthClient) Auth() (string, int64, error) { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint: gomnd defer cancel() req := &proto.AuthRequest{ diff --git a/agent/rpc/client_grpc.go b/agent/rpc/client_grpc.go index 39e7e2365a..5648d31cff 100644 --- a/agent/rpc/client_grpc.go +++ b/agent/rpc/client_grpc.go @@ -53,8 +53,8 @@ func (c *client) Close() error { func (c *client) newBackOff() backoff.BackOff { b := backoff.NewExponentialBackOff() - b.MaxInterval = 10 * time.Second - b.InitialInterval = 10 * time.Millisecond + b.MaxInterval = 10 * time.Second //nolint: gomnd + b.InitialInterval = 10 * time.Millisecond //nolint: gomnd return b } diff --git a/agent/runner.go b/agent/runner.go index 21413f9ef4..22a8729ef9 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -158,7 +158,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { if canceled.IsSet() { state.Error = "" - state.ExitCode = 137 + state.ExitCode = pipeline.ExitCodeKilled } else if err != nil { pExitError := &pipeline.ExitError{} switch { @@ -166,7 +166,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { state.ExitCode = pExitError.Code case errors.Is(err, pipeline.ErrCancel): state.Error = "" - state.ExitCode = 137 + state.ExitCode = pipeline.ExitCodeKilled canceled.SetTo(true) default: state.ExitCode = 1 diff --git a/cli/deploy/deploy.go b/cli/deploy/deploy.go index bcc2019a8d..9a9ffd32c5 100644 --- a/cli/deploy/deploy.go +++ b/cli/deploy/deploy.go @@ -106,7 +106,8 @@ func deploy(c *cli.Context) error { } } - env := c.Args().Get(2) + envArgIndex := 2 + env := c.Args().Get(envArgIndex) if env == "" { return fmt.Errorf("please specify the target environment (i.e. production)") } diff --git a/cli/exec/exec.go b/cli/exec/exec.go index aa13b37bfc..3808cc5891 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -123,13 +123,13 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error pipelineEnv := make(map[string]string) for _, env := range c.StringSlice("env") { - envs := strings.SplitN(env, "=", 2) - pipelineEnv[envs[0]] = envs[1] - if oldVar, exists := environ[envs[0]]; exists { + before, after, _ := strings.Cut(env, "=") + pipelineEnv[before] = after + if oldVar, exists := environ[before]; exists { // override existing values, but print a warning - log.Warn().Msgf("environment variable '%s' had value '%s', but got overwritten", envs[0], oldVar) + log.Warn().Msgf("environment variable '%s' had value '%s', but got overwritten", before, oldVar) } - environ[envs[0]] = envs[1] + environ[before] = after } tmpl, err := envsubst.ParseFile(file) diff --git a/cli/internal/util.go b/cli/internal/util.go index 47eff719ad..35a1d8da07 100644 --- a/cli/internal/util.go +++ b/cli/internal/util.go @@ -107,11 +107,11 @@ func ParseRepo(client woodpecker.Client, str string) (repoID int64, err error) { func ParseKeyPair(p []string) map[string]string { params := map[string]string{} for _, i := range p { - parts := strings.SplitN(i, "=", 2) - if len(parts) != 2 { + before, after, ok := strings.Cut(i, "=") + if !ok || before == "" { continue } - params[parts[0]] = parts[1] + params[before] = after } return params } diff --git a/cli/pipeline/create.go b/cli/pipeline/create.go index c64362bf86..3db896a9f1 100644 --- a/cli/pipeline/create.go +++ b/cli/pipeline/create.go @@ -60,9 +60,9 @@ func pipelineCreate(c *cli.Context) error { variables := make(map[string]string) for _, vaz := range c.StringSlice("var") { - sp := strings.SplitN(vaz, "=", 2) - if len(sp) == 2 { - variables[sp[0]] = sp[1] + before, after, _ := strings.Cut(vaz, "=") + if before != "" && after != "" { + variables[before] = after } } diff --git a/cli/pipeline/list.go b/cli/pipeline/list.go index 5a2fbaf92d..5cac3aa4d8 100644 --- a/cli/pipeline/list.go +++ b/cli/pipeline/list.go @@ -24,6 +24,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/cli/internal" ) +//nolint:gomnd var pipelineListCmd = &cli.Command{ Name: "ls", Usage: "show pipeline history", diff --git a/cli/pipeline/logs.go b/cli/pipeline/logs.go index 8951b20521..116d68a4d2 100644 --- a/cli/pipeline/logs.go +++ b/cli/pipeline/logs.go @@ -41,12 +41,14 @@ func pipelineLogs(c *cli.Context) error { return err } - number, err := strconv.ParseInt(c.Args().Get(1), 10, 64) + numberArgIndex := 1 + number, err := strconv.ParseInt(c.Args().Get(numberArgIndex), 10, 64) if err != nil { return err } - step, err := strconv.ParseInt(c.Args().Get(2), 10, 64) + stepArgIndex := 2 + step, err := strconv.ParseInt(c.Args().Get(stepArgIndex), 10, 64) if err != nil { return err } diff --git a/cmd/agent/core/agent.go b/cmd/agent/core/agent.go index 8a1c972cdd..02d2f21f31 100644 --- a/cmd/agent/core/agent.go +++ b/cmd/agent/core/agent.go @@ -89,7 +89,7 @@ func run(c *cli.Context, backends []types.Backend) error { agentToken := c.String("grpc-token") authClient := agentRpc.NewAuthGrpcClient(authConn, agentToken, agentConfig.AgentID) - authInterceptor, err := agentRpc.NewAuthInterceptor(authClient, 30*time.Minute) + authInterceptor, err := agentRpc.NewAuthInterceptor(authClient, 30*time.Minute) //nolint: gomnd if err != nil { return err } diff --git a/cmd/agent/core/health.go b/cmd/agent/core/health.go index 80d5135eaf..c05765bcc0 100644 --- a/cmd/agent/core/health.go +++ b/cmd/agent/core/health.go @@ -39,14 +39,14 @@ func initHealth() { func handleHeartbeat(w http.ResponseWriter, _ *http.Request) { if counter.Healthy() { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } } func handleVersion(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Header().Add("Content-Type", "text/json") err := json.NewEncoder(w).Encode(versionResp{ Source: "https://github.com/woodpecker-ci/woodpecker", @@ -59,9 +59,9 @@ func handleVersion(w http.ResponseWriter, _ *http.Request) { func handleStats(w http.ResponseWriter, _ *http.Request) { if counter.Healthy() { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } w.Header().Add("Content-Type", "text/json") if _, err := counter.WriteTo(w); err != nil { @@ -92,8 +92,8 @@ func pinger(c *cli.Context) error { return err } defer resp.Body.Close() - if resp.StatusCode != 200 { - return fmt.Errorf("agent returned non-200 status code") + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("agent returned non-http.StatusOK status code") } return nil } diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 69b2a7ddea..3567100a6d 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -42,6 +42,8 @@ import ( const ( EngineName = "kubernetes" + // TODO 5 seconds is against best practice, k3s didn't work otherwise + defaultResyncDuration = 5 * time.Second ) var defaultDeleteOptions = newDefaultDeleteOptions() @@ -249,8 +251,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } } - // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) + si := informers.NewSharedInformerFactoryWithOptions(e.client, defaultResyncDuration, informers.WithNamespace(e.config.Namespace)) if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, @@ -322,8 +323,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) } } - // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) + si := informers.NewSharedInformerFactoryWithOptions(e.client, defaultResyncDuration, informers.WithNamespace(e.config.Namespace)) if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, diff --git a/pipeline/const.go b/pipeline/const.go new file mode 100644 index 0000000000..be0592f01f --- /dev/null +++ b/pipeline/const.go @@ -0,0 +1,33 @@ +// Copyright 2023 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pipeline + +// StatusValue represent pipeline states woodpecker know +type StatusValue string // @name StatusValue + +const ( + StatusSkipped StatusValue = "skipped" // skipped as another step failed + StatusPending StatusValue = "pending" // pending to be executed + StatusRunning StatusValue = "running" // currently running + StatusSuccess StatusValue = "success" // successfully finished + StatusFailure StatusValue = "failure" // failed to finish (exit code != 0) + StatusKilled StatusValue = "killed" // killed by user + StatusError StatusValue = "error" // error with the config / while parsing / some other system problem + StatusBlocked StatusValue = "blocked" // waiting for approval + StatusDeclined StatusValue = "declined" // blocked and declined + StatusCreated StatusValue = "created" // created / internal use only +) + +const ExitCodeKilled int = 137 diff --git a/pipeline/frontend/metadata/drone_compatibility_test.go b/pipeline/frontend/metadata/drone_compatibility_test.go index 292d45e679..c2334585c2 100644 --- a/pipeline/frontend/metadata/drone_compatibility_test.go +++ b/pipeline/frontend/metadata/drone_compatibility_test.go @@ -113,11 +113,11 @@ DRONE_TARGET_BRANCH=main` func convertListToEnvMap(t *testing.T, list string) map[string]string { result := make(map[string]string) for _, s := range strings.Split(list, "\n") { - ss := strings.SplitN(strings.TrimSpace(s), "=", 2) - if len(ss) != 2 { + before, after, _ := strings.Cut(strings.TrimSpace(s), "=") + if before == "" || after == "" { t.Fatal("helper function got invalid test data") } - result[ss[0]] = ss[1] + result[before] = after } return result } diff --git a/pipeline/frontend/metadata/environment.go b/pipeline/frontend/metadata/environment.go index d63eae0189..d32378d024 100644 --- a/pipeline/frontend/metadata/environment.go +++ b/pipeline/frontend/metadata/environment.go @@ -38,7 +38,7 @@ func (m *Metadata) Environ() map[string]string { ) branchParts := strings.Split(m.Curr.Commit.Refspec, ":") - if len(branchParts) == 2 { + if len(branchParts) == 2 { //nolint: gomnd sourceBranch = branchParts[0] targetBranch = branchParts[1] } diff --git a/pipeline/frontend/yaml/types/base/map.go b/pipeline/frontend/yaml/types/base/map.go index 849adc96d0..daa83a44f8 100644 --- a/pipeline/frontend/yaml/types/base/map.go +++ b/pipeline/frontend/yaml/types/base/map.go @@ -31,13 +31,7 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { for _, s := range sliceType { if str, ok := s.(string); ok { str := strings.TrimSpace(str) - keyValueSlice := strings.SplitN(str, "=", 2) - - key := keyValueSlice[0] - val := "" - if len(keyValueSlice) == 2 { - val = keyValueSlice[1] - } + key, val, _ := strings.Cut(str, "=") parts[key] = val } else { return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", s, s) diff --git a/pipeline/frontend/yaml/types/volume.go b/pipeline/frontend/yaml/types/volume.go index d07a3eec60..3d925e25b3 100644 --- a/pipeline/frontend/yaml/types/volume.go +++ b/pipeline/frontend/yaml/types/volume.go @@ -68,6 +68,7 @@ func (v *Volumes) UnmarshalYAML(unmarshal func(any) error) error { } elts := strings.SplitN(name, ":", 3) var vol *Volume + //nolint: gomnd switch { case len(elts) == 1: vol = &Volume{ diff --git a/server/api/login.go b/server/api/login.go index 8e72285dcf..4daea4a572 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -86,8 +86,8 @@ func HandleAuth(c *gin.Context) { if server.Config.Permissions.Orgs.IsConfigured { teams, terr := _forge.Teams(c, tmpuser) if terr != nil || !server.Config.Permissions.Orgs.IsMember(teams) { - log.Error().Err(terr).Msgf("cannot verify team membership for %s", u.Login) - c.Redirect(303, server.Config.Server.RootPath+"/login?error=access_denied") + log.Error().Err(terr).Msgf("cannot verify team membership for %s.", u.Login) + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied") return } } diff --git a/server/api/registry.go b/server/api/registry.go index 60e6648ff8..be2cc6b76d 100644 --- a/server/api/registry.go +++ b/server/api/registry.go @@ -44,7 +44,7 @@ func GetRegistry(c *gin.Context) { handleDBError(c, err) return } - c.JSON(200, registry.Copy()) + c.JSON(http.StatusOK, registry.Copy()) } // PostRegistry diff --git a/server/cache/membership.go b/server/cache/membership.go index 3c73c12e03..45b6af5d1e 100644 --- a/server/cache/membership.go +++ b/server/cache/membership.go @@ -39,6 +39,7 @@ type membershipCache struct { // NewMembershipService creates a new membership service. func NewMembershipService(f forge.Forge) MembershipService { + //nolint:gomnd return &membershipCache{ ttl: 10 * time.Minute, forge: f, diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index 095836be90..c1590fde73 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -38,6 +38,7 @@ import ( const ( DefaultAPI = "https://api.bitbucket.org" DefaultURL = "https://bitbucket.org" + pageSize = 100 ) // Opts are forge options for bitbucket @@ -141,7 +142,7 @@ func (c *config) Refresh(ctx context.Context, user *model.User) (bool, error) { func (c *config) Teams(ctx context.Context, u *model.User) ([]*model.Team, error) { return shared_utils.Paginate(func(page int) ([]*model.Team, error) { opts := &internal.ListWorkspacesOpts{ - PageLen: 100, + PageLen: pageSize, Page: page, Role: "member", } @@ -190,7 +191,7 @@ func (c *config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error workspaces, err := shared_utils.Paginate(func(page int) ([]*internal.Workspace, error) { resp, err := client.ListWorkspaces(&internal.ListWorkspacesOpts{ Page: page, - PageLen: 100, + PageLen: pageSize, Role: "member", }) if err != nil { diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go index a2a42d8aa5..73aa6a66a9 100644 --- a/server/forge/bitbucket/fixtures/handler.go +++ b/server/forge/bitbucket/fixtures/handler.go @@ -45,26 +45,26 @@ func Handler() http.Handler { func getOauth(c *gin.Context) { if c.PostForm("error") == "invalid_scope" { - c.String(500, "") + c.String(http.StatusInternalServerError, "") } switch c.PostForm("code") { case "code_bad_request": - c.String(500, "") + c.String(http.StatusInternalServerError, "") return case "code_user_not_found": - c.String(200, tokenNotFoundPayload) + c.String(http.StatusOK, tokenNotFoundPayload) return } switch c.PostForm("refresh_token") { case "refresh_token_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "refresh_token_is_empty": c.Header("Content-Type", "application/json") - c.String(200, "{}") + c.String(http.StatusOK, "{}") default: c.Header("Content-Type", "application/json") - c.String(200, tokenPayload) + c.String(http.StatusOK, tokenPayload) } } @@ -74,12 +74,12 @@ func getWorkspaces(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer teams_not_found", "Bearer c81e728d": - c.String(404, "") + c.String(http.StatusNotFound, "") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, workspacesPayload) + c.String(http.StatusOK, workspacesPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -87,25 +87,25 @@ func getWorkspaces(c *gin.Context) { func getRepo(c *gin.Context) { switch c.Param("name") { case "not_found", "repo_unknown", "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "permission_read", "permission_write", "permission_admin": - c.String(200, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) + c.String(http.StatusOK, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoHooks(c *gin.Context) { switch c.Param("name") { case "hooks_not_found", "repo_no_hooks": - c.String(404, "") + c.String(http.StatusNotFound, "") case "hook_empty": - c.String(200, "{}") + c.String(http.StatusOK, "{}") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, repoHookPayload) + c.String(http.StatusOK, repoHookPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -113,74 +113,74 @@ func getRepoHooks(c *gin.Context) { func getRepoFile(c *gin.Context) { switch c.Param("file") { case "dir": - c.String(200, repoDirPayload) + c.String(http.StatusOK, repoDirPayload) case "dir_not_found/": - c.String(404, "") + c.String(http.StatusNotFound, "") case "file_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoFilePayload) + c.String(http.StatusOK, repoFilePayload) } } func getBranchHead(c *gin.Context) { switch c.Param("commit") { case "branch_name": - c.String(200, branchCommitsPayload) + c.String(http.StatusOK, branchCommitsPayload) default: - c.String(404, "") + c.String(http.StatusNotFound, "") } } func getPullRequests(c *gin.Context) { switch c.Param("name") { case "repo_name": - c.String(200, pullRequestsPayload) + c.String(http.StatusOK, pullRequestsPayload) default: - c.String(404, "") + c.String(http.StatusNotFound, "") } } func createRepoStatus(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, "") + c.String(http.StatusOK, "") } } func createRepoHook(c *gin.Context) { - c.String(200, "") + c.String(http.StatusOK, "") } func deleteRepoHook(c *gin.Context) { switch c.Param("name") { case "hook_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, "") + c.String(http.StatusOK, "") } } func getUser(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer user_not_found", "Bearer a87ff679": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, userPayload) + c.String(http.StatusOK, userPayload) } } func getUserRepos(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer repos_not_found", "Bearer 70efdf2e": - c.String(404, "") + c.String(http.StatusNotFound, "") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, userRepoPayload) + c.String(http.StatusOK, userRepoPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go index 779029ca9c..2d420e738c 100644 --- a/server/forge/bitbucket/internal/client.go +++ b/server/forge/bitbucket/internal/client.go @@ -53,6 +53,7 @@ const ( pathPullRequests = "%s/2.0/repositories/%s/%s/pullrequests?%s" pathBranchCommits = "%s/2.0/repositories/%s/%s/commits/%s" pathDir = "%s/2.0/repositories/%s/%s/src/%s%s" + pageSize = 100 ) type Client struct { @@ -115,7 +116,7 @@ func (c *Client) ListRepos(workspace string, opts *ListOpts) (*RepoResp, error) func (c *Client) ListReposAll(workspace string) ([]*Repo, error) { return shared_utils.Paginate(func(page int) ([]*Repo, error) { - resp, err := c.ListRepos(workspace, &ListOpts{Page: page, PageLen: 100}) + resp, err := c.ListRepos(workspace, &ListOpts{Page: page, PageLen: pageSize}) if err != nil { return nil, err } @@ -213,7 +214,7 @@ func (c *Client) GetBranchHead(owner, name, branch string) (*Commit, error) { func (c *Client) GetUserWorkspaceMembership(workspace, user string) (string, error) { out := new(WorkspaceMembershipResp) - opts := &ListOpts{Page: 1, PageLen: 100} + opts := &ListOpts{Page: 1, PageLen: pageSize} for { uri := fmt.Sprintf(pathOrgPerms, c.base, workspace, opts.Encode()) _, err := c.do(uri, get, nil, out) diff --git a/server/forge/gitea/fixtures/handler.go b/server/forge/gitea/fixtures/handler.go index d6ff48fa1a..7bc2bbb42a 100644 --- a/server/forge/gitea/fixtures/handler.go +++ b/server/forge/gitea/fixtures/handler.go @@ -43,35 +43,35 @@ func Handler() http.Handler { func listRepoHooks(c *gin.Context) { page := c.Query("page") if page != "" && page != "1" { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } else { - c.String(200, listRepoHookPayloads) + c.String(http.StatusOK, listRepoHookPayloads) } } func getRepo(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoByID(c *gin.Context) { switch c.Param("id") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func createRepoCommitStatus(c *gin.Context) { if c.Param("commit") == "v1.0.0" || c.Param("commit") == "9ecad50" { - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } - c.String(404, "") + c.String(http.StatusNotFound, "") } func getRepoFile(c *gin.Context) { @@ -79,12 +79,12 @@ func getRepoFile(c *gin.Context) { ref := c.Query("ref") if file == "file_not_found" { - c.String(404, "") + c.String(http.StatusNotFound, "") } if ref == "v1.0.0" || ref == "9ecad50" { - c.String(200, repoFilePayload) + c.String(http.StatusOK, repoFilePayload) } - c.String(404, "") + c.String(http.StatusNotFound, "") } func createRepoHook(c *gin.Context) { @@ -99,41 +99,41 @@ func createRepoHook(c *gin.Context) { if in.Type != "gitea" || in.Conf.Type != "json" || in.Conf.URL != "http://localhost" { - c.String(500, "") + c.String(http.StatusInternalServerError, "") return } - c.String(200, "{}") + c.String(http.StatusOK, "{}") } func deleteRepoHook(c *gin.Context) { - c.String(200, "{}") + c.String(http.StatusOK, "{}") } func getUserRepos(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "token repos_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: page := c.Query("page") if page != "" && page != "1" { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } else { - c.String(200, userRepoPayload) + c.String(http.StatusOK, userRepoPayload) } } } func getVersion(c *gin.Context) { - c.JSON(200, map[string]any{"version": "1.18.0"}) + c.JSON(http.StatusOK, map[string]any{"version": "1.18.0"}) } func getPRFiles(c *gin.Context) { page := c.Query("page") if page == "1" { - c.String(200, prFilesPayload) + c.String(http.StatusOK, prFilesPayload) } else { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } } diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index 59907e9480..eba7baabff 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -399,10 +399,10 @@ func (c *Gitea) Activate(ctx context.Context, u *model.User, r *model.Repo, link _, response, err := client.CreateRepoHook(r.Owner, r.Name, hook) if err != nil { if response != nil { - if response.StatusCode == 404 { + if response.StatusCode == http.StatusNotFound { return fmt.Errorf("could not find repository") } - if response.StatusCode == 200 { + if response.StatusCode == http.StatusOK { return fmt.Errorf("could not find repository, repository was probably renamed") } } diff --git a/server/forge/github/fixtures/handler.go b/server/forge/github/fixtures/handler.go index 320805a517..0f02557053 100644 --- a/server/forge/github/fixtures/handler.go +++ b/server/forge/github/fixtures/handler.go @@ -37,29 +37,29 @@ func Handler() http.Handler { func getRepo(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoByID(c *gin.Context) { switch c.Param("id") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getMembership(c *gin.Context) { switch c.Param("org") { case "org_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "github": - c.String(200, membershipIsMemberPayload) + c.String(http.StatusOK, membershipIsMemberPayload) default: - c.String(200, membershipIsOwnerPayload) + c.String(http.StatusOK, membershipIsOwnerPayload) } } diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index 008d037a19..79c51462b4 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -544,7 +544,7 @@ func (g *GitLab) Deactivate(ctx context.Context, user *model.User, repo *model.R hookID := -1 listProjectHooksOptions := &gitlab.ListProjectHooksOptions{ - PerPage: 10, + PerPage: perPage, Page: 1, } for { @@ -685,7 +685,7 @@ func (g *GitLab) OrgMembership(ctx context.Context, u *model.User, owner string) groups, _, err := client.Groups.ListGroups(&gitlab.ListGroupsOptions{ ListOptions: gitlab.ListOptions{ Page: 1, - PerPage: 100, + PerPage: perPage, }, Search: gitlab.Ptr(owner), }, gitlab.WithContext(ctx)) @@ -706,7 +706,7 @@ func (g *GitLab) OrgMembership(ctx context.Context, u *model.User, owner string) opts := &gitlab.ListGroupMembersOptions{ ListOptions: gitlab.ListOptions{ Page: 1, - PerPage: 100, + PerPage: perPage, }, } diff --git a/server/forge/refresh.go b/server/forge/refresh.go index dd904c91f5..931b67d6e8 100644 --- a/server/forge/refresh.go +++ b/server/forge/refresh.go @@ -32,11 +32,14 @@ type Refresher interface { } func Refresh(c context.Context, forge Forge, _store store.Store, user *model.User) { + // Remaining ttl of 30 minutes (1800 seconds) until a token is refreshed. + const tokenMinTTL = 1800 + if refresher, ok := forge.(Refresher); ok { // Check to see if the user token is expired or // will expire within the next 30 minutes (1800 seconds). // If not, there is nothing we really need to do here. - if time.Now().UTC().Unix() < (user.Expiry - 1800) { + if time.Now().UTC().Unix() < (user.Expiry - tokenMinTTL) { return } diff --git a/server/model/user.go b/server/model/user.go index 73eca72634..152df68ac3 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -25,6 +25,8 @@ var reUsername = regexp.MustCompile("^[a-zA-Z0-9-_.]+$") var errUserLoginInvalid = errors.New("invalid user login") +const maxLoginLen = 250 + // User represents a registered user. type User struct { // the id for this user. @@ -79,7 +81,7 @@ func (u *User) Validate() error { switch { case len(u.Login) == 0: return errUserLoginInvalid - case len(u.Login) > 250: + case len(u.Login) > maxLoginLen: return errUserLoginInvalid case !reUsername.MatchString(u.Login): return errUserLoginInvalid diff --git a/server/pipeline/step_status.go b/server/pipeline/step_status.go index 8d0c1d538f..4fb869fccd 100644 --- a/server/pipeline/step_status.go +++ b/server/pipeline/step_status.go @@ -18,6 +18,7 @@ package pipeline import ( "time" + "go.woodpecker-ci.org/woodpecker/v2/pipeline" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/store" @@ -32,7 +33,7 @@ func UpdateStepStatus(store store.Store, step *model.Step, state rpc.State) erro if state.ExitCode != 0 || state.Error != "" { step.State = model.StatusFailure } - if state.ExitCode == 137 { + if state.ExitCode == pipeline.ExitCodeKilled { step.State = model.StatusKilled } } else if step.Stopped == 0 { @@ -78,6 +79,6 @@ func UpdateStepToStatusKilled(store store.Store, step model.Step) (*model.Step, if step.Started == 0 { step.Started = step.Stopped } - step.ExitCode = 137 + step.ExitCode = pipeline.ExitCodeKilled return &step, store.StepUpdate(&step) } diff --git a/server/pipeline/step_status_test.go b/server/pipeline/step_status_test.go index b0ecd85f44..9ac124d994 100644 --- a/server/pipeline/step_status_test.go +++ b/server/pipeline/step_status_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "go.woodpecker-ci.org/woodpecker/v2/pipeline" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/store" @@ -45,7 +46,7 @@ func TestUpdateStepStatusNotExited(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: 137, + ExitCode: pipeline.ExitCodeKilled, Error: "not an error", } @@ -69,7 +70,7 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: 137, + ExitCode: pipeline.ExitCodeKilled, Error: "not an error", } @@ -93,7 +94,7 @@ func TestUpdateStepStatusExited(t *testing.T) { Started: int64(42), Exited: true, Finished: int64(34), - ExitCode: 137, + ExitCode: pipeline.ExitCodeKilled, Error: "an error", } @@ -102,7 +103,7 @@ func TestUpdateStepStatusExited(t *testing.T) { assert.EqualValues(t, model.StatusKilled, step.State) assert.EqualValues(t, 42, step.Started) assert.EqualValues(t, 34, step.Stopped) - assert.EqualValues(t, 137, step.ExitCode) + assert.EqualValues(t, pipeline.ExitCodeKilled, step.ExitCode) assert.EqualValues(t, "an error", step.Error) } diff --git a/server/plugins/config/http.go b/server/plugins/config/http.go new file mode 100644 index 0000000000..e3a393ff7d --- /dev/null +++ b/server/plugins/config/http.go @@ -0,0 +1,85 @@ +// Copyright 2022 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "context" + "crypto" + "fmt" + + httpStatus "net/http" + + forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" + "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/plugins/utils" +) + +type http struct { + endpoint string + privateKey crypto.PrivateKey +} + +// Same as forge.FileMeta but with json tags and string data +type config struct { + Name string `json:"name"` + Data string `json:"data"` +} + +type requestStructure struct { + Repo *model.Repo `json:"repo"` + Pipeline *model.Pipeline `json:"pipeline"` + Configuration []*config `json:"configs"` + Netrc *model.Netrc `json:"netrc"` +} + +type responseStructure struct { + Configs []config `json:"configs"` +} + +func NewHTTP(endpoint string, privateKey crypto.PrivateKey) Extension { + return &http{endpoint, privateKey} +} + +func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta, netrc *model.Netrc) (configData []*forge_types.FileMeta, useOld bool, err error) { + currentConfigs := make([]*config, len(currentFileMeta)) + for i, pipe := range currentFileMeta { + currentConfigs[i] = &config{Name: pipe.Name, Data: string(pipe.Data)} + } + + response := new(responseStructure) + body := requestStructure{ + Repo: repo, + Pipeline: pipeline, + Configuration: currentConfigs, + Netrc: netrc, + } + + status, err := utils.Send(ctx, "POST", cp.endpoint, cp.privateKey, body, response) + if err != nil && status != 204 { + return nil, false, fmt.Errorf("failed to fetch config via http (%d) %w", status, err) + } + + var newFileMeta []*forge_types.FileMeta + if status != httpStatus.StatusOK { + newFileMeta = make([]*forge_types.FileMeta, 0) + } else { + newFileMeta = make([]*forge_types.FileMeta, len(response.Configs)) + for i, pipe := range response.Configs { + newFileMeta[i] = &forge_types.FileMeta{Name: pipe.Name, Data: []byte(pipe.Data)} + } + } + + return newFileMeta, status == httpStatus.StatusNoContent, nil +} diff --git a/server/queue/fifo.go b/server/queue/fifo.go index 29e24d1035..476df8eba9 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -52,6 +52,8 @@ type fifo struct { } // New returns a new fifo queue. +// +//nolint:gomnd func New(_ context.Context) Queue { return &fifo{ workers: map[*worker]struct{}{}, diff --git a/server/router/middleware/header/header.go b/server/router/middleware/header/header.go index 571354604f..663bf451db 100644 --- a/server/router/middleware/header/header.go +++ b/server/router/middleware/header/header.go @@ -42,7 +42,7 @@ func Options(c *gin.Context) { c.Header("Access-Control-Allow-Headers", "authorization, origin, content-type, accept") c.Header("Allow", "HEAD,GET,POST,PUT,PATCH,DELETE,OPTIONS") c.Header("Content-Type", "application/json") - c.AbortWithStatus(200) + c.AbortWithStatus(http.StatusOK) } } diff --git a/server/router/middleware/session/agent.go b/server/router/middleware/session/agent.go index 3f96a57c07..cacaf0d44a 100644 --- a/server/router/middleware/session/agent.go +++ b/server/router/middleware/session/agent.go @@ -16,6 +16,8 @@ package session import ( + "net/http" + "github.com/gin-gonic/gin" "go.woodpecker-ci.org/woodpecker/v2/shared/token" @@ -25,7 +27,7 @@ import ( func AuthorizeAgent(c *gin.Context) { secret, _ := c.MustGet("agent").(string) if secret == "" { - c.String(401, "invalid or empty token.") + c.String(http.StatusUnauthorized, "invalid or empty token.") return } @@ -34,10 +36,10 @@ func AuthorizeAgent(c *gin.Context) { }) switch { case err != nil: - c.String(500, "invalid or empty token. %s", err) + c.String(http.StatusInternalServerError, "invalid or empty token. %s", err) c.Abort() case parsed.Kind != token.AgentToken: - c.String(403, "invalid token. please use an agent token") + c.String(http.StatusForbidden, "invalid token. please use an agent token") c.Abort() default: c.Next() diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index a622f9c92f..b0c1b71762 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -75,10 +75,10 @@ func MustAdmin() gin.HandlerFunc { user := User(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() case !user.Admin: - c.String(403, "User not authorized") + c.String(http.StatusForbidden, "User not authorized") c.Abort() default: c.Next() @@ -92,10 +92,10 @@ func MustRepoAdmin() gin.HandlerFunc { perm := Perm(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() case !perm.Admin: - c.String(403, "User not authorized") + c.String(http.StatusForbidden, "User not authorized") c.Abort() default: c.Next() @@ -108,7 +108,7 @@ func MustUser() gin.HandlerFunc { user := User(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() default: c.Next() diff --git a/server/services/environment/parse.go b/server/services/environment/parse.go index c1a6b8c878..0a1ccb0c99 100644 --- a/server/services/environment/parse.go +++ b/server/services/environment/parse.go @@ -31,13 +31,13 @@ func Parse(params []string) Service { var globals []*model.Environ for _, item := range params { - kvPair := strings.SplitN(item, ":", 2) - if len(kvPair) != 2 { + before, after, _ := strings.Cut(item, ":") + if after == "" { // ignore items only containing a key and no value - log.Warn().Msgf("key '%s' has no value, will be ignored", kvPair[0]) + log.Warn().Msgf("key '%s' has no value, will be ignored", before) continue } - globals = append(globals, &model.Environ{Name: kvPair[0], Value: kvPair[1]}) + globals = append(globals, &model.Environ{Name: before, Value: after}) } return &builtin{globals} } diff --git a/server/services/registry/filesystem.go b/server/services/registry/filesystem.go index 4d98818cc0..6d70062746 100644 --- a/server/services/registry/filesystem.go +++ b/server/services/registry/filesystem.go @@ -113,10 +113,10 @@ func decodeAuth(authStr string) (string, string, error) { if n > decLen { return "", "", fmt.Errorf("something went wrong decoding auth config") } - arr := strings.SplitN(string(decoded), ":", 2) - if len(arr) != 2 { + before, after, _ := strings.Cut(string(decoded), ":") + if before == "" || after == "" { return "", "", fmt.Errorf("invalid auth configuration file") } - password := strings.Trim(arr[1], "\x00") - return arr[0], password, nil + password := strings.Trim(after, "\x00") + return before, password, nil } diff --git a/server/services/utils/http.go b/server/services/utils/http.go index 7282340129..c11bfe27f0 100644 --- a/server/services/utils/http.go +++ b/server/services/utils/http.go @@ -65,7 +65,7 @@ func Send(ctx context.Context, method, path string, privateKey crypto.PrivateKey } defer resp.Body.Close() - if resp.StatusCode != 200 { + if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { return resp.StatusCode, err diff --git a/shared/httputil/httputil.go b/shared/httputil/httputil.go index 11c9a3fe9c..ce44152d53 100644 --- a/shared/httputil/httputil.go +++ b/shared/httputil/httputil.go @@ -15,6 +15,7 @@ package httputil import ( + "math" "net/http" "strings" ) @@ -47,7 +48,7 @@ func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { Domain: r.URL.Host, HttpOnly: true, Secure: IsHTTPS(r), - MaxAge: 2147483647, // the cooke value (token) is responsible for expiration + MaxAge: math.MaxInt32, // the cookie value (token) is responsible for expiration } http.SetCookie(w, &cookie) From e861b9ddeaad41498f834e43e84d87628216fcd1 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 11 Jan 2024 17:34:49 +0100 Subject: [PATCH 2/4] cleanup --- pipeline/const.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/pipeline/const.go b/pipeline/const.go index be0592f01f..a4bec9789b 100644 --- a/pipeline/const.go +++ b/pipeline/const.go @@ -14,20 +14,4 @@ package pipeline -// StatusValue represent pipeline states woodpecker know -type StatusValue string // @name StatusValue - -const ( - StatusSkipped StatusValue = "skipped" // skipped as another step failed - StatusPending StatusValue = "pending" // pending to be executed - StatusRunning StatusValue = "running" // currently running - StatusSuccess StatusValue = "success" // successfully finished - StatusFailure StatusValue = "failure" // failed to finish (exit code != 0) - StatusKilled StatusValue = "killed" // killed by user - StatusError StatusValue = "error" // error with the config / while parsing / some other system problem - StatusBlocked StatusValue = "blocked" // waiting for approval - StatusDeclined StatusValue = "declined" // blocked and declined - StatusCreated StatusValue = "created" // created / internal use only -) - const ExitCodeKilled int = 137 From 629e1e3f7c0cb4ae5be2eeeb8595de76fbfaa10f Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sat, 13 Jan 2024 15:01:22 +0100 Subject: [PATCH 3/4] use cut instead of split --- server/model/repo.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/model/repo.go b/server/model/repo.go index 34d50bb508..b99c66b15c 100644 --- a/server/model/repo.go +++ b/server/model/repo.go @@ -65,13 +65,13 @@ func (r *Repo) ResetVisibility() { // ParseRepo parses the repository owner and name from a string. func ParseRepo(str string) (user, repo string, err error) { - parts := strings.Split(str, "/") - if len(parts) != 2 { - err = fmt.Errorf("error: Invalid or missing repository. eg octocat/hello-world") + before, after, _ := strings.Cut(str, "/") + if before == "" || after == "" { + err = fmt.Errorf("invalid or missing repository (e.g. octocat/hello-world)") return } - user = parts[0] - repo = parts[1] + user = before + repo = after return } From 4c6c54c100d110f2001c8fba8186ccfd92e9b482 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Fri, 15 Mar 2024 08:38:36 +0100 Subject: [PATCH 4/4] fix missing --- .golangci.yaml | 2 +- cli/exec/exec.go | 8 ++ cli/setup/token_fetcher.go | 15 ++-- cmd/agent/core/agent.go | 12 +-- cmd/agent/core/flags.go | 1 + pipeline/backend/docker/convert.go | 23 +++-- pipeline/shared/replace_secrets.go | 12 ++- server/forge/bitbucket/fixtures/handler.go | 4 +- server/forge/bitbucket/internal/client.go | 2 +- .../bitbucketdatacenter.go | 8 +- server/forge/bitbucketdatacenter/convert.go | 5 +- server/forge/github/github.go | 2 + server/forge/gitlab/convert.go | 11 ++- server/plugins/config/http.go | 85 ------------------- server/services/config/http.go | 3 +- 15 files changed, 78 insertions(+), 115 deletions(-) delete mode 100644 server/plugins/config/http.go diff --git a/.golangci.yaml b/.golangci.yaml index 6070e87e0a..703304a524 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -170,7 +170,7 @@ linters: issues: exclude-rules: - - path: 'cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go' + - path: 'fixtures|cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go' linters: - gomnd diff --git a/cli/exec/exec.go b/cli/exec/exec.go index 3808cc5891..44dabe116e 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -244,8 +244,16 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error ).Run(c.Context) } +// convertPathForWindows converts a path to use slash separators +// for Windows. If the path is a Windows volume name like C:, it +// converts it to an absolute root path starting with slash (e.g. +// C: -> /c). Otherwise it just converts backslash separators to +// slashes. func convertPathForWindows(path string) string { base := filepath.VolumeName(path) + + // Check if path is volume name like C: + //nolint: gomnd if len(base) == 2 { path = strings.TrimPrefix(path, base) base = strings.ToLower(strings.TrimSuffix(base, ":")) diff --git a/cli/setup/token_fetcher.go b/cli/setup/token_fetcher.go index 54dd18eee3..e96772cac5 100644 --- a/cli/setup/token_fetcher.go +++ b/cli/setup/token_fetcher.go @@ -61,7 +61,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine { c.Writer.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS, GET, PUT") if c.Request.Method == "OPTIONS" { - c.AbortWithStatus(204) + c.AbortWithStatus(http.StatusNoContent) return } @@ -76,7 +76,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine { err := c.BindJSON(&data) if err != nil { log.Debug().Err(err).Msg("Failed to bind JSON") - c.JSON(400, gin.H{ + c.JSON(http.StatusBadRequest, gin.H{ "error": "invalid request", }) return @@ -84,7 +84,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine { tokenReceived <- data.Token - c.JSON(200, gin.H{ + c.JSON(http.StatusOK, gin.H{ "ok": "true", }) }) @@ -111,7 +111,10 @@ func openBrowser(url string) error { } func randomPort() int { - s1 := rand.NewSource(time.Now().UnixNano()) - r1 := rand.New(s1) - return r1.Intn(10000) + 20000 + const minPort = 10000 + const maxPort = 65535 + + source := rand.NewSource(time.Now().UnixNano()) + rand := rand.New(source) + return rand.Intn(maxPort-minPort+1) + minPort } diff --git a/cmd/agent/core/agent.go b/cmd/agent/core/agent.go index 02d2f21f31..c845222dca 100644 --- a/cmd/agent/core/agent.go +++ b/cmd/agent/core/agent.go @@ -276,12 +276,12 @@ func stringSliceAddToMap(sl []string, m map[string]string) error { m = make(map[string]string) } for _, v := range utils.StringSliceDeleteEmpty(sl) { - parts := strings.SplitN(v, "=", 2) - switch len(parts) { - case 2: - m[parts[0]] = parts[1] - case 1: - return fmt.Errorf("key '%s' does not have a value assigned", parts[0]) + before, after, _ := strings.Cut(v, "=") + switch { + case before != "" && after != "": + m[before] = after + case before != "": + return fmt.Errorf("key '%s' does not have a value assigned", before) default: return fmt.Errorf("empty string in slice") } diff --git a/cmd/agent/core/flags.go b/cmd/agent/core/flags.go index 5e92f74be3..43bd564369 100644 --- a/cmd/agent/core/flags.go +++ b/cmd/agent/core/flags.go @@ -22,6 +22,7 @@ import ( "github.com/urfave/cli/v2" ) +//nolint:gomnd var flags = []cli.Flag{ &cli.StringFlag{ EnvVars: []string{"WOODPECKER_SERVER"}, diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index 86afd070f6..1e4ad589bd 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -27,6 +27,9 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" ) +// Valid container volumes must have at least two components, source and destination. +const minVolumeComponents = 2 + // returns a container configuration. func (e *docker) toConfig(step *types.Step) *container.Config { config := &container.Config{ @@ -131,7 +134,7 @@ func toVol(paths []string) map[string]struct{} { if err != nil { continue } - if len(parts) < 2 { + if len(parts) < minVolumeComponents { continue } set[parts[1]] = struct{}{} @@ -149,16 +152,18 @@ func toEnv(env map[string]string) []string { return envs } -// helper function that converts a slice of device paths to a slice of -// container.DeviceMapping. +// toDev converts a slice of volume paths to a set of device mappings for +// use in a Docker container config. It handles splitting the volume paths +// into host and container paths, and setting default permissions. func toDev(paths []string) []container.DeviceMapping { var devices []container.DeviceMapping + for _, path := range paths { parts, err := splitVolumeParts(path) if err != nil { continue } - if len(parts) < 2 { + if len(parts) < minVolumeComponents { continue } if strings.HasSuffix(parts[1], ":ro") || strings.HasSuffix(parts[1], ":rw") { @@ -183,7 +188,15 @@ func encodeAuthToBase64(authConfig types.Auth) (string, error) { return base64.URLEncoding.EncodeToString(buf), nil } -// helper function that split volume path +// splitVolumeParts splits a volume string into its constituent parts. +// +// The parts are: +// +// 1. The path on the host machine +// 2. The path inside the container +// 3. The read/write mode +// +// It handles Windows and Linux style volume paths. func splitVolumeParts(volumeParts string) ([]string, error) { pattern := `^((?:[\w]\:)?[^\:]*)\:((?:[\w]\:)?[^\:]*)(?:\:([rwom]*))?` r, err := regexp.Compile(pattern) diff --git a/pipeline/shared/replace_secrets.go b/pipeline/shared/replace_secrets.go index 668874b9a6..2232538002 100644 --- a/pipeline/shared/replace_secrets.go +++ b/pipeline/shared/replace_secrets.go @@ -16,11 +16,21 @@ package shared import "strings" +// NewSecretsReplacer creates a new strings.Replacer to replace sensitive +// strings with asterisks. It takes a slice of secrets strings as input +// and returns a populated strings.Replacer that will replace those +// secrets with asterisks. Each secret string is split on newlines to +// handle multi-line secrets. func NewSecretsReplacer(secrets []string) *strings.Replacer { var oldnew []string + + // Strings shorter than minStringLength are not considered secrets. + // Do not sanitize them. + const minStringLength = 3 + for _, old := range secrets { old = strings.TrimSpace(old) - if len(old) <= 3 { + if len(old) <= minStringLength { continue } // since replacer is executed on each line we have to split multi-line-secrets diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go index 73aa6a66a9..ab2104f7b6 100644 --- a/server/forge/bitbucket/fixtures/handler.go +++ b/server/forge/bitbucket/fixtures/handler.go @@ -187,9 +187,9 @@ func getUserRepos(c *gin.Context) { func getPermissions(c *gin.Context) { if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, permissionsPayLoad) + c.String(http.StatusOK, permissionsPayLoad) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go index 2d420e738c..16e4380bc1 100644 --- a/server/forge/bitbucket/internal/client.go +++ b/server/forge/bitbucket/internal/client.go @@ -184,7 +184,7 @@ func (c *Client) ListPermissions(opts *ListOpts) (*RepoPermResp, error) { func (c *Client) ListPermissionsAll() ([]*RepoPerm, error) { return shared_utils.Paginate(func(page int) ([]*RepoPerm, error) { - resp, err := c.ListPermissions(&ListOpts{Page: page, PageLen: 100}) + resp, err := c.ListPermissions(&ListOpts{Page: page, PageLen: pageSize}) if err != nil { return nil, err } diff --git a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go index 386be1a809..b881a701db 100644 --- a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go +++ b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go @@ -34,6 +34,8 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/store" ) +const listLimit = 250 + // Opts defines configuration options. type Opts struct { URL string // Bitbucket server url for API access. @@ -166,7 +168,7 @@ func (c *client) Repo(ctx context.Context, u *model.User, rID model.ForgeRemoteI var repo *bb.Repository if rID.IsValid() { - opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: 250}} + opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: listLimit}} for { repos, resp, err := bc.Projects.SearchRepositories(ctx, opts) if err != nil { @@ -213,7 +215,7 @@ func (c *client) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error return nil, fmt.Errorf("unable to create bitbucket client: %w", err) } - opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: 250}} + opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: listLimit}} var all []*model.Repo for { repos, resp, err := bc.Projects.SearchRepositories(ctx, opts) @@ -231,7 +233,7 @@ func (c *client) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error } // Add admin permissions to relevant repositories - opts = &bb.RepositorySearchOptions{Permission: bb.PermissionRepoAdmin, ListOptions: bb.ListOptions{Limit: 250}} + opts = &bb.RepositorySearchOptions{Permission: bb.PermissionRepoAdmin, ListOptions: bb.ListOptions{Limit: listLimit}} for { repos, resp, err := bc.Projects.SearchRepositories(ctx, opts) if err != nil { diff --git a/server/forge/bitbucketdatacenter/convert.go b/server/forge/bitbucketdatacenter/convert.go index 0e9f685107..bef0c30d55 100644 --- a/server/forge/bitbucketdatacenter/convert.go +++ b/server/forge/bitbucketdatacenter/convert.go @@ -136,7 +136,10 @@ func convertPullRequestEvent(ev *bb.PullRequestEvent, baseURL string) *model.Pip func authorLabel(name string) string { var result string - if len(name) > 40 { + + const maxNameLength = 40 + + if len(name) > maxNameLength { result = name[0:37] + "..." } else { result = name diff --git a/server/forge/github/github.go b/server/forge/github/github.go index b94401f2b2..e272b0f343 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -489,7 +489,9 @@ func (c *client) Status(ctx context.Context, user *model.User, repo *model.Repo, client := c.newClientToken(ctx, user.Token) if pipeline.Event == model.EventDeploy { + // Get id from url. If not found, skip. matches := reDeploy.FindStringSubmatch(pipeline.ForgeURL) + //nolint:gomnd if len(matches) != 2 { return nil } diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index 7c8f4995bd..9e9adc7cd0 100644 --- a/server/forge/gitlab/convert.go +++ b/server/forge/gitlab/convert.go @@ -28,7 +28,8 @@ import ( ) const ( - mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base + mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base + VisibilityLevelInternal = 10 ) func (g *GitLab) convertGitLabRepo(_repo *gitlab.Project, projectMember *gitlab.ProjectMember) (*model.Repo, error) { @@ -258,7 +259,7 @@ func convertReleaseHook(hook *gitlab.ReleaseEvent) (*model.Repo, *model.Pipeline repo.CloneSSH = hook.Project.GitSSHURL repo.FullName = hook.Project.PathWithNamespace repo.Branch = hook.Project.DefaultBranch - repo.IsSCMPrivate = hook.Project.VisibilityLevel > 10 + repo.IsSCMPrivate = hook.Project.VisibilityLevel > VisibilityLevelInternal pipeline := &model.Pipeline{ Event: model.EventRelease, @@ -292,9 +293,13 @@ func getUserAvatar(email string) string { ) } +// extractFromPath splits a repository path string into owner and name components. +// It requires at least two path components, otherwise an error is returned. func extractFromPath(str string) (string, string, error) { + const minPathComponents = 2 + s := strings.Split(str, "/") - if len(s) < 2 { + if len(s) < minPathComponents { return "", "", fmt.Errorf("minimum match not found") } return s[0], s[1], nil diff --git a/server/plugins/config/http.go b/server/plugins/config/http.go deleted file mode 100644 index e3a393ff7d..0000000000 --- a/server/plugins/config/http.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2022 Woodpecker Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package config - -import ( - "context" - "crypto" - "fmt" - - httpStatus "net/http" - - forge_types "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" - "go.woodpecker-ci.org/woodpecker/v2/server/model" - "go.woodpecker-ci.org/woodpecker/v2/server/plugins/utils" -) - -type http struct { - endpoint string - privateKey crypto.PrivateKey -} - -// Same as forge.FileMeta but with json tags and string data -type config struct { - Name string `json:"name"` - Data string `json:"data"` -} - -type requestStructure struct { - Repo *model.Repo `json:"repo"` - Pipeline *model.Pipeline `json:"pipeline"` - Configuration []*config `json:"configs"` - Netrc *model.Netrc `json:"netrc"` -} - -type responseStructure struct { - Configs []config `json:"configs"` -} - -func NewHTTP(endpoint string, privateKey crypto.PrivateKey) Extension { - return &http{endpoint, privateKey} -} - -func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta, netrc *model.Netrc) (configData []*forge_types.FileMeta, useOld bool, err error) { - currentConfigs := make([]*config, len(currentFileMeta)) - for i, pipe := range currentFileMeta { - currentConfigs[i] = &config{Name: pipe.Name, Data: string(pipe.Data)} - } - - response := new(responseStructure) - body := requestStructure{ - Repo: repo, - Pipeline: pipeline, - Configuration: currentConfigs, - Netrc: netrc, - } - - status, err := utils.Send(ctx, "POST", cp.endpoint, cp.privateKey, body, response) - if err != nil && status != 204 { - return nil, false, fmt.Errorf("failed to fetch config via http (%d) %w", status, err) - } - - var newFileMeta []*forge_types.FileMeta - if status != httpStatus.StatusOK { - newFileMeta = make([]*forge_types.FileMeta, 0) - } else { - newFileMeta = make([]*forge_types.FileMeta, len(response.Configs)) - for i, pipe := range response.Configs { - newFileMeta[i] = &forge_types.FileMeta{Name: pipe.Name, Data: []byte(pipe.Data)} - } - } - - return newFileMeta, status == httpStatus.StatusNoContent, nil -} diff --git a/server/services/config/http.go b/server/services/config/http.go index dc51a47349..3912fdbeb1 100644 --- a/server/services/config/http.go +++ b/server/services/config/http.go @@ -18,6 +18,7 @@ import ( "context" "crypto" "fmt" + nethttp "net/http" "go.woodpecker-ci.org/woodpecker/v2/server/forge" "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" @@ -75,7 +76,7 @@ func (h *http) Fetch(ctx context.Context, forge forge.Forge, user *model.User, r return nil, fmt.Errorf("failed to fetch config via http (%d) %w", status, err) } - if status != 200 { + if status != nethttp.StatusOK { return oldConfigData, nil }