Merge pull request #1885 from docker/improve_authconfig_genconfig_separation

Improve separation between auth_configs and general_configs

12 files changed, 149 insertions, 111 deletions

diff --git a/docker/api/build.py b/docker/api/build.py
index 32238ef..220c93f 100644
--- a/docker/api/build.py
+++ b/docker/api/build.py
@@ -300,14 +300,12 @@ class BuildApiMixin(object):
                 # Matches CLI behavior: https://github.com/docker/docker/blob/
                 # 67b85f9d26f1b0b2b240f2d794748fac0f45243c/cliconfig/
                 # credentials/native_store.go#L68-L83
-                for registry in self._auth_configs.keys():
-                    if registry == 'credsStore' or registry == 'HttpHeaders':
-                        continue
+                for registry in self._auth_configs.get('auths', {}).keys():
                     auth_data[registry] = auth.resolve_authconfig(
                         self._auth_configs, registry
                     )
             else:
-                auth_data = self._auth_configs.copy()
+                auth_data = self._auth_configs.get('auths', {}).copy()
                 # See https://github.com/docker/docker-py/issues/1683
                 if auth.INDEX_NAME in auth_data:
                     auth_data[auth.INDEX_URL] = auth_data[auth.INDEX_NAME]
diff --git a/docker/api/client.py b/docker/api/client.py
index 10640e1..07bcfae 100644
--- a/docker/api/client.py
+++ b/docker/api/client.py
@@ -87,6 +87,7 @@ class APIClient(
     """
 
     __attrs__ = requests.Session.__attrs__ + ['_auth_configs',
+                                              '_general_configs',
                                               '_version',
                                               'base_url',
                                               'timeout']
@@ -105,8 +106,10 @@ class APIClient(
         self.timeout = timeout
         self.headers['User-Agent'] = user_agent
 
-        self._auth_configs = auth.load_config()
         self._general_configs = config.load_general_config()
+        self._auth_configs = auth.load_config(
+            config_dict=self._general_configs
+        )
 
         base_url = utils.parse_host(
             base_url, IS_WINDOWS_PLATFORM, tls=bool(tls)
diff --git a/docker/api/daemon.py b/docker/api/daemon.py
index 285b742..3998967 100644
--- a/docker/api/daemon.py
+++ b/docker/api/daemon.py
@@ -144,6 +144,8 @@ class DaemonApiMixin(object):
 
         response = self._post_json(self._url('/auth'), data=req_data)
         if response.status_code == 200:
+            if 'auths' not in self._auth_configs:
+                self._auth_configs['auths'] = {}
             self._auth_configs[registry or auth.INDEX_NAME] = req_data
         return self._result(response, json=True)
 
diff --git a/docker/auth.py b/docker/auth.py
index 79f63cc..91be2b8 100644
--- a/docker/auth.py
+++ b/docker/auth.py
@@ -98,11 +98,12 @@ def resolve_authconfig(authconfig, registry=None):
     registry = resolve_index_name(registry) if registry else INDEX_NAME
     log.debug("Looking for auth entry for {0}".format(repr(registry)))
 
-    if registry in authconfig:
+    authdict = authconfig.get('auths', {})
+    if registry in authdict:
         log.debug("Found {0}".format(repr(registry)))
-        return authconfig[registry]
+        return authdict[registry]
 
-    for key, conf in six.iteritems(authconfig):
+    for key, conf in six.iteritems(authdict):
         if resolve_index_name(key) == registry:
             log.debug("Found {0}".format(repr(key)))
             return conf
@@ -220,7 +221,7 @@ def parse_auth(entries, raise_on_error=False):
     return conf
 
 
-def load_config(config_path=None):
+def load_config(config_path=None, config_dict=None):
     """
     Loads authentication data from a Docker configuration file in the given
     root directory or if config_path is passed use given path.
@@ -228,39 +229,45 @@ def load_config(config_path=None):
         explicit config_path parameter > DOCKER_CONFIG environment variable >
         ~/.docker/config.json > ~/.dockercfg
     """
-    config_file = config.find_config_file(config_path)
 
-    if not config_file:
-        return {}
+    if not config_dict:
+        config_file = config.find_config_file(config_path)
+
+        if not config_file:
+            return {}
+        try:
+            with open(config_file) as f:
+                config_dict = json.load(f)
+        except (IOError, KeyError, ValueError) as e:
+            # Likely missing new Docker config file or it's in an
+            # unknown format, continue to attempt to read old location
+            # and format.
+            log.debug(e)
+            return _load_legacy_config(config_file)
+
+    res = {}
+    if config_dict.get('auths'):
+        log.debug("Found 'auths' section")
+        res.update({
+            'auths': parse_auth(config_dict.pop('auths'), raise_on_error=True)
+        })
+    if config_dict.get('credsStore'):
+        log.debug("Found 'credsStore' section")
+        res.update({'credsStore': config_dict.pop('credsStore')})
+    if config_dict.get('credHelpers'):
+        log.debug("Found 'credHelpers' section")
+        res.update({'credHelpers': config_dict.pop('credHelpers')})
+    if res:
+        return res
+
+    log.debug(
+        "Couldn't find auth-related section ; attempting to interpret"
+        "as auth-only file"
+    )
+    return parse_auth(config_dict)
 
-    try:
-        with open(config_file) as f:
-            data = json.load(f)
-            res = {}
-            if data.get('auths'):
-                log.debug("Found 'auths' section")
-                res.update(parse_auth(data['auths'], raise_on_error=True))
-            if data.get('HttpHeaders'):
-                log.debug("Found 'HttpHeaders' section")
-                res.update({'HttpHeaders': data['HttpHeaders']})
-            if data.get('credsStore'):
-                log.debug("Found 'credsStore' section")
-                res.update({'credsStore': data['credsStore']})
-            if data.get('credHelpers'):
-                log.debug("Found 'credHelpers' section")
-                res.update({'credHelpers': data['credHelpers']})
-            if res:
-                return res
-            else:
-                log.debug("Couldn't find 'auths' or 'HttpHeaders' sections")
-                f.seek(0)
-                return parse_auth(json.load(f))
-    except (IOError, KeyError, ValueError) as e:
-        # Likely missing new Docker config file or it's in an
-        # unknown format, continue to attempt to read old location
-        # and format.
-        log.debug(e)
 
+def _load_legacy_config(config_file):
     log.debug("Attempting to parse legacy auth file format")
     try:
         data = []
diff --git a/docker/utils/config.py b/docker/utils/config.py
index 8417261..82a0e2a 100644
--- a/docker/utils/config.py
+++ b/docker/utils/config.py
@@ -57,9 +57,10 @@ def load_general_config(config_path=None):
     try:
         with open(config_file) as f:
             return json.load(f)
-    except Exception as e:
+    except (IOError, ValueError) as e:
+        # In the case of a legacy `.dockercfg` file, we won't
+        # be able to load any JSON data.
         log.debug(e)
-        pass
 
     log.debug("All parsing attempts failed - returning empty config")
     return {}
diff --git a/docker/utils/decorators.py b/docker/utils/decorators.py
index 5e195c0..c975d4b 100644
--- a/docker/utils/decorators.py
+++ b/docker/utils/decorators.py
@@ -38,10 +38,10 @@ def minimum_version(version):
 
 def update_headers(f):
     def inner(self, *args, **kwargs):
-        if 'HttpHeaders' in self._auth_configs:
+        if 'HttpHeaders' in self._general_configs:
             if not kwargs.get('headers'):
-                kwargs['headers'] = self._auth_configs['HttpHeaders']
+                kwargs['headers'] = self._general_configs['HttpHeaders']
             else:
-                kwargs['headers'].update(self._auth_configs['HttpHeaders'])
+                kwargs['headers'].update(self._general_configs['HttpHeaders'])
         return f(self, *args, **kwargs)
     return inner
diff --git a/tests/integration/api_container_test.py b/tests/integration/api_container_test.py
index f48e78e..e5d7943 100644
--- a/tests/integration/api_container_test.py
+++ b/tests/integration/api_container_test.py
@@ -161,7 +161,7 @@ class CreateContainerTest(BaseAPIIntegrationTest):
         self.client.start(container3_id)
 
         info = self.client.inspect_container(res2['Id'])
-        self.assertCountEqual(info['HostConfig']['VolumesFrom'], vol_names)
+        assert len(info['HostConfig']['VolumesFrom']) == len(vol_names)
 
     def create_container_readonly_fs(self):
         ctnr = self.client.create_container(
diff --git a/tests/integration/base.py b/tests/integration/base.py
index 4f92901..04d9afd 100644
--- a/tests/integration/base.py
+++ b/tests/integration/base.py
@@ -4,7 +4,6 @@ import unittest
 
 import docker
 from docker.utils import kwargs_from_env
-import six
 
 from .. import helpers
 
@@ -19,9 +18,6 @@ class BaseIntegrationTest(unittest.TestCase):
     """
 
     def setUp(self):
-        if six.PY2:
-            self.assertRegex = self.assertRegexpMatches
-            self.assertCountEqual = self.assertItemsEqual
         self.tmp_imgs = []
         self.tmp_containers = []
         self.tmp_folders = []
diff --git a/tests/unit/api_build_test.py b/tests/unit/api_build_test.py
index e366bce..b20daea 100644
--- a/tests/unit/api_build_test.py
+++ b/tests/unit/api_build_test.py
@@ -73,10 +73,12 @@ class BuildTest(BaseAPIClientTest):
 
     def test_build_remote_with_registry_auth(self):
         self.client._auth_configs = {
-            'https://example.com': {
-                'user': 'example',
-                'password': 'example',
-                'email': 'example@example.com'
+            'auths': {
+                'https://example.com': {
+                    'user': 'example',
+                    'password': 'example',
+                    'email': 'example@example.com'
+                }
             }
         }
 
@@ -85,7 +87,10 @@ class BuildTest(BaseAPIClientTest):
                            'forcerm': False,
                            'remote': 'https://github.com/docker-library/mongo'}
         expected_headers = {
-            'X-Registry-Config': auth.encode_header(self.client._auth_configs)}
+            'X-Registry-Config': auth.encode_header(
+                self.client._auth_configs['auths']
+            )
+        }
 
         self.client.build(path='https://github.com/docker-library/mongo')
 
