Merge pull request #36 from civichero/fix/normalize-image-ref-digest-tag

fix(image-ref): preserve digest and tag refs in PYTHON_EXECUTOR_DOCKER_IMAGE

Author: Danelegend

code-interpreter/app/image_ref.py

@@ -0,0 +1,28 @@
+"""Helpers for normalizing Docker image references."""
+
+from __future__ import annotations
+
+
+def normalize_image_ref(ref: str) -> str:
+    """Return ``ref`` with an explicit ``:latest`` tag if it has neither tag nor digest.
+
+    Docker image references follow the grammar
+    ``[registry[:port]/]repo[:tag|@digest]``. A bare repository
+    (``repo``, ``owner/repo``, ``registry.io/owner/repo``) needs an explicit
+    tag for some operations such as ``docker image inspect``. References
+    that already carry a tag (``repo:v1``) or a digest
+    (``repo@sha256:…``) must be returned unchanged — appending ``:latest``
+    to either produces an invalid reference.
+
+    Registry ports require care: in ``registry.io:443/owner/repo``, the
+    ``:`` is a port separator, not a tag separator. The rule we apply is
+    that ``:`` is only a tag separator when it appears after the rightmost
+    ``/``.
+    """
+    if "@" in ref:
+        return ref
+    last_slash = ref.rfind("/")
+    last_colon = ref.rfind(":")
+    if last_colon > last_slash:
+        return ref
+    return f"{ref}:latest"

code-interpreter/app/main.py

@@ -13,6 +13,7 @@
 
 from app.api.routes import router as api_router
 from app.app_configs import EXECUTOR_BACKEND, HOST, PORT, PYTHON_EXECUTOR_DOCKER_IMAGE
+from app.image_ref import normalize_image_ref
 from app.models.schemas import HealthResponse
 from app.services.executor_factory import get_executor
 
@@ -41,7 +42,7 @@ def _ensure_docker_image_available() -> None:
         logger.warning("Docker binary not found, skipping image check")
         return
 
-    image_with_tag = f"{PYTHON_EXECUTOR_DOCKER_IMAGE}:latest"
+    image_with_tag = normalize_image_ref(PYTHON_EXECUTOR_DOCKER_IMAGE)
 
     # Check if image exists locally
     logger.info(f"Checking for Docker image: {image_with_tag}")

code-interpreter/app/services/executor_docker.py

@@ -20,6 +20,7 @@
     PYTHON_EXECUTOR_DOCKER_IMAGE,
     PYTHON_EXECUTOR_DOCKER_RUN_ARGS,
 )
+from app.image_ref import normalize_image_ref
 from app.services.executor_base import (
     SESSION_APP_LABEL,
     SESSION_COMPONENT_LABEL,
@@ -85,7 +86,7 @@ def check_health(self) -> HealthCheck:
             )
 
         # Check executor image is available locally
-        image_with_tag = f"{self.image}:latest"
+        image_with_tag = normalize_image_ref(self.image)
         try:
             img_result = subprocess.run(
                 [self.docker_binary, "image", "inspect", image_with_tag],

code-interpreter/tests/integration_tests/test_image_ref.py

@@ -0,0 +1,74 @@
+"""Tests for ``app.image_ref.normalize_image_ref``.
+
+These exercise every shape of Docker image reference we expect operators
+to set in ``PYTHON_EXECUTOR_DOCKER_IMAGE``:
+
+* bare repository (legacy default, must get ``:latest`` appended);
+* tagged reference (must be returned unchanged);
+* digest reference (must be returned unchanged — appending ``:latest``
+  produces an invalid reference);
+* registry-with-port variants, where a ``:`` before the last ``/`` is a
+  port separator and must NOT be mistaken for a tag.
+"""
+
+from __future__ import annotations
+
+from app.image_ref import normalize_image_ref
+
+
+def test_bare_repo_gets_latest() -> None:
+    assert normalize_image_ref("python-executor-sci") == "python-executor-sci:latest"
+
+
+def test_namespaced_bare_repo_gets_latest() -> None:
+    assert (
+        normalize_image_ref("onyxdotapp/python-executor-sci")
+        == "onyxdotapp/python-executor-sci:latest"
+    )
+
+
+def test_registry_bare_repo_gets_latest() -> None:
+    assert normalize_image_ref("ghcr.io/owner/repo") == "ghcr.io/owner/repo:latest"
+
+
+def test_tagged_reference_unchanged() -> None:
+    assert normalize_image_ref("python-executor-sci:0.4.0") == "python-executor-sci:0.4.0"
+    assert (
+        normalize_image_ref("onyxdotapp/python-executor-sci:0.4.0")
+        == "onyxdotapp/python-executor-sci:0.4.0"
+    )
+    assert normalize_image_ref("ghcr.io/owner/repo:v1") == "ghcr.io/owner/repo:v1"
+
+
+def test_digest_reference_unchanged() -> None:
+    digest = (
+        "onyxdotapp/python-executor-sci"
+        "@sha256:462c2fb0ed8998b75418d7a3f9d7fb75f61ce4c4605a1468436d5af09b9971b8"
+    )
+    assert normalize_image_ref(digest) == digest
+
+
+def test_registry_with_port_bare_repo_gets_latest() -> None:
+    # A ``:`` BEFORE the last ``/`` is a port separator, not a tag.
+    assert (
+        normalize_image_ref("registry.example.com:5000/owner/repo")
+        == "registry.example.com:5000/owner/repo:latest"
+    )
+
+
+def test_registry_with_port_and_tag_unchanged() -> None:
+    ref = "registry.example.com:5000/owner/repo:v2"
+    assert normalize_image_ref(ref) == ref
+
+
+def test_registry_with_port_and_digest_unchanged() -> None:
+    ref = "registry.example.com:5000/owner/repo@sha256:abc"
+    assert normalize_image_ref(ref) == ref
+
+
+def test_idempotent_on_already_tagged() -> None:
+    # Running the function twice on its own output must be a no-op once the
+    # first application has produced a valid tagged reference.
+    once = normalize_image_ref("onyxdotapp/python-executor-sci")
+    twice = normalize_image_ref(once)
+    assert once == twice == "onyxdotapp/python-executor-sci:latest"