feat: automatically remove user registry secrets

[olevski]

Closes #333

A few things to note:

• The scripts that are used by the cronjob can be moved to a fully independent docker image. I decided to not do this just because it was easier to piggy-back on the notebooks docker image. This is now an independent image.
• Use {servername}-registry to name user registry secrets (servername is escaped to prevent underscores from the project name causing issues.) It was decided to use the following pattern {safeusername}-registry-{uuid4()}.
• Wrote unit tests following the ones that are already present for the api and other code
• The logic for removing secrets is as follows:
  1. In the cronjob check for any secret in the designated namespace that match the regex (.+)-registry$ .+-registry-[a-z0-9-]{36}$ and has the label component=singleuser-server, with the capture group indicating the servername for the user server.
  2. In addition to the regex test also test for secret.type == "kubernetes.io/dockerconfigjson"and for the presence of the labels ["renku.io/commit-sha", "renku.io/projectName", "renku.io/username"] which are used to then identify the pod that used the secret.
  3. If any secret passes all tests from the previous two steps, then:
     a) Check to see if the pod that used the secret is still around, if so and if the pod's status is running or completed, then delete the secret
     b) If the pod that used the secret cannot be found (based on the servername labels) then delete the secret only if the age of the secret is older than the designated threshold

This is currently in place at https://tasko.dev.renku.ch/. Tested with a public and private projects.

[rokroskar]

I believe you need to also check the username of the pod explicitly - checking the server name is not enough. See GHSA-v7m9-9497-p9gr - you can create user names that will map to the same escaped version of the name. Come to think of it, all of the secret handling probably needs to be rethought a bit - adding annotations would probably be the way to go.

[ableuler]

Thanks @olevski for this nicely done and well documented PR and particularly for test-covering the added functionality right away!

I think the biggest change necessary is related to how we map a secret to a pod (see comment below). Particularly, there's a difference between the k8s pod name and the JH server name that I didn't really point out (sorry about that). While the k8s pod name IS unique and includes both username, repo name and commit, different users can have servers with the same servername, even after fixing the CVE that @rokroskar has pointed out. For example check out https://<renku-domain>/jupyterhub/hub/home for a list of the servernames of your running servers. My initial thought was to add the pod name directly as annotation to the secret, but the secret is created in the notebooks-service while the pod name is generated in the spawner, so we'd either have to duplicate the logic which generates the pod name or - what I have described in the comment - rely custom renku.io annotations (or labels!) for matching secrets and pods.

[rokroskar]

Thanks @olevski this looks good! Just a few minor comments for clarification.

[rokroskar]

oops missed this the first time around.. copy/paste error I presume

[olevski]

@ableuler , @rokroskar I addressed all the comments. Also I deployed the latest from this branch to my namespace and tested the cases when:

• the secret is deleted when the pod is around but is running
• the secret is not deleted when the age of the secret is less than 5 mins but the pod is not around
• the secret is deleted when the pod is not around and the secret is old enough

[rokroskar]

One last thing and you might leave this to another PR, which would be fine imo, but we should have some way of controlling, through the values file, a) the cron setup and b) the max secret age

[olevski]

One last thing and you might leave this to another PR, which would be fine imo, but we should have some way of controlling, through the values file, a) the cron setup and b) the max secret age

No problem I can add this to this PR @rokroskar.

[rokroskar]

🚀