diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..927a5f4816 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,87 @@ +version: "2" + +run: + modules-download-mode: vendor + +linters: + default: none + enable: + - bodyclose + - depguard + - forbidigo + - gocritic + - gosec + - govet + - ineffassign + - makezero + - misspell + - noctx + - nolintlint + - revive + - staticcheck + - testifylint + - unused + - whitespace + settings: + depguard: + rules: + main: + deny: + - pkg: "io/ioutil" + desc: The io/ioutil package has been deprecated. + forbidigo: + forbid: + - pattern: ^fmt\.Errorf(# use errors\.Errorf instead)?$ + - pattern: ^platforms\.DefaultString(# use platforms\.Format(platforms\.DefaultSpec()) instead\.)?$ + importas: + alias: + - pkg: "github.com/opencontainers/image-spec/specs-go/v1" + alias: "ocispecs" + - pkg: "github.com/opencontainers/go-digest" + alias: "digest" + testifylint: + disable: + - empty + - bool-compare + - len + - negative-positive + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - revive + text: stutters + - linters: + - revive + text: var-naming + - linters: + - revive + text: empty-block + - linters: + - revive + text: superfluous-else + - linters: + - revive + text: unused-parameter + - linters: + - revive + text: redefines-builtin-id + - linters: + - revive + text: if-return + +formatters: + enable: + - gofmt + - goimports + exclusions: + generated: lax + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/cmd/gotestmetrics/parse.go b/cmd/gotestmetrics/parse.go index 3ed196fd5c..e172b78b82 100644 --- a/cmd/gotestmetrics/parse.go +++ b/cmd/gotestmetrics/parse.go @@ -40,7 +40,7 @@ func (c *parseCmd) Run(ctx *Context) error { if err != nil { return errors.Wrap(err, "failed to marshal result") } - if err := os.WriteFile(c.Output, dt, 0644); err != nil { + if err := os.WriteFile(c.Output, dt, 0600); err != nil { return errors.Wrap(err, "failed to write result to output file") } } diff --git a/cmd/refcandidates/main.go b/cmd/refcandidates/main.go index 054d788672..20ad2719a9 100644 --- a/cmd/refcandidates/main.go +++ b/cmd/refcandidates/main.go @@ -1,13 +1,13 @@ package main import ( + "crypto/rand" "encoding/json" "log" - "math/rand" + "math/big" "os" "path/filepath" "strings" - "time" "github.com/alecthomas/kong" "github.com/moby/buildkit-bench/util/candidates" @@ -60,7 +60,7 @@ func writeFile(f string, c *candidates.Candidates) error { if err != nil { return errors.Wrap(err, "failed to marshal candidates") } - if err := os.WriteFile(f, dt, 0644); err != nil { + if err := os.WriteFile(f, dt, 0600); err != nil { return errors.Wrap(err, "failed to write candidates to output file") } log.Printf("Candidates written to %q", f) @@ -106,12 +106,15 @@ func setGhaOutput(name string, c *candidates.Candidates) error { } if gha.IsPullRequestEvent() { - r := rand.New(rand.NewSource(time.Now().UnixNano())) if len(includes) > 2 { s := make([]include, 0, 2) si := make(map[int]struct{}) for len(s) < 2 { - idx := r.Intn(len(includes)) + n, err := rand.Int(rand.Reader, big.NewInt(int64(len(includes)))) + if err != nil { + return errors.Wrap(err, "failed to sample candidate") + } + idx := int(n.Int64()) if _, exists := si[idx]; !exists { si[idx] = struct{}{} s = append(s, includes[idx]) diff --git a/docker-bake.hcl b/docker-bake.hcl index fe8f9b1c08..7d7ea9dc50 100644 --- a/docker-bake.hcl +++ b/docker-bake.hcl @@ -77,7 +77,7 @@ group "default" { } group "validate" { - targets = ["validate-vendor"] + targets = ["lint", "validate-vendor"] } target "_common" { @@ -191,6 +191,12 @@ target "vendor" { output = ["."] } +target "lint" { + inherits = ["_common"] + dockerfile = "./hack/dockerfiles/lint.Dockerfile" + output = ["type=cacheonly"] +} + variable "WEBSITE_PUBLIC_PATH" { default = null } diff --git a/hack/dockerfiles/lint.Dockerfile b/hack/dockerfiles/lint.Dockerfile new file mode 100644 index 0000000000..c0bb6d0d80 --- /dev/null +++ b/hack/dockerfiles/lint.Dockerfile @@ -0,0 +1,13 @@ +# syntax=docker/dockerfile:1 + +ARG GO_VERSION=1.25 +ARG ALPINE_VERSION=3.23 +ARG GOLANGCI_LINT_VERSION=v2.8.0 + +FROM golang:${GO_VERSION}-alpine${ALPINE_VERSION} +RUN apk add --no-cache gcc musl-dev +ARG GOLANGCI_LINT_VERSION +RUN wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s ${GOLANGCI_LINT_VERSION} +WORKDIR /go/src/github.com/moby/buildkit-bench +RUN --mount=target=. --mount=target=/root/.cache,type=cache \ + golangci-lint run diff --git a/test/build_test.go b/test/build_test.go index 2d699333d6..3617c52ac3 100644 --- a/test/build_test.go +++ b/test/build_test.go @@ -2,6 +2,7 @@ package test import ( "fmt" + "os" "strings" "sync" "testing" @@ -9,6 +10,7 @@ import ( "github.com/containerd/continuity/fs/fstest" "github.com/containerd/platforms" "github.com/moby/buildkit-bench/util/testutil" + "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -59,7 +61,7 @@ func BenchmarkBuild(b *testing.B) { benchmarkBuildExportUncompressed, benchmarkBuildExportGzip, benchmarkBuildExportEstargz, - //benchmarkBuildExportZstd, https://github.com/moby/buildkit-bench/pull/146#discussion_r1771519112 + // benchmarkBuildExportZstd, https://github.com/moby/buildkit-bench/pull/146#discussion_r1771519112 ), testutil.WithMirroredImages(mirroredImages), ) @@ -172,20 +174,31 @@ RUN cp /etc/foo /etc/bar } var wg sync.WaitGroup + errCh := make(chan error, n) for i := 0; i < n; i++ { wg.Add(1) go func() { defer wg.Done() - dir := tmpdir( - b, + dir, err := os.MkdirTemp("", "bkbench-build-context") + if err != nil { + errCh <- err + return + } + defer os.RemoveAll(dir) + if err := fstest.Apply( fstest.CreateFile("Dockerfile", dockerfile, 0600), fstest.CreateFile("foo", []byte("foo"), 0600), - ) + ).Apply(dir); err != nil { + errCh <- err + return + } out, err := buildxBuildCmd(sb, withArgs("--output=type=image", dir)) // TODO: use sb.WriteLogFile to write buildx logs in a defer with a // semaphore using a buffered channel to limit the number of // concurrent goroutines. This might affect timing. - require.NoError(b, err, out) + if err != nil { + errCh <- errors.Wrap(err, out) + } }() } reportBuildkitdAlloc(b, sb, func() { @@ -193,6 +206,10 @@ RUN cp /etc/foo /etc/bar wg.Wait() b.StopTimer() }) + close(errCh) + for err := range errCh { + require.NoError(b, err) + } } func benchmarkBuildFileTransfer(b *testing.B, sb testutil.Sandbox) { @@ -290,9 +307,7 @@ func benchmarkBuildExportGzip(b *testing.B, sb testutil.Sandbox) { func benchmarkBuildExportEstargz(b *testing.B, sb testutil.Sandbox) { benchmarkBuildExport(b, sb, "gzip") } -func benchmarkBuildExportZstd(b *testing.B, sb testutil.Sandbox) { - benchmarkBuildExport(b, sb, "zstd") -} + func benchmarkBuildExport(b *testing.B, sb testutil.Sandbox, compression string) { dockerfile := []byte(` FROM python:latest diff --git a/test/buildx_test.go b/test/buildx_test.go index 849f1abb2f..7e3f133a8d 100644 --- a/test/buildx_test.go +++ b/test/buildx_test.go @@ -1,6 +1,7 @@ package test import ( + "context" "os" "os/exec" "testing" @@ -23,14 +24,14 @@ func BenchmarkBuildx(b *testing.B) { } func testBuildxVersion(t *testing.T, sb testutil.Sandbox) { - output, err := exec.Command(sb.BuildxBin(), "version").Output() + output, err := exec.CommandContext(context.Background(), sb.BuildxBin(), "version").Output() //nolint:gosec // test utility require.NoError(t, err) t.Log(string(output)) } func benchmarkBuildxVersion(b *testing.B, sb testutil.Sandbox) { b.StartTimer() - err := exec.Command(sb.BuildxBin(), "version").Run() + err := exec.CommandContext(context.Background(), sb.BuildxBin(), "version").Run() //nolint:gosec // test utility b.StopTimer() require.NoError(b, err) } diff --git a/test/daemon_test.go b/test/daemon_test.go index ae2bbe0fdd..fb1a4cf3e1 100644 --- a/test/daemon_test.go +++ b/test/daemon_test.go @@ -1,6 +1,7 @@ package test import ( + "context" "encoding/json" "fmt" "net/http" @@ -32,7 +33,7 @@ func BenchmarkDaemon(b *testing.B) { func testDaemonVersion(t *testing.T, sb testutil.Sandbox) { buildkitdPath := path.Join(sb.BuildKitBinsDir(), sb.Name(), "buildkitd") - output, err := exec.Command(buildkitdPath, "--version").Output() + output, err := exec.CommandContext(context.Background(), buildkitdPath, "--version").Output() require.NoError(t, err) versionParts := strings.Fields(string(output)) @@ -46,7 +47,9 @@ func testDaemonVersion(t *testing.T, sb testutil.Sandbox) { func testDaemonDebugHeap(t *testing.T, sb testutil.Sandbox) { client := &http.Client{} - resp, err := client.Get(fmt.Sprintf("http://%s/debug/pprof/heap?debug=1", sb.DebugAddress())) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, fmt.Sprintf("http://%s/debug/pprof/heap?debug=1", sb.DebugAddress()), nil) + require.NoError(t, err) + resp, err := client.Do(req) require.NoError(t, err) defer resp.Body.Close() @@ -63,7 +66,7 @@ func testDaemonDebugHeap(t *testing.T, sb testutil.Sandbox) { func benchmarkDaemonVersion(b *testing.B, sb testutil.Sandbox) { buildkitdPath := path.Join(sb.BuildKitBinsDir(), sb.Name(), "buildkitd") b.StartTimer() - err := exec.Command(buildkitdPath, "--version").Run() + err := exec.CommandContext(context.Background(), buildkitdPath, "--version").Run() b.StopTimer() require.NoError(b, err) } diff --git a/test/util.go b/test/util.go index b14de13612..c3ef2e38b4 100644 --- a/test/util.go +++ b/test/util.go @@ -1,11 +1,11 @@ package test import ( + "context" "fmt" "net/http" "os" "os/exec" - "path" "testing" "github.com/containerd/continuity/fs/fstest" @@ -24,12 +24,6 @@ func tmpdir(tb testing.TB, appliers ...fstest.Applier) string { type cmdOpt func(*exec.Cmd) -func withEnv(env ...string) cmdOpt { - return func(cmd *exec.Cmd) { - cmd.Env = append(cmd.Env, env...) - } -} - func withArgs(args ...string) cmdOpt { return func(cmd *exec.Cmd) { cmd.Args = append(cmd.Args, args...) @@ -43,7 +37,7 @@ func withDir(dir string) cmdOpt { } func buildxCmd(sb testutil.Sandbox, opts ...cmdOpt) *exec.Cmd { - cmd := exec.Command(sb.BuildxBin()) + cmd := exec.CommandContext(context.Background(), sb.BuildxBin()) //nolint:gosec // test utility cmd.Env = append([]string{}, os.Environ()...) for _, opt := range opts { opt(cmd) @@ -64,26 +58,6 @@ func buildxBuildCmd(sb testutil.Sandbox, opts ...cmdOpt) (string, error) { return string(out), err } -func buildctlCmd(sb testutil.Sandbox, opts ...cmdOpt) *exec.Cmd { - cmd := exec.Command(path.Join(sb.BuildKitBinsDir(), sb.Name(), "buildctl")) - cmd.Args = append(cmd.Args, "--debug") - if buildkitAddr := sb.Address(); buildkitAddr != "" { - cmd.Args = append(cmd.Args, "--addr", buildkitAddr) - } - cmd.Env = append([]string{}, os.Environ()...) - for _, opt := range opts { - opt(cmd) - } - return cmd -} - -func buildctlBuildCmd(sb testutil.Sandbox, opts ...cmdOpt) (string, error) { - opts = append([]cmdOpt{withArgs("build", "--frontend=dockerfile.v0")}, opts...) - cmd := buildctlCmd(sb, opts...) - out, err := cmd.CombinedOutput() - return string(out), err -} - func reportBuildkitdAlloc(b *testing.B, sb testutil.Sandbox, cb func()) { beforeAlloc, errb := buildkitdAlloc(sb) cb() @@ -95,7 +69,11 @@ func reportBuildkitdAlloc(b *testing.B, sb testutil.Sandbox, cb func()) { func buildkitdAlloc(sb testutil.Sandbox) (int64, error) { client := &http.Client{} - resp, err := client.Get(fmt.Sprintf("http://%s/debug/pprof/heap?gc=1", sb.DebugAddress())) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, fmt.Sprintf("http://%s/debug/pprof/heap?gc=1", sb.DebugAddress()), nil) + if err != nil { + return 0, err + } + resp, err := client.Do(req) if err != nil { return 0, err } diff --git a/util/candidates/candidates.go b/util/candidates/candidates.go index 589a5e4678..b46a92f96b 100644 --- a/util/candidates/candidates.go +++ b/util/candidates/candidates.go @@ -159,11 +159,12 @@ func (c *Candidates) setCommits(lastDays int) error { } res := make(map[string]Commit) for date, cm := range lastCommitByDay(filterMergeCommits(commits)) { - if containsCommitSha(c.Refs, *cm.SHA) { + switch { + case containsCommitSha(c.Refs, *cm.SHA): log.Printf("Skipping commit %s, already in refs", *cm.SHA) - } else if containsCommitSha(c.Releases, *cm.SHA) { + case containsCommitSha(c.Releases, *cm.SHA): log.Printf("Skipping commit %s, already in releases", *cm.SHA) - } else { + default: res[date] = Commit{ SHA: *cm.SHA, Date: *cm.Commit.Committer.Date.GetTime(), diff --git a/util/gotest/benchmark.go b/util/gotest/benchmark.go index e5c4a8fae8..cab05b710a 100644 --- a/util/gotest/benchmark.go +++ b/util/gotest/benchmark.go @@ -150,15 +150,16 @@ func (b *BenchmarkInfo) update(output string) bool { return false } // https://github.com/golang/go/blob/f38d42f2c4c6ad0d7cbdad5e1417cac3be2a5dcb/src/testing/benchmark.go#L246-L255 - if strings.HasPrefix(output, "goos: ") { + switch { + case strings.HasPrefix(output, "goos: "): b.OS = strings.TrimPrefix(output, "goos: ") - } else if strings.HasPrefix(output, "goarch: ") { + case strings.HasPrefix(output, "goarch: "): b.Architecture = strings.TrimPrefix(output, "goarch: ") - } else if strings.HasPrefix(output, "pkg: ") { + case strings.HasPrefix(output, "pkg: "): b.Package = strings.TrimPrefix(output, "pkg: ") - } else if strings.HasPrefix(output, "cpu: ") { + case strings.HasPrefix(output, "cpu: "): b.CPU = strings.TrimPrefix(output, "cpu: ") - } else { + default: return false } return true diff --git a/util/gotest/benchmark/parse.go b/util/gotest/benchmark/parse.go index 82f3a935aa..170606eccf 100644 --- a/util/gotest/benchmark/parse.go +++ b/util/gotest/benchmark/parse.go @@ -5,10 +5,11 @@ package benchmark import ( "bufio" - "fmt" "io" "strconv" "strings" + + "github.com/pkg/errors" ) // Flags used by Benchmark.Measured to indicate @@ -40,10 +41,10 @@ func ParseLine(line string) (*Benchmark, error) { // Two required, positional fields: Name and iterations. if len(fields) < 2 { - return nil, fmt.Errorf("two fields required, have %d", len(fields)) + return nil, errors.Errorf("two fields required, have %d", len(fields)) } if !strings.HasPrefix(fields[0], "Benchmark") { - return nil, fmt.Errorf(`first field does not start with "Benchmark"`) + return nil, errors.Errorf(`first field does not start with "Benchmark"`) } n, err := strconv.Atoi(fields[1]) if err != nil { diff --git a/util/gotest/parse.go b/util/gotest/parse.go index 66dfc3f540..24a1c635e9 100644 --- a/util/gotest/parse.go +++ b/util/gotest/parse.go @@ -65,7 +65,7 @@ func (e *eventHandler) Event(event testjson.TestEvent, _ *testjson.Execution) er return nil } if e.log != nil { - log.Printf(event.Output) + e.log.Printf("%s", event.Output) } e.mu.Lock() diff --git a/util/testutil/ref.go b/util/testutil/ref.go index cf60892761..daf31776d7 100644 --- a/util/testutil/ref.go +++ b/util/testutil/ref.go @@ -124,7 +124,7 @@ func (c *Ref) New(ctx context.Context, cfg *BackendConfig) (b Backend, cl func() // Create a remote buildx instance builderName := "remote-" + identity.NewID() buildxConfigDir := filepath.Join(tmpdir, "buildx") - cmd := exec.Command(cfg.BuildxBin, "create", + cmd := exec.CommandContext(ctx, cfg.BuildxBin, "create", //nolint:gosec // test utility "--bootstrap", "--name", builderName, "--driver", "remote", @@ -135,7 +135,7 @@ func (c *Ref) New(ctx context.Context, cfg *BackendConfig) (b Backend, cl func() return nil, nil, errors.Wrapf(err, "failed to create buildx instance %s", builderName) } deferF.Append(func() error { - cmd := exec.Command(cfg.BuildxBin, "rm", "-f", builderName) + cmd := exec.CommandContext(context.Background(), cfg.BuildxBin, "rm", "-f", builderName) //nolint:gosec // test utility cmd.Env = append(os.Environ(), "BUILDX_CONFIG="+buildxConfigDir) return cmd.Run() }) diff --git a/util/testutil/registry.go b/util/testutil/registry.go index c0aa683570..6932d07196 100644 --- a/util/testutil/registry.go +++ b/util/testutil/registry.go @@ -56,7 +56,7 @@ http: } } - cmd := exec.Command("registry", "serve", filepath.Join(dir, "config.yaml")) //nolint:gosec // test utility + cmd := exec.CommandContext(context.Background(), "registry", "serve", filepath.Join(dir, "config.yaml")) //nolint:gosec // test utility rc, err := cmd.StderrPipe() if err != nil { return "", nil, err @@ -68,8 +68,8 @@ http: deferF.Append(stop) ctx, cancel := context.WithCancelCause(context.Background()) - ctx, _ = context.WithTimeoutCause(ctx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + ctx, _ = context.WithTimeoutCause(ctx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) //nolint:govet + defer func() { cancel(errors.WithStack(context.Canceled)) }() url, err = detectPort(ctx, rc) if err != nil { return "", nil, err diff --git a/util/testutil/run.go b/util/testutil/run.go index 59e5e641e2..eba6232665 100644 --- a/util/testutil/run.go +++ b/util/testutil/run.go @@ -19,9 +19,9 @@ import ( "github.com/containerd/containerd/v2/core/content" "github.com/containerd/containerd/v2/core/remotes/docker" + "github.com/docker/cli/cli/config" "github.com/gofrs/flock" "github.com/moby/buildkit/util/appcontext" - "github.com/docker/cli/cli/config" "github.com/moby/buildkit/util/bklog" "github.com/moby/buildkit/util/contentutil" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" @@ -266,7 +266,7 @@ func Run(tb testing.TB, runners []Runner, opt ...TestOpt) { runWithSandbox := func(tb testing.TB) { sb, closer, err := newSandbox(ctx, br, mirror, mv) - require.NoError(tb, err, errors.Unwrap(err)) + require.NoError(tb, err) tb.Cleanup(func() { _ = closer() }) defer func() { sb.WriteLogs(tb) @@ -328,7 +328,7 @@ func writeConfig(updaters []ConfigUpdater) (string, error) { s = upt.UpdateConfigFile(s) } - if err := os.WriteFile(filepath.Join(tmpdir, buildkitdConfigFile), []byte(s), 0644); err != nil { + if err := os.WriteFile(filepath.Join(tmpdir, buildkitdConfigFile), []byte(s), 0600); err != nil { return "", err } return filepath.Join(tmpdir, buildkitdConfigFile), nil diff --git a/util/testutil/sandbox.go b/util/testutil/sandbox.go index a225ec2585..595578881a 100644 --- a/util/testutil/sandbox.go +++ b/util/testutil/sandbox.go @@ -62,7 +62,7 @@ func (sb *sandbox) WriteLogFile(tb testing.TB, name string, dt []byte) { if err := os.MkdirAll(testLogsDir, 0755); err != nil { tb.Fatalf("failed to create logs directory: %v", err) } - if err := os.WriteFile(filepath.Join(testLogsDir, fmt.Sprintf("%s-run%s.log", name, run)), dt, 0644); err != nil { + if err := os.WriteFile(filepath.Join(testLogsDir, fmt.Sprintf("%s-run%s.log", name, run)), dt, 0600); err != nil { tb.Fatalf("writing log file: %v", err) } } diff --git a/util/testutil/util.go b/util/testutil/util.go index 5088a81207..e20961be09 100644 --- a/util/testutil/util.go +++ b/util/testutil/util.go @@ -29,7 +29,7 @@ func runBuildkitd(conf *BackendConfig, tmpdir string, args []string, logs map[st } }() - cfgfile, err := writeConfig(append(conf.DaemonConfig)) + cfgfile, err := writeConfig(conf.DaemonConfig) if err != nil { return "", "", nil, err } @@ -40,7 +40,7 @@ func runBuildkitd(conf *BackendConfig, tmpdir string, args []string, logs map[st args = append(args, "--config="+cfgfile) address = getBuildkitdAddr(tmpdir) - l, err := net.Listen("tcp", "localhost:0") + l, err := (&net.ListenConfig{}).Listen(context.Background(), "tcp", "localhost:0") if err != nil { return "", "", nil, err } @@ -50,7 +50,7 @@ func runBuildkitd(conf *BackendConfig, tmpdir string, args []string, logs map[st } args = append(args, "--root", tmpdir, "--addr", address, "--debug", "--debugaddr", debugAddress) - cmd := exec.Command(args[0], args[1:]...) //nolint:gosec // test utility + cmd := exec.CommandContext(context.Background(), args[0], args[1:]...) //nolint:gosec // test utility cmd.Env = append( os.Environ(), "BUILDKIT_DEBUG_EXEC_OUTPUT=1",