Check presence of errors in server response to image push

Fixes: docker#3277

Signed-off-by: Francesco Zardi <frazar0@hotmail.it>

Author: frazar

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)
 

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)
 

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

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':