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 <joffrey@docker.com>

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

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):

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

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'),