Merge pull request #37302 from cyphar/nis-domainname

oci: include the domainname in "kernel.domainname"

daemon/oci_linux.go

@@ -678,7 +678,15 @@ func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container)
 	s.Process.Cwd = cwd
 	s.Process.Env = c.CreateDaemonEnvironment(c.Config.Tty, linkedEnv)
 	s.Process.Terminal = c.Config.Tty
-	s.Hostname = c.FullHostname()
+
+	s.Hostname = c.Config.Hostname
+	// 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 c.Config.Domainname != "" {
+		s.Linux.Sysctl["kernel.domainname"] = c.Config.Domainname
+	}
 
 	return nil
 }
@@ -714,7 +722,11 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e
 	if err := setResources(&s, c.HostConfig.Resources); err != nil {
 		return nil, fmt.Errorf("linux runtime spec resources: %v", err)
 	}
-	s.Linux.Sysctl = c.HostConfig.Sysctls
+	// We merge the sysctls injected above with the HostConfig (latter takes
+	// precedence for backwards-compatibility reasons).
+	for k, v := range c.HostConfig.Sysctls {
+		s.Linux.Sysctl[k] = v
+	}
 
 	p := s.Linux.CgroupsPath
 	if useSystemd {

daemon/oci_linux_test.go

@@ -1,29 +1,67 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/config"
-	"github.com/docker/docker/oci"
+	"github.com/docker/docker/daemon/network"
+	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/docker/pkg/idtools"
+	"github.com/docker/libnetwork"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 )
 
+func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon {
+	root, err := ioutil.TempDir("", "oci_linux_test-root")
+	assert.NilError(t, err)
+
+	rootfs := filepath.Join(root, "rootfs")
+	err = os.MkdirAll(rootfs, 0755)
+	assert.NilError(t, err)
+
+	netController, err := libnetwork.New()
+	assert.NilError(t, err)
+
+	d := &Daemon{
+		// some empty structs to avoid getting a panic
+		// caused by a null pointer dereference
+		idMapping:     &idtools.IdentityMapping{},
+		configStore:   &config.Config{},
+		linkIndex:     newLinkIndex(),
+		netController: netController,
+	}
+
+	c.Root = root
+	c.BaseFS = containerfs.NewLocalContainerFS(rootfs)
+
+	if c.Config == nil {
+		c.Config = new(containertypes.Config)
+	}
+	if c.HostConfig == nil {
+		c.HostConfig = new(containertypes.HostConfig)
+	}
+	if c.NetworkSettings == nil {
+		c.NetworkSettings = &network.Settings{Networks: make(map[string]*network.EndpointSettings)}
+	}
+
+	return d
+}
+
+func cleanupFakeContainer(c *container.Container) {
+	os.RemoveAll(c.Root)
+}
+
 // TestTmpfsDevShmNoDupMount checks that a user-specified /dev/shm tmpfs
 // mount (as in "docker run --tmpfs /dev/shm:rw,size=NNN") does not result
 // in "Duplicate mount point" error from the engine.
 // https://github.com/moby/moby/issues/35455
 func TestTmpfsDevShmNoDupMount(t *testing.T) {
-	d := Daemon{
-		// some empty structs to avoid getting a panic
-		// caused by a null pointer dereference
-		idMapping:   &idtools.IdentityMapping{},
-		configStore: &config.Config{},
-	}
 	c := &container.Container{
 		ShmPath: "foobar", // non-empty, for c.IpcMounts() to work
 		HostConfig: &containertypes.HostConfig{
@@ -34,19 +72,10 @@ func TestTmpfsDevShmNoDupMount(t *testing.T) {
 			},
 		},
 	}
+	d := setupFakeDaemon(t, c)
+	defer cleanupFakeContainer(c)
 
-	// Mimic the code flow of daemon.createSpec(), enough to reproduce the issue
-	ms, err := d.setupMounts(c)
-	assert.Check(t, err)
-
-	ms = append(ms, c.IpcMounts()...)
-
-	tmpfsMounts, err := c.TmpfsMounts()
-	assert.Check(t, err)
-	ms = append(ms, tmpfsMounts...)
-
-	s := oci.DefaultSpec()
-	err = setMounts(&d, &s, c, ms)
+	_, err := d.createSpec(c)
 	assert.Check(t, err)
 }
 
