Secrets field in ListSnapshotsRequest

[hakanmemisoglu]

Right now, CSI specification defines a secrets field in CreateSnapshotRequest to pass credentials around.

However, ListSnapshotsRequest does not have this field, meanwhile listing snapshots might require some credentials with read access to work properly.

I think we need an optional secrets field in ListSnapshotRequest.

[j-griffith]

Do you have an example use case of why list snapshots would need/want secrets? We have the parameter for create volume and snapshot as it's pretty common to need special credentials to create resources on a plugin. I'm not too familiar with restrictive list operations though. Would it be possible to use a reference in the plugin from object creation?

If this is something that's needed it has to be considered for ListVolumesRequest as well.

[travisghansen]

I'm currently planning/designing a csi driver for FreeNAS (and likely various other zfs based storage systems). In an effort to remain as stateless as possible (hopefully totally stateless) I think secrets should be passed not only to this method but any other controller methods that don't currently receive them.

I need to know which appliance I'm dealing with and how to communicate with it (ie: creds).

GetCapacityRequest appears to be missing secrets.
ListVolumesRequest appears to be missing secrets.

ControllerGetCapabilitiesRequest is probably the only exception.

[hakanmemisoglu]

Ceph CSI driver is another one. It doesn't support ListSnapshots because there is no easy way to pass cluster credentials to the method since they switched to a stateless implementation.

[xing-yang]

@hakanmemisoglu Do you also need secrets to support ListVolumes for the Ceph CSI driver? If secret is needed to list snapshots, it should be needed to list volumes as well.

[xing-yang]

I thought only write operations need secrets. List snapshots/volumes are read operations. I didn't expect that they need secrets as well.

[hakanmemisoglu]

@xing-yang, yes you are right. We can include ListVolumes too.

I think all of the read operations might need read credentials depending on the SP. It includes ListVolumes as you noted and maybe others, like GetCapacity as @travisghansen commented.

[shay-berman]

Yes, also IBM storage requires secret to list snapshot, like every csi controller operations that communicates with the storage.

[j-griffith]

I'm currently planning/designing a csi driver for FreeNAS (and likely various other zfs based storage systems). In an effort to remain as stateless as possible (hopefully totally stateless) I think secrets should be passed not only to this method but any other controller methods that don't currently receive them.

I need to know which appliance I'm dealing with and how to communicate with it (ie: creds).

GetCapacityRequest appears to be missing secrets.
ListVolumesRequest appears to be missing secrets.

ControllerGetCapabilitiesRequest is probably the only exception.

So you're proposal is that a secrets parameter be added to every csi request? I see what you're getting at now, storage/cluster creds. I think a number of folks are doing that now by providing credentials to the plugin startup then relying on namespace to filter out access. Anyway, I see what you're aiming at now I think.

[travisghansen]

@j-griffith I'm proposing they be added to all requests for the Controller service. Other use-cases may depend on them for the Node services but I'm guessing that's less an issue for most people.

I think you're on to the design idea yes. I don't really want to deploy the creds with the driver at all, as that requires me to know in advance what appliances may be used etc, etc. Using kubernetes parlance I prefer to store all those details in the StorageClass and each class includes api endpoints/ip address(es), username, password, ssh key, etc that may be required to integrate the given appliance.

Ideally the appliances are fully dynamic from the perspective of the csi driver and do not require re-deploying the driver to support new appliances. The driver simply becomes a conduit for various/appropriate calls but is provided all the details from 'external' configuration.

I do this currently with the crude non-csi provisioner I built over a weekend a year ago (be gentle if you review the code as I also taught myself go over the same weekend). You may be able to get the general idea by reviewing 2 files:

• https://github.com/nmaupu/freenas-provisioner/blob/master/deploy/class.yaml
• https://github.com/nmaupu/freenas-provisioner/blob/master/deploy/secret.yaml

Each class references a given secret and each secret has specific server details. To reiterate, no specific configuration is then deployed with the provisioner itself. When the various requests go into the provisioner the provisioner simply parses the secret name from the class (which is part of the various method inputs), uses the kubernetes api to retrieve the secret and then builds an http client/etc with those details for that specific request. In the csi world all of the class parameters and secrets will need to be combined to be csi secrets to support the same general idea since there is no notion of storage classes, but that same general idea should be feasible (if secrets are always passed along).

Happy to provide more details where helpful.

[j-griffith]

This makes sense, I'm happy to propose it or if you'd like to that's fine as well. Go ahead and assign it to me if you want, or yourself etc.

[travisghansen]

@j-griffith I'm happy to do whatever..not sure what you mean by propose in this context though. Open a new issue?

