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>

4 files changed, 55 insertions, 41 deletions

diff --git a/docker/api/image.py b/docker/api/image.py
index f891e21..8493b38 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 416dd7c..f771ded 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))
+            '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 366f869..560ee8e 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 6783038..8e0b1d4 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'),