csi: revisit rbac rules, add Snapshotter sidecar and roles #104
Conversation
fatih
changed the title
* Most of the rules are now removed and will not be part of upcoming k8s releases. Going forward drivers have to install themself. * Add `csi-snapshotter` sidecar, needed to handle Volume snapshots * This also adds the necessary roles and bindings needed for the csi-snapshotter sidecar.
| @@ -47,7 +47,7 @@ test: | |||
| test-integration: | |||
|
|
|||
| @echo "==> Started integration tests" | |||
| @env GOCACHE=off go test -v -tags integration ./test/... | |||
There was a problem hiding this comment.
Because go modules won't work if you turn off GOCACHE
| @@ -118,6 +118,20 @@ provisioner: dobs.csi.digitalocean.com | |||
|
|
|||
| --- | |||
|
|
|||
| # NOTE(arslan): this will probably fail , because the CRD is created via the | |||
| # csi-snapshotter sidecar, that is part of the csi-do-controller statefulset. | |||
| # We need to create this seperately. | |||
There was a problem hiding this comment.
Add to this comment some documentation about how other devs need to proceed to get this working (eg "apply this section separately after XYZ")
There was a problem hiding this comment.
This comment will be removed in upcoming PR's it's just here to remind myself.
| resources: ["volumesnapshots"] | ||
| verbs: ["get", "list", "watch", "update"] | ||
| - apiGroups: ["apiextensions.k8s.io"] | ||
| resources: ["customresourcedefinitions"] |
There was a problem hiding this comment.
why do you need these permissions to create snapshots?
There was a problem hiding this comment.
The csi-snapshotter sidecar needs them to create the VolumeSnapshot and VolumeSnapshotClass custom resource definitions.
These are pulled from the appropriate repo, each sidecar now contains the RBAC rules it needs to operate, as an example for the above: https://github.com/kubernetes-csi/external-snapshotter/blob/master/deploy/kubernetes/rbac.yaml
There was a problem hiding this comment.
Hm I see that the binary tries to create its own definition. Seems wrong to me but until patched, I guess we have no alternative:
|
|
No description provided.