Skip to content

Commit

Permalink
fix: more reliably validate Podman API version (#2016)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
3 people authored Oct 1, 2024
1 parent 735e88d commit dfd01af
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 18 deletions.
4 changes: 3 additions & 1 deletion cibuildwheel/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,6 @@ def __init__(self, wheel_name: str) -> None:


class OCIEngineTooOldError(FatalError):
return_code = 7
def __init__(self, message: str) -> None:
super().__init__(message)
self.return_code = 7
50 changes: 35 additions & 15 deletions cibuildwheel/oci_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import shutil
import subprocess
import sys
import textwrap
import typing
import uuid
from collections.abc import Mapping, Sequence
Expand All @@ -17,14 +18,13 @@
from types import TracebackType
from typing import IO, Dict, Literal

from packaging.version import InvalidVersion, Version

from ._compat.typing import Self, assert_never
from .errors import OCIEngineTooOldError
from .logger import log
from .typing import PathOrStr, PopenBytes
from .util import (
CIProvider,
FlexibleVersion,
call,
detect_ci_provider,
parse_key_value_string,
Expand Down Expand Up @@ -103,25 +103,45 @@ def _check_engine_version(engine: OCIContainerEngineConfig) -> None:
version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True)
version_info = json.loads(version_string.strip())
if engine.name == "docker":
# --platform support was introduced in 1.32 as experimental
# docker cp, as used by cibuildwheel, has been fixed in v24 => API 1.43, https://github.com/moby/moby/issues/38995
client_api_version = Version(version_info["Client"]["ApiVersion"])
engine_api_version = Version(version_info["Server"]["ApiVersion"])
version_supported = min(client_api_version, engine_api_version) >= Version("1.43")
client_api_version = FlexibleVersion(version_info["Client"]["ApiVersion"])
server_api_version = FlexibleVersion(version_info["Server"]["ApiVersion"])
# --platform support was introduced in 1.32 as experimental, 1.41 removed the experimental flag
version = min(client_api_version, server_api_version)
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":
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:
engine_api_version = Version(version_info["Server"]["APIVersion"])
server_version = FlexibleVersion(version_info["Server"]["Version"])
else:
engine_api_version = client_api_version
server_version = client_version
# --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:
assert_never(engine.name)
if not version_supported:
raise OCIEngineTooOldError() from None
except (subprocess.CalledProcessError, KeyError, InvalidVersion) as e:
raise OCIEngineTooOldError() from e
if version < minimum_version:
raise OCIEngineTooOldError(error_msg) from None
except (subprocess.CalledProcessError, KeyError, ValueError) as e:
msg = f"Build failed because {engine.name} is too old or is not working properly."
raise OCIEngineTooOldError(msg) from e


class OCIContainer:
Expand Down
50 changes: 49 additions & 1 deletion cibuildwheel/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from collections.abc import Generator, Iterable, Mapping, MutableMapping, Sequence
from dataclasses import dataclass
from enum import Enum
from functools import lru_cache
from functools import lru_cache, total_ordering
from pathlib import Path, PurePath
from tempfile import TemporaryDirectory
from time import sleep
Expand Down Expand Up @@ -899,3 +899,51 @@ def combine_constraints(
env["UV_CONSTRAINT"] = env["PIP_CONSTRAINT"] = " ".join(
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
71 changes: 70 additions & 1 deletion unit_test/oci_container_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,21 @@
import subprocess
import sys
import textwrap
from contextlib import nullcontext
from pathlib import Path, PurePath, PurePosixPath

import pytest
import tomli_w

import cibuildwheel.oci_container
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

# Test utilities
Expand Down Expand Up @@ -569,3 +577,64 @@ def test_multiarch_image(container_engine, platform):
OCIPlatform.S390X: "s390x",
}
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 changes: 15 additions & 0 deletions unit_test/utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pytest

from cibuildwheel.util import (
FlexibleVersion,
find_compatible_wheel,
fix_ansi_codes_for_github_actions,
format_safe,
Expand Down Expand Up @@ -206,3 +207,17 @@ def test_parse_key_value_string():
"name": ["docker"],
"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")

0 comments on commit dfd01af

Please sign in to comment.