From 20b5575d4668ebd75d8541649cab797881db3cb9 Mon Sep 17 00:00:00 2001 From: Francesco Zardi Date: Sat, 14 Sep 2024 17:25:37 +0200 Subject: [PATCH] Check presence of errors in server response to image push Fixes: #3277 Signed-off-by: Francesco Zardi --- docker/api/image.py | 28 ++++++++++++++++++- tests/unit/api_image_test.py | 53 ++++++++++++++++++++++++++++++++++++ tests/unit/api_test.py | 2 ++ tests/unit/fake_api.py | 11 ++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/docker/api/image.py b/docker/api/image.py index 85109473b..8ffc3a1f0 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,16 @@ def pull(self, repository, tag=None, stream=False, auth_config=None, return self._result(response) + @staticmethod + def _raise_if_error(chunk, response): + """ + Raise an exception if the specified chunk of a JSON 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): """ @@ -495,7 +508,20 @@ def push(self, repository, tag=None, stream=False, auth_config=None, self._raise_for_status(response) 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 692f4cf73..321421b0e 100644 --- a/tests/unit/api_image_test.py +++ b/tests/unit/api_image_test.py @@ -271,6 +271,33 @@ def test_push_image_with_auth(self): 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 @@ def test_push_image_stream_with_auth(self): ) + 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 9d652dd6a..2882ad5c3 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 03e53cc64..a4bd72d9e 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_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' @@ -358,6 +361,12 @@ def post_fake_push(): response = {'Id': FAKE_IMAGE_ID} return status_code, response +def post_fake_push_error(): + status_code = 200 + response = "\r\n".join( + json.dumps(c) for c in [{'Id': FAKE_IMAGE_ID}, {'error': 'bad auth'}]) + return status_code, response + def post_fake_build_container(): status_code = 200 @@ -603,6 +612,8 @@ def post_fake_config(): 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':