jenkinsci / helm-charts

Jenkins helm charts
https://artifacthub.io/packages/helm/jenkinsci/jenkins
Apache License 2.0
561 stars 889 forks source link

Add support for custom RBAC roles or cluster roles #232

Open jalaziz opened 3 years ago

jalaziz commented 3 years ago

Is your feature request related to a problem? Please describe.

This issue has been brought up before in the helm repo (see helm/charts#1092 and helm/charts#13458).

In our case, the issue is we have a Jenkins job that uses helm to deploy a service to test against. When the k8s cluster has RBAC enabled, the default Jenkins service account does not have the necessary permissions to deploy a helm chart.

Describe the solution you'd like

It would be great if we could either (a) provide a custom role with rules to be deployed when creating the service accounts, or (b) provide an option to bind th agent service account to an appropriate cluster role.

I understand that giving the agent cluster role permissions can negate the fine-grained permissions of the main Jenkins service account. However, adding support for defining the namespace for the role binding can help ensure that the permissions are only granted for a namespace dedicated to the agents.

Describe alternatives you've considered

For now, I've manually added a RoleBinding to the agent's service account for the edit role. However, it would be great be great to codify this as part of the helm chart values.

Additional context

I'd be happy to contribute a solution to this, but I'd like some input to what the ideal approach would be. The most flexible approach might be to allow fully custom roles and role bindings to be provided via the values.yaml, but a structured approach may ultimately be better.

cmoulliard commented 3 years ago

Supporting such an option is needed as it will allow the jenkins master to execute commands to figure out by example what it the Node IP address that by default the RBAC role dont support

io.fabric8.kubernetes.client.KubernetesClientException: 
Failure executing: GET at: https://10.96.0.1/api/v1/nodes. Message: Forbidden!
Configured service account doesn't have access.
Service account may have been revoked.
nodes is forbidden: User "system:serviceaccount:ci:jenkinsci" cannot list resource "nodes" 
in API group "" at the cluster scope
torstenwalter commented 3 years ago

The helm chart already has quite many configuration options. I am not sure if this use case it worth being added. It's possible to create additional roles outside of the chart and add those via rolebindings to the service account. If you want to manage these via helm you could just create a new helm chart let's call it my-jenkins and reference this helm chart as dependency. That way you can add whatever resources you wish.

If permissions are only needed for build agents I would even argue not to add those permissions to the jenkins service account, but to a service account which is only used by agents. That's also the reason why this chart allows to run agents in different namespaces then the controller to reduce security attack surface.

cmoulliard commented 3 years ago

If you want to manage these via helm you could just create a new helm chart let's call it my-jenkins and reference this helm chart as dependency.

This is a bit overkill to create a chart, publish it under a repository to simply create 3 K8s resources: role, rolebinding, sa or clusterrole, clusterrolebinding and sa. WDYT ?

How can you then define such a dependency as the owner chart to be deployed is this jenkinsci chart ?

@torstenwalter

jalaziz commented 3 years ago

If permissions are only needed for build agents I would even argue not to add those permissions to the jenkins service account, but to a service account which is only used by agents. That's also the reason why this chart allows to run agents in different namespaces then the controller to reduce security attack surface.

Are you really reducing the attack service though? Using a different namespace for agents still requires the controller to have pretty broad permissions in the agent's namespace. Sure, the controller doesn't need access to secrets, but it's not like the pod scheduling permissions are limited to just the agent pods. You can still do a fair amount of damage with those permissions.

Even if we want to use agent namespacing, AFAICT only one agent namespace is supported, so that doesn't help us when trying to deploy to multiple namespaces. It simply punts the same problem down to the agent service accounts.

Yes, it would be better to attach the permissions to the agent service accounts, but the problem still stands of where do we add those permissions?

Sure, we could manage them outside of this helm chart and even use our own helm chart, but as @cmoulliard said, it's a bit overkill for 3 resources when this chart can and is already creating them (they're just not customizable).

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

torstenwalter commented 3 years ago

@jalaziz I just noticed that I did not reply to your question. One reduced the attack surface in that way that agents can not access the controller.

If they are started in a different namespace they can't use the serviceaccount of the controller.

jalaziz commented 3 years ago

@jalaziz I just noticed that I did not reply to your question. One reduced the attack surface in that way that agents can not access the controller.

If they are started in a different namespace they can't use the aerviceaccount of the controller.

@torstenwalter That makes sense, but I think that's inline with what this issue is about. Right now we can't really customize the role without creating a separate chart or creating the resources out-of-band, which is quite burdensome for a small set of resources.

I'm certainly happy to put in the work to add support for customizable roles, just wasn't sure if you'd prefer a structured approach or just allow full customization by supplying the raw Role/RoleBinding.

shellshock1953 commented 2 years ago

Hi, Will this feature be implemented in the near future? thanks

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.