Tighten up RBAC by using `Role` instead of `ClusteRole`

[brouberol]

As the chart is currently setup, it is relying on ClusterRole and ClusterRoleBinding resources to grant the operator CRUD permissions cluster-wide, to allow it to manage its associated resources in any namespaces in which its custom resources are created. However, one might argue that this is a lot of permissions to grant cluster-wide

After the reading the operator code, I realized that the operator can be passed a WATCH_NAMESPACE environment variable containing a comma-separated list of namespaces to watch. If we exposed that namespace list as a chart value (e.g. $.Values.watchedNamespaces), we could then loop over it, and create Role and RoleBinding resources scoped to each namespace and the operator ServiceAccount. As a result, I believe that the operator would still work as expected, but would only have permissions to act on the configured namespaces, instead of the whole kubernetes cluster.

I'm happy to hear your thoughts.

[sxd]

Hi @brouberol

Everything that you say is true and yes it's a good idea, but there's some on going work on the operator side to reduce the amount of permissions required, we already removed a lot of permissions, but sadly, there's some permissions that will require to be ClsuterRoleBinding, like the one require for the nodes information, since the resources nodes aren't namespaced resources.

Some PRs are being created, if you can create one to make the Helm operator work namespaced will be awesome!

Thanks in advance!

[brouberol]

Hi @sxd! I'll be experimenting with tweaks at the RBAC level. When I'm sure everything is working properly, I'll send out a PR to leverage Role and RoleBinding resources where possible!

Thanks for reaching back quickly 🙏

[heruan]

After the reading the operator code, I realized that the operator can be passed a WATCH_NAMESPACE environment variable containing a comma-separated list of namespaces to watch.

It would be great to have such value, and remove the cluster wide role. Ingress NGINX for example can be installed from the chart scoped to the chart namespace:

https://github.com/kubernetes/ingress-nginx/blob/76f90ec8cf67f6cc6650865a1916aa2581ea205f/charts/ingress-nginx/values.yaml#L182-L186

This would help installing CNPG as a chart dependency.

[dosubot[bot]]

Hi, @brouberol. I'm Dosu, and I'm helping the charts team manage their backlog. I'm marking this issue as stale.

Issue Summary:

• You proposed replacing ClusterRole and ClusterRoleBinding with Role and RoleBinding for enhanced security.
• @sxd noted that some permissions, like node information, require cluster-wide access.
• You planned to experiment with RBAC adjustments and submit a PR.
• @heruan supported the idea, highlighting its benefits for namespace scoping similar to Ingress NGINX.

Next Steps:

• Please let me know if this issue is still relevant to the latest version of the charts repository by commenting here.
• If no updates are provided, the issue will be automatically closed in 7 days.

Thank you for your understanding and contribution!