From 97b5299cd737c624887a39400e8377c8355a2a85 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 23 May 2018 10:17:59 +0200 Subject: [PATCH 1/9] Add GetMode to mounter interface. Kubelet must not call os.Lstat on raw volume paths when it runs in a container. Mounter knows where the file really is. --- pkg/kubelet/cm/container_manager_linux_test.go | 4 ++++ pkg/kubelet/kubelet_pods.go | 11 +++++------ pkg/util/mount/exec_mount.go | 4 ++++ pkg/util/mount/exec_mount_test.go | 4 ++++ pkg/util/mount/exec_mount_unsupported.go | 4 ++++ pkg/util/mount/fake.go | 4 ++++ pkg/util/mount/mount.go | 2 ++ pkg/util/mount/mount_linux.go | 13 +++++++++++++ pkg/util/mount/mount_unsupported.go | 4 ++++ pkg/util/mount/mount_windows.go | 8 ++++++++ pkg/util/mount/nsenter_mount.go | 8 ++++++++ pkg/util/mount/nsenter_mount_unsupported.go | 4 ++++ pkg/util/removeall/removeall_test.go | 4 ++++ pkg/volume/host_path/host_path_test.go | 4 ++++ 14 files changed, 72 insertions(+), 6 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_linux_test.go b/pkg/kubelet/cm/container_manager_linux_test.go index cd4b0460c6f..be744604f39 100644 --- a/pkg/kubelet/cm/container_manager_linux_test.go +++ b/pkg/kubelet/cm/container_manager_linux_test.go @@ -120,6 +120,10 @@ func (mi *fakeMountInterface) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("not implemented") } +func (mi *fakeMountInterface) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} + func fakeContainerMgrMountInt() mount.Interface { return &fakeMountInterface{ []mount.MountPoint{ diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 46a17a01a58..a0bd2febf39 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -175,12 +175,6 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h return nil, cleanupAction, fmt.Errorf("unable to provision SubPath `%s`: %v", mount.SubPath, err) } - fileinfo, err := os.Lstat(hostPath) - if err != nil { - return nil, cleanupAction, err - } - perm := fileinfo.Mode() - volumePath, err := filepath.EvalSymlinks(hostPath) if err != nil { return nil, cleanupAction, err @@ -195,6 +189,11 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h // when the pod specifies an fsGroup, and if the directory is not created here, Docker will // later auto-create it with the incorrect mode 0750 // Make extra care not to escape the volume! + perm, err := mounter.GetMode(volumePath) + if err != nil { + return nil, cleanupAction, err + } + if err := mounter.SafeMakeDir(hostPath, volumePath, perm); err != nil { glog.Errorf("failed to mkdir %q: %v", hostPath, err) return nil, cleanupAction, err diff --git a/pkg/util/mount/exec_mount.go b/pkg/util/mount/exec_mount.go index fcb948aa34e..1dab57880b0 100644 --- a/pkg/util/mount/exec_mount.go +++ b/pkg/util/mount/exec_mount.go @@ -163,3 +163,7 @@ func (m *execMounter) GetFSGroup(pathname string) (int64, error) { func (m *execMounter) GetSELinuxSupport(pathname string) (bool, error) { return m.wrappedMounter.GetSELinuxSupport(pathname) } + +func (m *execMounter) GetMode(pathname string) (os.FileMode, error) { + return m.wrappedMounter.GetMode(pathname) +} diff --git a/pkg/util/mount/exec_mount_test.go b/pkg/util/mount/exec_mount_test.go index b3af0a46fbb..c48133ba2a2 100644 --- a/pkg/util/mount/exec_mount_test.go +++ b/pkg/util/mount/exec_mount_test.go @@ -176,3 +176,7 @@ func (fm *fakeMounter) GetFSGroup(pathname string) (int64, error) { func (fm *fakeMounter) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("not implemented") } + +func (fm *fakeMounter) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} diff --git a/pkg/util/mount/exec_mount_unsupported.go b/pkg/util/mount/exec_mount_unsupported.go index cbb5bbc1591..f7cf757f26e 100644 --- a/pkg/util/mount/exec_mount_unsupported.go +++ b/pkg/util/mount/exec_mount_unsupported.go @@ -110,3 +110,7 @@ func (mounter *execMounter) GetFSGroup(pathname string) (int64, error) { func (mounter *execMounter) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("not implemented") } + +func (mounter *execMounter) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} diff --git a/pkg/util/mount/fake.go b/pkg/util/mount/fake.go index f82f669b2eb..be4a033649a 100644 --- a/pkg/util/mount/fake.go +++ b/pkg/util/mount/fake.go @@ -232,3 +232,7 @@ func (f *FakeMounter) GetFSGroup(pathname string) (int64, error) { func (f *FakeMounter) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("GetSELinuxSupport not implemented") } + +func (f *FakeMounter) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index 0c59ca9ebb7..d1edbfa266a 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -117,6 +117,8 @@ type Interface interface { // GetSELinuxSupport returns true if given path is on a mount that supports // SELinux. GetSELinuxSupport(pathname string) (bool, error) + // GetMode returns permissions of the path. + GetMode(pathname string) (os.FileMode, error) } type Subpath struct { diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index dbb864cd479..466fb9f8e03 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -982,6 +982,10 @@ func (mounter *Mounter) GetFSGroup(pathname string) (int64, error) { return getFSGroup(realpath) } +func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) { + return getMode(pathname) +} + // This implementation is shared between Linux and NsEnterMounter func getFSGroup(pathname string) (int64, error) { info, err := os.Stat(pathname) @@ -991,6 +995,15 @@ func getFSGroup(pathname string) (int64, error) { return int64(info.Sys().(*syscall.Stat_t).Gid), nil } +// This implementation is shared between Linux and NsEnterMounter +func getMode(pathname string) (os.FileMode, error) { + info, err := os.Stat(pathname) + if err != nil { + return 0, err + } + return info.Mode(), nil +} + // This implementation is shared between Linux and NsEnterMounter func doSafeMakeDir(pathname string, base string, perm os.FileMode) error { glog.V(4).Infof("Creating directory %q within base %q", pathname, base) diff --git a/pkg/util/mount/mount_unsupported.go b/pkg/util/mount/mount_unsupported.go index 6e268e0f43d..2b7a329340b 100644 --- a/pkg/util/mount/mount_unsupported.go +++ b/pkg/util/mount/mount_unsupported.go @@ -138,3 +138,7 @@ func (mounter *Mounter) GetFSGroup(pathname string) (int64, error) { func (mounter *Mounter) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("not implemented") } + +func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index f31f99bd66d..2c87755c9f2 100644 --- a/pkg/util/mount/mount_windows.go +++ b/pkg/util/mount/mount_windows.go @@ -461,6 +461,14 @@ func (mounter *Mounter) GetSELinuxSupport(pathname string) (bool, error) { return false, nil } +func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) { + info, err := os.Stat(pathname) + if err != nil { + return 0, err + } + return info.Mode(), nil +} + // SafeMakeDir makes sure that the created directory does not escape given base directory mis-using symlinks. func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { return doSafeMakeDir(pathname, base, perm) diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index 4c48d673254..1936d521e78 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -347,3 +347,11 @@ func (mounter *NsenterMounter) GetFSGroup(pathname string) (int64, error) { func (mounter *NsenterMounter) GetSELinuxSupport(pathname string) (bool, error) { return getSELinuxSupport(pathname, procMountInfoPath) } + +func (mounter *NsenterMounter) GetMode(pathname string) (os.FileMode, error) { + kubeletpath, err := mounter.ne.KubeletPath(pathname) + if err != nil { + return 0, err + } + return getMode(kubeletpath) +} diff --git a/pkg/util/mount/nsenter_mount_unsupported.go b/pkg/util/mount/nsenter_mount_unsupported.go index 8cf79de472c..007a456d490 100644 --- a/pkg/util/mount/nsenter_mount_unsupported.go +++ b/pkg/util/mount/nsenter_mount_unsupported.go @@ -110,3 +110,7 @@ func (*NsenterMounter) GetFSGroup(pathname string) (int64, error) { func (*NsenterMounter) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("not implemented") } + +func (*NsenterMounter) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} diff --git a/pkg/util/removeall/removeall_test.go b/pkg/util/removeall/removeall_test.go index 134326369ff..878db734cdf 100644 --- a/pkg/util/removeall/removeall_test.go +++ b/pkg/util/removeall/removeall_test.go @@ -103,6 +103,10 @@ func (mounter *fakeMounter) GetSELinuxSupport(pathname string) (bool, error) { return false, errors.New("not implemented") } +func (mounter *fakeMounter) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} + func (mounter *fakeMounter) IsLikelyNotMountPoint(file string) (bool, error) { name := path.Base(file) if strings.HasPrefix(name, "mount") { diff --git a/pkg/volume/host_path/host_path_test.go b/pkg/volume/host_path/host_path_test.go index 199880c247e..b8ae4fa9fc0 100644 --- a/pkg/volume/host_path/host_path_test.go +++ b/pkg/volume/host_path/host_path_test.go @@ -401,6 +401,10 @@ func (fftc *fakeFileTypeChecker) GetSELinuxSupport(pathname string) (bool, error return false, errors.New("not implemented") } +func (fftc *fakeFileTypeChecker) GetMode(pathname string) (os.FileMode, error) { + return 0, errors.New("not implemented") +} + func setUp() error { err := os.MkdirAll("/tmp/ExistingFolder", os.FileMode(0755)) if err != nil { From 7450d1b4277998eea436e5daa552a8bf94adef06 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:25 +0200 Subject: [PATCH 2/9] Allow EvalSymlinks target not to exist. Various NsEnterMounter function need to resolve the part of the path that exists and blindly add the part that doesn't. --- pkg/util/mount/nsenter_mount.go | 10 ++++++---- pkg/util/nsenter/nsenter.go | 30 +++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index 1936d521e78..9531edddb25 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -329,7 +329,7 @@ func (mounter *NsenterMounter) SafeMakeDir(pathname string, base string, perm os } func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) { - hostpath, err := mounter.ne.EvalSymlinks(pathname) + hostpath, err := mounter.ne.EvalSymlinks(pathname, true /* mustExist */) if err != nil { return nil, err } @@ -337,10 +337,11 @@ func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) { } func (mounter *NsenterMounter) GetFSGroup(pathname string) (int64, error) { - kubeletpath, err := mounter.ne.KubeletPath(pathname) + hostPath, err := mounter.ne.EvalSymlinks(pathname, true /* mustExist */) if err != nil { - return 0, err + return -1, err } + kubeletpath := mounter.ne.KubeletPath(hostPath) return getFSGroup(kubeletpath) } @@ -349,9 +350,10 @@ func (mounter *NsenterMounter) GetSELinuxSupport(pathname string) (bool, error) } func (mounter *NsenterMounter) GetMode(pathname string) (os.FileMode, error) { - kubeletpath, err := mounter.ne.KubeletPath(pathname) + hostPath, err := mounter.ne.EvalSymlinks(pathname, true /* mustExist */) if err != nil { return 0, err } + kubeletpath := mounter.ne.KubeletPath(hostPath) return getMode(kubeletpath) } diff --git a/pkg/util/nsenter/nsenter.go b/pkg/util/nsenter/nsenter.go index 11e9fe4a6dc..477950476b1 100644 --- a/pkg/util/nsenter/nsenter.go +++ b/pkg/util/nsenter/nsenter.go @@ -128,8 +128,23 @@ func (ne *Nsenter) SupportsSystemd() (string, bool) { // EvalSymlinks returns the path name on the host after evaluating symlinks on the // host. -func (ne *Nsenter) EvalSymlinks(pathname string) (string, error) { - args := []string{"-m", pathname} +// mustExist makes EvalSymlinks to return error when the path does not +// exist. When it's false, it evaluates symlinks of the existing part and +// blindly adds the non-existing part: +// pathname: /mnt/volume/non/existing/directory +// /mnt/volume exists +// non/existing/directory does not exist +// -> It resolves symlinks in /mnt/volume to say /mnt/foo and returns +// /mnt/foo/non/existing/directory. +func (ne *Nsenter) EvalSymlinks(pathname string, mustExist bool) (string, error) { + var args []string + if mustExist { + // "realpath -e: all components of the path must exist" + args = []string{"-e", pathname} + } else { + // "realpath -m: no path components need exist or be a directory" + args = []string{"-m", pathname} + } outBytes, err := ne.Exec("realpath", args).CombinedOutput() if err != nil { glog.Infof("failed to resolve symbolic links on %s: %v", pathname, err) @@ -139,11 +154,8 @@ func (ne *Nsenter) EvalSymlinks(pathname string) (string, error) { } // KubeletPath returns the path name that can be accessed by containerized -// kubelet, after evaluating symlinks on the host. -func (ne *Nsenter) KubeletPath(pathname string) (string, error) { - hostpath, err := ne.EvalSymlinks(pathname) - if err != nil { - return "", err - } - return filepath.Join(hostRootFsPath, hostpath), nil +// kubelet. It is recommended to resolve symlinks on the host by EvalSymlinks +// before calling this function +func (ne *Nsenter) KubeletPath(pathname string) string { + return filepath.Join(hostRootFsPath, pathname) } From 74ba0878a1240729cf49e0ec52f122cdbb1fee4d Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:25 +0200 Subject: [PATCH 3/9] Enhance ExistsPath check It should return error when the check fails (e.g. no permissions, symlink link loop etc.) --- pkg/kubelet/cm/container_manager_linux_test.go | 4 ++-- pkg/kubelet/kubelet_pods.go | 3 +-- pkg/util/mount/BUILD | 4 ++++ pkg/util/mount/exec_mount.go | 2 +- pkg/util/mount/exec_mount_test.go | 4 ++-- pkg/util/mount/exec_mount_unsupported.go | 4 ++-- pkg/util/mount/fake.go | 4 ++-- pkg/util/mount/mount.go | 6 +++--- pkg/util/mount/mount_linux.go | 9 +++------ pkg/util/mount/mount_unsupported.go | 7 ++----- pkg/util/mount/mount_windows.go | 18 ++++++++++-------- pkg/util/mount/nsenter_mount.go | 15 +++++++++------ pkg/util/mount/nsenter_mount_unsupported.go | 4 ++-- pkg/util/removeall/removeall_test.go | 4 ++-- pkg/volume/host_path/host_path.go | 3 ++- pkg/volume/host_path/host_path_test.go | 4 ++-- 16 files changed, 49 insertions(+), 46 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_linux_test.go b/pkg/kubelet/cm/container_manager_linux_test.go index be744604f39..127140e751a 100644 --- a/pkg/kubelet/cm/container_manager_linux_test.go +++ b/pkg/kubelet/cm/container_manager_linux_test.go @@ -92,8 +92,8 @@ func (mi *fakeMountInterface) MakeFile(pathname string) error { return nil } -func (mi *fakeMountInterface) ExistsPath(pathname string) bool { - return true +func (mi *fakeMountInterface) ExistsPath(pathname string) (bool, error) { + return true, errors.New("not implemented") } func (mi *fakeMountInterface) PrepareSafeSubpath(subPath mount.Subpath) (newHostPath string, cleanupAction func(), err error) { diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index a0bd2febf39..c1d2c54f090 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -60,7 +60,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/status" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/kubelet/util/format" - utilfile "k8s.io/kubernetes/pkg/util/file" mountutil "k8s.io/kubernetes/pkg/util/mount" volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/volumepathhandler" @@ -181,7 +180,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h } hostPath = filepath.Join(volumePath, mount.SubPath) - if subPathExists, err := utilfile.FileOrSymlinkExists(hostPath); err != nil { + if subPathExists, err := mounter.ExistsPath(hostPath); err != nil { glog.Errorf("Could not determine if subPath %s exists; will not attempt to change its permissions", hostPath) } else if !subPathExists { // Create the sub path now because if it's auto-created later when referenced, it may have an diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD index 9040cd93b26..6d2e65cbc96 100644 --- a/pkg/util/mount/BUILD +++ b/pkg/util/mount/BUILD @@ -72,11 +72,15 @@ go_library( "//vendor/k8s.io/utils/exec:go_default_library", ] + select({ "@io_bazel_rules_go//go/platform:linux": [ + "//pkg/util/file:go_default_library", "//pkg/util/io:go_default_library", "//pkg/util/nsenter:go_default_library", "//vendor/golang.org/x/sys/unix:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], + "@io_bazel_rules_go//go/platform:windows": [ + "//pkg/util/file:go_default_library", + ], "//conditions:default": [], }), ) diff --git a/pkg/util/mount/exec_mount.go b/pkg/util/mount/exec_mount.go index 1dab57880b0..fe7dcbd7ef9 100644 --- a/pkg/util/mount/exec_mount.go +++ b/pkg/util/mount/exec_mount.go @@ -136,7 +136,7 @@ func (m *execMounter) MakeDir(pathname string) error { return m.wrappedMounter.MakeDir(pathname) } -func (m *execMounter) ExistsPath(pathname string) bool { +func (m *execMounter) ExistsPath(pathname string) (bool, error) { return m.wrappedMounter.ExistsPath(pathname) } diff --git a/pkg/util/mount/exec_mount_test.go b/pkg/util/mount/exec_mount_test.go index c48133ba2a2..9619356f71b 100644 --- a/pkg/util/mount/exec_mount_test.go +++ b/pkg/util/mount/exec_mount_test.go @@ -147,8 +147,8 @@ func (fm *fakeMounter) MakeFile(pathname string) error { func (fm *fakeMounter) MakeDir(pathname string) error { return nil } -func (fm *fakeMounter) ExistsPath(pathname string) bool { - return false +func (fm *fakeMounter) ExistsPath(pathname string) (bool, error) { + return false, errors.New("not implemented") } func (fm *fakeMounter) GetFileType(pathname string) (FileType, error) { return FileTypeFile, nil diff --git a/pkg/util/mount/exec_mount_unsupported.go b/pkg/util/mount/exec_mount_unsupported.go index f7cf757f26e..6854b32b26c 100644 --- a/pkg/util/mount/exec_mount_unsupported.go +++ b/pkg/util/mount/exec_mount_unsupported.go @@ -83,8 +83,8 @@ func (mounter *execMounter) MakeFile(pathname string) error { return nil } -func (mounter *execMounter) ExistsPath(pathname string) bool { - return true +func (mounter *execMounter) ExistsPath(pathname string) (bool, error) { + return true, errors.New("not implemented") } func (mounter *execMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) { diff --git a/pkg/util/mount/fake.go b/pkg/util/mount/fake.go index be4a033649a..10832fd321b 100644 --- a/pkg/util/mount/fake.go +++ b/pkg/util/mount/fake.go @@ -201,8 +201,8 @@ func (f *FakeMounter) MakeFile(pathname string) error { return nil } -func (f *FakeMounter) ExistsPath(pathname string) bool { - return false +func (f *FakeMounter) ExistsPath(pathname string) (bool, error) { + return false, errors.New("not implemented") } func (f *FakeMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) { diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index d1edbfa266a..06ee97e5d27 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -91,9 +91,9 @@ type Interface interface { // else. E.g. if the directory already exists, it may exists outside of the // base due to symlinks. SafeMakeDir(pathname string, base string, perm os.FileMode) error - // ExistsPath checks whether the path exists. - // Will operate in the host mount namespace if kubelet is running in a container - ExistsPath(pathname string) bool + // Will operate in the host mount namespace if kubelet is running in a container. + // Error is returned on any other error than "file not found". + ExistsPath(pathname string) (bool, error) // CleanSubPaths removes any bind-mounts created by PrepareSafeSubpath in given // pod volume directory. CleanSubPaths(podDir string, volumeName string) error diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 466fb9f8e03..a72774ab415 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -33,6 +33,7 @@ import ( "github.com/golang/glog" "golang.org/x/sys/unix" "k8s.io/apimachinery/pkg/util/sets" + utilfile "k8s.io/kubernetes/pkg/util/file" utilio "k8s.io/kubernetes/pkg/util/io" utilexec "k8s.io/utils/exec" ) @@ -447,12 +448,8 @@ func (mounter *Mounter) MakeFile(pathname string) error { return nil } -func (mounter *Mounter) ExistsPath(pathname string) bool { - _, err := os.Stat(pathname) - if err != nil { - return false - } - return true +func (mounter *Mounter) ExistsPath(pathname string) (bool, error) { + return utilfile.FileExists(pathname) } // formatAndMount uses unix utils to format and mount the given disk diff --git a/pkg/util/mount/mount_unsupported.go b/pkg/util/mount/mount_unsupported.go index 2b7a329340b..6143aec1827 100644 --- a/pkg/util/mount/mount_unsupported.go +++ b/pkg/util/mount/mount_unsupported.go @@ -21,8 +21,6 @@ package mount import ( "errors" "os" - - "github.com/golang/glog" ) type Mounter struct { @@ -110,9 +108,8 @@ func (mounter *Mounter) MakeFile(pathname string) error { return unsupportedErr } -func (mounter *Mounter) ExistsPath(pathname string) bool { - glog.Errorf("%s", unsupportedErr) - return true +func (mounter *Mounter) ExistsPath(pathname string) (bool, error) { + return true, errors.New("not implemented") } func (mounter *Mounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) { diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index 2c87755c9f2..3079b8c65de 100644 --- a/pkg/util/mount/mount_windows.go +++ b/pkg/util/mount/mount_windows.go @@ -29,6 +29,8 @@ import ( "syscall" "github.com/golang/glog" + + utilfile "k8s.io/kubernetes/pkg/util/file" ) // Mounter provides the default implementation of mount.Interface @@ -147,9 +149,13 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { if stat.Mode()&os.ModeSymlink != 0 { target, err := os.Readlink(file) if err != nil { - return true, fmt.Errorf("Readlink error: %v", err) + return true, fmt.Errorf("readlink error: %v", err) } - return !mounter.ExistsPath(target), nil + exists, err := mounter.ExistsPath(target) + if err != nil { + return true, err + } + return !exists, nil } return true, nil @@ -232,12 +238,8 @@ func (mounter *Mounter) MakeFile(pathname string) error { } // ExistsPath checks whether the path exists -func (mounter *Mounter) ExistsPath(pathname string) bool { - _, err := os.Stat(pathname) - if err != nil { - return false - } - return true +func (mounter *Mounter) ExistsPath(pathname string) (bool, error) { + return utilfile.FileExists(pathname) } // check whether hostPath is within volume path diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index 9531edddb25..4a920d63b51 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -27,6 +27,7 @@ import ( "strings" "github.com/golang/glog" + utilfile "k8s.io/kubernetes/pkg/util/file" utilio "k8s.io/kubernetes/pkg/util/io" "k8s.io/kubernetes/pkg/util/nsenter" ) @@ -281,13 +282,15 @@ func (mounter *NsenterMounter) MakeFile(pathname string) error { return nil } -func (mounter *NsenterMounter) ExistsPath(pathname string) bool { - args := []string{pathname} - _, err := mounter.ne.Exec("ls", args).CombinedOutput() - if err == nil { - return true +func (mounter *NsenterMounter) ExistsPath(pathname string) (bool, error) { + // Resolve the symlinks but allow the target not to exist. EvalSymlinks + // would return an generic error when the target does not exist. + hostPath, err := mounter.ne.EvalSymlinks(pathname, false /* mustExist */) + if err != nil { + return false, err } - return false + kubeletpath := mounter.ne.KubeletPath(hostPath) + return utilfile.FileExists(kubeletpath) } func (mounter *NsenterMounter) CleanSubPaths(podDir string, volumeName string) error { diff --git a/pkg/util/mount/nsenter_mount_unsupported.go b/pkg/util/mount/nsenter_mount_unsupported.go index 007a456d490..401e1371f6a 100644 --- a/pkg/util/mount/nsenter_mount_unsupported.go +++ b/pkg/util/mount/nsenter_mount_unsupported.go @@ -83,8 +83,8 @@ func (*NsenterMounter) MakeFile(pathname string) error { return nil } -func (*NsenterMounter) ExistsPath(pathname string) bool { - return true +func (*NsenterMounter) ExistsPath(pathname string) (bool, error) { + return true, errors.New("not implemented") } func (*NsenterMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { diff --git a/pkg/util/removeall/removeall_test.go b/pkg/util/removeall/removeall_test.go index 878db734cdf..1b67cc981f8 100644 --- a/pkg/util/removeall/removeall_test.go +++ b/pkg/util/removeall/removeall_test.go @@ -75,8 +75,8 @@ func (mounter *fakeMounter) MakeFile(pathname string) error { return nil } -func (mounter *fakeMounter) ExistsPath(pathname string) bool { - return true +func (mounter *fakeMounter) ExistsPath(pathname string) (bool, error) { + return true, errors.New("not implemented") } func (mounter *fakeMounter) PrepareSafeSubpath(subPath mount.Subpath) (newHostPath string, cleanupAction func(), err error) { diff --git a/pkg/volume/host_path/host_path.go b/pkg/volume/host_path/host_path.go index 4a3cb76b6af..4bde9891cdf 100644 --- a/pkg/volume/host_path/host_path.go +++ b/pkg/volume/host_path/host_path.go @@ -350,7 +350,8 @@ type fileTypeChecker struct { } func (ftc *fileTypeChecker) Exists() bool { - return ftc.mounter.ExistsPath(ftc.path) + exists, err := ftc.mounter.ExistsPath(ftc.path) + return exists && err == nil } func (ftc *fileTypeChecker) IsFile() bool { diff --git a/pkg/volume/host_path/host_path_test.go b/pkg/volume/host_path/host_path_test.go index b8ae4fa9fc0..39696d765df 100644 --- a/pkg/volume/host_path/host_path_test.go +++ b/pkg/volume/host_path/host_path_test.go @@ -369,8 +369,8 @@ func (fftc *fakeFileTypeChecker) MakeDir(pathname string) error { return nil } -func (fftc *fakeFileTypeChecker) ExistsPath(pathname string) bool { - return true +func (fftc *fakeFileTypeChecker) ExistsPath(pathname string) (bool, error) { + return true, nil } func (fftc *fakeFileTypeChecker) GetFileType(_ string) (utilmount.FileType, error) { From 7e3fb502a80a005962306302878ec525af2c43f7 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:25 +0200 Subject: [PATCH 4/9] Change SafeMakeDir to resolve symlinks in mounter implementation Kubelet should not resolve symlinks outside of mounter interface. Only mounter interface knows, how to resolve them properly on the host. As consequence, declaration of SafeMakeDir changes to simplify the implementation: from SafeMakeDir(fullPath string, base string, perm os.FileMode) to SafeMakeDir(subdirectoryInBase string, base string, perm os.FileMode) --- pkg/kubelet/kubelet_pods.go | 13 +++++-------- pkg/util/mount/mount.go | 16 +++++++++------- pkg/util/mount/mount_linux.go | 15 ++++++++++++--- pkg/util/mount/mount_windows.go | 10 ++++++++-- pkg/util/mount/nsenter_mount.go | 17 +++++++++++++++-- 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index c1d2c54f090..55dfce9a589 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -174,10 +174,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h return nil, cleanupAction, fmt.Errorf("unable to provision SubPath `%s`: %v", mount.SubPath, err) } - volumePath, err := filepath.EvalSymlinks(hostPath) - if err != nil { - return nil, cleanupAction, err - } + volumePath := hostPath hostPath = filepath.Join(volumePath, mount.SubPath) if subPathExists, err := mounter.ExistsPath(hostPath); err != nil { @@ -192,10 +189,10 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h if err != nil { return nil, cleanupAction, err } - - if err := mounter.SafeMakeDir(hostPath, volumePath, perm); err != nil { - glog.Errorf("failed to mkdir %q: %v", hostPath, err) - return nil, cleanupAction, err + if err := mounter.SafeMakeDir(mount.SubPath, volumePath, perm); err != nil { + // Don't pass detailed error back to the user because it could give information about host filesystem + glog.Errorf("failed to create subPath directory for volumeMount %q of container %q: %v", mount.Name, container.Name, err) + return nil, cleanupAction, fmt.Errorf("failed to create subPath directory for volumeMount %q of container %q", mount.Name, container.Name) } } hostPath, cleanupAction, err = mounter.PrepareSafeSubpath(mountutil.Subpath{ diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index 06ee97e5d27..49351394dd8 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -84,13 +84,15 @@ type Interface interface { // MakeDir creates a new directory. // Will operate in the host mount namespace if kubelet is running in a container MakeDir(pathname string) error - // SafeMakeDir makes sure that the created directory does not escape given - // base directory mis-using symlinks. The directory is created in the same - // mount namespace as where kubelet is running. Note that the function makes - // sure that it creates the directory somewhere under the base, nothing - // else. E.g. if the directory already exists, it may exists outside of the - // base due to symlinks. - SafeMakeDir(pathname string, base string, perm os.FileMode) error + // SafeMakeDir creates subdir within given base. It makes sure that the + // created directory does not escape given base directory mis-using + // symlinks. Note that the function makes sure that it creates the directory + // somewhere under the base, nothing else. E.g. if the directory already + // exists, it may exist outside of the base due to symlinks. + // This method should be used if the directory to create is inside volume + // that's under user control. User must not be able to use symlinks to + // escape the volume to create directories somewhere else. + SafeMakeDir(subdir string, base string, perm os.FileMode) error // Will operate in the host mount namespace if kubelet is running in a container. // Error is returned on any other error than "file not found". ExistsPath(pathname string) (bool, error) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index a72774ab415..9998792893f 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -955,8 +955,15 @@ func removeEmptyDirs(baseDir, endDir string) error { return nil } -func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { - return doSafeMakeDir(pathname, base, perm) +func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error { + realBase, err := filepath.EvalSymlinks(base) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", base, err) + } + + realFullPath := filepath.Join(realBase, subdir) + + return doSafeMakeDir(realFullPath, realBase, perm) } func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { @@ -1001,7 +1008,9 @@ func getMode(pathname string) (os.FileMode, error) { return info.Mode(), nil } -// This implementation is shared between Linux and NsEnterMounter +// This implementation is shared between Linux and NsEnterMounter. Both pathname +// and base must be either already resolved symlinks or thet will be resolved in +// kubelet's mount namespace (in case it runs containerized). func doSafeMakeDir(pathname string, base string, perm os.FileMode) error { glog.V(4).Infof("Creating directory %q within base %q", pathname, base) diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index 3079b8c65de..64d39a59f7e 100644 --- a/pkg/util/mount/mount_windows.go +++ b/pkg/util/mount/mount_windows.go @@ -472,8 +472,14 @@ func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) { } // SafeMakeDir makes sure that the created directory does not escape given base directory mis-using symlinks. -func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { - return doSafeMakeDir(pathname, base, perm) +func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error { + realBase, err := filepath.EvalSymlinks(base) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", base, err) + } + + realFullPath := filepath.Join(realBase, subdir) + return doSafeMakeDir(realFullPath, realBase, perm) } func doSafeMakeDir(pathname string, base string, perm os.FileMode) error { diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index 4a920d63b51..dfc690a64bc 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -327,8 +327,21 @@ func (mounter *NsenterMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath return newHostPath, cleanupAction, err } -func (mounter *NsenterMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { - return doSafeMakeDir(pathname, base, perm) +func (mounter *NsenterMounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error { + fullSubdirPath := filepath.Join(base, subdir) + evaluatedSubdirPath, err := mounter.ne.EvalSymlinks(fullSubdirPath, false /* mustExist */) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", fullSubdirPath, err) + } + kubeletSubdirPath := mounter.ne.KubeletPath(evaluatedSubdirPath) + + evaluatedBase, err := mounter.ne.EvalSymlinks(base, true /* mustExist */) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", base, err) + } + kubeletBase := mounter.ne.KubeletPath(evaluatedBase) + + return doSafeMakeDir(kubeletSubdirPath, kubeletBase, perm) } func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) { From 225a879b07a63288a90cc7cb28a5d500975f2c63 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:25 +0200 Subject: [PATCH 5/9] Refactor doBindSubPath into smaller functions: - getSubpathBindTarget() computes final target of subpath bind-mount. - prepareSubpathTarget() creates target for bind-mount. - safeOpenSubPath() checks symlinks in Subpath and safely opens it. --- pkg/util/mount/mount_linux.go | 147 +++++++++++++++++------------ pkg/util/mount/mount_linux_test.go | 8 -- 2 files changed, 88 insertions(+), 67 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 9998792893f..0671099b727 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -721,37 +721,109 @@ func getSELinuxSupport(path string, mountInfoFilename string) (bool, error) { func (mounter *Mounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) { newHostPath, err = doBindSubPath(mounter, subPath, os.Getpid()) + // There is no action when the container starts. Bind-mount will be cleaned // when container stops by CleanSubPaths. cleanupAction = nil return newHostPath, cleanupAction, err } +// This implementation is shared between Linux and NsEnterMounter +func safeOpenSubPath(mounter Interface, subpath Subpath) (int, error) { + if !pathWithinBase(subpath.Path, subpath.VolumePath) { + return -1, fmt.Errorf("subpath %q not within volume path %q", subpath.Path, subpath.VolumePath) + } + fd, err := doSafeOpen(subpath.Path, subpath.VolumePath) + if err != nil { + return -1, fmt.Errorf("error opening subpath %v: %v", subpath.Path, err) + } + return fd, nil +} + +// prepareSubpathTarget creates target for bind-mount of subpath. It returns +// "true" when the target already exists and something is mounted there. +func prepareSubpathTarget(mounter Interface, subpath Subpath) (bool, string, error) { + // Early check for already bind-mounted subpath. + bindPathTarget := getSubpathBindTarget(subpath) + notMount, err := IsNotMountPoint(mounter, bindPathTarget) + if err != nil { + if !os.IsNotExist(err) { + return false, "", fmt.Errorf("error checking path %s for mount: %s", bindPathTarget, err) + } + // Ignore ErrorNotExist: the file/directory will be created below if it does not exist yet. + notMount = true + } + if !notMount { + // It's already mounted + glog.V(5).Infof("Skipping bind-mounting subpath %s: already mounted", bindPathTarget) + return true, bindPathTarget, nil + } + + // bindPathTarget is in /var/lib/kubelet and thus reachable without any + // translation even to containerized kubelet. + bindParent := filepath.Dir(bindPathTarget) + err = os.MkdirAll(bindParent, 0750) + if err != nil && !os.IsExist(err) { + return false, "", fmt.Errorf("error creating directory %s: %s", bindParent, err) + } + + t, err := os.Lstat(subpath.Path) + if err != nil { + return false, "", fmt.Errorf("lstat %s failed: %s", subpath.Path, err) + } + + if t.Mode()&os.ModeDir > 0 { + if err = os.Mkdir(bindPathTarget, 0750); err != nil && !os.IsExist(err) { + return false, "", fmt.Errorf("error creating directory %s: %s", bindPathTarget, err) + } + } else { + // "/bin/touch ". + // A file is enough for all possible targets (symlink, device, pipe, + // socket, ...), bind-mounting them into a file correctly changes type + // of the target file. + if err = ioutil.WriteFile(bindPathTarget, []byte{}, 0640); err != nil { + return false, "", fmt.Errorf("error creating file %s: %s", bindPathTarget, err) + } + } + return false, bindPathTarget, nil +} + +func getSubpathBindTarget(subpath Subpath) string { + // containerName is DNS label, i.e. safe as a directory name. + return filepath.Join(subpath.PodDir, containerSubPathDirectoryName, subpath.VolumeName, subpath.ContainerName, strconv.Itoa(subpath.VolumeMountIndex)) +} + // This implementation is shared between Linux and NsEnterMounter // kubeletPid is PID of kubelet in the PID namespace where bind-mount is done, // i.e. pid on the *host* if kubelet runs in a container. func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath string, err error) { - // Check early for symlink. This is just a pre-check to avoid bind-mount - // before the final check. - evalSubPath, err := filepath.EvalSymlinks(subpath.Path) + // Evaluate all symlinks here once for all subsequent functions. + newVolumePath, err := filepath.EvalSymlinks(subpath.VolumePath) if err != nil { - return "", fmt.Errorf("evalSymlinks %q failed: %v", subpath.Path, err) + return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.VolumePath, err) } - glog.V(5).Infof("doBindSubPath %q, full subpath %q for volumepath %q", subpath.Path, evalSubPath, subpath.VolumePath) + newPath, err := filepath.EvalSymlinks(subpath.Path) + if err != nil { + return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err) + } + glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, newPath, subpath.VolumePath) + subpath.VolumePath = newVolumePath + subpath.Path = newPath - evalSubPath = filepath.Clean(evalSubPath) - if !pathWithinBase(evalSubPath, subpath.VolumePath) { - return "", fmt.Errorf("subpath %q not within volume path %q", evalSubPath, subpath.VolumePath) + // Check the subpath is correct and open it + fd, err := safeOpenSubPath(mounter, subpath) + if err != nil { + return "", err } + defer syscall.Close(fd) - // Prepare directory for bind mounts - // containerName is DNS label, i.e. safe as a directory name. - bindDir := filepath.Join(subpath.PodDir, containerSubPathDirectoryName, subpath.VolumeName, subpath.ContainerName) - err = os.MkdirAll(bindDir, 0750) - if err != nil && !os.IsExist(err) { - return "", fmt.Errorf("error creating directory %s: %s", bindDir, err) + alreadyMounted, bindPathTarget, err := prepareSubpathTarget(mounter, subpath) + if err != nil { + return "", err + } + if alreadyMounted { + return bindPathTarget, nil } - bindPathTarget := filepath.Join(bindDir, strconv.Itoa(subpath.VolumeMountIndex)) success := false defer func() { @@ -764,49 +836,6 @@ func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath } }() - // Check it's not already bind-mounted - notMount, err := IsNotMountPoint(mounter, bindPathTarget) - if err != nil { - if !os.IsNotExist(err) { - return "", fmt.Errorf("error checking path %s for mount: %s", bindPathTarget, err) - } - // Ignore ErrorNotExist: the file/directory will be created below if it does not exist yet. - notMount = true - } - if !notMount { - // It's already mounted - glog.V(5).Infof("Skipping bind-mounting subpath %s: already mounted", bindPathTarget) - success = true - return bindPathTarget, nil - } - - // Create target of the bind mount. A directory for directories, empty file - // for everything else. - t, err := os.Lstat(subpath.Path) - if err != nil { - return "", fmt.Errorf("lstat %s failed: %s", subpath.Path, err) - } - if t.Mode()&os.ModeDir > 0 { - if err = os.Mkdir(bindPathTarget, 0750); err != nil && !os.IsExist(err) { - return "", fmt.Errorf("error creating directory %s: %s", bindPathTarget, err) - } - } else { - // "/bin/touch ". - // A file is enough for all possible targets (symlink, device, pipe, - // socket, ...), bind-mounting them into a file correctly changes type - // of the target file. - if err = ioutil.WriteFile(bindPathTarget, []byte{}, 0640); err != nil { - return "", fmt.Errorf("error creating file %s: %s", bindPathTarget, err) - } - } - - // Safe open subpath and get the fd - fd, err := doSafeOpen(evalSubPath, subpath.VolumePath) - if err != nil { - return "", fmt.Errorf("error opening subpath %v: %v", evalSubPath, err) - } - defer syscall.Close(fd) - mountSource := fmt.Sprintf("/proc/%d/fd/%v", kubeletPid, fd) // Do the bind mount @@ -819,8 +848,8 @@ func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath if err = mounter.Mount(mountSource, bindPathTarget, "" /*fstype*/, options); err != nil { return "", fmt.Errorf("error mounting %s: %s", subpath.Path, err) } - success = true + glog.V(3).Infof("Bound SubPath %s into %s", subpath.Path, bindPathTarget) return bindPathTarget, nil } diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go index 6028d608e97..86fb25597c6 100644 --- a/pkg/util/mount/mount_linux_test.go +++ b/pkg/util/mount/mount_linux_test.go @@ -1193,10 +1193,6 @@ func TestBindSubPath(t *testing.T) { return nil, "", "", err } - if err := os.MkdirAll(subpathMount, defaultPerm); err != nil { - return nil, "", "", err - } - socketFile, socketCreateError := createSocketFile(volpath) return mounts, volpath, socketFile, socketCreateError @@ -1212,10 +1208,6 @@ func TestBindSubPath(t *testing.T) { return nil, "", "", err } - if err := os.MkdirAll(subpathMount, defaultPerm); err != nil { - return nil, "", "", err - } - testFifo := filepath.Join(volpath, "mount_test.fifo") err := syscall.Mkfifo(testFifo, 0) return mounts, volpath, testFifo, err From 9f80de3772b41553770f1d0654d3e291a5bec50c Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 23 May 2018 10:15:12 +0200 Subject: [PATCH 6/9] Split NsEnterMounter and Mounter implementation of doBindSubpath nsenter implementation needs to mount different thing in the end and do different checks on the result. --- pkg/util/mount/BUILD | 1 + pkg/util/mount/mount_linux.go | 22 ++-- pkg/util/mount/mount_linux_test.go | 2 +- pkg/util/mount/nsenter_mount.go | 123 +++++++++++++++------ pkg/util/mount/nsenter_mount_test.go | 156 ++++++++++----------------- 5 files changed, 167 insertions(+), 137 deletions(-) diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD index 6d2e65cbc96..a12f1397bf5 100644 --- a/pkg/util/mount/BUILD +++ b/pkg/util/mount/BUILD @@ -106,6 +106,7 @@ go_test( ] + select({ "@io_bazel_rules_go//go/platform:linux": [ "//vendor/github.com/golang/glog:go_default_library", + "//vendor/golang.org/x/sys/unix:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", ], "@io_bazel_rules_go//go/platform:windows": [ diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 0671099b727..27bb91249ce 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -720,7 +720,7 @@ func getSELinuxSupport(path string, mountInfoFilename string) (bool, error) { } func (mounter *Mounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) { - newHostPath, err = doBindSubPath(mounter, subPath, os.Getpid()) + newHostPath, err = doBindSubPath(mounter, subPath) // There is no action when the container starts. Bind-mount will be cleaned // when container stops by CleanSubPaths. @@ -742,6 +742,11 @@ func safeOpenSubPath(mounter Interface, subpath Subpath) (int, error) { // prepareSubpathTarget creates target for bind-mount of subpath. It returns // "true" when the target already exists and something is mounted there. +// Given Subpath must have all paths with already resolved symlinks and with +// paths relevant to kubelet (when it runs in a container). +// This function is called also by NsEnterMounter. It works because +// /var/lib/kubelet is mounted from the host into the container with Kubelet as +// /var/lib/kubelet too. func prepareSubpathTarget(mounter Interface, subpath Subpath) (bool, string, error) { // Early check for already bind-mounted subpath. bindPathTarget := getSubpathBindTarget(subpath) @@ -793,10 +798,12 @@ func getSubpathBindTarget(subpath Subpath) string { return filepath.Join(subpath.PodDir, containerSubPathDirectoryName, subpath.VolumeName, subpath.ContainerName, strconv.Itoa(subpath.VolumeMountIndex)) } -// This implementation is shared between Linux and NsEnterMounter -// kubeletPid is PID of kubelet in the PID namespace where bind-mount is done, -// i.e. pid on the *host* if kubelet runs in a container. -func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath string, err error) { +func doBindSubPath(mounter Interface, subpath Subpath) (hostPath string, err error) { + // Linux, kubelet runs on the host: + // - safely open the subpath + // - bind-mount /proc//fd/ to subpath target + // User can't change /proc//fd/ to point to a bad place. + // Evaluate all symlinks here once for all subsequent functions. newVolumePath, err := filepath.EvalSymlinks(subpath.VolumePath) if err != nil { @@ -810,7 +817,6 @@ func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath subpath.VolumePath = newVolumePath subpath.Path = newPath - // Check the subpath is correct and open it fd, err := safeOpenSubPath(mounter, subpath) if err != nil { return "", err @@ -836,6 +842,7 @@ func doBindSubPath(mounter Interface, subpath Subpath, kubeletPid int) (hostPath } }() + kubeletPid := os.Getpid() mountSource := fmt.Sprintf("/proc/%d/fd/%v", kubeletPid, fd) // Do the bind mount @@ -1193,6 +1200,9 @@ func findExistingPrefix(base, pathname string) (string, []string, error) { // Symlinks are disallowed (pathname must already resolve symlinks), // and the path must be within the base directory. func doSafeOpen(pathname string, base string) (int, error) { + pathname = filepath.Clean(pathname) + base = filepath.Clean(base) + // Calculate segments to follow subpath, err := filepath.Rel(base, pathname) if err != nil { diff --git a/pkg/util/mount/mount_linux_test.go b/pkg/util/mount/mount_linux_test.go index 86fb25597c6..36972c67c29 100644 --- a/pkg/util/mount/mount_linux_test.go +++ b/pkg/util/mount/mount_linux_test.go @@ -1291,7 +1291,7 @@ func TestBindSubPath(t *testing.T) { } _, subpathMount := getTestPaths(base) - bindPathTarget, err := doBindSubPath(fm, subpath, 1) + bindPathTarget, err := doBindSubPath(fm, subpath) if test.expectError { if err == nil { t.Errorf("test %q failed: expected error, got success", test.name) diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index dfc690a64bc..e1e02b7d82b 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -22,13 +22,12 @@ import ( "fmt" "os" "path/filepath" - "regexp" - "strconv" "strings" + "syscall" "github.com/golang/glog" + "golang.org/x/sys/unix" utilfile "k8s.io/kubernetes/pkg/util/file" - utilio "k8s.io/kubernetes/pkg/util/io" "k8s.io/kubernetes/pkg/util/nsenter" ) @@ -37,13 +36,6 @@ const ( hostProcMountsPath = "/rootfs/proc/1/mounts" // hostProcMountinfoPath is the default mount info path for rootfs hostProcMountinfoPath = "/rootfs/proc/1/mountinfo" - // hostProcSelfStatusPath is the default path to /proc/self/status on the host - hostProcSelfStatusPath = "/rootfs/proc/self/status" -) - -var ( - // pidRegExp matches "Pid: " in /proc/self/status - pidRegExp = regexp.MustCompile(`\nPid:\t([0-9]*)\n`) ) // Currently, all docker containers receive their own mount namespaces. @@ -297,29 +289,9 @@ func (mounter *NsenterMounter) CleanSubPaths(podDir string, volumeName string) e return doCleanSubPaths(mounter, podDir, volumeName) } -// getPidOnHost returns kubelet's pid in the host pid namespace -func (mounter *NsenterMounter) getPidOnHost(procStatusPath string) (int, error) { - // Get the PID from /rootfs/proc/self/status - statusBytes, err := utilio.ConsistentRead(procStatusPath, maxListTries) - if err != nil { - return 0, fmt.Errorf("error reading %s: %s", procStatusPath, err) - } - matches := pidRegExp.FindSubmatch(statusBytes) - if len(matches) < 2 { - return 0, fmt.Errorf("cannot parse %s: no Pid:", procStatusPath) - } - return strconv.Atoi(string(matches[1])) -} - func (mounter *NsenterMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) { - hostPid, err := mounter.getPidOnHost(hostProcSelfStatusPath) - if err != nil { - return "", nil, err - } - glog.V(4).Infof("Kubelet's PID on the host is %d", hostPid) - // Bind-mount the subpath to avoid using symlinks in subpaths. - newHostPath, err = doBindSubPath(mounter, subPath, hostPid) + newHostPath, err = doNsEnterBindSubPath(mounter, subPath) // There is no action when the container starts. Bind-mount will be cleaned // when container stops by CleanSubPaths. @@ -352,6 +324,95 @@ func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) { return getMountRefsByDev(mounter, hostpath) } +func doNsEnterBindSubPath(mounter *NsenterMounter, subpath Subpath) (hostPath string, err error) { + // Linux, kubelet runs in a container: + // - safely open the subpath + // - bind-mount the subpath to target (this can be unsafe) + // - check that we mounted the right thing by comparing device ID and inode + // of the subpath (via safely opened fd) and the target (that's under our + // control) + + // Evaluate all symlinks here once for all subsequent functions. + evaluatedHostVolumePath, err := mounter.ne.EvalSymlinks(subpath.VolumePath, true /*mustExist*/) + if err != nil { + return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.VolumePath, err) + } + evaluatedHostSubpath, err := mounter.ne.EvalSymlinks(subpath.Path, true /*mustExist*/) + if err != nil { + return "", fmt.Errorf("error resolving symlinks in %q: %v", subpath.Path, err) + } + glog.V(5).Infof("doBindSubPath %q (%q) for volumepath %q", subpath.Path, evaluatedHostSubpath, subpath.VolumePath) + subpath.VolumePath = mounter.ne.KubeletPath(evaluatedHostVolumePath) + subpath.Path = mounter.ne.KubeletPath(evaluatedHostSubpath) + + // Check the subpath is correct and open it + fd, err := safeOpenSubPath(mounter, subpath) + if err != nil { + return "", err + } + defer syscall.Close(fd) + + alreadyMounted, bindPathTarget, err := prepareSubpathTarget(mounter, subpath) + if err != nil { + return "", err + } + if alreadyMounted { + return bindPathTarget, nil + } + + success := false + defer func() { + // Cleanup subpath on error + if !success { + glog.V(4).Infof("doNsEnterBindSubPath() failed for %q, cleaning up subpath", bindPathTarget) + if cleanErr := cleanSubPath(mounter, subpath); cleanErr != nil { + glog.Errorf("Failed to clean subpath %q: %v", bindPathTarget, cleanErr) + } + } + }() + + // Leap of faith: optimistically expect that nobody has modified previously + // expanded evalSubPath with evil symlinks and bind-mount it. + // Mount is done on the host! don't use kubelet path! + glog.V(5).Infof("bind mounting %q at %q", evaluatedHostSubpath, bindPathTarget) + if err = mounter.Mount(evaluatedHostSubpath, bindPathTarget, "" /*fstype*/, []string{"bind"}); err != nil { + return "", fmt.Errorf("error mounting %s: %s", evaluatedHostSubpath, err) + } + + // Check that the bind-mount target is the same inode and device as the + // source that we keept open, i.e. we mounted the right thing. + err = checkDeviceInode(fd, bindPathTarget) + if err != nil { + return "", fmt.Errorf("error checking bind mount for subpath %s: %s", subpath.VolumePath, err) + } + + success = true + glog.V(3).Infof("Bound SubPath %s into %s", subpath.Path, bindPathTarget) + return bindPathTarget, nil +} + +// checkDeviceInode checks that opened file and path represent the same file. +func checkDeviceInode(fd int, path string) error { + var srcStat, dstStat unix.Stat_t + err := unix.Fstat(fd, &srcStat) + if err != nil { + return fmt.Errorf("error running fstat on subpath FD: %v", err) + } + + err = unix.Stat(path, &dstStat) + if err != nil { + return fmt.Errorf("error running fstat on %s: %v", path, err) + } + + if srcStat.Dev != dstStat.Dev { + return fmt.Errorf("different device number") + } + if srcStat.Ino != dstStat.Ino { + return fmt.Errorf("different inode") + } + return nil +} + func (mounter *NsenterMounter) GetFSGroup(pathname string) (int64, error) { hostPath, err := mounter.ne.EvalSymlinks(pathname, true /* mustExist */) if err != nil { diff --git a/pkg/util/mount/nsenter_mount_test.go b/pkg/util/mount/nsenter_mount_test.go index 2aee4ad5f9d..2edb6febf97 100644 --- a/pkg/util/mount/nsenter_mount_test.go +++ b/pkg/util/mount/nsenter_mount_test.go @@ -21,9 +21,11 @@ package mount import ( "io/ioutil" "os" - "path" - "strconv" + "path/filepath" + "strings" "testing" + + "golang.org/x/sys/unix" ) func TestParseFindMnt(t *testing.T) { @@ -72,120 +74,76 @@ func TestParseFindMnt(t *testing.T) { } } -func TestGetPidOnHost(t *testing.T) { - tempDir, err := ioutil.TempDir("", "get_pid_on_host_tests") +func TestCheckDeviceInode(t *testing.T) { + testDir, err := ioutil.TempDir("", "nsenter-mounter-device-") if err != nil { - t.Fatalf(err.Error()) + t.Fatalf("Cannot create temporary directory: %s", err) } - defer os.RemoveAll(tempDir) + defer os.RemoveAll(testDir) tests := []struct { name string - procFile string - expectedPid int - expectError bool + srcPath string + dstPath string + expectError string }{ { - name: "valid status file", - procFile: `Name: cat -Umask: 0002 -State: R (running) -Tgid: 15041 -Ngid: 0 -Pid: 15041 -PPid: 22699 -TracerPid: 0 -Uid: 1000 1000 1000 1000 -Gid: 1000 1000 1000 1000 -FDSize: 256 -Groups: 10 135 156 157 158 973 984 1000 1001 -NStgid: 15041 -NSpid: 15041 -NSpgid: 15041 -NSsid: 22699 -VmPeak: 115016 kB -VmSize: 115016 kB -VmLck: 0 kB -VmPin: 0 kB -VmHWM: 816 kB -VmRSS: 816 kB -RssAnon: 64 kB -RssFile: 752 kB -RssShmem: 0 kB -VmData: 312 kB -VmStk: 136 kB -VmExe: 32 kB -VmLib: 2060 kB -VmPTE: 44 kB -VmPMD: 12 kB -VmSwap: 0 kB -HugetlbPages: 0 kB -Threads: 1 -SigQ: 2/60752 -SigPnd: 0000000000000000 -ShdPnd: 0000000000000000 -SigBlk: 0000000000000000 -SigIgn: 0000000000000000 -SigCgt: 0000000000000000 -CapInh: 0000000000000000 -CapPrm: 0000000000000000 -CapEff: 0000000000000000 -CapBnd: 0000003fffffffff -CapAmb: 0000000000000000 -NoNewPrivs: 0 -Seccomp: 0 -Cpus_allowed: ff -Cpus_allowed_list: 0-7 -Mems_allowed: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000001 -Mems_allowed_list: 0 -voluntary_ctxt_switches: 0 -nonvoluntary_ctxt_switches: 0 -`, - expectedPid: 15041, + name: "the same file", + srcPath: filepath.Join(testDir, "1"), + dstPath: filepath.Join(testDir, "1"), + expectError: "", }, { - name: "no Pid:", - procFile: `Name: cat -Umask: 0002 -State: R (running) -Tgid: 15041 -Ngid: 0 -PPid: 22699 -`, - expectedPid: 0, - expectError: true, + name: "different file on the same FS", + srcPath: filepath.Join(testDir, "2.1"), + dstPath: filepath.Join(testDir, "2.2"), + expectError: "different inode", }, { - name: "invalid Pid:", - procFile: `Name: cat -Umask: 0002 -State: R (running) -Tgid: 15041 -Ngid: 0 -Pid: invalid -PPid: 22699 -`, - expectedPid: 0, - expectError: true, + name: "different file on different device", + srcPath: filepath.Join(testDir, "3"), + // /proc is always on a different "device" than /tmp (or $TEMP) + dstPath: "/proc/self/status", + expectError: "different device", }, } - for i, test := range tests { - filename := path.Join(tempDir, strconv.Itoa(i)) - err := ioutil.WriteFile(filename, []byte(test.procFile), 0666) + for _, test := range tests { + if err := ioutil.WriteFile(test.srcPath, []byte{}, 0644); err != nil { + t.Errorf("Test %q: cannot create srcPath %s: %s", test.name, test.srcPath, err) + continue + } + + // Don't create dst if it exists + if _, err := os.Stat(test.dstPath); os.IsNotExist(err) { + if err := ioutil.WriteFile(test.dstPath, []byte{}, 0644); err != nil { + t.Errorf("Test %q: cannot create dstPath %s: %s", test.name, test.dstPath, err) + continue + } + } else if err != nil { + t.Errorf("Test %q: cannot check existence of dstPath %s: %s", test.name, test.dstPath, err) + continue + } + + fd, err := unix.Open(test.srcPath, unix.O_CREAT, 0644) if err != nil { - t.Fatalf(err.Error()) + t.Errorf("Test %q: cannot open srcPath %s: %s", test.name, test.srcPath, err) + continue } - mounter := NsenterMounter{} - pid, err := mounter.getPidOnHost(filename) - if err != nil && !test.expectError { - t.Errorf("Test %q: unexpected error: %s", test.name, err) + + err = checkDeviceInode(fd, test.dstPath) + + if test.expectError == "" && err != nil { + t.Errorf("Test %q: expected no error, got %s", test.name, err) } - if err == nil && test.expectError { - t.Errorf("Test %q: expected error, got none", test.name) - } - if pid != test.expectedPid { - t.Errorf("Test %q: expected pid %d, got %d", test.name, test.expectedPid, pid) + if test.expectError != "" { + if err == nil { + t.Errorf("Test %q: expected error, got none", test.name) + } else { + if !strings.Contains(err.Error(), test.expectError) { + t.Errorf("Test %q: expected error %q, got %q", test.name, test.expectError, err) + } + } } } } From a8a37fb714da452258f2ce84bbb51c73bb2f1ac8 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:25 +0200 Subject: [PATCH 7/9] Created directories in /var/lib/kubelet directly. --- cmd/kubelet/app/server.go | 2 +- pkg/util/mount/nsenter_mount.go | 26 ++++++++++++++++++--- pkg/util/mount/nsenter_mount_unsupported.go | 2 +- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 10bbae78cfc..761cd49df32 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -360,7 +360,7 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err var writer kubeio.Writer = &kubeio.StdWriter{} if s.Containerized { glog.V(2).Info("Running kubelet in containerized mode") - mounter, err = mount.NewNsenterMounter() + mounter, err = mount.NewNsenterMounter(s.RootDirectory) if err != nil { return nil, err } diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index e1e02b7d82b..71660405f5e 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -43,9 +43,15 @@ const ( // the host's mount namespace. type NsenterMounter struct { ne *nsenter.Nsenter + // rootDir is location of /var/lib/kubelet directory. + rootDir string } -func NewNsenterMounter() (*NsenterMounter, error) { +// NewNsenterMounter creates a new mounter for kubelet that runs as a container. +// rootDir is location of /var/lib/kubelet directory (in case it's not on the +// default place). This directory must be available in the container +// on the same place as it's on the host. +func NewNsenterMounter(rootDir string) (*NsenterMounter, error) { ne, err := nsenter.NewNsenter() if err != nil { return nil, err @@ -305,14 +311,28 @@ func (mounter *NsenterMounter) SafeMakeDir(subdir string, base string, perm os.F if err != nil { return fmt.Errorf("error resolving symlinks in %s: %s", fullSubdirPath, err) } - kubeletSubdirPath := mounter.ne.KubeletPath(evaluatedSubdirPath) + evaluatedSubdirPath = filepath.Clean(evaluatedSubdirPath) evaluatedBase, err := mounter.ne.EvalSymlinks(base, true /* mustExist */) if err != nil { return fmt.Errorf("error resolving symlinks in %s: %s", base, err) } - kubeletBase := mounter.ne.KubeletPath(evaluatedBase) + evaluatedBase = filepath.Clean(evaluatedBase) + rootDir := filepath.Clean(mounter.rootDir) + if pathWithinBase(evaluatedBase, rootDir) { + // Base is in /var/lib/kubelet. This directory is shared between the + // container with kubelet and the host. We don't need to add '/rootfs'. + // This is useful when /rootfs is mounted as read-only - we can still + // create subpaths for paths in /var/lib/kubelet. + return doSafeMakeDir(evaluatedSubdirPath, evaluatedBase, perm) + } + + // Base is somewhere on the host's filesystem. Add /rootfs and try to make + // the directory there. + // This requires /rootfs to be writable. + kubeletSubdirPath := mounter.ne.KubeletPath(evaluatedSubdirPath) + kubeletBase := mounter.ne.KubeletPath(evaluatedBase) return doSafeMakeDir(kubeletSubdirPath, kubeletBase, perm) } diff --git a/pkg/util/mount/nsenter_mount_unsupported.go b/pkg/util/mount/nsenter_mount_unsupported.go index 401e1371f6a..ef5cb5a107d 100644 --- a/pkg/util/mount/nsenter_mount_unsupported.go +++ b/pkg/util/mount/nsenter_mount_unsupported.go @@ -25,7 +25,7 @@ import ( type NsenterMounter struct{} -func NewNsenterMounter() (*NsenterMounter, error) { +func NewNsenterMounter(rootDir string) (*NsenterMounter, error) { return &NsenterMounter{}, nil } From 9b741254400e68e79feb5d28250e966c01c9d34b Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:26 +0200 Subject: [PATCH 8/9] Pass Nsenter to NsenterMounter and NsenterWriter So Nsenter is initialized only once and with the right parameters. --- cmd/kubelet/app/BUILD | 2 ++ cmd/kubelet/app/server.go | 7 ++++-- pkg/util/io/writer.go | 20 +++++++++------ pkg/util/mount/BUILD | 28 +++++++++++++++++++++ pkg/util/mount/nsenter_mount.go | 12 +++------ pkg/util/mount/nsenter_mount_unsupported.go | 6 +++-- 6 files changed, 56 insertions(+), 19 deletions(-) diff --git a/cmd/kubelet/app/BUILD b/cmd/kubelet/app/BUILD index af2fcc09ad5..b2f3efbc793 100644 --- a/cmd/kubelet/app/BUILD +++ b/cmd/kubelet/app/BUILD @@ -110,6 +110,7 @@ go_library( "//pkg/util/io:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/util/node:go_default_library", + "//pkg/util/nsenter:go_default_library", "//pkg/util/oom:go_default_library", "//pkg/util/rlimit:go_default_library", "//pkg/version:go_default_library", @@ -170,6 +171,7 @@ go_library( "//vendor/k8s.io/client-go/tools/record:go_default_library", "//vendor/k8s.io/client-go/util/cert:go_default_library", "//vendor/k8s.io/client-go/util/certificate:go_default_library", + "//vendor/k8s.io/utils/exec:go_default_library", ] + select({ "@io_bazel_rules_go//go/platform:linux": [ "//vendor/golang.org/x/exp/inotify:go_default_library", diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 761cd49df32..f4bd1a05050 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -91,10 +91,12 @@ import ( kubeio "k8s.io/kubernetes/pkg/util/io" "k8s.io/kubernetes/pkg/util/mount" nodeutil "k8s.io/kubernetes/pkg/util/node" + "k8s.io/kubernetes/pkg/util/nsenter" "k8s.io/kubernetes/pkg/util/oom" "k8s.io/kubernetes/pkg/util/rlimit" "k8s.io/kubernetes/pkg/version" "k8s.io/kubernetes/pkg/version/verflag" + "k8s.io/utils/exec" ) const ( @@ -360,11 +362,12 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err var writer kubeio.Writer = &kubeio.StdWriter{} if s.Containerized { glog.V(2).Info("Running kubelet in containerized mode") - mounter, err = mount.NewNsenterMounter(s.RootDirectory) + ne, err := nsenter.NewNsenter(nsenter.DefaultHostRootFsPath, exec.New()) if err != nil { return nil, err } - writer = &kubeio.NsenterWriter{} + mounter = mount.NewNsenterMounter(s.RootDirectory, ne) + writer = kubeio.NewNsenterWriter(ne) } var dockerClientConfig *dockershim.ClientConfig diff --git a/pkg/util/io/writer.go b/pkg/util/io/writer.go index 4c0c1c4b37c..7c457d04a7d 100644 --- a/pkg/util/io/writer.go +++ b/pkg/util/io/writer.go @@ -50,18 +50,24 @@ func (writer *StdWriter) WriteFile(filename string, data []byte, perm os.FileMod // it will not see the mounted device in its own namespace. To work around this // limitation one has to first enter hosts namespace (by using 'nsenter') and // only then write data. -type NsenterWriter struct{} +type NsenterWriter struct { + ne *nsenter.Nsenter +} + +// NewNsenterWriter creates a new Writer that allows writing data to file using +// nsenter command. +func NewNsenterWriter(ne *nsenter.Nsenter) *NsenterWriter { + return &NsenterWriter{ + ne: ne, + } +} // WriteFile calls 'nsenter cat - > ' and 'nsenter chmod' to create a // file on the host. func (writer *NsenterWriter) WriteFile(filename string, data []byte, perm os.FileMode) error { - ne, err := nsenter.NewNsenter() - if err != nil { - return err - } echoArgs := []string{"-c", fmt.Sprintf("cat > %s", filename)} glog.V(5).Infof("nsenter: write data to file %s by nsenter", filename) - command := ne.Exec("sh", echoArgs) + command := writer.ne.Exec("sh", echoArgs) command.SetStdin(bytes.NewBuffer(data)) outputBytes, err := command.CombinedOutput() if err != nil { @@ -71,7 +77,7 @@ func (writer *NsenterWriter) WriteFile(filename string, data []byte, perm os.Fil chmodArgs := []string{fmt.Sprintf("%o", perm), filename} glog.V(5).Infof("nsenter: change permissions of file %s to %s", filename, chmodArgs[0]) - outputBytes, err = ne.Exec("chmod", chmodArgs).CombinedOutput() + outputBytes, err = writer.ne.Exec("chmod", chmodArgs).CombinedOutput() if err != nil { glog.Errorf("Output from chmod command: %v", string(outputBytes)) return err diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD index a12f1397bf5..40778e7c73b 100644 --- a/pkg/util/mount/BUILD +++ b/pkg/util/mount/BUILD @@ -71,6 +71,18 @@ go_library( "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", ] + select({ + "@io_bazel_rules_go//go/platform:android": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:darwin": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:dragonfly": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:freebsd": [ + "//pkg/util/nsenter:go_default_library", + ], "@io_bazel_rules_go//go/platform:linux": [ "//pkg/util/file:go_default_library", "//pkg/util/io:go_default_library", @@ -78,8 +90,24 @@ go_library( "//vendor/golang.org/x/sys/unix:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], + "@io_bazel_rules_go//go/platform:nacl": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:netbsd": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:openbsd": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:plan9": [ + "//pkg/util/nsenter:go_default_library", + ], + "@io_bazel_rules_go//go/platform:solaris": [ + "//pkg/util/nsenter:go_default_library", + ], "@io_bazel_rules_go//go/platform:windows": [ "//pkg/util/file:go_default_library", + "//pkg/util/nsenter:go_default_library", ], "//conditions:default": [], }), diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index 71660405f5e..19ba068ba39 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -48,15 +48,11 @@ type NsenterMounter struct { } // NewNsenterMounter creates a new mounter for kubelet that runs as a container. -// rootDir is location of /var/lib/kubelet directory (in case it's not on the -// default place). This directory must be available in the container -// on the same place as it's on the host. -func NewNsenterMounter(rootDir string) (*NsenterMounter, error) { - ne, err := nsenter.NewNsenter() - if err != nil { - return nil, err +func NewNsenterMounter(rootDir string, ne *nsenter.Nsenter) *NsenterMounter { + return &NsenterMounter{ + rootDir: rootDir, + ne: ne, } - return &NsenterMounter{ne: ne}, nil } // NsenterMounter implements mount.Interface diff --git a/pkg/util/mount/nsenter_mount_unsupported.go b/pkg/util/mount/nsenter_mount_unsupported.go index ef5cb5a107d..f417ba9bc15 100644 --- a/pkg/util/mount/nsenter_mount_unsupported.go +++ b/pkg/util/mount/nsenter_mount_unsupported.go @@ -21,12 +21,14 @@ package mount import ( "errors" "os" + + "k8s.io/kubernetes/pkg/util/nsenter" ) type NsenterMounter struct{} -func NewNsenterMounter(rootDir string) (*NsenterMounter, error) { - return &NsenterMounter{}, nil +func NewNsenterMounter(rootDir string, ne *nsenter.Nsenter) *NsenterMounter { + return &NsenterMounter{} } var _ = Interface(&NsenterMounter{}) From cb5eb25ec197e6bb97c4044effa5e2489b889651 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:26 +0200 Subject: [PATCH 9/9] Nsenter unit tests --- pkg/util/mount/BUILD | 1 + pkg/util/mount/nsenter_mount_test.go | 560 ++++++++++++++++++++++++ pkg/util/nsenter/BUILD | 19 +- pkg/util/nsenter/nsenter.go | 121 ++++- pkg/util/nsenter/nsenter_test.go | 311 +++++++++++++ pkg/util/nsenter/nsenter_unsupported.go | 8 +- 6 files changed, 995 insertions(+), 25 deletions(-) create mode 100644 pkg/util/nsenter/nsenter_test.go diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD index 40778e7c73b..6f2df9acae8 100644 --- a/pkg/util/mount/BUILD +++ b/pkg/util/mount/BUILD @@ -133,6 +133,7 @@ go_test( "//vendor/k8s.io/utils/exec/testing:go_default_library", ] + select({ "@io_bazel_rules_go//go/platform:linux": [ + "//pkg/util/nsenter:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/golang.org/x/sys/unix:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", diff --git a/pkg/util/mount/nsenter_mount_test.go b/pkg/util/mount/nsenter_mount_test.go index 2edb6febf97..c541a4cdf74 100644 --- a/pkg/util/mount/nsenter_mount_test.go +++ b/pkg/util/mount/nsenter_mount_test.go @@ -26,6 +26,7 @@ import ( "testing" "golang.org/x/sys/unix" + "k8s.io/kubernetes/pkg/util/nsenter" ) func TestParseFindMnt(t *testing.T) { @@ -147,3 +148,562 @@ func TestCheckDeviceInode(t *testing.T) { } } } + +func newFakeNsenterMounter(tmpdir string, t *testing.T) (mounter *NsenterMounter, rootfsPath string, varlibPath string, err error) { + rootfsPath = filepath.Join(tmpdir, "rootfs") + if err := os.Mkdir(rootfsPath, 0755); err != nil { + return nil, "", "", err + } + ne, err := nsenter.NewFakeNsenter(rootfsPath) + if err != nil { + return nil, "", "", err + } + + varlibPath = filepath.Join(tmpdir, "/var/lib/kubelet") + if err := os.MkdirAll(varlibPath, 0755); err != nil { + return nil, "", "", err + } + + return NewNsenterMounter(varlibPath, ne), rootfsPath, varlibPath, nil +} + +func TestNsenterExistsFile(t *testing.T) { + tests := []struct { + name string + prepare func(base, rootfs string) (string, error) + expectedOutput bool + expectError bool + }{ + { + name: "simple existing file", + prepare: func(base, rootfs string) (string, error) { + // On the host: /base/file + path := filepath.Join(base, "file") + if err := ioutil.WriteFile(path, []byte{}, 0644); err != nil { + return "", err + } + // In kubelet: /rootfs/base/file + if _, err := writeRootfsFile(rootfs, path, 0644); err != nil { + return "", err + } + return path, nil + }, + expectedOutput: true, + }, + { + name: "simple non-existing file", + prepare: func(base, rootfs string) (string, error) { + path := filepath.Join(base, "file") + return path, nil + }, + expectedOutput: false, + }, + { + name: "simple non-accessible file", + prepare: func(base, rootfs string) (string, error) { + // On the host: + // create /base/dir/file, then make the dir inaccessible + dir := filepath.Join(base, "dir") + if err := os.MkdirAll(dir, 0755); err != nil { + return "", err + } + path := filepath.Join(dir, "file") + if err := ioutil.WriteFile(path, []byte{}, 0); err != nil { + return "", err + } + if err := os.Chmod(dir, 0644); err != nil { + return "", err + } + + // In kubelet: do the same with /rootfs/base/dir/file + rootfsPath, err := writeRootfsFile(rootfs, path, 0777) + if err != nil { + return "", err + } + rootfsDir := filepath.Dir(rootfsPath) + if err := os.Chmod(rootfsDir, 0644); err != nil { + return "", err + } + + return path, nil + }, + expectedOutput: false, + expectError: true, + }, + { + name: "relative symlink to existing file", + prepare: func(base, rootfs string) (string, error) { + // On the host: /base/link -> file + file := filepath.Join(base, "file") + if err := ioutil.WriteFile(file, []byte{}, 0); err != nil { + return "", err + } + path := filepath.Join(base, "link") + if err := os.Symlink("file", path); err != nil { + return "", err + } + // In kubelet: /rootfs/base/file + if _, err := writeRootfsFile(rootfs, file, 0644); err != nil { + return "", err + } + return path, nil + }, + expectedOutput: true, + }, + { + name: "absolute symlink to existing file", + prepare: func(base, rootfs string) (string, error) { + // On the host: /base/link -> /base/file + file := filepath.Join(base, "file") + if err := ioutil.WriteFile(file, []byte{}, 0); err != nil { + return "", err + } + path := filepath.Join(base, "link") + if err := os.Symlink(file, path); err != nil { + return "", err + } + // In kubelet: /rootfs/base/file + if _, err := writeRootfsFile(rootfs, file, 0644); err != nil { + return "", err + } + + return path, nil + }, + expectedOutput: true, + }, + { + name: "relative symlink to non-existing file", + prepare: func(base, rootfs string) (string, error) { + path := filepath.Join(base, "link") + if err := os.Symlink("file", path); err != nil { + return "", err + } + return path, nil + }, + expectedOutput: false, + }, + { + name: "absolute symlink to non-existing file", + prepare: func(base, rootfs string) (string, error) { + file := filepath.Join(base, "file") + path := filepath.Join(base, "link") + if err := os.Symlink(file, path); err != nil { + return "", err + } + return path, nil + }, + expectedOutput: false, + }, + { + name: "symlink loop", + prepare: func(base, rootfs string) (string, error) { + path := filepath.Join(base, "link") + if err := os.Symlink(path, path); err != nil { + return "", err + } + return path, nil + }, + expectedOutput: false, + // TODO: realpath -m is not able to detect symlink loop. Should we care? + expectError: false, + }, + } + + for _, test := range tests { + tmpdir, err := ioutil.TempDir("", "nsenter-exists-file") + if err != nil { + t.Error(err) + continue + } + defer os.RemoveAll(tmpdir) + + testBase := filepath.Join(tmpdir, "base") + if err := os.Mkdir(testBase, 0755); err != nil { + t.Error(err) + continue + } + + mounter, rootfs, _, err := newFakeNsenterMounter(tmpdir, t) + if err != nil { + t.Error(err) + continue + } + + path, err := test.prepare(testBase, rootfs) + if err != nil { + t.Error(err) + continue + } + + out, err := mounter.ExistsPath(path) + if err != nil && !test.expectError { + t.Errorf("Test %q: unexpected error: %s", test.name, err) + } + if err == nil && test.expectError { + t.Errorf("Test %q: expected error, got none", test.name) + } + + if out != test.expectedOutput { + t.Errorf("Test %q: expected return value %v, got %v", test.name, test.expectedOutput, out) + } + } +} + +func TestNsenterGetMode(t *testing.T) { + tests := []struct { + name string + prepare func(base, rootfs string) (string, error) + expectedMode os.FileMode + expectError bool + }{ + { + name: "simple file", + prepare: func(base, rootfs string) (string, error) { + // On the host: /base/file + path := filepath.Join(base, "file") + if err := ioutil.WriteFile(path, []byte{}, 0644); err != nil { + return "", err + } + + // Prepare a different file as /rootfs/base/file (="the host + // visible from container") to check that NsEnterMounter calls + // stat on this file and not on /base/file. + // Visible from kubelet: /rootfs/base/file + if _, err := writeRootfsFile(rootfs, path, 0777); err != nil { + return "", err + } + + return path, nil + }, + expectedMode: 0777, + }, + { + name: "non-existing file", + prepare: func(base, rootfs string) (string, error) { + path := filepath.Join(base, "file") + return path, nil + }, + expectedMode: 0, + expectError: true, + }, + { + name: "absolute symlink to existing file", + prepare: func(base, rootfs string) (string, error) { + // On the host: /base/link -> /base/file + file := filepath.Join(base, "file") + if err := ioutil.WriteFile(file, []byte{}, 0644); err != nil { + return "", err + } + path := filepath.Join(base, "link") + if err := os.Symlink(file, path); err != nil { + return "", err + } + + // Visible from kubelet: + // /rootfs/base/file + if _, err := writeRootfsFile(rootfs, file, 0747); err != nil { + return "", err + } + + return path, nil + }, + expectedMode: 0747, + }, + { + name: "relative symlink to existing file", + prepare: func(base, rootfs string) (string, error) { + // On the host: /base/link -> file + file := filepath.Join(base, "file") + if err := ioutil.WriteFile(file, []byte{}, 0741); err != nil { + return "", err + } + path := filepath.Join(base, "link") + if err := os.Symlink("file", path); err != nil { + return "", err + } + + // Visible from kubelet: + // /rootfs/base/file + if _, err := writeRootfsFile(rootfs, file, 0647); err != nil { + return "", err + } + + return path, nil + }, + expectedMode: 0647, + }, + } + + for _, test := range tests { + tmpdir, err := ioutil.TempDir("", "nsenter-get-mode-") + if err != nil { + t.Error(err) + continue + } + defer os.RemoveAll(tmpdir) + + testBase := filepath.Join(tmpdir, "base") + if err := os.Mkdir(testBase, 0755); err != nil { + t.Error(err) + continue + } + + mounter, rootfs, _, err := newFakeNsenterMounter(tmpdir, t) + if err != nil { + t.Error(err) + continue + } + + path, err := test.prepare(testBase, rootfs) + if err != nil { + t.Error(err) + continue + } + + mode, err := mounter.GetMode(path) + if err != nil && !test.expectError { + t.Errorf("Test %q: unexpected error: %s", test.name, err) + } + if err == nil && test.expectError { + t.Errorf("Test %q: expected error, got none", test.name) + } + + if mode != test.expectedMode { + t.Errorf("Test %q: expected return value %v, got %v", test.name, test.expectedMode, mode) + } + } +} + +func writeRootfsFile(rootfs, path string, mode os.FileMode) (string, error) { + fullPath := filepath.Join(rootfs, path) + dir := filepath.Dir(fullPath) + if err := os.MkdirAll(dir, 0755); err != nil { + return "", err + } + if err := ioutil.WriteFile(fullPath, []byte{}, mode); err != nil { + return "", err + } + // Use chmod, io.WriteFile is affected by umask + if err := os.Chmod(fullPath, mode); err != nil { + return "", err + } + return fullPath, nil +} + +func TestNsenterSafeMakeDir(t *testing.T) { + tests := []struct { + name string + prepare func(base, rootfs, varlib string) (expectedDir string, err error) + subdir string + expectError bool + // If true, "base" directory for SafeMakeDir will be /var/lib/kubelet + baseIsVarLib bool + }{ + { + name: "simple directory", + // evaluated in base + subdir: "some/subdirectory/structure", + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // expected to be created in /roots/ + expectedDir = filepath.Join(rootfs, base, "some/subdirectory/structure") + return expectedDir, nil + }, + }, + { + name: "simple existing directory", + // evaluated in base + subdir: "some/subdirectory/structure", + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // On the host: directory exists + hostPath := filepath.Join(base, "some/subdirectory/structure") + if err := os.MkdirAll(hostPath, 0755); err != nil { + return "", err + } + // In rootfs: directory exists + kubeletPath := filepath.Join(rootfs, hostPath) + if err := os.MkdirAll(kubeletPath, 0755); err != nil { + return "", err + } + // expected to be created in /roots/ + expectedDir = kubeletPath + return expectedDir, nil + }, + }, + { + name: "absolute symlink into safe place", + // evaluated in base + subdir: "some/subdirectory/structure", + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // On the host: /base/other/subdirectory exists, /base/some is link to /base/other + hostPath := filepath.Join(base, "other/subdirectory") + if err := os.MkdirAll(hostPath, 0755); err != nil { + return "", err + } + somePath := filepath.Join(base, "some") + otherPath := filepath.Join(base, "other") + if err := os.Symlink(otherPath, somePath); err != nil { + return "", err + } + + // In rootfs: /base/other/subdirectory exists + kubeletPath := filepath.Join(rootfs, hostPath) + if err := os.MkdirAll(kubeletPath, 0755); err != nil { + return "", err + } + // expected 'structure' to be created + expectedDir = filepath.Join(rootfs, hostPath, "structure") + return expectedDir, nil + }, + }, + { + name: "relative symlink into safe place", + // evaluated in base + subdir: "some/subdirectory/structure", + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // On the host: /base/other/subdirectory exists, /base/some is link to other + hostPath := filepath.Join(base, "other/subdirectory") + if err := os.MkdirAll(hostPath, 0755); err != nil { + return "", err + } + somePath := filepath.Join(base, "some") + if err := os.Symlink("other", somePath); err != nil { + return "", err + } + + // In rootfs: /base/other/subdirectory exists + kubeletPath := filepath.Join(rootfs, hostPath) + if err := os.MkdirAll(kubeletPath, 0755); err != nil { + return "", err + } + // expected 'structure' to be created + expectedDir = filepath.Join(rootfs, hostPath, "structure") + return expectedDir, nil + }, + }, + { + name: "symlink into unsafe place", + // evaluated in base + subdir: "some/subdirectory/structure", + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // On the host: /base/some is link to /bin/other + somePath := filepath.Join(base, "some") + if err := os.Symlink("/bin", somePath); err != nil { + return "", err + } + return "", nil + }, + expectError: true, + }, + { + name: "simple directory in /var/lib/kubelet", + // evaluated in varlib + subdir: "some/subdirectory/structure", + baseIsVarLib: true, + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // expected to be created in /base/var/lib/kubelet, not in /rootfs! + expectedDir = filepath.Join(varlib, "some/subdirectory/structure") + return expectedDir, nil + }, + }, + { + name: "safe symlink in /var/lib/kubelet", + // evaluated in varlib + subdir: "some/subdirectory/structure", + baseIsVarLib: true, + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // On the host: /varlib/kubelet/other/subdirectory exists, /varlib/some is link to other + hostPath := filepath.Join(varlib, "other/subdirectory") + if err := os.MkdirAll(hostPath, 0755); err != nil { + return "", err + } + somePath := filepath.Join(varlib, "some") + if err := os.Symlink("other", somePath); err != nil { + return "", err + } + + // expected to be created in /base/var/lib/kubelet, not in /rootfs! + expectedDir = filepath.Join(varlib, "other/subdirectory/structure") + return expectedDir, nil + }, + }, + { + name: "unsafe symlink in /var/lib/kubelet", + // evaluated in varlib + subdir: "some/subdirectory/structure", + baseIsVarLib: true, + prepare: func(base, rootfs, varlib string) (expectedDir string, err error) { + // On the host: /varlib/some is link to /bin + somePath := filepath.Join(varlib, "some") + if err := os.Symlink("/bin", somePath); err != nil { + return "", err + } + + return "", nil + }, + expectError: true, + }, + } + for _, test := range tests { + tmpdir, err := ioutil.TempDir("", "nsenter-get-mode-") + if err != nil { + t.Error(err) + continue + } + defer os.RemoveAll(tmpdir) + + mounter, rootfs, varlib, err := newFakeNsenterMounter(tmpdir, t) + if err != nil { + t.Error(err) + continue + } + // Prepare base directory for the test + testBase := filepath.Join(tmpdir, "base") + if err := os.Mkdir(testBase, 0755); err != nil { + t.Error(err) + continue + } + // Prepare base directory also in /rootfs + rootfsBase := filepath.Join(rootfs, testBase) + if err := os.MkdirAll(rootfsBase, 0755); err != nil { + t.Error(err) + continue + } + + expectedDir := "" + if test.prepare != nil { + expectedDir, err = test.prepare(testBase, rootfs, varlib) + if err != nil { + t.Error(err) + continue + } + } + + if test.baseIsVarLib { + // use /var/lib/kubelet as the test base so we can test creating + // subdirs there directly in /var/lib/kubenet and not in + // /rootfs/var/lib/kubelet + testBase = varlib + } + + err = mounter.SafeMakeDir(test.subdir, testBase, 0755) + if err != nil && !test.expectError { + t.Errorf("Test %q: unexpected error: %s", test.name, err) + } + if test.expectError { + if err == nil { + t.Errorf("Test %q: expected error, got none", test.name) + } else { + if !strings.Contains(err.Error(), "is outside of allowed base") { + t.Errorf("Test %q: expected error to contain \"is outside of allowed base\", got this one instead: %s", test.name, err) + } + } + } + + if expectedDir != "" { + _, err := os.Stat(expectedDir) + if err != nil { + t.Errorf("Test %q: expected %q to exist, got error: %s", test.name, expectedDir, err) + } + } + } +} diff --git a/pkg/util/nsenter/BUILD b/pkg/util/nsenter/BUILD index 988fef01b59..286b8882773 100644 --- a/pkg/util/nsenter/BUILD +++ b/pkg/util/nsenter/BUILD @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -92,3 +92,20 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = select({ + "@io_bazel_rules_go//go/platform:linux": [ + "nsenter_test.go", + ], + "//conditions:default": [], + }), + embed = [":go_default_library"], + deps = select({ + "@io_bazel_rules_go//go/platform:linux": [ + "//vendor/k8s.io/utils/exec:go_default_library", + ], + "//conditions:default": [], + }), +) diff --git a/pkg/util/nsenter/nsenter.go b/pkg/util/nsenter/nsenter.go index 477950476b1..e928a57ac9f 100644 --- a/pkg/util/nsenter/nsenter.go +++ b/pkg/util/nsenter/nsenter.go @@ -19,6 +19,8 @@ limitations under the License. package nsenter import ( + "context" + "errors" "fmt" "os" "path/filepath" @@ -30,9 +32,11 @@ import ( ) const ( - hostRootFsPath = "/rootfs" - // hostProcMountNsPath is the default mount namespace for rootfs - hostProcMountNsPath = "/rootfs/proc/1/ns/mnt" + // DefaultHostRootFsPath is path to host's filesystem mounted into container + // with kubelet. + DefaultHostRootFsPath = "/rootfs" + // mountNsPath is the default mount namespace of the host + mountNsPath = "/proc/1/ns/mnt" // nsenterPath is the default nsenter command nsenterPath = "nsenter" ) @@ -65,30 +69,46 @@ const ( type Nsenter struct { // a map of commands to their paths on the host filesystem paths map[string]string + + // Path to the host filesystem, typically "/rootfs". Used only for testing. + hostRootFsPath string + + // Exec implementation, used only for testing + executor exec.Interface } // NewNsenter constructs a new instance of Nsenter -func NewNsenter() (*Nsenter, error) { +func NewNsenter(hostRootFsPath string, executor exec.Interface) (*Nsenter, error) { ne := &Nsenter{ - paths: map[string]string{ - "mount": "", - "findmnt": "", - "umount": "", - "systemd-run": "", - "stat": "", - "touch": "", - "mkdir": "", - "ls": "", - "sh": "", - "chmod": "", - }, + hostRootFsPath: hostRootFsPath, + executor: executor, + } + if err := ne.initPaths(); err != nil { + return nil, err + } + return ne, nil +} + +func (ne *Nsenter) initPaths() error { + ne.paths = map[string]string{} + binaries := []string{ + "mount", + "findmnt", + "umount", + "systemd-run", + "stat", + "touch", + "mkdir", + "sh", + "chmod", + "realpath", } // search for the required commands in other locations besides /usr/bin - for binary := range ne.paths { + for _, binary := range binaries { // check for binary under the following directories for _, path := range []string{"/", "/bin", "/usr/sbin", "/usr/bin"} { binPath := filepath.Join(path, binary) - if _, err := os.Stat(filepath.Join(hostRootFsPath, binPath)); err != nil { + if _, err := os.Stat(filepath.Join(ne.hostRootFsPath, binPath)); err != nil { continue } ne.paths[binary] = binPath @@ -96,19 +116,19 @@ func NewNsenter() (*Nsenter, error) { } // systemd-run is optional, bailout if we don't find any of the other binaries if ne.paths[binary] == "" && binary != "systemd-run" { - return nil, fmt.Errorf("unable to find %v", binary) + return fmt.Errorf("unable to find %v", binary) } } - return ne, nil + return nil } // Exec executes nsenter commands in hostProcMountNsPath mount namespace func (ne *Nsenter) Exec(cmd string, args []string) exec.Cmd { + hostProcMountNsPath := filepath.Join(ne.hostRootFsPath, mountNsPath) fullArgs := append([]string{fmt.Sprintf("--mount=%s", hostProcMountNsPath), "--"}, append([]string{ne.AbsHostPath(cmd)}, args...)...) glog.V(5).Infof("Running nsenter command: %v %v", nsenterPath, fullArgs) - exec := exec.New() - return exec.Command(nsenterPath, fullArgs...) + return ne.executor.Command(nsenterPath, fullArgs...) } // AbsHostPath returns the absolute runnable path for a specified command @@ -136,6 +156,9 @@ func (ne *Nsenter) SupportsSystemd() (string, bool) { // non/existing/directory does not exist // -> It resolves symlinks in /mnt/volume to say /mnt/foo and returns // /mnt/foo/non/existing/directory. +// +// BEWARE! EvalSymlinks is not able to detect symlink looks with mustExist=false! +// If /tmp/link is symlink to /tmp/link, EvalSymlinks(/tmp/link/foo) returns /tmp/link/foo. func (ne *Nsenter) EvalSymlinks(pathname string, mustExist bool) (string, error) { var args []string if mustExist { @@ -157,5 +180,57 @@ func (ne *Nsenter) EvalSymlinks(pathname string, mustExist bool) (string, error) // kubelet. It is recommended to resolve symlinks on the host by EvalSymlinks // before calling this function func (ne *Nsenter) KubeletPath(pathname string) string { - return filepath.Join(hostRootFsPath, pathname) + return filepath.Join(ne.hostRootFsPath, pathname) } + +// NewFakeNsenter returns a Nsenter that does not run "nsenter --mount=... --", +// but runs everything in the same mount namespace as the unit test binary. +// rootfsPath is supposed to be a symlink, e.g. /tmp/xyz/rootfs -> /. +// This fake Nsenter is enough for most operations, e.g. to resolve symlinks, +// but it's not enough to call /bin/mount - unit tests don't run as root. +func NewFakeNsenter(rootfsPath string) (*Nsenter, error) { + executor := &fakeExec{ + rootfsPath: rootfsPath, + } + // prepare /rootfs/bin, usr/bin and usr/sbin + bin := filepath.Join(rootfsPath, "bin") + if err := os.Symlink("/bin", bin); err != nil { + return nil, err + } + + usr := filepath.Join(rootfsPath, "usr") + if err := os.Mkdir(usr, 0755); err != nil { + return nil, err + } + usrbin := filepath.Join(usr, "bin") + if err := os.Symlink("/usr/bin", usrbin); err != nil { + return nil, err + } + usrsbin := filepath.Join(usr, "sbin") + if err := os.Symlink("/usr/sbin", usrsbin); err != nil { + return nil, err + } + + return NewNsenter(rootfsPath, executor) +} + +type fakeExec struct { + rootfsPath string +} + +func (f fakeExec) Command(cmd string, args ...string) exec.Cmd { + // This will intentionaly panic if Nsenter does not provide enough arguments. + realCmd := args[2] + realArgs := args[3:] + return exec.New().Command(realCmd, realArgs...) +} + +func (fakeExec) LookPath(file string) (string, error) { + return "", errors.New("not implemented") +} + +func (fakeExec) CommandContext(ctx context.Context, cmd string, args ...string) exec.Cmd { + return nil +} + +var _ exec.Interface = fakeExec{} diff --git a/pkg/util/nsenter/nsenter_test.go b/pkg/util/nsenter/nsenter_test.go new file mode 100644 index 00000000000..3158a55bbec --- /dev/null +++ b/pkg/util/nsenter/nsenter_test.go @@ -0,0 +1,311 @@ +// +build linux + +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nsenter + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "k8s.io/utils/exec" +) + +func TestExec(t *testing.T) { + tests := []struct { + name string + command string + args []string + expectedOutput string + expectError bool + }{ + { + name: "simple command", + command: "echo", + args: []string{"hello", "world"}, + expectedOutput: "hello world\n", + }, + { + name: "nozero exit code", + command: "false", + expectError: true, + }, + } + + executor := fakeExec{ + rootfsPath: "/rootfs", + } + for _, test := range tests { + ns := Nsenter{ + hostRootFsPath: "/rootfs", + executor: executor, + } + cmd := ns.Exec(test.command, test.args) + outBytes, err := cmd.CombinedOutput() + out := string(outBytes) + if err != nil && !test.expectError { + t.Errorf("Test %q: unexpected error: %s", test.name, err) + } + if err == nil && test.expectError { + t.Errorf("Test %q: expected error, got none", test.name) + } + if test.expectedOutput != out { + t.Errorf("test %q: expected output %q, got %q", test.name, test.expectedOutput, out) + } + } +} + +func TestKubeletPath(t *testing.T) { + tests := []struct { + rootfs string + hostpath string + expectedKubeletPath string + }{ + { + // simple join + "/rootfs", + "/some/path", + "/rootfs/some/path", + }, + { + // squash slashes + "/rootfs/", + "//some/path", + "/rootfs/some/path", + }, + } + + for _, test := range tests { + ns := Nsenter{ + hostRootFsPath: test.rootfs, + } + out := ns.KubeletPath(test.hostpath) + if out != test.expectedKubeletPath { + t.Errorf("Expected path %q, got %q", test.expectedKubeletPath, out) + } + + } +} + +func TestEvalSymlinks(t *testing.T) { + tests := []struct { + name string + mustExist bool + prepare func(tmpdir string) (src string, expectedDst string, err error) + expectError bool + }{ + { + name: "simple file /src", + mustExist: true, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + src = filepath.Join(tmpdir, "src") + err = ioutil.WriteFile(src, []byte{}, 0644) + return src, src, err + }, + }, + { + name: "non-existing file /src", + mustExist: true, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + src = filepath.Join(tmpdir, "src") + return src, "", nil + }, + expectError: true, + }, + { + name: "non-existing file /src/ with mustExist=false", + mustExist: false, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + src = filepath.Join(tmpdir, "src") + return src, src, nil + }, + }, + { + name: "non-existing file /existing/path/src with mustExist=false with existing directories", + mustExist: false, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + src = filepath.Join(tmpdir, "existing/path") + if err := os.MkdirAll(src, 0755); err != nil { + return "", "", err + } + src = filepath.Join(src, "src") + return src, src, nil + }, + }, + { + name: "simple symlink /src -> /dst", + mustExist: false, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + dst := filepath.Join(tmpdir, "dst") + if err = ioutil.WriteFile(dst, []byte{}, 0644); err != nil { + return "", "", err + } + src = filepath.Join(tmpdir, "src") + err = os.Symlink(dst, src) + return src, dst, err + }, + }, + { + name: "dangling symlink /src -> /non-existing-path", + mustExist: true, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + dst := filepath.Join(tmpdir, "non-existing-path") + src = filepath.Join(tmpdir, "src") + err = os.Symlink(dst, src) + return src, "", err + }, + expectError: true, + }, + { + name: "dangling symlink /src -> /non-existing-path with mustExist=false", + mustExist: false, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + dst := filepath.Join(tmpdir, "non-existing-path") + src = filepath.Join(tmpdir, "src") + err = os.Symlink(dst, src) + return src, dst, err + }, + }, + { + name: "symlink to directory /src/file, where /src is link to /dst", + mustExist: true, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + dst := filepath.Join(tmpdir, "dst") + if err = os.Mkdir(dst, 0755); err != nil { + return "", "", err + } + dstFile := filepath.Join(dst, "file") + if err = ioutil.WriteFile(dstFile, []byte{}, 0644); err != nil { + return "", "", err + } + + src = filepath.Join(tmpdir, "src") + if err = os.Symlink(dst, src); err != nil { + return "", "", err + } + srcFile := filepath.Join(src, "file") + return srcFile, dstFile, nil + }, + }, + { + name: "symlink to non-existing directory: /src/file, where /src is link to /dst and dst does not exist", + mustExist: true, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + dst := filepath.Join(tmpdir, "dst") + + src = filepath.Join(tmpdir, "src") + if err = os.Symlink(dst, src); err != nil { + return "", "", err + } + srcFile := filepath.Join(src, "file") + return srcFile, "", nil + }, + expectError: true, + }, + { + name: "symlink to non-existing directory: /src/file, where /src is link to /dst and dst does not exist with mustExist=false", + mustExist: false, + prepare: func(tmpdir string) (src string, expectedDst string, err error) { + dst := filepath.Join(tmpdir, "dst") + dstFile := filepath.Join(dst, "file") + + src = filepath.Join(tmpdir, "src") + if err = os.Symlink(dst, src); err != nil { + return "", "", err + } + srcFile := filepath.Join(src, "file") + return srcFile, dstFile, nil + }, + }, + } + + for _, test := range tests { + ns := Nsenter{ + hostRootFsPath: "/rootfs", + executor: fakeExec{ + rootfsPath: "/rootfs", + }, + } + + tmpdir, err := ioutil.TempDir("", "nsenter-hostpath-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + + src, expectedDst, err := test.prepare(tmpdir) + if err != nil { + t.Error(err) + continue + } + + dst, err := ns.EvalSymlinks(src, test.mustExist) + if err != nil && !test.expectError { + t.Errorf("Test %q: unexpected error: %s", test.name, err) + } + if err == nil && test.expectError { + t.Errorf("Test %q: expected error, got none", test.name) + } + if dst != expectedDst { + t.Errorf("Test %q: expected destination %q, got %q", test.name, expectedDst, dst) + } + } +} + +func TestNewNsenter(t *testing.T) { + // Create a symlink /tmp/xyz/rootfs -> / and use it as rootfs path + // It should resolve all binaries correctly, the test runs on Linux + + tmpdir, err := ioutil.TempDir("", "nsenter-hostpath-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + + rootfs := filepath.Join(tmpdir, "rootfs") + if err = os.Symlink("/", rootfs); err != nil { + t.Fatal(err) + } + + _, err = NewNsenter(rootfs, exec.New()) + if err != nil { + t.Errorf("Error: %s", err) + } +} + +func TestNewNsenterError(t *testing.T) { + // Create empty dir /tmp/xyz/rootfs and use it as rootfs path + // It should resolve all binaries correctly, the test runs on Linux + + tmpdir, err := ioutil.TempDir("", "nsenter-hostpath-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + + rootfs := filepath.Join(tmpdir, "rootfs") + if err = os.MkdirAll(rootfs, 0755); err != nil { + t.Fatal(err) + } + + _, err = NewNsenter(rootfs, exec.New()) + if err == nil { + t.Errorf("Expected error, got none") + } +} diff --git a/pkg/util/nsenter/nsenter_unsupported.go b/pkg/util/nsenter/nsenter_unsupported.go index 842cf046731..0618b9da469 100644 --- a/pkg/util/nsenter/nsenter_unsupported.go +++ b/pkg/util/nsenter/nsenter_unsupported.go @@ -22,6 +22,12 @@ import ( "k8s.io/utils/exec" ) +const ( + // DefaultHostRootFsPath is path to host's filesystem mounted into container + // with kubelet. + DefaultHostRootFsPath = "/rootfs" +) + // Nsenter is part of experimental support for running the kubelet // in a container. type Nsenter struct { @@ -30,7 +36,7 @@ type Nsenter struct { } // NewNsenter constructs a new instance of Nsenter -func NewNsenter() (*Nsenter, error) { +func NewNsenter(hostRootFsPath string, executor exec.Interface) (*Nsenter, error) { return &Nsenter{}, nil }