document ClusterRole Secret access

[sepich]

Describe the problem/challenge you have
We are investigating migration to your project from metal-stack/csi-driver-lvm.
Unfortunately with more features it seems also comes more complexity, and we cannot find reasons for that:

1. Most important question is why do you need permission to read all cluster Secrets?
   

   lvm-localpv/deploy/helm/charts/templates/rbac.yaml

   Line 18 in db17d08

   resources: ["secrets"]

   
   We cannot find any mentions for this in docs, and in go code

2. Another thing which seems poorly documented is why do you need custom CRDs?
   Putting aside snapshots, which we do not use. What kind of problem do you solve with lvmnodes.local.openebs.io and lvmvolumes.local.openebs.io? Why do you need separate controller and state, which can just be stored directly on nodes.

Describe the solution you'd like
It is nice you have ability to disable crd.volumeSnapshot in your helm chart.
Would be nice to also have ability to disable Secrets permissions and the rest of CRD, and clearly understand what would it break.

Thank you.

[abhilashshetty04]

Hi @sepich , Thanks for raising the issue.

2. This was the design decision in the zfs local pv as well. By having resources like lvmvolume and lvmnode as CR we make sure:
   a. We keep node and controller component completely stateless. Rely on k8s etcd to store the resource objects.
   b. We dont have to have gRPC CRUD interface between controller and node. Since its controller driven, core storage operations are driven by CR state.
   c. This makes lvm-localpv completely k8s native in a way. users can investigate storage resource with kubectl plugin.

3. We already have the support to disable snapshot cr install. Part of following PR
   feat(release): add changes for v1.2.0 release #243
   https://github.com/openebs/lvm-localpv/blob/develop/deploy/helm/charts/values.yaml#L171C20-L171C20

[abhilashshetty04]

For the rbac query, We will raise a PR to remove the rule. Do you want to discuss 2nd and 3rd query? Were you able to disable snapshot crd?

[abhilashshetty04]

Hi @sepich , We have removed rbac rule for secrets in #277. We will proceed with closure of this issue for now. Please let us know incase you have more questions.

[sepich]

You only removed Secrets from the first ClusterRole, but they are still in the second:

lvm-localpv/deploy/helm/charts/templates/rbac.yaml

Line 86 in c6dbe6a

resources: ["secrets"]

And why does CSI driver needs permission to Services?

lvm-localpv/deploy/helm/charts/templates/rbac.yaml

Line 21 in c6dbe6a

resources: ["persistentvolumes", "services"]

[abhilashshetty04]

@sepich , Thanks for replying. We will look into these rules. Would you mind raising a PR with all changes that you are suggesting.

Moreover, Have you considered using Mayastor for your storage deployment. Its more actively developed. Its faster , Completely written in rust, Uses faster Nvme Tcp stack.

You could find more information here if you are interested.
https://mayastor.gitbook.io/introduction/