diff --git a/Dockerfile b/Dockerfile index 13e439d2e692..2cd0fd2246fb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,10 +16,10 @@ ARG MINIO_VERSION=RELEASE.2025-09-07T16-13-09Z ARG MINIO_MC_VERSION=RELEASE.2025-08-13T08-35-41Z ARG AZURITE_VERSION=3.35.0 ARG GOTESTSUM_VERSION=v1.13.0 -ARG DELVE_VERSION=v1.26.0 +ARG DELVE_VERSION=v1.26.3 ARG DOCKER_VERSION=29.4 ARG DOCKER_CLI_VERSION=${DOCKER_VERSION} -ARG BUILDX_VERSION=0.31.1 +ARG BUILDX_VERSION=0.34.0 ARG EXPORT_BASE=alpine ARG ALPINE_VERSION=3.23 diff --git a/cache/blobs.go b/cache/blobs.go index 09836772d08a..3acc23c00997 100644 --- a/cache/blobs.go +++ b/cache/blobs.go @@ -109,7 +109,7 @@ func computeBlobChain(ctx context.Context, sr *immutableRef, createIfNeeded bool } defer func() { if err != nil { - l.Discard() + l.Discard(ctx) } }() @@ -461,7 +461,7 @@ func ensureCompression(ctx context.Context, ref *immutableRef, comp compression. } defer func() { if err != nil { - l.Discard() + l.Discard(ctx) } }() diff --git a/cache/manager.go b/cache/manager.go index bd109c9ebafc..7cb408115362 100644 --- a/cache/manager.go +++ b/cache/manager.go @@ -235,7 +235,7 @@ func (cm *cacheManager) GetByBlob(ctx context.Context, desc ocispecs.Descriptor, snapshotID := chainID.String() if link != nil { snapshotID = link.getSnapshotID() - go link.Release(context.TODO()) + go link.Release(context.WithoutCancel(ctx)) } l, err := cm.LeaseManager.Create(ctx, func(l *leases.Lease) error { diff --git a/cache/refs.go b/cache/refs.go index 220c629f7646..1ad7caf473fc 100644 --- a/cache/refs.go +++ b/cache/refs.go @@ -1519,8 +1519,9 @@ func (cr *cacheRecord) finalize(ctx context.Context) error { go func() { cr.cm.mu.Lock() defer cr.cm.mu.Unlock() - if err := mutable.remove(context.TODO(), true); err != nil { - bklog.G(ctx).Error(err) + cleanupCtx := context.WithoutCancel(ctx) + if err := mutable.remove(cleanupCtx, true); err != nil { + bklog.G(cleanupCtx).Error(err) } }() diff --git a/client/build_test.go b/client/build_test.go index 95238e37648d..461d6cbcb0a1 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -2349,7 +2349,7 @@ func testClientGatewayCanceledCredentialsCallbackReturns(t *testing.T, sb integr t.Cleanup(proxyServer.Close) ref := strings.TrimPrefix(proxyServer.URL, "http://") + "/" + repo + ":latest" - host := strings.SplitN(ref, "/", 2)[0] + host, _, _ := strings.Cut(ref, "/") started := make(chan struct{}) release := make(chan struct{}) diff --git a/client/client_test.go b/client/client_test.go index 1ea3b36ba117..7c76d0aa8475 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -13726,8 +13726,12 @@ func testGitBundleRoundTrip(t *testing.T, sb integration.Sandbox) { localBare := t.TempDir() err = runInDir(localBare, "git init --bare") require.NoError(t, err) - bundlePath := filepath.Join(t.TempDir(), "bundle.pack") - require.NoError(t, os.WriteFile(bundlePath, bundleBytes, 0644)) + bundleRoot := t.TempDir() + root, err := os.OpenRoot(bundleRoot) + require.NoError(t, err) + require.NoError(t, root.WriteFile("bundle.pack", bundleBytes, 0644)) + require.NoError(t, root.Close()) + bundlePath := filepath.Join(bundleRoot, "bundle.pack") err = runInDir(localBare, fmt.Sprintf("git fetch %s +refs/*:refs/*", bundlePath)) require.NoError(t, err) //nolint:gosec // Test-controlled temp dir and commit SHA are not attacker input. diff --git a/control/control.go b/control/control.go index caf6f045d746..e4704fb3919b 100644 --- a/control/control.go +++ b/control/control.go @@ -81,8 +81,7 @@ type Opt struct { } type Controller struct { // TODO: ControlService - // buildCount needs to be 64bit aligned - buildCount int64 + buildCount atomic.Int64 opt Opt solver *llbsolver.Solver history *history.Queue @@ -213,7 +212,7 @@ func (c *Controller) releaseUnreferencedCache(ctx context.Context) error { } func (c *Controller) Prune(req *controlapi.PruneRequest, stream controlapi.Control_PruneServer) error { - if atomic.LoadInt64(&c.buildCount) == 0 { + if c.buildCount.Load() == 0 { imageutil.CancelCacheLeases() } @@ -383,8 +382,8 @@ func translateLegacySolveRequest(req *controlapi.SolveRequest) { func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*controlapi.SolveResponse, error) { defer trace.StartRegion(ctx, "Solve").End() trace.Logf(ctx, "Request", "solve request: %v", req.Ref) - atomic.AddInt64(&c.buildCount, 1) - defer atomic.AddInt64(&c.buildCount, -1) + c.buildCount.Add(1) + defer c.buildCount.Add(-1) if req.Cache == nil { req.Cache = &controlapi.CacheOptions{} // make sure cache options are initialized diff --git a/docs/generate.go b/docs/generate.go index 4b710371e9e8..70c3044d4760 100644 --- a/docs/generate.go +++ b/docs/generate.go @@ -15,18 +15,24 @@ import ( func main() { re := regexp.MustCompile("(?s)(.*?)\n") - err := filepath.Walk("./docs", func(path string, stat fs.FileInfo, err error) error { + root, err := os.OpenRoot("./docs") + if err != nil { + fmt.Println(err) + os.Exit(1) + } + + err = fs.WalkDir(root.FS(), ".", func(path string, d fs.DirEntry, err error) error { if err != nil { return err } - if stat.IsDir() { + if d.IsDir() { return nil } if filepath.Ext(path) != ".md" { return nil } - data, err := os.ReadFile(path) + data, err := root.ReadFile(path) if err != nil { return err } @@ -61,14 +67,21 @@ func main() { } if !bytes.Equal(data, dataNew) { - fmt.Println(path) - if err := os.WriteFile(path, dataNew, stat.Mode()); err != nil { + info, err := d.Info() + if err != nil { + return err + } + fmt.Println(filepath.Join("docs", path)) + if err := root.WriteFile(path, dataNew, info.Mode()); err != nil { return err } } return nil }) + if closeErr := root.Close(); closeErr != nil && err == nil { + err = closeErr + } if err != nil { fmt.Println(err) os.Exit(1) diff --git a/executor/containerdexecutor/executor_unix.go b/executor/containerdexecutor/executor_unix.go index ef0e5e630664..328f0568c5e1 100644 --- a/executor/containerdexecutor/executor_unix.go +++ b/executor/containerdexecutor/executor_unix.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "slices" ctd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/mount" @@ -46,8 +47,8 @@ func getUserSpec(user, rootfsPath string) (specs.User, error) { func (w *containerdExecutor) prepareExecutionEnv(ctx context.Context, rootMount executor.Mount, mounts []executor.Mount, meta executor.Meta, details *containerState, netMode pb.NetMode) (string, string, func(), error) { var releasers []func() releaseAll := func() { - for i := len(releasers) - 1; i >= 0; i-- { - releasers[i]() + for _, release := range slices.Backward(releasers) { + release() } } @@ -133,8 +134,8 @@ func (w *containerdExecutor) ensureCWD(details *containerState, meta executor.Me func (w *containerdExecutor) createOCISpec(ctx context.Context, id, resolvConf, hostsFile string, namespace network.Namespace, mounts []executor.Mount, meta executor.Meta, details *containerState) (*specs.Spec, func(), error) { var releasers []func() releaseAll := func() { - for i := len(releasers) - 1; i >= 0; i-- { - releasers[i]() + for _, release := range slices.Backward(releasers) { + release() } } diff --git a/executor/runcexecutor/executor.go b/executor/runcexecutor/executor.go index c4e5f8fd0674..5bf2c78ec0ff 100644 --- a/executor/runcexecutor/executor.go +++ b/executor/runcexecutor/executor.go @@ -646,7 +646,7 @@ func runcProcessHandle(ctx context.Context, killer procKiller) (*procHandle, con for { select { case <-ctx.Done(): - killCtx, timeout := context.WithCancelCause(context.Background()) //nolint:govet + killCtx, timeout := context.WithCancelCause(context.WithoutCancel(ctx)) //nolint:govet killCtx, _ = context.WithTimeoutCause(killCtx, 7*time.Second, errors.WithStack(context.DeadlineExceeded)) //nolint:govet if err := p.killer.Kill(killCtx); err != nil { select { diff --git a/exporter/tar/export.go b/exporter/tar/export.go index 02000593b89e..c3bc64dca2e0 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -3,6 +3,7 @@ package local import ( "context" "os" + "slices" "strings" "time" @@ -81,8 +82,8 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source var defers []func() error defer func() { - for i := len(defers) - 1; i >= 0; i-- { - defers[i]() + for _, f := range slices.Backward(defers) { + f() } }() diff --git a/frontend/dockerfile/parser/dumper/main.go b/frontend/dockerfile/parser/dumper/main.go index 461066b7319a..10ea8e5522f2 100644 --- a/frontend/dockerfile/parser/dumper/main.go +++ b/frontend/dockerfile/parser/dumper/main.go @@ -8,16 +8,13 @@ import ( ) func main() { - var f *os.File - var err error - if len(os.Args) < 2 { fmt.Println("please supply filename(s)") os.Exit(1) } for _, fn := range os.Args[1:] { - f, err = os.Open(fn) + f, err := os.Open(fn) //nolint:gosec // not using os.Root to support symlinks if err != nil { panic(err) } diff --git a/frontend/gateway/container/container.go b/frontend/gateway/container/container.go index 042b7e793a90..458e3d445df5 100644 --- a/frontend/gateway/container/container.go +++ b/frontend/gateway/container/container.go @@ -97,8 +97,8 @@ func NewContainer(ctx context.Context, cm cache.Manager, exec executor.Executor, return cm.New(ctx, ref, g) }, platform.OS) if err != nil { - for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order - p.Actives[i].Ref.Release(context.TODO()) + for _, active := range slices.Backward(p.Actives) { // call in LIFO order + active.Ref.Release(context.TODO()) } for _, o := range p.OutputRefs { o.Ref.Release(context.TODO()) @@ -435,8 +435,8 @@ func (gwCtr *gatewayContainer) Release(ctx context.Context) error { err1 := gwCtr.errGroup.Wait() var err2 error - for i := len(gwCtr.cleanup) - 1; i >= 0; i-- { // call in LIFO order - err := gwCtr.cleanup[i]() + for _, cleanup := range slices.Backward(gwCtr.cleanup) { // call in LIFO order + err := cleanup() if err2 == nil { err2 = err } diff --git a/hack/dockerfiles/doctoc.Dockerfile b/hack/dockerfiles/doctoc.Dockerfile index cff6e90dee5a..a84b1177352f 100644 --- a/hack/dockerfiles/doctoc.Dockerfile +++ b/hack/dockerfiles/doctoc.Dockerfile @@ -10,7 +10,7 @@ FROM base AS doctoc # DOCTOC_VERSION is the version of doctoc to install # see https://github.com/thlorenz/doctoc/tags for available releases. -ARG DOCTOC_VERSION=v2.3.0 +ARG DOCTOC_VERSION=v2.4.1 RUN npm install -g doctoc@${DOCTOC_VERSION#v} RUN --mount=type=bind,source=README.md,target=README.md,rw < 1 { + if c := dialCount.Add(1); c > 1 { return nil, errors.Errorf("only one connection allowed") } return conn, nil diff --git a/session/sshforward/sshprovider/agentprovider.go b/session/sshforward/sshprovider/agentprovider.go index f29531a56d9d..78930203d863 100644 --- a/session/sshforward/sshprovider/agentprovider.go +++ b/session/sshforward/sshprovider/agentprovider.go @@ -130,7 +130,7 @@ func toDialer(paths []string, raw bool) (func(context.Context) (net.Conn, error) continue } - fi, err := os.Stat(p) + fi, err := os.Stat(p) //nolint:gosec // SSH agent paths are explicit user inputs and may intentionally be symlinks. if err != nil { return nil, errors.WithStack(err) } @@ -142,7 +142,7 @@ func toDialer(paths []string, raw bool) (func(context.Context) (net.Conn, error) return nil, errors.Errorf("raw mode only supported with socket paths") } - f, err := os.Open(p) + f, err := os.Open(p) //nolint:gosec // SSH key paths are explicit user inputs and may intentionally be symlinks. if err != nil { return nil, errors.Wrapf(err, "failed to open %s", p) } diff --git a/session/testutil/testutil.go b/session/testutil/testutil.go index 34d969eff946..755b11b2689c 100644 --- a/session/testutil/testutil.go +++ b/session/testutil/testutil.go @@ -20,7 +20,7 @@ func TestStream(handler Handler) Dialer { s1, s2 := sockPair() return func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) { go func() { - err := handler(context.TODO(), s1, meta) + err := handler(context.WithoutCancel(ctx), s1, meta) if err != nil { bklog.G(ctx).Error(err) } diff --git a/solver/edge.go b/solver/edge.go index 94ff753f3a56..8059bdf45d8a 100644 --- a/solver/edge.go +++ b/solver/edge.go @@ -972,7 +972,7 @@ func (e *edge) execOp(ctx context.Context) (any, error) { for i := range results { if i != int(index) { - go results[i].Release(context.TODO()) + go results[i].Release(context.WithoutCancel(ctx)) } } diff --git a/solver/llbsolver/history/buildhistory.go b/solver/llbsolver/history/buildhistory.go index 2e43bae660d2..9349f2b2a6c4 100644 --- a/solver/llbsolver/history/buildhistory.go +++ b/solver/llbsolver/history/buildhistory.go @@ -290,7 +290,7 @@ func (h *Queue) addResource(ctx context.Context, l leases.Lease, desc *controlap if err != nil { return err } - defer lr.Discard() + defer lr.Discard(ctx) ok, err := h.migrateBlobV2(ctx, desc.Digest, detectSkipLayers) if err != nil { return err diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 09b37702cf8f..7a79ae076a02 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -412,8 +412,8 @@ func (e *ExecOp) Exec(ctx context.Context, jobCtx solver.JobContext, inputs []so err = errdefs.WithExecError(err, execInputs, execMounts) } else { // Only release actives if err is nil. - for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order - p.Actives[i].Ref.Release(context.TODO()) + for _, active := range slices.Backward(p.Actives) { // call in LIFO order + active.Ref.Release(context.TODO()) } } for _, o := range p.OutputRefs { diff --git a/solver/result.go b/solver/result.go index e5f4b57e162a..5216972ea24e 100644 --- a/solver/result.go +++ b/solver/result.go @@ -36,23 +36,23 @@ func (r *SharedResult) Release(ctx context.Context) error { } func dup(res Result) (Result, Result) { - sem := int64(0) + var sem atomic.Int64 return &splitResult{Result: res, sem: &sem}, &splitResult{Result: res, sem: &sem} } type splitResult struct { - released int64 - sem *int64 + released atomic.Int64 + sem *atomic.Int64 Result } func (r *splitResult) Release(ctx context.Context) error { - if atomic.AddInt64(&r.released, 1) > 1 { + if r.released.Add(1) > 1 { err := errors.Errorf("releasing already released reference %+v", r.ID()) bklog.G(ctx).Error(err) return err } - if atomic.AddInt64(r.sem, 1) == 2 { + if r.sem.Add(1) == 2 { return r.Result.Release(ctx) } return nil @@ -110,24 +110,24 @@ type SharedCachedResult struct { } type splitResultProxy struct { - released int64 - sem *int64 + released atomic.Int64 + sem *atomic.Int64 ResultProxy } func (r *splitResultProxy) Release(ctx context.Context) error { - if atomic.AddInt64(&r.released, 1) > 1 { + if r.released.Add(1) > 1 { err := errors.New("releasing already released reference") bklog.G(ctx).Error(err) return err } - if atomic.AddInt64(r.sem, 1) == 2 { + if r.sem.Add(1) == 2 { return r.ResultProxy.Release(ctx) } return nil } func SplitResultProxy(res ResultProxy) (ResultProxy, ResultProxy) { - sem := int64(0) + var sem atomic.Int64 return &splitResultProxy{ResultProxy: res, sem: &sem}, &splitResultProxy{ResultProxy: res, sem: &sem} } diff --git a/source/git/identifier.go b/source/git/identifier.go index 2fc875b97c38..3563da351153 100644 --- a/source/git/identifier.go +++ b/source/git/identifier.go @@ -146,11 +146,11 @@ func validateBundleAttrs(id *GitIdentifier) error { // a colon after '@' (the digest) and confuses url.Parse. func splitBundleLocator(raw string) (string, string) { const sep = "://" - i := strings.Index(raw, sep) - if i < 0 { + scheme, body, ok := strings.Cut(raw, sep) + if !ok { return "", "" } - return raw[:i], raw[i+len(sep):] + return scheme, body } // parseBundleLocator extracts the scheme, normalized reference string, the diff --git a/source/git/source.go b/source/git/source.go index 2571d09c29ce..81d15fc57bf2 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "io" + "io/fs" "net/url" "os" "os/exec" @@ -12,6 +13,7 @@ import ( "path" "path/filepath" "regexp" + "slices" "strconv" "strings" "time" @@ -1326,8 +1328,14 @@ func (gs *gitSourceHandler) checkout(ctx context.Context, repo *gitRepo, g sessi } } + checkoutRoot, err := os.OpenRoot(checkoutDir) + if err != nil { + return nil, errors.Wrap(err, "failed to open git checkout root") + } + defer checkoutRoot.Close() + if compatibilityVersion == compat.CompatibilityVersion013 { - if err := resetCompatibility014FileModes(checkoutDir); err != nil { + if err := resetCompatibility014FileModes(checkoutRoot); err != nil { return nil, errors.Wrapf(err, "failed to normalize compatibility file modes for %s", urlutil.RedactCredentials(gs.src.Remote)) } } @@ -1337,16 +1345,23 @@ func (gs *gitSourceHandler) checkout(ctx context.Context, repo *gitRepo, g sessi if err != nil { return nil, errors.Wrapf(err, "failed to get commit time for %s", urlutil.RedactCredentials(gs.src.Remote)) } - if err := resetSnapshotMtimes(checkoutDir, commitTime); err != nil { + if err := resetSnapshotMtimes(checkoutRoot, commitTime); err != nil { return nil, errors.Wrapf(err, "failed to normalize mtimes for %s", urlutil.RedactCredentials(gs.src.Remote)) } } if idmap := mount.IdentityMapping(); idmap != nil { uid, gid := idmap.RootPair() - err := filepath.WalkDir(gitDir, func(p string, _ os.DirEntry, _ error) error { - return os.Lchown(p, uid, gid) + root, err := os.OpenRoot(gitDir) + if err != nil { + return nil, errors.Wrap(err, "failed to open git checkout root") + } + err = fs.WalkDir(root.FS(), ".", func(p string, _ os.DirEntry, _ error) error { + return root.Lchown(p, uid, gid) }) + if closeErr := root.Close(); closeErr != nil && err == nil { + err = closeErr + } if err != nil { return nil, errors.Wrap(err, "failed to remap git checkout") } @@ -1388,9 +1403,9 @@ func getCommitTime(ctx context.Context, git *gitutil.GitCLI, ref string) (time.T // resetSnapshotMtimes walks dir and sets the mtime of every file, // symlink, and directory to t. Directories are set bottom-up so that // a parent's mtime is not invalidated by a later child write. -func resetSnapshotMtimes(dir string, t time.Time) error { +func resetSnapshotMtimes(root *os.Root, t time.Time) error { var dirs []string - err := filepath.WalkDir(dir, func(p string, d os.DirEntry, err error) error { + err := fs.WalkDir(root.FS(), ".", func(p string, d os.DirEntry, err error) error { if err != nil { return err } @@ -1399,15 +1414,15 @@ func resetSnapshotMtimes(dir string, t time.Time) error { return nil } if d.Type()&os.ModeSymlink != 0 { - return lchtimes(p, t) + return lchtimes(filepath.Join(root.Name(), p), t) } - return os.Chtimes(p, t, t) + return root.Chtimes(p, t, t) }) if err != nil { return err } - for i := len(dirs) - 1; i >= 0; i-- { - if err := os.Chtimes(dirs[i], t, t); err != nil { + for _, dir := range slices.Backward(dirs) { + if err := root.Chtimes(dir, t, t); err != nil { return err } } @@ -1420,8 +1435,8 @@ func resetSnapshotMtimes(dir string, t time.Time) error { // left untouched: their pre-v0.15 behavior is not covered by the current // compatibility matrix, and blindly adding write bits to 0o755 would be a // guess. -func resetCompatibility014FileModes(dir string) error { - return filepath.WalkDir(dir, func(p string, d os.DirEntry, err error) error { +func resetCompatibility014FileModes(root *os.Root) error { + return fs.WalkDir(root.FS(), ".", func(p string, d os.DirEntry, err error) error { if err != nil { return err } @@ -1437,7 +1452,7 @@ func resetCompatibility014FileModes(dir string) error { if !mode.IsRegular() || mode&0o111 != 0 { return nil } - return os.Chmod(p, mode|0o222) + return root.Chmod(p, mode|0o222) }) } diff --git a/source/git/source_test.go b/source/git/source_test.go index 3adeb9ed73ad..a7b2ed53ada8 100644 --- a/source/git/source_test.go +++ b/source/git/source_test.go @@ -2016,13 +2016,17 @@ func testCheckSignatures(t *testing.T, keepGitDir bool, format string) { fixturesBase := os.Getenv("BUILDKIT_TEST_SIGN_FIXTURES") - user1GPGPub, err := os.ReadFile(fixturesBase + "/user1.gpg.pub") + fixturesRoot, err := os.OpenRoot(fixturesBase) + require.NoError(t, err) + defer fixturesRoot.Close() + + user1GPGPub, err := fixturesRoot.ReadFile("user1.gpg.pub") require.NoError(t, err) err = gitsign.VerifySignature(ob, user1GPGPub, nil) require.NoError(t, err) - user2GPGPub, err := os.ReadFile(fixturesBase + "/user2.gpg.pub") + user2GPGPub, err := fixturesRoot.ReadFile("user2.gpg.pub") require.NoError(t, err) err = gitsign.VerifySignature(ob, user2GPGPub, nil) @@ -2052,10 +2056,10 @@ func testCheckSignatures(t *testing.T, keepGitDir bool, format string) { require.NoError(t, err) require.Greater(t, len(ob.Signature), 50) - sshkey1, err := os.ReadFile(fixturesBase + "/user1.ssh.pub") + sshkey1, err := fixturesRoot.ReadFile("user1.ssh.pub") require.NoError(t, err) - sshkey2, err := os.ReadFile(fixturesBase + "/user2.ssh.pub") + sshkey2, err := fixturesRoot.ReadFile("user2.ssh.pub") require.NoError(t, err) err = gitsign.VerifySignature(ob, sshkey2, nil) @@ -2107,16 +2111,20 @@ func testVerifySignatures(t *testing.T, keepGitDir bool, format string) { fixturesBase := os.Getenv("BUILDKIT_TEST_SIGN_FIXTURES") - user1GPGPub, err := os.ReadFile(fixturesBase + "/user1.gpg.pub") + fixturesRoot, err := os.OpenRoot(fixturesBase) require.NoError(t, err) + defer fixturesRoot.Close() - user2GPGPub, err := os.ReadFile(fixturesBase + "/user2.gpg.pub") + user1GPGPub, err := fixturesRoot.ReadFile("user1.gpg.pub") require.NoError(t, err) - user1SSHPub, err := os.ReadFile(fixturesBase + "/user1.ssh.pub") + user2GPGPub, err := fixturesRoot.ReadFile("user2.gpg.pub") require.NoError(t, err) - user2SSHPub, err := os.ReadFile(fixturesBase + "/user2.ssh.pub") + user1SSHPub, err := fixturesRoot.ReadFile("user1.ssh.pub") + require.NoError(t, err) + + user2SSHPub, err := fixturesRoot.ReadFile("user2.ssh.pub") require.NoError(t, err) // a/v1.2.3 commit is signed by user1 gpg @@ -2672,7 +2680,7 @@ func logProgressStreams(ctx context.Context, t *testing.T) context.Context { go func() { defer close(done) for { - prog, err := pr.Read(context.Background()) + prog, err := pr.Read(ctx) if err != nil { return } @@ -2714,7 +2722,10 @@ func TestResetSnapshotMtimes(t *testing.T) { require.NoError(t, os.Symlink("file.txt", filepath.Join(dir, "link"))) target := time.Date(2023, 6, 15, 12, 0, 0, 0, time.UTC) - require.NoError(t, resetSnapshotMtimes(dir, target)) + root, err := os.OpenRoot(dir) + require.NoError(t, err) + require.NoError(t, resetSnapshotMtimes(root, target)) + require.NoError(t, root.Close()) // Regular files should have mtime set fi, err := os.Lstat(filepath.Join(dir, "file.txt")) diff --git a/util/appdefaults/appdefaults_unix.go b/util/appdefaults/appdefaults_unix.go index a351a7944e85..beb2e39e7932 100644 --- a/util/appdefaults/appdefaults_unix.go +++ b/util/appdefaults/appdefaults_unix.go @@ -37,11 +37,15 @@ func EnsureUserAddressDir() error { xdgRuntimeDir := os.Getenv("XDG_RUNTIME_DIR") if xdgRuntimeDir != "" { dirs := strings.Split(xdgRuntimeDir, ":") - dir := filepath.Join(dirs[0], "buildkit") - if err := os.MkdirAll(dir, 0700); err != nil { + root, err := os.OpenRoot(dirs[0]) + if err != nil { return err } - return os.Chmod(dir, 0700|os.ModeSticky) + defer root.Close() + if err := root.MkdirAll("buildkit", 0700); err != nil { + return err + } + return root.Chmod("buildkit", 0700|os.ModeSticky) } return nil } diff --git a/util/contentutil/copy.go b/util/contentutil/copy.go index 1d575d4b589b..5ec7b21bc54f 100644 --- a/util/contentutil/copy.go +++ b/util/contentutil/copy.go @@ -3,6 +3,7 @@ package contentutil import ( "context" "io" + "slices" "strings" "sync" @@ -117,8 +118,7 @@ func copyChain(ctx context.Context, ingester content.Ingester, provider content. return errors.WithStack(err) } - for i := len(manifestStack) - 1; i >= 0; i-- { - desc := manifestStack[i] + for _, desc := range slices.Backward(manifestStack) { if err := Copy(ctx, ingester, provider, desc, "", nil); err != nil { return errors.WithStack(err) } diff --git a/util/leaseutil/manager.go b/util/leaseutil/manager.go index da90a68276a1..5ccb9eefcc21 100644 --- a/util/leaseutil/manager.go +++ b/util/leaseutil/manager.go @@ -47,8 +47,8 @@ type LeaseRef struct { err error } -func (l *LeaseRef) Discard() error { - return l.lm.Delete(context.Background(), l.l) +func (l *LeaseRef) Discard(ctx context.Context) error { + return l.lm.Delete(context.WithoutCancel(ctx), l.l) } func (l *LeaseRef) Adopt(ctx context.Context) error { @@ -73,10 +73,10 @@ func (l *LeaseRef) Adopt(ctx context.Context) error { } } if len(l.resources) == 0 { - l.Discard() + l.Discard(ctx) return nil } - go l.Discard() + go l.Discard(ctx) return nil } diff --git a/util/network/cniprovider/cni_linux.go b/util/network/cniprovider/cni_linux.go index 526fc71bc05b..24f2f66b4502 100644 --- a/util/network/cniprovider/cni_linux.go +++ b/util/network/cniprovider/cni_linux.go @@ -83,8 +83,13 @@ func readFileAt(dirfd int, filename string, buf []byte) (int64, error) { // https://github.com/containerd/nerdctl/pull/2723 func withDetachedNetNSIfAny(ctx context.Context, fn func(context.Context) error) error { if stateDir := os.Getenv("ROOTLESSKIT_STATE_DIR"); stateDir != "" { - detachedNetNS := filepath.Join(stateDir, "netns") - if _, err := os.Lstat(detachedNetNS); !errors.Is(err, os.ErrNotExist) { + root, err := os.OpenRoot(stateDir) + if err != nil { + return err + } + defer root.Close() + if _, err := root.Lstat("netns"); err == nil { + detachedNetNS := filepath.Join(stateDir, "netns") return ns.WithNetNSPath(detachedNetNS, func(_ ns.NetNS) error { ctx := context.WithValue(ctx, contextKeyDetachedNetNS, detachedNetNS) bklog.G(ctx).Debugf("Entering RootlessKit's detached netns %q", detachedNetNS) @@ -92,6 +97,8 @@ func withDetachedNetNSIfAny(ctx context.Context, fn func(context.Context) error) bklog.G(ctx).WithError(err2).Debugf("Leaving RootlessKit's detached netns %q", detachedNetNS) return err2 }) + } else if !errors.Is(err, os.ErrNotExist) { + return err } } return fn(ctx) diff --git a/util/progress/multireader.go b/util/progress/multireader.go index bcc9eb897abc..3a1b15b67ed8 100644 --- a/util/progress/multireader.go +++ b/util/progress/multireader.go @@ -100,16 +100,16 @@ func (mr *MultiReader) Reader(ctx context.Context) Reader { }() if !mr.initialized { - go mr.handle() + go mr.handle(context.WithoutCancel(ctx)) mr.initialized = true } return pr } -func (mr *MultiReader) handle() error { +func (mr *MultiReader) handle(ctx context.Context) error { for { - p, err := mr.main.Read(context.TODO()) + p, err := mr.main.Read(ctx) if err != nil { if errors.Is(err, io.EOF) { mr.mu.Lock() diff --git a/util/pull/pullprogress/progress.go b/util/pull/pullprogress/progress.go index df56b985acc1..fa71d69fafe9 100644 --- a/util/pull/pullprogress/progress.go +++ b/util/pull/pullprogress/progress.go @@ -31,9 +31,9 @@ func (p *ProviderWithProgress) ReaderAt(ctx context.Context, desc ocispecs.Descr return nil, err } - ctx, cancel := context.WithCancelCause(ctx) + progressCtx, cancel := context.WithCancelCause(context.WithoutCancel(ctx)) doneCh := make(chan struct{}) - go trackProgress(ctx, desc, p.Manager, doneCh) + go trackProgress(progressCtx, desc, p.Manager, doneCh) //nolint:gosec // Progress tracking is tied to the reader lifetime and canceled by Close. return readerAtWithCancel{ReaderAt: ra, cancel: cancel, doneCh: doneCh, logger: bklog.G(ctx)}, nil } @@ -65,9 +65,9 @@ func (f *FetcherWithProgress) Fetch(ctx context.Context, desc ocispecs.Descripto return nil, err } - ctx, cancel := context.WithCancelCause(ctx) + progressCtx, cancel := context.WithCancelCause(context.WithoutCancel(ctx)) doneCh := make(chan struct{}) - go trackProgress(ctx, desc, f.Manager, doneCh) + go trackProgress(progressCtx, desc, f.Manager, doneCh) //nolint:gosec // Progress tracking is tied to the reader lifetime and canceled by Close. return readerWithCancel{ReadCloser: rc, cancel: cancel, doneCh: doneCh, logger: bklog.G(ctx)}, nil } diff --git a/util/push/push.go b/util/push/push.go index 77788da20cb4..0ffe86757e9a 100644 --- a/util/push/push.go +++ b/util/push/push.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "strings" "sync" @@ -138,8 +139,8 @@ func Push(ctx context.Context, sm *session.Manager, sid string, provider content } mfstDone := progress.OneOff(ctx, fmt.Sprintf("pushing manifest for %s", ref)) - for i := len(manifestStack) - 1; i >= 0; i-- { - if _, err := pushHandler(ctx, manifestStack[i]); err != nil { + for _, desc := range slices.Backward(manifestStack) { + if _, err := pushHandler(ctx, desc); err != nil { return mfstDone(err) } } diff --git a/util/resolvconf/resolvconf.go b/util/resolvconf/resolvconf.go index c00a50c7d914..43cd936be8b5 100644 --- a/util/resolvconf/resolvconf.go +++ b/util/resolvconf/resolvconf.go @@ -174,8 +174,8 @@ func (rc *ResolvConf) Options() []string { // Option("ndots") -> ("1", true) // Option("edns0") -> ("", true) func (rc *ResolvConf) Option(search string) (string, bool) { - for i := len(rc.options) - 1; i >= 0; i-- { - k, v, _ := strings.Cut(rc.options[i], ":") + for _, option := range slices.Backward(rc.options) { + k, v, _ := strings.Cut(option, ":") if k == search { return v, true } diff --git a/util/resolver/authorizer.go b/util/resolver/authorizer.go index 4a62e5b01631..da772460b776 100644 --- a/util/resolver/authorizer.go +++ b/util/resolver/authorizer.go @@ -9,6 +9,7 @@ import ( "slices" "strings" "sync" + "sync/atomic" "time" "github.com/containerd/containerd/v2/core/remotes/docker" @@ -28,7 +29,7 @@ import ( const defaultExpiration = 60 type authHandlerNS struct { - counter int64 // needs to be 64bit aligned for 32bit systems + counter atomic.Int64 fetchers map[string]*authFetcher muHandlers sync.Mutex diff --git a/util/resolver/pool.go b/util/resolver/pool.go index fdc0fb3a0a27..06d8be655521 100644 --- a/util/resolver/pool.go +++ b/util/resolver/pool.go @@ -6,7 +6,6 @@ import ( "net/http" "strings" "sync" - "sync/atomic" "time" "github.com/containerd/containerd/v2/core/images" @@ -233,7 +232,7 @@ func (r *Resolver) WithImageStore(is images.Store, mode ResolveMode) *Resolver { // Fetcher returns a new fetcher for the provided reference. func (r *Resolver) Fetcher(ctx context.Context, ref string) (remotes.Fetcher, error) { - if atomic.LoadInt64(&r.handler.counter) == 0 { + if r.handler.counter.Load() == 0 { r.Resolve(ctx, ref) } return r.Resolver.Fetcher(ctx, ref) @@ -249,7 +248,7 @@ func (r *Resolver) Resolve(ctx context.Context, ref string) (string, ocispecs.De n, desc, err := r.Resolver.Resolve(ctx, ref) if err == nil { - atomic.AddInt64(&r.handler.counter, 1) + r.handler.counter.Add(1) return n, desc, nil } diff --git a/util/testutil/integration/registry.go b/util/testutil/integration/registry.go index 0cbc7434266a..2c595a0146f5 100644 --- a/util/testutil/integration/registry.go +++ b/util/testutil/integration/registry.go @@ -38,7 +38,13 @@ func NewRegistry(dir string) (url string, cl func() error, err error) { dir = tmpdir } - if _, err := os.Stat(filepath.Join(dir, "config.yaml")); err != nil { + root, err := os.OpenRoot(dir) + if err != nil { + return "", nil, errors.WithStack(err) + } + defer root.Close() + + if _, err := root.Stat("config.yaml"); err != nil { if !errors.Is(err, os.ErrNotExist) { return "", nil, err } @@ -51,7 +57,7 @@ http: addr: 127.0.0.1:0 `, filepath.Join(dir, "data")) - if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(template), 0600); err != nil { + if err := root.WriteFile("config.yaml", []byte(template), 0600); err != nil { return "", nil, err } } diff --git a/util/throttle/throttle_test.go b/util/throttle/throttle_test.go index fd9d013ae639..9d06729e07d4 100644 --- a/util/throttle/throttle_test.go +++ b/util/throttle/throttle_test.go @@ -11,9 +11,9 @@ import ( func TestThrottle(t *testing.T) { t.Parallel() - var i int64 + var i atomic.Int64 f := func() { - atomic.AddInt64(&i, 1) + i.Add(1) } f = Throttle(50*time.Millisecond, f) @@ -21,14 +21,14 @@ func TestThrottle(t *testing.T) { f() f() - require.Equal(t, int64(0), atomic.LoadInt64(&i)) + require.Equal(t, int64(0), i.Load()) // test that i is never incremented twice and at least once in next 600ms retries := 0 for { require.Less(t, retries, 10) time.Sleep(60 * time.Millisecond) - v := atomic.LoadInt64(&i) + v := i.Load() require.LessOrEqual(t, v, int64(1)) if v == 1 { break @@ -36,7 +36,7 @@ func TestThrottle(t *testing.T) { retries++ } - require.Equal(t, int64(1), atomic.LoadInt64(&i)) + require.Equal(t, int64(1), i.Load()) f() @@ -44,7 +44,7 @@ func TestThrottle(t *testing.T) { for { require.Less(t, retries, 10) time.Sleep(60 * time.Millisecond) - v := atomic.LoadInt64(&i) + v := i.Load() if v == 2 { break } @@ -55,9 +55,9 @@ func TestThrottle(t *testing.T) { func TestAfter(t *testing.T) { t.Parallel() - var i int64 + var i atomic.Int64 f := func() { - atomic.AddInt64(&i, 1) + i.Add(1) } f = After(100*time.Millisecond, f) @@ -65,11 +65,11 @@ func TestAfter(t *testing.T) { f() time.Sleep(10 * time.Millisecond) - require.Equal(t, int64(1), atomic.LoadInt64(&i)) + require.Equal(t, int64(1), i.Load()) f() time.Sleep(10 * time.Millisecond) - require.Equal(t, int64(1), atomic.LoadInt64(&i)) + require.Equal(t, int64(1), i.Load()) time.Sleep(200 * time.Millisecond) - require.Equal(t, int64(2), atomic.LoadInt64(&i)) + require.Equal(t, int64(2), i.Load()) } diff --git a/worker/base/worker.go b/worker/base/worker.go index 8fcb472317b8..b81967b3311e 100644 --- a/worker/base/worker.go +++ b/worker/base/worker.go @@ -395,7 +395,7 @@ func (w *Worker) PruneCacheMounts(ctx context.Context, ids map[string]bool) erro } // if ref is unused try to clean it up right away by releasing it if mref, err := w.CacheMgr.GetMutable(ctx, md.ID()); err == nil { - go mref.Release(context.TODO()) + go mref.Release(context.WithoutCancel(ctx)) } } }