Merge pull request #929 from docker/927-config-parsing

Don't break when parsing unknown config keys

Author: shin-

docker/auth/auth.py

@@ -46,7 +46,7 @@ def resolve_repository_name(repo_name):
 
 def resolve_index_name(index_name):
     index_name = convert_to_hostname(index_name)
-    if index_name == 'index.'+INDEX_NAME:
+    if index_name == 'index.' + INDEX_NAME:
         index_name = INDEX_NAME
     return index_name
 
@@ -102,19 +102,34 @@ def encode_header(auth):
     return base64.urlsafe_b64encode(auth_json)
 
 
-def parse_auth(entries):
+def parse_auth(entries, raise_on_error=False):
     """
     Parses authentication entries
 
     Args:
-      entries: Dict of authentication entries.
+      entries:        Dict of authentication entries.
+      raise_on_error: If set to true, an invalid format will raise
+                      InvalidConfigFile
 
     Returns:
       Authentication registry.
     """
 
     conf = {}
     for registry, entry in six.iteritems(entries):
+        if not (isinstance(entry, dict) and 'auth' in entry):
+            log.debug(
+                'Config entry for key {0} is not auth config'.format(registry)
+            )
+            # We sometimes fall back to parsing the whole config as if it was
+            # the auth config by itself, for legacy purposes. In that case, we
+            # fail silently and return an empty conf if any of the keys is not
+            # formatted properly.
+            if raise_on_error:
+                raise errors.InvalidConfigFile(
+                    'Invalid configuration for registry {0}'.format(registry)
+                )
+            return {}
         username, password = decode_auth(entry['auth'])
         log.debug(
             'Found entry (registry={0}, username={1})'
@@ -170,7 +185,7 @@ def load_config(config_path=None):
             res = {}
             if data.get('auths'):
                 log.debug("Found 'auths' section")
-                res.update(parse_auth(data['auths']))
+                res.update(parse_auth(data['auths'], raise_on_error=True))
             if data.get('HttpHeaders'):
                 log.debug("Found 'HttpHeaders' section")
                 res.update({'HttpHeaders': data['HttpHeaders']})

tests/unit/auth_test.py

@@ -433,3 +433,32 @@ def test_load_config_custom_config_env_with_headers(self):
 
             self.assertEqual(cfg['Name'], 'Spike')
             self.assertEqual(cfg['Surname'], 'Spiegel')
+
+    def test_load_config_unknown_keys(self):
+        folder = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, folder)
+        dockercfg_path = os.path.join(folder, 'config.json')
+        config = {
+            'detachKeys': 'ctrl-q, ctrl-u, ctrl-i'
+        }
+        with open(dockercfg_path, 'w') as f:
+            json.dump(config, f)
+
+        cfg = auth.load_config(dockercfg_path)
+        assert cfg == {}
+
+    def test_load_config_invalid_auth_dict(self):
+        folder = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, folder)
+        dockercfg_path = os.path.join(folder, 'config.json')
+        config = {
+            'auths': {
+                'scarlet.net': {'sakuya': 'izayoi'}
+            }
+        }
+        with open(dockercfg_path, 'w') as f:
+            json.dump(config, f)
+
+        self.assertRaises(
+            errors.InvalidConfigFile, auth.load_config, dockercfg_path
+        )