Skip to content

Commit

Permalink
add image_prefix validation
Browse files Browse the repository at this point in the history
test image_prefix trait validation
add image name parser
simplify get_image_manifest method
extend get_image_manifest tests
update zero-to-binderhub doc, private registry section
  • Loading branch information
adriendelsalle committed Feb 17, 2021
1 parent e3627e5 commit 0e02519
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 15 deletions.
10 changes: 10 additions & 0 deletions binderhub/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@ def _valid_badge_base_url(self, proposal):
config=True
)

@validate('image_prefix')
def _valid_image_prefix(self, proposal):
image_regex = re.compile(r'^([^\/:]+(?::[0-9]+)?\/)?' +
r'([_a-zA-Z0-9\-\.]+(?:\/[_a-zA-Z0-9\-\.]+)*)$')
if image_regex.match(proposal.value) is None:
raise AttributeError("'image_prefix' does not match the expected pattern "+
"[<registry>[:<port>]/]<repo>")

return proposal.value

build_memory_request = ByteSpecification(
0,
help="""
Expand Down
2 changes: 1 addition & 1 deletion binderhub/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ async def get(self, provider_prefix, _unescaped_spec):
if self.settings['use_registry']:
for _ in range(3):
try:
image_manifest = await self.registry.get_image_manifest(*'/'.join(image_name.split('/')[-2:]).split(':', 1))
image_manifest = await self.registry.get_image_manifest(image_name)
image_found = bool(image_manifest)
break
except HTTPClientError:
Expand Down
4 changes: 1 addition & 3 deletions binderhub/health.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,7 @@ async def check_docker_registry(self):
# we are only interested in getting a response from the registry, we
# don't care if the image actually exists or not
image_name = self.settings["image_prefix"] + "some-image-name:12345"
await registry.get_image_manifest(
*'/'.join(image_name.split('/')[-2:]).split(':', 1)
)
await registry.get_image_manifest(image_name)
return True

async def check_pod_quota(self):
Expand Down
22 changes: 21 additions & 1 deletion binderhub/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,28 @@ def _default_password(self):
base64.b64decode(b64_auth.encode("utf-8")).decode("utf-8").split(":", 1)[1]
)

async def get_image_manifest(self, image, tag):
@staticmethod
def parse_image_name(name):
"""Parse a docker image string into (registry, image, tag)"""
# name could be "localhost:5000/foo:tag"
# or "gcr.io/project/subrepo/sub/sub/sub:tag"
# or "org/repo:tag" for implicit docker.io
registry, _, tagged_image = name.partition("/")
if tagged_image and (registry == "localhost" or any(c in registry for c in (".", ":"))):
# registry host is specified
pass
else:
# registry not given, use implicit default docker.io registry
registry = "docker.io"
tagged_image = name
image, _, tag = tagged_image.partition(":")

return registry, image, tag

async def get_image_manifest(self, name):
client = httpclient.AsyncHTTPClient()
_, image, tag = DockerRegistry.parse_image_name(name)

url = "{}/v2/{}/manifests/{}".format(self.url, image, tag)
# first, get a token to perform the manifest request
if self.token_url:
Expand Down
19 changes: 19 additions & 0 deletions binderhub/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,29 @@

from subprocess import check_output
import sys
import pytest

from binderhub.app import BinderHub


def test_help():
check_output([sys.executable, '-m', 'binderhub', '-h'])

def test_help_all():
check_output([sys.executable, '-m', 'binderhub', '--help-all'])

def test_image_prefix():
b = BinderHub()

prefixes = ["foo/bar", "foo-bar/baz", "foo/bar-", "localhost/foo", "localhost/foo/bar/baz",
"localhost:8080/foo/bar/baz", "127.0.0.1/foo", "127.0.0.1:5000/foo/b",
"f/o/o/b/a/r/b/a/z", "gcr.io/foo", "my.gcr.io:5000/foo", "foo_bar/baz-",
"foo_ba.r/baz-"]
for name in prefixes:
b.image_prefix = name

wrong_prefixes = ["foo/bar-baz:", "foo/bar-baz:", "foo/bar-baz:10", "foo/bar/", "/foo/bar",
"http://localhost/foo/bar"]
for name in wrong_prefixes:
with pytest.raises(AttributeError):
b.image_prefix = name
10 changes: 7 additions & 3 deletions binderhub/tests/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ async def test_get_image_manifest(tmpdir, request):
[
(r"/token", MockTokenHandler, {"test_handle": test_handle}),
(
r"/v2/([^/]+)/manifests/([^/]+)",
r"/v2/([a-zA-Z0-9]+(?:/[a-zA-Z0-9]+)*)/manifests/([a-zA-Z0-9]+)",
MockManifestHandler,
{"test_handle": test_handle},
),
Expand Down Expand Up @@ -145,5 +145,9 @@ async def test_get_image_manifest(tmpdir, request):
assert registry.token_url == url + "/token"
assert registry.username == username
assert registry.password == password
manifest = await registry.get_image_manifest("myimage", "abc123")
assert manifest == {"image": "myimage", "tag": "abc123"}

names = ["myimage:abc123", "localhost/myimage:abc1234", "localhost:8080/myimage:abc3",
"192.168.1.1:8080/myimage:abc321", "192.168.1.1:8080/some/repo/myimage:abc3210"]
for name in names:
manifest = await registry.get_image_manifest(name)
assert manifest == {"image": name.split("/", 1)[-1].rsplit(":", 1)[0], "tag": name.rsplit(":", 1)[-1]}
19 changes: 12 additions & 7 deletions doc/zero-to-binderhub/setup-binderhub.rst
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,18 @@ If you setup your own local registry using

.. important::

BinderHub assumes that `image_prefix` contains an extra level for the project name such that: `gcr.io/<project-id>/<prefix>-name:tag`.
Hence, your `image_prefix` field should be set to: `your-registry.io/<some-project-name>/<prefix>-`.
See `this issue <https://github.com/jupyterhub/binderhub/issues/800>`_ for more details.

`<some-project-name>` can be completely arbitrary and/or made-up.
For example, it could be the name you give your BinderHub.
Without this extra level, you may find that your BinderHub always rebuilds images instead of pulling them from the registry.
BinderHub validates that `image_prefix` matches the following pattern: `[<registry>/]<prefix>`.

<registry> is optional and defaults to `docker.io`. It may include the server port but does not
include the protocol (http[s]://).

<prefix> supports any repository depth.

Examples:
- `binder-`
- `foo/binder-`
- `docker.io/foo/bar/baz/binder-`
- `127.0.0.1:5000/binder-`

Install BinderHub
-----------------
Expand Down

0 comments on commit 0e02519

Please sign in to comment.