Skip to content

Commit 0ede04d

Browse files
Gracefully handle credential store errors in get_all_credentials
When `get_all_credentials` iterates over credential stores (both the default `credsStore` and per-registry `credHelpers`), a `StoreError` from any single store would propagate as a `DockerException` and abort the entire operation. This caused image builds to fail even when the failing credentials were not needed (e.g., expired gcloud auth tokens when building a publicly accessible image). This change wraps each call to `_resolve_authconfig_credstore` inside `get_all_credentials` with a try/except that catches `DockerException`, logs a warning, and skips the failing entry instead of propagating the error. All other valid credentials are still collected and returned. Fixes #3379 Signed-off-by: Krishna Chaitanya Balusu <krishnabkc15@gmail.com>
1 parent df3f8e2 commit 0ede04d

2 files changed

Lines changed: 112 additions & 8 deletions

File tree

docker/auth.py

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -288,17 +288,35 @@ def get_all_credentials(self):
288288
# Retrieve all credentials from the default store
289289
store = self._get_store_instance(self.creds_store)
290290
for k in store.list().keys():
291-
auth_data[k] = self._resolve_authconfig_credstore(
292-
k, self.creds_store
293-
)
294-
auth_data[convert_to_hostname(k)] = auth_data[k]
291+
try:
292+
cred = self._resolve_authconfig_credstore(
293+
k, self.creds_store
294+
)
295+
except errors.DockerException as e:
296+
log.warning(
297+
'Failed to retrieve credentials from store '
298+
'"%s" for %s: %s — skipping this entry',
299+
self.creds_store, k, e,
300+
)
301+
continue
302+
auth_data[k] = cred
303+
auth_data[convert_to_hostname(k)] = cred
295304

296305
# credHelpers entries take priority over all others
297306
for reg, store_name in self.cred_helpers.items():
298-
auth_data[reg] = self._resolve_authconfig_credstore(
299-
reg, store_name
300-
)
301-
auth_data[convert_to_hostname(reg)] = auth_data[reg]
307+
try:
308+
cred = self._resolve_authconfig_credstore(
309+
reg, store_name
310+
)
311+
except errors.DockerException as e:
312+
log.warning(
313+
'Failed to retrieve credentials from store '
314+
'"%s" for %s: %s — skipping this entry',
315+
store_name, reg, e,
316+
)
317+
continue
318+
auth_data[reg] = cred
319+
auth_data[convert_to_hostname(reg)] = cred
302320

303321
return auth_data
304322

tests/unit/auth_test.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,51 @@ def test_get_all_credentials_3_sources(self):
772772
}
773773

774774

775+
def test_get_all_credentials_credstore_error_skipped(self):
776+
"""StoreError in get_all_credentials should be logged and skipped,
777+
not raised, so that a single broken credential store does not
778+
prevent building images that do not need those credentials.
779+
780+
See https://github.com/docker/docker-py/issues/3379
781+
"""
782+
failing_store = FailingStore('badstore')
783+
self.authconfig['credHelpers'] = {
784+
'broken-registry.io': 'badstore',
785+
}
786+
self.authconfig._stores['badstore'] = failing_store
787+
788+
# Should not raise; the broken entry should simply be omitted.
789+
result = self.authconfig.get_all_credentials()
790+
assert 'broken-registry.io' not in result
791+
792+
# Valid entries from the default store should still be present.
793+
assert 'gensokyo.jp' in result
794+
assert result['gensokyo.jp'] == {
795+
'Username': 'sakuya',
796+
'Password': 'izayoi',
797+
'ServerAddress': 'https://gensokyo.jp/v2',
798+
}
799+
800+
def test_get_all_credentials_default_store_error_skipped(self):
801+
"""StoreError from the default credsStore should be caught per-entry
802+
so that other valid entries are still returned."""
803+
mixed_store = PartiallyFailingStore('default')
804+
mixed_store.store(
805+
'https://good.io/v2', 'user', 'pass',
806+
)
807+
mixed_store.mark_failing('https://bad.io/v2')
808+
self.authconfig._stores['default'] = mixed_store
809+
810+
result = self.authconfig.get_all_credentials()
811+
assert 'good.io' in result
812+
assert result['good.io'] == {
813+
'Username': 'user',
814+
'Password': 'pass',
815+
'ServerAddress': 'https://good.io/v2',
816+
}
817+
assert 'bad.io' not in result
818+
819+
775820
class InMemoryStore(credentials.Store):
776821
def __init__(self, *args, **kwargs):
777822
self.__store = {}
@@ -796,3 +841,44 @@ def list(self):
796841

797842
def erase(self, server):
798843
del self.__store[server]
844+
845+
846+
class FailingStore(credentials.Store):
847+
"""A credential store that always raises StoreError on get()."""
848+
849+
def __init__(self, *args, **kwargs):
850+
pass
851+
852+
def get(self, server):
853+
raise credentials.errors.StoreError(
854+
f'Simulated store failure for {server}'
855+
)
856+
857+
def store(self, server, username, secret):
858+
pass
859+
860+
def list(self):
861+
return {}
862+
863+
def erase(self, server):
864+
pass
865+
866+
867+
class PartiallyFailingStore(InMemoryStore):
868+
"""A credential store that fails for specific servers."""
869+
870+
def __init__(self, *args, **kwargs):
871+
super().__init__(*args, **kwargs)
872+
self._failing = set()
873+
874+
def mark_failing(self, server):
875+
# Also register it so it appears in list()
876+
self.store(server, 'dummy', 'dummy')
877+
self._failing.add(server)
878+
879+
def get(self, server):
880+
if server in self._failing:
881+
raise credentials.errors.StoreError(
882+
f'Simulated store failure for {server}'
883+
)
884+
return super().get(server)

0 commit comments

Comments
 (0)