Adding PVC's annotation to CreateVolumeRequest.Parameters

[orainxiong]

As the title says, there is a great possibility that the application-specific Operator would leverage PVC's Annotation for storing custom data. The custom data will tell storage provisioner how CSI drive create volume correctly.

Here is already existing logic for provision volume :

    // Create a CSI CreateVolumeRequest and Response
    req := csi.CreateVolumeRequest{

        Name:       pvName,
        Parameters: options.Parameters,
        VolumeCapabilities: []*csi.VolumeCapability{
            {
                AccessType: accessType,
                AccessMode: accessMode,
            },
        },
        CapacityRange: &csi.CapacityRange{
            RequiredBytes: int64(volSizeBytes),
        },
    }

Note : more details

CSI Plugin specifies that CreateVolumeRequest's field Parameters is OPTIONAL and opaque. What I am suggesting is that we could pass in PVC's Annotation in addition to Storageclass.Parameter.

[msau42]

Yes setting PV.ClaimRef is the recommended way

[haibinxie]

@msau42 is volumeHandle in PV object gets passed to CSI CreateVolumeRequest.Name so CSI driver just needs to check whether the volume exists or not, and import if it exists.

if that's the case can we also fill it somewhere in PVC to be carried to CreateVolumeRequest.name?

[msau42]

@haibinxie if you manually create the PV then CreateVolume does not get invoked. Can we open another issue to discuss this import case? This is veering off topic from the original issue.

[ofek]

Has there been any update on implementing this feature? We would greatly appreciate it.

cc @saad-ali

[eladtamary]

We also use tags to calculate the cost of each environment. Therefore we need a way to tag AWS volumes created by CSI external provisioner.

Until there will be a proper way to define the tags, I use this script to go over all un-tagged volumes and tag them according to the attached instance tag: https://gist.github.com/eladtamary/49de1406e2605d2d659cbcd0b420eba1 Maybe it will help someone else.

[fejta-bot]

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

[msvticket]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

[ofek]

/remove-lifecycle stale

[pohly]

/lifecycle frozen

[silentred]

This argument extra-create-metadata passes pvc ns/name to CreateVolumeRequest.Parameters. This is really useful.

[ppenzo]

I need to map existing shares of NAS filers to PV without the cluster/storage admins intervention #https://github.com/kubernetes-csi/csi-driver-smb/issues/398 hence placing the sharename in the PVC would be the solution to achieve this.

If using an annotation is not suitable since it would break the CSI specs, would it be possible to use an extra parameter as described in #714 to pass parameters directly to the driver?

[mloskot]

This would also help to tag Azure file share/disk by overriding parameters.tags of StorageClass based on Azure CSI drivers Interestingly, there are CSI drivers which apparently provide support for PVC annotations to override SC parameters: https://scod.hpedev.io/csi_driver/using.html#using_pvc_overrides

[andyzhangx]

since passing pvc annotation to CreateVolumeRequest.Parameters is not supported by external-provisioner, I see there is other project(e.g. HPE CSI Driver) trying to cache all pvc and then pass pvc annotation to the CSI driver, not a graceful implementation, but looks like this is the only way now: https://github.com/hpe-storage/csi-driver/blob/abf94a40b242477e51afac75bb9a68772aede0e7/pkg/flavor/kubernetes/flavor.go#L163-L172

[egoldblum]

We're also in need of a feature like this to be able to associate disks with applications to better track costs.

[sathieu]

This PR: https://github.com/kubernetes-csi/external-provisioner/pull/1239

[Garagoth]

Hi!

I came here from https://github.com/kubernetes-csi/external-provisioner/pull/808, but since it is closed and several people there advised to comment further here and not on a closed PR I am writing here.

Firstly I would like to address main concerns from PR that were raised:

• "it makes PVCs not portable" Well, yes, but this already happened. You can pass secret names as annotations from PVC to StorageClass (https://kubernetes-csi.github.io/docs/secrets-and-credentials-storage-class.html#per-volume-secrets). Also, maybe this prompts to extend PVC/PV/SC specification to accommodate such parameters in new resource version? Changing StorageClass driver to different CSI driver would not be made difficult by that as well - either it would use annotation in subDir (as administrator permits and configures explicitly) or not and user would get share with subDir that simply ignores annotation. There are other resources (like ingresses) that are not portable in same way because they use implementation-specific annotations to pass extra info. I am aware that this is a very poor excuse :-).
• "It opens doors for security issues (...) anyone who can create a PVC can actually read data of other PVs that are dynamically provisioned from the same StorageClass, just by guessing the subdirectory name." It is right now possible to start guessing secret name and secret namespace that are passed as annotations on PVC and possibly elevate share privileges from RO to RW. Or, in situation where already supported subDir: '${pvc.metadata.name}' can make PVC name collide with PVC from another namespace and effectively mount their share. Configuration wise there are plenty of places that one can already create a security hole. But configuring SC subDir: '/prefix/${namespace}/${pvc.annotations['csi.storage.k8s.io/subDir']} does not allow much wiggle room (as opposed to subDir: '${pvc.annotations['subDir']}'). Kubernetes allows a lot of freedom and opportunity to shoot own foot in plenty other places by incorrect configuration, but as with every tool some wisdom in configuration/usage is required. There is a security concern here that was not addressed - path traversal using .. but it is easy to fix.

This issue is open for 6 years already and I think that most PR rejections that tried to implement this failed on portability, meanwhile several other drivers struggle to work around this and implement their own mechanisms (HPE, NetApp, Linode, Digitalocean, ...) based on PVC annotations. Several CSI users voiced their need for configurable/template-able subDir in specific drivers (smb and nfs among others) and it is really hard to meet externally enforced directory structure on shared storage (or impossible in some cases). There other use cases out there that could benefit from this (like different uid/gid for each PVC among other parameters, which could be at least temporarily implemented using annotations) Even in kubernetes-sigs there is a project that walks around this: https://github.com/kubernetes-sigs/nfs-subdir-external-provisioner with pathPattern...

To sum up: What can be done to move this issue forward (fully or partially, like passing annotations but only those prefixed with csi.storage.k8s.io/ to avoid accidentally leaking everything to driver - that would satisfy need for tags and could allow more subDir templating options)? Any possibilities at all or any other ideas what could be done to support use cases that people have? Is it possible to change PVC to add new stuff there that would be portable (and make new version of resource) - like parameters section (but that would still be a freeform and driver specific meaning, same as parameters on StorageClass)? There are plenty of examples where annotation-based config migrated to proper resources in subsequent resource versions.

Cheers!