Gitlab registry support

[nichoio]

This PR provides support for GitLab image registry and possibly also further non-Docker Hub image registries, the latter not tested.
New code was tested against following image registries using minikube:

• Docker Hub
• Image registry of Gitab.com
• Image registry of a private GitLab instance

Besides parsing image names, we also append the query param service=container_registry when sending requests to read manifests. I have noticed that without, both Gitlab.com and my private instance, can't be accessed properly. See Gitlab docs for more context. I can tell that in case of Docker Hub, the extra param is no issue and i assume same applies for other Image registries.

Note that this is mostly not my own work, most of the code is based on #986 .

For reference, the relevant parts of an example config using Gitlab.com image registry:

config:
  BinderHub:
    use_registry: true
    image_prefix: "registry.gitlab.com/<gitlab-user>/<repo>/"
  DockerRegistry:
    token_url: "https://gitlab.com/jwt/auth"
    url: "https://registry.gitlab.com"
    username: <deploy-token-username>
    password: <deploy-token-pw>
registry:
  url: https://registry.gitlab.com
  username: <deploy-token-username>
  password: <deploy-token-pw>

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

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

Welcome to the Jupyter community! 🎉

[ctr26]

Would be very hyped to see this PR go through

[yuvipanda]

Thank you for opening this! I've left some comments, will be happy to continue reviewing and getting this in. \o/

[yuvipanda]

In the current implementation, I think the tag is left unspecified if it isn't present. Let's preserve that?

[nichoio]

I wouldn't mind. If i see correctly, the tag is passed to registry.get_image_manifest() where the following happens:
"{}/v2/{}/manifests/{}".format(self.url, image, tag)
So if tag is an empty string, could this be a problem for the manifests API? Idk personally, as i am pretty much clueless about this.

[yuvipanda]

@rappertomate I think tag is just always required to be specified when pushing, and is never left empty. I wouldn't mind raising an exception if tag is empty or None, but we can tackle that in another PR. I mostly just wanted to make sure the behavior doesn't change.

I think once that's done I'll happily merge this.

[yuvipanda]

Could we not instead just check for the number of / present? The full validator for image names is at https://github.com/distribution/distribution/blob/main/reference/regexp.go, and I'm not sure if this is exactly right. Can we get away with not using regexes?

[nichoio]

Not so sure. What if we have self-host registries without any paths to specify user or project? The unit test shows an example:
("weirdregistry.com/image:tag", "image", "tag")
vs.
("jupyterhub/k8s-binderhub:0.2.0-a2079a5", "jupyterhub/k8s-binderhub", "0.2.0-a2079a5")

This distinction cannot be covered by just counting /. And the purpose of this PR should be to cover these edge cases as well imo.

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

[yuvipanda]

I just merged this, let's deploy this and see how it goes :) I think the behavior change with :latest is acceptable, and leaving the tag out was a bug we never hit earlier. Thanks for pushing this through, @rappertomate! Please test it and let me know if it works?

[betatim]

Have you deployed this yet @yuvipanda ?

[nichoio]

I gave it a quick shot on minikube and it works as expected. I have tested it with a private GitLab instance using Helm values as described in the initial post of this PR. I'm not involved with operating any BinderHub instance anymore these days which is why I'm not available to test this more thoroughly.
As far as i can tell everything looks goof though.

[ctr26]

I'll give it a go too

[yuvipanda]

@betatim jupyterhub/mybinder.org-deploy#1991 just deployed it