Check presence of errors in server response to image push

When pushing an image, the server response might have status code
200 (OK) even though the operation has failed.

To detect the occurrence of an error, inspect each JSON chunk in
the server response and verify that no "error" field is present.

Fixes: docker#3277

Signed-off-by: Francesco Zardi <[email protected]>

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 @@ 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 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 @@ def push(self, repository, tag=None, stream=False, auth_config=None,
 
         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)
 

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