From 6d0e2d69d52733bbc02bf63cccce11fd9961c32d Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 23 Nov 2015 18:28:55 -0800 Subject: [PATCH 1/2] Update auth.resolve_repository_name More relaxed version that matches the constraints imposed by the current version of the docker daemon. Few unit tests to verify the new cases. Client.pull was trying to set the tag value when it wasn't supposed to, fixed now. utils.parse_repository_tag is simpler and supports @sha... notation Signed-off-by: Joffrey F --- docker/api/image.py | 5 ++--- docker/auth/auth.py | 38 +++++++++++++++++--------------------- docker/utils/utils.py | 18 ++++++++---------- tests/unit/auth_test.py | 35 ++++++++++++++++++++++++++++------- 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/docker/api/image.py b/docker/api/image.py index f891e2101a..8493b38d0f 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -158,8 +158,6 @@ def pull(self, repository, tag=None, stream=False, if not tag: repository, tag = utils.parse_repository_tag(repository) registry, repo_name = auth.resolve_repository_name(repository) - if repo_name.count(":") == 1: - repository, tag = repository.rsplit(":", 1) params = { 'tag': tag, @@ -174,7 +172,8 @@ def pull(self, repository, tag=None, stream=False, log.debug('Looking for auth config') if not self._auth_configs: log.debug( - "No auth config in memory - loading from filesystem") + "No auth config in memory - loading from filesystem" + ) self._auth_configs = auth.load_config() authcfg = auth.resolve_authconfig(self._auth_configs, registry) # Do not fail here if no authentication exists for this diff --git a/docker/auth/auth.py b/docker/auth/auth.py index 416dd7c471..f771dedd97 100644 --- a/docker/auth/auth.py +++ b/docker/auth/auth.py @@ -16,11 +16,9 @@ import json import logging import os -import warnings import six -from .. import constants from .. import errors INDEX_NAME = 'index.docker.io' @@ -31,31 +29,29 @@ log = logging.getLogger(__name__) -def resolve_repository_name(repo_name, insecure=False): - if insecure: - warnings.warn( - constants.INSECURE_REGISTRY_DEPRECATION_WARNING.format( - 'resolve_repository_name()' - ), DeprecationWarning - ) - +def resolve_repository_name(repo_name): if '://' in repo_name: raise errors.InvalidRepository( - 'Repository name cannot contain a scheme ({0})'.format(repo_name)) - parts = repo_name.split('/', 1) - if '.' not in parts[0] and ':' not in parts[0] and parts[0] != 'localhost': - # This is a docker index repo (ex: foo/bar or ubuntu) - return INDEX_NAME, repo_name - if len(parts) < 2: - raise errors.InvalidRepository( - 'Invalid repository name ({0})'.format(repo_name)) + 'Repository name cannot contain a scheme ({0})'.format(repo_name) + ) - if 'index.docker.io' in parts[0]: + index_name, remote_name = split_repo_name(repo_name) + if index_name[0] == '-' or index_name[-1] == '-': raise errors.InvalidRepository( - 'Invalid repository name, try "{0}" instead'.format(parts[1]) + 'Invalid index name ({0}). Cannot begin or end with a' + ' hyphen.'.format(index_name) ) + return index_name, remote_name + - return parts[0], parts[1] +def split_repo_name(repo_name): + parts = repo_name.split('/', 1) + if len(parts) == 1 or ( + '.' not in parts[0] and ':' not in parts[0] and parts[0] != 'localhost' + ): + # This is a docker index repo (ex: username/foobar or ubuntu) + return INDEX_NAME, repo_name + return tuple(parts) def resolve_authconfig(authconfig, registry=None): diff --git a/docker/utils/utils.py b/docker/utils/utils.py index 366f86965a..560ee8e223 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -283,16 +283,14 @@ def convert_volume_binds(binds): return result -def parse_repository_tag(repo): - column_index = repo.rfind(':') - if column_index < 0: - return repo, None - tag = repo[column_index + 1:] - slash_index = tag.find('/') - if slash_index < 0: - return repo[:column_index], tag - - return repo, None +def parse_repository_tag(repo_name): + parts = repo_name.rsplit('@', 1) + if len(parts) == 2: + return tuple(parts) + parts = repo_name.rsplit(':', 1) + if len(parts) == 2 and '/' not in parts[1]: + return tuple(parts) + return repo_name, None # Based on utils.go:ParseHost http://tinyurl.com/nkahcfh diff --git a/tests/unit/auth_test.py b/tests/unit/auth_test.py index 6783038166..8e0b1d4372 100644 --- a/tests/unit/auth_test.py +++ b/tests/unit/auth_test.py @@ -9,6 +9,7 @@ import tempfile from docker import auth +from docker import errors from .. import base @@ -29,25 +30,31 @@ def test_803_urlsafe_encode(self): assert b'_' in encoded -class ResolveAuthTest(base.BaseTestCase): - auth_config = { - 'https://index.docker.io/v1/': {'auth': 'indexuser'}, - 'my.registry.net': {'auth': 'privateuser'}, - 'http://legacy.registry.url/v1/': {'auth': 'legacyauth'} - } - +class ResolveRepositoryNameTest(base.BaseTestCase): def test_resolve_repository_name_hub_library_image(self): self.assertEqual( auth.resolve_repository_name('image'), ('index.docker.io', 'image'), ) + def test_resolve_repository_name_dotted_hub_library_image(self): + self.assertEqual( + auth.resolve_repository_name('image.valid'), + ('index.docker.io', 'image.valid') + ) + def test_resolve_repository_name_hub_image(self): self.assertEqual( auth.resolve_repository_name('username/image'), ('index.docker.io', 'username/image'), ) + def test_explicit_hub_index_library_image(self): + self.assertEqual( + auth.resolve_repository_name('index.docker.io/image'), + ('index.docker.io', 'image') + ) + def test_resolve_repository_name_private_registry(self): self.assertEqual( auth.resolve_repository_name('my.registry.net/image'), @@ -90,6 +97,20 @@ def test_resolve_repository_name_localhost_with_username(self): ('localhost', 'username/image'), ) + def test_invalid_index_name(self): + self.assertRaises( + errors.InvalidRepository, + lambda: auth.resolve_repository_name('-gecko.com/image') + ) + + +class ResolveAuthTest(base.BaseTestCase): + auth_config = { + 'https://index.docker.io/v1/': {'auth': 'indexuser'}, + 'my.registry.net': {'auth': 'privateuser'}, + 'http://legacy.registry.url/v1/': {'auth': 'legacyauth'} + } + def test_resolve_authconfig_hostname_only(self): self.assertEqual( auth.resolve_authconfig(self.auth_config, 'my.registry.net'), From 00c0baf40f0c42b9562c2156c8bf7c2b94037f8e Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Tue, 24 Nov 2015 12:15:03 -0800 Subject: [PATCH 2/2] Add tests for new cases covered by parse_repository_tag Signed-off-by: Joffrey F --- tests/unit/utils_test.py | 60 ++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py index 3c9f6e2fc8..57ad443582 100644 --- a/tests/unit/utils_test.py +++ b/tests/unit/utils_test.py @@ -352,23 +352,55 @@ def test_parse_host_empty_value(self): assert parse_host(val, 'win32') == tcp_port +class ParseRepositoryTagTest(base.BaseTestCase): + sha = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' + + def test_index_image_no_tag(self): + self.assertEqual( + parse_repository_tag("root"), ("root", None) + ) + + def test_index_image_tag(self): + self.assertEqual( + parse_repository_tag("root:tag"), ("root", "tag") + ) + + def test_index_user_image_no_tag(self): + self.assertEqual( + parse_repository_tag("user/repo"), ("user/repo", None) + ) + + def test_index_user_image_tag(self): + self.assertEqual( + parse_repository_tag("user/repo:tag"), ("user/repo", "tag") + ) + + def test_private_reg_image_no_tag(self): + self.assertEqual( + parse_repository_tag("url:5000/repo"), ("url:5000/repo", None) + ) + + def test_private_reg_image_tag(self): + self.assertEqual( + parse_repository_tag("url:5000/repo:tag"), ("url:5000/repo", "tag") + ) + + def test_index_image_sha(self): + self.assertEqual( + parse_repository_tag("root@sha256:{0}".format(self.sha)), + ("root", "sha256:{0}".format(self.sha)) + ) + + def test_private_reg_image_sha(self): + self.assertEqual( + parse_repository_tag("url:5000/repo@sha256:{0}".format(self.sha)), + ("url:5000/repo", "sha256:{0}".format(self.sha)) + ) + + class UtilsTest(base.BaseTestCase): longMessage = True - def test_parse_repository_tag(self): - self.assertEqual(parse_repository_tag("root"), - ("root", None)) - self.assertEqual(parse_repository_tag("root:tag"), - ("root", "tag")) - self.assertEqual(parse_repository_tag("user/repo"), - ("user/repo", None)) - self.assertEqual(parse_repository_tag("user/repo:tag"), - ("user/repo", "tag")) - self.assertEqual(parse_repository_tag("url:5000/repo"), - ("url:5000/repo", None)) - self.assertEqual(parse_repository_tag("url:5000/repo:tag"), - ("url:5000/repo", "tag")) - def test_parse_bytes(self): self.assertEqual(parse_bytes("512MB"), (536870912)) self.assertEqual(parse_bytes("512M"), (536870912))