Gracefully handle credential store errors in get_all_credentials

docker/auth.py

@@ -288,17 +288,35 @@ def get_all_credentials(self):
             # Retrieve all credentials from the default store
             store = self._get_store_instance(self.creds_store)
             for k in store.list().keys():
-                auth_data[k] = self._resolve_authconfig_credstore(
-                    k, self.creds_store
-                )
-                auth_data[convert_to_hostname(k)] = auth_data[k]
+                try:
+                    cred = self._resolve_authconfig_credstore(
+                        k, self.creds_store
+                    )
+                except errors.DockerException as e:
+                    log.warning(
+                        'Failed to retrieve credentials from store '
+                        '"%s" for %s: %s — skipping this entry',
+                        self.creds_store, k, e,
+                    )
+                    continue
+                auth_data[k] = cred
+                auth_data[convert_to_hostname(k)] = cred
 
         # credHelpers entries take priority over all others
         for reg, store_name in self.cred_helpers.items():
-            auth_data[reg] = self._resolve_authconfig_credstore(
-                reg, store_name
-            )
-            auth_data[convert_to_hostname(reg)] = auth_data[reg]
+            try:
+                cred = self._resolve_authconfig_credstore(
+                    reg, store_name
+                )
+            except errors.DockerException as e:
+                log.warning(
+                    'Failed to retrieve credentials from store '
+                    '"%s" for %s: %s — skipping this entry',
+                    store_name, reg, e,
+                )
+                continue
+            auth_data[reg] = cred
+            auth_data[convert_to_hostname(reg)] = cred
 
         return auth_data
 

tests/unit/auth_test.py

@@ -772,6 +772,51 @@ def test_get_all_credentials_3_sources(self):
         }
 
 
+    def test_get_all_credentials_credstore_error_skipped(self):
+        """StoreError in get_all_credentials should be logged and skipped,
+        not raised, so that a single broken credential store does not
+        prevent building images that do not need those credentials.
+
+        See https://github.com/docker/docker-py/issues/3379
+        """
+        failing_store = FailingStore('badstore')
+        self.authconfig['credHelpers'] = {
+            'broken-registry.io': 'badstore',
+        }
+        self.authconfig._stores['badstore'] = failing_store
+
+        # Should not raise; the broken entry should simply be omitted.
+        result = self.authconfig.get_all_credentials()
+        assert 'broken-registry.io' not in result
+
+        # Valid entries from the default store should still be present.
+        assert 'gensokyo.jp' in result
+        assert result['gensokyo.jp'] == {
+            'Username': 'sakuya',
+            'Password': 'izayoi',
+            'ServerAddress': 'https://gensokyo.jp/v2',
+        }
+
+    def test_get_all_credentials_default_store_error_skipped(self):
+        """StoreError from the default credsStore should be caught per-entry
+        so that other valid entries are still returned."""
+        mixed_store = PartiallyFailingStore('default')
+        mixed_store.store(
+            'https://good.io/v2', 'user', 'pass',
+        )
+        mixed_store.mark_failing('https://bad.io/v2')
+        self.authconfig._stores['default'] = mixed_store
+
+        result = self.authconfig.get_all_credentials()
+        assert 'good.io' in result
+        assert result['good.io'] == {
+            'Username': 'user',
+            'Password': 'pass',
+            'ServerAddress': 'https://good.io/v2',
+        }
+        assert 'bad.io' not in result
+
+
 class InMemoryStore(credentials.Store):
     def __init__(self, *args, **kwargs):
         self.__store = {}
@@ -796,3 +841,44 @@ def list(self):
 
     def erase(self, server):
         del self.__store[server]
+
+
+class FailingStore(credentials.Store):
+    """A credential store that always raises StoreError on get()."""
+
+    def __init__(self, *args, **kwargs):
+        pass
+
+    def get(self, server):
+        raise credentials.errors.StoreError(
+            f'Simulated store failure for {server}'
+        )
+
+    def store(self, server, username, secret):
+        pass
+
+    def list(self):
+        return {}
+
+    def erase(self, server):
+        pass
+
+
+class PartiallyFailingStore(InMemoryStore):
+    """A credential store that fails for specific servers."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._failing = set()
+
+    def mark_failing(self, server):
+        # Also register it so it appears in list()
+        self.store(server, 'dummy', 'dummy')
+        self._failing.add(server)
+
+    def get(self, server):
+        if server in self._failing:
+            raise credentials.errors.StoreError(
+                f'Simulated store failure for {server}'
+            )
+        return super().get(server)