[23.0 backport] Do not drop effective&permitted set

daemon/daemon_unix.go

@@ -109,7 +109,10 @@ func getMemoryResources(config containertypes.Resources) *specs.LinuxMemory {
 		memory.KernelTCP = &config.KernelMemoryTCP
 	}
 
-	return &memory
+	if memory != (specs.LinuxMemory{}) {
+		return &memory
+	}
+	return nil
 }
 
 func getPidsLimit(config containertypes.Resources) *specs.LinuxPids {
@@ -131,7 +134,7 @@ func getCPUResources(config containertypes.Resources) (*specs.LinuxCPU, error) {
 	if config.CPUShares < 0 {
 		return nil, fmt.Errorf("shares: invalid argument")
 	}
-	if config.CPUShares >= 0 {
+	if config.CPUShares > 0 {
 		shares := uint64(config.CPUShares)
 		cpu.Shares = &shares
 	}
@@ -172,7 +175,10 @@ func getCPUResources(config containertypes.Resources) (*specs.LinuxCPU, error) {
 		cpu.RealtimeRuntime = &c
 	}
 
-	return &cpu, nil
+	if cpu != (specs.LinuxCPU{}) {
+		return &cpu, nil
+	}
+	return nil, nil
 }
 
 func getBlkioWeightDevices(config containertypes.Resources) ([]specs.LinuxWeightDevice, error) {

daemon/oci_linux.go

@@ -54,6 +54,9 @@ func WithRlimits(daemon *Daemon, c *container.Container) coci.SpecOpts {
 			})
 		}
 
+		if s.Process == nil {
+			s.Process = &specs.Process{}
+		}
 		s.Process.Rlimits = rlimits
 		return nil
 	}
@@ -114,6 +117,9 @@ func WithRootless(daemon *Daemon) coci.SpecOpts {
 // WithOOMScore sets the oom score
 func WithOOMScore(score *int) coci.SpecOpts {
 	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
+		if s.Process == nil {
+			s.Process = &specs.Process{}
+		}
 		s.Process.OOMScoreAdj = score
 		return nil
 	}
@@ -122,6 +128,12 @@ func WithOOMScore(score *int) coci.SpecOpts {
 // WithSelinux sets the selinux labels
 func WithSelinux(c *container.Container) coci.SpecOpts {
 	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
+		if s.Process == nil {
+			s.Process = &specs.Process{}
+		}
+		if s.Linux == nil {
+			s.Linux = &specs.Linux{}
+		}
 		s.Process.SelinuxLabel = c.GetProcessLabel()
 		s.Linux.MountLabel = c.MountLabel
 		return nil
@@ -152,6 +164,9 @@ func WithApparmor(c *container.Container) coci.SpecOpts {
 					return err
 				}
 			}
+			if s.Process == nil {
+				s.Process = &specs.Process{}
+			}
 			s.Process.ApparmorProfile = appArmorProfile
 		}
 		return nil
@@ -214,6 +229,10 @@ func getUser(c *container.Container, username string) (specs.User, error) {
 }
 
 func setNamespace(s *specs.Spec, ns specs.LinuxNamespace) {
+	if s.Linux == nil {
+		s.Linux = &specs.Linux{}
+	}
+
 	for i, n := range s.Linux.Namespaces {
 		if n.Type == ns.Type {
 			s.Linux.Namespaces[i] = ns
@@ -608,6 +627,9 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
 				}
 				rootpg := mountPropagationMap[s.Linux.RootfsPropagation]
 				if rootpg != mount.SHARED && rootpg != mount.RSHARED {
+					if s.Linux == nil {
+						s.Linux = &specs.Linux{}
+					}
 					s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.SHARED]
 				}
 			case mount.SLAVE, mount.RSLAVE:
@@ -636,6 +658,9 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
 				if !fallback {
 					rootpg := mountPropagationMap[s.Linux.RootfsPropagation]
 					if rootpg != mount.SHARED && rootpg != mount.RSHARED && rootpg != mount.SLAVE && rootpg != mount.RSLAVE {
+						if s.Linux == nil {
+							s.Linux = &specs.Linux{}
+						}
 						s.Linux.RootfsPropagation = mountPropagationReverseMap[mount.RSLAVE]
 					}
 				}
@@ -691,8 +716,10 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
 					clearReadOnly(&s.Mounts[i])
 				}
 			}
