feat: repackage 412 Harbor error into a user-friendly error (#926)

Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com>

backend/image_transfer/decoder.py

@@ -142,7 +142,6 @@ def load_zip_images_in_registry(
         docker_image,
         manifest_path_in_zip,
     ) in payload_descriptor.manifests_paths.items():
-
         if repository is not None:
             # The repository depends on the organization registry.
             _, tag = get_repo_and_tag(docker_image)

backend/image_transfer/encoder.py

@@ -9,6 +9,7 @@
 
 from dxf import DXF
 from dxf import DXFBase
+from requests import HTTPError
 from tqdm import tqdm
 
 from image_transfer.common import Authenticator
@@ -19,6 +20,7 @@
 from image_transfer.common import PayloadDescriptor
 from image_transfer.common import PayloadSide
 from image_transfer.common import progress_as_string
+from substrapp.docker_registry import RegistryPreconditionFailedException
 from substrapp.utils import safezip
 
 
@@ -89,7 +91,13 @@ def get_manifests_and_list_of_all_blobs(
     manifests = []
     blobs_to_pull = []
     for docker_image in docker_images:
-        manifest, blobs = get_manifest_and_list_of_blobs_to_pull(dxf_base, docker_image, platform)
+        try:
+            manifest, blobs = get_manifest_and_list_of_blobs_to_pull(dxf_base, docker_image, platform)
+        except HTTPError as e:
+            if e.response.status_code == 412:
+                raise RegistryPreconditionFailedException(
+                    f"{docker_image} is either not scanned yet or not passing the vulnerability checks."
+                ) from e
         manifests.append(manifest)
         blobs_to_pull += blobs
     return manifests, blobs_to_pull

backend/substrapp/compute_tasks/execute.py

@@ -83,6 +83,7 @@ def execute_compute_task(ctx: Context) -> None:
     if _is_pod_creation_needed(compute_pod.label_selector, client=k8s_client):
         # save entrypoint to DB
         entrypoint = get_entrypoint(container_image_tag)
+
         ImageEntrypoint.objects.get_or_create(
             archive_checksum=ctx.function.archive_address.checksum, entrypoint_json=entrypoint
         )

backend/substrapp/docker_registry.py

@@ -23,6 +23,10 @@
 USER_IMAGE_REPOSITORY = settings.USER_IMAGE_REPOSITORY
 
 
+class RegistryPreconditionFailedException(requests.exceptions.HTTPError):
+    pass
+
+
 class ImageNotFoundError(Exception):
     pass
 
@@ -73,6 +77,14 @@ def get_request_docker_api(
     )
 
     if response.status_code != requests.status_codes.codes.ok:
+        if response.status_code == 412:
+            raise RegistryPreconditionFailedException(
+                f"The image requested at path {path} did not pass the "
+                "security checks; please contact an Harbor administrator "
+                "to ensure that the image was scanned, "
+                "and get more information about the CVE.",
+                response=response,
+            )
         raise ImageNotFoundError(f"Error when querying docker-registry, status code: {response.status_code}")
 
     return response.json()

backend/substrapp/tasks/tasks_save_image.py

@@ -10,12 +10,15 @@
 from django.core.files import File
 from django.urls import reverse
 
+import image_transfer
 import orchestrator
 from api.models import Function as ApiFunction
 from backend.celery import app
-from image_transfer import make_payload
+from builder.exceptions import BuildRetryError
 from substrapp.compute_tasks import utils
+from substrapp.compute_tasks.errors import CeleryNoRetryError
 from substrapp.docker_registry import USER_IMAGE_REPOSITORY
+from substrapp.docker_registry import RegistryPreconditionFailedException
 from substrapp.models import FailedAssetKind
 from substrapp.models import FunctionImage
 from substrapp.orchestrator import get_orchestrator_client
@@ -61,18 +64,8 @@ def on_success(self, retval: tuple[dict, str], task_id: str, args: tuple, kwargs
             )
 
 
-@app.task(
-    bind=True,
-    acks_late=True,
-    reject_on_worker_lost=True,
-    ignore_result=False,
-    base=SaveImageTask,
-)
-# Ack late and reject on worker lost allows use to
-# see http://docs.celeryproject.org/en/latest/userguide/configuration.html#task-reject-on-worker-lost
-# and https://github.com/celery/celery/issues/5106
-def save_image_task(task: SaveImageTask, function_serialized: str, channel_name: str) -> tuple[dict, str]:
-    logger.info("Starting save_image_task")
+def save_image(function_serialized: str, channel_name: str) -> dict:
+    logger.info("Starting save_image")
     logger.info(f"Parameters: function_serialized {function_serialized}, " f"channel_name {channel_name}")
     # create serialized image
     function = orchestrator.Function.model_validate_json(function_serialized)
@@ -84,12 +77,20 @@ def save_image_task(task: SaveImageTask, function_serialized: str, channel_name:
 
     with TemporaryDirectory(dir=SUBTUPLE_TMP_DIR) as tmp_dir:
         storage_path = pathlib.Path(tmp_dir) / f"{container_image_tag}.zip"
-        make_payload(
-            zip_file=storage_path,
-            docker_images_to_transfer=[f"{USER_IMAGE_REPOSITORY}:{container_image_tag}"],
-            registry=REGISTRY,
-            secure=REGISTRY_SCHEME == "https",
-        )
+        try:
+            image_transfer.make_payload(
+                zip_file=storage_path,
+                docker_images_to_transfer=[f"{USER_IMAGE_REPOSITORY}:{container_image_tag}"],
+                registry=REGISTRY,
+                secure=REGISTRY_SCHEME == "https",
+            )
+        except RegistryPreconditionFailedException as e:
+            raise BuildRetryError(
+                f"The image associated with the function {function.key} was built successfully "
+                f"but did not pass the security checks; "
+                "please contact an Harbor administrator to ensure that the image was scanned, "
+                "and get more information about the CVE."
+            ) from e
 
         logger.info("Start saving the serialized image")
         # save it
@@ -115,4 +116,24 @@ def save_image_task(task: SaveImageTask, function_serialized: str, channel_name:
             },
         }
 
-        return orc_update_function_param, channel_name
+        return orc_update_function_param
+
+
+@app.task(
+    bind=True,
+    acks_late=True,
+    reject_on_worker_lost=True,
+    ignore_result=False,
+    base=SaveImageTask,
+)
+# Ack late and reject on worker lost allows use to
+# see http://docs.celeryproject.org/en/latest/userguide/configuration.html#task-reject-on-worker-lost
+# and https://github.com/celery/celery/issues/5106
+def save_image_task(task: SaveImageTask, function_serialized: str, channel_name: str) -> tuple[dict, str]:
+    try:
+        orc_update_function_param = save_image(function_serialized, channel_name)
+    except BuildRetryError:
+        raise
+    except Exception as e:
+        raise CeleryNoRetryError from e
+    return orc_update_function_param, channel_name

backend/substrapp/tests/tasks/test_save_image.py

@@ -0,0 +1,29 @@
+import pytest
+
+import orchestrator.mock as orc_mock
+from builder.exceptions import BuildRetryError
+from substrapp.compute_tasks.errors import CeleryNoRetryError
+from substrapp.docker_registry import RegistryPreconditionFailedException
+from substrapp.tasks.tasks_save_image import save_image_task
+
+CHANNEL = "mychannel"
+
+
+def test_tasks_save_image_random_exception(celery_app, celery_worker, mocker):
+    mocker.patch("image_transfer.make_payload", side_effect=Exception("random error"))
+    function = orc_mock.FunctionFactory()
+    with pytest.raises(CeleryNoRetryError):
+        r = save_image_task.apply_async(
+            kwargs={"function_serialized": function.model_dump_json(), "channel_name": CHANNEL}
+        )
+        r.get()
+
+
+def test_tasks_save_image_412_exception(celery_app, celery_worker, mocker):
+    mocker.patch("image_transfer.make_payload", side_effect=RegistryPreconditionFailedException)
+    function = orc_mock.FunctionFactory()
+    with pytest.raises(BuildRetryError):
+        r = save_image_task.apply(
+            kwargs={"function_serialized": function.model_dump_json(), "channel_name": CHANNEL}, retries=1
+        )
+        r.get()

changes/926.added

@@ -0,0 +1 @@
+Repackage Harbor 412 errors to a user-friendly message.

changes/926.fixed

@@ -0,0 +1 @@
+Errors when saving the image to the registry are properly handled.