Move flag mapping to a separate function and unit test it

ii · Apr 8, 2024 · 0e13fa2 · 0e13fa2
1 parent 3e646cd
commit 0e13fa2
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 16 deletions.
diff --git a/staging/src/k8s.io/mount-utils/mount_linux.go b/staging/src/k8s.io/mount-utils/mount_linux.go
@@ -112,6 +112,31 @@ func (mounter *Mounter) hasSystemd() bool {
 	return *mounter.withSystemd
 }
 
+// Map unix.Statfs mount flags ro, nodev, noexec, nosuid, noatime, relatime,
+// nodiratime to mount option flag strings.
+func getUserNSBindMountOptions(path string, statfs func(path string, buf *unix.Statfs_t) (err error)) ([]string, error) {
+	var s unix.Statfs_t
+	var mountOpts []string
+	if err := statfs(path, &s); err != nil {
+		return nil, &os.PathError{Op: "statfs", Path: path, Err: err}
+	}
+	flagMapping := map[int]string{
+		unix.MS_RDONLY:     "ro",
+		unix.MS_NODEV:      "nodev",
+		unix.MS_NOEXEC:     "noexec",
+		unix.MS_NOSUID:     "nosuid",
+		unix.MS_NOATIME:    "noatime",
+		unix.MS_RELATIME:   "relatime",
+		unix.MS_NODIRATIME: "nodiratime",
+	}
+	for k, v := range flagMapping {
+		if int(s.Flags)&k == k {
+			mountOpts = append(mountOpts, v)
+		}
+	}
+	return mountOpts, nil
+}
+
 // Do a bind mount including the needed remount for applying the bind opts.
 // If the remount fails and we are running in a user namespace
 // figure out if the source filesystem has the ro, nodev, noexec, nosuid,
@@ -128,25 +153,12 @@ func (mounter *Mounter) bindMountSensitive(mounterPath string, mountCmd string,
 		}
 		// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
 		// nodiratime flag...
-		var s unix.Statfs_t
-		if err := unix.Statfs(source, &s); err != nil {
+		fixMountOpts, err := getUserNSBindMountOptions(source, unix.Statfs)
+		if err != nil {
 			return &os.PathError{Op: "statfs", Path: source, Err: err}
 		}
 		// ... and retry the mount with flags found above.
-		flagMapping := map[int]string{
-			unix.MS_RDONLY:     "ro",
-			unix.MS_NODEV:      "nodev",
-			unix.MS_NOEXEC:     "noexec",
-			unix.MS_NOSUID:     "nosuid",
-			unix.MS_NOATIME:    "noatime",
-			unix.MS_RELATIME:   "relatime",
-			unix.MS_NODIRATIME: "nodiratime",
-		}
-		for k, v := range flagMapping {
-			if int(s.Flags)&k == k {
-				bindRemountOpts = append(bindRemountOpts, v)
-			}
-		}
+		bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
 		return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
 	} else {
 		return err

diff --git a/staging/src/k8s.io/mount-utils/mount_linux_test.go b/staging/src/k8s.io/mount-utils/mount_linux_test.go
@@ -25,11 +25,13 @@ import (
 	"os"
 	"os/exec"
 	"reflect"
+	"sort"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
+	"golang.org/x/sys/unix"
 	"github.com/stretchr/testify/assert"
 	utilexec "k8s.io/utils/exec"
 	testexec "k8s.io/utils/exec/testing"
@@ -812,6 +814,43 @@ func TestFormatTimeout(t *testing.T) {
 	mu.Unlock()
 }
 
+func TestGetUserNSBindMountOptions(t *testing.T) {
+	var testCases = map[string]struct {
+		flags        int64
+		mountoptions string
+	}{
+		"ro":            {flags: unix.MS_RDONLY, mountoptions: "ro"},
+		"nodev":         {flags: unix.MS_NODEV, mountoptions: "nodev"},
+		"noexec":        {flags: unix.MS_NOEXEC, mountoptions: "noexec"},
+		"nosuid":        {flags: unix.MS_NOSUID, mountoptions: "nosuid"},
+		"noatime":       {flags: unix.MS_NOATIME, mountoptions: "noatime"},
+		"relatime":      {flags: unix.MS_RELATIME, mountoptions: "relatime"},
+		"nodiratime":    {flags: unix.MS_NODIRATIME, mountoptions: "nodiratime"},
+		"ronodev":       {flags: unix.MS_RDONLY | unix.MS_NODEV, mountoptions: "nodev,ro"},
+		"ronodevnoexec": {flags: unix.MS_RDONLY | unix.MS_NODEV | unix.MS_NOEXEC, mountoptions: "nodev,noexec,ro"},
+	}
+
+	statfsMock := func(path string, buf *unix.Statfs_t) (err error) {
+		*buf = unix.Statfs_t{Flags: testCases[path].flags}
+		return nil
+	}
+
+	testGetUserNSBindMountOptionsSingleCase := func(t *testing.T) {
+		path := strings.Split(t.Name(), "/")[1]
+		options, _ := getUserNSBindMountOptions(path, statfsMock)
+		sort.Strings(options)
+		optionString := strings.Join(options[:], ",")
+		mountOptions := testCases[path].mountoptions
+		if optionString != mountOptions {
+			t.Fatalf(`Mountoptions differ. Wanted: %s, returned: %s`, mountOptions, optionString)
+		}
+	}
+
+	for k, _ := range testCases {
+		t.Run(k, testGetUserNSBindMountOptionsSingleCase)
+	}
+}
+
 func makeFakeCommandAction(stdout string, err error, cmdFn func()) testexec.FakeCommandAction {
 	c := testexec.FakeCmd{
 		CombinedOutputScript: []testexec.FakeAction{