Skip to content
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

Better RBAC #624

Closed
jkremser opened this issue Apr 8, 2024 · 4 comments
Closed

Better RBAC #624

jkremser opened this issue Apr 8, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@jkremser
Copy link
Contributor

jkremser commented Apr 8, 2024

Currently, there is one service account shared across all 3 deployments, even though they need only a sub-set of those RBAC rules. Also it's not possible to set rbac per namespace when running KEDA together with the WATCH_NAMESPACE not being equal to "".

here are some of my thoughts:

I’d like to achieve a fine-tuned RBAC settings using the helm values as the input. So that we can change the values that drives the keda’s installation on our side in kedify agent, render the yaml template and just re-apply based on what rbac settings will be required in the dashboard.
here are some flaws that I found in the current helm chart:

  1. currently, there is a 1 clusterrole and 1 clusterbinding that gives very wide rights for the service account that is then used by the KEDA operator, the same service account is then also reused by the webhook deployment (although webook clearly doesn’t need 99% of those rights)

  2. the cluster role is too wide, giving the operator basically almost cluster admin, for instance there is a read only rule for anything anywhere

  3. watchNamespace is not being used anyhow, no matter if we configure it to an empty string (denoting the cluster-wide keda mode) or a given namespace, the cluster role is applied

  4. scattered rbac options across different section in values.yaml (.Values.rbac vs .Values.permissions). Can we afford breaking the backward compatibility here and merge it under .Values.rbac ?

  5. this looks like a typo to me, there is no standard resource called external , maybe it should have been this?

Some ideas:

One can mix a cluster role with (normal - not a clusterrolebinding). This way, we can reuse the same role (the clusterrole) and apply it only to a namespace using the RoleBinding ) this will reduce the number of manifest the helm chart will generate:

  • 1 cluster role
  • n role binding per each namespace specified in watchNamespace

Restrict the access by white-listing:

  • the allowed crds (that can appear as the scaleTargetRef
  • the names of secrets that can be read by KEDA operator (check k explain role.rules.resourceNames unfortunately this doesn't work for create verb)
@jkremser jkremser added the bug Something isn't working label Apr 8, 2024
@jkremser
Copy link
Contributor Author

jkremser commented Apr 8, 2024

so the current helm chart assigns all these rights to all 3 deployments (keda operator, webhook and metrics adapter/server):

k rbac-tool policy-rules -e 'keda-operator'                                                                                                                                                                                  ○ kedify-cluster
  TYPE           | SUBJECT       | VERBS  | NAMESPACE   | API GROUP                    | KIND                                 | NAMES                              | NONRESOURCEURI | ORIGINATED FROM
+----------------+---------------+--------+-------------+------------------------------+--------------------------------------+------------------------------------+----------------+--------------------------------------------------------------+
  ServiceAccount | keda-operator | *      | *           | autoscaling                  | horizontalpodautoscalers             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | batch                        | jobs                                 |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | core                         | events                               |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | eventing.keda.sh             | cloudeventsources                    |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | eventing.keda.sh             | cloudeventsources/status             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | clustertriggerauthentications        |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | clustertriggerauthentications/status |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | scaledjobs                           |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | scaledjobs/finalizers                |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | scaledjobs/status                    |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | scaledobjects                        |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | scaledobjects/finalizers             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | scaledobjects/status                 |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | triggerauthentications               |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | *           | keda.sh                      | triggerauthentications/status        |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | *      | keda        | coordination.k8s.io          | leases                               |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | create | *           | authentication.k8s.io        | tokenreviews                         |                                    |                | ClusterRoles>>system:auth-delegator
  ServiceAccount | keda-operator | create | *           | authorization.k8s.io         | subjectaccessreviews                 |                                    |                | ClusterRoles>>system:auth-delegator
  ServiceAccount | keda-operator | create | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | delete | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | get    | *           | *                            | *                                    |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | *                            | */scale                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | admissionregistration.k8s.io | validatingwebhookconfigurations      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | apiregistration.k8s.io       | apiservices                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | core                         | configmaps                           |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | core                         | configmaps/status                    |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | core                         | external                             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | core                         | pods                                 |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | core                         | secrets                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | *           | core                         | services                             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | get    | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | get    | kube-system | core                         | configmaps                           | extension-apiserver-authentication |                | Roles>>kube-system/extension-apiserver-authentication-reader
                 |               |        |             |                              |                                      |                                    |                |
  ServiceAccount | keda-operator | list   | *           | *                            | */scale                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | admissionregistration.k8s.io | validatingwebhookconfigurations      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | apiregistration.k8s.io       | apiservices                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | apps                         | deployments                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | apps                         | statefulsets                         |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | configmaps                           |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | configmaps/status                    |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | external                             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | limitranges                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | pods                                 |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | secrets                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | serviceaccounts                      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | *           | core                         | services                             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | list   | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | list   | kube-system | core                         | configmaps                           | extension-apiserver-authentication |                | Roles>>kube-system/extension-apiserver-authentication-reader
                 |               |        |             |                              |                                      |                                    |                |
  ServiceAccount | keda-operator | patch  | *           | *                            | */scale                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | patch  | *           | admissionregistration.k8s.io | validatingwebhookconfigurations      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | patch  | *           | apiregistration.k8s.io       | apiservices                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | patch  | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | update | *           | *                            | */scale                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | update | *           | admissionregistration.k8s.io | validatingwebhookconfigurations      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | update | *           | apiregistration.k8s.io       | apiservices                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | update | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | watch  | *           | *                            | */scale                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | admissionregistration.k8s.io | validatingwebhookconfigurations      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | apiregistration.k8s.io       | apiservices                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | apps                         | deployments                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | apps                         | statefulsets                         |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | configmaps                           |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | configmaps/status                    |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | external                             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | limitranges                          |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | pods                                 |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | secrets                              |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | serviceaccounts                      |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | *           | core                         | services                             |                                    |                | ClusterRoles>>keda-operator
  ServiceAccount | keda-operator | watch  | keda        | core                         | secrets                              |                                    |                | Roles>>keda/keda-operator
  ServiceAccount | keda-operator | watch  | kube-system | core                         | configmaps                           | extension-apiserver-authentication |                | Roles>>kube-system/extension-apiserver-authentication-reader

also some of the entries are pretty wild to my taste:

ServiceAccount | keda-operator | get    | *           | *                            | *                                    |                                    |                | ClusterRoles>>keda-operator

@zroubalik
Copy link
Member

point 5. is definitely a typo

@zroubalik
Copy link
Member

zroubalik commented Apr 11, 2024

Thanks for tackling this, is it possible to list here potential fields that can be deprecated in the future and are present only for backwards compatibility( if there are any of course)?

@jkremser
Copy link
Contributor Author

previously, there was only one section in helm's values.yaml for specifying the details for service account (because there was only one service account)

it looked like this:

serviceAccount:
  # -- Specifies whether a service account should be created
  create: true
  # -- The name of the service account to use.
  name: keda-operator
  # -- Specifies whether a service account should automount API-Credentials
  automountServiceAccountToken: true
  # -- Annotations to add to the service account
  annotations: {}

(link)

This whole section can be deprecated and replaced with:

serviceAccount:
  operator:
    # -- Specifies whether a service account should be created
    create: true
    # -- The name of the service account to use.
    name: keda-operator
    # -- Specifies whether a service account should automount API-Credentials
    automountServiceAccountToken: true
    # -- Annotations to add to the service account
    annotations: {}
  metricServer:
    # -- Specifies whether a service account should be created
    create: true
    # -- The name of the service account to use.
    name: keda-metrics-server
    # -- Specifies whether a service account should automount API-Credentials
    automountServiceAccountToken: true
    # -- Annotations to add to the service account
    annotations: {}
  webhooks:
    # -- Specifies whether a service account should be created
    create: true
    # -- The name of the service account to use.
    name: keda-webhook
    # -- Specifies whether a service account should automount API-Credentials
    automountServiceAccountToken: true
    # -- Annotations to add to the service account
    annotations: {}

this is the only "breaking"/deprecating work I've done (it's not breaking, there is fallback included, but you know what i mean). Other changes were additive with sane defaults (no-ops by default)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants