From efa5230180e85c5ee5aace5a5f161bfe1d2ebfff Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 1 Oct 2026 18:44:32 -0400 Subject: [PATCH] builder-next: set the sandbox key through ocihook BuildKit's bridge network provider creates its sandboxes with OptionUseExternalKey, so that the sandbox uses the network namespace runc creates inside the container's user namespace, and installs a prestart hook which passes that namespace to libnetwork. The hook re-executed dockerd as "libnetwork-setkey", which located libnetwork's external key listener from the controller's ID and the exec root, and the listener looked the sandbox up by container ID. Make the daemon own an ocihook.Server listening on /oci-hook.sock, shut it down before the network controller, and pass it to BuildKit. The provider registers a callback which calls SetKey on its own sandbox, and unregisters it when the namespace is closed. It registers the callback with the context BuildKit passed to the provider when creating the namespace, which is the context of the container run, so spans started by SetKey are children of the build step's span, and a cancelled build step cancels SetKey. Signed-off-by: Cory Snider Assisted-By: Claude Opus 5.5 --- daemon/command/daemon.go | 1 + daemon/daemon.go | 14 +++++++++++++ daemon/daemon_unix.go | 12 +++++++++++ daemon/daemon_windows.go | 5 +++++ daemon/internal/builder-next/builder.go | 2 ++ daemon/internal/builder-next/controller.go | 2 ++ daemon/internal/builder-next/executor.go | 18 ++++++++++++++--- .../internal/builder-next/executor_linux.go | 20 ++++++++++--------- daemon/internal/builder-next/executor_opts.go | 2 ++ daemon/network.go | 7 +++++++ 10 files changed, 71 insertions(+), 12 deletions(-) diff --git a/daemon/command/daemon.go b/daemon/command/daemon.go index 85c7709ad1..c79ccd958e 100644 --- a/daemon/command/daemon.go +++ b/daemon/command/daemon.go @@ -480,6 +480,7 @@ func initBuildkit(ctx context.Context, d *daemon.Daemon, cdiCache *cdi.Cache, co Dist: d.DistributionServices(), ImageTagger: d.ImageService(), NetworkController: d.NetworkController(), + OCIHooks: d.OCIHooks(), DefaultCgroupParent: newCgroupParent(&cfg), RegistryHosts: d.RegistryHosts, BuilderConfig: cfg.Builder, diff --git a/daemon/daemon.go b/daemon/daemon.go index fb82475719..910303d88c 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -70,6 +70,7 @@ import ( "github.com/moby/moby/v2/daemon/internal/libcontainerd" libcontainerdtypes "github.com/moby/moby/v2/daemon/internal/libcontainerd/types" "github.com/moby/moby/v2/daemon/internal/metrics" + "github.com/moby/moby/v2/daemon/internal/ocihook" pluginexec "github.com/moby/moby/v2/daemon/internal/plugin/executor/containerd" refstore "github.com/moby/moby/v2/daemon/internal/refstore" "github.com/moby/moby/v2/daemon/libnetwork" @@ -123,6 +124,7 @@ type Daemon struct { EventsService *events.Events netController *libnetwork.Controller netStore *boltdb.BoltDB + ociHooks *ocihook.Server volumes *volumesservice.VolumesService root string sysInfoOnce sync.Once @@ -1017,6 +1019,10 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S return nil, err } + if err := d.initOCIHooks(&cfgStore.Config); err != nil { + return nil, err + } + const connTimeout = 60 * time.Second gopts := []grpc.DialOption{ @@ -1593,6 +1599,14 @@ func (daemon *Daemon) Shutdown(ctx context.Context) error { daemon.nri.Shutdown(ctx) } + // Hook callbacks may use the network controller, so wait for them to + // return first. + if daemon.ociHooks != nil { + if err := daemon.ociHooks.Shutdown(ctx); err != nil { + log.G(ctx).WithError(err).Warn("error shutting down OCI hook server") + } + } + // trigger libnetwork Stop only if it's initialized if daemon.netController != nil { daemon.netController.Stop() diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index a3dc4649e7..62ef0725f9 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -28,6 +28,7 @@ import ( "github.com/moby/moby/v2/daemon/config" "github.com/moby/moby/v2/daemon/container" "github.com/moby/moby/v2/daemon/initlayer" + "github.com/moby/moby/v2/daemon/internal/ocihook" "github.com/moby/moby/v2/daemon/internal/otelutil" "github.com/moby/moby/v2/daemon/internal/usergroup" "github.com/moby/moby/v2/daemon/libnetwork" @@ -827,6 +828,17 @@ func configureKernelSecuritySupport(config *config.Config, driverName string) er return nil } +// initOCIHooks starts the server for OCI hooks the daemon installs in +// containers it does not start itself. +func (daemon *Daemon) initOCIHooks(cfg *config.Config) error { + s, err := ocihook.Listen(filepath.Join(cfg.GetExecRoot(), "oci-hook.sock")) + if err != nil { + return fmt.Errorf("error starting OCI hook server: %w", err) + } + daemon.ociHooks = s + return nil +} + // initNetworkController initializes the libnetwork controller and configures // network settings. If there's active sandboxes, configuration changes will not // take effect. diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index 39cf72b213..88d9b3a34b 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -233,6 +233,11 @@ func configureMaxThreads(_ context.Context) error { return nil } +// initOCIHooks is a no-op on Windows, where nothing installs OCI hooks. +func (daemon *Daemon) initOCIHooks(*config.Config) error { + return nil +} + func (daemon *Daemon) initNetworkController(daemonCfg *config.Config, activeSandboxes map[string]any) error { netOptions, err := daemon.networkOptions(daemonCfg, nil, daemon.id, nil) if err != nil { diff --git a/daemon/internal/builder-next/builder.go b/daemon/internal/builder-next/builder.go index 8f95729b30..87b14cbf10 100644 --- a/daemon/internal/builder-next/builder.go +++ b/daemon/internal/builder-next/builder.go @@ -30,6 +30,7 @@ import ( "github.com/moby/moby/v2/daemon/internal/builder-next/exporter" "github.com/moby/moby/v2/daemon/internal/builder-next/exporter/mobyexporter" "github.com/moby/moby/v2/daemon/internal/builder-next/exporter/overrides" + "github.com/moby/moby/v2/daemon/internal/ocihook" "github.com/moby/moby/v2/daemon/internal/streamformatter" "github.com/moby/moby/v2/daemon/internal/timestamp" "github.com/moby/moby/v2/daemon/libnetwork" @@ -89,6 +90,7 @@ type Opt struct { Dist images.DistributionServices ImageTagger mobyexporter.ImageTagger NetworkController *libnetwork.Controller + OCIHooks *ocihook.Server DefaultCgroupParent string RegistryHosts docker.RegistryHosts BuilderConfig config.BuilderConfig diff --git a/daemon/internal/builder-next/controller.go b/daemon/internal/builder-next/controller.go index 04e080bd27..6688493bda 100644 --- a/daemon/internal/builder-next/controller.go +++ b/daemon/internal/builder-next/controller.go @@ -166,6 +166,7 @@ func newSnapshotterController(ctx context.Context, rt http.RoundTripper, opt Opt exec, proxyProvider, err := newExecutor(executorOpts{ root: opt.Root, networkController: opt.NetworkController, + ociHooks: opt.OCIHooks, dnsConfig: dnsConfig, cdiManager: cdiManager, cgroupParent: opt.DefaultCgroupParent, @@ -386,6 +387,7 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt exec, proxyProvider, err := newExecutorGD(executorOpts{ root: root, networkController: opt.NetworkController, + ociHooks: opt.OCIHooks, dnsConfig: getDNSConfig(opt.DNSConfig), cdiManager: cdiManager, cgroupParent: opt.DefaultCgroupParent, diff --git a/daemon/internal/builder-next/executor.go b/daemon/internal/builder-next/executor.go index e7d9bcb5cd..2213a640a5 100644 --- a/daemon/internal/builder-next/executor.go +++ b/daemon/internal/builder-next/executor.go @@ -14,12 +14,14 @@ import ( "github.com/moby/buildkit/identity" "github.com/moby/buildkit/util/network" "github.com/moby/moby/v2/daemon/config" + "github.com/moby/moby/v2/daemon/internal/ocihook" "github.com/moby/moby/v2/daemon/libnetwork" ) type bridgeProvider struct { *libnetwork.Controller - Root string + Hooks *ocihook.Server + Root string } type lnInterface struct { @@ -29,15 +31,22 @@ type lnInterface struct { err error ready chan struct{} provider *bridgeProvider + + // ctx is the context of the container run the namespace was created + // for. It is the parent context of the hook which sets the sandbox key, + // so it must not be done before the container has started: the hook + // fails if it is. + ctx context.Context + unregisterHook func() } -func (p *bridgeProvider) New(_ context.Context, _ string, _ network.NamespaceOptions) (network.Namespace, error) { +func (p *bridgeProvider) New(ctx context.Context, _ string, _ network.NamespaceOptions) (network.Namespace, error) { n, err := p.NetworkByName(networkName) if err != nil { return nil, err } - iface := &lnInterface{ready: make(chan struct{}), provider: p} + iface := &lnInterface{ready: make(chan struct{}), provider: p, ctx: ctx} iface.Once.Do(func() { go iface.init(p.Controller, n) }) @@ -87,6 +96,9 @@ func (iface *lnInterface) Sample() (*resourcestypes.NetworkSample, error) { func (iface *lnInterface) Close() error { <-iface.ready + if iface.unregisterHook != nil { + iface.unregisterHook() + } if iface.sbx != nil { go func() { if err := iface.sbx.Delete(context.TODO()); err != nil { diff --git a/daemon/internal/builder-next/executor_linux.go b/daemon/internal/builder-next/executor_linux.go index 895408c2ef..26587f446c 100644 --- a/daemon/internal/builder-next/executor_linux.go +++ b/daemon/internal/builder-next/executor_linux.go @@ -2,10 +2,10 @@ package buildkit import ( "context" + "fmt" "net" "os" "path/filepath" - "strconv" "strings" "github.com/containerd/log" @@ -15,7 +15,6 @@ import ( "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/network" "github.com/moby/buildkit/util/network/proxyprovider" - "github.com/moby/moby/v2/daemon/internal/stringid" "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" ) @@ -25,7 +24,7 @@ const networkName = "bridge" func newExecutor(opts executorOpts) (executor.Executor, network.ProxyProvider, error) { netRoot := filepath.Join(opts.root, "net") networkProviders := map[pb.NetMode]network.Provider{ - pb.NetMode_UNSET: &bridgeProvider{Controller: opts.networkController, Root: netRoot}, + pb.NetMode_UNSET: &bridgeProvider{Controller: opts.networkController, Hooks: opts.ociHooks, Root: netRoot}, pb.NetMode_HOST: network.NewHostProvider(), pb.NetMode_NONE: network.NewNoneProvider(), } @@ -166,13 +165,16 @@ func (iface *lnInterface) Set(s *specs.Spec) error { log.G(context.TODO()).WithError(iface.err).Error("failed to set networking spec") return iface.err } - shortNetCtlrID := stringid.TruncateID(iface.provider.Controller.ID()) - // attach netns to bridge within the container namespace, using reexec in a prestart hook + if iface.unregisterHook != nil { + iface.unregisterHook() + } + // attach netns to bridge within the container namespace, in a prestart hook + hook, unregister := iface.provider.Hooks.Register(iface.ctx, func(ctx context.Context, st specs.State) error { + return iface.sbx.SetKey(ctx, fmt.Sprintf("/proc/%d/ns/net", st.Pid)) + }) + iface.unregisterHook = unregister s.Hooks = &specs.Hooks{ - Prestart: []specs.Hook{{ //nolint:staticcheck // ignore SA1019 (Prestart is deprecated); FIXME(thaJeztah): see https://github.com/moby/moby/issues/52488 - Path: filepath.Join("/proc", strconv.Itoa(os.Getpid()), "exe"), - Args: []string{"libnetwork-setkey", "-exec-root=" + iface.provider.Config().ExecRoot, iface.sbx.ContainerID(), shortNetCtlrID}, - }}, + Prestart: []specs.Hook{hook}, //nolint:staticcheck // ignore SA1019 (Prestart is deprecated); FIXME(thaJeztah): see https://github.com/moby/moby/issues/52488 } return nil } diff --git a/daemon/internal/builder-next/executor_opts.go b/daemon/internal/builder-next/executor_opts.go index 2890cfdeb6..7989ca10cc 100644 --- a/daemon/internal/builder-next/executor_opts.go +++ b/daemon/internal/builder-next/executor_opts.go @@ -7,6 +7,7 @@ import ( "github.com/moby/buildkit/executor/oci" "github.com/moby/buildkit/solver/llbsolver/cdidevices" "github.com/moby/buildkit/util/network" + "github.com/moby/moby/v2/daemon/internal/ocihook" "github.com/moby/moby/v2/daemon/libnetwork" "github.com/moby/sys/user" ) @@ -17,6 +18,7 @@ type executorOpts struct { // common fields root string networkController *libnetwork.Controller + ociHooks *ocihook.Server dnsConfig *oci.DNSConfig cdiManager *cdidevices.Manager proxyProvider network.ProxyProvider diff --git a/daemon/network.go b/daemon/network.go index 9a76ff6fb7..63d4716276 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -22,6 +22,7 @@ import ( "github.com/moby/moby/v2/daemon/internal/multierror" "github.com/moby/moby/v2/daemon/internal/netipstringer" "github.com/moby/moby/v2/daemon/internal/netiputil" + "github.com/moby/moby/v2/daemon/internal/ocihook" "github.com/moby/moby/v2/daemon/internal/otelutil" "github.com/moby/moby/v2/daemon/libnetwork" lncluster "github.com/moby/moby/v2/daemon/libnetwork/cluster" @@ -56,6 +57,12 @@ func (daemon *Daemon) NetworkController() *libnetwork.Controller { return daemon.netController } +// OCIHooks returns the server for OCI hooks the daemon installs in +// containers it does not start itself. It is nil on Windows. +func (daemon *Daemon) OCIHooks() *ocihook.Server { + return daemon.ociHooks +} + // FindNetwork returns a network based on: // 1. Full ID // 2. Full Name