From 0688b1cf1aafe49e5c3e6f09c860623f0ff889a1 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 8 Feb 2023 10:13:28 +0100 Subject: [PATCH] refactor: add more linters and fix findings (#195) --- .drone.yml | 10 +-- .golangci.yml | 107 ++++++++++++++++++++++++------ Makefile | 2 +- cmd/drone-docker-buildx/config.go | 2 + cmd/drone-docker-buildx/main.go | 11 ++- go.mod | 4 +- plugin/daemon.go | 5 +- plugin/docker.go | 72 ++++++++++++++------ plugin/docker_test.go | 1 - plugin/impl.go | 33 +++++---- plugin/plugin.go | 2 +- plugin/tags.go | 48 +++++++++++--- plugin/tags_test.go | 8 +++ 13 files changed, 229 insertions(+), 76 deletions(-) delete mode 100644 plugin/docker_test.go diff --git a/.drone.yml b/.drone.yml index 14baa65..f6fd7ae 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 @@ -286,6 +286,6 @@ depends_on: --- kind: signature -hmac: 5e54d2041c971a61b9545f50d3eb303541de10ef02f6ecf9f5043c49e2827657 +hmac: 99430572409aea6256230c2812e815fc98a8f5bba518455a4472f981817eba77 ... 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 7a27ca9..60d95ab 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/arm64 TARGETOS ?= linux diff --git a/cmd/drone-docker-buildx/config.go b/cmd/drone-docker-buildx/config.go index 04db420..ffa0b4a 100644 --- a/cmd/drone-docker-buildx/config.go +++ b/cmd/drone-docker-buildx/config.go @@ -7,6 +7,8 @@ import ( ) // settingsFlags has the cli.Flags for the plugin.Settings. +// +//nolint:maintidx func settingsFlags(settings *plugin.Settings, category string) []cli.Flag { return []cli.Flag{ &cli.BoolFlag{ diff --git a/cmd/drone-docker-buildx/main.go b/cmd/drone-docker-buildx/main.go index baa3888..a6a1a50 100644 --- a/cmd/drone-docker-buildx/main.go +++ b/cmd/drone-docker-buildx/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "os" @@ -13,11 +14,14 @@ import ( "github.com/thegeeklab/drone-plugin-lib/v2/urfave" ) +//nolint:gochecknoglobals var ( BuildVersion = "devel" BuildDate = "00000000" ) +var ErrTypeAssertionFailed = errors.New("type assertion failed") + func main() { settings := &plugin.Settings{} @@ -46,7 +50,12 @@ func run(settings *plugin.Settings) cli.ActionFunc { return func(ctx *cli.Context) error { urfave.LoggingFromContext(ctx) - settings.Build.CacheFrom = ctx.Generic("cache-from").(*drone.StringSliceFlag).Get() + cacheFrom, ok := ctx.Generic("cache-from").(*drone.StringSliceFlag) + if !ok { + return fmt.Errorf("%w: failed to read cache-from input", ErrTypeAssertionFailed) + } + + settings.Build.CacheFrom = cacheFrom.Get() plugin := plugin.New( *settings, diff --git a/go.mod b/go.mod index f0b085f..1680ca6 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/thegeeklab/drone-docker-buildx -go 1.19 +go 1.20 require ( github.com/coreos/go-semver v0.3.1 @@ -8,11 +8,11 @@ require ( 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/daemon.go b/plugin/daemon.go index 2cd0dce..ff15dc4 100644 --- a/plugin/daemon.go +++ b/plugin/daemon.go @@ -6,8 +6,8 @@ import ( ) const ( - dockerExe = "/usr/local/bin/docker" - dockerdExe = "/usr/local/bin/dockerd" + dockerBin = "/usr/local/bin/docker" + dockerdBin = "/usr/local/bin/dockerd" dockerHome = "/root/.docker/" buildkitConfig = "/tmp/buildkit.toml" ) @@ -21,6 +21,7 @@ func (p Plugin) startDaemon() { cmd.Stdout = io.Discard cmd.Stderr = io.Discard } + go func() { trace(cmd) _ = cmd.Run() diff --git a/plugin/docker.go b/plugin/docker.go index c505248..41deb46 100644 --- a/plugin/docker.go +++ b/plugin/docker.go @@ -3,47 +3,56 @@ package plugin import ( "fmt" "os" - "os/exec" "strings" "time" "github.com/urfave/cli/v2" + "golang.org/x/sys/execabs" ) // helper function to create the docker login command. -func commandLogin(login Login) *exec.Cmd { +func commandLogin(login Login) *execabs.Cmd { if login.Email != "" { return commandLoginEmail(login) } - return exec.Command( - dockerExe, "login", + + args := []string{ + "login", "-u", login.Username, "-p", login.Password, login.Registry, + } + + return execabs.Command( + dockerBin, args..., ) } -func commandLoginEmail(login Login) *exec.Cmd { - return exec.Command( - dockerExe, "login", +func commandLoginEmail(login Login) *execabs.Cmd { + args := []string{ + "login", "-u", login.Username, "-p", login.Password, "-e", login.Email, login.Registry, + } + + return execabs.Command( + dockerBin, args..., ) } // helper function to create the docker info command. -func commandVersion() *exec.Cmd { - return exec.Command(dockerExe, "version") +func commandVersion() *execabs.Cmd { + return execabs.Command(dockerBin, "version") } // helper function to create the docker info command. -func commandInfo() *exec.Cmd { - return exec.Command(dockerExe, "info") +func commandInfo() *execabs.Cmd { + return execabs.Command(dockerBin, "info") } -func commandBuilder(daemon Daemon) *exec.Cmd { +func commandBuilder(daemon Daemon) *execabs.Cmd { args := []string{ "buildx", "create", @@ -54,15 +63,15 @@ func commandBuilder(daemon Daemon) *exec.Cmd { args = append(args, "--config", buildkitConfig) } - return exec.Command(dockerExe, args...) + return execabs.Command(dockerBin, args...) } -func commandBuildx() *exec.Cmd { - return exec.Command(dockerExe, "buildx", "ls") +func commandBuildx() *execabs.Cmd { + return execabs.Command(dockerBin, "buildx", "ls") } // helper function to create the docker build command. -func commandBuild(build Build, dryrun bool) *exec.Cmd { +func commandBuild(build Build, dryrun bool) *execabs.Cmd { args := []string{ "buildx", "build", @@ -78,39 +87,51 @@ func commandBuild(build Build, dryrun bool) *exec.Cmd { if !dryrun && build.Output == "" && len(build.Tags.Value()) > 0 { args = append(args, "--push") } + if build.Compress { args = append(args, "--compress") } + if build.Pull { args = append(args, "--pull=true") } + if build.NoCache { args = append(args, "--no-cache") } + for _, arg := range build.CacheFrom { args = append(args, "--cache-from", arg) } + if build.CacheTo != "" { args = append(args, "--cache-to", build.CacheTo) } + for _, arg := range build.ArgsEnv.Value() { addProxyValue(&build, arg) } + for _, arg := range append(defaultBuildArgs, build.Args.Value()...) { args = append(args, "--build-arg", arg) } + for _, host := range build.AddHost.Value() { args = append(args, "--add-host", host) } + if build.Target != "" { args = append(args, "--target", build.Target) } + if build.Quiet { args = append(args, "--quiet") } + if build.Output != "" { args = append(args, "--output", build.Output) } + for _, arg := range build.NamedContext.Value() { args = append(args, "--build-context", arg) } @@ -135,10 +156,10 @@ func commandBuild(build Build, dryrun bool) *exec.Cmd { args = append(args, "--provenance", build.Provenance) } - return exec.Command(dockerExe, args...) + return execabs.Command(dockerBin, args...) } -// helper function to add proxy values from the environment +// helper function to add proxy values from the environment. func addProxyBuildArgs(build *Build) { addProxyValue(build, "http_proxy") addProxyValue(build, "https_proxy") @@ -182,7 +203,7 @@ func hasProxyBuildArg(build *Build, key string) bool { } // helper function to create the docker daemon command. -func commandDaemon(daemon Daemon) *exec.Cmd { +func commandDaemon(daemon Daemon) *execabs.Cmd { args := []string{ "--data-root", daemon.StoragePath, "--host=unix:///var/run/docker.sock", @@ -191,35 +212,44 @@ func commandDaemon(daemon Daemon) *exec.Cmd { if daemon.StorageDriver != "" { args = append(args, "-s", daemon.StorageDriver) } + if daemon.Insecure && daemon.Registry != "" { args = append(args, "--insecure-registry", daemon.Registry) } + if daemon.IPv6 { args = append(args, "--ipv6") } + if len(daemon.Mirror) != 0 { args = append(args, "--registry-mirror", daemon.Mirror) } + if len(daemon.Bip) != 0 { args = append(args, "--bip", daemon.Bip) } + for _, dns := range daemon.DNS.Value() { args = append(args, "--dns", dns) } + for _, dnsSearch := range daemon.DNSSearch.Value() { args = append(args, "--dns-search", dnsSearch) } + if len(daemon.MTU) != 0 { args = append(args, "--mtu", daemon.MTU) } + if daemon.Experimental { args = append(args, "--experimental") } - return exec.Command(dockerdExe, args...) + + return execabs.Command(dockerdBin, args...) } // trace writes each command to stdout with the command wrapped in an xml // tag so that it can be extracted and displayed in the logs. -func trace(cmd *exec.Cmd) { +func trace(cmd *execabs.Cmd) { fmt.Fprintf(os.Stdout, "+ %s\n", strings.Join(cmd.Args, " ")) } diff --git a/plugin/docker_test.go b/plugin/docker_test.go deleted file mode 100644 index b0736c3..0000000 --- a/plugin/docker_test.go +++ /dev/null @@ -1 +0,0 @@ -package plugin diff --git a/plugin/impl.go b/plugin/impl.go index 2cb10bf..4340588 100644 --- a/plugin/impl.go +++ b/plugin/impl.go @@ -3,12 +3,12 @@ package plugin import ( "fmt" "os" - "os/exec" "path/filepath" "time" "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" + "golang.org/x/sys/execabs" ) // Daemon defines Docker daemon parameters. @@ -74,6 +74,8 @@ type Settings struct { Dryrun bool } +const strictFilePerm = 0o600 + // Validate handles the settings validation of the plugin. func (p *Plugin) Validate() error { p.settings.Build.Branch = p.pipeline.Repo.Branch @@ -92,11 +94,14 @@ func (p *Plugin) Validate() error { ) if err != nil { logrus.Infof("cannot generate tags from %s, invalid semantic version", p.settings.Build.Ref) + return err } + p.settings.Build.Tags = *cli.NewStringSlice(tag...) } else { logrus.Infof("skip auto-tagging for %s, not on default branch or tag", p.settings.Build.Ref) + return nil } } @@ -115,55 +120,59 @@ func (p *Plugin) Execute() error { // ready to accept connections before we proceed. for i := 0; i < 15; i++ { cmd := commandInfo() + err := cmd.Run() if err == nil { break } + time.Sleep(time.Second * 1) } // Create Auth Config File if p.settings.Login.Config != "" { - if err := os.MkdirAll(dockerHome, 0o600); err != nil { - return fmt.Errorf("failed to create docker home: %s", err) + if err := os.MkdirAll(dockerHome, strictFilePerm); err != nil { + return fmt.Errorf("failed to create docker home: %w", err) } path := filepath.Join(dockerHome, "config.json") - err := os.WriteFile(path, []byte(p.settings.Login.Config), 0o600) + + err := os.WriteFile(path, []byte(p.settings.Login.Config), strictFilePerm) if err != nil { - return fmt.Errorf("error writing config.json: %s", err) + return fmt.Errorf("error writing config.json: %w", err) } } // login to the Docker registry if p.settings.Login.Password != "" { cmd := commandLogin(p.settings.Login) + err := cmd.Run() if err != nil { - return fmt.Errorf("error authenticating: %s", err) + return fmt.Errorf("error authenticating: %w", err) } } if p.settings.Daemon.BuildkitConfig != "" { - err := os.WriteFile(buildkitConfig, []byte(p.settings.Daemon.BuildkitConfig), 0o600) + err := os.WriteFile(buildkitConfig, []byte(p.settings.Daemon.BuildkitConfig), strictFilePerm) if err != nil { - return fmt.Errorf("error writing buildkit.toml: %s", err) + return fmt.Errorf("error writing buildkit.toml: %w", err) } } switch { case p.settings.Login.Password != "": - fmt.Println("Detected registry credentials") + logrus.Info("Detected registry credentials") case p.settings.Login.Config != "": - fmt.Println("Detected registry credentials file") + logrus.Info("Detected registry credentials file") default: - fmt.Println("Registry credentials or Docker config not provided. Guest mode enabled.") + logrus.Info("Registry credentials or Docker config not provided. Guest mode enabled.") } // add proxy build args addProxyBuildArgs(&p.settings.Build) - var cmds []*exec.Cmd + var cmds []*execabs.Cmd cmds = append(cmds, commandVersion()) // docker version cmds = append(cmds, commandInfo()) // docker info cmds = append(cmds, commandBuilder(p.settings.Daemon)) 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/tags.go b/plugin/tags.go index e5307ad..7a04867 100644 --- a/plugin/tags.go +++ b/plugin/tags.go @@ -14,9 +14,11 @@ func DefaultTagSuffix(ref, suffix string) ([]string, error) { if err != nil { return nil, err } + if len(suffix) == 0 { return tags, nil } + for i, tag := range tags { if tag == "latest" { tags[i] = suffix @@ -24,13 +26,15 @@ func DefaultTagSuffix(ref, suffix string) ([]string, error) { tags[i] = fmt.Sprintf("%s-%s", tag, suffix) } } + return tags, nil } func splitOff(input, delim string) string { - parts := strings.SplitN(input, delim, 2) + const splits = 2 + parts := strings.SplitN(input, delim, splits) - if len(parts) == 2 { + if len(parts) == splits { return parts[0] } @@ -43,42 +47,65 @@ func DefaultTags(ref string) ([]string, error) { if !strings.HasPrefix(ref, "refs/tags/") { return []string{"latest"}, nil } - v := stripTagPrefix(ref) - version, err := semver.NewVersion(v) + + rawVersion := stripTagPrefix(ref) + + version, err := semver.NewVersion(rawVersion) if err != nil { return []string{"latest"}, err } + if version.PreRelease != "" || version.Metadata != "" { return []string{ version.String(), }, nil } - v = stripTagPrefix(ref) - v = splitOff(splitOff(v, "+"), "-") - dotParts := strings.SplitN(v, ".", 3) + rawVersion = stripTagPrefix(ref) + rawVersion = splitOff(splitOff(rawVersion, "+"), "-") + //nolint:gomnd + dotParts := strings.SplitN(rawVersion, ".", 3) if version.Major == 0 { return []string{ fmt.Sprintf("%0*d.%0*d", len(dotParts[0]), version.Major, len(dotParts[1]), version.Minor), - fmt.Sprintf("%0*d.%0*d.%0*d", len(dotParts[0]), version.Major, len(dotParts[1]), version.Minor, len(dotParts[2]), version.Patch), + fmt.Sprintf( + "%0*d.%0*d.%0*d", + len(dotParts[0]), + version.Major, + len(dotParts[1]), + version.Minor, + len(dotParts[2]), + version.Patch, + ), }, nil } + return []string{ fmt.Sprintf("%0*d", len(dotParts[0]), version.Major), fmt.Sprintf("%0*d.%0*d", len(dotParts[0]), version.Major, len(dotParts[1]), version.Minor), - fmt.Sprintf("%0*d.%0*d.%0*d", len(dotParts[0]), version.Major, len(dotParts[1]), version.Minor, len(dotParts[2]), version.Patch), + fmt.Sprintf( + "%0*d.%0*d.%0*d", + len(dotParts[0]), + version.Major, + len(dotParts[1]), + version.Minor, + len(dotParts[2]), + version.Patch, + ), }, nil } -// UseDefaultTag for keep only default branch for latest tag +// UseDefaultTag to keep only default branch for latest tag. func UseDefaultTag(ref, defaultBranch string) bool { if strings.HasPrefix(ref, "refs/tags/") { return true } + if stripHeadPrefix(ref) == defaultBranch { return true } + return false } @@ -89,5 +116,6 @@ func stripHeadPrefix(ref string) string { func stripTagPrefix(ref string) string { ref = strings.TrimPrefix(ref, "refs/tags/") ref = strings.TrimPrefix(ref, "v") + return ref } diff --git a/plugin/tags_test.go b/plugin/tags_test.go index 27218de..0624155 100644 --- a/plugin/tags_test.go +++ b/plugin/tags_test.go @@ -40,8 +40,10 @@ func TestDefaultTags(t *testing.T) { tags, err := DefaultTags(test.Before) if err != nil { t.Error(err) + continue } + got, want := tags, test.After if !reflect.DeepEqual(got, want) { t.Errorf("Got tag %v, want %v", got, want) @@ -123,8 +125,10 @@ func TestDefaultTagSuffix(t *testing.T) { tag, err := DefaultTagSuffix(test.Before, test.Suffix) if err != nil { t.Error(err) + continue } + got, want := tag, test.After if !reflect.DeepEqual(got, want) { t.Errorf("Got tag %v, want %v", got, want) @@ -136,6 +140,7 @@ func Test_stripHeadPrefix(t *testing.T) { type args struct { ref string } + tests := []struct { args args want string @@ -147,6 +152,7 @@ func Test_stripHeadPrefix(t *testing.T) { want: "main", }, } + for _, tt := range tests { if got := stripHeadPrefix(tt.args.ref); got != tt.want { t.Errorf("stripHeadPrefix() = %v, want %v", got, tt.want) @@ -159,6 +165,7 @@ func TestUseDefaultTag(t *testing.T) { ref string defaultBranch string } + tests := []struct { name string args args @@ -189,6 +196,7 @@ func TestUseDefaultTag(t *testing.T) { want: false, }, } + for _, tt := range tests { if got := UseDefaultTag(tt.args.ref, tt.args.defaultBranch); got != tt.want { t.Errorf("%q. UseDefaultTag() = %v, want %v", tt.name, got, tt.want)