Merge pull request #105512 from gnufied/add-subpath-mount-source-check

Fix check for subpath source

pkg/volume/util/subpath/subpath_linux.go

@@ -30,7 +30,6 @@ import (
 
 	"golang.org/x/sys/unix"
 	"k8s.io/klog/v2"
-	"k8s.io/kubernetes/pkg/volume/util/hostutil"
 	"k8s.io/mount-utils"
 )
 
@@ -110,12 +109,12 @@ func prepareSubpathTarget(mounter mount.Interface, subpath Subpath) (bool, strin
 		notMount = true
 	}
 	if !notMount {
-		linuxHostUtil := hostutil.NewHostUtil()
+		// It's already mounted, so check if it's bind-mounted to the same path
-		mntInfo, err := linuxHostUtil.FindMountInfo(bindPathTarget)
+		samePath, err := checkSubPathFileEqual(subpath, bindPathTarget)
 		if err != nil {
-			return false, "", fmt.Errorf("error calling findMountInfo for %s: %s", bindPathTarget, err)
+			return false, "", fmt.Errorf("error checking subpath mount info for %s: %s", bindPathTarget, err)
 		}
-		if mntInfo.Root != subpath.Path {
+		if !samePath {
 			// It's already mounted but not what we want, unmount it
 			if err = mounter.Unmount(bindPathTarget); err != nil {
 				return false, "", fmt.Errorf("error ummounting %s: %s", bindPathTarget, err)
@@ -156,6 +155,23 @@ func prepareSubpathTarget(mounter mount.Interface, subpath Subpath) (bool, strin
 	return false, bindPathTarget, nil
 }
 
+func checkSubPathFileEqual(subpath Subpath, bindMountTarget string) (bool, error) {
+	s, err := os.Lstat(subpath.Path)
+	if err != nil {
+		return false, fmt.Errorf("stat %s failed: %s", subpath.Path, err)
+	}
+
+	t, err := os.Lstat(bindMountTarget)
+	if err != nil {
+		return false, fmt.Errorf("lstat %s failed: %s", bindMountTarget, err)
+	}
+
+	if !os.SameFile(s, t) {
+		return false, nil
+	}
+	return true, 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))

pkg/volume/util/subpath/subpath_linux_test.go

@@ -921,6 +921,143 @@ func TestBindSubPath(t *testing.T) {
 	}
 }
 
+func TestSubpath_PrepareSafeSubpath(t *testing.T) {
+	//complete code
+	defaultPerm := os.FileMode(0750)
+
+	tests := []struct {
+		name string
+		// Function that prepares directory structure for the test under given
+		// base.
+		prepare      func(base string) ([]string, string, string, error)
+		expectError  bool
+		expectAction []mount.FakeAction
+		mountExists  bool
+	}{
+		{
+			name: "subpath-mount-already-exists-with-mismatching-mount",
+			prepare: func(base string) ([]string, string, string, error) {
+				volpath, subpathMount := getTestPaths(base)
+				mounts := []string{subpathMount}
+				if err := os.MkdirAll(subpathMount, defaultPerm); err != nil {
+					return nil, "", "", err
+				}
+
+				subpath := filepath.Join(volpath, "dir0")
+				return mounts, volpath, subpath, os.MkdirAll(subpath, defaultPerm)
+			},
+			expectError:  false,
+			expectAction: []mount.FakeAction{{Action: "unmount"}},
+			mountExists:  false,
+		},
+		{
+			name: "subpath-mount-already-exists-with-samefile",
+			prepare: func(base string) ([]string, string, string, error) {
+				volpath, subpathMount := getTestPaths(base)
+				mounts := []string{subpathMount}
+				subpathMountRoot := filepath.Dir(subpathMount)
+
+				if err := os.MkdirAll(subpathMountRoot, defaultPerm); err != nil {
+					return nil, "", "", err
+				}
+				targetFile, err := os.Create(subpathMount)
+				if err != nil {
+					return nil, "", "", err
+				}
+				defer targetFile.Close()
+
+				if err := os.MkdirAll(volpath, defaultPerm); err != nil {
+					return nil, "", "", err
+				}
+				subpath := filepath.Join(volpath, "file0")
+				// using hard link to simulate bind mounts
+				err = os.Link(subpathMount, subpath)
+				if err != nil {
+					return nil, "", "", err
+				}
+				return mounts, volpath, subpath, nil
+			},
+			expectError:  false,
+			expectAction: []mount.FakeAction{},
+			mountExists:  true,
+		},
+	}
+	for _, test := range tests {
+		klog.V(4).Infof("test %q", test.name)
+		base, err := ioutil.TempDir("", "bind-subpath-"+test.name+"-")
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+		defer os.RemoveAll(base)
+
+		mounts, volPath, subPath, err := test.prepare(base)
+		if err != nil {
+			os.RemoveAll(base)
+			t.Fatalf("failed to prepare test %q: %v", test.name, err.Error())
+		}
+
+		fm := setupFakeMounter(mounts)
+
+		subpath := Subpath{
+			VolumeMountIndex: testSubpath,
+			Path:             subPath,
+			VolumeName:       testVol,
+			VolumePath:       volPath,
+			PodDir:           filepath.Join(base, "pod0"),
+			ContainerName:    testContainer,
+		}
+
+		_, subpathMount := getTestPaths(base)
+		bindMountExists, bindPathTarget, err := prepareSubpathTarget(fm, subpath)
+
+		if bindMountExists != test.mountExists {
+			t.Errorf("test %q failed: expected bindMountExists %v, got %v", test.name, test.mountExists, bindMountExists)
+		}
+
+		logActions := fm.GetLog()
+		if len(test.expectAction) == 0 && len(logActions) > 0 {
+			t.Errorf("test %q failed: expected no actions, got %v", test.name, logActions)
+		}
+
+		if len(test.expectAction) > 0 {
+			foundMatchingAction := false
+			testAction := test.expectAction[0]
+			for _, action := range logActions {
+				if action.Action == testAction.Action {
+					foundMatchingAction = true
+					break
+				}
+			}
+			if !foundMatchingAction {
+				t.Errorf("test %q failed: expected action %q, got %v", test.name, testAction.Action, logActions)
+			}
+		}
+
+		if test.expectError {
+			if err == nil {
+				t.Errorf("test %q failed: expected error, got success", test.name)
+			}
+			if bindPathTarget != "" {
+				t.Errorf("test %q failed: expected empty bindPathTarget, got %v", test.name, bindPathTarget)
+			}
+			if err = validateDirNotExists(subpathMount); err != nil {
+				t.Errorf("test %q failed: %v", test.name, err)
+			}
+		}
+		if !test.expectError {
+			if err != nil {
+				t.Errorf("test %q failed: %v", test.name, err)
+			}
+			if bindPathTarget != subpathMount {
+				t.Errorf("test %q failed: expected bindPathTarget %v, got %v", test.name, subpathMount, bindPathTarget)
+			}
+			if err = validateFileExists(subpathMount); err != nil {
+				t.Errorf("test %q failed: %v", test.name, err)
+			}
+		}
+	}
+}
+
 func TestSafeOpen(t *testing.T) {
 	defaultPerm := os.FileMode(0750)