From 7a3af2ae4fa7f26526a97dc6c12e900d870ff9aa Mon Sep 17 00:00:00 2001 From: Andrey Epifanov Date: Fri, 16 Feb 2024 01:37:21 -0800 Subject: [PATCH 01/10] Move container.go to a separate module Signed-off-by: Andrey Epifanov --- frontend/gateway/{ => container}/container.go | 8 ++++---- frontend/gateway/{ => container}/util.go | 2 +- frontend/gateway/forwarder/forward.go | 12 ++++++------ frontend/gateway/gateway.go | 11 ++++++----- solver/llbsolver/ops/exec.go | 6 +++--- 5 files changed, 20 insertions(+), 19 deletions(-) rename frontend/gateway/{ => container}/container.go (98%) rename frontend/gateway/{ => container}/util.go (96%) diff --git a/frontend/gateway/container.go b/frontend/gateway/container/container.go similarity index 98% rename from frontend/gateway/container.go rename to frontend/gateway/container/container.go index 45cf2d90eb50..9ebc896de93a 100644 --- a/frontend/gateway/container.go +++ b/frontend/gateway/container/container.go @@ -1,4 +1,4 @@ -package gateway +package container import ( "context" @@ -255,9 +255,9 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage }) root = active } - p.Root = mountWithSession(root, g) + p.Root = MountWithSession(root, g) } else { - mws := mountWithSession(mountable, g) + mws := MountWithSession(mountable, g) dest := m.Dest if !filepath.IsAbs(filepath.Clean(dest)) { dest = filepath.Join("/", cwd, dest) @@ -457,7 +457,7 @@ func addDefaultEnvvar(env []string, k, v string) []string { return append(env, k+"="+v) } -func mountWithSession(m cache.Mountable, g session.Group) executor.Mount { +func MountWithSession(m cache.Mountable, g session.Group) executor.Mount { _, readonly := m.(cache.ImmutableRef) return executor.Mount{ Src: &mountable{m: m, g: g}, diff --git a/frontend/gateway/util.go b/frontend/gateway/container/util.go similarity index 96% rename from frontend/gateway/util.go rename to frontend/gateway/container/util.go index 0de8353402fa..1a1fb25138e3 100644 --- a/frontend/gateway/util.go +++ b/frontend/gateway/container/util.go @@ -1,4 +1,4 @@ -package gateway +package container import ( "net" diff --git a/frontend/gateway/forwarder/forward.go b/frontend/gateway/forwarder/forward.go index 0a95de377dc2..2ff277d4aa27 100644 --- a/frontend/gateway/forwarder/forward.go +++ b/frontend/gateway/forwarder/forward.go @@ -7,8 +7,8 @@ import ( cacheutil "github.com/moby/buildkit/cache/util" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/frontend" - "github.com/moby/buildkit/frontend/gateway" "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/frontend/gateway/container" gwpb "github.com/moby/buildkit/frontend/gateway/pb" "github.com/moby/buildkit/identity" "github.com/moby/buildkit/session" @@ -235,10 +235,10 @@ func (c *bridgeClient) Warn(ctx context.Context, dgst digest.Digest, msg string, } func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainerRequest) (client.Container, error) { - ctrReq := gateway.NewContainerRequest{ + ctrReq := container.NewContainerRequest{ ContainerID: identity.NewID(), NetMode: req.NetMode, - Mounts: make([]gateway.Mount, len(req.Mounts)), + Mounts: make([]container.Mount, len(req.Mounts)), } eg, ctx := errgroup.WithContext(ctx) @@ -269,7 +269,7 @@ func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainer return errors.Errorf("failed to find ref %s for %q mount", m.ResultID, m.Dest) } } - ctrReq.Mounts[i] = gateway.Mount{ + ctrReq.Mounts[i] = container.Mount{ WorkerRef: workerRef, Mount: &opspb.Mount{ Dest: m.Dest, @@ -290,7 +290,7 @@ func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainer return nil, err } - ctrReq.ExtraHosts, err = gateway.ParseExtraHosts(req.ExtraHosts) + ctrReq.ExtraHosts, err = container.ParseExtraHosts(req.ExtraHosts) if err != nil { return nil, err } @@ -301,7 +301,7 @@ func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainer } group := session.NewGroup(c.sid) - ctr, err := gateway.NewContainer(ctx, w, c.sm, group, ctrReq) + ctr, err := container.NewContainer(ctx, w, c.sm, group, ctrReq) if err != nil { return nil, err } diff --git a/frontend/gateway/gateway.go b/frontend/gateway/gateway.go index 85a42e299def..798a89790b98 100644 --- a/frontend/gateway/gateway.go +++ b/frontend/gateway/gateway.go @@ -29,6 +29,7 @@ import ( "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/frontend" gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/frontend/gateway/container" pb "github.com/moby/buildkit/frontend/gateway/pb" "github.com/moby/buildkit/identity" "github.com/moby/buildkit/session" @@ -275,7 +276,7 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten mnts = append(mnts, *mdmnt) } - err = w.Executor().Run(ctx, "", mountWithSession(rootFS, session.NewGroup(sid)), mnts, executor.ProcessInfo{Meta: meta, Stdin: lbf.Stdin, Stdout: lbf.Stdout, Stderr: os.Stderr}, nil) + err = w.Executor().Run(ctx, "", container.MountWithSession(rootFS, session.NewGroup(sid)), mnts, executor.ProcessInfo{Meta: meta, Stdin: lbf.Stdin, Stdout: lbf.Stdout, Stderr: os.Stderr}, nil) if err != nil { if errdefs.IsCanceled(ctx, err) && lbf.isErrServerClosed { @@ -930,7 +931,7 @@ func (lbf *llbBridgeForwarder) Inputs(ctx context.Context, in *pb.InputsRequest) func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewContainerRequest) (_ *pb.NewContainerResponse, err error) { bklog.G(ctx).Debugf("|<--- NewContainer %s", in.ContainerID) - ctrReq := NewContainerRequest{ + ctrReq := container.NewContainerRequest{ ContainerID: in.ContainerID, NetMode: in.Network, Platform: in.Platform, @@ -959,7 +960,7 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta } } } - ctrReq.Mounts = append(ctrReq.Mounts, Mount{ + ctrReq.Mounts = append(ctrReq.Mounts, container.Mount{ WorkerRef: workerRef, Mount: &opspb.Mount{ Dest: m.Dest, @@ -982,12 +983,12 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta return nil, stack.Enable(err) } - ctrReq.ExtraHosts, err = ParseExtraHosts(in.ExtraHosts) + ctrReq.ExtraHosts, err = container.ParseExtraHosts(in.ExtraHosts) if err != nil { return nil, stack.Enable(err) } - ctr, err := NewContainer(context.Background(), w, lbf.sm, group, ctrReq) + ctr, err := container.NewContainer(context.Background(), w, lbf.sm, group, ctrReq) if err != nil { return nil, stack.Enable(err) } diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 6cca733c0bf2..c2b40d6a3aa5 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -13,7 +13,7 @@ import ( "github.com/containerd/containerd/platforms" "github.com/moby/buildkit/cache" "github.com/moby/buildkit/executor" - "github.com/moby/buildkit/frontend/gateway" + "github.com/moby/buildkit/frontend/gateway/container" "github.com/moby/buildkit/session" "github.com/moby/buildkit/session/secrets" "github.com/moby/buildkit/solver" @@ -258,7 +258,7 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu } } - p, err := gateway.PrepareMounts(ctx, e.mm, e.cm, g, e.op.Meta.Cwd, e.op.Mounts, refs, func(m *pb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) { + p, err := container.PrepareMounts(ctx, e.mm, e.cm, g, e.op.Meta.Cwd, e.op.Mounts, refs, func(m *pb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) { desc := fmt.Sprintf("mount %s from exec %s", m.Dest, strings.Join(e.op.Meta.Args, " ")) return e.cm.New(ctx, ref, g, cache.WithDescription(desc)) }) @@ -305,7 +305,7 @@ func (e *execOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu return nil, err } - extraHosts, err := gateway.ParseExtraHosts(e.op.Meta.ExtraHosts) + extraHosts, err := container.ParseExtraHosts(e.op.Meta.ExtraHosts) if err != nil { return nil, err } From a9b0450f67915cfce2e07b0c056f82edd0261738 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 12 Dec 2023 13:42:46 -0800 Subject: [PATCH 02/10] gateway: pass executor with build and not access worker directly Running interactive container APIs was done by giving the gateway implementation access to worker controller directly, but it should be passed with a build job instead. Signed-off-by: Tonis Tiigi (cherry picked from commit 0971dffaab93d91e51af984b44c745b35b3c5b4d) (cherry picked from commit 564f884e7bb6db9c63e03c3b081ea71e15aa7980) (cherry picked from commit 5026d95aa3336e97cfe46e3764f52d08bac7a10e) Signed-off-by: Andrey Epifanov `bridgeClient` is made exported since exported func LLBBridgeToGatewayClient should have exported-return Signed-off-by: Andrey Epifanov # Conflicts: # executor/executor.go # frontend/gateway/container/container.go # frontend/gateway/forwarder/forward.go # frontend/gateway/forwarder/frontend.go # frontend/gateway/gateway.go # solver/llbsolver/bridge.go # solver/llbsolver/provenance.go # solver/llbsolver/solver.go # worker/workercontroller.go --- cmd/buildkitd/main.go | 4 +-- executor/executor.go | 10 +++++-- frontend/frontend.go | 3 +- frontend/gateway/container/container.go | 9 +++--- frontend/gateway/forwarder/forward.go | 37 +++++++++++++------------ frontend/gateway/forwarder/frontend.go | 5 ++-- frontend/gateway/gateway.go | 28 ++++++++----------- snapshot/snapshotter.go | 7 ++--- solver/llbsolver/bridge.go | 33 +++++++++++++++++++++- solver/llbsolver/solver.go | 9 ++++-- worker/worker.go | 2 +- worker/workercontroller.go | 23 +++++++++++++++ 12 files changed, 116 insertions(+), 54 deletions(-) diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index 126ba0dbe2c8..c7c444df4e9a 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -636,8 +636,8 @@ func newController(c *cli.Context, cfg *config.Config) (*control.Controller, err return nil, err } frontends := map[string]frontend.Frontend{} - frontends["dockerfile.v0"] = forwarder.NewGatewayForwarder(wc, dockerfile.Build) - frontends["gateway.v0"] = gateway.NewGatewayFrontend(wc) + frontends["dockerfile.v0"] = forwarder.NewGatewayForwarder(wc.Infos(), dockerfile.Build) + frontends["gateway.v0"] = gateway.NewGatewayFrontend(wc.Infos()) cacheStorage, err := bboltcachestorage.NewStore(filepath.Join(cfg.Root, "cache.db")) if err != nil { diff --git a/executor/executor.go b/executor/executor.go index 4727af4b03ef..937acc227371 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -6,7 +6,8 @@ import ( "net" "syscall" - "github.com/moby/buildkit/snapshot" + "github.com/containerd/containerd/mount" + "github.com/docker/docker/pkg/idtools" "github.com/moby/buildkit/solver/pb" ) @@ -25,8 +26,13 @@ type Meta struct { SecurityMode pb.SecurityMode } +type MountableRef interface { + Mount() ([]mount.Mount, func() error, error) + IdentityMapping() *idtools.IdentityMapping +} + type Mountable interface { - Mount(ctx context.Context, readonly bool) (snapshot.Mountable, error) + Mount(ctx context.Context, readonly bool) (MountableRef, error) } type Mount struct { diff --git a/frontend/frontend.go b/frontend/frontend.go index dedda54c6104..c6dc80755be9 100644 --- a/frontend/frontend.go +++ b/frontend/frontend.go @@ -4,6 +4,7 @@ import ( "context" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/executor" gw "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/session" "github.com/moby/buildkit/solver/pb" @@ -11,7 +12,7 @@ import ( ) type Frontend interface { - Solve(ctx context.Context, llb FrontendLLBBridge, opt map[string]string, inputs map[string]*pb.Definition, sid string, sm *session.Manager) (*Result, error) + Solve(ctx context.Context, llb FrontendLLBBridge, exec executor.Executor, opt map[string]string, inputs map[string]*pb.Definition, sid string, sm *session.Manager) (*Result, error) } type FrontendLLBBridge interface { diff --git a/frontend/gateway/container/container.go b/frontend/gateway/container/container.go index 9ebc896de93a..27874819199e 100644 --- a/frontend/gateway/container/container.go +++ b/frontend/gateway/container/container.go @@ -43,7 +43,7 @@ type Mount struct { WorkerRef *worker.WorkerRef } -func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g session.Group, req NewContainerRequest) (client.Container, error) { +func NewContainer(ctx context.Context, cm cache.Manager, exec executor.Executor, sm *session.Manager, g session.Group, req NewContainerRequest) (client.Container, error) { ctx, cancel := context.WithCancel(ctx) eg, ctx := errgroup.WithContext(ctx) platform := opspb.Platform{ @@ -58,7 +58,7 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s netMode: req.NetMode, extraHosts: req.ExtraHosts, platform: platform, - executor: w.Executor(), + executor: exec, errGroup: eg, ctx: ctx, cancel: cancel, @@ -79,9 +79,8 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s } name := fmt.Sprintf("container %s", req.ContainerID) - mm := mounts.NewMountManager(name, w.CacheManager(), sm) - p, err := PrepareMounts(ctx, mm, w.CacheManager(), g, "", mnts, refs, func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) { - cm := w.CacheManager() + mm := mounts.NewMountManager(name, cm, sm) + p, err := PrepareMounts(ctx, mm, cm, g, "", mnts, refs, func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) { if m.Input != opspb.Empty { cm = refs[m.Input].Worker.CacheManager() } diff --git a/frontend/gateway/forwarder/forward.go b/frontend/gateway/forwarder/forward.go index 2ff277d4aa27..69d94c337fe2 100644 --- a/frontend/gateway/forwarder/forward.go +++ b/frontend/gateway/forwarder/forward.go @@ -6,6 +6,7 @@ import ( cacheutil "github.com/moby/buildkit/cache/util" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/executor" "github.com/moby/buildkit/frontend" "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/frontend/gateway/container" @@ -25,8 +26,8 @@ import ( "golang.org/x/sync/errgroup" ) -func llbBridgeToGatewayClient(ctx context.Context, llbBridge frontend.FrontendLLBBridge, opts map[string]string, inputs map[string]*opspb.Definition, w worker.Infos, sid string, sm *session.Manager) (*bridgeClient, error) { - bc := &bridgeClient{ +func LLBBridgeToGatewayClient(ctx context.Context, llbBridge frontend.FrontendLLBBridge, exec executor.Executor, opts map[string]string, inputs map[string]*opspb.Definition, w worker.Infos, sid string, sm *session.Manager) (*BridgeClient, error) { + bc := &BridgeClient{ opts: opts, inputs: inputs, FrontendLLBBridge: llbBridge, @@ -35,12 +36,13 @@ func llbBridgeToGatewayClient(ctx context.Context, llbBridge frontend.FrontendLL workers: w, final: map[*ref]struct{}{}, workerRefByID: make(map[string]*worker.WorkerRef), + executor: exec, } bc.buildOpts = bc.loadBuildOpts() return bc, nil } -type bridgeClient struct { +type BridgeClient struct { frontend.FrontendLLBBridge mu sync.Mutex opts map[string]string @@ -53,9 +55,10 @@ type bridgeClient struct { workerRefByID map[string]*worker.WorkerRef buildOpts client.BuildOpts ctrs []client.Container + executor executor.Executor } -func (c *bridgeClient) Solve(ctx context.Context, req client.SolveRequest) (*client.Result, error) { +func (c *BridgeClient) Solve(ctx context.Context, req client.SolveRequest) (*client.Result, error) { res, err := c.FrontendLLBBridge.Solve(ctx, frontend.SolveRequest{ Evaluate: req.Evaluate, Definition: req.Definition, @@ -91,7 +94,7 @@ func (c *bridgeClient) Solve(ctx context.Context, req client.SolveRequest) (*cli return cRes, nil } -func (c *bridgeClient) loadBuildOpts() client.BuildOpts { +func (c *BridgeClient) loadBuildOpts() client.BuildOpts { wis := c.workers.WorkerInfos() workers := make([]client.WorkerInfo, len(wis)) for i, w := range wis { @@ -112,11 +115,11 @@ func (c *bridgeClient) loadBuildOpts() client.BuildOpts { } } -func (c *bridgeClient) BuildOpts() client.BuildOpts { +func (c *BridgeClient) BuildOpts() client.BuildOpts { return c.buildOpts } -func (c *bridgeClient) Inputs(ctx context.Context) (map[string]llb.State, error) { +func (c *BridgeClient) Inputs(ctx context.Context) (map[string]llb.State, error) { inputs := make(map[string]llb.State) for key, def := range c.inputs { defop, err := llb.NewDefinitionOp(def) @@ -128,7 +131,7 @@ func (c *bridgeClient) Inputs(ctx context.Context) (map[string]llb.State, error) return inputs, nil } -func (c *bridgeClient) wrapSolveError(solveErr error) error { +func (c *BridgeClient) wrapSolveError(solveErr error) error { var ( ee *llberrdefs.ExecError fae *llberrdefs.FileActionError @@ -162,7 +165,7 @@ func (c *bridgeClient) wrapSolveError(solveErr error) error { return errdefs.WithSolveError(solveErr, subject, inputIDs, mountIDs) } -func (c *bridgeClient) registerResultIDs(results ...solver.Result) (ids []string, err error) { +func (c *BridgeClient) registerResultIDs(results ...solver.Result) (ids []string, err error) { c.mu.Lock() defer c.mu.Unlock() @@ -181,7 +184,7 @@ func (c *bridgeClient) registerResultIDs(results ...solver.Result) (ids []string return ids, nil } -func (c *bridgeClient) toFrontendResult(r *client.Result) (*frontend.Result, error) { +func (c *BridgeClient) toFrontendResult(r *client.Result) (*frontend.Result, error) { if r == nil { return nil, nil } @@ -212,7 +215,7 @@ func (c *bridgeClient) toFrontendResult(r *client.Result) (*frontend.Result, err return res, nil } -func (c *bridgeClient) discard(err error) { +func (c *BridgeClient) discard(err error) { for _, ctr := range c.ctrs { ctr.Release(context.TODO()) } @@ -230,11 +233,11 @@ func (c *bridgeClient) discard(err error) { } } -func (c *bridgeClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts client.WarnOpts) error { +func (c *BridgeClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts client.WarnOpts) error { return c.FrontendLLBBridge.Warn(ctx, dgst, msg, opts) } -func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainerRequest) (client.Container, error) { +func (c *BridgeClient) NewContainer(ctx context.Context, req client.NewContainerRequest) (client.Container, error) { ctrReq := container.NewContainerRequest{ ContainerID: identity.NewID(), NetMode: req.NetMode, @@ -295,13 +298,13 @@ func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainer return nil, err } - w, err := c.workers.GetDefault() + cm, err := c.workers.DefaultCacheManager() if err != nil { return nil, err } group := session.NewGroup(c.sid) - ctr, err := container.NewContainer(ctx, w, c.sm, group, ctrReq) + ctr, err := container.NewContainer(ctx, cm, c.executor, c.sm, group, ctrReq) if err != nil { return nil, err } @@ -312,10 +315,10 @@ func (c *bridgeClient) NewContainer(ctx context.Context, req client.NewContainer type ref struct { solver.ResultProxy session session.Group - c *bridgeClient + c *BridgeClient } -func (c *bridgeClient) newRef(r solver.ResultProxy, s session.Group) (*ref, error) { +func (c *BridgeClient) newRef(r solver.ResultProxy, s session.Group) (*ref, error) { return &ref{ResultProxy: r, session: s, c: c}, nil } diff --git a/frontend/gateway/forwarder/frontend.go b/frontend/gateway/forwarder/frontend.go index 7cd25a0e8ea0..9b6381df517c 100644 --- a/frontend/gateway/forwarder/frontend.go +++ b/frontend/gateway/forwarder/frontend.go @@ -3,6 +3,7 @@ package forwarder import ( "context" + "github.com/moby/buildkit/executor" "github.com/moby/buildkit/frontend" "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/session" @@ -22,8 +23,8 @@ type GatewayForwarder struct { f client.BuildFunc } -func (gf *GatewayForwarder) Solve(ctx context.Context, llbBridge frontend.FrontendLLBBridge, opts map[string]string, inputs map[string]*pb.Definition, sid string, sm *session.Manager) (retRes *frontend.Result, retErr error) { - c, err := llbBridgeToGatewayClient(ctx, llbBridge, opts, inputs, gf.workers, sid, sm) +func (gf *GatewayForwarder) Solve(ctx context.Context, llbBridge frontend.FrontendLLBBridge, exec executor.Executor, opts map[string]string, inputs map[string]*pb.Definition, sid string, sm *session.Manager) (retRes *frontend.Result, retErr error) { + c, err := LLBBridgeToGatewayClient(ctx, llbBridge, exec, opts, inputs, gf.workers, sid, sm) if err != nil { return nil, err } diff --git a/frontend/gateway/gateway.go b/frontend/gateway/gateway.go index 798a89790b98..540a345c3362 100644 --- a/frontend/gateway/gateway.go +++ b/frontend/gateway/gateway.go @@ -84,7 +84,7 @@ func filterPrefix(opts map[string]string, pfx string) map[string]string { return m } -func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.FrontendLLBBridge, opts map[string]string, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) (*frontend.Result, error) { +func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.FrontendLLBBridge, exec executor.Executor, opts map[string]string, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) (*frontend.Result, error) { source, ok := opts[keySource] if !ok { return nil, errors.Errorf("no source specified for gateway") @@ -252,18 +252,13 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten } } - lbf, ctx, err := serveLLBBridgeForwarder(ctx, llbBridge, gf.workers, inputs, sid, sm) + lbf, ctx, err := serveLLBBridgeForwarder(ctx, llbBridge, exec, gf.workers, inputs, sid, sm) defer lbf.conn.Close() //nolint if err != nil { return nil, err } defer lbf.Discard() - w, err := gf.workers.GetDefault() - if err != nil { - return nil, err - } - mdmnt, release, err := metadataMount(frontendDef) if err != nil { return nil, err @@ -276,8 +271,7 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten mnts = append(mnts, *mdmnt) } - err = w.Executor().Run(ctx, "", container.MountWithSession(rootFS, session.NewGroup(sid)), mnts, executor.ProcessInfo{Meta: meta, Stdin: lbf.Stdin, Stdout: lbf.Stdout, Stderr: os.Stderr}, nil) - + err = exec.Run(ctx, "", container.MountWithSession(rootFS, session.NewGroup(sid)), mnts, executor.ProcessInfo{Meta: meta, Stdin: lbf.Stdin, Stdout: lbf.Stdout, Stderr: os.Stderr}, nil) if err != nil { if errdefs.IsCanceled(ctx, err) && lbf.isErrServerClosed { err = errors.Errorf("frontend grpc server closed unexpectedly") @@ -412,11 +406,11 @@ func (lbf *llbBridgeForwarder) Result() (*frontend.Result, error) { return lbf.result, nil } -func NewBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, workers worker.Infos, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) LLBBridgeForwarder { - return newBridgeForwarder(ctx, llbBridge, workers, inputs, sid, sm) +func NewBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, exec executor.Executor, workers worker.Infos, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) LLBBridgeForwarder { + return newBridgeForwarder(ctx, llbBridge, exec, workers, inputs, sid, sm) } -func newBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, workers worker.Infos, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) *llbBridgeForwarder { +func newBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, exec executor.Executor, workers worker.Infos, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) *llbBridgeForwarder { lbf := &llbBridgeForwarder{ callCtx: ctx, llbBridge: llbBridge, @@ -429,13 +423,14 @@ func newBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridg sid: sid, sm: sm, ctrs: map[string]gwclient.Container{}, + executor: exec, } return lbf } -func serveLLBBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, workers worker.Infos, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) (*llbBridgeForwarder, context.Context, error) { +func serveLLBBridgeForwarder(ctx context.Context, llbBridge frontend.FrontendLLBBridge, exec executor.Executor, workers worker.Infos, inputs map[string]*opspb.Definition, sid string, sm *session.Manager) (*llbBridgeForwarder, context.Context, error) { ctx, cancel := context.WithCancel(ctx) - lbf := newBridgeForwarder(ctx, llbBridge, workers, inputs, sid, sm) + lbf := newBridgeForwarder(ctx, llbBridge, exec, workers, inputs, sid, sm) server := grpc.NewServer(grpc.UnaryInterceptor(grpcerrors.UnaryServerInterceptor), grpc.StreamInterceptor(grpcerrors.StreamServerInterceptor)) grpc_health_v1.RegisterHealthServer(server, health.NewServer()) pb.RegisterLLBBridgeServer(server, lbf) @@ -530,6 +525,7 @@ type llbBridgeForwarder struct { isErrServerClosed bool sid string sm *session.Manager + executor executor.Executor *pipe ctrs map[string]gwclient.Container ctrsMu sync.Mutex @@ -978,7 +974,7 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta // and we want the context to live for the duration of the container. group := session.NewGroup(lbf.sid) - w, err := lbf.workers.GetDefault() + cm, err := lbf.workers.DefaultCacheManager() if err != nil { return nil, stack.Enable(err) } @@ -988,7 +984,7 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta return nil, stack.Enable(err) } - ctr, err := container.NewContainer(context.Background(), w, lbf.sm, group, ctrReq) + ctr, err := container.NewContainer(context.Background(), cm, lbf.executor, lbf.sm, group, ctrReq) if err != nil { return nil, stack.Enable(err) } diff --git a/snapshot/snapshotter.go b/snapshot/snapshotter.go index edf95cee70cd..3150815bb3bc 100644 --- a/snapshot/snapshotter.go +++ b/snapshot/snapshotter.go @@ -10,14 +10,11 @@ import ( "github.com/containerd/containerd/pkg/userns" "github.com/containerd/containerd/snapshots" "github.com/docker/docker/pkg/idtools" + "github.com/moby/buildkit/executor" "github.com/pkg/errors" ) -type Mountable interface { - // ID() string - Mount() ([]mount.Mount, func() error, error) - IdentityMapping() *idtools.IdentityMapping -} +type Mountable = executor.MountableRef // Snapshotter defines interface that any snapshot implementation should satisfy type Snapshotter interface { diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index 8507280a109a..40ea2fdc7382 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -11,6 +11,7 @@ import ( "github.com/moby/buildkit/cache/remotecache" "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/executor" "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/frontend" gw "github.com/moby/buildkit/frontend/gateway/client" @@ -38,6 +39,10 @@ type llbBridge struct { cms map[string]solver.CacheManager cmsMu sync.Mutex sm *session.Manager + + executorOnce sync.Once + executorErr error + executor executor.Executor } func (b *llbBridge) Warn(ctx context.Context, dgst digest.Digest, msg string, opts frontend.WarnOpts) error { @@ -150,7 +155,7 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest, sid st if !ok { return nil, errors.Errorf("invalid frontend: %s", req.Frontend) } - res, err = f.Solve(ctx, b, req.FrontendOpt, req.FrontendInputs, sid, b.sm) + res, err = f.Solve(ctx, b, b, req.FrontendOpt, req.FrontendInputs, sid, b.sm) if err != nil { return nil, err } @@ -187,6 +192,32 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest, sid st return } +func (b *llbBridge) Run(ctx context.Context, id string, rootfs executor.Mount, mounts []executor.Mount, process executor.ProcessInfo, started chan<- struct{}) error { + if err := b.loadExecutor(); err != nil { + return err + } + return b.executor.Run(ctx, id, rootfs, mounts, process, started) +} + +func (b *llbBridge) Exec(ctx context.Context, id string, process executor.ProcessInfo) error { + if err := b.loadExecutor(); err != nil { + return err + } + return b.executor.Exec(ctx, id, process) +} + +func (b *llbBridge) loadExecutor() error { + b.executorOnce.Do(func() { + w, err := b.resolveWorker() + if err != nil { + b.executorErr = err + return + } + b.executor = w.Executor() + }) + return b.executorErr +} + type resultProxy struct { cb func(context.Context) (solver.CachedResult, solver.BuildSources, error) def *pb.Definition diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index ee06233da5ad..3b1149087bd2 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -81,7 +81,7 @@ func (s *Solver) resolver() solver.ResolveOpFunc { } } -func (s *Solver) Bridge(b solver.Builder) frontend.FrontendLLBBridge { +func (s *Solver) bridge(b solver.Builder) *llbBridge { return &llbBridge{ builder: b, frontends: s.frontends, @@ -93,6 +93,10 @@ func (s *Solver) Bridge(b solver.Builder) frontend.FrontendLLBBridge { } } +func (s *Solver) Bridge(b solver.Builder) frontend.FrontendLLBBridge { + return s.bridge(b) +} + func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req frontend.SolveRequest, exp ExporterRequest, ent []entitlements.Entitlement) (*client.SolveResponse, error) { j, err := s.solver.NewJob(id) if err != nil { @@ -109,9 +113,10 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro j.SessionID = sessionID + br := s.bridge(j) var res *frontend.Result if s.gatewayForwarder != nil && req.Definition == nil && req.Frontend == "" { - fwd := gateway.NewBridgeForwarder(ctx, s.Bridge(j), s.workerController, req.FrontendInputs, sessionID, s.sm) + fwd := gateway.NewBridgeForwarder(ctx, br, br, s.workerController.Infos(), req.FrontendInputs, sessionID, s.sm) defer fwd.Discard() if err := s.gatewayForwarder.RegisterBuild(ctx, id, fwd); err != nil { return nil, err diff --git a/worker/worker.go b/worker/worker.go index 86521c5bab40..39c20738b96f 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -38,6 +38,6 @@ type Worker interface { } type Infos interface { - GetDefault() (Worker, error) + DefaultCacheManager() (cache.Manager, error) WorkerInfos() []client.WorkerInfo } diff --git a/worker/workercontroller.go b/worker/workercontroller.go index 26ca9459231c..c611a47fd379 100644 --- a/worker/workercontroller.go +++ b/worker/workercontroller.go @@ -2,6 +2,7 @@ package worker import ( "github.com/containerd/containerd/filters" + "github.com/moby/buildkit/cache" "github.com/moby/buildkit/client" "github.com/pkg/errors" ) @@ -69,3 +70,25 @@ func (c *Controller) WorkerInfos() []client.WorkerInfo { } return out } + +func (c *Controller) Infos() Infos { + return &infosController{c: c} +} + +type infosController struct { + c *Controller +} + +var _ Infos = &infosController{} + +func (c *infosController) DefaultCacheManager() (cache.Manager, error) { + w, err := c.c.GetDefault() + if err != nil { + return nil, err + } + return w.CacheManager(), nil +} + +func (c *infosController) WorkerInfos() []client.WorkerInfo { + return c.c.WorkerInfos() +} From 8487af10f0c5a775fd0194239a876938778a3a87 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 12 Dec 2023 18:05:01 -0800 Subject: [PATCH 03/10] llbsolver: make sure interactive container API validates entitlements Ensure interactive calls validate same conditions that the build requests do. Refactor of the build side is to ensure we use the same validation function for both cases. There was no validation issue with the LLB validation. Signed-off-by: Tonis Tiigi (cherry picked from commit d1970522d7145be5f4a1f1a028b1910bb527126c) (cherry picked from commit e1e30278d0a491dfd34bd80fa66b54106614cffa) (cherry picked from commit 92cc595cfb12891d4b3ae476e067c74250e4b71e) Signed-off-by: Andrey Epifanov # Conflicts: # client/build_test.go # solver/llbsolver/bridge.go --- client/build_test.go | 58 +++++++++++++++++++++++++++---- solver/llbsolver/bridge.go | 21 +++++++++++ solver/llbsolver/vertex.go | 14 +++----- util/entitlements/entitlements.go | 20 +++++++++++ 4 files changed, 97 insertions(+), 16 deletions(-) diff --git a/client/build_test.go b/client/build_test.go index 23642de80f60..6f3b2068c25e 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -63,7 +63,8 @@ func TestClientGatewayIntegration(t *testing.T) { ), integration.WithMirroredImages(integration.OfficialImages("busybox:latest"))) integration.Run(t, integration.TestFuncs( - testClientGatewayContainerSecurityMode, + testClientGatewayContainerSecurityModeCaps, + testClientGatewayContainerSecurityModeValidation, ), integration.WithMirroredImages(integration.OfficialImages("busybox:latest")), integration.WithMatrix("secmode", map[string]interface{}{ "sandbox": securitySandbox, @@ -72,7 +73,8 @@ func TestClientGatewayIntegration(t *testing.T) { ) integration.Run(t, integration.TestFuncs( - testClientGatewayContainerHostNetworking, + testClientGatewayContainerHostNetworkingAccess, + testClientGatewayContainerHostNetworkingValidation, ), integration.WithMirroredImages(integration.OfficialImages("busybox:latest")), integration.WithMatrix("netmode", map[string]interface{}{ @@ -1621,9 +1623,17 @@ func testClientGatewayExecFileActionError(t *testing.T, sb integration.Sandbox) checkAllReleasable(t, c, sb, true) } -// testClientGatewayContainerSecurityMode ensures that the correct security mode +// testClientGatewayContainerSecurityModeCaps ensures that the correct security mode // is propagated to the gateway container -func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox) { +func testClientGatewayContainerSecurityModeCaps(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerSecurityMode(t, sb, false) +} + +func testClientGatewayContainerSecurityModeValidation(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerSecurityMode(t, sb, true) +} + +func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox, expectFail bool) { requiresLinux(t) ctx := sb.Context() @@ -1649,6 +1659,9 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox require.EqualValues(t, 0xa80425fb, caps) } allowedEntitlements = []entitlements.Entitlement{} + if expectFail { + return + } } else { integration.SkipIfDockerd(t, sb) assertCaps = func(caps uint64) { @@ -1666,6 +1679,9 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox } mode = llb.SecurityModeInsecure allowedEntitlements = []entitlements.Entitlement{entitlements.EntitlementSecurityInsecure} + if expectFail { + allowedEntitlements = []entitlements.Entitlement{} + } } b := func(ctx context.Context, c client.Client) (*client.Result, error) { @@ -1715,6 +1731,12 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox t.Logf("Stdout: %q", stdout.String()) t.Logf("Stderr: %q", stderr.String()) + if expectFail { + require.Error(t, err) + require.Contains(t, err.Error(), "security.insecure is not allowed") + return nil, err + } + require.NoError(t, err) capsValue, err := strconv.ParseUint(strings.TrimSpace(stdout.String()), 16, 64) @@ -1729,7 +1751,13 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox AllowedEntitlements: allowedEntitlements, } _, err = c.Build(ctx, solveOpts, product, b, nil) - require.NoError(t, err) + + if expectFail { + require.Error(t, err) + require.Contains(t, err.Error(), "security.insecure is not allowed") + } else { + require.NoError(t, err) + } checkAllReleasable(t, c, sb, true) } @@ -1805,7 +1833,15 @@ func testClientGatewayContainerExtraHosts(t *testing.T, sb integration.Sandbox) checkAllReleasable(t, c, sb, true) } -func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandbox) { +func testClientGatewayContainerHostNetworkingAccess(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerHostNetworking(t, sb, false) +} + +func testClientGatewayContainerHostNetworkingValidation(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerHostNetworking(t, sb, true) +} + +func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandbox, expectFail bool) { if os.Getenv("BUILDKIT_RUN_NETWORK_INTEGRATION_TESTS") == "" { t.SkipNow() } @@ -1826,6 +1862,9 @@ func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandb if sb.Value("netmode") == hostNetwork { netMode = pb.NetMode_HOST allowedEntitlements = []entitlements.Entitlement{entitlements.EntitlementNetworkHost} + if expectFail { + allowedEntitlements = []entitlements.Entitlement{} + } } c, err := New(sb.Context(), sb.Address()) require.NoError(t, err) @@ -1884,7 +1923,12 @@ func testClientGatewayContainerHostNetworking(t *testing.T, sb integration.Sandb t.Logf("Stderr: %q", stderr.String()) if netMode == pb.NetMode_HOST { - require.NoError(t, err) + if expectFail { + require.Error(t, err) + require.Contains(t, err.Error(), "network.host is not allowed") + } else { + require.NoError(t, err) + } } else { require.Error(t, err) } diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index 40ea2fdc7382..1707654fe4e5 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -23,6 +23,7 @@ import ( "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/bklog" "github.com/moby/buildkit/util/buildinfo" + "github.com/moby/buildkit/util/entitlements" "github.com/moby/buildkit/util/flightcontrol" "github.com/moby/buildkit/util/progress" "github.com/moby/buildkit/worker" @@ -140,6 +141,18 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp return res, bi, nil } +func (b *llbBridge) validateEntitlements(p executor.ProcessInfo) error { + ent, err := loadEntitlements(b.builder) + if err != nil { + return err + } + v := entitlements.Values{ + NetworkHost: p.Meta.NetMode == pb.NetMode_HOST, + SecurityInsecure: p.Meta.SecurityMode == pb.SecurityMode_INSECURE, + } + return ent.Check(v) +} + func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest, sid string) (res *frontend.Result, err error) { if req.Definition != nil && req.Definition.Def != nil && req.Frontend != "" { return nil, errors.New("cannot solve with both Definition and Frontend specified") @@ -193,6 +206,10 @@ func (b *llbBridge) Solve(ctx context.Context, req frontend.SolveRequest, sid st } func (b *llbBridge) Run(ctx context.Context, id string, rootfs executor.Mount, mounts []executor.Mount, process executor.ProcessInfo, started chan<- struct{}) error { + if err := b.validateEntitlements(process); err != nil { + return err + } + if err := b.loadExecutor(); err != nil { return err } @@ -200,6 +217,10 @@ func (b *llbBridge) Run(ctx context.Context, id string, rootfs executor.Mount, m } func (b *llbBridge) Exec(ctx context.Context, id string, process executor.ProcessInfo) error { + if err := b.validateEntitlements(process); err != nil { + return err + } + if err := b.loadExecutor(); err != nil { return err } diff --git a/solver/llbsolver/vertex.go b/solver/llbsolver/vertex.go index 4f36c2eddbb3..ab1778686481 100644 --- a/solver/llbsolver/vertex.go +++ b/solver/llbsolver/vertex.go @@ -99,16 +99,12 @@ func ValidateEntitlements(ent entitlements.Set) LoadOpt { return func(op *pb.Op, _ *pb.OpMetadata, opt *solver.VertexOptions) error { switch op := op.Op.(type) { case *pb.Op_Exec: - if op.Exec.Network == pb.NetMode_HOST { - if !ent.Allowed(entitlements.EntitlementNetworkHost) { - return errors.Errorf("%s is not allowed", entitlements.EntitlementNetworkHost) - } + v := entitlements.Values{ + NetworkHost: op.Exec.Network == pb.NetMode_HOST, + SecurityInsecure: op.Exec.Security == pb.SecurityMode_INSECURE, } - - if op.Exec.Security == pb.SecurityMode_INSECURE { - if !ent.Allowed(entitlements.EntitlementSecurityInsecure) { - return errors.Errorf("%s is not allowed", entitlements.EntitlementSecurityInsecure) - } + if err := ent.Check(v); err != nil { + return err } } return nil diff --git a/util/entitlements/entitlements.go b/util/entitlements/entitlements.go index f65b426bb201..328580c326df 100644 --- a/util/entitlements/entitlements.go +++ b/util/entitlements/entitlements.go @@ -58,3 +58,23 @@ func (s Set) Allowed(e Entitlement) bool { _, ok := s[e] return ok } + +func (s Set) Check(v Values) error { + if v.NetworkHost { + if !s.Allowed(EntitlementNetworkHost) { + return errors.Errorf("%s is not allowed", EntitlementNetworkHost) + } + } + + if v.SecurityInsecure { + if !s.Allowed(EntitlementSecurityInsecure) { + return errors.Errorf("%s is not allowed", EntitlementSecurityInsecure) + } + } + return nil +} + +type Values struct { + NetworkHost bool + SecurityInsecure bool +} From f58634f21be4c49e1f876efbea2e5c6f250eff0c Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Wed, 5 Oct 2022 01:41:31 +0900 Subject: [PATCH 04/10] MountStubsCleaner: preserve timestamps Fix issue 3148 Signed-off-by: Akihiro Suda (cherry picked from commit 0b5a315c221e18eca0ecec4a97f411fcf9b64a74) Signed-off-by: Andrey Epifanov # Conflicts: # client/client_test.go --- client/client_test.go | 70 ++++++++++++++++++++++++++++++++++++ executor/stubs.go | 26 +++++++++++++- util/system/atime_unix.go | 21 +++++++++++ util/system/atime_windows.go | 17 +++++++++ 4 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 util/system/atime_unix.go create mode 100644 util/system/atime_windows.go diff --git a/client/client_test.go b/client/client_test.go index 98d924597563..8df006e9ec64 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -162,6 +162,7 @@ func TestIntegration(t *testing.T) { testUncompressedRegistryCacheImportExport, testStargzLazyRegistryCacheImportExport, testValidateDigestOrigin, + testMountStubsTimestamp, ) tests = append(tests, diffOpTestCases()...) integration.Run(t, tests, mirrors) @@ -5888,3 +5889,72 @@ func fixedWriteCloser(wc io.WriteCloser) func(map[string]string) (io.WriteCloser return wc, nil } } + +// https://github.com/moby/buildkit/issues/3148 +func testMountStubsTimestamp(t *testing.T, sb integration.Sandbox) { + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + const sourceDateEpoch = int64(1234567890) // Fri Feb 13 11:31:30 PM UTC 2009 + st := llb.Image("busybox:latest").Run( + llb.Args([]string{"/bin/touch", fmt.Sprintf("--date=@%d", sourceDateEpoch), + "/bin", + "/etc", + "/var", + "/var/foo", + "/tmp", + "/tmp/foo2", + "/tmp/foo2/bar", + }), + llb.AddMount("/var/foo", llb.Scratch(), llb.Tmpfs()), + llb.AddMount("/tmp/foo2/bar", llb.Scratch(), llb.Tmpfs()), + ) + def, err := st.Marshal(sb.Context()) + require.NoError(t, err) + + tmpDir := t.TempDir() + tarFile := filepath.Join(tmpDir, "out.tar") + tarFileW, err := os.Create(tarFile) + require.NoError(t, err) + defer tarFileW.Close() + + _, err = c.Solve(sb.Context(), def, SolveOpt{ + Exports: []ExportEntry{ + { + Type: ExporterTar, + Output: fixedWriteCloser(tarFileW), + }, + }, + }, nil) + require.NoError(t, err) + tarFileW.Close() + + tarFileR, err := os.Open(tarFile) + require.NoError(t, err) + defer tarFileR.Close() + tarR := tar.NewReader(tarFileR) + touched := map[string]*tar.Header{ + "bin/": nil, // Regular dir + "etc/": nil, // Parent of file mounts (etc/{resolv.conf, hosts}) + "var/": nil, // Parent of dir mount (var/foo/) + "tmp/": nil, // Grandparent of dir mount (tmp/foo2/bar/) + // No support for reproducing the timestamps of mount point directories such as var/foo/ and tmp/foo2/bar/, + // because the touched timestamp value is lost when the mount is unmounted. + } + for { + hd, err := tarR.Next() + if errors.Is(err, io.EOF) { + break + } + require.NoError(t, err) + if x, ok := touched[hd.Name]; ok && x == nil { + touched[hd.Name] = hd + } + } + for name, hd := range touched { + t.Logf("Verifying %q (%+v)", name, hd) + require.NotNil(t, hd, name) + require.Equal(t, sourceDateEpoch, hd.ModTime.Unix(), name) + } +} diff --git a/executor/stubs.go b/executor/stubs.go index 2c13b13053a4..167db203f657 100644 --- a/executor/stubs.go +++ b/executor/stubs.go @@ -7,6 +7,8 @@ import ( "syscall" "github.com/containerd/continuity/fs" + "github.com/moby/buildkit/util/system" + "github.com/sirupsen/logrus" ) func MountStubsCleaner(dir string, mounts []Mount) func() { @@ -43,7 +45,29 @@ func MountStubsCleaner(dir string, mounts []Mount) func() { if st.Size() != 0 { continue } - os.Remove(p) + // Back up the timestamps of the dir for reproducible builds + // https://github.com/moby/buildkit/issues/3148 + dir := filepath.Dir(p) + dirSt, err := os.Stat(dir) + if err != nil { + logrus.WithError(err).Warnf("Failed to stat %q (parent of mount stub %q)", dir, p) + continue + } + mtime := dirSt.ModTime() + atime, err := system.Atime(dirSt) + if err != nil { + logrus.WithError(err).Warnf("Failed to stat atime of %q (parent of mount stub %q)", dir, p) + atime = mtime + } + + if err := os.Remove(p); err != nil { + logrus.WithError(err).Warnf("Failed to remove mount stub %q", p) + } + + // Restore the timestamps of the dir + if err := os.Chtimes(dir, atime, mtime); err != nil { + logrus.WithError(err).Warnf("Failed to restore time time mount stub timestamp (os.Chtimes(%q, %v, %v))", dir, atime, mtime) + } } } } diff --git a/util/system/atime_unix.go b/util/system/atime_unix.go new file mode 100644 index 000000000000..d3f44aa53a32 --- /dev/null +++ b/util/system/atime_unix.go @@ -0,0 +1,21 @@ +//go:build !windows +// +build !windows + +package system + +import ( + "fmt" + iofs "io/fs" + "syscall" + "time" + + "github.com/containerd/continuity/fs" +) + +func Atime(st iofs.FileInfo) (time.Time, error) { + stSys, ok := st.Sys().(*syscall.Stat_t) + if !ok { + return time.Time{}, fmt.Errorf("expected st.Sys() to be *syscall.Stat_t, got %T", st.Sys()) + } + return fs.StatATimeAsTime(stSys), nil +} diff --git a/util/system/atime_windows.go b/util/system/atime_windows.go new file mode 100644 index 000000000000..808408b613cf --- /dev/null +++ b/util/system/atime_windows.go @@ -0,0 +1,17 @@ +package system + +import ( + "fmt" + iofs "io/fs" + "syscall" + "time" +) + +func Atime(st iofs.FileInfo) (time.Time, error) { + stSys, ok := st.Sys().(*syscall.Win32FileAttributeData) + if !ok { + return time.Time{}, fmt.Errorf("expected st.Sys() to be *syscall.Win32FileAttributeData, got %T", st.Sys()) + } + // ref: https://github.com/golang/go/blob/go1.19.2/src/os/types_windows.go#L230 + return time.Unix(0, stSys.LastAccessTime.Nanoseconds()), nil +} From 5c3e1eb47f70efdcfe8f9c7a5ad0dabecd2f9b96 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 23 Nov 2022 18:13:22 +0000 Subject: [PATCH 05/10] executor: stubs cleaner should remove empty directory mounts On Linux, an empty directory is usually 4096 bytes, not 0, so we need an additional explicit check here. Signed-off-by: Justin Chadwell (cherry picked from commit 67789737769f02861d9b4d5a2ec5e14d4e490a29) Signed-off-by: Andrey Epifanov # Conflicts: # client/client_test.go --- client/client_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++ executor/stubs.go | 11 +++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/client/client_test.go b/client/client_test.go index 8df006e9ec64..5a0fc4991ea3 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -163,6 +163,7 @@ func TestIntegration(t *testing.T) { testStargzLazyRegistryCacheImportExport, testValidateDigestOrigin, testMountStubsTimestamp, + testMountStubsDirectory, ) tests = append(tests, diffOpTestCases()...) integration.Run(t, tests, mirrors) @@ -5890,6 +5891,52 @@ func fixedWriteCloser(wc io.WriteCloser) func(map[string]string) (io.WriteCloser } } +func testMountStubsDirectory(t *testing.T, sb integration.Sandbox) { + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + st := llb.Image("busybox:latest").Run( + llb.Args([]string{"/bin/echo", "dummy"}), + llb.AddMount("/foo/bar", llb.Scratch(), llb.Tmpfs()), + ) + def, err := st.Marshal(sb.Context()) + require.NoError(t, err) + + tmpDir := t.TempDir() + tarFile := filepath.Join(tmpDir, "out.tar") + tarFileW, err := os.Create(tarFile) + require.NoError(t, err) + defer tarFileW.Close() + + _, err = c.Solve(sb.Context(), def, SolveOpt{ + Exports: []ExportEntry{ + { + Type: ExporterTar, + Output: fixedWriteCloser(tarFileW), + }, + }, + }, nil) + require.NoError(t, err) + tarFileW.Close() + + tarFileR, err := os.Open(tarFile) + require.NoError(t, err) + defer tarFileR.Close() + tarR := tar.NewReader(tarFileR) + + for { + hd, err := tarR.Next() + if errors.Is(err, io.EOF) { + break + } + require.NoError(t, err) + if hd.Name == "foo/bar/" { + require.Fail(t, "foo/bar/ should not be in the tar") + } + } +} + // https://github.com/moby/buildkit/issues/3148 func testMountStubsTimestamp(t *testing.T, sb integration.Sandbox) { c, err := New(sb.Context(), sb.Address()) diff --git a/executor/stubs.go b/executor/stubs.go index 167db203f657..f4b424d6e889 100644 --- a/executor/stubs.go +++ b/executor/stubs.go @@ -42,9 +42,18 @@ func MountStubsCleaner(dir string, mounts []Mount) func() { if err != nil { continue } - if st.Size() != 0 { + if st.IsDir() { + entries, err := os.ReadDir(p) + if err != nil { + continue + } + if len(entries) != 0 { + continue + } + } else if st.Size() != 0 { continue } + // Back up the timestamps of the dir for reproducible builds // https://github.com/moby/buildkit/issues/3148 dir := filepath.Dir(p) From 067baa8471242212f958f633fbe6d8d0d1933933 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Wed, 23 Nov 2022 18:24:45 +0000 Subject: [PATCH 06/10] chore: tidy atime_unix.go to use errors pkg Signed-off-by: Justin Chadwell (cherry picked from commit 32b5e4dacf083ad1b26d4458558641a2a7196db6) --- util/system/atime_unix.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/system/atime_unix.go b/util/system/atime_unix.go index d3f44aa53a32..9a7af36ffcc6 100644 --- a/util/system/atime_unix.go +++ b/util/system/atime_unix.go @@ -4,18 +4,18 @@ package system import ( - "fmt" iofs "io/fs" "syscall" "time" "github.com/containerd/continuity/fs" + "github.com/pkg/errors" ) func Atime(st iofs.FileInfo) (time.Time, error) { stSys, ok := st.Sys().(*syscall.Stat_t) if !ok { - return time.Time{}, fmt.Errorf("expected st.Sys() to be *syscall.Stat_t, got %T", st.Sys()) + return time.Time{}, errors.Errorf("expected st.Sys() to be *syscall.Stat_t, got %T", st.Sys()) } return fs.StatATimeAsTime(stSys), nil } From 51f945032ece023ac66860d28e7a5e0ad6eb0edb Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 12 Dec 2023 18:41:21 -0800 Subject: [PATCH 07/10] executor: recheck mount stub path within root after container run Signed-off-by: Tonis Tiigi (cherry picked from commit 96ccaec09c51176a6d954fd7c4ce57d519bae1b2) (cherry picked from commit a9523c6476f39bb44dd02bcab19e8cb25c5bc37b) (cherry picked from commit 00fe637d43aba66f0937f5bdf4b9fc96991794fd) Signed-off-by: Andrey Epifanov # Conflicts: # executor/stubs.go --- executor/stubs.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/executor/stubs.go b/executor/stubs.go index f4b424d6e889..c179ff21a9ca 100644 --- a/executor/stubs.go +++ b/executor/stubs.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "strings" "syscall" "github.com/containerd/continuity/fs" @@ -38,6 +39,11 @@ func MountStubsCleaner(dir string, mounts []Mount) func() { return func() { for _, p := range paths { + p, err := fs.RootPath(dir, strings.TrimPrefix(p, dir)) + if err != nil { + continue + } + st, err := os.Lstat(p) if err != nil { continue @@ -56,8 +62,12 @@ func MountStubsCleaner(dir string, mounts []Mount) func() { // Back up the timestamps of the dir for reproducible builds // https://github.com/moby/buildkit/issues/3148 - dir := filepath.Dir(p) - dirSt, err := os.Stat(dir) + parent := filepath.Dir(p) + if realPath, err := fs.RootPath(dir, strings.TrimPrefix(parent, dir)); err != nil || realPath != parent { + continue + } + + dirSt, err := os.Stat(parent) if err != nil { logrus.WithError(err).Warnf("Failed to stat %q (parent of mount stub %q)", dir, p) continue @@ -74,7 +84,7 @@ func MountStubsCleaner(dir string, mounts []Mount) func() { } // Restore the timestamps of the dir - if err := os.Chtimes(dir, atime, mtime); err != nil { + if err := os.Chtimes(parent, atime, mtime); err != nil { logrus.WithError(err).Warnf("Failed to restore time time mount stub timestamp (os.Chtimes(%q, %v, %v))", dir, atime, mtime) } } From eb5b3262881671f50eb8dabaad672b5896042240 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Jan 2024 15:05:14 -0800 Subject: [PATCH 08/10] oci: fix error handling on submount calls Signed-off-by: Tonis Tiigi (cherry picked from commit 42d866ef3bcb333b21734acb861e3009449f4058) (cherry picked from commit e81066f8a8623dc876f3d64fae8f693c17ecdc1a) (cherry picked from commit d089e0b9527ba85075a2db02b55e7002847a0e8d) --- executor/oci/spec.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/oci/spec.go b/executor/oci/spec.go index 94b48a7aa9ff..725a6aa23d4d 100644 --- a/executor/oci/spec.go +++ b/executor/oci/spec.go @@ -213,12 +213,12 @@ func (s *submounts) subMount(m mount.Mount, subPath string) (mount.Mount, error) } h, err := hashstructure.Hash(m, hashstructure.FormatV2, nil) if err != nil { - return mount.Mount{}, nil + return mount.Mount{}, err } if mr, ok := s.m[h]; ok { sm, err := sub(mr.mount, subPath) if err != nil { - return mount.Mount{}, nil + return mount.Mount{}, err } return sm, nil } From b379ed2564e0268ab3963ebfb349774ff36ab935 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Dec 2023 20:23:10 -0800 Subject: [PATCH 09/10] exec: add extra validation for submount sources While submount paths were already validated there are some cases where the parent mount may not be immutable while the submount is created. Signed-off-by: Tonis Tiigi (cherry picked from commit 2529ec4121bcd8c35bcd96218083da175c2e5b77) (cherry picked from commit cbc233b3b695918d92fd5b1407b829296c53db70) (cherry picked from commit f781267af1acb688e94740e1fdc22c1bf587d7fd) Signed-off-by: Andrey Epifanov # Conflicts: # executor/oci/spec.go # executor/oci/spec_windows.go # snapshot/localmounter_unix.go --- executor/oci/spec.go | 30 ++++++++++-------- executor/oci/spec_freebsd.go | 15 +++++++++ executor/oci/spec_linux.go | 57 +++++++++++++++++++++++++++++++++++ executor/oci/spec_windows.go | 11 +++++++ snapshot/localmounter.go | 35 +++++++++++++++------ snapshot/localmounter_unix.go | 46 +++++++++++++++++++--------- 6 files changed, 159 insertions(+), 35 deletions(-) create mode 100644 executor/oci/spec_freebsd.go create mode 100644 executor/oci/spec_linux.go diff --git a/executor/oci/spec.go b/executor/oci/spec.go index 725a6aa23d4d..012bfb725126 100644 --- a/executor/oci/spec.go +++ b/executor/oci/spec.go @@ -11,7 +11,6 @@ import ( "github.com/containerd/containerd/mount" "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/oci" - "github.com/containerd/continuity/fs" "github.com/docker/docker/pkg/idtools" "github.com/mitchellh/hashstructure/v2" "github.com/moby/buildkit/executor" @@ -198,6 +197,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou type mountRef struct { mount mount.Mount unmount func() error + subRefs map[string]mountRef } type submounts struct { @@ -216,10 +216,17 @@ func (s *submounts) subMount(m mount.Mount, subPath string) (mount.Mount, error) return mount.Mount{}, err } if mr, ok := s.m[h]; ok { - sm, err := sub(mr.mount, subPath) + if sm, ok := mr.subRefs[subPath]; ok { + return sm.mount, nil + } + sm, unmount, err := sub(mr.mount, subPath) if err != nil { return mount.Mount{}, err } + mr.subRefs[subPath] = mountRef{ + mount: sm, + unmount: unmount, + } return sm, nil } @@ -244,12 +251,17 @@ func (s *submounts) subMount(m mount.Mount, subPath string) (mount.Mount, error) Options: opts, }, unmount: lm.Unmount, + subRefs: map[string]mountRef{}, } - sm, err := sub(s.m[h].mount, subPath) + sm, unmount, err := sub(s.m[h].mount, subPath) if err != nil { return mount.Mount{}, err } + s.m[h].subRefs[subPath] = mountRef{ + mount: sm, + unmount: unmount, + } return sm, nil } @@ -259,6 +271,9 @@ func (s *submounts) cleanup() { for _, m := range s.m { func(m mountRef) { go func() { + for _, sm := range m.subRefs { + sm.unmount() + } m.unmount() wg.Done() }() @@ -267,15 +282,6 @@ func (s *submounts) cleanup() { wg.Wait() } -func sub(m mount.Mount, subPath string) (mount.Mount, error) { - src, err := fs.RootPath(m.Source, subPath) - if err != nil { - return mount.Mount{}, err - } - m.Source = src - return m, nil -} - func specMapping(s []idtools.IDMap) []specs.LinuxIDMapping { var ids []specs.LinuxIDMapping for _, item := range s { diff --git a/executor/oci/spec_freebsd.go b/executor/oci/spec_freebsd.go new file mode 100644 index 000000000000..0810bc428867 --- /dev/null +++ b/executor/oci/spec_freebsd.go @@ -0,0 +1,15 @@ +package oci + +import ( + "github.com/containerd/containerd/mount" + "github.com/containerd/continuity/fs" +) + +func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) { + src, err := fs.RootPath(m.Source, subPath) + if err != nil { + return mount.Mount{}, nil, err + } + m.Source = src + return m, func() error { return nil }, nil +} diff --git a/executor/oci/spec_linux.go b/executor/oci/spec_linux.go new file mode 100644 index 000000000000..abbf0879d87a --- /dev/null +++ b/executor/oci/spec_linux.go @@ -0,0 +1,57 @@ +//go:build linux +// +build linux + +package oci + +import ( + "os" + "strconv" + + "github.com/containerd/containerd/mount" + "github.com/containerd/continuity/fs" + "github.com/moby/buildkit/snapshot" + "github.com/pkg/errors" + "golang.org/x/sys/unix" +) + +func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) { + var retries = 10 + root := m.Source + for { + src, err := fs.RootPath(root, subPath) + if err != nil { + return mount.Mount{}, nil, err + } + // similar to runc.WithProcfd + fh, err := os.OpenFile(src, unix.O_PATH|unix.O_CLOEXEC, 0) + if err != nil { + return mount.Mount{}, nil, err + } + + fdPath := "/proc/self/fd/" + strconv.Itoa(int(fh.Fd())) + if resolved, err := os.Readlink(fdPath); err != nil { + fh.Close() + return mount.Mount{}, nil, err + } else if resolved != src { + retries-- + if retries <= 0 { + fh.Close() + return mount.Mount{}, nil, errors.Errorf("unable to safely resolve subpath %s", subPath) + } + fh.Close() + continue + } + + m.Source = fdPath + lm := snapshot.LocalMounterWithMounts([]mount.Mount{m}, snapshot.ForceRemount()) + mp, err := lm.Mount() + if err != nil { + fh.Close() + return mount.Mount{}, nil, err + } + m.Source = mp + fh.Close() // release the fd, we don't need it anymore + + return m, lm.Unmount, nil + } +} diff --git a/executor/oci/spec_windows.go b/executor/oci/spec_windows.go index 48b0969e3922..757bd397dec4 100644 --- a/executor/oci/spec_windows.go +++ b/executor/oci/spec_windows.go @@ -4,7 +4,9 @@ package oci import ( + "github.com/containerd/containerd/mount" "github.com/containerd/containerd/oci" + "github.com/containerd/continuity/fs" "github.com/docker/docker/pkg/idtools" "github.com/moby/buildkit/solver/pb" "github.com/pkg/errors" @@ -43,3 +45,12 @@ func generateRlimitOpts(ulimits []*pb.Ulimit) ([]oci.SpecOpts, error) { } return nil, errors.New("no support for POSIXRlimit on Windows") } + +func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) { + src, err := fs.RootPath(m.Source, subPath) + if err != nil { + return mount.Mount{}, nil, err + } + m.Source = src + return m, func() error { return nil }, nil +} diff --git a/snapshot/localmounter.go b/snapshot/localmounter.go index 9ddb7c1af642..304eebc9e02d 100644 --- a/snapshot/localmounter.go +++ b/snapshot/localmounter.go @@ -11,22 +11,39 @@ type Mounter interface { Unmount() error } +type LocalMounterOpt func(*localMounter) + // LocalMounter is a helper for mounting mountfactory to temporary path. In // addition it can mount binds without privileges -func LocalMounter(mountable Mountable) Mounter { - return &localMounter{mountable: mountable} +func LocalMounter(mountable Mountable, opts ...LocalMounterOpt) Mounter { + lm := &localMounter{mountable: mountable} + for _, opt := range opts { + opt(lm) + } + return lm } // LocalMounterWithMounts is a helper for mounting to temporary path. In // addition it can mount binds without privileges -func LocalMounterWithMounts(mounts []mount.Mount) Mounter { - return &localMounter{mounts: mounts} +func LocalMounterWithMounts(mounts []mount.Mount, opts ...LocalMounterOpt) Mounter { + lm := &localMounter{mounts: mounts} + for _, opt := range opts { + opt(lm) + } + return lm } type localMounter struct { - mu sync.Mutex - mounts []mount.Mount - mountable Mountable - target string - release func() error + mu sync.Mutex + mounts []mount.Mount + mountable Mountable + target string + release func() error + forceRemount bool +} + +func ForceRemount() LocalMounterOpt { + return func(lm *localMounter) { + lm.forceRemount = true + } } diff --git a/snapshot/localmounter_unix.go b/snapshot/localmounter_unix.go index ef73e263fc91..4bace4e664c7 100644 --- a/snapshot/localmounter_unix.go +++ b/snapshot/localmounter_unix.go @@ -4,8 +4,8 @@ package snapshot import ( - "io/ioutil" "os" + "path/filepath" "syscall" "github.com/containerd/containerd/mount" @@ -25,30 +25,48 @@ func (lm *localMounter) Mount() (string, error) { lm.release = release } + var isFile bool if len(lm.mounts) == 1 && (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") { - ro := false - for _, opt := range lm.mounts[0].Options { - if opt == "ro" { - ro = true - break + if !lm.forceRemount { + ro := false + for _, opt := range lm.mounts[0].Options { + if opt == "ro" { + ro = true + break + } } + if !ro { + return lm.mounts[0].Source, nil + } + } + fi, err := os.Stat(lm.mounts[0].Source) + if err != nil { + return "", err } - if !ro { - return lm.mounts[0].Source, nil + if !fi.IsDir() { + isFile = true } } - dir, err := ioutil.TempDir("", "buildkit-mount") + dest, err := os.MkdirTemp("", "buildkit-mount") if err != nil { return "", errors.Wrap(err, "failed to create temp dir") } - if err := mount.All(lm.mounts, dir); err != nil { - os.RemoveAll(dir) - return "", errors.Wrapf(err, "failed to mount %s: %+v", dir, lm.mounts) + if isFile { + dest = filepath.Join(dest, "file") + if err := os.WriteFile(dest, []byte{}, 0644); err != nil { + os.RemoveAll(dest) + return "", errors.Wrap(err, "failed to create temp file") + } + } + + if err := mount.All(lm.mounts, dest); err != nil { + os.RemoveAll(dest) + return "", errors.Wrapf(err, "failed to mount %s: %+v", dest, lm.mounts) } - lm.target = dir - return dir, nil + lm.target = dest + return dest, nil } func (lm *localMounter) Unmount() error { From 7f341668b8be66bfb3c592a439a219b801f9cb34 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 29 Jan 2024 15:03:28 -0800 Subject: [PATCH 10/10] update runc to v1.1.12 Signed-off-by: Tonis Tiigi (cherry picked from commit bac3f2b673f3f9d33e79046008e7a38e856b3dc6) Signed-off-by: Andrey Epifanov # Conflicts: # Dockerfile --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 3dc9c06c60ff..ce5d8addf13c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # syntax=docker/dockerfile-upstream:master -ARG RUNC_VERSION=v1.0.2 -ARG CONTAINERD_VERSION=v1.6.2 +ARG RUNC_VERSION=v1.1.12 +ARG CONTAINERD_VERSION=v1.6.28 # containerd v1.5 for integration tests ARG CONTAINERD_ALT_VERSION_15=v1.5.11 # containerd v1.4 for integration tests