Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add support for 3+-part image names (like on GitLab) #986

Open
wants to merge 2 commits into
base: master
from

Conversation

@bdrian
Copy link
Contributor

bdrian commented Oct 23, 2019

This PR enables the use of "longer" image names such as gitlab.com/bdrian/binderhub/prod/<generated-image-slug>. Only the lookup for possibly existing images has to be changed, because this is the only place we cannot use the full form (repo+image names together).

To allow both magical Docker Hub short names (like jupyterhub/k8s-binderhub) and regular names referring to a registry (gcr.io/project/something), this approach cuts off the first part unless the image name looks like it may come from Docker Hub -- those image names always have two parts and certain restrictions that are not all present in Docker itself:

Organization name must contain a combination of alphanumeric characters of 4-30 characters in length. Letters must be lowercase.

Your repository name must contain a combination of alphanumeric characters and may contain the special characters ., _, or -. Letters must be lowercase.

Additionally, less than two or more than 255 characters are also refused as repository names.

To be able to test this with the existing Travis setup right away, I had to single the basename derivation out; alternatively we'd have to find a publicly reachable registry that supports 3+-part image names and is not GitLab, because getting a token from GitLab's registry doesn't work yet (see #965). Local testing with "long names" in a remote registry was fine, though.

This change breaks compatibility with registries that do not operate at their domain root (something like mycompany.com/registry), but I'm not sure if that's actually used or even possible. Additional feedback is welcome.

Contributes to #965, but doesn't close it.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 8, 2019

@rochaporto what do you think of this/could you give it a spin with your setup?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 8, 2019

I think we should merge and take this for a test drive and see what happens.

One more thing: what should happen is someone puts in an image name/registry that violates the assumptions made in the name derivation function? Right now I'd say "stuff" happens aka undefined behaviour. Is it easy to raise a helpful error message or some such to help out the user?

@bdrian

This comment has been minimized.

Copy link
Contributor Author

bdrian commented Nov 8, 2019

Hm, I agree error handling is not done well in this PR. I'll reiterate that.

I found something else I overlooked before when looking for a way to generate useful error messages: We actually already know the registry's base address in DockerRegistry.url, which also takes care of Docker Hub references. With this, we can reliably determine which part of the image prefix is the actual image name, and don't need to assume anything about its structure.

Closing this PR to rework it

@bdrian bdrian closed this Nov 8, 2019
@betatim

This comment has been minimized.

Copy link
Member

betatim commented Nov 8, 2019

Ok, looking forward to an improved version.

ps. you could keep this PR open, add [WIP] to the start of the title and work on it. No need to close the PR.

@bdrian bdrian changed the title Add support for 3+-part image names (like on GitLab) [WIP] Add support for 3+-part image names (like on GitLab) Nov 8, 2019
@bdrian bdrian reopened this Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.