@@ -118,32 +123,43 @@ class BuildTest(BaseAPIClientTest):
 
     def test_set_auth_headers_with_empty_dict_and_auth_configs(self):
         self.client._auth_configs = {
-            'https://example.com': {
-                'user': 'example',
-                'password': 'example',
-                'email': 'example@example.com'
+            'auths': {
+                'https://example.com': {
+                    'user': 'example',
+                    'password': 'example',
+                    'email': 'example@example.com'
+                }
             }
         }
 
         headers = {}
         expected_headers = {
-            'X-Registry-Config': auth.encode_header(self.client._auth_configs)}
+            'X-Registry-Config': auth.encode_header(
+                self.client._auth_configs['auths']
+            )
+        }
+
         self.client._set_auth_headers(headers)
         assert headers == expected_headers
 
     def test_set_auth_headers_with_dict_and_auth_configs(self):
         self.client._auth_configs = {
-            'https://example.com': {
-                'user': 'example',
-                'password': 'example',
-                'email': 'example@example.com'
+            'auths': {
+                'https://example.com': {
+                    'user': 'example',
+                    'password': 'example',
+                    'email': 'example@example.com'
+                }
             }
         }
 
         headers = {'foo': 'bar'}
         expected_headers = {
-            'foo': 'bar',
-            'X-Registry-Config': auth.encode_header(self.client._auth_configs)}
+            'X-Registry-Config': auth.encode_header(
+                self.client._auth_configs['auths']
+            ),
+            'foo': 'bar'
+        }
 
         self.client._set_auth_headers(headers)
         assert headers == expected_headers
