From 22ebe21da370b926ab5cd2126fb38cbfc7304035 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 8 Feb 2023 10:16:10 +0100 Subject: [PATCH] refactor: add more linters and fix findings (#27) --- .drone.yml | 10 +-- .golangci.yml | 107 +++++++++++++++++++++++++++------ Makefile | 2 +- cmd/drone-git-action/config.go | 2 +- cmd/drone-git-action/main.go | 1 + git/clone.go | 23 ++++--- git/commit.go | 45 ++++++++------ git/config.go | 43 +++++++++---- git/init.go | 19 ++++++ git/remote.go | 33 +++++++--- git/status.go | 14 +++-- git/type.go | 2 + git/utils.go | 28 ++++++--- go.mod | 4 +- plugin/impl.go | 62 ++++++++++++------- plugin/plugin.go | 2 +- plugin/utils.go | 12 ++-- 17 files changed, 286 insertions(+), 123 deletions(-) create mode 100644 git/init.go diff --git a/.drone.yml b/.drone.yml index 1dbd1bb..834889d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -8,7 +8,7 @@ platform: steps: - name: deps - image: golang:1.19 + image: golang:1.20 commands: - make deps volumes: @@ -16,7 +16,7 @@ steps: path: /go - name: lint - image: golang:1.19 + image: golang:1.20 commands: - make lint volumes: @@ -24,7 +24,7 @@ steps: path: /go - name: test - image: golang:1.19 + image: golang:1.20 commands: - make test volumes: @@ -51,7 +51,7 @@ platform: steps: - name: build - image: techknowlogick/xgo:go-1.19.x + image: techknowlogick/xgo:go-1.20.x commands: - ln -s /drone/src /source - make release @@ -292,6 +292,6 @@ depends_on: --- kind: signature -hmac: 8b920283ba7cc82bad64ea0eb55b8cf3de5aae95ca0f7018bac9f0deb8b22244 +hmac: 89039f09e352a636edf52307e5e48f8bd54f4d9f0b2cf55dabddfdca88702bda ... diff --git a/.golangci.yml b/.golangci.yml index 7bb18ea..2faa799 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,25 +1,92 @@ linters: - enable: - - gosimple - - deadcode - - typecheck - - govet - - errcheck - - staticcheck - - unused - - structcheck - - varcheck - - dupl - - gofmt - - misspell - - gocritic - - bidichk - - ineffassign - - revive - - gofumpt - - depguard enable-all: false disable-all: true + enable: + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - typecheck + - unused + - asasalint + - asciicheck + - bidichk + - bodyclose + - containedctx + - contextcheck + - decorder + - depguard + - dogsled + - dupl + - dupword + - durationcheck + - errchkjson + - errname + - errorlint + - execinquery + - exhaustive + - exportloopref + - forcetypeassert + - ginkgolinter + - gocheckcompilerdirectives + - gochecknoglobals + - gochecknoinits + - gocognit + - goconst + - gocritic + - gocyclo + - godot + - godox + - goerr113 + - gofmt + - gofumpt + - goheader + - goimports + - gomnd + - gomoddirectives + - gomodguard + - goprintffuncname + - gosec + - grouper + - importas + - interfacebloat + - ireturn + - lll + - loggercheck + - maintidx + - makezero + - misspell + - musttag + - nakedret + - nestif + - nilerr + - nilnil + - nlreturn + - noctx + - nolintlint + - nonamedreturns + - nosprintfhostport + - prealloc + - predeclared + - promlinter + - reassign + - revive + # - rowserrcheck + # - sqlclosecheck + # - structcheck + - stylecheck + - tagliatelle + - tenv + - testableexamples + - thelper + - tparallel + - unconvert + - unparam + - usestdlibvars + # - wastedassign + - whitespace + - wsl fast: false run: @@ -28,4 +95,4 @@ run: linters-settings: gofumpt: extra-rules: true - lang-version: "1.18" + lang-version: "1.20" diff --git a/Makefile b/Makefile index cc38274..20731ca 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@$(G XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest GENERATE ?= -XGO_VERSION := go-1.19.x +XGO_VERSION := go-1.20.x XGO_TARGETS ?= linux/amd64,linux/arm-6,linux/arm-7,linux/arm64 TARGETOS ?= linux diff --git a/cmd/drone-git-action/config.go b/cmd/drone-git-action/config.go index 6a764fd..7fcdc3a 100644 --- a/cmd/drone-git-action/config.go +++ b/cmd/drone-git-action/config.go @@ -10,7 +10,7 @@ func settingsFlags(settings *plugin.Settings, category string) []cli.Flag { return []cli.Flag{ &cli.StringSliceFlag{ Name: "action", - Usage: "git action to to execute", + Usage: "git action to execute", EnvVars: []string{"PLUGIN_ACTION"}, Destination: &settings.Action, Required: true, diff --git a/cmd/drone-git-action/main.go b/cmd/drone-git-action/main.go index 1a8f4d9..8ee2888 100644 --- a/cmd/drone-git-action/main.go +++ b/cmd/drone-git-action/main.go @@ -11,6 +11,7 @@ import ( "github.com/urfave/cli/v2" ) +//nolint:gochecknoglobals var ( BuildVersion = "devel" BuildDate = "00000000" diff --git a/git/clone.go b/git/clone.go index 3621018..260e3bc 100644 --- a/git/clone.go +++ b/git/clone.go @@ -3,16 +3,21 @@ package git import ( "fmt" "os" - "os/exec" + + "golang.org/x/sys/execabs" ) // FetchSource fetches the source from remote. -func FetchSource(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func FetchSource(repo Repository) *execabs.Cmd { + args := []string{ "fetch", "origin", fmt.Sprintf("+%s:", repo.Branch), + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -21,12 +26,16 @@ func FetchSource(repo Repository) *exec.Cmd { } // CheckoutHead handles branch checkout. -func CheckoutHead(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func CheckoutHead(repo Repository) *execabs.Cmd { + args := []string{ "checkout", "-qf", repo.Branch, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr diff --git a/git/commit.go b/git/commit.go index d2fa230..6938336 100644 --- a/git/commit.go +++ b/git/commit.go @@ -2,13 +2,14 @@ package git import ( "os" - "os/exec" + + "golang.org/x/sys/execabs" ) // ForceAdd forces the addition of all dirty files. -func ForceAdd(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func ForceAdd(repo Repository) *execabs.Cmd { + cmd := execabs.Command( + gitBin, "add", "--all", "--force", @@ -20,9 +21,9 @@ func ForceAdd(repo Repository) *exec.Cmd { } // Add updates the index to match the working tree. -func Add(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func Add(repo Repository) *execabs.Cmd { + cmd := execabs.Command( + gitBin, "add", ) cmd.Dir = repo.WorkDir @@ -37,10 +38,10 @@ func Add(repo Repository) *exec.Cmd { return cmd } -// TestCleanTree returns non-zero if diff between index and local repository -func TestCleanTree(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +// TestCleanTree returns non-zero if diff between index and local repository. +func TestCleanTree(repo Repository) *execabs.Cmd { + cmd := execabs.Command( + gitBin, "diff-index", "--quiet", "HEAD", @@ -52,14 +53,18 @@ func TestCleanTree(repo Repository) *exec.Cmd { return cmd } -// EmptyCommit simply create an empty commit -func EmptyCommit(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +// EmptyCommit simply create an empty commit. +func EmptyCommit(repo Repository) *execabs.Cmd { + args := []string{ "commit", "--allow-empty", "-m", repo.CommitMsg, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -72,12 +77,16 @@ func EmptyCommit(repo Repository) *exec.Cmd { } // ForceCommit commits every change while skipping CI. -func ForceCommit(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func ForceCommit(repo Repository) *execabs.Cmd { + args := []string{ "commit", "-m", repo.CommitMsg, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr diff --git a/git/config.go b/git/config.go index add7d9e..cbb7e1f 100644 --- a/git/config.go +++ b/git/config.go @@ -2,18 +2,23 @@ package git import ( "os" - "os/exec" "strconv" + + "golang.org/x/sys/execabs" ) // repoUserEmail sets the global git author email. -func ConfigAutocorrect(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func ConfigAutocorrect(repo Repository) *execabs.Cmd { + args := []string{ "config", "--local", "help.autocorrect", repo.Autocorrect, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -22,13 +27,17 @@ func ConfigAutocorrect(repo Repository) *exec.Cmd { } // repoUserEmail sets the global git author email. -func ConfigUserEmail(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func ConfigUserEmail(repo Repository) *execabs.Cmd { + args := []string{ "config", "--local", "user.email", repo.Author.Email, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -37,13 +46,17 @@ func ConfigUserEmail(repo Repository) *exec.Cmd { } // repoUserName sets the global git author name. -func ConfigUserName(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func ConfigUserName(repo Repository) *execabs.Cmd { + args := []string{ "config", "--local", "user.name", repo.Author.Name, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -52,13 +65,17 @@ func ConfigUserName(repo Repository) *exec.Cmd { } // repoSSLVerify disables globally the git ssl verification. -func ConfigSSLVerify(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func ConfigSSLVerify(repo Repository) *execabs.Cmd { + args := []string{ "config", "--local", "http.sslVerify", strconv.FormatBool(repo.SSLVerify), + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr diff --git a/git/init.go b/git/init.go new file mode 100644 index 0000000..2c0c076 --- /dev/null +++ b/git/init.go @@ -0,0 +1,19 @@ +package git + +import ( + "os" + + "golang.org/x/sys/execabs" +) + +// RemoteRemove drops the defined remote from a git repo. +func Init(repo Repository) *execabs.Cmd { + cmd := execabs.Command( + gitBin, + "init", + ) + cmd.Dir = repo.WorkDir + cmd.Stderr = os.Stderr + + return cmd +} diff --git a/git/remote.go b/git/remote.go index 331f8bc..e0f9632 100644 --- a/git/remote.go +++ b/git/remote.go @@ -3,16 +3,21 @@ package git import ( "fmt" "os" - "os/exec" + + "golang.org/x/sys/execabs" ) // RemoteRemove drops the defined remote from a git repo. -func RemoteRemove(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func RemoteRemove(repo Repository) *execabs.Cmd { + args := []string{ "remote", "rm", repo.RemoteName, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -21,13 +26,17 @@ func RemoteRemove(repo Repository) *exec.Cmd { } // RemoteAdd adds an additional remote to a git repo. -func RemoteAdd(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func RemoteAdd(repo Repository) *execabs.Cmd { + args := []string{ "remote", "add", repo.RemoteName, repo.RemoteURL, + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr @@ -36,12 +45,16 @@ func RemoteAdd(repo Repository) *exec.Cmd { } // RemotePush pushs the changes from the local head to a remote branch. -func RemotePush(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func RemotePush(repo Repository) *execabs.Cmd { + args := []string{ "push", repo.RemoteName, fmt.Sprintf("HEAD:%s", repo.Branch), + } + + cmd := execabs.Command( + gitBin, + args..., ) cmd.Dir = repo.WorkDir cmd.Stderr = os.Stderr diff --git a/git/status.go b/git/status.go index 225f05e..7bca2f1 100644 --- a/git/status.go +++ b/git/status.go @@ -2,14 +2,15 @@ package git import ( "bytes" - "fmt" "os" - "os/exec" + + "github.com/sirupsen/logrus" + "golang.org/x/sys/execabs" ) -func Status(repo Repository) *exec.Cmd { - cmd := exec.Command( - "git", +func Status(repo Repository) *execabs.Cmd { + cmd := execabs.Command( + gitBin, "status", "--porcelain", ) @@ -34,7 +35,8 @@ func IsDirty(repo Repository) bool { } if res.Len() > 0 { - fmt.Print(res.String()) + logrus.Debug(res.String()) + return true } diff --git a/git/type.go b/git/type.go index 0114333..ab42a0e 100644 --- a/git/type.go +++ b/git/type.go @@ -25,3 +25,5 @@ type Repository struct { Author Author } + +const gitBin = "/usr/bin/git" diff --git a/git/utils.go b/git/utils.go index 7bf489e..5370921 100644 --- a/git/utils.go +++ b/git/utils.go @@ -3,23 +3,30 @@ package git import ( "fmt" "os" - "os/exec" "os/user" "path/filepath" "strings" + + "golang.org/x/sys/execabs" ) -const netrcFile = ` +const ( + netrcFile = ` machine %s login %s password %s ` - -const configFile = ` + configFile = ` Host * StrictHostKeyChecking no UserKnownHostsFile=/dev/null ` +) + +const ( + strictFilePerm = 0o600 + strictDirPerm = 0o600 +) // WriteKey writes the SSH private key. func WriteSSHKey(privateKey string) error { @@ -31,7 +38,7 @@ func WriteSSHKey(privateKey string) error { sshpath := filepath.Join(home, ".ssh") - if err := os.MkdirAll(sshpath, 0o700); err != nil { + if err := os.MkdirAll(sshpath, strictDirPerm); err != nil { return err } @@ -40,7 +47,7 @@ func WriteSSHKey(privateKey string) error { if err := os.WriteFile( confpath, []byte(configFile), - 0o700, + strictFilePerm, ); err != nil { return err } @@ -50,7 +57,7 @@ func WriteSSHKey(privateKey string) error { if err := os.WriteFile( privpath, []byte(privateKey), - 0o600, + strictFilePerm, ); err != nil { return err } @@ -81,15 +88,15 @@ func WriteNetrc(machine, login, password string) error { return os.WriteFile( netpath, []byte(netrcContent), - 0o600, + strictFilePerm, ) } -func trace(cmd *exec.Cmd) { +func trace(cmd *execabs.Cmd) { fmt.Fprintf(os.Stdout, "+ %s\n", strings.Join(cmd.Args, " ")) } -func runCommand(cmd *exec.Cmd) error { +func runCommand(cmd *execabs.Cmd) error { if cmd.Stdout == nil { cmd.Stdout = os.Stdout } @@ -99,5 +106,6 @@ func runCommand(cmd *exec.Cmd) error { } trace(cmd) + return cmd.Run() } diff --git a/go.mod b/go.mod index 7368bac..134e274 100644 --- a/go.mod +++ b/go.mod @@ -1,17 +1,17 @@ module github.com/thegeeklab/drone-git-action -go 1.19 +go 1.20 require ( github.com/joho/godotenv v1.4.0 github.com/sirupsen/logrus v1.9.0 github.com/thegeeklab/drone-plugin-lib/v2 v2.2.1 github.com/urfave/cli/v2 v2.24.3 + golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 ) require ( github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect - golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect ) diff --git a/plugin/impl.go b/plugin/impl.go index 21cf214..7a6c73a 100644 --- a/plugin/impl.go +++ b/plugin/impl.go @@ -1,9 +1,9 @@ package plugin import ( + "errors" "fmt" "os" - "os/exec" "path/filepath" "github.com/thegeeklab/drone-git-action/git" @@ -32,6 +32,16 @@ type Settings struct { Repo git.Repository } +var ( + ErrAuthSourceNotSet = errors.New("either SSH key or netrc password is required") + ErrPagesDirectoryNotExist = errors.New("pages directory must exist") + ErrPagesDirectoryNotValid = errors.New("pages directory not valid") + ErrPagesSourceNotSet = errors.New("pages source directory must be set") + ErrPagesActionNotExclusive = errors.New("pages action is mutual exclusive") + ErrActionUnknown = errors.New("action not found") + ErrGitCloneDestintionNotValid = errors.New("destination not valid") +) + // Validate handles the settings validation of the plugin. func (p *Plugin) Validate() error { var err error @@ -43,6 +53,7 @@ func (p *Plugin) Validate() error { if p.settings.Repo.WorkDir == "" { p.settings.Repo.WorkDir, err = os.Getwd() } + if err != nil { return err } @@ -55,33 +66,33 @@ func (p *Plugin) Validate() error { continue case "push": if p.settings.SSHKey == "" && p.settings.Netrc.Password == "" { - return fmt.Errorf("either SSH key or netrc password is required") + return ErrAuthSourceNotSet } case "pages": p.settings.Pages.Directory = filepath.Join(p.settings.Repo.WorkDir, p.settings.Pages.Directory) p.settings.Repo.WorkDir = filepath.Join(p.settings.Repo.WorkDir, ".tmp") if _, err := os.Stat(p.settings.Pages.Directory); os.IsNotExist(err) { - return fmt.Errorf("pages directory '%s' must exist", p.settings.Pages.Directory) + return fmt.Errorf("%w: '%s' not found", ErrPagesDirectoryNotExist, p.settings.Pages.Directory) } if info, _ := os.Stat(p.settings.Pages.Directory); !info.IsDir() { - return fmt.Errorf("pages directory '%s' is not a directory", p.settings.Pages.Directory) + return fmt.Errorf("%w: '%s' not a directory", ErrPagesDirectoryNotValid, p.settings.Pages.Directory) } if p.settings.SSHKey == "" && p.settings.Netrc.Password == "" { - return fmt.Errorf("either SSH key or netrc password is required") + return ErrAuthSourceNotSet } if p.settings.Pages.Directory == "" { - return fmt.Errorf("pages source directory needs to be set") + return ErrPagesSourceNotSet } if len(p.settings.Action.Value()) > 1 { - return fmt.Errorf("pages action can not be combined with other actions") + return ErrPagesActionNotExclusive } default: - return fmt.Errorf("unknown action %s", action) + return fmt.Errorf("%w: %s", ErrActionUnknown, action) } } @@ -98,28 +109,33 @@ func (p *Plugin) Execute() error { "GIT_COMMITTER_EMAIL", "GIT_COMMITTER_DATE", } + for _, env := range gitEnv { if err := os.Unsetenv(env); err != nil { return err } } + if err := os.Setenv("GIT_TERMINAL_PROMPT", "0"); err != nil { return err } - if err := p.initRepo(); err != nil { + if err := p.handleInit(); err != nil { return err } if err := git.ConfigAutocorrect(p.settings.Repo).Run(); err != nil { return err } + if err := git.ConfigUserName(p.settings.Repo).Run(); err != nil { return err } + if err := git.ConfigUserEmail(p.settings.Repo).Run(); err != nil { return err } + if err := git.ConfigSSLVerify(p.settings.Repo).Run(); err != nil { return err } @@ -158,31 +174,31 @@ func (p *Plugin) Execute() error { return nil } -// InitRepo initializes the repository. -func (p *Plugin) initRepo() error { +// handleInit initializes the repository. +func (p *Plugin) handleInit() error { path := filepath.Join(p.settings.Repo.WorkDir, ".git") + if err := os.MkdirAll(p.settings.Repo.WorkDir, os.ModePerm); err != nil { return err } if _, err := os.Stat(path); !os.IsNotExist(err) { p.settings.Repo.InitExists = true + return nil } - cmd := exec.Command( - "git", - "init", - ) - cmd.Dir = p.settings.Repo.WorkDir + if err := execute(git.Init(p.settings.Repo)); err != nil { + return err + } - return execute(cmd) + return nil } // HandleClone clones remote. func (p *Plugin) handleClone() error { if p.settings.Repo.InitExists { - return fmt.Errorf("destination '%s' already exists and is not an empty directory", p.settings.Repo.WorkDir) + return fmt.Errorf("%w: %s exists and not empty", ErrGitCloneDestintionNotValid, p.settings.Repo.WorkDir) } if p.settings.Repo.RemoteURL != "" { @@ -212,11 +228,11 @@ func (p *Plugin) handleCommit() error { if err := execute(git.ForceCommit(p.settings.Repo)); err != nil { return err } - } else { - if p.settings.Repo.EmptyCommit { - if err := execute(git.EmptyCommit(p.settings.Repo)); err != nil { - return err - } + } + + if p.settings.Repo.EmptyCommit { + if err := execute(git.EmptyCommit(p.settings.Repo)); err != nil { + return err } } diff --git a/plugin/plugin.go b/plugin/plugin.go index 65eada6..85551fc 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -12,7 +12,7 @@ type Plugin struct { } // New initializes a plugin from the given Settings, Pipeline, and Network. -func New(settings Settings, pipeline drone.Pipeline, network drone.Network) drone.Plugin { +func New(settings Settings, pipeline drone.Pipeline, network drone.Network) *Plugin { return &Plugin{ settings: settings, pipeline: pipeline, diff --git a/plugin/utils.go b/plugin/utils.go index 1d931da..d0914a0 100644 --- a/plugin/utils.go +++ b/plugin/utils.go @@ -1,17 +1,17 @@ package plugin import ( - "fmt" "os" - "os/exec" "strings" + "github.com/sirupsen/logrus" "github.com/thegeeklab/drone-git-action/git" + "golang.org/x/sys/execabs" ) // helper function to simply wrap os execte command. -func execute(cmd *exec.Cmd) error { - fmt.Println("+", strings.Join(cmd.Args, " ")) +func execute(cmd *execabs.Cmd) error { + logrus.Debug("+", strings.Join(cmd.Args, " ")) cmd.Env = os.Environ() cmd.Stderr = os.Stderr @@ -20,7 +20,7 @@ func execute(cmd *exec.Cmd) error { return cmd.Run() } -func rsyncDirectories(pages Pages, repo git.Repository) *exec.Cmd { +func rsyncDirectories(pages Pages, repo git.Repository) *execabs.Cmd { args := []string{ "-r", "--exclude", @@ -48,7 +48,7 @@ func rsyncDirectories(pages Pages, repo git.Repository) *exec.Cmd { repo.WorkDir, ) - cmd := exec.Command( + cmd := execabs.Command( "rsync", args..., )