Avoid listing all NS, Reflector per NS instead of shared for enable_user_namespaces

[josefhandl]

Proposed change

Use reflectors per each user namespace instead of shared reflectors across multiple namespaces for allowed enable_user_namespaces. Enabling enable_user_namespaces will create a shared reflector and list all namespaces that is a fairly rich permission. Creating a reflector per namespace requires only get permission for given namespace, which might be a more achievable permission for non-admin user.

Alternative options

Add option to select whether to list all namespaces.

Who would use this feature?

On many shared clusters, listing all namespaces could be disabled for security reasons. However, creating a new namespace might be a reasonable compromise for applications like jupyterhub.

Rancher users are a good example. Rancher has the concept of Projects, that group namespaces together. A standard Rancher user might be able to get namespaces within his project. However, listing all namespaces could not be allowed for security reasons and is disabled in the default Rancher installation.

(Optional): Suggest a solution

I found a few places in code that control this logic.

When the enable_user_namespaces option is enabled, the internal variable omit_namespace is set to true, which causes the list method called list_*_for_all_namespaces to be used.

# spawner.py, line cca 2470
        return await self._start_reflector(
            kind="events",
            reflector_class=EventReflector,
            fields={"involvedObject.kind": "Pod"},
-            omit_namespace=self.enable_user_namespaces,
            replace=replace,
        )

# spawner.py, line cca 2490
        return await self._start_reflector(
            kind="pods",
            reflector_class=PodReflector,
            labels={"component": self.component_label},
-            omit_namespace=self.enable_user_namespaces,
            replace=replace,
        )

Also disable creating shared reflector:

# spawner.py, line cca 130
    def _get_reflector_key(self, kind: str) -> Tuple[str, str, Optional[str]]:
-        if self.enable_user_namespaces:
-            # one reflector fo all namespaces
-            return (kind, None)
-
        return (kind, self.namespace)

(The line number are relevant for this commit: https://github.com/jupyterhub/kubespawner/blob/b9368d209619b25691c552be9a724f7bc74de6d1/kubespawner/spawner.py)

To use Rancher's projects concept you need to use Rancher user using the API instead of a Service Account.
With these lines edits, it works as expected for me, but of course these edits are not final if this request is acceptable to you.

[welcome]

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.

You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[minrk]

I think this is a good suggestion, but a bit tricky. The downside is scalability - multiple reflectors per user may start to have problems with open watch connections when you have hundreds of users. There won't be any more events to reflect (fewer, in fact), but there will be lots of Python objects and open watch connections.

The only behavior problem I see with the proposed patch is that the watches never stop, which will be important when there could be an unbounded number of reflectors. In that case, they should stop when the last server in that namespace stops, which may be tricky with named servers where a user may have multiple servers in the same namespace. Either that, or try to guess at it with an LRU cache of reflectors, purging/stopping the one with the oldest activity when it's full.

As a result, I think it's probably best to keep the single global reflector pattern, but optionally have per-namespace reflectors as well.