From 6e9f428547ef8708c424f11c9b6645ce5859f2eb Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Fri, 9 Sep 2022 04:20:28 -0700 Subject: [PATCH] Fixes getNestedMountpoints grouping Currently, getNestedMountpoints sorts using sort.Strings, which would sort the following strings in this exact order: /dir/nested, /dir/nested-vol, /dir/nested.vol, /dir/nested/double, /dir/nested2 Because of this, "nested/double" is returned as well, even though it shouldn't have been. This issue is worse on Windows, where the path separator is typically the backslash. This commit addresses this issue by checking if a nested mount point has been previously seen or not. --- pkg/volume/util/nested_volumes.go | 22 ++++++++++++++++++---- pkg/volume/util/nested_volumes_test.go | 5 ++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/volume/util/nested_volumes.go b/pkg/volume/util/nested_volumes.go index 2ceab422382..27a6c29cd98 100644 --- a/pkg/volume/util/nested_volumes.go +++ b/pkg/volume/util/nested_volumes.go @@ -51,8 +51,14 @@ func getNestedMountpoints(name, baseDir string, pod v1.Pod) ([]string, error) { return fmt.Errorf("invalid container mount point %v", myMountPoint) } myMPSlash := myMountPoint + string(os.PathSeparator) - // The previously found nested mountpoint (or "" if none found yet) - prevNestedMP := "" + // The previously found nested mountpoints. + // NOTE: We can't simply rely on sort.Strings to have all the mountpoints sorted and + // grouped. For example, the following strings are sorted in this exact order: + // /dir/nested, /dir/nested-vol, /dir/nested.vol, /dir/nested/double, /dir/nested2 + // The issue is a bit worse for Windows paths, since the \'s value is higher than /'s: + // \dir\nested, \dir\nested-vol, \dir\nested.vol, \dir\nested2, \dir\nested\double + // Because of this, we should use a list of previously mounted mountpoints, rather than only one. + prevNestedMPs := []string{} // examine each mount point to see if it's nested beneath this volume // (but skip any that are double-nested beneath this volume) // For example, if this volume is mounted as /dir and other volumes are mounted @@ -61,11 +67,19 @@ func getNestedMountpoints(name, baseDir string, pod v1.Pod) ([]string, error) { if !strings.HasPrefix(mp, myMPSlash) { continue // skip -- not nested beneath myMountPoint } - if prevNestedMP != "" && strings.HasPrefix(mp, prevNestedMP) { + + isNested := false + for _, prevNestedMP := range prevNestedMPs { + if strings.HasPrefix(mp, prevNestedMP) { + isNested = true + break + } + } + if isNested { continue // skip -- double nested beneath myMountPoint } // since this mount point is nested, remember it so that we can check that following ones aren't nested beneath this one - prevNestedMP = mp + string(os.PathSeparator) + prevNestedMPs = append(prevNestedMPs, mp+string(os.PathSeparator)) retval = append(retval, mp[len(myMPSlash):]) } } diff --git a/pkg/volume/util/nested_volumes_test.go b/pkg/volume/util/nested_volumes_test.go index f4f4adda0b8..b136eaeaf75 100644 --- a/pkg/volume/util/nested_volumes_test.go +++ b/pkg/volume/util/nested_volumes_test.go @@ -89,7 +89,7 @@ func TestGetNestedMountpoints(t *testing.T) { { name: "Unsorted Nested Pod", err: false, - expected: sets.NewString("nested", "nested2"), + expected: sets.NewString("nested", "nested2", "nested-vol", "nested.vol"), volname: "vol1", pod: v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -105,6 +105,9 @@ func TestGetNestedMountpoints(t *testing.T) { {MountPath: "/dir/nested", Name: "vol2"}, {MountPath: "/ignore2", Name: "vol5"}, {MountPath: "/dir", Name: "vol1"}, + {MountPath: "/dir/nested-vol", Name: "vol6"}, + {MountPath: "/dir/nested.vol", Name: "vol7"}, + {MountPath: "/dir/nested2/double", Name: "vol8"}, {MountPath: "/dir/nested2", Name: "vol3"}, }, },