Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions compose2pod/parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def _validate_string_list(name: str, key: str, value: Any) -> None: # noqa: ANN
# here, or we would refuse a file it runs.
IGNORED_SERVICE_KEYS: dict[str, Callable[[str, str, Any], None]] = {
"ports": values.validate_ports,
"expose": values.validate_expose,
"restart": values.validate_string,
"stdin_open": _validate_bool,
"tty": _validate_bool,
Expand All @@ -58,7 +59,24 @@ def _validate_string_list(name: str, key: str, value: Any) -> None: # noqa: ANN
"honouring it would move the container out of the pod's shared network namespace, "
"where the localhost ports and hostnames its dependents use stop resolving"
),
"external_links": (
"it names a container this script does not create, and every name compose2pod writes "
"into the pod's hosts file resolves to 127.0.0.1 -- give the address explicitly in "
"'extra_hosts' instead"
),
}
# Refused because compose2pod does not read the key yet, which ADR-0006 calls a tracked
# limitation rather than a design position. Kept apart from the table above so the two
# never share a sentence: this one is expected to shrink.
_UNIMPLEMENTED_REFUSALS = {
"links": (
"docker reads it as a dependency on the linked service plus a hostname alias "
"(measured, v5.1.2), and compose2pod takes neither from this key -- declare the "
"dependency in 'depends_on' and the alias in 'networks.<network>.aliases'"
),
}
# The categories above are the documented distinction; the gate only needs the reason.
_REFUSAL_REASONS = _POD_MODEL_REFUSALS | _UNIMPLEMENTED_REFUSALS
# The only service keys Docker tolerates an explicit null on, where it means
# "not specified" (measured against `docker compose config`). Every other key
# with a bare `key:` is refused -- see `_reject_null_values`.
Expand Down Expand Up @@ -797,8 +815,8 @@ def _validate_service(name: str, svc: Any) -> list[str]: # noqa: ANN401 - Compo
if key in IGNORED_SERVICE_KEYS:
IGNORED_SERVICE_KEYS[key](name, key, svc[key])
warnings.append(f"service {name!r}: ignoring '{key}'")
elif key in _POD_MODEL_REFUSALS:
msg = f"service {name!r}: {key!r} is not supported: {_POD_MODEL_REFUSALS[key]}"
elif key in _REFUSAL_REASONS:
msg = f"service {name!r}: {key!r} is not supported: {_REFUSAL_REASONS[key]}"
raise UnsupportedComposeError(msg)
elif key not in SUPPORTED_SERVICE_KEYS:
msg = f"service {name!r}: unsupported key '{key}'"
Expand Down
17 changes: 17 additions & 0 deletions compose2pod/values.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,23 @@ def _validate_port_entry(name: str, key: str, entry: Any) -> None: # noqa: ANN4
raise UnsupportedComposeError(msg)


