Preserve mount options in container bind mounts

[#155869620]

Signed-off-by: Danail Branekov <[email protected]>

cmd/dadoo/dadoo_linux_test.go

@@ -15,6 +15,7 @@ import (
 
 	"code.cloudfoundry.org/commandrunner/linux_command_runner"
 	"code.cloudfoundry.org/garden"
+	"code.cloudfoundry.org/guardian/rundmc"
 	"code.cloudfoundry.org/guardian/rundmc/cgroups"
 	"code.cloudfoundry.org/guardian/rundmc/goci"
 	"code.cloudfoundry.org/lager/lagertest"
@@ -893,7 +894,7 @@ func setupCgroups(cgroupsRoot string) error {
 	logger := lagertest.NewTestLogger("test")
 	runner := linux_command_runner.New()
 
-	starter := cgroups.NewStarter(logger, mustOpen("/proc/cgroups"), mustOpen("/proc/self/cgroup"), cgroupsRoot, "garden", []specs.LinuxDeviceCgroup{}, runner, &cgroups.OSChowner{}, cgroups.IsMountPoint)
+	starter := cgroups.NewStarter(logger, mustOpen("/proc/cgroups"), mustOpen("/proc/self/cgroup"), cgroupsRoot, "garden", []specs.LinuxDeviceCgroup{}, runner, &cgroups.OSChowner{}, rundmc.IsMountPoint)
 
 	return starter.Start()
 }

gqt/bind_mount_test.go

@@ -24,6 +24,7 @@ var _ = Describe("Bind mount", func() {
 		dstPath             string                 // bm: destination
 		bindMountMode       garden.BindMountMode   // bm: RO or RW
 		bindMountOrigin     garden.BindMountOrigin // bm: Container or Host
+		mountOptions        []string
 
 		// pre-existing file for permissions testing
 		testFileName string
@@ -37,12 +38,12 @@ var _ = Describe("Bind mount", func() {
 		bindMountMode = garden.BindMountModeRO
 		bindMountOrigin = garden.BindMountOriginHost
 		testFileName = ""
-
-		srcPath, testFileName = createTestHostDirAndTestFile()
+		mountOptions = []string{"--bind"}
 		bindMountOrigin = garden.BindMountOriginHost
 	})
 
 	JustBeforeEach(func() {
+		srcPath, testFileName = createTestHostDirAndTestFile(mountOptions)
 		client = runner.Start(config)
 
 		var err error
@@ -203,6 +204,17 @@ var _ = Describe("Bind mount", func() {
 				})
 			})
 		})
+
+		Context("when mount directory is a mountpoint with an extra option", func() {
+			BeforeEach(func() {
+				mountOptions = []string{"-t", "tmpfs", "-o", "noexec"}
+			})
+
+			It("should suceed", func() {
+				readProcess := containerReadFile(container, dstPath, testFileName, "root")
+				Expect(readProcess.Wait()).To(Equal(0))
+			})
+		})
 	})
 
 	Context("which is read-write", func() {
@@ -340,7 +352,7 @@ var _ = Describe("Bind mount", func() {
 	})
 })
 
