From fc62fa4e5e1570ac8fed921381df36ddcae82a12 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 11 Jan 2024 16:20:58 +0100 Subject: [PATCH 1/2] Enable golangci linter goconst --- .golangci.yml | 7 +++++++ cli/pipeline/info.go | 2 +- pipeline/backend/local/clone.go | 8 +++++--- pipeline/frontend/yaml/compiler/compiler.go | 2 +- .../store/datastore/migration/008_secrets_add_user.go | 10 ++++++---- .../datastore/migration/013_rename_remote_to_forge.go | 6 ++++-- 6 files changed, 24 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 02b479aa40..1a72e7ecf2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -164,6 +164,13 @@ linters: - gocritic - nolintlint - stylecheck + - goconst + +issues: + exclude-rules: + - path: '_test.go' + linters: + - goconst run: timeout: 15m diff --git a/cli/pipeline/info.go b/cli/pipeline/info.go index 2b1e9103fc..51d7554438 100644 --- a/cli/pipeline/info.go +++ b/cli/pipeline/info.go @@ -46,7 +46,7 @@ func pipelineInfo(c *cli.Context) error { pipelineArg := c.Args().Get(1) var number int64 - if pipelineArg == "last" || len(pipelineArg) == 0 { + if pipelineArg == "last" || len(pipelineArg) == 0 { //nolint:goconst // Fetch the pipeline number from the last pipeline pipeline, err := client.PipelineLast(repoID, "") if err != nil { diff --git a/pipeline/backend/local/clone.go b/pipeline/backend/local/clone.go index 8dd2bc8169..f92956057c 100644 --- a/pipeline/backend/local/clone.go +++ b/pipeline/backend/local/clone.go @@ -29,6 +29,8 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" ) +const osWindows = "windows" + // checkGitCloneCap check if we have the git binary on hand func checkGitCloneCap() error { _, err := exec.LookPath("git") @@ -54,7 +56,7 @@ func (e *local) setupClone(state *workflowState) error { log.Info().Msg("no global 'plugin-git' installed, try to download for current workflow") state.pluginGitBinary = filepath.Join(state.homeDir, "plugin-git") - if e.os == "windows" { + if e.os == osWindows { state.pluginGitBinary += ".exe" } return e.downloadLatestGitPluginBinary(state.pluginGitBinary) @@ -87,7 +89,7 @@ func (e *local) execClone(ctx context.Context, step *types.Step, state *workflow var cmd *exec.Cmd if rmCmd != "" { // if we have a netrc injected we have to make sure it's deleted in any case after clone was attempted - if e.os == "windows" { + if e.os == osWindows { pwsh, err := exec.LookPath("powershell.exe") if err != nil { return err @@ -121,7 +123,7 @@ func (e *local) writeNetRC(step *types.Step, state *workflowState) (string, erro file := filepath.Join(state.homeDir, ".netrc") rmCmd := fmt.Sprintf("rm \"%s\"", file) - if e.os == "windows" { + if e.os == osWindows { file = filepath.Join(state.homeDir, "_netrc") rmCmd = fmt.Sprintf("del \"%s\"", file) } diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index 3e82fcaaa1..fd980757e5 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -151,7 +151,7 @@ func (c *Compiler) Compile(conf *yaml_types.Workflow) (*backend_types.Config, er if !c.local && len(conf.Clone.ContainerList) == 0 && !conf.SkipClone { cloneSettings := map[string]any{"depth": "0"} if c.metadata.Curr.Event == metadata.EventTag { - cloneSettings["tags"] = "true" + cloneSettings["tags"] = "true" //nolint:goconst } container := &yaml_types.Container{ Name: defaultCloneName, diff --git a/server/store/datastore/migration/008_secrets_add_user.go b/server/store/datastore/migration/008_secrets_add_user.go index b77208f750..0a5320e68a 100644 --- a/server/store/datastore/migration/008_secrets_add_user.go +++ b/server/store/datastore/migration/008_secrets_add_user.go @@ -19,6 +19,8 @@ import ( "xorm.io/xorm" ) +const secretsTableName = "secrets" + type SecretV008 struct { Owner string `json:"-" xorm:"NOT NULL DEFAULT '' UNIQUE(s) INDEX 'secret_owner'"` RepoID int64 `json:"-" xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_repo_id'"` @@ -27,7 +29,7 @@ type SecretV008 struct { // TableName return database table name for xorm func (SecretV008) TableName() string { - return "secrets" + return secretsTableName } var alterTableSecretsAddUserCol = xormigrate.Migration{ @@ -36,12 +38,12 @@ var alterTableSecretsAddUserCol = xormigrate.Migration{ if err := sess.Sync(new(SecretV008)); err != nil { return err } - if err := alterColumnDefault(sess, "secrets", "secret_repo_id", "0"); err != nil { + if err := alterColumnDefault(sess, secretsTableName, "secret_repo_id", "0"); err != nil { return err } - if err := alterColumnNull(sess, "secrets", "secret_repo_id", false); err != nil { + if err := alterColumnNull(sess, secretsTableName, "secret_repo_id", false); err != nil { return err } - return alterColumnNull(sess, "secrets", "secret_name", false) + return alterColumnNull(sess, secretsTableName, "secret_name", false) }, } diff --git a/server/store/datastore/migration/013_rename_remote_to_forge.go b/server/store/datastore/migration/013_rename_remote_to_forge.go index 78eff328be..870f6000ea 100644 --- a/server/store/datastore/migration/013_rename_remote_to_forge.go +++ b/server/store/datastore/migration/013_rename_remote_to_forge.go @@ -19,13 +19,15 @@ import ( "xorm.io/xorm" ) +const reposTableName = "repos" + type oldRepo013 struct { ID int64 `xorm:"pk autoincr 'repo_id'"` RemoteID string `xorm:"remote_id"` } func (oldRepo013) TableName() string { - return "repos" + return reposTableName } var renameRemoteToForge = xormigrate.Migration{ @@ -40,6 +42,6 @@ var renameRemoteToForge = xormigrate.Migration{ return err } - return renameColumn(sess, "repos", "remote_id", "forge_id") + return renameColumn(sess, reposTableName, "remote_id", "forge_id") }, } From d0411621b438b067c90f7def957c47873331d399 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 11 Jan 2024 17:47:10 +0100 Subject: [PATCH 2/2] fix ci --- server/api/login.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/server/api/login.go b/server/api/login.go index 06ed4fe991..c69052e319 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -32,6 +32,11 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/shared/token" ) +const ( + accessDeniedRoute = "/login?error=access_denied" + internalErrorRoute = "/login?error=internal_error" +) + func HandleLogin(c *gin.Context) { if err := c.Request.FormValue("error"); err != "" { c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login/error?code="+err) @@ -71,7 +76,7 @@ func HandleAuth(c *gin.Context) { // if self-registration is disabled we should return a not authorized error if !server.Config.Permissions.Open && !server.Config.Permissions.Admins.IsAdmin(tmpuser) { log.Error().Msgf("cannot register %s. registration closed", tmpuser.Login) - c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied") + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+accessDeniedRoute) return } @@ -81,7 +86,7 @@ func HandleAuth(c *gin.Context) { 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") + c.Redirect(303, server.Config.Server.RootPath+accessDeniedRoute) return } } @@ -102,7 +107,7 @@ func HandleAuth(c *gin.Context) { // insert the user into the database if err := _store.CreateUser(u); err != nil { log.Error().Err(err).Msgf("cannot insert %s", u.Login) - c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error") + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+internalErrorRoute) return } @@ -131,14 +136,14 @@ func HandleAuth(c *gin.Context) { teams, terr := _forge.Teams(c, u) if terr != nil || !server.Config.Permissions.Orgs.IsMember(teams) { 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") + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+accessDeniedRoute) return } } if err := _store.UpdateUser(u); err != nil { log.Error().Err(err).Msgf("cannot update %s", u.Login) - c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error") + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+internalErrorRoute) return } @@ -146,7 +151,7 @@ func HandleAuth(c *gin.Context) { tokenString, err := token.New(token.SessToken, u.Login).SignExpires(u.Hash, exp) if err != nil { log.Error().Msgf("cannot create token for %s", u.Login) - c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error") + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+internalErrorRoute) return } @@ -158,7 +163,7 @@ func HandleAuth(c *gin.Context) { } if err != nil { log.Error().Err(err).Msgf("cannot list repos for %s", u.Login) - c.Redirect(http.StatusSeeOther, "/login?error=internal_error") + c.Redirect(http.StatusSeeOther, internalErrorRoute) return } @@ -174,7 +179,7 @@ func HandleAuth(c *gin.Context) { perm.Synced = time.Now().Unix() if err := _store.PermUpsert(perm); err != nil { log.Error().Err(err).Msgf("cannot update permissions for %s", u.Login) - c.Redirect(http.StatusSeeOther, "/login?error=internal_error") + c.Redirect(http.StatusSeeOther, internalErrorRoute) return } }