-			s.Linux.ReadonlyPaths = nil
-			s.Linux.MaskedPaths = nil
+			if s.Linux != nil {
+				s.Linux.ReadonlyPaths = nil
+				s.Linux.MaskedPaths = nil
+			}
 		}
 
 		// TODO: until a kernel/mount solution exists for handling remount in a user namespace,
@@ -738,6 +765,9 @@ func WithCommonOptions(daemon *Daemon, c *container.Container) coci.SpecOpts {
 		if len(cwd) == 0 {
 			cwd = "/"
 		}
+		if s.Process == nil {
+			s.Process = &specs.Process{}
+		}
 		s.Process.Args = append([]string{c.Path}, c.Args...)
 
 		// only add the custom init if it is specified and the container is running in its
@@ -817,6 +847,9 @@ func WithCgroups(daemon *Daemon, c *container.Container) coci.SpecOpts {
 		} else {
 			cgroupsPath = filepath.Join(parent, c.ID)
 		}
+		if s.Linux == nil {
+			s.Linux = &specs.Linux{}
+		}
 		s.Linux.CgroupsPath = cgroupsPath
 
 		// the rest is only needed for CPU RT controller
@@ -917,8 +950,14 @@ func WithDevices(daemon *Daemon, c *container.Container) coci.SpecOpts {
 			}
 		}
 
+		if s.Linux == nil {
+			s.Linux = &specs.Linux{}
+		}
+		if s.Linux.Resources == nil {
+			s.Linux.Resources = &specs.LinuxResources{}
+		}
 		s.Linux.Devices = append(s.Linux.Devices, devs...)
