Consider creating secrets in plugins

[gman0]

This proposal is intended to open a discussion about the feasibility of making Controller Plugins able to create their own secrets (generated in CreateVolume/ControllerPublishVolume RPCs), and then passing those secrets to Node Plugins - if this is something worth pursuing or not.

There's already a way of passing non-sensitive data from the Controller Plugin to the Node Plugin via CreateVolumeResponse's attributes. Passing secrets in similar fashion would complement this feature.

I'll start with two use-cases, both of them are related mainly to Ceph:

1. csi-cephfs plugin is able to provision and mount new CephFS shares. As a security measure, for each provisioned share it also creates a dedicated Ceph user who can access this share only. When attaching/mounting such share to a workload, the Plugin has to provide a keyring containing the user's credentials.

   It would be really nice if the Controller Plugin, once the volume is created, would be able to pass those credentials to the CO, and the CO would in turn pass those to the Node Plugin when needed.

2. csi-manila is able to provision shared FS volumes managed by OpenStack's Manila service. Each provisioned volume needs a dedicated access and the Controller Plugin may need to expose sensitive data to make accessing the volumes from the Node Plugin possible.

So far I see two ways of achieving this:

1. Amending the contents of existing "node secrets" fields

   Add a new optional field amend_node_secrets to either CreateVolumeResponse or ControllerPublishVolumeResponse. If non-empty, coalesce this field with node_stage_secrets and node_publish_secrets.

   Or

   Add new optional fields amend_node_stage_secrets, amend_node_publish_secrets to either CreateVolumeResponse or ControllerPublishVolumeResponse. If non-empty, coalesce these fields with node_stage_secrets and node_publish_secrets respectively.

   E.g. read stage volume secrets, read the amending stage secrets, unify both into a single string-to-string map and pass the result into node_stage_secrets.

2. Creating separate fields for plugin-supplied secrets

   Add a new optional field provision_secrets to CreateVolumeResponse (or ControllerPublishVolumeResponse), NodeStageVolumeRequest and NodePublishVolumeRequest messages. Populate those similarly to option (1), only without coalescing.

   Or

   Add new optional fields provision_node_stage_secrets and provision_node_publish_secrets to CreateVolumeResponse (or ControllerPublishVolumeResponse). Add those also to NodeStageVolumeRequest and NodePublishVolumeRequest messages respectively.

The CO is responsible for managing the life-time of the secret and possibly making sure the keys of the "amending" secret don't collide with existing keys from "node_*_secrets" in option (1).

The life-time of the secret would span from CreateVolume/ControllerPublishVolume till DeleteVolume/ControllerUnpublishVolume respectively. The CO has to create the secret, track the mapping between the secret and its corresponding volume, so that it can be disposed of.

Failing to create the secret (either as a result of CO's internal error or because of conflicting keys in option (1)) would result in an error, calling DeleteVolume/ControllerUnpublishVolume and retrying.

Failing to delete the secret should be a bit more permissive. If the secret doesn't exist, the CO should continue with DeleteVolume/ControllerUnpublishVolume normally. Otherwise issue an error and try deleting again.

Is this something that would fit CSI?

cc @saad-ali @jdef

[jdef]

Thanks for sharing this use case. I don't think we've considered a case where a plugin itself is generating secrets that need to be passed around. Coalescing fields is somewhat dangerous for a CO to do, namely because keys/values are opaque and it won't know how to handle possible key collisions.

What other design approaches are being considered?

[gman0]

The second option - where the plugin-supplied secrets would have their own fields. The two obvious approaches for this are:

• having a common field provision_secrets (or some other name?) in both NodeStageVolumeRequest and NodePublishVolumeRequest
• finer-grained control: having separate secret fields for NodeStageVolumeRequest and NodePublishVolumeRequest

This option has the advantage of being the least surprising - the SP knows exactly which secrets are coming from where and the CO doesn't have to deal with possible key collisions. On the other hand, it clutters the spec more than the "coalescing" option by adding more fields. Having said that, it's clearer and safer from CO's perspective to not having to deal with the actual contents of the maps, as you pointed out.

[jdef]

I should have been more clear w/ respect to my question. Other than all of your proposed changes to CSI, what other design changes are being considered in order to accomplish your goals?

[gman0]

Ah sorry, yes there's one more thing but I'm afraid it might be too CO specific, let's see. This one is more about architecture - separation of concerns and less code duplication in plugins. It essentially tries to emulate Kubernetes' PersistentVolumeSource in CSI.

The main use-cases for this are OpenStack's Manila and Cinder services, but any such scenario where a single Controller Plugin acts as a multiplexer and is able to provision volumes of multiple kinds could benefit from this.

I'll describe the scenario with Manila: the Manila service can provision shared file-systems for CephFS, HDFS, GlusterFS, NetApp, etc. There are specialized CSI plugins for (at least, at the moment) CephFS, NetApp storage. The idea is to accommodate a Controller-only plugin (i.e. the "multiplexer") which would only provision the volume + instruct the CO which Node plugin (i.e. the "specialized" one) should be used when placing the volume to workload, granted all the necessary volume attributes are in place, as well as any possible secrets. This is as opposed to what e.g. csi-cinder is doing which, due to CSI limitations, has to do both provisioning multiple kinds of block-storage and attaching&mounting on its own - I'd say reusing already existing plugins that deal with a specific FS is cleaner than dealing with everything in one place.

Plugin name is a well-established entity in Identity Service so this should be ok as an identifier, and I'm guessing at some point each CO has to decide where to route the Node Service RPCs. This is the place where the plugin name from Controller plugin could be injected. A new optional field node_plugin_name could be added to CreateVolumeResponse which, if non-empty, must contain a valid Identity Service's plugin name, and it instructs the CO which Node plugin shall be used for this volume. If left empty, the behaviour stays the same as it is now.

[jdef]

@gman0 controller-only plugins (that route to various node plugin backends) sounds like a topic that should have its own issue to track discussion; it has very little to do with the original topic of this issue.

[gman0]

Sure, I'll open a new issue for it. Other than that, no other design changes are required regarding the secrets.

[jdef]

Thanks. I'm trying to understand if there are design alternatives being considered w/ respect to the original topic of this issue OTHER than modifying CSI for the additional secrets handling that you've proposed here.

…

On Tue, Aug 7, 2018 at 1:41 PM Róbert Vašek ***@***.***> wrote: Sure, I'll open a new issue for it. Other than that, no other design changes are required regarding the secrets. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#261 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPVLHWd19dLItue0AbxlBCvgfsBsEXiks5uOdE0gaJpZM4VwO_j> .

[gman0]

@jdef In simpler cases this could be circumvented by having some sort of a shared vault - I believe I even saw some mention of WIP csi-vault with secrets-as-filesystem, but such ad-hoc approach would be problematic at least for #263. I realize that managing secrets in large is quite out-of-scope for CSI and there are not that many storage providers that require such feature, but both attributes and secrets are a part of what describes a volume so IMHO at least some passing around of those should be made possible.

[saad-ali]

Seems like a good idea. Let's add this in a backwards compatible way post-1.0