Add e2e permissions test #45
Conversation
vladem
force-pushed
the
permissions-e2e
branch
from
2fc4331 to
dc8487c
Compare
| pDriver, _ := config.Driver.(storageframework.PreprovisionedPVTestDriver) | ||
| r.Volume = pDriver.CreateVolume(ctx, config, storageframework.PreprovisionedPV) | ||
| pvSource, volumeNodeAffinity := pDriver.GetPersistentVolumeSource(false, "", r.Volume) | ||
| pv := &v1.PersistentVolume{ |
There was a problem hiding this comment.
We talked about this a bit in standup, but is this (mounting a pv with options) significantly easier using the k8s library if we have more standard mount options? Do you know if many customers will do this programmatically?
There was a problem hiding this comment.
The library in question is k8s.io/kubernetes/test/e2e/storage/framework which is testing only.
The way in which we retrieve mountOptions from the NodePublishVolume request (req.GetVolumeCapability().GetMount().GetMountFlags()) is common enough among other csi drivers:
- our way: https://github.com/awslabs/mountpoint-s3-csi-driver/blob/main/pkg/driver/node.go#L83
- alibaba cloud csi driver: https://github.com/kubernetes-sigs/alibaba-cloud-csi-driver/blob/a2bc48af6f21c00136d1e722b7a16097636d9128/pkg/local/nodeutils.go#L93
- azure blob storage: https://github.com/kubernetes-sigs/blob-csi-driver/blob/e995ab093af3883320c0f78b4d68f6bc72605547/pkg/blob/nodeserver.go#L238
- gcp file store: https://github.com/kubernetes-sigs/gcp-filestore-csi-driver/blob/e0efd30d0e2ab793003bf0d2f7a1ee2450ffc6e9/pkg/csi_driver/node.go#L155
- secrets store: https://github.com/kubernetes-sigs/secrets-store-csi-driver/blob/ac51f4738d370d54f2e185f70cc27fcaa650bdb6/pkg/secrets-store/nodeserver.go#L115
- nfs: https://github.com/kubernetes-csi/csi-driver-nfs/blob/master/pkg/nfs/nodeserver.go#L55C25-L55C51
| l.config = driver.PrepareTest(ctx, f) | ||
| ginkgo.DeferCleanup(cleanup) | ||
| }) | ||
| ginkgo.It("should access volume as a non-root user", func(ctx context.Context) { |
There was a problem hiding this comment.
A negative test (non-root can't access without the allow-other flag) would be great as well. Fairly low priority though- we're more testing mountpoint functionality at that point.
| ginkgo.DeferCleanup(cleanup) | ||
| }) | ||
| ginkgo.It("should access volume as a non-root user", func(ctx context.Context) { | ||
| resource := createVolumeResourceWithMountOptions(ctx, l.config, pattern, []string{"uid=1000", "gid=2000", "allow-other"}) |
There was a problem hiding this comment.
Do you actually need the uid/gid options? I think that will set ownership on the files right? But non-root should be still be able to access root owned files as long as you have allow-other (assuming file permissions are set appropriately. Not suggesting a change here, just trying to make sure I understand how all this works.
There was a problem hiding this comment.
haven't tested that yet, but interesting point; according to fuse and mountpoint documentation customer can use --file-mode and --dir-mode flags instead of --uid and --gid to keep files owned by the root, but make them accessible by other
There was a problem hiding this comment.
MP uses default_permissions mount flag, which delegates permissions checking to the kernel; it can be the case that some additional permission checking is done on top of that in MPs code
| @@ -0,0 +1,50 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
can we add documentation about this in our examples README?
Description of changes:
TODO: What we do not support is setting volume's group owner according to
fsGroupoption from pod security context:By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.