[ShyamsundarR]

Ceph CSI, as noted, requires secrets to read and list the volumes provisioned.

Further, as discussed by @travisghansen it also needs VolumeCreate parameters (using kubernetes parlance StorageClass parameters). The need being, to identify which storage end point needs to be listed and also to pass and use the appropriate secrets that belongs to the right end point. This is needed as a single Ceph CSI plugin instance can serve storage from different Ceph pools or filesystems, or even different Ceph clusters, based on the VolumeCreate parameters.

IMHO, using the secrets to carry the same parameters as the VolumeCreate parameters, is a workaround and not a solution to the problem. If the parameters could be and hence were passed to the respective calls including the secret, it would be a cleaner interface. Although, hearsay has it that, when the CO invokes ListVolumes/Snapshots it may not have a copy of the parameters to pass.

Alternatives that were considered and dropped look similar to what others have posted,

• Pass the CSI plugin the required details from the parameters and secrets, for all storage end points in use
• Run multiple instance of CSI plugin, one for each storage end point, hence pass the plugin instance only it's parameters and secrets

Both, were found inadequate from a resource intensiveness perspective (more controller and node plugins is a cluster), possibility of sharing multiple end points secrets within the same namespace and/or having to use coarse grained secrets (ones with wider ACLs so to speak) for the provisioning needs.

[travisghansen]

@ShyamsundarR I think my understanding is the same as yours...especially if you're referring to the volume_context.

It appears our storage designs/moving parts are very similar (different pools, clusters, etc, etc) and as a result the design goals of the respective drivers seem similar as well.

Arbitrary volume metadata NOT being stored by the CO and passed around to DeleteVolume etc is a giant PITA with CSI for the type of architecture/design we're coming from (where a bunch of details of the storage system(s) are not implicit as they are in various clouds etc).

This is especially true if you want the driver to remain stateless, less so if not. However, user-expectations are generally that the driver(s) will be deployed in the/a cluster, which results in a classic chicken/egg scenario of:

Driver: hey cluster, I need storage so I can be stateful
Cluster: hey driver, I'll let you know when I've got a driver that can provide storage

This has been discussed previously but if it could be revisited that would be fantastic.

• Why is VolumeAttributes not passed in csi.DeleteVolumeRequest? kubernetes-csi/external-provisioner#87
• Insufficient data on some RPCs #216

For my particular purposes I came up with a hair-brained idea to store metadata like this by setting zfs native properties on the applicable datasets/volumes. This is all theory at this point, but for some operations it's incredibly inefficient since I'll have to scan all child datasets in some scenarios and then 'query' the results in memory. It's not elegant but it keeps the driver stateless.

[j-griffith]

@ShyamsundarR

The need being, to identify which storage end point needs to be listed and also to pass and use the appropriate secrets that belongs to the right end point. This is needed as a single Ceph CSI plugin instance can serve storage from different Ceph pools or filesystems, or even different Ceph clusters, based on the VolumeCreate parameters.

I'm not sure I agree that it's up to the CSI Spec or the CO to know how to deal with aggregate plugins. If you choose to aggregate multiple endpoints/devices behind your plugin the details of how that works are up to you IMHO. Storage class constructs seems to work, but that's outside the scope of the CSI Spec here IMHO.

IMHO, using the secrets to carry the same parameters as the VolumeCreate parameters, is a workaround and not a solution to the problem.

To be fair, the problem statement in this Github issue was precisely a request to add acceptance of the secrets parameter to the list request calls in the spec.

If the parameters could be and hence were passed to the respective calls including the secret, it would be a cleaner interface. Although, hearsay has it that, when the CO invokes ListVolumes/Snapshots it may not have a copy of the parameters to pass.

So what you'd like to see is the credentials used for create basically stored by the CO and have the CO be responsible for including those credentials used at create time for any request operation involving that object (volume or snapshot). I don't necessarily know that this is any different in terms of the interface, you're still passing this information in for the other calls. In other words it would have to be added to the spec and the interface regardless no?

It seems to me that this is more about catering to a stateless solution for plugins that aggregate multiple storage devices or endpoints so I guess it needs a good deal more discussion. It seems storage classes isn't doing what you want here.

[j-griffith]

@j-griffith I'm happy to do whatever..not sure what you mean by propose in this context though. Open a new issue?

I meant propose a change to the spec via PR, but with the further context it sounds like this isn't what you really want anyway. Instead it seems that you want the CO to store creds from the create requests and pass them through to subsequent calls to your plugin. That's still going to require something passed to the plugin, although there other ways to do that besides the API Interface (not saying that's the right or wrong answer here).