diff --git a/tests/unit/auth_test.py b/tests/unit/auth_test.py
index e3356d3..d6981cd 100644
--- a/tests/unit/auth_test.py
+++ b/tests/unit/auth_test.py
@@ -106,11 +106,13 @@ class ResolveAuthTest(unittest.TestCase):
     private_config = {'auth': encode_auth({'username': 'privateuser'})}
     legacy_config = {'auth': encode_auth({'username': 'legacyauth'})}
 
-    auth_config = auth.parse_auth({
-        'https://index.docker.io/v1/': index_config,
-        'my.registry.net': private_config,
-        'http://legacy.registry.url/v1/': legacy_config,
-    })
+    auth_config = {
+        'auths': auth.parse_auth({
+            'https://index.docker.io/v1/': index_config,
+            'my.registry.net': private_config,
+            'http://legacy.registry.url/v1/': legacy_config,
+        })
+    }
 
     def test_resolve_authconfig_hostname_only(self):
         assert auth.resolve_authconfig(
@@ -360,9 +362,8 @@ class LoadConfigTest(unittest.TestCase):
 
         with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}):
             cfg = auth.load_config(None)
-            assert registry in cfg
-            assert cfg[registry] is not None
-            cfg = cfg[registry]
+            assert registry in cfg['auths']
+            cfg = cfg['auths'][registry]
             assert cfg['username'] == 'sakuya'
             assert cfg['password'] == 'izayoi'
             assert cfg['email'] == 'sakuya@scarlet.net'
@@ -390,38 +391,13 @@ class LoadConfigTest(unittest.TestCase):
 
         with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}):
             cfg = auth.load_config(None)
-            assert registry in cfg
-            assert cfg[registry] is not None
-            cfg = cfg[registry]
+            assert registry in cfg['auths']
+            cfg = cfg['auths'][registry]
             assert cfg['username'] == b'sakuya\xc3\xa6'.decode('utf8')
             assert cfg['password'] == b'izayoi\xc3\xa6'.decode('utf8')
             assert cfg['email'] == 'sakuya@scarlet.net'
             assert cfg.get('auth') is None
 
