feat: support watching a specific namespace only

[maxime1907]

trafficstars

Proposal

Currently, dask-gateway fetches custom resources (DaskCluster, IngressRoute...) across all namespaces of the cluster. This behavior is fine but if we want to use it strictly in a specific namespace, it is not possible with the state of the stack. I have then thought of a way to adapt it without creating any breaking change with the following implementation.

Implementation

• Add an environment variable WATCH_NAMESPACE that lets dask operators to specify a namespace so that all kubernetes commands that are launched within dask-gateway are targeted only on this specific namespace. If this environment variable is not specified, it will start normally in cluster wide mode.
• Change the helm chart to add support for the environment variable WATCH_NAMESPACE thanks to the following fields:
  • runInClusterScope: Run the whole stack either at the cluster scope or at the namespace scope
  • watchNamespace: Watch only a specific namespace
• In namespaced mode, disable this traefik option: "--providers.kubernetescrd.allowCrossNamespace=true"
• Change the RBAC files to reflect the requirements of this new behavior:
  • Cluster wide config: ServiceAccount + ClusterRole + ClusterRoleBinding
  • Namespaced config: ServiceAccount + Role + RoleBinding

Behind the scene

Cluster wide

From the user and dask operator POV, it doesn't have any impact on the current behavior of dask-gateway which currently fetches kubernetes informations cluster wide (all namespaces).

Namespaced

From the user and dask operator POV, it restricts them to the namespace where the stack is currently deployed.

[maxime1907]

cc @consideRatio

[consideRatio]

Hi @maxime1907!

Can you document what you have done and why in the PR description?

Ideally for changes made:

• we come to agreement on a change is worth pursueing (not done ahead of time in this case), which is often coupled with how it could be implemented and documented practically
• describe the proposed/provided implementation: what is changed for accomplish the result? Focus both on summarizing user facing changes and behind-the-scenes changes.

Try to lead with a very easy to understand summary, and then go into a bit more details in following parts.

[maxime1907]

Hi @maxime1907!

Can you document what you have done and why in the PR description?

Ideally for changes made:

• we come to agreement on a change is worth pursueing (not done ahead of time in this case), which is often coupled with how it could be implemented and documented practically
• describe the proposed/provided implementation: what is changed for accomplish the result? Focus both on summarizing user facing changes and behind-the-scenes changes.

Try to lead with a very easy to understand summary, and then go into a bit more details in following parts.

Hi @consideRatio, Thanks for the directives and sorry for not doing it properly in the first place. I did my best trying to explain what this PR is all about in its description :wink:

[holzman]

@maxime1907: I've been looking at the same issue in #626 - my fault for not checking for open PRs first!

I think the only issue I see with this is that it doesn't look at gateway.backend.namespace. My thought -- rather than add more configuration values, what about:

If gateway.backend.namespace is not set (or set to the same as the release namespace), only watch the release namespace; otherwise, keep things in cluster scope.

Of course this changes the default behavior (since gateway.backend.namespace is null by default), but I think that's a good thing and the default behavior is overly permissive; most users don't want to grant cluster-wide permission to read secrets.

[maxime1907]

@maxime1907: I've been looking at the same issue in #626 - my fault for not checking for open PRs first!

I think the only issue I see with this is that it doesn't look at gateway.backend.namespace. My thought -- rather than add more configuration values, what about:

If gateway.backend.namespace is not set (or set to the same as the release namespace), only watch the release namespace; otherwise, keep things in cluster scope.

Of course this changes the default behavior (since gateway.backend.namespace is null by default), but I think that's a good thing and the default behavior is overly permissive; most users don't want to grant cluster-wide permission to read secrets.

Hi @holzman,

Nice idea about putting the configuration in gateway.backend

About changing the default behavior, i prefer to keep the same as its currently done so cluster wide because that would be a breaking change for all users.

If'd prefer something like: If gateway.backend.namespace not set then cluster wide (default behavior) If gateway.backend.namespace is set then its restricted to that specific namespace (even if its the same namespace as the release, specify it)

I am waiting for @consideRatio for his toughts on the subject

[holzman]

If'd prefer something like: If gateway.backend.namespace not set then cluster wide (default behavior) If gateway.backend.namespace is set then its restricted to that specific namespace (even if its the same namespace as the release, specify it)

Well, this doesn't quite make sense to me - if gateway.backend.namespace is unset, we don't need cluster-wide permissions since it will be in the same namespace. OTOH if gateway.backend.namespace is set and different than the release namespace, we will need cluster-level permissions.

Not my preference, but if we really want to preserve the current behavior than we could add the runInClusterScope helm value, but catch the case when runinClusterScope is false but the namespace specified is different than the Release namespace, something like:

{{- if and (not .Values.runInClusterScope) (ne .Values.gateway.backend.namespace .Release.Namespace) }}
{{- fail "runInClusterScope is false, but a namespace different than the release namespace was specified" }}
{{- end }}

Maybe also emit a warning for the inverse as well that cluster permissions were configured but probably overpermissive.

[maxime1907]

If'd prefer something like: If gateway.backend.namespace not set then cluster wide (default behavior) If gateway.backend.namespace is set then its restricted to that specific namespace (even if its the same namespace as the release, specify it)

Well, this doesn't quite make sense to me - if gateway.backend.namespace is unset, we don't need cluster-wide permissions since it will be in the same namespace. OTOH if gateway.backend.namespace is set and different than the release namespace, we will need cluster-level permissions.

Not my preference, but if we really want to preserve the current behavior than we could add the runInClusterScope helm value, but catch the case when runinClusterScope is false but the namespace specified is different than the Release namespace, something like:

{{- if and (not .Values.runInClusterScope) (ne .Values.gateway.backend.namespace .Release.Namespace) }}
{{- fail "runInClusterScope is false, but a namespace different than the release namespace was specified" }}
{{- end }}

Maybe also emit a warning for the inverse as well that cluster permissions were configured but probably overpermissive.

Ahh mb on this one, idk what was i thinking, you are actually right! :slightly_smiling_face:

Then we just need to determine if @consideRatio wants to keep the current behavior or if he lets us change it