Skip to content

Commit 6ffd5c7

Browse files
committed
Ensure that APIError is thrown on failure to pull image
Podman part of the functionality implemented in containers/podman#28109 Fixes: #301 Signed-off-by: Šimon Brauner <[email protected]>
1 parent df7bca4 commit 6ffd5c7

2 files changed

Lines changed: 81 additions & 0 deletions

File tree

podman/domain/images_manager.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,9 @@ def pull(
400400
params["compatMode"] = True
401401
stream = True
402402

403+
if not params["compatMode"] and not stream:
404+
params["quiet"] = True
405+
403406
response = self.client.post("/images/pull", params=params, stream=stream, headers=headers)
404407
response.raise_for_status(not_found=ImageNotFound)
405408

@@ -572,8 +575,30 @@ def _stream_helper(self, response, decode=False):
572575
break
573576
if reader._fp.chunk_left:
574577
data += reader.read(reader._fp.chunk_left)
578+
try:
579+
data_dictionary = json.loads(data)
580+
except json.JSONDecodeError as e:
581+
self._stream_error_helper("Service returned invalid JSON", e.msg)
582+
except UnicodeDecodeError as e:
583+
self._stream_error_helper("Service returned wrongly encoded data", e.msg)
584+
error = data_dictionary.get("error")
585+
if error:
586+
self._stream_error_helper(
587+
"Service returned an error after streaming started", error
588+
)
575589
yield data
576590
else:
577591
# Response isn't chunked, meaning we probably
578592
# encountered an error immediately
579593
yield self._result(response, json=decode)
594+
595+
def _stream_error_helper(self, message, explanation):
596+
"""Helper to handle errors after streaming started."""
597+
error_response = requests.Response()
598+
error_response.status_code = 500
599+
error_response.reason = "Internal Server Error"
600+
raise APIError(
601+
message,
602+
response=error_response,
603+
explanation=explanation,
604+
)

podman/tests/unit/test_imagesmanager.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,62 @@ def test_pull_policy(self, mock):
670670
image = self.client.images.pull("quay.io/fedora:latest", policy="missing")
671671
self.assertEqual(image.id, image_id)
672672

673+
@requests_mock.Mocker()
674+
def test_pull_no_compat_mode(self, mock):
675+
image_id = "sha256:326dd9d7add24646a325e8eaa82125294027db2332e49c5828d96312c5d773ab"
676+
mock.post(
677+
tests.LIBPOD_URL + "/images/pull?reference=quay.io%2ffedora%3Alatest&quiet=True",
678+
json={
679+
"error": "",
680+
"id": image_id,
681+
"images": [image_id],
682+
"stream": "",
683+
},
684+
)
685+
mock.get(
686+
tests.LIBPOD_URL + "/images"
687+
"/sha256%3A326dd9d7add24646a325e8eaa82125294027db2332e49c5828d96312c5d773ab/json",
688+
json=FIRST_IMAGE,
689+
)
690+
691+
image = self.client.images.pull("quay.io/fedora", "latest", compatMode=False)
692+
self.assertEqual(image.id, image_id)
693+
694+
@requests_mock.Mocker()
695+
def test_pull_no_compat_mode_no_image(self, mock):
696+
non_existing_reference = "quay.io/f4ee35641334/f6fda4bb"
697+
error = {
698+
"cause": "access to the requested resource is not authorized",
699+
"message": "unable to copy from source",
700+
"response": 401,
701+
}
702+
703+
stream_cases = [
704+
dict(stream=False, quiet_postfix="&quiet=True"),
705+
dict(stream=True, quiet_postfix=""),
706+
]
707+
708+
for stream_case in stream_cases:
709+
with self.subTest(stream_case=stream_case):
710+
mock.post(
711+
tests.LIBPOD_URL
712+
+ f"/images/pull?reference={non_existing_reference}%3Alatest"
713+
+ f"{stream_case['quiet_postfix']}",
714+
status_code=error["response"],
715+
json=error,
716+
)
717+
718+
with self.assertRaises(APIError) as context:
719+
self.client.images.pull(
720+
non_existing_reference,
721+
"latest",
722+
compatMode=False,
723+
stream=stream_case["stream"],
724+
)
725+
self.assertEqual(context.exception.args[0], error["cause"])
726+
self.assertEqual(context.exception.explanation, error["message"])
727+
self.assertEqual(context.exception.response.status_code, error["response"])
728+
673729
@requests_mock.Mocker()
674730
def test_list_with_name_parameter(self, mock):
675731
"""Test that name parameter is correctly converted to a reference filter"""

0 commit comments

Comments
 (0)