-		s.Linux.Resources.Devices = devPermissions
+		s.Linux.Resources.Devices = append(s.Linux.Resources.Devices, devPermissions...)
 
 		for _, req := range c.HostConfig.DeviceRequests {
 			if err := daemon.handleDevice(req, s); err != nil {
@@ -959,34 +998,44 @@ func WithResources(c *container.Container) coci.SpecOpts {
 		if err != nil {
 			return err
 		}
-		blkioWeight := r.BlkioWeight
 
-		specResources := &specs.LinuxResources{
-			Memory: memoryRes,
-			CPU:    cpuRes,
-			BlockIO: &specs.LinuxBlockIO{
-				Weight:                  &blkioWeight,
-				WeightDevice:            weightDevices,
-				ThrottleReadBpsDevice:   readBpsDevice,
-				ThrottleWriteBpsDevice:  writeBpsDevice,
-				ThrottleReadIOPSDevice:  readIOpsDevice,
-				ThrottleWriteIOPSDevice: writeIOpsDevice,
-			},
-			Pids: getPidsLimit(r),
+		if s.Linux == nil {
+			s.Linux = &specs.Linux{}
 		}
-
-		if s.Linux.Resources != nil && len(s.Linux.Resources.Devices) > 0 {
-			specResources.Devices = s.Linux.Resources.Devices
+		if s.Linux.Resources == nil {
+			s.Linux.Resources = &specs.LinuxResources{}
+		}
+		s.Linux.Resources.Memory = memoryRes
+		s.Linux.Resources.CPU = cpuRes
+		s.Linux.Resources.BlockIO = &specs.LinuxBlockIO{
+			WeightDevice:            weightDevices,
+			ThrottleReadBpsDevice:   readBpsDevice,
+			ThrottleWriteBpsDevice:  writeBpsDevice,
+			ThrottleReadIOPSDevice:  readIOpsDevice,
+			ThrottleWriteIOPSDevice: writeIOpsDevice,
+		}
+		if r.BlkioWeight != 0 {
+			w := r.BlkioWeight
+			s.Linux.Resources.BlockIO.Weight = &w
 		}
+		s.Linux.Resources.Pids = getPidsLimit(r)
 
-		s.Linux.Resources = specResources
 		return nil
 	}
 }
 
 // WithSysctls sets the container's sysctls
 func WithSysctls(c *container.Container) coci.SpecOpts {
 	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
+		if len(c.HostConfig.Sysctls) == 0 {
+			return nil
+		}
+		if s.Linux == nil {
+			s.Linux = &specs.Linux{}
+		}
+		if s.Linux.Sysctl == nil {
+			s.Linux.Sysctl = make(map[string]string)
+		}
 		// We merge the sysctls injected above with the HostConfig (latter takes
 		// precedence for backwards-compatibility reasons).
 		for k, v := range c.HostConfig.Sysctls {
@@ -999,6 +1048,9 @@ func WithSysctls(c *container.Container) coci.SpecOpts {
 // WithUser sets the container's user
 func WithUser(c *container.Container) coci.SpecOpts {
 	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
+		if s.Process == nil {
+			s.Process = &specs.Process{}
+		}
 		var err error
 		s.Process.User, err = getUser(c, c.Config.User)
 		return err

daemon/oci_linux_test.go

@@ -11,17 +11,20 @@ import (
 	"github.com/docker/docker/daemon/network"
 	"github.com/docker/docker/libnetwork"
 	"github.com/docker/docker/pkg/containerfs"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"golang.org/x/sys/unix"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
 )
 
 func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon {
-	root, err := os.MkdirTemp("", "oci_linux_test-root")
-	assert.NilError(t, err)
+	t.Helper()
+	root := t.TempDir()
 
 	rootfs := filepath.Join(root, "rootfs")
-	err = os.MkdirAll(rootfs, 0755)
+	err := os.MkdirAll(rootfs, 0755)
 	assert.NilError(t, err)
 
 	netController, err := libnetwork.New()
@@ -48,11 +51,19 @@ func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon {
 		c.NetworkSettings = &network.Settings{Networks: make(map[string]*network.EndpointSettings)}
 	}
 
-	return d
-}
+	// HORRIBLE HACK: clean up shm mounts leaked by some tests. Otherwise the
+	// offending tests would fail due to the mounts blocking the temporary
+	// directory from being cleaned up.
+	t.Cleanup(func() {
+		if c.ShmPath != "" {
+			var err error
+			for err == nil { // Some tests over-mount over the same path multiple times.
+				err = unix.Unmount(c.ShmPath, unix.MNT_DETACH)
+			}
+		}
+	})
 
-func cleanupFakeContainer(c *container.Container) {
-	_ = os.RemoveAll(c.Root)
+	return d
 }
 
 // TestTmpfsDevShmNoDupMount checks that a user-specified /dev/shm tmpfs
@@ -72,7 +83,6 @@ func TestTmpfsDevShmNoDupMount(t *testing.T) {
 		},
 	}
 	d := setupFakeDaemon(t, c)
-	defer cleanupFakeContainer(c)
 
 	_, err := d.createSpec(c)
 	assert.Check(t, err)
@@ -91,7 +101,6 @@ func TestIpcPrivateVsReadonly(t *testing.T) {
 		},
 	}
 	d := setupFakeDaemon(t, c)
-	defer cleanupFakeContainer(c)
 
 	s, err := d.createSpec(c)
 	assert.Check(t, err)
@@ -120,7 +129,6 @@ func TestSysctlOverride(t *testing.T) {
 		},
 	}
 	d := setupFakeDaemon(t, c)
-	defer cleanupFakeContainer(c)
 
 	// Ensure that the implicit sysctl is set correctly.
 	s, err := d.createSpec(c)
@@ -172,7 +180,6 @@ func TestSysctlOverrideHost(t *testing.T) {
 		},
 	}
 	d := setupFakeDaemon(t, c)
-	defer cleanupFakeContainer(c)
 
 	// Ensure that the implicit sysctl is not set
 	s, err := d.createSpec(c)
@@ -200,3 +207,38 @@ func TestGetSourceMount(t *testing.T) {
 	_, _, err = getSourceMount(cwd)
 	assert.NilError(t, err)
 }
+
+func TestDefaultResources(t *testing.T) {
+	skip.If(t, os.Getuid() != 0, "skipping test that requires root") // TODO: is this actually true? I'm guilty of following the cargo cult here.
+
+	c := &container.Container{
+		HostConfig: &containertypes.HostConfig{
+			IpcMode: containertypes.IPCModeNone,
+		},
+	}
+	d := setupFakeDaemon(t, c)
+
+	s, err := d.createSpec(c)
+	assert.NilError(t, err)
+	checkResourcesAreUnset(t, s.Linux.Resources)
+}
+
+func checkResourcesAreUnset(t *testing.T, r *specs.LinuxResources) {
+	t.Helper()
+
+	if r != nil {
+		if r.Memory != nil {
+			assert.Check(t, is.DeepEqual(r.Memory, &specs.LinuxMemory{}))
+		}
+		if r.CPU != nil {
+			assert.Check(t, is.DeepEqual(r.CPU, &specs.LinuxCPU{}))
+		}
+		assert.Check(t, is.Nil(r.Pids))
+		if r.BlockIO != nil {
+			assert.Check(t, is.DeepEqual(r.BlockIO, &specs.LinuxBlockIO{}, cmpopts.EquateEmpty()))
+		}
+		if r.Network != nil {
+			assert.Check(t, is.DeepEqual(r.Network, &specs.LinuxNetwork{}, cmpopts.EquateEmpty()))
+		}
+	}
+}

daemon/oci_opts.go

@@ -13,6 +13,9 @@ import (
 func WithConsoleSize(c *container.Container) coci.SpecOpts {
 	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
 		if c.HostConfig.ConsoleSize[0] > 0 || c.HostConfig.ConsoleSize[1] > 0 {
+			if s.Process == nil {
+				s.Process = &specs.Process{}
+			}
 			s.Process.ConsoleSize = &specs.Box{
 				Height: c.HostConfig.ConsoleSize[0],
 				Width:  c.HostConfig.ConsoleSize[1],

daemon/oci_utils.go

@@ -9,7 +9,12 @@ func setLinuxDomainname(c *container.Container, s *specs.Spec) {
 	// There isn't a field in the OCI for the NIS domainname, but luckily there
 	// is a sysctl which has an identical effect to setdomainname(2) so there's
 	// no explicit need for runtime support.
-	s.Linux.Sysctl = make(map[string]string)
+	if s.Linux == nil {
+		s.Linux = &specs.Linux{}
+	}
+	if s.Linux.Sysctl == nil {
+		s.Linux.Sysctl = make(map[string]string)
+	}
 	if c.Config.Domainname != "" {
 		s.Linux.Sysctl["kernel.domainname"] = c.Config.Domainname
 	}

daemon/seccomp_linux.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/docker/container"
 	dconfig "github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/profiles/seccomp"
+	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 )
 
@@ -31,6 +32,9 @@ func WithSeccomp(daemon *Daemon, c *container.Container) coci.SpecOpts {
 			c.SeccompProfile = dconfig.SeccompProfileUnconfined
 			return nil
 		}
+		if s.Linux == nil {
+			s.Linux = &specs.Linux{}
+		}
 		var err error
 		switch {
 		case c.SeccompProfile == dconfig.SeccompProfileDefault: