dask / dask/dask-gateway

Slimming down Helm chart k8s api-server privileges (RBAC)

Open
#661 10 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
148
Forks
93
PR merge metrics
No merged PRs in 30d

Description

Currently the Helm chart provides more cluster wide permissions against the k8s api-server than may be needed, depending on configuration of dask-gateway.

The following PRs are opened related to this, but I felt that reviewing them was focusing on technical details before I had considered the configuration API that would make to support. Due to that, I've opened this issue to brainstorm and try to establish what I'm looking for.

- #626
- #594

## Situations

The helm chart is installed in one location which provides a dask-gateway-controller, dask-gateway-server (api pod), dask-gateway-proxy (traefik pod). They should be provided with the relevant permissions to create and interact with the k8s resources created.

Thanks to configuration, on demand created Dask clusters can be created in different namespaces. Due to that, we provide permissions to work with resources in all namespaces.

### 1. _Local namespace_: clusters created only in helm chart local namespace

Reduced permissions required.

With local I mean where the daks-gateway Helm chart was installed.

### 2. _Other namespace_: clusters created only in another pre-configured namespace

Reduced permissions required, but not just like as if everything was put in a local namespace.

### 3. _Any namespace_: clusters created in any namespace

Full cluster wide permissions required. My assumption is that it is not possible to provide RBAC resources defining permissions to namespaces matching a certain naming pattern or having a certain label, if that is incorrect we can do something smarter here as well.

## Current configuration options

- Currently, we support the Helm chart configuration [`gateway.backend,namespace`](https://github.com/dask/dask-gateway/blob/e31d01f0dd218419601cc441c182b113fee4b971/resources/helm/dask-gateway/values.yaml#L123-L125).
- `gateway.backend.namespace` [is mapped to](https://github.com/dask/dask-gateway/blob/e31d01f0dd218419601cc441c182b113fee4b971/resources/helm/dask-gateway/templates/gateway/configmap.yaml#L56-L80) the Python configuration [`KubeClusterConfig.namespace`](https://github.com/dask/dask-gateway/blob/e31d01f0dd218419601cc441c182b113fee4b971/dask-gateway-server/dask_gateway_server/backends/kubernetes/backend.py#L23-L43) within the dask-gateway server (api pod).

## Future configuration options?

What we currently have seems sufficient to me.

- If someone wants to accomplish a resitrction of permissions when a single namespace is to be used, we can require the helm chart configuration `gateway.backend.namespace` is used (as compared to setting `c.KubeConfigCluster.namespace = "something"` in a custom Python config file provided to dask-gateway-server).
- If someone wants to accomplish a restriction of permissions in situations where users are provided their own namespaces etc if such feature would be supported, we should go with cluster wide permissions.

## Implementation ideas

- I think when someone configures the `gateway.backend.namespace`, we should reduce the permissions to that.
- I think when someone doesn't configure the `gateway.backend.namespace`, we should reduce the permissions to the local namespace.

A secure default is far better than an unsecure default. At the same time, a user should be able to request cluster wide permissions so that c.KubeClusterConfig.namespace` can be dynamically updated and still work.

So, we would need another Helm char configuration option to provide cluster wide permissions. I note that [`rbac.scope` is used by the prometheus helm chart](https://github.com/kubernetes/ingress-nginx/blob/6ed6a76200595a7c6843322c60491fef2c86a864/charts/ingress-nginx/templates/clusterrole.yaml#L3-L5).

With all this considered, I think we should let `gateway.backend.namespace` just be something we pass through to the Python configuration `c.KubeConfigCluster.namespace`, and let `rbac.scope` decide if we provide cluster wide or namespace scoped RBAC permissions. That way, we have sufficient flexibility I think.

## Towards resolving this

- [ ] Propose a `rbac.scope` like configuration syntax. It should support cluster wide mode and single namespace mode, support for multiple namespaces seems like more complexity than merited in my mind.
- [ ] Seek and reach agreement on configuration syntax
- [ ] Implement it technically and make sure we have at least one test case for this as well in our CI system

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.