From 6d0e2d69d52733bbc02bf63cccce11fd9961c32d Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 23 Nov 2015 18:28:55 -0800 Subject: [PATCH] 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 | 42 +++++++++++++++++++---------------------- docker/utils/utils.py | 18 ++++++++---------- tests/unit/auth_test.py | 35 +++++++++++++++++++++++++++------- 4 files changed, 57 insertions(+), 43 deletions(-) diff --git a/docker/api/image.py b/docker/api/image.py index f891e210..8493b38d 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -158,8 +158,6 @@ class ImageApiMixin(object): 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 @@ class ImageApiMixin(object): 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 416dd7c4..f771dedd 100644 --- a/docker/auth/auth.py +++ b/docker/auth/auth.py @@ -16,11 +16,9 @@ import base64 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 @@ LEGACY_DOCKER_CONFIG_FILENAME = '.dockercfg' 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)) - - if 'index.docker.io' in parts[0]: - raise errors.InvalidRepository( - 'Invalid repository name, try "{0}" instead'.format(parts[1]) + 'Repository name cannot contain a scheme ({0})'.format(repo_name) ) - return parts[0], parts[1] + index_name, remote_name = split_repo_name(repo_name) + if index_name[0] == '-' or index_name[-1] == '-': + raise errors.InvalidRepository( + 'Invalid index name ({0}). Cannot begin or end with a' + ' hyphen.'.format(index_name) + ) + return index_name, remote_name + + +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 366f8696..560ee8e2 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 67830381..8e0b1d43 100644 --- a/tests/unit/auth_test.py +++ b/tests/unit/auth_test.py @@ -9,6 +9,7 @@ import shutil import tempfile from docker import auth +from docker import errors from .. import base @@ -29,25 +30,31 @@ class RegressionTest(base.BaseTestCase): 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 @@ class ResolveAuthTest(base.BaseTestCase): ('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'),