implement Ephemeral Inline Volumes support

[andyzhangx]

Is your feature request related to a problem?/Why is this needed

Describe the solution you'd like in detail

refer to
https://kubernetes-csi.github.io/docs/ephemeral-local-volumes.html#which-feature-should-my-driver-support

Describe alternatives you've considered

Additional context

[boddumanohar]

Generic ephemeral volumes feature was alpha in Kubernetes 1.19. A CSI driver is suitable for generic ephemeral inline volumes if it supports dynamic provisioning of volumes. It creates PVCs underneath so any CSI driver that supports PVCs will work without any special support. As dynamic volume provisioning feature is already there in csi-driver-nfs, I think we already have support for this.

more info: https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/#generic-ephemeral-volumes

[boddumanohar]

should we rename this issue to implement Generic ephemeral volumes support?

[kfox1111]

@boddumanohar that is a different feature. That is the ability to ask for temporary volumes with no details.

inline ephemeral is needed to replace the built in functionality to directly mount an nfs volume by server:/path specified in the pod spec without making a pv

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[kfox1111]

/remove-lifecycle rotten

[kfox1111]

Still valid.

[jsafrane]

Why is this valid? The linked document says:

A CSI driver is not suitable for CSI ephemeral inline volumes when:

• provisioning is not local to the node
• ephemeral volume creation requires volumeAttributes that should be restricted to an administrator, for example parameters that are otherwise set in a StorageClass or PV. Ephemeral inline volumes allow these attributes to be set directly in the Pod spec, and so are not restricted to an admin.

There is no storage class, so the only option is to pass NFS server name + base path via VolumeAttributes. That could allow anyone who can write a pod to access any NFS server reachable to a node. That does not sound right.

I don't think a generic NFS driver should implement in-line ephemeral volumes. Users should use Generic Ephemeral Inline Volumes and the driver "implements" that already (for the driver it's just a generic PVC + dynamically provisioned PV).

[kfox1111]

There are sites out there that have existing NFS volumes, that are currently being used with the existing pod spec like:

spec:
  volumes:
    - name: nfs-volume
      nfs: 
        # URL for the NFS server
        server: 10.108.211.244
        path: /

Yes, this functionality can be a security issue, but when tied in with a policy by PSP, OPA Gatekeeper, or Kyverno, it can be safe for those allowed to use it. If the nfs csi driver is to fully take over from what the current podspec supports, it will need to support this access mode.

[jsafrane]

I can't speak for 3rd party policy management tools, IMO they should not be required to run a safe Kubernetes deployment.
PSP is being replaced. The new Kubernetes pod security standards allows in-line CSI volumes in the restricted policy, assuming:

• Inline CSI volumes should only be used for ephemeral volumes
• The CSIDriver object spec controls whether a driver can be used inline, and can be modified without binary changes to disable inline usage.
• Risky inline drivers should already use a 3rd party admission controller, since they are usable by the baseline policy.
• We should thoroughly document safe usage, both on the documentation for this (pod security admission) feature, as well as in the CSI driver documentation.

CSIDriver.Spec is IMO not very suitable, it's read-only after creation and who knows who creates it and how (vendor's helm chart, operator, shell script...) In addition, it does not allow any policy about who can use the driver's volumes in-line in pods or not.
Do you plan to ship a 3rd party admission webhook and NFS driver specific policy then?

If the nfs csi driver is to fully take over from what the current podspec supports, it will need to support this access mode.

It depends. With restricted pod security standard, no in-line volume can be used in pod except for ephemeral ones. NFS is not an ephemeral one and should be out. Most of the other CSI drivers that replace in-tree volume plugins don't implement in-line volumes. I am trying to steer the other away from in-line volumes. Having persistent volumes in-line in pods was a mistake we're trying to fix here. I know PVs / PVCs are not great for volumes shared across namespaces, still, it's IMO better / safer than the pod security standard today.

[kfox1111]

Whatever the replacement is for PSP, (Still unconvinced thats a good idea), it has to deal with the existing functionality where inline ephemeral csi supporting drivers may or may not be safe. There are probably existing drivers supporting unsafe usage.

That being said, I do have a custom webhook that currently validates explicit nfs volumes/config in the pod spec and lets it through under certain safe conditions. I would still need to do that kind of thing going forward.

[andyzhangx]

