Stop pinging registries from the client

The daemon already pings the registry, so doing it on our end is
redundant and error-prone.

The `insecure_registry` argument to `push()`, `pull()` and `login()` has
been deprecated - in the latter case, it wasn't being used anyway.

The `insecure` argument to `docker.auth.resolve_repository_name()` has
also been deprecated.

`docker.utils.ping_registry()` has been deprecated.

`docker.auth.expand_registry_url()` has been removed.

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>

7 files changed, 138 insertions, 28 deletions

diff --git a/docker/auth/__init__.py b/docker/auth/__init__.py
index d068b7f..6fc83f8 100644
--- a/docker/auth/__init__.py
+++ b/docker/auth/__init__.py
@@ -1,4 +1,5 @@
 from .auth import (
+    INDEX_NAME,
     INDEX_URL,
     encode_header,
     load_config,
diff --git a/docker/auth/auth.py b/docker/auth/auth.py
index 1c29615..7c4876a 100644
--- a/docker/auth/auth.py
+++ b/docker/auth/auth.py
@@ -16,38 +16,33 @@ import base64
 import fileinput
 import json
 import os
+import warnings
 
 import six
 
-from ..utils import utils
 from .. import errors
 
-INDEX_URL = 'https://index.docker.io/v1/'
+INDEX_NAME = 'index.docker.io'
+INDEX_URL = 'https://{0}/v1/'.format(INDEX_NAME)
 DOCKER_CONFIG_FILENAME = os.path.join('.docker', 'config.json')
 LEGACY_DOCKER_CONFIG_FILENAME = '.dockercfg'
 
 
-def expand_registry_url(hostname, insecure=False):
-    if hostname.startswith('http:') or hostname.startswith('https:'):
-        return hostname
-    if utils.ping_registry('https://' + hostname):
-        return 'https://' + hostname
-    elif insecure:
-        return 'http://' + hostname
-    else:
-        raise errors.DockerException(
-            "HTTPS endpoint unresponsive and insecure mode isn't enabled."
+def resolve_repository_name(repo_name, insecure=False):
+    if insecure:
+        warnings.warn(
+            'The `insecure` argument to resolve_repository_name() '
+            'is deprecated and non-functional. Please remove it.',
+            DeprecationWarning
         )
 
-
-def resolve_repository_name(repo_name, insecure=False):
     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_URL, repo_name
+        return INDEX_NAME, repo_name
     if len(parts) < 2:
         raise errors.InvalidRepository(
             'Invalid repository name ({0})'.format(repo_name))
@@ -57,7 +52,7 @@ def resolve_repository_name(repo_name, insecure=False):
             'Invalid repository name, try "{0}" instead'.format(parts[1])
         )
 
-    return expand_registry_url(parts[0], insecure), parts[1]
+    return parts[0], parts[1]
 
 
 def resolve_authconfig(authconfig, registry=None):
@@ -68,7 +63,7 @@ def resolve_authconfig(authconfig, registry=None):
     Returns None if no match was found.
     """
     # Default to the public index server
-    registry = convert_to_hostname(registry) if registry else INDEX_URL
+    registry = convert_to_hostname(registry) if registry else INDEX_NAME
 
     if registry in authconfig:
         return authconfig[registry]
@@ -185,7 +180,7 @@ def load_config(config_path=None):
                 'Invalid or empty configuration file!')
 
         username, password = decode_auth(data[0])
-        conf[INDEX_URL] = {
+        conf[INDEX_NAME] = {
             'username': username,
             'password': password,
             'email': data[1],
diff --git a/docker/client.py b/docker/client.py
index e52cb53..285718b 100644
--- a/docker/client.py
+++ b/docker/client.py
@@ -25,6 +25,7 @@ from . import constants
 from . import errors
 from .auth import auth
 from .utils import utils, check_resource
+from .constants import INSECURE_REGISTRY_DEPRECATION_WARNING
 
 
 class Client(clientbase.ClientBase):
@@ -499,6 +500,12 @@ class Client(clientbase.ClientBase):
 
     def login(self, username, password=None, email=None, registry=None,
               reauth=False, insecure_registry=False, dockercfg_path=None):
+        if insecure_registry:
+            warnings.warn(
+                INSECURE_REGISTRY_DEPRECATION_WARNING.format('login()'),
+                DeprecationWarning
+            )
+
         # If we don't have any auth data so far, try reloading the config file
         # one more time in case anything showed up in there.
         # If dockercfg_path is passed check to see if the config file exists,
@@ -584,11 +591,15 @@ class Client(clientbase.ClientBase):
 
     def pull(self, repository, tag=None, stream=False,
              insecure_registry=False, auth_config=None):
+        if insecure_registry:
+            warnings.warn(
+                INSECURE_REGISTRY_DEPRECATION_WARNING.format('pull()'),
+                DeprecationWarning
+            )
+
         if not tag:
             repository, tag = utils.parse_repository_tag(repository)
-        registry, repo_name = auth.resolve_repository_name(
-            repository, insecure=insecure_registry
-        )
+        registry, repo_name = auth.resolve_repository_name(repository)
         if repo_name.count(":") == 1:
             repository, tag = repository.rsplit(":", 1)
 
@@ -631,11 +642,15 @@ class Client(clientbase.ClientBase):
 
     def push(self, repository, tag=None, stream=False,
              insecure_registry=False):
+        if insecure_registry:
+            warnings.warn(
+                INSECURE_REGISTRY_DEPRECATION_WARNING.format('push()'),
+                DeprecationWarning
+            )
+
         if not tag:
             repository, tag = utils.parse_repository_tag(repository)
-        registry, repo_name = auth.resolve_repository_name(
-            repository, insecure=insecure_registry
-        )
+        registry, repo_name = auth.resolve_repository_name(repository)
         u = self._url("/images/{0}/push".format(repository))
         params = {
             'tag': tag
diff --git a/docker/constants.py b/docker/constants.py
index f99f192..10a2fee 100644
--- a/docker/constants.py
+++ b/docker/constants.py
@@ -4,3 +4,7 @@ STREAM_HEADER_SIZE_BYTES = 8
 CONTAINER_LIMITS_KEYS = [
     'memory', 'memswap', 'cpushares', 'cpusetcpus'
 ]
+
+INSECURE_REGISTRY_DEPRECATION_WARNING = \
+    'The `insecure_registry` argument to {} ' \
+    'is deprecated and non-functional. Please remove it.'
diff --git a/docker/utils/utils.py b/docker/utils/utils.py
index 175a7e0..10c08de 100644
--- a/docker/utils/utils.py
+++ b/docker/utils/utils.py
@@ -19,6 +19,7 @@ import json
 import shlex
 import tarfile
 import tempfile
+import warnings
 from distutils.version import StrictVersion
 from fnmatch import fnmatch
 from datetime import datetime
@@ -120,6 +121,11 @@ def compare_version(v1, v2):
 
 
 def ping_registry(url):
+    warnings.warn(
+        'The `ping_registry` method is deprecated and will be removed.',
+        DeprecationWarning
+    )
+
     return ping(url + '/v2/', [401]) or ping(url + '/v1/_ping')
 
 
diff --git a/tests/test.py b/tests/test.py
index f6535b2..3171bf9 100644
--- a/tests/test.py
+++ b/tests/test.py
@@ -2424,9 +2424,9 @@ class DockerClientTest(Cleanup, base.BaseTestCase):
             f.write('auth = {0}\n'.format(auth_))
             f.write('email = sakuya@scarlet.net')
         cfg = docker.auth.load_config(dockercfg_path)
-        self.assertTrue(docker.auth.INDEX_URL in cfg)
-        self.assertNotEqual(cfg[docker.auth.INDEX_URL], None)
-        cfg = cfg[docker.auth.INDEX_URL]
+        self.assertTrue(docker.auth.INDEX_NAME in cfg)
+        self.assertNotEqual(cfg[docker.auth.INDEX_NAME], None)
+        cfg = cfg[docker.auth.INDEX_NAME]
         self.assertEqual(cfg['username'], 'sakuya')
         self.assertEqual(cfg['password'], 'izayoi')
         self.assertEqual(cfg['email'], 'sakuya@scarlet.net')
diff --git a/tests/utils_test.py b/tests/utils_test.py
index 716cde5..1c8729c 100644
--- a/tests/utils_test.py
+++ b/tests/utils_test.py
@@ -9,7 +9,7 @@ from docker.utils import (
     create_host_config, Ulimit, LogConfig, parse_bytes
 )
 from docker.utils.ports import build_port_bindings, split_port
-from docker.auth import resolve_authconfig
+from docker.auth import resolve_repository_name, resolve_authconfig
 
 import base
 
@@ -167,6 +167,61 @@ class UtilsTest(base.BaseTestCase):
             type=LogConfig.types.JSON, config='helloworld'
         ))
 
+    def test_resolve_repository_name(self):
+        # docker hub library image
+        self.assertEqual(
+            resolve_repository_name('image'),
+            ('index.docker.io', 'image'),
+        )
+
+        # docker hub image
+        self.assertEqual(
+            resolve_repository_name('username/image'),
+            ('index.docker.io', 'username/image'),
+        )
+
+        # private registry
+        self.assertEqual(
+            resolve_repository_name('my.registry.net/image'),
+            ('my.registry.net', 'image'),
+        )
+
+        # private registry with port
+        self.assertEqual(
+            resolve_repository_name('my.registry.net:5000/image'),
+            ('my.registry.net:5000', 'image'),
+        )
+
+        # private registry with username
+        self.assertEqual(
+            resolve_repository_name('my.registry.net/username/image'),
+            ('my.registry.net', 'username/image'),
+        )
+
+        # no dots but port
+        self.assertEqual(
+            resolve_repository_name('hostname:5000/image'),
+            ('hostname:5000', 'image'),
+        )
+
+        # no dots but port and username
+        self.assertEqual(
+            resolve_repository_name('hostname:5000/username/image'),
+            ('hostname:5000', 'username/image'),
+        )
+
+        # localhost
+        self.assertEqual(
+            resolve_repository_name('localhost/image'),
+            ('localhost', 'image'),
+        )
+
+        # localhost with username
+        self.assertEqual(
+            resolve_repository_name('localhost/username/image'),
+            ('localhost', 'username/image'),
+        )
+
     def test_resolve_authconfig(self):
         auth_config = {
             'https://index.docker.io/v1/': {'auth': 'indexuser'},
@@ -231,6 +286,40 @@ class UtilsTest(base.BaseTestCase):
             resolve_authconfig(auth_config, 'does.not.exist') is None
         )
 
+    def test_resolve_registry_and_auth(self):
+        auth_config = {
+            'https://index.docker.io/v1/': {'auth': 'indexuser'},
+            'my.registry.net': {'auth': 'privateuser'},
+        }
+
+        # library image
+        image = 'image'
+        self.assertEqual(
+            resolve_authconfig(auth_config, resolve_repository_name(image)[0]),
+            {'auth': 'indexuser'},
+        )
+
+        # docker hub image
+        image = 'username/image'
+        self.assertEqual(
+            resolve_authconfig(auth_config, resolve_repository_name(image)[0]),
+            {'auth': 'indexuser'},
+        )
+
+        # private registry
+        image = 'my.registry.net/image'
+        self.assertEqual(
+            resolve_authconfig(auth_config, resolve_repository_name(image)[0]),
+            {'auth': 'privateuser'},
+        )
+
+        # unauthenticated registry
+        image = 'other.registry.net/image'
+        self.assertEqual(
+            resolve_authconfig(auth_config, resolve_repository_name(image)[0]),
+            None,
+        )
+
     def test_split_port_with_host_ip(self):
         internal_port, external_port = split_port("127.0.0.1:1000:2000")
         self.assertEqual(internal_port, ["2000"])