fix: mount options for fsGroup delegation must ensure RWX for the group #848
Conversation
If user sets `fsGroup: <gid>` in Pod's spec.securityContext, kubelet delegate fsGroup to CSI Driver, and NodeStageVolume() adds `gid=<gid>` to mount options. This might be not enough to make volume writable for the user: ``` $ kubectl exec fedora -- ls -ld /mnt/claim drwxr-xr-x. 2 root 1002 0 Sep 13 12:04 /mnt/claim $ kubectl exec fedora -- touch /mnt/claim/FILE touch: cannot touch '/mnt/claim/FILE': Permission denied ``` See kubernetes-csi#835
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mpatlasov 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 |
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @mpatlasov. Thanks for your PR. I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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
added
needs-ok-to-test
Pull Request Test Coverage Report for Build 10848904027Details
💛 - Coveralls |
| mountOptions = append(mountOptions, "file_mode=0774") | ||
| } | ||
| if !raiseGroupRWXInMountFlags(mountOptions, "dir_mode") { | ||
| mountOptions = append(mountOptions, "dir_mode=0775") |
There was a problem hiding this comment.
if dir_mode=0775 and file_mode=0774 is required, why not set those mount options in storage class or pv directly?
There was a problem hiding this comment.
For dynamic provisioning case, the user doesn't create pv. The storage class can be used both by privileged users (who doesn't need fsGroup) and ordinary ones who expect that everything is fine if security context is set correctly for a pod. I think setting fsGroup for a pod must give access to the volume for any storage class. Is this wrong assumption?
There was a problem hiding this comment.
that's reasonable, and I think if file_mode is not specified in pv, then we should append file_mode=0774, same for dir_mode, if it's not specified in pv, then append dir_mode=0775, since if file_mode, dir_mode is specified in pv, we should respect the setting, that would make the logic more straightforward.
btw, in nfs volume mount, we use SetOwnerShip to recursively set the ownership of the volume per fsGroup, while this solution won't work for smb mount since the ownership cannot be changed after smb volume is mounted.
There was a problem hiding this comment.
if file_mode, dir_mode is specified in pv, we should respect the setting
That conflicts with fsGroup implementation in Kubernetes and all other CSI drivers. If fsGroup in a Pod depends on StorageClass parameters, it will need a very visible documentation that fsGroup support needs file_mode: 0770 and dir_mode: 0770 in a StorageClass or even make it a default.
I would prefer if the driver did the right thing out of the box, as this PR proposes.
If user sets
fsGroup: <gid>in Pod's spec.securityContext, kubelet delegate fsGroup to CSI Driver, and NodeStageVolume() addsgid=<gid>to mount options. This might be not enough to make volume writable for the user:/kind bug
Fixes #835
Release note: