Skip to content

Commit f0c0ac8

Browse files
committed
fix(image-ref): preserve digest and tag refs in PYTHON_EXECUTOR_DOCKER_IMAGE
`_ensure_docker_image_available` (app/main.py) and `DockerExecutor.check_health` (app/services/executor_docker.py) both unconditionally appended `:latest` to whatever value `PYTHON_EXECUTOR_DOCKER_IMAGE` held. When an operator sets that env var to a digest reference (e.g. `repo@sha256:abc...`) for reproducible deploys, the resulting `repo@sha256:abc...:latest` is invalid Docker syntax: image lookup fails, the FastAPI lifespan aborts, and the container crashloops. Refactor both call sites to use a new `normalize_image_ref` helper. The helper returns the reference unchanged when it already carries a tag (`:` after the last `/`) or a digest (`@`), and only appends `:latest` for bare repositories. Registry ports are handled correctly because a `:` before the last `/` is a port separator, not a tag. Coverage added in tests/integration_tests/test_image_ref.py: * bare repo / namespaced repo / registry/repo all gain :latest * tagged refs unchanged at every nesting level * digest refs unchanged at every nesting level * registry-with-port variants (port + bare, port + tag, port + digest) * idempotence Refs #35
1 parent bd084c5 commit f0c0ac8

4 files changed

Lines changed: 106 additions & 2 deletions

File tree

code-interpreter/app/image_ref.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
"""Helpers for normalizing Docker image references."""
2+
3+
from __future__ import annotations
4+
5+
6+
def normalize_image_ref(ref: str) -> str:
7+
"""Return ``ref`` with an explicit ``:latest`` tag if it has neither tag nor digest.
8+
9+
Docker image references follow the grammar
10+
``[registry[:port]/]repo[:tag|@digest]``. A bare repository
11+
(``repo``, ``owner/repo``, ``registry.io/owner/repo``) needs an explicit
12+
tag for some operations such as ``docker image inspect``. References
13+
that already carry a tag (``repo:v1``) or a digest
14+
(``repo@sha256:…``) must be returned unchanged — appending ``:latest``
15+
to either produces an invalid reference.
16+
17+
Registry ports require care: in ``registry.io:443/owner/repo``, the
18+
``:`` is a port separator, not a tag separator. The rule we apply is
19+
that ``:`` is only a tag separator when it appears after the rightmost
20+
``/``.
21+
"""
22+
if "@" in ref:
23+
return ref
24+
last_slash = ref.rfind("/")
25+
last_colon = ref.rfind(":")
26+
if last_colon > last_slash:
27+
return ref
28+
return f"{ref}:latest"

code-interpreter/app/main.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from app.api.routes import router as api_router
1515
from app.app_configs import EXECUTOR_BACKEND, HOST, PORT, PYTHON_EXECUTOR_DOCKER_IMAGE
16+
from app.image_ref import normalize_image_ref
1617
from app.models.schemas import HealthResponse
1718
from app.services.executor_factory import get_executor
1819

@@ -41,7 +42,7 @@ def _ensure_docker_image_available() -> None:
4142
logger.warning("Docker binary not found, skipping image check")
4243
return
4344

44-
image_with_tag = f"{PYTHON_EXECUTOR_DOCKER_IMAGE}:latest"
45+
image_with_tag = normalize_image_ref(PYTHON_EXECUTOR_DOCKER_IMAGE)
4546

4647
# Check if image exists locally
4748
logger.info(f"Checking for Docker image: {image_with_tag}")

code-interpreter/app/services/executor_docker.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
PYTHON_EXECUTOR_DOCKER_IMAGE,
2121
PYTHON_EXECUTOR_DOCKER_RUN_ARGS,
2222
)
23+
from app.image_ref import normalize_image_ref
2324
from app.services.executor_base import (
2425
SESSION_APP_LABEL,
2526
SESSION_COMPONENT_LABEL,
@@ -85,7 +86,7 @@ def check_health(self) -> HealthCheck:
8586
)
8687

8788
# Check executor image is available locally
88-
image_with_tag = f"{self.image}:latest"
89+
image_with_tag = normalize_image_ref(self.image)
8990
try:
9091
img_result = subprocess.run(
9192
[self.docker_binary, "image", "inspect", image_with_tag],
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
"""Tests for ``app.image_ref.normalize_image_ref``.
2+
3+
These exercise every shape of Docker image reference we expect operators
4+
to set in ``PYTHON_EXECUTOR_DOCKER_IMAGE``:
5+
6+
* bare repository (legacy default, must get ``:latest`` appended);
7+
* tagged reference (must be returned unchanged);
8+
* digest reference (must be returned unchanged — appending ``:latest``
9+
produces an invalid reference);
10+
* registry-with-port variants, where a ``:`` before the last ``/`` is a
11+
port separator and must NOT be mistaken for a tag.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
from app.image_ref import normalize_image_ref
17+
18+
19+
def test_bare_repo_gets_latest() -> None:
20+
assert normalize_image_ref("python-executor-sci") == "python-executor-sci:latest"
21+
22+
23+
def test_namespaced_bare_repo_gets_latest() -> None:
24+
assert (
25+
normalize_image_ref("onyxdotapp/python-executor-sci")
26+
== "onyxdotapp/python-executor-sci:latest"
27+
)
28+
29+
30+
def test_registry_bare_repo_gets_latest() -> None:
31+
assert normalize_image_ref("ghcr.io/owner/repo") == "ghcr.io/owner/repo:latest"
32+
33+
34+
def test_tagged_reference_unchanged() -> None:
35+
assert normalize_image_ref("python-executor-sci:0.4.0") == "python-executor-sci:0.4.0"
36+
assert (
37+
normalize_image_ref("onyxdotapp/python-executor-sci:0.4.0")
38+
== "onyxdotapp/python-executor-sci:0.4.0"
39+
)
40+
assert normalize_image_ref("ghcr.io/owner/repo:v1") == "ghcr.io/owner/repo:v1"
41+
42+
43+
def test_digest_reference_unchanged() -> None:
44+
digest = (
45+
"onyxdotapp/python-executor-sci"
46+
"@sha256:462c2fb0ed8998b75418d7a3f9d7fb75f61ce4c4605a1468436d5af09b9971b8"
47+
)
48+
assert normalize_image_ref(digest) == digest
49+
50+
51+
def test_registry_with_port_bare_repo_gets_latest() -> None:
52+
# A ``:`` BEFORE the last ``/`` is a port separator, not a tag.
53+
assert (
54+
normalize_image_ref("registry.example.com:5000/owner/repo")
55+
== "registry.example.com:5000/owner/repo:latest"
56+
)
57+
58+
59+
def test_registry_with_port_and_tag_unchanged() -> None:
60+
ref = "registry.example.com:5000/owner/repo:v2"
61+
assert normalize_image_ref(ref) == ref
62+
63+
64+
def test_registry_with_port_and_digest_unchanged() -> None:
65+
ref = "registry.example.com:5000/owner/repo@sha256:abc"
66+
assert normalize_image_ref(ref) == ref
67+
68+
69+
def test_idempotent_on_already_tagged() -> None:
70+
# Running the function twice on its own output must be a no-op once the
71+
# first application has produced a valid tagged reference.
72+
once = normalize_image_ref("onyxdotapp/python-executor-sci")
73+
twice = normalize_image_ref(once)
74+
assert once == twice == "onyxdotapp/python-executor-sci:latest"

0 commit comments

Comments
 (0)