Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 7, 2025

What type of PR is this?
/kind bug

What this PR does / why we need it:

Mount options like noexec, nosuid, and nodev specified in StorageClass or PersistentVolume were applied at the node NFS mount but not visible inside containers. Linux bind mounts don't inherit security mount options from source mounts.

Solution:
Implement CSI staging/unstaging workflow:

  • NodeStageVolume: Mount NFS to staging path with all options
  • NodePublishVolume: Bind mount from staging to target, then remount with security options to apply them to the bind mount
  • NodeUnstageVolume: Unmount staged volumes

Changes:

  • Add STAGE_UNSTAGE_VOLUME capability
  • Implement staging/unstaging with metadata replacement support
  • Enhance NodePublishVolume to remount bind mounts with security options
  • Maintain backward compatibility for direct mount path (when staging not used)
  • Add test coverage for new staging functionality

Key insight: After bind mount, explicit remount operation required to apply security options:

// Bind mount
ns.mounter.Mount(stagingTargetPath, targetPath, "", []string{"bind"})

// Remount with security options
ns.mounter.Mount("", targetPath, "", []string{"remount", "noexec", "nosuid", "nodev"})

Which issue(s) this PR fixes:

Special notes for your reviewer:

This changes the driver's advertised capabilities to include STAGE_UNSTAGE_VOLUME. Kubernetes will use the staging workflow for new mounts. Existing mounts continue to work via the legacy direct mount path.

Does this PR introduce a user-facing change?:

Fix mount options like noexec, nosuid, and nodev not being propagated to containers by implementing CSI staging/unstaging workflow
Original prompt

This section details on the original issue you should resolve

<issue_title>mountOptions are not propagated</issue_title>
<issue_description>What happened:
I did add a storage class as :

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: nfs-csi-nsfserver
provisioner: nfs.csi.k8s.io
parameters:
  server: nsfserver
  share: /
reclaimPolicy: Retain
volumeBindingMode: WaitForFirstConsumer
allowVolumeExpansion: true
mountOptions:
  - nfsvers=4.2
  - fsc
  - noexec
  - nosuid
  - nodev

then I add associated PV as

apiVersion: v1
kind: PersistentVolume
metadata:
  annotations:
    pv.kubernetes.io/provisioned-by: nfs.csi.k8s.io
  name: pv-nfs-myshare-service1
spec:
  capacity:
    storage: 900Gi
  accessModes:
    - ReadWriteMany
  persistentVolumeReclaimPolicy: Retain
  storageClassName: nfs-csi-nsfserver
  mountOptions:
    - fsc
    - noexec
    - nosuid
    - nodev
  csi:
    driver: nfs.csi.k8s.io
    volumeHandle: nfs/nfs-csi-nsfserver/myshare/service1
    volumeAttributes:
      server: nfs-csi-nsfserver
      share: /myshare/

then I did create the related PVC as :

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: nfs-myshare-pv-claim
  namespace: service1
spec:
  volumeName: pv-nfs-myshare-service1
  storageClassName: nfs-csi-nsfserver
  accessModes:
    - ReadWriteMany
  resources:
    requests:
      storage: 800Gi

and then, I did mount the pv claim in my pod :

volumes:
  - name: somefiles
    persistentVolumeClaim:
      claimName: nfs-myshare-pv-claim

When I run :

:~$ kubectl exec -it csi-nfs-node-rvkvp -n nfs -c nfs -- mount | grep nfs
nsfserver:/myshare on /var/lib/kubelet/pods/bea13e5e-01bd-4d64-87e5-2329b6ce5c65/volumes/kubernetes.io~csi/pv-nfs-myshare-service1/mount type nfs4 (rw,nosuid,nodev,noexec,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,fatal_neterrors=none,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.1.1,fsc,local_lock=none,addr=192.168.1.2)
nsfserver:/myshare on /var/lib/kubelet/pods/8ede7914-d858-43d4-96a0-3b41625299a3/volumes/kubernetes.io~csi/pv-nfs-myshare-service2/mount type nfs4 (rw,nosuid,nodev,noexec,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,fatal_neterrors=none,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.1.1,fsc,local_lock=none,addr=192.168.1.2)

(Yes, I'm using this nfs server twice on this node for 2 differents services)

we can see that added mountOptions ( fsc, noexec, nosuid, nodev, nfsvers=4.2) are here

but from inside the pod I can see

:~$ kubectl -n namespace exec -ti pods/service1-57bc4d687-djjjr -- sh
~ $ mount | grep nsfserver
nsfserver:/myshare on /somefiles type nfs4 (rw,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,fatal_neterrors=none,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.1.1,fsc,local_lock=none,addr=192.168.1.2)

I can see mountOptions (noexec, nosuid, nodev)

What you expected to happen:
having the defined mountOptions

Environment:

  • CSI Driver version: v4.12.1
  • Kubernetes version (use kubectl version): v1.34.2
  • OS (e.g. from /etc/os-release): Fedora CoreOS
  • Kernel (e.g. uname -a): 6.17.7-300.fc43.x86_64
  • Install tools: Kubeadm and helm chart 4.12.1
  • Others: /
    </issue_description>

<agent_instructions>do you know how to explain this issue? is it needed to fix or by design?</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI self-assigned this Dec 7, 2025
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 7, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @Copilot. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 7, 2025
- Add STAGE_UNSTAGE_VOLUME capability to driver
- Implement NodeStageVolume to mount NFS share at staging path with all mount options
- Modify NodePublishVolume to bind mount from staging to target and remount with security options
- Implement NodeUnstageVolume to properly unmount staged volumes
- Add comprehensive tests for staging and unstaging functionality
- Maintain backward compatibility with legacy direct mount path

Co-authored-by: andyzhangx <4178417+andyzhangx@users.noreply.github.com>
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Copilot
Once this PR has been reviewed and has the lgtm label, please ask for approval from andyzhangx. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Copilot
Once this PR has been reviewed and has the lgtm label, please ask for approval from andyzhangx. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 7, 2025
Copilot AI and others added 2 commits December 7, 2025 14:45
- Add subDirReplaceMap to handle pv/pvc metadata in subDir during staging
- Ensure consistent behavior between staging and legacy mount paths

Co-authored-by: andyzhangx <4178417+andyzhangx@users.noreply.github.com>
- Use same force unmount pattern as NodeUnpublishVolume for cleanup
- Ensures proper cleanup even if mount is stuck

Co-authored-by: andyzhangx <4178417+andyzhangx@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix mountOptions propagation in StorageClass and PV Fix mount options not propagated to containers (noexec, nosuid, nodev) Dec 7, 2025
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 7, 2025
Copilot AI requested a review from andyzhangx December 7, 2025 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mountOptions are not propagated

3 participants