Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csi: revisit rbac rules, add Snapshotter sidecar and roles #104

Merged
merged 1 commit into from
Nov 19, 2018

Conversation

fatih
Copy link
Contributor

@fatih fatih commented Nov 19, 2018

No description provided.

@fatih fatih changed the title csi: revist rbac rules, add Snapshotter sidecar and roles csi: revisit rbac rules, add Snapshotter sidecar and roles Nov 19, 2018
* 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/...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this?

Copy link
Contributor Author

@fatih fatih Nov 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add to this comment some documentation about how other devs need to proceed to get this working (eg "apply this section separately after XYZ")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need these permissions to create snapshots?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:/kubernetes-csi/external-snapshotter/blob/master/deploy/kubernetes/rbac.yaml

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

https:/kubernetes-csi/external-snapshotter/blob/a2f8b41c08d7d795ba08ca8a87b942fb9b5dac44/cmd/csi-snapshotter/main.go#L107-L111

@aybabtme
Copy link

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants