diff --git a/docker/api/image.py b/docker/api/image.py index 85109473..5f34d362 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -1,8 +1,11 @@ +import itertools +import json import logging import os from .. import auth, errors, utils from ..constants import DEFAULT_DATA_CHUNK_SIZE +from ..utils.json_stream import json_stream log = logging.getLogger(__name__) @@ -433,6 +436,29 @@ class ImageApiMixin: return self._result(response) + @staticmethod + def _raise_if_error(chunk, response): + """ + Raise an exception if the given chunk of the JSON server response is a + dictionary and contains an "error" field. Otherwise, return the chunk + as-is. + + Args: + chunk (object): A chunk of the server response. + response (Response): The full server response. This will be attached + to the exception in the event that chunk indicates an error. + + Returns: + (object): The input chunk. + + Raises: + :py:class:`docker.errors.APIError` + If the chunk of the server response contains an error message. + """ + if isinstance(chunk, dict) and 'error' in chunk: + raise errors.APIError(chunk['error'], response=response) + return chunk + def push(self, repository, tag=None, stream=False, auth_config=None, decode=False): """ @@ -494,8 +520,25 @@ class ImageApiMixin: self._raise_for_status(response) + # The server response might have status code 200 (OK) even though the + # push operation has failed. To detect errors, inspect each JSON chunk + # of the server response and check if an "error" entry is present. + # See: https://github.com/docker/docker-py/issues/3277 if stream: - return self._stream_helper(response, decode=decode) + if decode: + return (self._raise_if_error(chunk, response) for chunk in + self._stream_helper(response, decode=True)) + else: + result_stream, internal_stream = itertools.tee( + self._stream_helper(response, decode=False)) + for chunk_json in json_stream(internal_stream): + self._raise_if_error(chunk_json, response) + return result_stream + + for chunk_str in response.text.splitlines(): + chunk_json = json.loads(chunk_str) + if 'error' in chunk_json: + raise errors.APIError(chunk_json['error'], response=response) return self._result(response) diff --git a/tests/unit/api_image_test.py b/tests/unit/api_image_test.py index 692f4cf7..321421b0 100644 --- a/tests/unit/api_image_test.py +++ b/tests/unit/api_image_test.py @@ -271,6 +271,33 @@ class ImageTest(BaseAPIClientTest): timeout=DEFAULT_TIMEOUT_SECONDS ) + + def test_push_image_with_auth_error(self): + auth_config = { + 'username': "test_user", + 'password': "test_password", + 'serveraddress': "test_server", + } + encoded_auth = auth.encode_header(auth_config) + with pytest.raises(docker.errors.APIError, match='bad auth'): + self.client.push( + fake_api.FAKE_IMAGE_NAME_ERROR, tag=fake_api.FAKE_TAG_NAME, + auth_config=auth_config + ) + + fake_request.assert_called_with( + 'POST', + f"{url_prefix}images/test_image_error/push", + params={ + 'tag': fake_api.FAKE_TAG_NAME, + }, + data='{}', + headers={'Content-Type': 'application/json', + 'X-Registry-Auth': encoded_auth}, + stream=False, + timeout=DEFAULT_TIMEOUT_SECONDS + ) + def test_push_image_stream(self): with mock.patch('docker.auth.resolve_authconfig', fake_resolve_authconfig): @@ -315,6 +342,32 @@ class ImageTest(BaseAPIClientTest): ) + def test_push_image_stream_with_auth_error(self): + auth_config = { + 'username': "test_user", + 'password': "test_password", + 'serveraddress': "test_server", + } + encoded_auth = auth.encode_header(auth_config) + with pytest.raises(docker.errors.APIError, match='bad auth'): + self.client.push( + fake_api.FAKE_IMAGE_NAME_ERROR, tag=fake_api.FAKE_TAG_NAME, + auth_config=auth_config, stream=True + ) + + fake_request.assert_called_with( + 'POST', + f"{url_prefix}images/test_image_error/push", + params={ + 'tag': fake_api.FAKE_TAG_NAME, + }, + data='{}', + headers={'Content-Type': 'application/json', + 'X-Registry-Auth': encoded_auth}, + stream=True, + timeout=DEFAULT_TIMEOUT_SECONDS + ) + def test_tag_image(self): self.client.tag(fake_api.FAKE_IMAGE_ID, fake_api.FAKE_REPO_NAME) diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index 9d652dd6..2882ad5c 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -31,6 +31,8 @@ def response(status_code=200, content='', headers=None, reason=None, elapsed=0, request=None, raw=None): res = requests.Response() res.status_code = status_code + if isinstance(content, str): + content = content.encode('ascii') if not isinstance(content, bytes): content = json.dumps(content).encode('ascii') res._content = content diff --git a/tests/unit/fake_api.py b/tests/unit/fake_api.py index 03e53cc6..e44ed5bb 100644 --- a/tests/unit/fake_api.py +++ b/tests/unit/fake_api.py @@ -1,3 +1,5 @@ +import json + from docker import constants from . import fake_stat @@ -9,6 +11,7 @@ FAKE_IMAGE_ID = 'sha256:fe7a8fc91d3f17835cbb3b86a1c60287500ab01a53bc79c4497d09f0 FAKE_EXEC_ID = 'b098ec855f10434b5c7c973c78484208223a83f663ddaefb0f02a242840cb1c7' FAKE_NETWORK_ID = '1999cfb42e414483841a125ade3c276c3cb80cb3269b14e339354ac63a31b02c' FAKE_IMAGE_NAME = 'test_image' +FAKE_IMAGE_NAME_ERROR = 'test_image_error' FAKE_TARBALL_PATH = '/path/to/tarball' FAKE_REPO_NAME = 'repo' FAKE_TAG_NAME = 'tag' @@ -359,6 +362,12 @@ def post_fake_push(): return status_code, response +def post_fake_push_error(): + status_code = 200 + response = '{"status": "intermediate update"}\r\n{"error": "bad auth"}\r\n' + return status_code, response + + def post_fake_build_container(): status_code = 200 response = {'Id': FAKE_CONTAINER_ID} @@ -603,6 +612,8 @@ fake_responses = { get_fake_insert_image, f'{prefix}/{CURRENT_VERSION}/images/test_image/push': post_fake_push, + f'{prefix}/{CURRENT_VERSION}/images/test_image_error/push': + post_fake_push_error, f'{prefix}/{CURRENT_VERSION}/commit': post_fake_commit, f'{prefix}/{CURRENT_VERSION}/containers/create':