-    def test_load_config_custom_config_env_with_headers(self):
-        folder = tempfile.mkdtemp()
-        self.addCleanup(shutil.rmtree, folder)
-
-        dockercfg_path = os.path.join(folder, 'config.json')
-        config = {
-            'HttpHeaders': {
-                'Name': 'Spike',
-                'Surname': 'Spiegel'
-            },
-        }
-
-        with open(dockercfg_path, 'w') as f:
-            json.dump(config, f)
-
-        with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}):
-            cfg = auth.load_config(None)
-            assert 'HttpHeaders' in cfg
-            assert cfg['HttpHeaders'] is not None
-            cfg = cfg['HttpHeaders']
-
-            assert cfg['Name'] == 'Spike'
-            assert cfg['Surname'] == 'Spiegel'
-
     def test_load_config_unknown_keys(self):
         folder = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, folder)
@@ -448,7 +424,7 @@ class LoadConfigTest(unittest.TestCase):
             json.dump(config, f)
 
         cfg = auth.load_config(dockercfg_path)
-        assert cfg == {'scarlet.net': {}}
+        assert cfg == {'auths': {'scarlet.net': {}}}
 
     def test_load_config_identity_token(self):
         folder = tempfile.mkdtemp()
@@ -469,7 +445,7 @@ class LoadConfigTest(unittest.TestCase):
             json.dump(config, f)
 
         cfg = auth.load_config(dockercfg_path)
-        assert registry in cfg
-        cfg = cfg[registry]
+        assert registry in cfg['auths']
+        cfg = cfg['auths'][registry]
         assert 'IdentityToken' in cfg
         assert cfg['IdentityToken'] == token
diff --git a/tests/unit/utils_config_test.py b/tests/unit/utils_config_test.py
index 45f75ff..50ba383 100644
--- a/tests/unit/utils_config_test.py
+++ b/tests/unit/utils_config_test.py
@@ -69,16 +69,55 @@ class LoadConfigTest(unittest.TestCase):
         folder = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, folder)
         cfg = config.load_general_config(folder)
-        self.assertTrue(cfg is not None)
+        assert cfg is not None
+        assert isinstance(cfg, dict)
+        assert not cfg
 
-    def test_load_config(self):
+    def test_load_config_custom_headers(self):
         folder = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, folder)
-        dockercfg_path = os.path.join(folder, '.dockercfg')
-        cfg = {
+
+        dockercfg_path = os.path.join(folder, 'config.json')
+        config_data = {
+            'HttpHeaders': {
+                'Name': 'Spike',
+                'Surname': 'Spiegel'
+            },
+        }
+
+        with open(dockercfg_path, 'w') as f:
+            json.dump(config_data, f)
+
+        cfg = config.load_general_config(dockercfg_path)
+        assert 'HttpHeaders' in cfg
+        assert cfg['HttpHeaders'] == {
+            'Name': 'Spike',
+            'Surname': 'Spiegel'
+        }
+
+    def test_load_config_detach_keys(self):
+        folder = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, folder)
+        dockercfg_path = os.path.join(folder, 'config.json')
+        config_data = {
+            'detachKeys': 'ctrl-q, ctrl-u, ctrl-i'
+        }
+        with open(dockercfg_path, 'w') as f:
+            json.dump(config_data, f)
+
+        cfg = config.load_general_config(dockercfg_path)
+        assert cfg == config_data
+
+    def test_load_config_from_env(self):
+        folder = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, folder)
+        dockercfg_path = os.path.join(folder, 'config.json')
+        config_data = {
             'detachKeys': 'ctrl-q, ctrl-u, ctrl-i'
         }
         with open(dockercfg_path, 'w') as f:
-            json.dump(cfg, f)
+            json.dump(config_data, f)
 
-        self.assertEqual(config.load_general_config(dockercfg_path), cfg)
+        with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}):
+            cfg = config.load_general_config(None)
+        assert cfg == config_data
diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py
index 73f95d6..1f9daf6 100644
--- a/tests/unit/utils_test.py
+++ b/tests/unit/utils_test.py
@@ -46,7 +46,7 @@ class DecoratorsTest(unittest.TestCase):
             return headers
 
         client = APIClient()
-        client._auth_configs = {}
+        client._general_configs = {}
 
         g = update_headers(f)
         assert g(client, headers=None) is None
@@ -55,7 +55,7 @@ class DecoratorsTest(unittest.TestCase):
             'Content-type': 'application/json',
         }
 
-        client._auth_configs = {
+        client._general_configs = {
             'HttpHeaders': sample_headers
         }