def validate_expose(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped YAML/JSON
"""Check `value` is a list of strings or whole numbers, which is all Docker checks.

Unlike `ports`, the content is never validated: `docker compose config` v5.1.2 keeps
`expose: [banana]`, `[""]` and `[-1]` verbatim, and enumerating a port grammar here
would refuse files it runs. What it does refuse is a non-list, and an entry that is
null, a bool, a float, a map or a list.
"""
if not isinstance(value, list):
msg = f"service {name!r}: {key!r} must be a list"
raise UnsupportedComposeError(msg)
for entry in value:
if not isinstance(entry, str) and not _is_int(entry):
msg = f"service {name!r}: {key!r} entry {entry!r} must be a string or a whole number"
raise UnsupportedComposeError(msg)


def validate_ports(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped YAML/JSON
"""Check `value` is a list of port mappings. A bare string is refused, as Docker refuses it."""
if not isinstance(value, list):
Expand Down
34 changes: 25 additions & 9 deletions docs/adr/0003-the-shared-namespace-decides-key-classification.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
# The shared namespace decides which keys are refused, inert, or pod-level

Every service runs in one pod sharing `net`, `uts`, `ipc` and `cgroup`, and the shared network
namespace, with localhost discovery through per-container `--add-host`, is the reason the tool
exists. Keys that pull a container out of it (`network_mode`, `links`, `external_links`,
`expose`) are refused permanently; per-container namespace overrides (`ipc`, `uts`, `domainname`,
`cgroup`, `userns_mode`) are refused while the pod keeps its default `--share`. `dns*` and
`sysctls` are pod-level, unioned and conflict-checked across the closure onto
`podman pod create`, because a container that joined the pod owns neither namespace and podman
rejects the per-container flag. `stop_signal` and `stop_grace_period` are accepted but inert, in
`IGNORED_SERVICE_KEYS` with a warning: the script tears down with `pod rm -f` and never runs
`podman stop`, so the flags would set metadata nothing consults.
namespace, with localhost discovery through a bind-mounted hosts file, is the reason the tool
exists. Keys that pull a container out of it (`network_mode`, `external_links`) are refused
permanently: `network_mode` moves the container to another namespace, and `external_links` names
a container the generated script never creates, which the pod's hosts file -- where every name
compose2pod writes resolves to `127.0.0.1` -- has no address for, `extra_hosts` being the
supported way to name one. This clause once also swept up `links` and `expose`, which arrived in
the same sentence and belong in neither category
([#120](https://github.com/modern-python/compose2pod/issues/120), measured against
`docker compose config` v5.1.2). `links` normalises to a `depends_on` edge plus a hostname alias
-- docker refuses `links: [ghost]` exactly as it refuses a ghost `depends_on` -- so it neither
escapes the namespace nor is satisfied by it, and ignoring it would drop a dependency the
`--target` closure is built from. It is refused as a tracked limitation under
[ADR-0006](0006-docker-rejection-parity.md), and both halves are mechanisms compose2pod already
has, so this one is expected to shrink. `expose` carries no edge, is never published, and is
validated by docker no further than its list shape (it keeps `expose: [banana]`), which makes it
inert exactly as `ports` is: it sits in `IGNORED_SERVICE_KEYS` with a warning, not refused.
Per-container namespace overrides (`ipc`, `uts`, `domainname`, `cgroup`, `userns_mode`) are
refused while the pod keeps its default `--share`. `dns*` and `sysctls` are pod-level, unioned
and conflict-checked across the closure onto `podman pod create`, because a container that joined
the pod owns neither namespace and podman rejects the per-container flag. `stop_signal` and
`stop_grace_period` are accepted but inert for a different reason, in that same table with a
warning: the script tears down with `pod rm -f` and never runs `podman stop`, so the flags would
set metadata nothing consults. Inert is not unchecked -- every key in the table keeps docker's own
shape rule at the gate, since a document carrying a malformed one is a document docker will not
run.
5 changes: 5 additions & 0 deletions tests/conformance/corpus/service_external_links.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
services:
app:
image: nginx
external_links:
- other:alias
7 changes: 7 additions & 0 deletions tests/conformance/corpus/service_links_alias.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
db:
image: nginx
app:
image: nginx
links:
- db:database
28 changes: 28 additions & 0 deletions tests/conformance/test_corpus.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,34 @@ def test_volumes_long_form_image_type_is_no_longer_an_over_rejection(
assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept"


def test_service_links_is_a_catalogued_over_rejection(
assert_rule: Callable[[dict[str, Any]], str],
) -> None:
"""Docker accepts `links: [db:database]`; compose2pod does not read the key yet.

Asserted rather than left to the generic corpus run because `over-reject` is an allowed
verdict either way. What it pins is the measurement that reclassified this key in issue
120: docker normalises it to a `depends_on` edge plus an alias, so ignoring it with a
warning would drop a dependency the closure is built from. The day compose2pod reads
both halves, this flips to `both-accept` and the assertion says so.
"""
path = Path(__file__).parent / "corpus" / "service_links_alias.yaml"
assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject"


def test_service_external_links_is_a_catalogued_over_rejection(
assert_rule: Callable[[dict[str, Any]], str],
) -> None:
"""Docker accepts `external_links`; the pod's hosts file has no address to give it.

Same reason for asserting it as the row above. Unlike `links` this one is not expected
to flip: a container the script never creates is outside the pod model, and `extra_hosts`
is the supported way to name one.
"""
path = Path(__file__).parent / "corpus" / "service_external_links.yaml"
assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject"


def test_volume_windows_drive_letter_bind_is_a_catalogued_over_rejection(
assert_rule: Callable[[dict[str, Any]], str],
) -> None:
Expand Down
10 changes: 7 additions & 3 deletions tests/integration/refusals.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@
in four of them. Only a `LIMITATIONS` row leaves both empty, and only where the limit really
is the short form's rather than podman's.

Four claims, four experiments. `network_mode` alone has no row: it is refused under
ADR-0003, not rule two, and podman honours it (#115). The gate that every rule-two site
has a row is issue #109 phase 3, and that is the exemption it has to know about.
Four claims, four experiments. The keys refused under ADR-0003 have no rows, because
their reason is the pod model rather than rule two: podman honours `network_mode` for a
container that joined a pod (#115), and `external_links` names a container the generated
script never creates, which is a fact about the script. `links` has none either, for a
third reason -- it is a form compose2pod does not read yet (#120). The gate that every
rule-two site has a row is issue #109 phase 3, and those are the exemptions it has to
know about.

A `subpath` row measures the floor, not podman as such: podman gained the option above
the supported minimum (ADR-0006), so the row goes red on a runner newer than the floor,
Expand Down
35 changes: 35 additions & 0 deletions tests/test_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,41 @@ def test_network_mode_is_refused_for_leaving_the_pod_rather_than_as_an_unknown_k
with pytest.raises(UnsupportedComposeError, match=r"out of the pod's shared network namespace"):
validate({"services": {"app": {"image": "x", "network_mode": mode}}})

def test_links_is_refused_as_a_key_compose2pod_does_not_read_yet(self) -> None:
# `docker compose config` v5.1.2 normalises links into a depends_on edge plus an
# alias, so it is neither inert nor a namespace escape -- ADR-0003 swept it up with
# network_mode, and the measurement in issue 120 says otherwise.
for entry in (["db"], ["db:database"]):
with pytest.raises(UnsupportedComposeError, match=r"'links' is not supported: docker reads it as"):
validate({"services": {"db": {"image": "x"}, "app": {"image": "x", "links": entry}}})

def test_links_refusal_names_the_two_keys_that_replace_it(self) -> None:
with pytest.raises(UnsupportedComposeError, match=r"'depends_on'.*aliases") as refusal:
validate({"services": {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:database"]}}})

assert "podman" not in str(refusal.value)

def test_external_links_is_refused_for_naming_a_container_the_script_never_creates(self) -> None:
with pytest.raises(UnsupportedComposeError, match=r"'external_links' is not supported:.*extra_hosts"):
validate({"services": {"app": {"image": "x", "external_links": ["other:alias"]}}})

def test_expose_is_ignored_with_a_warning_rather_than_refused(self) -> None:
# Inert in a shared namespace: no dependency edge, never published, and docker
# validates nothing beyond the list shape (it keeps `banana`, measured v5.1.2).
warnings = validate({"services": {"app": {"image": "x", "expose": [8080, "9000/udp", "banana", ""]}}})

assert any("expose" in warning for warning in warnings)

def test_expose_shape_is_checked_even_though_it_is_ignored(self) -> None:
for bad in ("8080", 8080, {"a": 1}, [None], [True], [1.5], [{"target": 80}], [[80]]):
with pytest.raises(UnsupportedComposeError, match=r"'expose'"):
validate({"services": {"app": {"image": "x", "expose": bad}}})

def test_expose_accepts_the_empty_list(self) -> None:
# Warned like any ignored key, as `ports: []` is -- docker takes it, so the shape
# check must not treat an empty list as a missing one.
assert validate({"services": {"app": {"image": "x", "expose": []}}}) == ["service 'app': ignoring 'expose'"]

def test_unsupported_healthcheck_key_raises(self) -> None:
compose = {"services": {"app": {"image": "x", "healthcheck": {"test": "true", "start_interval": "1s"}}}}
with pytest.raises(UnsupportedComposeError, match="start_interval"):
Expand Down
Loading