Improve error messaging around auth failures
Rather than just dumping the raw HTTPError exception, we should provide some additional guidance for the end user in the specific case that it's still returning 401 when we attempt to authenticate. Because we retry authentication to reissue tokens, if the authentication ultimately fails with 401 then it points to bad credentials or the container/registry is not actually available. Some registries return 401 instead of a 404. Change-Id: Ic8da92d98656bcea7db5701b44eeadc1d5c87198 Closes-Bug: #1860040
This commit is contained in:
parent
6f9b854ddc
commit
d15d8df6ce
|
@ -845,8 +845,16 @@ class BaseImageUploader(object):
|
|||
fallback_tag=None, username=None, password=None):
|
||||
image_url = self._image_to_url(image)
|
||||
self.is_insecure_registry(registry_host=image_url.netloc)
|
||||
session = self.authenticate(
|
||||
image_url, username=username, password=password)
|
||||
try:
|
||||
session = self.authenticate(
|
||||
image_url, username=username, password=password)
|
||||
except requests.exceptions.HTTPError as e:
|
||||
if e.response.status_code == 401:
|
||||
raise ImageUploaderException(
|
||||
'Unable to authenticate. This may indicate '
|
||||
'missing registry credentials or the provided '
|
||||
'container or namespace does not exist. %s' % e)
|
||||
raise
|
||||
|
||||
i = self._inspect(image_url, session)
|
||||
return self._discover_tag_from_inspect(i, image, tag_from_label,
|
||||
|
@ -858,8 +866,16 @@ class BaseImageUploader(object):
|
|||
for image in images:
|
||||
url = self._image_to_url(image)
|
||||
self.is_insecure_registry(registry_host=url.netloc)
|
||||
session = self.authenticate(
|
||||
url, username=username, password=password)
|
||||
try:
|
||||
session = self.authenticate(
|
||||
url, username=username, password=password)
|
||||
except requests.exceptions.HTTPError as e:
|
||||
if e.response.status_code == 401:
|
||||
raise ImageUploaderException(
|
||||
'Unable to authenticate. This may indicate '
|
||||
'missing registry credentials or the provided '
|
||||
'container or namespace does not exist. %s' % e)
|
||||
raise
|
||||
image_labels = self._image_labels(
|
||||
url, session=session)
|
||||
if set(labels).issubset(set(image_labels)):
|
||||
|
@ -1295,11 +1311,19 @@ class PythonImageUploader(BaseImageUploader):
|
|||
|
||||
target_username, target_password = self.credentials_for_registry(
|
||||
t.target_image_url.netloc)
|
||||
target_session = self.authenticate(
|
||||
t.target_image_url,
|
||||
username=target_username,
|
||||
password=target_password
|
||||
)
|
||||
try:
|
||||
target_session = self.authenticate(
|
||||
t.target_image_url,
|
||||
username=target_username,
|
||||
password=target_password
|
||||
)
|
||||
except requests.exceptions.HTTPError as e:
|
||||
if e.response.status_code == 401:
|
||||
raise ImageUploaderException(
|
||||
'Unable to authenticate. This may indicate '
|
||||
'missing registry credentials or the provided '
|
||||
'container or namespace does not exist. %s' % e)
|
||||
raise
|
||||
|
||||
try:
|
||||
self._detect_target_export(t.target_image_url, target_session)
|
||||
|
@ -1355,11 +1379,19 @@ class PythonImageUploader(BaseImageUploader):
|
|||
|
||||
source_username, source_password = self.credentials_for_registry(
|
||||
t.source_image_url.netloc)
|
||||
source_session = self.authenticate(
|
||||
t.source_image_url,
|
||||
username=source_username,
|
||||
password=source_password
|
||||
)
|
||||
try:
|
||||
source_session = self.authenticate(
|
||||
t.source_image_url,
|
||||
username=source_username,
|
||||
password=source_password
|
||||
)
|
||||
except requests.exceptions.HTTPError as e:
|
||||
if e.response.status_code == 401:
|
||||
raise ImageUploaderException(
|
||||
'Unable to authenticate. This may indicate '
|
||||
'missing registry credentials or the provided '
|
||||
'container or namespace does not exist. %s' % e)
|
||||
raise
|
||||
|
||||
source_layers = []
|
||||
manifests_str = []
|
||||
|
@ -2312,8 +2344,16 @@ def discover_tag_from_inspect(args):
|
|||
self, image, tag_from_label = args
|
||||
image_url = self._image_to_url(image)
|
||||
username, password = self.credentials_for_registry(image_url.netloc)
|
||||
session = self.authenticate(
|
||||
image_url, username=username, password=password)
|
||||
try:
|
||||
session = self.authenticate(
|
||||
image_url, username=username, password=password)
|
||||
except requests.exceptions.HTTPError as e:
|
||||
if e.response.status_code == 401:
|
||||
raise ImageUploaderException(
|
||||
'Unable to authenticate. This may indicate '
|
||||
'missing registry credentials or the provided '
|
||||
'container or namespace does not exist. %s' % e)
|
||||
raise
|
||||
i = self._inspect(image_url, session=session)
|
||||
session.close()
|
||||
if ':' in image_url.path:
|
||||
|
|
|
@ -394,6 +394,25 @@ class TestBaseImageUploader(base.TestCase):
|
|||
'docker.io/t/foo:b',
|
||||
'rdo_version')
|
||||
|
||||
# handle auth issues
|
||||
mock_401 = mock.Mock()
|
||||
mock_401.status_code = 401
|
||||
mock_401_except = requests.exceptions.HTTPError(response=mock_401)
|
||||
mock_404 = mock.Mock()
|
||||
mock_404.status_code = 404
|
||||
mock_404_except = requests.exceptions.HTTPError(response=mock_404)
|
||||
mock_auth.side_effect = [mock_401_except, mock_404_except]
|
||||
self.assertRaises(
|
||||
ImageUploaderException,
|
||||
image_uploader.discover_tag_from_inspect,
|
||||
(self.uploader, 'docker.io/t/foo', 'rdo_version')
|
||||
)
|
||||
self.assertRaises(
|
||||
requests.exceptions.HTTPError,
|
||||
image_uploader.discover_tag_from_inspect,
|
||||
(self.uploader, 'docker.io/t/foo', 'rdo_version')
|
||||
)
|
||||
|
||||
@mock.patch('tripleo_common.image.image_uploader.'
|
||||
'BaseImageUploader.authenticate')
|
||||
@mock.patch('tripleo_common.image.image_uploader.'
|
||||
|
@ -480,6 +499,25 @@ class TestBaseImageUploader(base.TestCase):
|
|||
(self.uploader, 'docker.io/t/foo', 'rdo_version')
|
||||
)
|
||||
|
||||
# handle auth issues
|
||||
mock_401 = mock.Mock()
|
||||
mock_401.status_code = 401
|
||||
mock_401_except = requests.exceptions.HTTPError(response=mock_401)
|
||||
mock_404 = mock.Mock()
|
||||
mock_404.status_code = 404
|
||||
mock_404_except = requests.exceptions.HTTPError(response=mock_404)
|
||||
mock_auth.side_effect = [mock_401_except, mock_404_except]
|
||||
self.assertRaises(
|
||||
ImageUploaderException,
|
||||
image_uploader.discover_tag_from_inspect,
|
||||
(self.uploader, 'docker.io/t/foo', 'rdo_version')
|
||||
)
|
||||
self.assertRaises(
|
||||
requests.exceptions.HTTPError,
|
||||
image_uploader.discover_tag_from_inspect,
|
||||
(self.uploader, 'docker.io/t/foo', 'rdo_version')
|
||||
)
|
||||
|
||||
@mock.patch('concurrent.futures.ThreadPoolExecutor')
|
||||
def test_discover_image_tags(self, mock_pool):
|
||||
mock_map = mock.Mock()
|
||||
|
|
Loading…
Reference in New Issue