diff --git a/daemon/command/daemon_linux.go b/daemon/command/daemon_linux.go index ec74f151bf..e283c533cd 100644 --- a/daemon/command/daemon_linux.go +++ b/daemon/command/daemon_linux.go @@ -1,7 +1,10 @@ package command import ( + "context" + cdcgroups "github.com/containerd/cgroups/v3" + "github.com/containerd/log" systemdDaemon "github.com/coreos/go-systemd/v22/daemon" "github.com/moby/moby/v2/daemon" "github.com/moby/moby/v2/daemon/config" @@ -22,6 +25,20 @@ func setPlatformOptions(conf *config.Config) error { conf.ContainerdNamespace = containerdNamespace conf.ContainerdPluginNamespace = containerdPluginNamespace + // Buildkit breaks when userns remapping is enabled and containerd snapshotter is used. As a temporary workaround, + // if containerd snapshotter is explicitly enabled, and userns remapping is enabled too, return an error. If userns + // remapping is enabled, but containerd-snapshotter is enabled by default, disable it. See https://github.com/moby/moby/issues/47377. + enabled := conf.Features["containerd-snapshotter"] + if enabled { + return errors.New("containerd-snapshotter is explicitly enabled, but is not compatible with userns remapping. Please disable userns remapping or containerd-snapshotter") + } + + log.G(context.TODO()).Warn("userns remapping enabled, disabling containerd snapshotter") + if conf.Features == nil { + conf.Features = make(map[string]bool) + } + conf.Features["containerd-snapshotter"] = false + return nil } diff --git a/daemon/command/daemon_linux_test.go b/daemon/command/daemon_linux_test.go index 2ebeabe93a..d39daa126e 100644 --- a/daemon/command/daemon_linux_test.go +++ b/daemon/command/daemon_linux_test.go @@ -8,6 +8,7 @@ import ( "strconv" "testing" + "github.com/google/go-cmp/cmp" "github.com/moby/moby/v2/daemon/config" "github.com/moby/sys/reexec" "golang.org/x/sys/unix" @@ -86,3 +87,67 @@ func TestLoadListenerNoAddr(t *testing.T) { assert.NilError(t, json.NewDecoder(stdout).Decode(&resp)) assert.Equal(t, resp.Err, "") } + +func TestC8dSnapshotterWithUsernsRemap(t *testing.T) { + testcases := []struct { + name string + cfg *config.Config + expCfg *config.Config + expErr string + }{ + { + name: "no remap, no snapshotter", + cfg: &config.Config{}, + expCfg: &config.Config{}, + }, + { + name: "userns remap, no explicit containerd-snapshotter feature", + cfg: &config.Config{RemappedRoot: "default"}, + expCfg: &config.Config{ + RemappedRoot: "dockremap:dockremap", + CommonConfig: config.CommonConfig{ + ContainerdNamespace: "-100000.100000", + ContainerdPluginNamespace: "-100000.100000", + Features: map[string]bool{"containerd-snapshotter": false}, + }, + }, + }, + { + name: "userns remap, explicit containerd-snapshotter feature", + cfg: &config.Config{ + RemappedRoot: "default", + CommonConfig: config.CommonConfig{Features: map[string]bool{"containerd-snapshotter": true}}, + }, + expCfg: &config.Config{ + RemappedRoot: "dockremap:dockremap", + CommonConfig: config.CommonConfig{ + ContainerdNamespace: "-100000.100000", + ContainerdPluginNamespace: "-100000.100000", + Features: map[string]bool{"containerd-snapshotter": true}, + }, + }, + expErr: "containerd-snapshotter is explicitly enabled, but is not compatible with userns remapping. Please disable userns remapping or containerd-snapshotter", + }, + { + name: "no remap, explicit containerd-snapshotter feature", + cfg: &config.Config{ + CommonConfig: config.CommonConfig{Features: map[string]bool{"containerd-snapshotter": true}}, + }, + expCfg: &config.Config{ + CommonConfig: config.CommonConfig{Features: map[string]bool{"containerd-snapshotter": true}}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := setPlatformOptions(tc.cfg) + assert.DeepEqual(t, tc.expCfg, tc.cfg, cmp.AllowUnexported(config.DefaultBridgeConfig{})) + if tc.expErr != "" { + assert.Equal(t, tc.expErr, err.Error()) + } else { + assert.NilError(t, err) + } + }) + } +} diff --git a/integration-cli/docker_cli_userns_test.go b/integration-cli/docker_cli_userns_test.go index ccdcc0b52d..a6000f9f7c 100644 --- a/integration-cli/docker_cli_userns_test.go +++ b/integration-cli/docker_cli_userns_test.go @@ -26,7 +26,7 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *testing.T) { testRequires(c, UserNamespaceInKernel) ctx := testutil.GetContext(c) - s.d.StartWithBusybox(ctx, c, "--userns-remap", "default") + s.d.StartWithBusybox(ctx, c, "--userns-remap", "default", "--storage-driver", "vfs") out, err := s.d.Cmd("run", "busybox", "stat", "-c", "%u:%g", "/bin/cat") assert.Check(c, err) diff --git a/integration/build/build_userns_linux_test.go b/integration/build/build_userns_linux_test.go index 591fc006bf..18e0f177ca 100644 --- a/integration/build/build_userns_linux_test.go +++ b/integration/build/build_userns_linux_test.go @@ -38,8 +38,8 @@ func TestBuildUserNamespaceValidateCapabilitiesAreV2(t *testing.T) { tmpDir := t.TempDir() - dUserRemap := daemon.New(t) - dUserRemap.Start(t, "--userns-remap", "default") + dUserRemap := daemon.New(t, daemon.WithUserNsRemap("default")) + dUserRemap.Start(t) clientUserRemap := dUserRemap.NewClientT(t) defer clientUserRemap.Close() diff --git a/internal/testutil/daemon/ops.go b/internal/testutil/daemon/ops.go index 31a8cd347a..e7455a5fc8 100644 --- a/internal/testutil/daemon/ops.go +++ b/internal/testutil/daemon/ops.go @@ -2,6 +2,7 @@ package daemon import ( "net/netip" + "os" "os/user" "github.com/moby/moby/v2/internal/testutil/environment" @@ -23,6 +24,21 @@ func WithContainerdSocket(socket string) Option { func WithUserNsRemap(remap string) Option { return func(d *Daemon) { d.usernsRemap = remap + // The dind container used by the CI has the env var DOCKER_GRAPHDRIVER + // set to 'overlayfs' which is a valid driver when the containerd image + // store is used, but not a valid graphdriver. OTOH the test daemon + // started by this package uses DOCKER_GRAPHDRIVER to set the storage + // backend. However, the daemon doesn't enable the containerd image + // store automatically when userns remapping is enabled, so using the + // storage driver set through DOCKER_GRAPHDRIVER will cause the daemon + // to fail to start. This should be removed once a proper fix for [1] + // is implemented. + // + // [1]: https://github.com/moby/moby/issues/47377 + d.storageDriver = "" + if storage := os.Getenv("DOCKER_GRAPHDRIVER"); storage == "overlayfs" { + d.storageDriver = "overlay2" + } } }