-func createTestHostDirAndTestFile() (string, string) {
+func createTestHostDirAndTestFile(mountOptions []string) (string, string) {
 	tstHostDir, err := ioutil.TempDir("", "bind-mount-test-dir")
 	Expect(err).ToNot(HaveOccurred())
 	err = os.Chown(tstHostDir, 0, 0)
@@ -349,7 +361,7 @@ func createTestHostDirAndTestFile() (string, string) {
 	Expect(err).ToNot(HaveOccurred())
 
 	var cmd *exec.Cmd
-	cmd = exec.Command("mount", "--bind", tstHostDir, tstHostDir)
+	cmd = exec.Command("mount", append(mountOptions, tstHostDir, tstHostDir)...)
 	Expect(cmd.Run()).To(Succeed())
 
 	debugOut, err := exec.Command("mount").CombinedOutput()

guardiancmd/command.go

@@ -818,7 +818,7 @@ func (cmd *ServerCommand) wireContainerizer(log lager.Logger, factory GardenFact
 		bundlerules.CGroupPath{
 			Path: cgroupRootPath,
 		},
-		bundlerules.Mounts{},
+		wireMounts(),
 		bundlerules.Env{},
 		bundlerules.Hostname{},
 		bundlerules.Windows{},

guardiancmd/command_linux.go

@@ -103,14 +103,14 @@ func (f *LinuxFactory) WireExecRunner(runMode string) runrunc.ExecRunner {
 }
 
 func (f *LinuxFactory) WireCgroupsStarter(logger lager.Logger) gardener.Starter {
-	return createCgroupsStarter(logger, f.config.Server.Tag, &cgroups.OSChowner{}, cgroups.IsMountPoint)
+	return createCgroupsStarter(logger, f.config.Server.Tag, &cgroups.OSChowner{}, rundmc.IsMountPoint)
 }
 
 func (cmd *SetupCommand) WireCgroupsStarter(logger lager.Logger) gardener.Starter {
-	return createCgroupsStarter(logger, cmd.Tag, &cgroups.OSChowner{UID: cmd.RootlessUID, GID: cmd.RootlessGID}, cgroups.IsMountPoint)
+	return createCgroupsStarter(logger, cmd.Tag, &cgroups.OSChowner{UID: cmd.RootlessUID, GID: cmd.RootlessGID}, rundmc.IsMountPoint)
 }
 
-func createCgroupsStarter(logger lager.Logger, tag string, chowner cgroups.Chowner, mountPointChecker cgroups.MountPointChecker) gardener.Starter {
+func createCgroupsStarter(logger lager.Logger, tag string, chowner cgroups.Chowner, mountPointChecker rundmc.MountPointChecker) gardener.Starter {
 	cgroupsMountpoint := "/sys/fs/cgroup"
 	gardenCgroup := "garden"
 	if tag != "" {
@@ -308,3 +308,7 @@ func (f *LinuxFactory) wireShed(logger lager.Logger) *rootfs_provider.CakeOrdina
 		rootfs_provider.NewMetricsAdapter(quotaManager.GetUsage, quotaedGraphDriver.GetMntPath),
 		ovenCleaner)
 }
+
+func wireMounts() bundlerules.Mounts {
+	return bundlerules.Mounts{MountPointChecker: rundmc.IsMountPoint, MountOptionsGetter: rundmc.GetMountOptions}
+}

guardiancmd/command_windows.go

@@ -10,6 +10,7 @@ import (
 	"code.cloudfoundry.org/guardian/gardener"
 	"code.cloudfoundry.org/guardian/kawasaki"
 	"code.cloudfoundry.org/guardian/rundmc"
+	"code.cloudfoundry.org/guardian/rundmc/bundlerules"
 	"code.cloudfoundry.org/guardian/rundmc/execrunner"
 	"code.cloudfoundry.org/guardian/rundmc/preparerootfs"
 	"code.cloudfoundry.org/guardian/rundmc/runrunc"
@@ -101,6 +102,16 @@ func wireEnvFunc() runrunc.EnvFunc {
 	return runrunc.EnvFunc(runrunc.WindowsEnvFor)
 }
 
+func wireMounts() bundlerules.Mounts {
+	noopMountpointChecker := func(path string) (bool, error) {
+		return false, nil
+	}
+	noopMountOptionsGetter := func(path string) ([]string, error) {
+		return []string{}, nil
+	}
+	return bundlerules.Mounts{MountPointChecker: noopMountpointChecker, MountOptionsGetter: noopMountOptionsGetter}
+}
+
 // Note - it's not possible to bind mount a single file in Windows, so we are
 // using a directory instead
 func initBindMountAndPath(initPathOnHost string) (specs.Mount, string) {

rundmc/bundlerules/mounts.go

@@ -3,31 +3,69 @@ package bundlerules
 import (
 	"code.cloudfoundry.org/garden"
 	spec "code.cloudfoundry.org/guardian/gardener/container-spec"
+	"code.cloudfoundry.org/guardian/rundmc"
 	"code.cloudfoundry.org/guardian/rundmc/goci"
 	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
 type Mounts struct {
+	MountPointChecker  rundmc.MountPointChecker
+	MountOptionsGetter rundmc.MountOptionsGetter
 }
 
 func (b Mounts) Apply(bndl goci.Bndl, spec spec.DesiredContainerSpec, _ string) (goci.Bndl, error) {
 	var mounts []specs.Mount
 	for _, m := range spec.BindMounts {
-		modeOpt := "ro"
-		if m.Mode == garden.BindMountModeRW {
-			modeOpt = "rw"
+		mountOptions, err := b.buildMountOptions(m)
+		if err != nil {
+			return goci.Bndl{}, err
 		}
 
 		mounts = append(mounts, specs.Mount{
 			Destination: m.DstPath,
 			Source:      m.SrcPath,
 			Type:        "bind",
-			Options:     []string{"bind", modeOpt},
+			Options:     mountOptions,
 		})
 	}
 
-	bndl = bndl.WithPrependedMounts(spec.BaseConfig.Mounts...)
-	bndl = bndl.WithMounts(mounts...)
+	return bndl.WithPrependedMounts(spec.BaseConfig.Mounts...).WithMounts(mounts...), nil
+}
+
+func (b Mounts) buildMountOptions(m garden.BindMount) ([]string, error) {
+	mountOptions := []string{"bind", getMountMode(m)}
+
+	isSrcMountpoint, err := b.MountPointChecker(m.SrcPath)
+	if err != nil {
+		return nil, err
+	}
+
+	if !isSrcMountpoint {
+		return mountOptions, nil
+	}
+
+	srcMountOptions, err := b.MountOptionsGetter(m.SrcPath)
+	if err != nil {
+		return nil, err
+	}
+	return append(mountOptions, filterModeOption(srcMountOptions)...), nil
+}
+
+func getMountMode(m garden.BindMount) string {
+	if m.Mode == garden.BindMountModeRW {
+		return "rw"
+	}
+
+	return "ro"
+}
+
+func filterModeOption(mountOptions []string) []string {
+	filteredOptions := []string{}
+	for i, o := range mountOptions {
+		if o == "rw" || o == "ro" || o == "bind" {
+			filteredOptions = append(mountOptions[0:i], mountOptions[i+1:]...)
+		}
+	}
 
-	return bndl, nil
+	return filteredOptions
 }

rundmc/bundlerules/mounts_test.go

@@ -1,6 +1,8 @@
 package bundlerules_test
 
 import (
+	"errors"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -9,13 +11,24 @@ import (
 	spec "code.cloudfoundry.org/guardian/gardener/container-spec"
 	"code.cloudfoundry.org/guardian/rundmc/bundlerules"
 	"code.cloudfoundry.org/guardian/rundmc/goci"
+	"code.cloudfoundry.org/guardian/rundmc/rundmcfakes"
 )
 
 var _ = Describe("MountsRule", func() {
-	var bndl goci.Bndl
+	var (
+		bndl               goci.Bndl
+		bundleApplyErr     error
+		originalBndl       goci.Bndl
+		mountpointChecker  *rundmcfakes.FakeMountPointChecker
+		mountOptionsGetter *rundmcfakes.FakeMountOptionsGetter
+
+		bindMounts             []garden.BindMount
+		desiredImageSpecMounts []specs.Mount
+	)
 
 	BeforeEach(func() {
-		var err error
+		mountpointChecker = new(rundmcfakes.FakeMountPointChecker)
+		mountOptionsGetter = new(rundmcfakes.FakeMountOptionsGetter)
 
 		preConfiguredMounts := []specs.Mount{
 			{
@@ -24,7 +37,7 @@ var _ = Describe("MountsRule", func() {
 				Type:        "preconfigured-mount",
 			},
 		}
-		bindMounts := []garden.BindMount{
+		bindMounts = []garden.BindMount{
 			{
 				SrcPath: "/path/to/ro/src",
 				DstPath: "/path/to/ro/dest",
@@ -36,7 +49,7 @@ var _ = Describe("MountsRule", func() {
 				Mode:    garden.BindMountModeRW,
 			},
 		}
-		desiredImageSpecMounts := []specs.Mount{
+		desiredImageSpecMounts = []specs.Mount{
 			{
 				Source:      "src",
 				Destination: "dest",
@@ -45,15 +58,80 @@ var _ = Describe("MountsRule", func() {
 			},
 		}
 
-		originalBndl := goci.Bundle().WithMounts(preConfiguredMounts...)
+		originalBndl = goci.Bundle().WithMounts(preConfiguredMounts...)
+	})
 
-		bndl, err = bundlerules.Mounts{}.Apply(
+	JustBeforeEach(func() {
+		rule := bundlerules.Mounts{
+			MountPointChecker:  mountpointChecker.Spy,
+			MountOptionsGetter: mountOptionsGetter.Spy,
+		}
+		bndl, bundleApplyErr = rule.Apply(
 			originalBndl,
 			spec.DesiredContainerSpec{
 				BindMounts: bindMounts,
 				BaseConfig: specs.Spec{Mounts: desiredImageSpecMounts},
 			}, "not-needed-path")
-		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("checks whether the source path is a mount", func() {
+		Expect(mountpointChecker.CallCount()).To(Equal(2))
+		Expect(mountpointChecker.ArgsForCall(0)).To(Equal("/path/to/ro/src"))
+		Expect(mountpointChecker.ArgsForCall(1)).To(Equal("/path/to/rw/src"))
+	})
+
+	Context("when checking whether source path is a mount point fails", func() {
+		BeforeEach(func() {
+			mountpointChecker.Returns(false, errors.New("check-failure"))
+		})
+
+		It("returns an error", func() {
+			Expect(bundleApplyErr).To(MatchError("check-failure"))
+			Expect(bndl).To(Equal(goci.Bndl{}))
+		})
+
+		It("does not check mount options for the source path", func() {
+			Expect(mountOptionsGetter.CallCount()).To(Equal(0))
+		})
+	})
+
+	Context("when the source is a mountpoint", func() {
+		BeforeEach(func() {
+			mountpointChecker.Returns(true, nil)
+			mountOptionsGetter.Returns([]string{"rw", "noexec"}, nil)
+		})
+
+		It("checks mount options for the source path", func() {
+			Expect(mountOptionsGetter.CallCount()).To(Equal(2))
+			Expect(mountOptionsGetter.ArgsForCall(0)).To(Equal("/path/to/ro/src"))
+			Expect(mountOptionsGetter.ArgsForCall(1)).To(Equal("/path/to/rw/src"))
+		})
+
+		Context("when checking mount options for the source path fails", func() {
+			BeforeEach(func() {
+				mountOptionsGetter.Returns(nil, errors.New("options-check-failure"))
+			})
+
+			It("returns an error", func() {
+				Expect(bundleApplyErr).To(MatchError("options-check-failure"))
+				Expect(bndl).To(Equal(goci.Bndl{}))
+			})
+		})
+
+		It("preserves source mount options on the bind mount", func() {
+			Expect(bndl.Mounts()[2]).To(Equal(
+				specs.Mount{
+					Source:      "/path/to/ro/src",
+					Destination: "/path/to/ro/dest",
+					Options:     []string{"bind", "ro", "noexec"},
+					Type:        "bind",
+				},
+			))
+		})
+	})
+
+	It("succeeds", func() {
+		Expect(bundleApplyErr).NotTo(HaveOccurred())
 	})
 
 	It("adds mounts to the bundle, ensuring desiredImageSpecMounts appear first", func() {

rundmc/cgroups/starter_linux.go

@@ -15,6 +15,7 @@ import (
 
 	"code.cloudfoundry.org/commandrunner"
 	"code.cloudfoundry.org/guardian/logging"
+	"code.cloudfoundry.org/guardian/rundmc"
 	"code.cloudfoundry.org/lager"
 )
 
@@ -37,7 +38,7 @@ func NewStarter(
 	allowedDevices []specs.LinuxDeviceCgroup,
 	runner commandrunner.CommandRunner,
 	chowner Chowner,
-	mountPointChecker MountPointChecker,
+	mountPointChecker rundmc.MountPointChecker,
 ) *CgroupStarter {
 	return &CgroupStarter{
 		CgroupPath:        cgroupMountpoint,
@@ -63,7 +64,7 @@ type CgroupStarter struct {
 
 	Logger            lager.Logger
 	Chowner           Chowner
-	MountPointChecker MountPointChecker
+	MountPointChecker rundmc.MountPointChecker
 }
 
 func (s *CgroupStarter) Start() error {