feat: add RBAC for OpenShift SecurityContextConstraints #272
Conversation
Signed-off-by: Adam Talbot <[email protected]>
cert-manager-prow
bot
added
dco-signoff: yes
…late Signed-off-by: Adam Talbot <[email protected]>
deploy/charts/csi-driver/values.yaml
Outdated
| # Name if the SecurityContextConstraints to create RBAC for. | ||
| name: privileged |
There was a problem hiding this comment.
nitpick: spelling
| # Name if the SecurityContextConstraints to create RBAC for. | |
| name: privileged | |
| # Name of the SecurityContextConstraints to create RBAC for. | |
| name: privileged |
There was a problem hiding this comment.
Fixed, I have made this exact typo so many times 🤣.
| {{- $securityContextConstraintImplicitlyEnabled := and (kindIs "string" .Values.openshift.securityContextConstraint.enabled) (eq .Values.openshift.securityContextConstraint.enabled "detect") (.Capabilities.APIVersions.Has "security.openshift.io/v1") }} | ||
| {{- $securityContextConstraintExplicitlyEnabled := and (kindIs "bool" .Values.openshift.securityContextConstraint.enabled) (.Values.openshift.securityContextConstraint.enabled) }} | ||
| {{- $securityContextConstraintEnabled := or $securityContextConstraintImplicitlyEnabled $securityContextConstraintExplicitlyEnabled }} |
There was a problem hiding this comment.
nitpick: I think this logic makes sense to me but given it's Helm template logic it's quite busy.
Could we add some template comments here in this file explaining what the logic is doing and why? I imagine users will quite often check the roles / clusterroles in a chart to see what permissions are enabled and especially for permissions which have "security" in them it would be helpful to be really clear about what's going on in the template
There was a problem hiding this comment.
I have added some comments to make this clear
Signed-off-by: Adam Talbot <[email protected]>
There was a problem hiding this comment.
/lgtm
/approve
Tested locally, this looks correct to me!
$ helm template csi-driver ./deploy/charts/csi-driver --api-versions security.openshift.io/v1
# contains the securitycontextconstraints
$ helm template csi-driver ./deploy/charts/csi-driver --set openshift.securityContextConstraint.enabled=true
# contains the securitycontextconstraints
$ helm template csi-driver ./deploy/charts/csi-driver --set openshift.securityContextConstraint.enabled=false --api-versions security.openshift.io/v1
# doesn't contain it
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SgtCoDFish The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cert-manager-prow
bot
added
the
approved
The CSI driver needs higher than normal perms, this falls foul of the default OpenShift SecurityContextConstraints.
This updates the helm chart to include RBAC to bind to the "privileged" SecurityContextConstraints.
This is what I found an official CSI driver doing, so it seemed like the correct behaviour:
https://github.com/search?q=repo%3Aopenshift%2Fsecrets-store-csi-driver-operator%20%20SecurityContextConstraints&type=code