fix: more reliably validate Podman API version (#2016)

* fix: parse version strings that include dashes

It's possible for some container engines to report their versions with a dash (e.g., "4.9.4-rhel"), which breaks packaging.version.Version's ability to parse the string. This commit introduces a version_from_string method which santizies the version string and returns an instance of Version.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* cleanup: pacify ruff

* cleanup: further pacify ruff

* fix: properly define _version_from_string method

Also, lift the method up and prefix with a "_" to better match the existing conventions

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* refactor: more robust podman ver check

Use the "podman --version" command instead of "podman version -f {{json .}}" for better reliability across distributions.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix: oci engine version check

Lower Docker API check to 1.41
Podman versions are not PEP440 compliant, remove distro specific suffixes before parsing.
Add tests with real-world outputs and some made up ones.

* fix: UX on OCIEngineTooOldError

* Add FlexibleVersion

per review comment

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: mayeut <mayeut@users.noreply.github.com>
This commit is contained in:
zachlewis
2024-10-01 01:10:58 -04:00
committed by GitHub
co-authored by pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> mayeut
parent 735e88deba
commit dfd01af9aa
5 changed files with 172 additions and 18 deletions
+3 -1
View File
@@ -61,4 +61,6 @@ class AlreadyBuiltWheelError(FatalError):
class OCIEngineTooOldError(FatalError): class OCIEngineTooOldError(FatalError):
return_code = 7 def __init__(self, message: str) -> None:
super().__init__(message)
self.return_code = 7
+35 -15
View File
@@ -8,6 +8,7 @@ import shlex
import shutil import shutil
import subprocess import subprocess
import sys import sys
import textwrap
import typing import typing
import uuid import uuid
from collections.abc import Mapping, Sequence from collections.abc import Mapping, Sequence
@@ -17,14 +18,13 @@ from pathlib import Path, PurePath, PurePosixPath
from types import TracebackType from types import TracebackType
from typing import IO, Dict, Literal from typing import IO, Dict, Literal
from packaging.version import InvalidVersion, Version
from ._compat.typing import Self, assert_never from ._compat.typing import Self, assert_never
from .errors import OCIEngineTooOldError from .errors import OCIEngineTooOldError
from .logger import log from .logger import log
from .typing import PathOrStr, PopenBytes from .typing import PathOrStr, PopenBytes
from .util import ( from .util import (
CIProvider, CIProvider,
FlexibleVersion,
call, call,
detect_ci_provider, detect_ci_provider,
parse_key_value_string, parse_key_value_string,
@@ -103,25 +103,45 @@ def _check_engine_version(engine: OCIContainerEngineConfig) -> None:
version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True) version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True)
version_info = json.loads(version_string.strip()) version_info = json.loads(version_string.strip())
if engine.name == "docker": if engine.name == "docker":
# --platform support was introduced in 1.32 as experimental client_api_version = FlexibleVersion(version_info["Client"]["ApiVersion"])
# docker cp, as used by cibuildwheel, has been fixed in v24 => API 1.43, https://github.com/moby/moby/issues/38995 server_api_version = FlexibleVersion(version_info["Server"]["ApiVersion"])
client_api_version = Version(version_info["Client"]["ApiVersion"]) # --platform support was introduced in 1.32 as experimental, 1.41 removed the experimental flag
engine_api_version = Version(version_info["Server"]["ApiVersion"]) version = min(client_api_version, server_api_version)
version_supported = min(client_api_version, engine_api_version) >= Version("1.43") minimum_version = FlexibleVersion("1.41")
minimum_version_str = "20.10.0" # docker version
error_msg = textwrap.dedent(
f"""
Build failed because {engine.name} is too old.
cibuildwheel requires {engine.name}>={minimum_version_str} running API version {minimum_version}.
The API version found by cibuildwheel is {version}.
"""
)
elif engine.name == "podman": elif engine.name == "podman":
client_api_version = Version(version_info["Client"]["APIVersion"]) # podman uses the same version string for "Version" & "ApiVersion"
client_version = FlexibleVersion(version_info["Client"]["Version"])
if "Server" in version_info: if "Server" in version_info:
engine_api_version = Version(version_info["Server"]["APIVersion"]) server_version = FlexibleVersion(version_info["Server"]["Version"])
else: else:
engine_api_version = client_api_version server_version = client_version
# --platform support was introduced in v3 # --platform support was introduced in v3
version_supported = min(client_api_version, engine_api_version) >= Version("3") version = min(client_version, server_version)
minimum_version = FlexibleVersion("3")
error_msg = textwrap.dedent(
f"""
Build failed because {engine.name} is too old.
cibuildwheel requires {engine.name}>={minimum_version}.
The version found by cibuildwheel is {version}.
"""
)
else: else:
assert_never(engine.name) assert_never(engine.name)
if not version_supported: if version < minimum_version:
raise OCIEngineTooOldError() from None raise OCIEngineTooOldError(error_msg) from None
except (subprocess.CalledProcessError, KeyError, InvalidVersion) as e: except (subprocess.CalledProcessError, KeyError, ValueError) as e:
raise OCIEngineTooOldError() from e msg = f"Build failed because {engine.name} is too old or is not working properly."
raise OCIEngineTooOldError(msg) from e
class OCIContainer: class OCIContainer:
+49 -1
View File
@@ -19,7 +19,7 @@ from collections import defaultdict
from collections.abc import Generator, Iterable, Mapping, MutableMapping, Sequence from collections.abc import Generator, Iterable, Mapping, MutableMapping, Sequence
from dataclasses import dataclass from dataclasses import dataclass
from enum import Enum from enum import Enum
from functools import lru_cache from functools import lru_cache, total_ordering
from pathlib import Path, PurePath from pathlib import Path, PurePath
from tempfile import TemporaryDirectory from tempfile import TemporaryDirectory
from time import sleep from time import sleep
@@ -899,3 +899,51 @@ def combine_constraints(
env["UV_CONSTRAINT"] = env["PIP_CONSTRAINT"] = " ".join( env["UV_CONSTRAINT"] = env["PIP_CONSTRAINT"] = " ".join(
c for c in [our_constraints, user_constraints] if c c for c in [our_constraints, user_constraints] if c
) )
@total_ordering
class FlexibleVersion:
version_str: str
version_parts: tuple[int, ...]
suffix: str
def __init__(self, version_str: str) -> None:
self.version_str = version_str
# Split into numeric parts and the optional suffix
match = re.match(r"^[v]?(\d+(\.\d+)*)(.*)$", version_str)
if not match:
msg = f"Invalid version string: {version_str}"
raise ValueError(msg)
version_part, _, suffix = match.groups()
# Convert numeric version part into a tuple of integers
self.version_parts = tuple(map(int, version_part.split(".")))
self.suffix = suffix.strip() if suffix else ""
# Normalize by removing trailing zeros
self.version_parts = self._remove_trailing_zeros(self.version_parts)
def _remove_trailing_zeros(self, parts: tuple[int, ...]) -> tuple[int, ...]:
# Remove trailing zeros for accurate comparisons
# without this, "3.0" would be considered greater than "3"
while parts and parts[-1] == 0:
parts = parts[:-1]
return parts
def __eq__(self, other: object) -> bool:
if not isinstance(other, FlexibleVersion):
raise NotImplementedError()
return (self.version_parts, self.suffix) == (other.version_parts, other.suffix)
def __lt__(self, other: object) -> bool:
if not isinstance(other, FlexibleVersion):
raise NotImplementedError()
return (self.version_parts, self.suffix) < (other.version_parts, other.suffix)
def __repr__(self) -> str:
return f"FlexibleVersion('{self.version_str}')"
def __str__(self) -> str:
return self.version_str
+70 -1
View File
@@ -8,13 +8,21 @@ import shutil
import subprocess import subprocess
import sys import sys
import textwrap import textwrap
from contextlib import nullcontext
from pathlib import Path, PurePath, PurePosixPath from pathlib import Path, PurePath, PurePosixPath
import pytest import pytest
import tomli_w import tomli_w
import cibuildwheel.oci_container
from cibuildwheel.environment import EnvironmentAssignmentBash from cibuildwheel.environment import EnvironmentAssignmentBash
from cibuildwheel.oci_container import OCIContainer, OCIContainerEngineConfig, OCIPlatform from cibuildwheel.errors import OCIEngineTooOldError
from cibuildwheel.oci_container import (
OCIContainer,
OCIContainerEngineConfig,
OCIPlatform,
_check_engine_version,
)
from cibuildwheel.util import CIProvider, detect_ci_provider from cibuildwheel.util import CIProvider, detect_ci_provider
# Test utilities # Test utilities
@@ -569,3 +577,64 @@ def test_multiarch_image(container_engine, platform):
OCIPlatform.S390X: "s390x", OCIPlatform.S390X: "s390x",
} }
assert output_map[platform] == output.strip() assert output_map[platform] == output.strip()
@pytest.mark.parametrize(
("engine_name", "version", "context"),
[
(
"docker",
None, # 17.12.1-ce does supports "docker version --format '{{json . }}'" so a version before that
pytest.raises(OCIEngineTooOldError),
),
(
"docker",
'{"Client":{"Version":"19.03.15","ApiVersion": "1.40"},"Server":{"ApiVersion": "1.40"}}',
pytest.raises(OCIEngineTooOldError),
),
(
"docker",
'{"Client":{"Version":"20.10.0","ApiVersion":"1.41"},"Server":{"ApiVersion":"1.41"}}',
nullcontext(),
),
(
"docker",
'{"Client":{"Version":"24.0.0","ApiVersion":"1.43"},"Server":{"ApiVersion":"1.43"}}',
nullcontext(),
),
(
"docker",
'{"Client":{"ApiVersion":"1.43"},"Server":{"ApiVersion":"1.30"}}',
pytest.raises(OCIEngineTooOldError),
),
(
"docker",
'{"Client":{"ApiVersion":"1.30"},"Server":{"ApiVersion":"1.43"}}',
pytest.raises(OCIEngineTooOldError),
),
("podman", '{"Client":{"Version":"5.2.0"},"Server":{"Version":"5.1.2"}}', nullcontext()),
("podman", '{"Client":{"Version":"4.9.4-rhel"}}', nullcontext()),
(
"podman",
'{"Client":{"Version":"5.2.0"},"Server":{"Version":"2.1.2"}}',
pytest.raises(OCIEngineTooOldError),
),
(
"podman",
'{"Client":{"Version":"2.2.0"},"Server":{"Version":"5.1.2"}}',
pytest.raises(OCIEngineTooOldError),
),
("podman", '{"Client":{"Version":"3.0~rc1-rhel"}}', nullcontext()),
("podman", '{"Client":{"Version":"2.1.0~rc1"}}', pytest.raises(OCIEngineTooOldError)),
],
)
def test_engine_version(engine_name, version, context, monkeypatch):
def mockcall(*args, **kwargs):
if version is None:
raise subprocess.CalledProcessError(1, " ".join(str(arg) for arg in args))
return version
monkeypatch.setattr(cibuildwheel.oci_container, "call", mockcall)
engine = OCIContainerEngineConfig.from_config_string(engine_name)
with context:
_check_engine_version(engine)
+15
View File
@@ -6,6 +6,7 @@ from pathlib import PurePath
import pytest import pytest
from cibuildwheel.util import ( from cibuildwheel.util import (
FlexibleVersion,
find_compatible_wheel, find_compatible_wheel,
fix_ansi_codes_for_github_actions, fix_ansi_codes_for_github_actions,
format_safe, format_safe,
@@ -206,3 +207,17 @@ def test_parse_key_value_string():
"name": ["docker"], "name": ["docker"],
"create_args": [], "create_args": [],
} }
def test_flexible_version_comparisons():
assert FlexibleVersion("2.0") == FlexibleVersion("2")
assert FlexibleVersion("2.0") < FlexibleVersion("2.1")
assert FlexibleVersion("2.1") > FlexibleVersion("2")
assert FlexibleVersion("1.9.9") < FlexibleVersion("2.0")
assert FlexibleVersion("1.10") > FlexibleVersion("1.9.9")
assert FlexibleVersion("3.0.1") > FlexibleVersion("3.0")
assert FlexibleVersion("3.0") < FlexibleVersion("3.0.1")
# Suffix should not affect comparisons
assert FlexibleVersion("1.0.1-rhel") > FlexibleVersion("1.0")
assert FlexibleVersion("1.0.1-rhel") < FlexibleVersion("1.1")
assert FlexibleVersion("1.0.1") == FlexibleVersion("v1.0.1")