Refactor subpath out of pkg/util/mount

cmd/kubelet/app/BUILD

@@ -108,6 +108,7 @@ go_library(
         "//pkg/volume/scaleio:go_default_library",
         "//pkg/volume/secret:go_default_library",
         "//pkg/volume/storageos:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//pkg/volume/vsphere_volume:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",

cmd/kubelet/app/server.go

@@ -95,6 +95,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/rlimit"
 	"k8s.io/kubernetes/pkg/version"
 	"k8s.io/kubernetes/pkg/version/verflag"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	nodeapiclientset "k8s.io/node-api/pkg/client/clientset/versioned"
 	"k8s.io/utils/exec"
 	"k8s.io/utils/nsenter"
@@ -364,6 +365,7 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err
 	}
 
 	mounter := mount.New(s.ExperimentalMounterPath)
+	subpather := subpath.New(mounter)
 	var pluginRunner = exec.New()
 	if s.Containerized {
 		klog.V(2).Info("Running kubelet in containerized mode")
@@ -372,6 +374,8 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err
 			return nil, err
 		}
 		mounter = mount.NewNsenterMounter(s.RootDirectory, ne)
+		// NSenter only valid on Linux
+		subpather = subpath.NewNSEnter(mounter, ne, s.RootDirectory)
 		// an exec interface which can use nsenter for flex plugin calls
 		pluginRunner, err = nsenter.NewNsenter(nsenter.DefaultHostRootFsPath, exec.New())
 		if err != nil {
@@ -399,6 +403,7 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err
 		CSIClient:           nil,
 		EventClient:         nil,
 		Mounter:             mounter,
+		Subpather:           subpather,
 		OOMAdjuster:         oom.NewOOMAdjuster(),
 		OSInterface:         kubecontainer.RealOS{},
 		VolumePlugins:       ProbeVolumePlugins(),

pkg/controller/.import-restrictions

@@ -167,6 +167,7 @@
       "AllowedPrefixes": [
         "k8s.io/kubernetes/pkg/api/legacyscheme",
         "k8s.io/kubernetes/pkg/api/v1/endpoints",
+        "k8s.io/kubernetes/pkg/api/v1/node",
         "k8s.io/kubernetes/pkg/api/v1/pod",
         "k8s.io/kubernetes/pkg/apis/apps/v1",
         "k8s.io/kubernetes/pkg/apis/autoscaling",
@@ -241,6 +242,7 @@
         "k8s.io/kubernetes/pkg/volume/util",
         "k8s.io/kubernetes/pkg/volume/util/operationexecutor",
         "k8s.io/kubernetes/pkg/volume/util/recyclerclient",
+        "k8s.io/kubernetes/pkg/volume/util/subpath",
         "k8s.io/kubernetes/pkg/volume/util/types",
         "k8s.io/kubernetes/pkg/volume/util/volumepathhandler",
         "k8s.io/kubernetes/pkg/api/service",

pkg/controller/volume/attachdetach/BUILD

@@ -22,6 +22,7 @@ go_library(
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//pkg/volume/util/operationexecutor:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//pkg/volume/util/volumepathhandler:go_default_library",
         "//staging/src/k8s.io/api/authentication/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -52,6 +52,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/operationexecutor"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 )
 
@@ -768,3 +769,8 @@ func (adc *attachDetachController) GetEventRecorder() record.EventRecorder {
 func (adc *attachDetachController) GetCSIClient() csiclient.Interface {
 	return adc.csiClient
 }
+
+func (adc *attachDetachController) GetSubpather() subpath.Interface {
+	// Subpaths not needed in attachdetach controller
+	return nil
+}

pkg/controller/volume/expand/BUILD

@@ -19,6 +19,7 @@ go_library(
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//pkg/volume/util/operationexecutor:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//pkg/volume/util/volumepathhandler:go_default_library",
         "//staging/src/k8s.io/api/authentication/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",

pkg/controller/volume/expand/expand_controller.go

@@ -46,6 +46,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/operationexecutor"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 )
 
@@ -339,3 +340,8 @@ func (expc *expandController) GetCSIClient() csiclientset.Interface {
 	// No volume plugin in expand controller needs csi.storage.k8s.io
 	return nil
 }
+
+func (expc *expandController) GetSubpather() subpath.Interface {
+	// not needed for expand controller
+	return nil
+}

pkg/controller/volume/persistentvolume/BUILD

@@ -33,6 +33,7 @@ go_library(
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//pkg/volume/util/recyclerclient:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//staging/src/k8s.io/api/authentication/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/storage/v1:go_default_library",

pkg/controller/volume/persistentvolume/volume_host.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	vol "k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 // VolumeHost interface implementation for PersistentVolumeController.
@@ -136,3 +137,8 @@ func (ctrl *PersistentVolumeController) GetCSIClient() csiclientset.Interface {
 	// No volume plugin needs csi.storage.k8s.io client in PV controller.
 	return nil
 }
+
+func (ctrl *PersistentVolumeController) GetSubpather() subpath.Interface {
+	// No volume plugin needs Subpaths in PV controller.
+	return nil
+}

pkg/kubelet/BUILD

@@ -112,6 +112,7 @@ go_library(
         "//pkg/volume:go_default_library",
         "//pkg/volume/csi:go_default_library",
         "//pkg/volume/util:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//pkg/volume/util/types:go_default_library",
         "//pkg/volume/util/volumepathhandler:go_default_library",
         "//pkg/volume/validation:go_default_library",
@@ -220,6 +221,7 @@ go_test(
         "//pkg/volume/host_path:go_default_library",
         "//pkg/volume/testing:go_default_library",
         "//pkg/volume/util:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",

pkg/kubelet/kubelet.go

@@ -113,6 +113,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/oom"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/csi"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	nodeapiclientset "k8s.io/node-api/pkg/client/clientset/versioned"
 	utilexec "k8s.io/utils/exec"
 	"k8s.io/utils/integer"
@@ -255,6 +256,7 @@ type Dependencies struct {
 	OSInterface             kubecontainer.OSInterface
 	PodConfig               *config.PodConfig
 	Recorder                record.EventRecorder
+	Subpather               subpath.Interface
 	VolumePlugins           []volume.VolumePlugin
 	DynamicPluginProber     volume.DynamicPluginProber
 	TLSOptions              *server.TLSOptions
@@ -519,6 +521,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		cgroupsPerQOS:                           kubeCfg.CgroupsPerQOS,
 		cgroupRoot:                              kubeCfg.CgroupRoot,
 		mounter:                                 kubeDeps.Mounter,
+		subpather:                               kubeDeps.Subpather,
 		maxPods:                                 int(kubeCfg.MaxPods),
 		podsPerCore:                             int(kubeCfg.PodsPerCore),
 		syncLoopMonitor:                         atomic.Value{},
@@ -1099,6 +1102,9 @@ type Kubelet struct {
 	// Mounter to use for volumes.
 	mounter mount.Interface
 
+	// subpather to execute subpath actions
+	subpather subpath.Interface
+
 	// Manager of non-Runtime containers.
 	containerManager cm.ContainerManager
 

pkg/kubelet/kubelet_pods.go

@@ -61,6 +61,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
 	mountutil "k8s.io/kubernetes/pkg/util/mount"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	volumevalidation "k8s.io/kubernetes/pkg/volume/validation"
 	"k8s.io/kubernetes/third_party/forked/golang/expansion"
@@ -127,7 +128,7 @@ func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVol
 }
 
 // makeMounts determines the mount points for the given container.
-func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface, expandEnvs []kubecontainer.EnvVar) ([]kubecontainer.Mount, func(), error) {
+func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface, subpather subpath.Interface, expandEnvs []kubecontainer.EnvVar) ([]kubecontainer.Mount, func(), error) {
 	// Kubernetes only mounts on /etc/hosts if:
 	// - container is not an infrastructure (pause) container
 	// - container is not already mounting on /etc/hosts
@@ -206,13 +207,13 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
 				if err != nil {
 					return nil, cleanupAction, err
 				}
-				if err := mounter.SafeMakeDir(subPath, volumePath, perm); err != nil {
+				if err := subpather.SafeMakeDir(subPath, volumePath, perm); err != nil {
 					// Don't pass detailed error back to the user because it could give information about host filesystem
 					klog.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{
+			hostPath, cleanupAction, err = subpather.PrepareSafeSubpath(subpath.Subpath{
 				VolumeMountIndex: i,
 				Path:             hostPath,
 				VolumeName:       vol.InnerVolumeSpecName,
@@ -464,7 +465,7 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Contai
 	}
 	opts.Envs = append(opts.Envs, envs...)
 
-	mounts, cleanupAction, err := makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, volumes, kl.mounter, opts.Envs)
+	mounts, cleanupAction, err := makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, volumes, kl.mounter, kl.subpather, opts.Envs)
 	if err != nil {
 		return nil, cleanupAction, err
 	}

pkg/kubelet/kubelet_pods_linux_test.go

@@ -29,6 +29,7 @@ import (
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/util/mount"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 func TestMakeMounts(t *testing.T) {
@@ -241,13 +242,14 @@ func TestMakeMounts(t *testing.T) {
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
 			fm := &mount.FakeMounter{}
+			fsp := &subpath.FakeSubpath{}
 			pod := v1.Pod{
 				Spec: v1.PodSpec{
 					HostNetwork: true,
 				},
 			}
 
-			mounts, _, err := makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm, nil)
+			mounts, _, err := makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm, fsp, nil)
 
 			// validate only the error if we expect an error
 			if tc.expectErr {

pkg/kubelet/kubelet_pods_test.go

@@ -49,10 +49,12 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/server/portforward"
 	"k8s.io/kubernetes/pkg/kubelet/server/remotecommand"
 	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 func TestDisabledSubpath(t *testing.T) {
 	fm := &mount.FakeMounter{}
+	fsp := &subpath.FakeSubpath{}
 	pod := v1.Pod{
 		Spec: v1.PodSpec{
 			HostNetwork: true,
@@ -95,7 +97,7 @@ func TestDisabledSubpath(t *testing.T) {
 
 	defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, false)()
 	for name, test := range cases {
-		_, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm, nil)
+		_, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm, fsp, nil)
 		if err != nil && !test.expectError {
 			t.Errorf("test %v failed: %v", name, err)
 		}

pkg/kubelet/kubelet_pods_windows_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/api/core/v1"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 func TestMakeMountsWindows(t *testing.T) {
@@ -82,7 +83,8 @@ func TestMakeMountsWindows(t *testing.T) {
 	}
 
 	fm := &mount.FakeMounter{}
-	mounts, _, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes, fm, nil)
+	fsp := &subpath.FakeSubpath{}
+	mounts, _, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes, fm, fsp, nil)
 
 	expectedMounts := []kubecontainer.Mount{
 		{

pkg/kubelet/kubelet_test.go

@@ -75,6 +75,7 @@ import (
 	_ "k8s.io/kubernetes/pkg/volume/host_path"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
 	"k8s.io/kubernetes/pkg/volume/util"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 func init() {
@@ -176,6 +177,7 @@ func newTestKubeletWithImageList(
 	kubelet.heartbeatClient = fakeKubeClient
 	kubelet.os = &containertest.FakeOS{}
 	kubelet.mounter = &mount.FakeMounter{}
+	kubelet.subpather = &subpath.FakeSubpath{}
 
 	kubelet.hostname = testKubeletHostname
 	kubelet.nodeName = types.NodeName(testKubeletHostname)

pkg/kubelet/volume_host.go

@@ -40,6 +40,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/util"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 // NewInitializedVolumePluginMgr returns a new instance of
@@ -126,6 +127,10 @@ func (kvh *kubeletVolumeHost) GetCSIClient() csiclientset.Interface {
 	return kvh.kubelet.csiClient
 }
 
+func (kvh *kubeletVolumeHost) GetSubpather() subpath.Interface {
+	return kvh.kubelet.subpather
+}
+
 func (kvh *kubeletVolumeHost) NewWrapperMounter(
 	volName string,
 	spec volume.Spec,

pkg/kubemark/BUILD

@@ -36,6 +36,7 @@ go_library(
         "//pkg/volume/emptydir:go_default_library",
         "//pkg/volume/projected:go_default_library",
         "//pkg/volume/secret:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",

pkg/kubemark/hollow_kubelet.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume/emptydir"
 	"k8s.io/kubernetes/pkg/volume/projected"
 	"k8s.io/kubernetes/pkg/volume/secret"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	"k8s.io/kubernetes/test/utils"
 
 	"k8s.io/klog"
@@ -78,6 +79,7 @@ func NewHollowKubelet(
 		TLSOptions:         nil,
 		OOMAdjuster:        oom.NewFakeOOMAdjuster(),
 		Mounter:            mount.New("" /* default mount path */),
+		Subpather:          &subpath.FakeSubpath{},
 	}
 
 	return &HollowKubelet{

pkg/util/mount/BUILD

@@ -80,8 +80,6 @@ go_test(
         "//vendor/k8s.io/utils/exec/testing:go_default_library",
     ] + select({
         "@io_bazel_rules_go//go/platform:linux": [
-            "//vendor/golang.org/x/sys/unix:go_default_library",
-            "//vendor/k8s.io/klog:go_default_library",
             "//vendor/k8s.io/utils/exec:go_default_library",
             "//vendor/k8s.io/utils/nsenter:go_default_library",
         ],

pkg/util/mount/exec_mount.go

@@ -144,18 +144,6 @@ func (m *execMounter) EvalHostSymlinks(pathname string) (string, error) {
 	return m.wrappedMounter.EvalHostSymlinks(pathname)
 }
 
-func (m *execMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	return m.wrappedMounter.PrepareSafeSubpath(subPath)
-}
-
-func (m *execMounter) CleanSubPaths(podDir string, volumeName string) error {
-	return m.wrappedMounter.CleanSubPaths(podDir, volumeName)
-}
-
-func (m *execMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return m.wrappedMounter.SafeMakeDir(pathname, base, perm)
-}
-
 func (m *execMounter) GetMountRefs(pathname string) ([]string, error) {
 	return m.wrappedMounter.GetMountRefs(pathname)
 }

pkg/util/mount/exec_mount_unsupported.go

@@ -91,18 +91,6 @@ func (m *execMounter) EvalHostSymlinks(pathname string) (string, error) {
 	return "", errors.New("not implemented")
 }
 
-func (mounter *execMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	return subPath.Path, nil, nil
-}
-
-func (mounter *execMounter) CleanSubPaths(podDir string, volumeName string) error {
-	return nil
-}
-
-func (mounter *execMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return nil
-}
-
 func (mounter *execMounter) GetMountRefs(pathname string) ([]string, error) {
 	return nil, errors.New("not implemented")
 }

pkg/util/mount/fake.go

@@ -220,17 +220,6 @@ func (f *FakeMounter) EvalHostSymlinks(pathname string) (string, error) {
 	return pathname, nil
 }
 
-func (f *FakeMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	return subPath.Path, nil, nil
-}
-
-func (f *FakeMounter) CleanSubPaths(podDir string, volumeName string) error {
-	return nil
-}
-func (mounter *FakeMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return nil
-}
-
 func (f *FakeMounter) GetMountRefs(pathname string) ([]string, error) {
 	realpath, err := filepath.EvalSymlinks(pathname)
 	if err != nil {

pkg/util/mount/mount.go

@@ -84,35 +84,12 @@ 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 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)
 	// EvalHostSymlinks returns the path name after evaluating symlinks.
 	// Will operate in the host mount namespace if kubelet is running in a container.
 	EvalHostSymlinks(pathname string) (string, error)
-	// CleanSubPaths removes any bind-mounts created by PrepareSafeSubpath in given
-	// pod volume directory.
-	CleanSubPaths(podDir string, volumeName string) error
-	// PrepareSafeSubpath does everything that's necessary to prepare a subPath
-	// that's 1) inside given volumePath and 2) immutable after this call.
-	//
-	// newHostPath - location of prepared subPath. It should be used instead of
-	// hostName when running the container.
-	// cleanupAction - action to run when the container is running or it failed to start.
-	//
-	// CleanupAction must be called immediately after the container with given
-	// subpath starts. On the other hand, Interface.CleanSubPaths must be called
-	// when the pod finishes.
-	PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error)
 	// GetMountRefs finds all mount references to the path, returns a
 	// list of paths. Path could be a mountpoint path, device or a normal
 	// directory (for bind mount).
@@ -355,15 +332,15 @@ func PathWithinBase(fullPath, basePath string) bool {
 	if err != nil {
 		return false
 	}
-	if startsWithBackstep(rel) {
+	if StartsWithBackstep(rel) {
 		// Needed to escape the base path
 		return false
 	}
 	return true
 }
 
-// startsWithBackstep checks if the given path starts with a backstep segment
-func startsWithBackstep(rel string) bool {
+// StartsWithBackstep checks if the given path starts with a backstep segment
+func StartsWithBackstep(rel string) bool {
 	// normalize to / and check for ../
 	return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../")
 }

pkg/util/mount/mount_unsupported.go

@@ -110,18 +110,6 @@ func (mounter *Mounter) EvalHostSymlinks(pathname string) (string, error) {
 	return "", unsupportedErr
 }
 
-func (mounter *Mounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	return subPath.Path, nil, unsupportedErr
-}
-
-func (mounter *Mounter) CleanSubPaths(podDir string, volumeName string) error {
-	return unsupportedErr
-}
-
-func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return unsupportedErr
-}
-
 func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
 	return nil, errors.New("not implemented")
 }

pkg/util/mount/mount_windows.go

@@ -26,7 +26,6 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"syscall"
 
 	"k8s.io/klog"
 
@@ -279,123 +278,6 @@ func (mounter *Mounter) EvalHostSymlinks(pathname string) (string, error) {
 	return filepath.EvalSymlinks(pathname)
 }
 
-// check whether hostPath is within volume path
-// this func will lock all intermediate subpath directories, need to close handle outside of this func after container started
-func lockAndCheckSubPath(volumePath, hostPath string) ([]uintptr, error) {
-	if len(volumePath) == 0 || len(hostPath) == 0 {
-		return []uintptr{}, nil
-	}
-
-	finalSubPath, err := filepath.EvalSymlinks(hostPath)
-	if err != nil {
-		return []uintptr{}, fmt.Errorf("cannot read link %s: %s", hostPath, err)
-	}
-	finalVolumePath, err := filepath.EvalSymlinks(volumePath)
-	if err != nil {
-		return []uintptr{}, fmt.Errorf("cannot read link %s: %s", volumePath, err)
-	}
-
-	return lockAndCheckSubPathWithoutSymlink(finalVolumePath, finalSubPath)
-}
-
-// lock all intermediate subPath directories and check they are all within volumePath
-// volumePath & subPath should not contain any symlink, otherwise it will return error
-func lockAndCheckSubPathWithoutSymlink(volumePath, subPath string) ([]uintptr, error) {
-	if len(volumePath) == 0 || len(subPath) == 0 {
-		return []uintptr{}, nil
-	}
-
-	// get relative path to volumePath
-	relSubPath, err := filepath.Rel(volumePath, subPath)
-	if err != nil {
-		return []uintptr{}, fmt.Errorf("Rel(%s, %s) error: %v", volumePath, subPath, err)
-	}
-	if startsWithBackstep(relSubPath) {
-		return []uintptr{}, fmt.Errorf("SubPath %q not within volume path %q", subPath, volumePath)
-	}
-
-	if relSubPath == "." {
-		// volumePath and subPath are equal
-		return []uintptr{}, nil
-	}
-
-	fileHandles := []uintptr{}
-	var errorResult error
-
-	currentFullPath := volumePath
-	dirs := strings.Split(relSubPath, string(os.PathSeparator))
-	for _, dir := range dirs {
-		// lock intermediate subPath directory first
-		currentFullPath = filepath.Join(currentFullPath, dir)
-		handle, err := lockPath(currentFullPath)
-		if err != nil {
-			errorResult = fmt.Errorf("cannot lock path %s: %s", currentFullPath, err)
-			break
-		}
-		fileHandles = append(fileHandles, handle)
-
-		// make sure intermediate subPath directory does not contain symlink any more
-		stat, err := os.Lstat(currentFullPath)
-		if err != nil {
-			errorResult = fmt.Errorf("Lstat(%q) error: %v", currentFullPath, err)
-			break
-		}
-		if stat.Mode()&os.ModeSymlink != 0 {
-			errorResult = fmt.Errorf("subpath %q is an unexpected symlink after EvalSymlinks", currentFullPath)
-			break
-		}
-
-		if !PathWithinBase(currentFullPath, volumePath) {
-			errorResult = fmt.Errorf("SubPath %q not within volume path %q", currentFullPath, volumePath)
-			break
-		}
-	}
-
-	return fileHandles, errorResult
-}
-
-// unlockPath unlock directories
-func unlockPath(fileHandles []uintptr) {
-	if fileHandles != nil {
-		for _, handle := range fileHandles {
-			syscall.CloseHandle(syscall.Handle(handle))
-		}
-	}
-}
-
-// lockPath locks a directory or symlink, return handle, exec "syscall.CloseHandle(handle)" to unlock the path
-func lockPath(path string) (uintptr, error) {
-	if len(path) == 0 {
-		return uintptr(syscall.InvalidHandle), syscall.ERROR_FILE_NOT_FOUND
-	}
-	pathp, err := syscall.UTF16PtrFromString(path)
-	if err != nil {
-		return uintptr(syscall.InvalidHandle), err
-	}
-	access := uint32(syscall.GENERIC_READ)
-	sharemode := uint32(syscall.FILE_SHARE_READ)
-	createmode := uint32(syscall.OPEN_EXISTING)
-	flags := uint32(syscall.FILE_FLAG_BACKUP_SEMANTICS | syscall.FILE_FLAG_OPEN_REPARSE_POINT)
-	fd, err := syscall.CreateFile(pathp, access, sharemode, nil, createmode, flags, 0)
-	return uintptr(fd), err
-}
-
-// Lock all directories in subPath and check they're not symlinks.
-func (mounter *Mounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	handles, err := lockAndCheckSubPath(subPath.VolumePath, subPath.Path)
-
-	// Unlock the directories when the container starts
-	cleanupAction = func() {
-		unlockPath(handles)
-	}
-	return subPath.Path, cleanupAction, err
-}
-
-// No bind-mounts for subpaths are necessary on Windows
-func (mounter *Mounter) CleanSubPaths(podDir string, volumeName string) error {
-	return nil
-}
-
 func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error {
 	// Try to mount the disk
 	klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, source, target)
@@ -522,126 +404,3 @@ func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) {
 	}
 	return info.Mode(), nil
 }
-
-// SafeMakeDir makes sure that the created directory does not escape given base directory mis-using symlinks.
-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 {
-	klog.V(4).Infof("Creating directory %q within base %q", pathname, base)
-
-	if !PathWithinBase(pathname, base) {
-		return fmt.Errorf("path %s is outside of allowed base %s", pathname, base)
-	}
-
-	// Quick check if the directory already exists
-	s, err := os.Stat(pathname)
-	if err == nil {
-		// Path exists
-		if s.IsDir() {
-			// The directory already exists. It can be outside of the parent,
-			// but there is no race-proof check.
-			klog.V(4).Infof("Directory %s already exists", pathname)
-			return nil
-		}
-		return &os.PathError{Op: "mkdir", Path: pathname, Err: syscall.ENOTDIR}
-	}
-
-	// Find all existing directories
-	existingPath, toCreate, err := findExistingPrefix(base, pathname)
-	if err != nil {
-		return fmt.Errorf("error opening directory %s: %s", pathname, err)
-	}
-	if len(toCreate) == 0 {
-		return nil
-	}
-
-	// Ensure the existing directory is inside allowed base
-	fullExistingPath, err := filepath.EvalSymlinks(existingPath)
-	if err != nil {
-		return fmt.Errorf("error opening existing directory %s: %s", existingPath, err)
-	}
-	fullBasePath, err := filepath.EvalSymlinks(base)
-	if err != nil {
-		return fmt.Errorf("cannot read link %s: %s", base, err)
-	}
-	if !PathWithinBase(fullExistingPath, fullBasePath) {
-		return fmt.Errorf("path %s is outside of allowed base %s", fullExistingPath, err)
-	}
-
-	// lock all intermediate directories from fullBasePath to fullExistingPath (top to bottom)
-	fileHandles, err := lockAndCheckSubPathWithoutSymlink(fullBasePath, fullExistingPath)
-	defer unlockPath(fileHandles)
-	if err != nil {
-		return err
-	}
-
-	klog.V(4).Infof("%q already exists, %q to create", fullExistingPath, filepath.Join(toCreate...))
-	currentPath := fullExistingPath
-	// create the directories one by one, making sure nobody can change
-	// created directory into symlink by lock that directory immediately
-	for _, dir := range toCreate {
-		currentPath = filepath.Join(currentPath, dir)
-		klog.V(4).Infof("Creating %s", dir)
-		if err := os.Mkdir(currentPath, perm); err != nil {
-			return fmt.Errorf("cannot create directory %s: %s", currentPath, err)
-		}
-		handle, err := lockPath(currentPath)
-		if err != nil {
-			return fmt.Errorf("cannot lock path %s: %s", currentPath, err)
-		}
-		defer syscall.CloseHandle(syscall.Handle(handle))
-		// make sure newly created directory does not contain symlink after lock
-		stat, err := os.Lstat(currentPath)
-		if err != nil {
-			return fmt.Errorf("Lstat(%q) error: %v", currentPath, err)
-		}
-		if stat.Mode()&os.ModeSymlink != 0 {
-			return fmt.Errorf("subpath %q is an unexpected symlink after Mkdir", currentPath)
-		}
-	}
-
-	return nil
-}
-
-// findExistingPrefix finds prefix of pathname that exists. In addition, it
-// returns list of remaining directories that don't exist yet.
-func findExistingPrefix(base, pathname string) (string, []string, error) {
-	rel, err := filepath.Rel(base, pathname)
-	if err != nil {
-		return base, nil, err
-	}
-
-	if startsWithBackstep(rel) {
-		return base, nil, fmt.Errorf("pathname(%s) is not within base(%s)", pathname, base)
-	}
-
-	if rel == "." {
-		// base and pathname are equal
-		return pathname, []string{}, nil
-	}
-
-	dirs := strings.Split(rel, string(filepath.Separator))
-
-	parent := base
-	currentPath := base
-	for i, dir := range dirs {
-		parent = currentPath
-		currentPath = filepath.Join(parent, dir)
-		if _, err := os.Lstat(currentPath); err != nil {
-			if os.IsNotExist(err) {
-				return parent, dirs[i:], nil
-			}
-			return base, nil, err
-		}
-	}
-
-	return pathname, []string{}, nil
-}

pkg/util/mount/nsenter_mount.go

@@ -23,9 +23,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 
-	"golang.org/x/sys/unix"
 	"k8s.io/klog"
 	"k8s.io/utils/nsenter"
 	utilpath "k8s.io/utils/path"
@@ -298,51 +296,6 @@ func (mounter *NsenterMounter) EvalHostSymlinks(pathname string) (string, error)
 	return mounter.ne.EvalSymlinks(pathname, true)
 }
 
-func (mounter *NsenterMounter) CleanSubPaths(podDir string, volumeName string) error {
-	return doCleanSubPaths(mounter, podDir, volumeName)
-}
-
-func (mounter *NsenterMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	// Bind-mount the subpath to avoid using symlinks in subpaths.
-	newHostPath, err = doNsEnterBindSubPath(mounter, subPath)
-
-	// There is no action when the container starts. Bind-mount will be cleaned
-	// when container stops by CleanSubPaths.
-	cleanupAction = nil
-	return newHostPath, cleanupAction, err
-}
-
-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)
-	}
-	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)
-	}
-	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)
-}
-
 func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) {
 	exists, err := mounter.ExistsPath(pathname)
 	if err != nil {
@@ -358,95 +311,6 @@ func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) {
 	return searchMountPoints(hostpath, hostProcMountinfoPath)
 }
 
-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)
-	}
-	klog.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 {
-			klog.V(4).Infof("doNsEnterBindSubPath() failed for %q, cleaning up subpath", bindPathTarget)
-			if cleanErr := cleanSubPath(mounter, subpath); cleanErr != nil {
-				klog.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!
-	klog.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
-	klog.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 {

pkg/util/mount/nsenter_mount_test.go

@@ -23,10 +23,8 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
-	"strings"
 	"testing"
 
-	"golang.org/x/sys/unix"
 	"k8s.io/utils/nsenter"
 )
 
@@ -76,80 +74,6 @@ func TestParseFindMnt(t *testing.T) {
 	}
 }
 
-func TestCheckDeviceInode(t *testing.T) {
-	testDir, err := ioutil.TempDir("", "nsenter-mounter-device-")
-	if err != nil {
-		t.Fatalf("Cannot create temporary directory: %s", err)
-	}
-	defer os.RemoveAll(testDir)
-
-	tests := []struct {
-		name        string
-		srcPath     string
-		dstPath     string
-		expectError string
-	}{
-		{
-			name:        "the same file",
-			srcPath:     filepath.Join(testDir, "1"),
-			dstPath:     filepath.Join(testDir, "1"),
-			expectError: "",
-		},
-		{
-			name:        "different file on the same FS",
-			srcPath:     filepath.Join(testDir, "2.1"),
-			dstPath:     filepath.Join(testDir, "2.2"),
-			expectError: "different inode",
-		},
-		{
-			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 _, 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.Errorf("Test %q: cannot open srcPath %s: %s", test.name, test.srcPath, err)
-			continue
-		}
-
-		err = checkDeviceInode(fd, test.dstPath)
-
-		if test.expectError == "" && err != nil {
-			t.Errorf("Test %q: expected no error, got %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(), test.expectError) {
-					t.Errorf("Test %q: expected error %q, got %q", test.name, test.expectError, err)
-				}
-			}
-		}
-	}
-}
-
 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 {
@@ -504,221 +428,3 @@ func writeRootfsFile(rootfs, path string, mode os.FileMode) (string, error) {
 	}
 	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)
-			}
-		}
-	}
-}

pkg/util/mount/nsenter_mount_unsupported.go

@@ -93,18 +93,6 @@ func (*NsenterMounter) EvalHostSymlinks(pathname string) (string, error) {
 	return "", errors.New("not implemented")
 }
 
-func (*NsenterMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return nil
-}
-
-func (*NsenterMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	return subPath.Path, nil, nil
-}
-
-func (*NsenterMounter) CleanSubPaths(podDir string, volumeName string) error {
-	return nil
-}
-
 func (*NsenterMounter) GetMountRefs(pathname string) ([]string, error) {
 	return nil, errors.New("not implemented")
 }

pkg/volume/BUILD

@@ -21,6 +21,7 @@ go_library(
         "//pkg/util/mount:go_default_library",
         "//pkg/volume/util/fs:go_default_library",
         "//pkg/volume/util/recyclerclient:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//staging/src/k8s.io/api/authentication/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",

pkg/volume/plugins.go

@@ -36,6 +36,7 @@ import (
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume/util/recyclerclient"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 )
 
 type ProbeOperation uint32
@@ -367,6 +368,9 @@ type VolumeHost interface {
 
 	// Returns the event recorder of kubelet.
 	GetEventRecorder() record.EventRecorder
+
+	// Returns an interface that should be used to execute subpath operations
+	GetSubpather() subpath.Interface
 }
 
 // VolumePluginMgr tracks registered plugins.

pkg/volume/testing/BUILD

@@ -17,6 +17,7 @@ go_library(
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//pkg/volume/util/recyclerclient:go_default_library",
+        "//pkg/volume/util/subpath:go_default_library",
         "//pkg/volume/util/volumepathhandler:go_default_library",
         "//staging/src/k8s.io/api/authentication/v1:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",

pkg/volume/testing/testing.go

@@ -42,6 +42,7 @@ import (
 	. "k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/recyclerclient"
+	"k8s.io/kubernetes/pkg/volume/util/subpath"
 	"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
 	utilstrings "k8s.io/utils/strings"
 )
@@ -71,6 +72,7 @@ type fakeVolumeHost struct {
 	exec       mount.Exec
 	nodeLabels map[string]string
 	nodeName   string
+	subpather  subpath.Interface
 }
 
 func NewFakeVolumeHost(rootDir string, kubeClient clientset.Interface, plugins []VolumePlugin) *fakeVolumeHost {
@@ -101,6 +103,7 @@ func newFakeVolumeHost(rootDir string, kubeClient clientset.Interface, plugins [
 	}
 	host.exec = mount.NewFakeExec(nil)
 	host.pluginMgr.InitPlugins(plugins, nil /* prober */, host)
+	host.subpather = &subpath.FakeSubpath{}
 	return host
 }
 
@@ -149,6 +152,10 @@ func (f *fakeVolumeHost) GetMounter(pluginName string) mount.Interface {
 	return f.mounter
 }
 
+func (f *fakeVolumeHost) GetSubpather() subpath.Interface {
+	return f.subpather
+}
+
 func (f *fakeVolumeHost) NewWrapperMounter(volName string, spec Spec, pod *v1.Pod, opts VolumeOptions) (Mounter, error) {
 	// The name of wrapper volume is set to "wrapped_{wrapped_volume_name}"
 	wrapperVolumeName := "wrapped_" + volName

pkg/volume/util/BUILD

@@ -88,6 +88,7 @@ filegroup(
         "//pkg/volume/util/nestedpendingoperations:all-srcs",
         "//pkg/volume/util/operationexecutor:all-srcs",
         "//pkg/volume/util/recyclerclient:all-srcs",
+        "//pkg/volume/util/subpath:all-srcs",
         "//pkg/volume/util/types:all-srcs",
         "//pkg/volume/util/volumepathhandler:all-srcs",
     ],

pkg/volume/util/operationexecutor/operation_generator.go

@@ -750,11 +750,11 @@ func (og *operationGenerator) GenerateUnmountVolumeFunc(
 	}
 
 	unmountVolumeFunc := func() (error, error) {
-		mounter := og.volumePluginMgr.Host.GetMounter(volumeToUnmount.PluginName)
+		subpather := og.volumePluginMgr.Host.GetSubpather()
 
 		// Remove all bind-mounts for subPaths
 		podDir := path.Join(podsDir, string(volumeToUnmount.PodUID))
-		if err := mounter.CleanSubPaths(podDir, volumeToUnmount.InnerVolumeSpecName); err != nil {
+		if err := subpather.CleanSubPaths(podDir, volumeToUnmount.InnerVolumeSpecName); err != nil {
 			return volumeToUnmount.GenerateError("error cleaning subPath mounts", err)
 		}
 

pkg/volume/util/subpath/BUILD

+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = [
+        "subpath.go",
+        "subpath_linux.go",
+        "subpath_nsenter.go",
+        "subpath_unsupported.go",
+        "subpath_windows.go",
+    ],
+    importpath = "k8s.io/kubernetes/pkg/volume/util/subpath",
+    visibility = ["//visibility:public"],
+    deps = select({
+        "@io_bazel_rules_go//go/platform:android": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:darwin": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:dragonfly": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:freebsd": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:linux": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/golang.org/x/sys/unix:go_default_library",
+            "//vendor/k8s.io/klog:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:nacl": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:netbsd": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:openbsd": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:plan9": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:solaris": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:windows": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/k8s.io/klog:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "//conditions:default": [],
+    }),
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = [
+        "subpath_linux_test.go",
+        "subpath_nsenter_test.go",
+        "subpath_windows_test.go",
+    ],
+    embed = [":go_default_library"],
+    deps = select({
+        "@io_bazel_rules_go//go/platform:linux": [
+            "//pkg/util/mount:go_default_library",
+            "//vendor/golang.org/x/sys/unix:go_default_library",
+            "//vendor/k8s.io/klog:go_default_library",
+            "//vendor/k8s.io/utils/nsenter:go_default_library",
+        ],
+        "@io_bazel_rules_go//go/platform:windows": [
+            "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        ],
+        "//conditions:default": [],
+    }),
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

pkg/volume/util/subpath/OWNERS

+# See the OWNERS docs at https://go.k8s.io/owners
+
+reviewers:
+  - jingxu97
+  - saad-ali
+  - jsafrane
+  - msau42
+  - andyzhangx
+approvers:
+  - jingxu97
+  - saad-ali
+  - jsafrane
+

pkg/volume/util/subpath/subpath.go

+/*
+Copyright 2019 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 subpath
+
+import "os"
+
+// Interface defines the set of methods all subpathers must implement
+type Interface interface {
+	// CleanSubPaths removes any bind-mounts created by PrepareSafeSubpath in given
+	// pod volume directory.
+	CleanSubPaths(poodDir string, volumeName string) error
+
+	// PrepareSafeSubpath does everything that's necessary to prepare a subPath
+	// that's 1) inside given volumePath and 2) immutable after this call.
+	//
+	// newHostPath - location of prepared subPath. It should be used instead of
+	// hostName when running the container.
+	// cleanupAction - action to run when the container is running or it failed to start.
+	//
+	// CleanupAction must be called immediately after the container with given
+	// subpath starts. On the other hand, Interface.CleanSubPaths must be called
+	// when the pod finishes.
+	PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err 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
+}
+
+// Subpath defines the attributes of a subpath
+type Subpath struct {
+	// index of the VolumeMount for this container
+	VolumeMountIndex int
+
+	// Full path to the subpath directory on the host
+	Path string
+
+	// name of the volume that is a valid directory name.
+	VolumeName string
+
+	// Full path to the volume path
+	VolumePath string
+
+	// Path to the pod's directory, including pod UID
+	PodDir string
+
+	// Name of the container
+	ContainerName string
+}
+
+// Compile time-check for all implementers of subpath interface
+var _ Interface = &subpath{}
+var _ Interface = &FakeSubpath{}
+
+// FakeSubpath is a subpather implementation for testing
+type FakeSubpath struct{}
+
+// PrepareSafeSubpath is a fake implementation of PrepareSafeSubpath. Always returns
+// newHostPath == subPath.Path
+func (fs *FakeSubpath) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
+	return subPath.Path, nil, nil
+}
+
+// CleanSubPaths is a fake implementation of CleanSubPaths. It is a noop
+func (fs *FakeSubpath) CleanSubPaths(podDir string, volumeName string) error {
+	return nil
+}
+
+// SafeMakeDir is a fake implementation of SafeMakeDir. It is a noop
+func (fs *FakeSubpath) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
+	return nil
+}

pkg/volume/util/subpath/subpath_unsupported.go

@@ -16,124 +16,39 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package mount
+package subpath
 
 import (
 	"errors"
 	"os"
-)
-
-type Mounter struct {
-	mounterPath string
-}
-
-var unsupportedErr = errors.New("util/mount on this platform is not supported")
-
-// New returns a mount.Interface for the current system.
-// It provides options to override the default mounter behavior.
-// mounterPath allows using an alternative to `/bin/mount` for mounting.
-func New(mounterPath string) Interface {
-	return &Mounter{
-		mounterPath: mounterPath,
-	}
-}
-
-func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error {
-	return unsupportedErr
-}
-
-func (mounter *Mounter) Unmount(target string) error {
-	return unsupportedErr
-}
-
-func (mounter *Mounter) List() ([]MountPoint, error) {
-	return []MountPoint{}, unsupportedErr
-}
-
-func (mounter *Mounter) IsMountPointMatch(mp MountPoint, dir string) bool {
-	return (mp.Path == dir)
-}
-
-func (mounter *Mounter) IsNotMountPoint(dir string) (bool, error) {
-	return isNotMountPoint(mounter, dir)
-}
-
-func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) {
-	return true, unsupportedErr
-}
-
-func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) {
-	return "", unsupportedErr
-}
-
-func getDeviceNameFromMount(mounter Interface, mountPath, pluginDir string) (string, error) {
-	return "", unsupportedErr
-}
 
-func (mounter *Mounter) DeviceOpened(pathname string) (bool, error) {
-	return false, unsupportedErr
-}
-
-func (mounter *Mounter) PathIsDevice(pathname string) (bool, error) {
-	return true, unsupportedErr
-}
-
-func (mounter *Mounter) MakeRShared(path string) error {
-	return unsupportedErr
-}
-
-func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error {
-	return mounter.Interface.Mount(source, target, fstype, options)
-}
-
-func (mounter *SafeFormatAndMount) diskLooksUnformatted(disk string) (bool, error) {
-	return true, unsupportedErr
-}
-
-func (mounter *Mounter) GetFileType(pathname string) (FileType, error) {
-	return FileType("fake"), unsupportedErr
-}
-
-func (mounter *Mounter) MakeDir(pathname string) error {
-	return unsupportedErr
-}
-
-func (mounter *Mounter) MakeFile(pathname string) error {
-	return unsupportedErr
-}
-
-func (mounter *Mounter) ExistsPath(pathname string) (bool, error) {
-	return true, errors.New("not implemented")
-}
+	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/utils/nsenter"
+)
 
-func (mounter *Mounter) EvalHostSymlinks(pathname string) (string, error) {
-	return "", unsupportedErr
-}
+type subpath struct{}
 
-func (mounter *Mounter) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
-	return subPath.Path, nil, unsupportedErr
-}
-
-func (mounter *Mounter) CleanSubPaths(podDir string, volumeName string) error {
-	return unsupportedErr
-}
+var errUnsupported = errors.New("util/subpath on this platform is not supported")
 
-func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
-	return unsupportedErr
+// New returns a subpath.Interface for the current system.
+func New(mount.Interface) Interface {
+	return &subpath{}
 }
 
-func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
-	return nil, errors.New("not implemented")
+// NewNSEnter is to satisfy the compiler for having NewSubpathNSEnter exist for all
+// OS choices. however, NSEnter is only valid on Linux
+func NewNSEnter(mounter mount.Interface, ne *nsenter.Nsenter, rootDir string) Interface {
+	return nil
 }
 
-func (mounter *Mounter) GetFSGroup(pathname string) (int64, error) {
-	return -1, errors.New("not implemented")
+func (sp *subpath) PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) {
+	return subPath.Path, nil, errUnsupported
 }
 
-func (mounter *Mounter) GetSELinuxSupport(pathname string) (bool, error) {
-	return false, errors.New("not implemented")
+func (sp *subpath) CleanSubPaths(podDir string, volumeName string) error {
+	return errUnsupported
 }
 
-func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) {
-	return 0, errors.New("not implemented")
+func (sp *subpath) SafeMakeDir(pathname string, base string, perm os.FileMode) error {
+	return errUnsupported
 }