feat(helm): add option to disable rbac creation

[Idotno-1]

Enable or disable ClusterRole and RoleBinding creation and add the ability to custom the ClusterRole rules

Aim of the merge request

Enabling the user to use existing ClusterRole and ClusterRoleBinding. The use case for us is to allow users to use this chart in clusters where creating such objetcs is not allowed by helm deployment but must go throught something else (eg Terraform).

Also, it's adding the ability to customise the rules, as not all of them are required for the operator to work.

Breaking change

None - this just adds a new feature

Testing process

Tested by hand, by trying different values in values.yaml

cd deploy/helm/grafana-operator
helm dependency build
helm template . --output-dir /tmp/grafana-operator

Test case with no creation of roles:

[...]
rbac:
  create: false
  rules:
    - apiGroups:
      - ""
      resources:
      - configmaps
      verbs:
      - create

Test case with creation of roles and custom rules:

[...]
rbac:
  create: true
  rules:
    - apiGroups:
      - ""
      resources:
      - configmaps
      verbs:
      - create

Test case with creation of roles (current behavior):

[...]
rbac:
  create: true
  rules: []

Related documents

N/A

Additional comments

Leaving rbac.rules at [] mimics the current behavior of fetching the rules from the file deploy/helm/grafana-operator/files/rbac.yaml. This could be a problem as the user might expect an empty list not give any rules.

It's explained in the documentation, but we could change this behaviour by putting as default the rules listed in the file mentioned (instead of []), and discarding the file (which would be breaking change ?). We could do something similar with the additional "OpenShift" rules.

[CLAassistant]

All committers have signed the CLA.

[weisdd]

Thanks for the PR!

First of all, I think the wording here is not perfect. - When I read the description, I'm not entirely sure what "The rules affected to the ClusterRole" means and why loading rules from files/ folder might not be desirable.
The variable itself should have probably been named ruleOverrides to highlight that the values will replace whatever exists in the chart.

Second, in the PR description, you mentioned that not all of the requested permissions are needed for the operator to run. If that's the case, I think it should be resolved not through rule overrides (it would be a nightmare to maintain, so I doubt that many people would want to use the feature), but rather through modifying the code of the operator. - RBAC definitions are generated on the fly based on the comments in Go code, so it's just a matter of changing one line and then running a command to regenerate definitions. - Can't say for other maintainers, but, personally, I would argue that we should only accept the change for making RBAC templates optional (rbac.create), but not the rule overrides, so I would advise to remove the latter.

[Idotno-1]

Thank you for the feedback !

I agree with your suggestion to include only the rbac.create option and remove the rbac.rules, for the reasons you mentionned. I will update the PR.

Regarding the unnecessary permissions, my intention was to address specific use cases where not all features of the Grafana Operator would be required (Ingress creation/deletion for example). In this cases, it could be great to reduce the set of permissions.

I wasn't aware theses files were generated from the Go code and did wonder why they were there, thanks for pointing that out !

[weisdd]

Ah, alright, I thought it was a generic case. Excluding ingresses is a corner-case, so it can only be done using custom manifests.

I'll merge the PR, thanks for your contribution!
The chart will get released along with the next version of the operator (particular date is not decided yet, most likely, it's a matter of few weeks time).