Merge pull request #127906 from yaroslavborbat/re-introduce-fix-mount-bind

Re-introduce 126599: fix: preserve options after remount for bind mounting
This commit is contained in:
Kubernetes Prow Robot 2024-12-12 02:57:06 +00:00 committed by GitHub
commit 2f7f755662
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 61 additions and 37 deletions

View File

@ -10,7 +10,6 @@ godebug winsymlink=0
require (
github.com/moby/sys/mountinfo v0.7.2
github.com/moby/sys/userns v0.1.0
github.com/stretchr/testify v1.9.0
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
golang.org/x/sys v0.26.0

View File

@ -13,8 +13,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/moby/sys/mountinfo v0.7.2 h1:1shs6aH5s4o5H2zQLn796ADW1wMrIwHsyJ2v9KouLrg=
github.com/moby/sys/mountinfo v0.7.2/go.mod h1:1YOa8w8Ih7uW0wALDUgT1dTTSBrZ+HiBLGws92L2RU4=
github.com/moby/sys/userns v0.1.0 h1:tVLXkFOxVu9A64/yh59slHVv9ahO9UIev4JZusOLG/g=
github.com/moby/sys/userns v0.1.0/go.mod h1:IHUYgu/kao6N8YZlp9Cf444ySSvCmDlmzUcYfDHOl28=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

View File

@ -35,7 +35,6 @@ import (
"github.com/moby/sys/mountinfo"
"golang.org/x/sys/unix"
inuserns "github.com/moby/sys/userns"
"k8s.io/klog/v2"
utilexec "k8s.io/utils/exec"
)
@ -114,7 +113,7 @@ func (mounter *Mounter) hasSystemd() bool {
// 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) {
func getBindMountOptions(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 {
@ -137,32 +136,23 @@ func getUserNSBindMountOptions(path string, statfs func(path string, buf *unix.S
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,
// noatime, relatime or nodiratime flag set and try another remount with the found flags.
// Performs a bind mount with the specified options, and then remounts
// the mount point with the same `nodev`, `nosuid`, `noexec`, `nosuid`, `noatime`,
// `relatime`, `nodiratime` options as the original mount point.
func (mounter *Mounter) bindMountSensitive(mounterPath string, mountCmd string, source string, target string, fstype string, bindOpts []string, bindRemountOpts []string, bindRemountOptsSensitive []string, mountFlags []string, systemdMountRequired bool) error {
err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
err := mounter.doMount(mounterPath, mountCmd, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
if err != nil {
return err
}
err = mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
if inuserns.RunningInUserNS() {
if err == nil {
return nil
}
// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
// nodiratime flag...
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.
bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
} else {
return err
// Check if the source has ro, nodev, noexec, nosuid, noatime, relatime,
// nodiratime flag...
fixMountOpts, err := getBindMountOptions(source, unix.Statfs)
if err != nil {
return &os.PathError{Op: "statfs", Path: source, Err: err}
}
// ... and retry the mount with flags found above.
bindRemountOpts = append(bindRemountOpts, fixMountOpts...)
return mounter.doMount(mounterPath, mountCmd, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, systemdMountRequired)
}
// Mount mounts source to target as fstype with given options. 'source' and 'fstype' must

View File

@ -823,7 +823,7 @@ func mkStatfsFlags[T1 constraints.Integer, T2 constraints.Integer](orig T1, add
return orig | T1(add)
}
func TestGetUserNSBindMountOptions(t *testing.T) {
func TestGetBindMountOptions(t *testing.T) {
var testCases = map[string]struct {
flags int32 // smallest size used by any platform we care about
mountoptions string
@ -845,9 +845,9 @@ func TestGetUserNSBindMountOptions(t *testing.T) {
return nil
}
testGetUserNSBindMountOptionsSingleCase := func(t *testing.T) {
testGetBindMountOptionsSingleCase := func(t *testing.T) {
path := strings.Split(t.Name(), "/")[1]
options, _ := getUserNSBindMountOptions(path, statfsMock)
options, _ := getBindMountOptions(path, statfsMock)
sort.Strings(options)
optionString := strings.Join(options, ",")
mountOptions := testCases[path].mountoptions
@ -857,7 +857,7 @@ func TestGetUserNSBindMountOptions(t *testing.T) {
}
for k := range testCases {
t.Run(k, testGetUserNSBindMountOptionsSingleCase)
t.Run(k, testGetBindMountOptionsSingleCase)
}
}

View File

@ -43,6 +43,7 @@ import (
"time"
"github.com/onsi/ginkgo/v2"
v1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
storagev1 "k8s.io/api/storage/v1"
@ -1138,19 +1139,54 @@ type localVolume struct {
var (
// capabilities
defaultLocalVolumeCapabilities = map[storageframework.Capability]bool{
storageframework.CapPersistence: true,
storageframework.CapFsGroup: true,
storageframework.CapBlock: false,
storageframework.CapExec: true,
storageframework.CapPersistence: true,
storageframework.CapFsGroup: true,
storageframework.CapBlock: false,
// To test CapExec, we need a volume with a filesystem.
// During end-to-end (e2e) testing, we utilize the `/tmp` directory for volume creation.
// However, best practices recommend mounting `/tmp` with the `noexec`, `nodev`, and `nosuid` parameters.
// This security measure prevents the execution of scripts and binaries within the `/tmp` directory.
// This practice, while promoting security, creates a dependency on the infrastructure configuration during e2e tests.
// This can result in "Permission Denied" errors when attempting to execute files from `/tmp`.
// To address this, we intentionally skip exec tests for certain types of LocalVolumes, such as `dir` or `dir-link`.
// This allows us to conduct comprehensive testing without relying on potentially restrictive security configurations.
storageframework.CapExec: false,
storageframework.CapMultiPODs: true,
storageframework.CapSingleNodeVolume: true,
storageframework.CapMultiplePVsSameID: true,
}
localVolumeCapabitilies = map[utils.LocalVolumeType]map[storageframework.Capability]bool{
utils.LocalVolumeTmpfs: {
storageframework.CapPersistence: true,
storageframework.CapFsGroup: true,
storageframework.CapBlock: false,
storageframework.CapExec: true,
storageframework.CapMultiPODs: true,
storageframework.CapSingleNodeVolume: true,
storageframework.CapMultiplePVsSameID: true,
},
utils.LocalVolumeBlock: {
storageframework.CapPersistence: true,
storageframework.CapFsGroup: true,
storageframework.CapBlock: true,
storageframework.CapExec: false,
storageframework.CapMultiPODs: true,
storageframework.CapSingleNodeVolume: true,
storageframework.CapMultiplePVsSameID: true,
},
utils.LocalVolumeBlockFS: {
storageframework.CapPersistence: true,
storageframework.CapFsGroup: true,
storageframework.CapBlock: false,
storageframework.CapExec: true,
storageframework.CapMultiPODs: true,
storageframework.CapSingleNodeVolume: true,
storageframework.CapMultiplePVsSameID: true,
},
utils.LocalVolumeGCELocalSSD: {
storageframework.CapPersistence: true,
storageframework.CapFsGroup: true,
storageframework.CapBlock: false,
storageframework.CapExec: true,
storageframework.CapMultiPODs: true,
storageframework.CapSingleNodeVolume: true,

View File

@ -27,6 +27,7 @@ import (
"strings"
"github.com/onsi/ginkgo/v2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/kubernetes/test/e2e/framework"
@ -215,7 +216,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectory(ctx context.Context, ltr *LocalTest
func (l *ltrMgr) setupLocalVolumeDirectoryLink(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
hostDir := l.getTestDir()
hostDirBackend := hostDir + "-backend"
cmd := fmt.Sprintf("mkdir %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDir)
cmd := fmt.Sprintf("mkdir -p %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDir)
err := l.hostExec.IssueCommand(ctx, cmd, node)
framework.ExpectNoError(err)
return &LocalTestResource{
@ -235,7 +236,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectoryLink(ctx context.Context, ltr *Local
func (l *ltrMgr) setupLocalVolumeDirectoryBindMounted(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
hostDir := l.getTestDir()
cmd := fmt.Sprintf("mkdir %s && mount --bind %s %s", hostDir, hostDir, hostDir)
cmd := fmt.Sprintf("mkdir -p %s && mount --bind %s %s", hostDir, hostDir, hostDir)
err := l.hostExec.IssueCommand(ctx, cmd, node)
framework.ExpectNoError(err)
return &LocalTestResource{
@ -255,7 +256,7 @@ func (l *ltrMgr) cleanupLocalVolumeDirectoryBindMounted(ctx context.Context, ltr
func (l *ltrMgr) setupLocalVolumeDirectoryLinkBindMounted(ctx context.Context, node *v1.Node, parameters map[string]string) *LocalTestResource {
hostDir := l.getTestDir()
hostDirBackend := hostDir + "-backend"
cmd := fmt.Sprintf("mkdir %s && mount --bind %s %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDirBackend, hostDirBackend, hostDir)
cmd := fmt.Sprintf("mkdir -p %s && mount --bind %s %s && ln -s %s %s", hostDirBackend, hostDirBackend, hostDirBackend, hostDirBackend, hostDir)
err := l.hostExec.IssueCommand(ctx, cmd, node)
framework.ExpectNoError(err)
return &LocalTestResource{