@@ -55,28 +84,16 @@ func TestTmpfsDevShmNoDupMount(t *testing.T) {
 // the resulting /dev/shm mount is NOT made read-only.
 // https://github.com/moby/moby/issues/36503
 func TestIpcPrivateVsReadonly(t *testing.T) {
-	d := Daemon{
-		// some empty structs to avoid getting a panic
-		// caused by a null pointer dereference
-		idMapping:   &idtools.IdentityMapping{},
-		configStore: &config.Config{},
-	}
 	c := &container.Container{
 		HostConfig: &containertypes.HostConfig{
 			IpcMode:        containertypes.IpcMode("private"),
 			ReadonlyRootfs: true,
 		},
 	}
+	d := setupFakeDaemon(t, c)
+	defer cleanupFakeContainer(c)
 
-	// We can't call createSpec() so mimick the minimal part
-	// of its code flow, just enough to reproduce the issue.
-	ms, err := d.setupMounts(c)
-	assert.Check(t, err)
-
-	s := oci.DefaultSpec()
-	s.Root.Readonly = c.HostConfig.ReadonlyRootfs
-
-	err = setMounts(&d, &s, c, ms)
+	s, err := d.createSpec(c)
 	assert.Check(t, err)
 
 	// Find the /dev/shm mount in ms, check it does not have ro
@@ -88,6 +105,37 @@ func TestIpcPrivateVsReadonly(t *testing.T) {
 	}
 }
 
+// TestSysctlOverride ensures that any implicit sysctls (such as
+// Config.Domainname) are overridden by an explicit sysctl in the HostConfig.
+func TestSysctlOverride(t *testing.T) {
+	c := &container.Container{
+		Config: &containertypes.Config{
+			Hostname:   "foobar",
+			Domainname: "baz.cyphar.com",
+		},
+		HostConfig: &containertypes.HostConfig{
+			Sysctls: map[string]string{},
+		},
+	}
+	d := setupFakeDaemon(t, c)
+	defer cleanupFakeContainer(c)
+
+	// Ensure that the implicit sysctl is set correctly.
+	s, err := d.createSpec(c)
+	assert.NilError(t, err)
+	assert.Equal(t, s.Hostname, "foobar")
+	assert.Equal(t, s.Linux.Sysctl["kernel.domainname"], c.Config.Domainname)
+
+	// Set an explicit sysctl.
+	c.HostConfig.Sysctls["kernel.domainname"] = "foobar.net"
+	assert.Assert(t, c.HostConfig.Sysctls["kernel.domainname"] != c.Config.Domainname)
+
+	s, err = d.createSpec(c)
+	assert.NilError(t, err)
+	assert.Equal(t, s.Hostname, "foobar")
+	assert.Equal(t, s.Linux.Sysctl["kernel.domainname"], c.HostConfig.Sysctls["kernel.domainname"])
+}
+
 func TestGetSourceMount(t *testing.T) {
 	// must be able to find source mount for /
 	mnt, _, err := getSourceMount("/")

integration/container/run_linux_test.go

@@ -49,3 +49,44 @@ func TestKernelTCPMemory(t *testing.T) {
 	assert.Equal(t, 0, res.ExitCode)
 	assert.Check(t, is.Equal(strconv.FormatInt(kernelMemoryTCP, 10), strings.TrimSpace(res.Stdout())))
 }
+
+func TestNISDomainname(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
+
+	defer setupTest(t)()
+	client := request.NewAPIClient(t)
+	ctx := context.Background()
+
+	const (
+		hostname   = "foobar"
+		domainname = "baz.cyphar.com"
+	)
+
+	cID := container.Run(t, ctx, client, func(c *container.TestContainerConfig) {
+		c.Config.Hostname = hostname
+		c.Config.Domainname = domainname
+	})
+
+	poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond))
+
+	inspect, err := client.ContainerInspect(ctx, cID)
+	assert.NilError(t, err)
+	assert.Check(t, is.Equal(hostname, inspect.Config.Hostname))
+	assert.Check(t, is.Equal(domainname, inspect.Config.Domainname))
+
+	// Check hostname.
+	res, err := container.Exec(ctx, client, cID,
+		[]string{"cat", "/proc/sys/kernel/hostname"})
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(res.Stderr(), 0))
+	assert.Equal(t, 0, res.ExitCode)
+	assert.Check(t, is.Equal(hostname, strings.TrimSpace(res.Stdout())))
+
+	// Check domainname.
+	res, err = container.Exec(ctx, client, cID,
+		[]string{"cat", "/proc/sys/kernel/domainname"})
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(res.Stderr(), 0))
+	assert.Equal(t, 0, res.ExitCode)
+	assert.Check(t, is.Equal(domainname, strings.TrimSpace(res.Stdout())))
+}