From 104dc5fff280a84f02082af6001b13819bc8d88a Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 12 Dec 2023 13:42:46 -0800 Subject: [PATCH 1/2] 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 0c5daa23277e9e1fa8b2d794903cd97df95496fb) --- cmd/buildkitd/main.go | 4 ++-- executor/executor.go | 11 +++++++-- frontend/frontend.go | 3 ++- frontend/gateway/container.go | 9 ++++---- frontend/gateway/forwarder/forward.go | 9 +++++--- frontend/gateway/forwarder/frontend.go | 5 ++-- frontend/gateway/gateway.go | 28 ++++++++++------------ snapshot/snapshotter.go | 7 ++---- solver/llbsolver/bridge.go | 32 ++++++++++++++++++++++++++ solver/llbsolver/provenance.go | 2 +- solver/llbsolver/solver.go | 2 +- worker/worker.go | 2 +- worker/workercontroller.go | 23 ++++++++++++++++++ 13 files changed, 98 insertions(+), 39 deletions(-) diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index ca411066b30f..404d3e3459aa 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -646,8 +646,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 a323bcc9cc94..c70ed047a9c5 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -6,7 +6,9 @@ import ( "net" "syscall" - "github.com/moby/buildkit/snapshot" + "github.com/containerd/containerd/mount" + "github.com/docker/docker/pkg/idtools" + resourcestypes "github.com/moby/buildkit/executor/resources/types" "github.com/moby/buildkit/solver/pb" ) @@ -27,8 +29,13 @@ type Meta struct { RemoveMountStubsRecursive bool } +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 024ac802045c..4a068d17d41f 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" @@ -17,7 +18,7 @@ type Result = result.Result[solver.ResultProxy] type Attestation = result.Attestation[solver.ResultProxy] 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.go b/frontend/gateway/container.go index d6161d1def93..9fb4d928d66d 100644 --- a/frontend/gateway/container.go +++ b/frontend/gateway/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 e13894ba37ed..4c374e781de9 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" "github.com/moby/buildkit/frontend/gateway/client" @@ -26,7 +27,7 @@ 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) { +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, @@ -35,6 +36,7 @@ func llbBridgeToGatewayClient(ctx context.Context, llbBridge frontend.FrontendLL sm: sm, workers: w, workerRefByID: make(map[string]*worker.WorkerRef), + executor: exec, } bc.buildOpts = bc.loadBuildOpts() return bc, nil @@ -52,6 +54,7 @@ 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) { @@ -292,13 +295,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 := gateway.NewContainer(ctx, w, c.sm, group, ctrReq) + ctr, err := gateway.NewContainer(ctx, cm, c.executor, c.sm, group, ctrReq) if err != nil { return nil, err } diff --git a/frontend/gateway/forwarder/frontend.go b/frontend/gateway/forwarder/frontend.go index 7cd25a0e8ea0..331559a39057 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 79825d0b651a..5cb39e07e965 100644 --- a/frontend/gateway/gateway.go +++ b/frontend/gateway/gateway.go @@ -82,7 +82,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") @@ -251,18 +251,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 @@ -275,8 +270,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 = 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") @@ -405,11 +399,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, @@ -422,13 +416,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) @@ -523,6 +518,7 @@ type llbBridgeForwarder struct { isErrServerClosed bool sid string sm *session.Manager + executor executor.Executor *pipe ctrs map[string]gwclient.Container ctrsMu sync.Mutex @@ -1001,7 +997,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) } @@ -1011,7 +1007,7 @@ func (lbf *llbBridgeForwarder) NewContainer(ctx context.Context, in *pb.NewConta return nil, stack.Enable(err) } - ctr, err := NewContainer(context.Background(), w, lbf.sm, group, ctrReq) + ctr, err := 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 185fe81f0649..efbfe5615e9a 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -11,6 +11,8 @@ import ( "github.com/moby/buildkit/cache/remotecache" "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/executor" + resourcestypes "github.com/moby/buildkit/executor/resources/types" "github.com/moby/buildkit/frontend" gw "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/identity" @@ -39,6 +41,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 { @@ -151,6 +157,32 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp return res, nil } +func (b *llbBridge) Run(ctx context.Context, id string, rootfs executor.Mount, mounts []executor.Mount, process executor.ProcessInfo, started chan<- struct{}) (resourcestypes.Recorder, error) { + if err := b.loadExecutor(); err != nil { + return nil, 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 { id string b *provenanceBridge diff --git a/solver/llbsolver/provenance.go b/solver/llbsolver/provenance.go index b30581c852d9..8b60f5e885fb 100644 --- a/solver/llbsolver/provenance.go +++ b/solver/llbsolver/provenance.go @@ -161,7 +161,7 @@ func (b *provenanceBridge) Solve(ctx context.Context, req frontend.SolveRequest, return nil, errors.Errorf("invalid frontend: %s", req.Frontend) } wb := &provenanceBridge{llbBridge: b.llbBridge, req: &req} - res, err = f.Solve(ctx, wb, req.FrontendOpt, req.FrontendInputs, sid, b.llbBridge.sm) + res, err = f.Solve(ctx, wb, b.llbBridge, req.FrontendOpt, req.FrontendInputs, sid, b.llbBridge.sm) if err != nil { return nil, err } diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index 94d25ce5b7b2..c9921085337c 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -440,7 +440,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro br := s.bridge(j) var fwd gateway.LLBBridgeForwarder if s.gatewayForwarder != nil && req.Definition == nil && req.Frontend == "" { - fwd = gateway.NewBridgeForwarder(ctx, br, s.workerController, req.FrontendInputs, sessionID, s.sm) + fwd = gateway.NewBridgeForwarder(ctx, br, br, s.workerController.Infos(), req.FrontendInputs, sessionID, s.sm) defer fwd.Discard() // Register build before calling s.recordBuildHistory, because // s.recordBuildHistory can block for several seconds on diff --git a/worker/worker.go b/worker/worker.go index 2f426e9ead40..0a708227204b 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -43,6 +43,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 e175b4002b4a..150eed352a3a 100644 --- a/worker/workercontroller.go +++ b/worker/workercontroller.go @@ -3,6 +3,7 @@ package worker import ( "github.com/containerd/containerd/filters" "github.com/hashicorp/go-multierror" + "github.com/moby/buildkit/cache" "github.com/moby/buildkit/client" "github.com/pkg/errors" ) @@ -81,3 +82,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 35615b97aa114f370bcb25de973208e89f0f23c2 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 12 Dec 2023 18:05:01 -0800 Subject: [PATCH 2/2] 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 65c3c9c135cba5c9e28d646e85b3c77fa93bbf0c) --- client/build_test.go | 58 +++++++++++++++++++++++++++---- executor/executor.go | 1 - frontend/gateway/gateway.go | 2 +- solver/llbsolver/bridge.go | 26 ++++++++++++-- solver/llbsolver/vertex.go | 14 +++----- util/entitlements/entitlements.go | 20 +++++++++++ 6 files changed, 100 insertions(+), 21 deletions(-) diff --git a/client/build_test.go b/client/build_test.go index a6bc37a558e9..7043cae9f155 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -60,7 +60,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, @@ -69,7 +70,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{}{ @@ -1684,9 +1686,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) { integration.CheckFeatureCompat(t, sb, integration.FeatureSecurityMode) requiresLinux(t) @@ -1713,6 +1723,9 @@ func testClientGatewayContainerSecurityMode(t *testing.T, sb integration.Sandbox require.EqualValues(t, 0xa80425fb, caps) } allowedEntitlements = []entitlements.Entitlement{} + if expectFail { + return + } } else { assertCaps = func(caps uint64) { /* @@ -1729,6 +1742,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) { @@ -1778,6 +1794,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) @@ -1792,7 +1814,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) } @@ -1868,7 +1896,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() } @@ -1889,6 +1925,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) @@ -1947,7 +1986,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/executor/executor.go b/executor/executor.go index c70ed047a9c5..61da4c9dd7c3 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -8,7 +8,6 @@ import ( "github.com/containerd/containerd/mount" "github.com/docker/docker/pkg/idtools" - resourcestypes "github.com/moby/buildkit/executor/resources/types" "github.com/moby/buildkit/solver/pb" ) diff --git a/frontend/gateway/gateway.go b/frontend/gateway/gateway.go index 5cb39e07e965..8f2af4d34101 100644 --- a/frontend/gateway/gateway.go +++ b/frontend/gateway/gateway.go @@ -270,7 +270,7 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten mnts = append(mnts, *mdmnt) } - _, 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) + err = exec.Run(ctx, "", 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") diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index efbfe5615e9a..5c95a3e7e958 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -12,7 +12,6 @@ import ( "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/executor" - resourcestypes "github.com/moby/buildkit/executor/resources/types" "github.com/moby/buildkit/frontend" gw "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/identity" @@ -25,6 +24,7 @@ import ( "github.com/moby/buildkit/sourcepolicy" spb "github.com/moby/buildkit/sourcepolicy/pb" "github.com/moby/buildkit/util/bklog" + "github.com/moby/buildkit/util/entitlements" "github.com/moby/buildkit/util/flightcontrol" "github.com/moby/buildkit/util/progress" "github.com/moby/buildkit/worker" @@ -157,14 +157,34 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp return res, nil } -func (b *llbBridge) Run(ctx context.Context, id string, rootfs executor.Mount, mounts []executor.Mount, process executor.ProcessInfo, started chan<- struct{}) (resourcestypes.Recorder, error) { +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) 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 nil, err + 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.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 41a31bb9bbba..d57f2a053db1 100644 --- a/solver/llbsolver/vertex.go +++ b/solver/llbsolver/vertex.go @@ -101,16 +101,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 +}