Merge pull request #861 from docker/860-deprecate-resolve-repo-name

Update repo name resolution method

5 files changed, 101 insertions, 55 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'),
diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py
index 3c9f6e2..57ad443 100644
--- a/tests/unit/utils_test.py
+++ b/tests/unit/utils_test.py
@@ -352,23 +352,55 @@ class ParseHostTest(base.BaseTestCase):
             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))