Repository: kubernetes-csi/csi-driver-nfs
Pull request #911 (2025-05-11 13:48:47)

chore: create voume/snapshot should respect mountOptions in secret
<!--  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: create voume/snapshot should respect mountOptions in secret, thus user could only set mountOptions in secret for both create and delete volume/snapshot

**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 #

**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: create voume/snapshot should respect mountOptions in secret
```



Patch:
diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go
index db34c340..b9eb4f27 100644
--- a/pkg/nfs/controllerserver.go
+++ b/pkg/nfs/controllerserver.go
@@ -163,7 +163,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	var volCap *csi.VolumeCapability
+	volCap := getVolumeCapabilityFromSecret(name, req.GetSecrets())
 	if len(req.GetVolumeCapabilities()) > 0 {
 		volCap = req.GetVolumeCapabilities()[0]
 	}
@@ -220,19 +220,6 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 		return &csi.DeleteVolumeResponse{}, nil
 	}
 
-	var volCap *csi.VolumeCapability
-	mountOptions := getMountOptions(req.GetSecrets())
-	if mountOptions != "" {
-		klog.V(2).Infof("DeleteVolume: found mountOptions(%s) for volume(%s)", mountOptions, volumeID)
-		volCap = &csi.VolumeCapability{
-			AccessType: &csi.VolumeCapability_Mount{
-				Mount: &csi.VolumeCapability_MountVolume{
-					MountFlags: []string{mountOptions},
-				},
-			},
-		}
-	}
-
 	if nfsVol.onDelete == "" {
 		nfsVol.onDelete = cs.Driver.defaultOnDeletePolicy
 	}
@@ -253,6 +240,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			return &csi.DeleteVolumeResponse{}, nil
 		}
 		// mount nfs base share so we can delete the subdirectory
+		volCap := getVolumeCapabilityFromSecret(volumeID, req.GetSecrets())
 		if err = cs.internalMount(ctx, nfsVol, nil, volCap); err != nil {
 			return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err)
 		}
@@ -376,7 +364,8 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		return nil, status.Errorf(codes.NotFound, "failed to create nfsSnapshot: %v", err)
 	}
 	snapVol := volumeFromSnapshot(snapshot)
-	if err = cs.internalMount(ctx, snapVol, req.GetParameters(), nil); err != nil {
+	volCap := getVolumeCapabilityFromSecret(req.GetSourceVolumeId(), req.GetSecrets())
+	if err = cs.internalMount(ctx, snapVol, req.GetParameters(), volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount snapshot nfs server: %v", err)
 	}
 	defer func() {
@@ -392,7 +381,7 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		return nil, err
 	}
 
-	if err = cs.internalMount(ctx, srcVol, req.GetParameters(), nil); err != nil {
+	if err = cs.internalMount(ctx, srcVol, req.GetParameters(), volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount src nfs server: %v", err)
 	}
 	defer func() {
@@ -445,18 +434,7 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS
 		return &csi.DeleteSnapshotResponse{}, nil
 	}
 
-	var volCap *csi.VolumeCapability
-	mountOptions := getMountOptions(req.GetSecrets())
-	if mountOptions != "" {
-		klog.V(2).Infof("DeleteSnapshot: found mountOptions(%s) for snapshot(%s)", mountOptions, req.GetSnapshotId())
-		volCap = &csi.VolumeCapability{
-			AccessType: &csi.VolumeCapability_Mount{
-				Mount: &csi.VolumeCapability_MountVolume{
-					MountFlags: []string{mountOptions},
-				},
-			},
-		}
-	}
+	volCap := getVolumeCapabilityFromSecret(req.SnapshotId, req.GetSecrets())
 	vol := volumeFromSnapshot(snap)
 	if err = cs.internalMount(ctx, vol, nil, volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount nfs server for snapshot deletion: %v", err)
diff --git a/pkg/nfs/utils.go b/pkg/nfs/utils.go
index 14d414ad..03ef6aaa 100644
--- a/pkg/nfs/utils.go
+++ b/pkg/nfs/utils.go
@@ -289,3 +289,21 @@ func WaitUntilTimeout(timeout time.Duration, execFunc ExecFunc, timeoutFunc Time
 		return timeoutFunc()
 	}
 }
+
+// getVolumeCapabilityFromSecret retrieves the volume capability from the secret
+// if secret contains mountOptions, it will return the volume capability
+// if secret does not contain mountOptions, it will return nil
+func getVolumeCapabilityFromSecret(volumeID string, secret map[string]string) *csi.VolumeCapability {
+	mountOptions := getMountOptions(secret)
+	if mountOptions != "" {
+		klog.V(2).Infof("found mountOptions(%s) for volume(%s)", mountOptions, volumeID)
+		return &csi.VolumeCapability{
+			AccessType: &csi.VolumeCapability_Mount{
+				Mount: &csi.VolumeCapability_MountVolume{
+					MountFlags: []string{mountOptions},
+				},
+			},
+		}
+	}
+	return nil
+}


Test patch:
diff --git a/pkg/nfs/utils_test.go b/pkg/nfs/utils_test.go
index c34512ed..b1cc9423 100644
--- a/pkg/nfs/utils_test.go
+++ b/pkg/nfs/utils_test.go
@@ -24,6 +24,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/container-storage-interface/spec/lib/go/csi"
 	"go.uber.org/goleak"
 )
 
@@ -505,3 +506,51 @@ func TestWaitUntilTimeout(t *testing.T) {
 		}
 	}
 }
+func TestGetVolumeCapabilityFromSecret(t *testing.T) {
+	tests := []struct {
+		desc     string
+		volumeID string
+		secret   map[string]string
+		expected *csi.VolumeCapability
+	}{
+		{
+			desc:     "secret contains mountOptions",
+			volumeID: "vol-123",
+			secret:   map[string]string{"mountOptions": "nfsvers=3"},
+			expected: &csi.VolumeCapability{
+				AccessType: &csi.VolumeCapability_Mount{
+					Mount: &csi.VolumeCapability_MountVolume{
+						MountFlags: []string{"nfsvers=3"},
+					},
+				},
+			},
+		},
+		{
+			desc:     "secret does not contain mountOptions",
+			volumeID: "vol-456",
+			secret:   map[string]string{"otherKey": "otherValue"},
+			expected: nil,
+		},
+		{
+			desc:     "empty secret",
+			volumeID: "vol-789",
+			secret:   map[string]string{},
+			expected: nil,
+		},
+		{
+			desc:     "nil secret",
+			volumeID: "vol-000",
+			secret:   nil,
+			expected: nil,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			result := getVolumeCapabilityFromSecret(test.volumeID, test.secret)
+			if !reflect.DeepEqual(result, test.expected) {
+				t.Errorf("test[%s]: unexpected result: %v, expected: %v", test.desc, result, test.expected)
+			}
+		})
+	}
+}