-
Notifications
You must be signed in to change notification settings - Fork 561
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
deploy: Use aggregated ClusterRoles #322
Conversation
@kshlm CI failure |
rbac.ceph.rook.io/aggregate-to-{{ include "ceph-csi-cephfs.nodeplugin.fullname" . }}: "true" | ||
rules: [] | ||
--- | ||
kind: ClusterRole |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kfox1111 PTAL
@kshlm do we need to use |
Not really. I just used the same labels I used in the rook PR. |
oh, this is very interesting... You taught me something new about rbac. Thanks. :) I agree, it feels a little weird for rook to be used as a label here. Please update to be something in the ceph namespace. For the helm chart, please break the two different resources into two different files. The helm convention is to have one object per file. say, nodeplugin-clusterrole.yaml and nodeplugin-rules-clusterrole.yaml Overall, looks to be a good change. Thanks. |
Oh. and please bump the helm chart versions. |
The kubernetes manifests and Helm templates have been updated to use aggregated ClusterRoles. The same change has been done in Rook as well. Refer rook/rook#2634 and rook/rook#2975 Signed-off-by: Kaushal M <kshlmster@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good.
@kfox1111 any final comments from your side ? This looks good to me.. |
deploy: Use aggregated ClusterRoles
Syncing latest changes from upstream devel for ceph-csi
The kubernetes manifests and Helm templates have been updated to use
aggregated ClusterRoles. The same change has been done in Rook as well.
Refer rook/rook#2634 and rook/rook#2975