Repository: kubernetes-csi/csi-driver-nfs (go)

Issue:
Cleaningup empty parents resulted in all data being deleted
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!
-->


**What happened**: if subDir is set to something like `foo/${pvc.metadata.namespace}/${pvc.metadata.name}/` and if there are no more PVCs left in a given namespace, all data under `/foo` will be deleted. 

**What you expected to happen**: This is arguably caused by bad config since something like `subDir=${pvc.metadata.namespace}/${pvc.metadata.name}/` would prevent the issue. But still, would be nice to have this highlighted in the docs at least.

I think the issue is caused [here](https://github.com/kubernetes-csi/csi-driver-nfs/blob/master/pkg/nfs/controllerserver.go#L295) specifically by [getRootDir](https://github.com/kubernetes-csi/csi-driver-nfs/blob/master/pkg/nfs/utils.go#L220). 

**How to reproduce it**: 
- Create a default storage class with `subDir=foo/${pvc.metadata.namespace}/${pvc.metadata.name}`
- Create something with a PVC (a database for example) in two different namespaces
- Deleting PVCs in one namespace should delete all data

**Anything else we need to know?**:

**Environment**:
<!-- 
Run following command to get CSI driver version:
kubectl get po -n kube-system -o yaml | grep gcr | grep nfs
-->
- CSI Driver version: 4.10
- Kubernetes version (use `kubectl version`): v1.31.2+rke2r1
- OS (e.g. from /etc/os-release): Ubuntu 24.04 LTS
- Kernel (e.g. `uname -a`): 6.8.0-31-generic


Pull request:
fix: only cleanup empty parent directories during DeleteVolume
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind bug

**What this PR does / why we need it**:

fix: only cleanup empty parent directories during DeleteVolume

<details>

```
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.629160       1 utils.go:111] GRPC call: /csi.v1.Controller/DeleteVolume
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.629419       1 utils.go:112] GRPC request: {"secrets":"***stripped***","volume_id":"nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c#"}
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.630434       1 controllerserver.go:226] DeleteVolume: found mountOptions(nfsvers=4.1) for volume(nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c#)
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.630557       1 controllerserver.go:525] internally mounting nfs-server.default.svc.cluster.local:/ at /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.630729       1 nodeserver.go:133] NodePublishVolume: volumeID(nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c#) source(nfs-server.default.svc.cluster.local:/) targetPath(/tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c) mountflags([nfsvers=4.1])
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.630993       1 mount_linux.go:270] Mounting cmd (mount) with arguments (-t nfs -o nfsvers=4.1 nfs-server.default.svc.cluster.local:/ /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c)
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.675955       1 nodeserver.go:153] skip chmod on targetPath(/tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c) since mountPermissions is set as 0
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.676031       1 nodeserver.go:155] volume(nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c#) mount nfs-server.default.svc.cluster.local:/ on /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c succeeded
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.676084       1 controllerserver.go:295] removing subdirectory at /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c/nfs-270/pvc-dxb27
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.685621       1 controllerserver.go:302] DeleteVolume: removing empty directories in /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c/nfs-270
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.687404       1 utils.go:245] Removing empty directory /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c/nfs-270
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.688227       1 controllerserver.go:540] internally unmounting /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.688245       1 nodeserver.go:176] NodeUnpublishVolume: unmounting volume nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c# on /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.688254       1 nodeserver.go:181] force unmount nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c# on /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.689332       1 mount_helper_common.go:56] unmounting "/tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c" (corruptedMount: false, mounterCanSkipMountPointChecks: true)
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.689352       1 mount_linux.go:892] Unmounting /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.724365       1 mount_helper_common.go:150] Deleting path "/tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c"
[pod/csi-nfs-controller-6dcc8cc68c-mn26w/nfs] I0313 10:33:11.724912       1 nodeserver.go:189] NodeUnpublishVolume: unmount volume nfs-server.default.svc.cluster.local##nfs-270/pvc-dxb27#pvc-ce9651cb-815e-463c-bd67-a7215b97b85c# on /tmp/pvc-ce9651cb-815e-463c-bd67-a7215b97b85c successfully
```

</details>

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #855

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
fix: only cleanup empty parent directories during DeleteVolume
```


Patch:
diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go
index e403b0d0..c572154b 100644
--- a/pkg/nfs/controllerserver.go
+++ b/pkg/nfs/controllerserver.go
@@ -292,17 +292,16 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			}
 			klog.V(2).Infof("archived subdirectory %s --> %s", internalVolumePath, archivedInternalVolumePath)
 		} else {
-			rootDir := getRootDir(nfsVol.subDir)
-			if rootDir != "" {
-				rootDir = filepath.Join(getInternalMountPath(cs.Driver.workingMountDir, nfsVol), rootDir)
-			} else {
-				rootDir = internalVolumePath
-			}
-			// delete subdirectory under base-dir
-			klog.V(2).Infof("removing subdirectory at %v on internalVolumePath %s", rootDir, internalVolumePath)
-			if err = os.RemoveAll(rootDir); err != nil {
+			klog.V(2).Infof("removing subdirectory at %v", internalVolumePath)
+			if err = os.RemoveAll(internalVolumePath); err != nil {
 				return nil, status.Errorf(codes.Internal, "delete subdirectory(%s) failed with %v", internalVolumePath, err)
 			}
+
+			parentDir := filepath.Dir(internalVolumePath)
+			klog.V(2).Infof("DeleteVolume: removing empty directories in %s", parentDir)
+			if err = removeEmptyDirs(getInternalMountPath(cs.Driver.workingMountDir, nfsVol), parentDir); err != nil {
+				return nil, status.Errorf(codes.Internal, "failed to remove empty directories: %v", err)
+			}
 		}
 	} else {
 		klog.V(2).Infof("DeleteVolume: volume(%s) is set to retain, not deleting/archiving subdirectory", volumeID)
diff --git a/pkg/nfs/utils.go b/pkg/nfs/utils.go
index 4c5ad1c6..53737cb2 100644
--- a/pkg/nfs/utils.go
+++ b/pkg/nfs/utils.go
@@ -19,6 +19,7 @@ package nfs
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -216,10 +217,50 @@ func waitForPathNotExistWithTimeout(path string, timeout time.Duration) error {
 	}
 }
 
-// getRootDir returns the root directory of the given directory
-func getRootDir(path string) string {
-	parts := strings.Split(path, "/")
-	return parts[0]
+// removeEmptyDirs removes empty directories in the given directory dir until the parent directory parentDir
+// It will remove all empty directories in the path from the given directory to the parent directory
+// It will not remove the parent directory parentDir
+func removeEmptyDirs(parentDir, dir string) error {
+	if parentDir == "" || dir == "" {
+		return nil
+	}
+
+	absParentDir, err := filepath.Abs(parentDir)
+	if err != nil {
+		return err
+	}
+
+	absDir, err := filepath.Abs(dir)
+	if err != nil {
+		return err
+	}
+
+	if !strings.HasPrefix(absDir, absParentDir) {
+		return fmt.Errorf("dir %s is not a subdirectory of parentDir %s", dir, parentDir)
+	}
+
+	var depth int
+	for absDir != absParentDir {
+		entries, err := os.ReadDir(absDir)
+		if err != nil {
+			return err
+		}
+		if len(entries) == 0 {
+			klog.V(2).Infof("Removing empty directory %s", absDir)
+			if err := os.Remove(absDir); err != nil {
+				return err
+			}
+		} else {
+			klog.V(2).Infof("Directory %s is not empty", absDir)
+			break
+		}
+		if depth++; depth > 10 {
+			return fmt.Errorf("depth of directory %s is too deep", dir)
+		}
+		absDir = filepath.Dir(absDir)
+	}
+
+	return nil
 }
 
 // ExecFunc returns a exec function's output and error


Test patch:
diff --git a/pkg/nfs/utils_test.go b/pkg/nfs/utils_test.go
index 66aa3fc4..58a457ff 100644
--- a/pkg/nfs/utils_test.go
+++ b/pkg/nfs/utils_test.go
@@ -388,43 +388,63 @@ func TestWaitForPathNotExistWithTimeout(t *testing.T) {
 	}
 }
 
-func TestGetRootPath(t *testing.T) {
+func TestRemoveEmptyDirs(t *testing.T) {
+	parentDir, _ := os.Getwd()
+	emptyDirOneLevel, _ := getWorkDirPath("emptyDir1")
+	emptyDirTwoLevels, _ := getWorkDirPath("emptyDir2/emptyDir2")
+	emptyDirThreeLevels, _ := getWorkDirPath("emptyDir3/emptyDir2/emptyDir3")
+
 	tests := []struct {
-		desc     string
-		dir      string
-		expected string
+		desc      string
+		parentDir string
+		dir       string
+		expected  error
 	}{
 		{
-			desc:     "empty path",
-			dir:      "",
-			expected: "",
+			desc:      "empty path",
+			parentDir: parentDir,
+			expected:  nil,
 		},
 		{
-			desc:     "root path",
-			dir:      "/",
-			expected: "",
+			desc:      "empty dir with one level",
+			parentDir: parentDir,
+			dir:       emptyDirOneLevel,
+			expected:  nil,
 		},
 		{
-			desc:     "subdir path",
-			dir:      "/subdir",
-			expected: "",
+			desc:      "dir is not a subdirectory of parentDir",
+			parentDir: "/dir1",
+			dir:       "/dir2",
+			expected:  fmt.Errorf("dir /dir2 is not a subdirectory of parentDir /dir1"),
 		},
 		{
-			desc:     "subdir path without leading slash",
-			dir:      "subdir",
-			expected: "subdir",
+			desc:      "empty dir with two levels",
+			parentDir: parentDir,
+			dir:       emptyDirTwoLevels,
+			expected:  nil,
 		},
 		{
-			desc:     "multiple subdir path without leading slash",
-			dir:      "subdir/subdir2",
-			expected: "subdir",
+			desc:      "empty dir with three levels",
+			parentDir: parentDir,
+			dir:       emptyDirThreeLevels,
+			expected:  nil,
 		},
 	}
 
 	for _, test := range tests {
-		result := getRootDir(test.dir)
-		if result != test.expected {
-			t.Errorf("Unexpected result: %s, expected: %s", result, test.expected)
+		if strings.Contains(test.dir, "emptyDir") {
+			_ = makeDir(test.dir)
+			defer os.RemoveAll(test.dir)
+		}
+		err := removeEmptyDirs(test.parentDir, test.dir)
+		if !reflect.DeepEqual(err, test.expected) {
+			t.Errorf("test[%s]: unexpected output: %v, expected result: %v", test.desc, err, test.expected)
+		}
+		if strings.Contains(test.dir, "emptyDir") {
+			// directory should be removed
+			if _, err := os.Stat(emptyDirOneLevel); !os.IsNotExist(err) {
+				t.Errorf("test[%s]: directory %s should be removed", test.desc, emptyDirOneLevel)
+			}
 		}
 	}
 }