Whatever the replacement is for PSP, (Still unconvinced thats a good idea), it has to deal with the existing functionality where inline ephemeral csi supporting drivers may or may not be safe. There are probably existing drivers supporting unsafe usage.

That being said, I do have a custom webhook that currently validates explicit nfs volumes/config in the pod spec and lets it through under certain safe conditions. I would still need to do that kind of thing going forward.

@kfox1111 partially agree with you, for example, we have azure file in-tree driver which provides inline volume, and after strong requirements from users (and some debates), finally we provided exact same inline volume func in azure file CSI driver. The security issue sometimes depends on how users use inline volume, if we don't provide inline volume func, it must be more "secure", while it would lose feasibility.

[andyzhangx]

@jsafrane What about add a feature flag in this driver to control whether inline volume is supported or not? If some users want more secure way, they could turn off inline volume support when deploying the nfs csi driver.

[kfox1111]

Every driver would have to add their own hooks or controls for it. That becomes an sysadmin and dev nightmare. The filtering feature really belongs in the PSP replacement IMO.

[kfox1111]

@msau42 What do you think? We worked on getting the ability for ephemeral volumes to work stipulated on PSP's being able to restrict access to unsafe drivers. How does this work safely with the PSP replacement?

One idea might be to in the CSIDriver spec to have new flags saying what built in profiles would be safe to use ephemeral with.

I'm still feeling quite restricted by what is being deprecated in PSP though... My clusters will want NFS ephemeral enabled but only with a validating webhook to ensure a restricted set of params. So the CSIDriver spec flags feel like maybe too coarse grained.

[msau42]

Now that we have the generic ephemeral volumes feature, I'm not sure there is a need for general purpose drivers to support the csi inline drivers feature. I would strongly discourage general purpose storage drivers from supporting csi inline volumes, and only encourage it for drivers that are able to expose attributes safely.

[kfox1111]

General drivers, no. nfs is not a general driver though.

Inline nfs is definitely needed for certain use cases, especially when needing to integrate existing (sometimes hpc) systems with Kubernetes. Its still a supported upstream use case as well. If its not supported in csi-driver-nfs, then its likely to get forked and will be supported in that driver and will be almost identical code.

I think at this point, a question is, can it be done in the same codebase, or does a fork need to happen here? My preference would be here.

The rest of my question, its more around the fact that ephemeral inline was specifically designed with psp protections in place, and then the kubernetes project is throwing that api away without (I think) defining how that use case is maintained in the new system. I think that still needs to be addressed rather then just say "don't do that". One of the great design features I think of Kubernetes is trying to separate policy from ability. This feels like one of those use cases. Most people would want such feature disabled by policy by default, but the ability to use it for some folks is important. So its a policy thing.

[kfox1111]

I found the relevant section: https://github.com/kubernetes/enhancements/pull/2582/files#diff-404b549b970315ca9f80146384602337b076b9dec92b7b5e17d63fbe700aa67eR542-R549

Looks like the default is the new replacement lets all inline ephemeral drivers through, then lets a webhook decide to filter, or it can be enabled/dislabed at the CSIDriver level. The CSIDriver is an all or nothing thing. So wouldn't support existing pods that may be run as unrestricted while still restricting other pods. So a webhook may be more the way to go. I don't know how the webhook could determine the profile of the pod though.Anyone know if that is passed through somehow?

[andyzhangx]

I have worked out a PR to enable inline volume, the main change is add - Ephemeral in CSIDriver, and we could leave this option to user, if user sets feature.enableInlineVolume as true, then inline volume is enabled, if they want more secure way, then just set as false, everyone would be happy with this solution?

csi-driver-nfs/charts/latest/csi-driver-nfs/templates/csi-nfs-driverinfo.yaml

Lines 9 to 11 in 16473fb

	{{- if .Values.feature.enableInlineVolume}}
	- Ephemeral
	{{- end}}

[andyzhangx]

any feedback on this PR? #266

[jsafrane]

So a webhook may be more the way to go. I don't know how the webhook could determine the profile of the pod though.Anyone know if that is passed through somehow?

Similarly to Pod Security Standards. The webhook gets a Pod, checks what level is in pod's namespace labels (restricted/baseline/privileged), check the driver's policy (I guess that will be different for each driver?) and either admit or reject the pod.