Skip to content

Commit 8e724e3

Browse files
authored
feat: classify links, external_links and expose by what docker says they mean (#131)
ADR-0003 refused all four of `network_mode`, `links`, `external_links` and `expose` as keys that pull a container out of the pod's shared namespace. Measured against `docker compose config` v5.1.2, two of them are not that: links: [db:database] -> depends_on: {db: {condition: service_started}} + the alias links: [ghost] -> rejected, "depends on undefined service" expose: [8080, banana, ""] -> kept verbatim, no edge, no validation `links` is a dependency edge, so ignoring it with a warning would drop a dependency the --target closure is built from and generate a script that starts a service without its database. It is refused as a tracked limitation under ADR-0006, with a message naming the two keys that already express both halves. `expose` carries no edge and is never published, which makes it inert exactly as `ports` is: it moves to IGNORED_SERVICE_KEYS with a warning and a shape validator, docker's own rule and no more, since docker keeps `banana`. This widens what compose2pod accepts; the generated probe matrix reads IGNORED_SERVICE_KEYS, so all fourteen hostile shapes are now checked against docker on every run. `external_links` stays a permanent pod-model refusal and gains the reason it was missing: it names a container the script never creates, and every name compose2pod writes into the pod's hosts file resolves to 127.0.0.1. Closes #120
1 parent 4731fab commit 8e724e3

8 files changed

Lines changed: 144 additions & 14 deletions

File tree

‎compose2pod/parsing.py‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ def _validate_string_list(name: str, key: str, value: Any) -> None: # noqa: ANN
4343
# here, or we would refuse a file it runs.
4444
IGNORED_SERVICE_KEYS: dict[str, Callable[[str, str, Any], None]] = {
4545
"ports": values.validate_ports,
46+
"expose": values.validate_expose,
4647
"restart": values.validate_string,
4748
"stdin_open": _validate_bool,
4849
"tty": _validate_bool,
@@ -58,7 +59,24 @@ def _validate_string_list(name: str, key: str, value: Any) -> None: # noqa: ANN
5859
"honouring it would move the container out of the pod's shared network namespace, "
5960
"where the localhost ports and hostnames its dependents use stop resolving"
6061
),
62+
"external_links": (
63+
"it names a container this script does not create, and every name compose2pod writes "
64+
"into the pod's hosts file resolves to 127.0.0.1 -- give the address explicitly in "
65+
"'extra_hosts' instead"
66+
),
67+
}
68+
# Refused because compose2pod does not read the key yet, which ADR-0006 calls a tracked
69+
# limitation rather than a design position. Kept apart from the table above so the two
70+
# never share a sentence: this one is expected to shrink.
71+
_UNIMPLEMENTED_REFUSALS = {
72+
"links": (
73+
"docker reads it as a dependency on the linked service plus a hostname alias "
74+
"(measured, v5.1.2), and compose2pod takes neither from this key -- declare the "
75+
"dependency in 'depends_on' and the alias in 'networks.<network>.aliases'"
76+
),
6177
}
78+
# The categories above are the documented distinction; the gate only needs the reason.
79+
_REFUSAL_REASONS = _POD_MODEL_REFUSALS | _UNIMPLEMENTED_REFUSALS
6280
# The only service keys Docker tolerates an explicit null on, where it means
6381
# "not specified" (measured against `docker compose config`). Every other key
6482
# with a bare `key:` is refused -- see `_reject_null_values`.
@@ -797,8 +815,8 @@ def _validate_service(name: str, svc: Any) -> list[str]: # noqa: ANN401 - Compo
797815
if key in IGNORED_SERVICE_KEYS:
798816
IGNORED_SERVICE_KEYS[key](name, key, svc[key])
799817
warnings.append(f"service {name!r}: ignoring '{key}'")
800-
elif key in _POD_MODEL_REFUSALS:
801-
msg = f"service {name!r}: {key!r} is not supported: {_POD_MODEL_REFUSALS[key]}"
818+
elif key in _REFUSAL_REASONS:
819+
msg = f"service {name!r}: {key!r} is not supported: {_REFUSAL_REASONS[key]}"
802820
raise UnsupportedComposeError(msg)
803821
elif key not in SUPPORTED_SERVICE_KEYS:
804822
msg = f"service {name!r}: unsupported key '{key}'"

‎compose2pod/values.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,23 @@ def _validate_port_entry(name: str, key: str, entry: Any) -> None: # noqa: ANN4
486486
raise UnsupportedComposeError(msg)
487487

488488

489+
def validate_expose(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped YAML/JSON
490+
"""Check `value` is a list of strings or whole numbers, which is all Docker checks.
491+
492+
Unlike `ports`, the content is never validated: `docker compose config` v5.1.2 keeps
493+
`expose: [banana]`, `[""]` and `[-1]` verbatim, and enumerating a port grammar here
494+
would refuse files it runs. What it does refuse is a non-list, and an entry that is
495+
null, a bool, a float, a map or a list.
496+
"""
497+
if not isinstance(value, list):
498+
msg = f"service {name!r}: {key!r} must be a list"
499+
raise UnsupportedComposeError(msg)
500+
for entry in value:
501+
if not isinstance(entry, str) and not _is_int(entry):
502+
msg = f"service {name!r}: {key!r} entry {entry!r} must be a string or a whole number"
503+
raise UnsupportedComposeError(msg)
504+
505+
489506
def validate_ports(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped YAML/JSON
490507
"""Check `value` is a list of port mappings. A bare string is refused, as Docker refuses it."""
491508
if not isinstance(value, list):
Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,28 @@
11
# The shared namespace decides which keys are refused, inert, or pod-level
22

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

‎tests/conformance/test_corpus.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,34 @@ def test_volumes_long_form_image_type_is_no_longer_an_over_rejection(
130130
assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept"
131131

132132

133+
def test_service_links_is_a_catalogued_over_rejection(
134+
assert_rule: Callable[[dict[str, Any]], str],
135+
) -> None:
136+
"""Docker accepts `links: [db:database]`; compose2pod does not read the key yet.
137+
138+
Asserted rather than left to the generic corpus run because `over-reject` is an allowed
139+
verdict either way. What it pins is the measurement that reclassified this key in issue
140+
120: docker normalises it to a `depends_on` edge plus an alias, so ignoring it with a
141+
warning would drop a dependency the closure is built from. The day compose2pod reads
142+
both halves, this flips to `both-accept` and the assertion says so.
143+
"""
144+
path = Path(__file__).parent / "corpus" / "service_links_alias.yaml"
145+
assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject"
146+
147+
148+
def test_service_external_links_is_a_catalogued_over_rejection(
149+
assert_rule: Callable[[dict[str, Any]], str],
150+
) -> None:
151+
"""Docker accepts `external_links`; the pod's hosts file has no address to give it.
152+
153+
Same reason for asserting it as the row above. Unlike `links` this one is not expected
154+
to flip: a container the script never creates is outside the pod model, and `extra_hosts`
155+
is the supported way to name one.
156+
"""
157+
path = Path(__file__).parent / "corpus" / "service_external_links.yaml"
158+
assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject"
159+
160+
133161
def test_volume_windows_drive_letter_bind_is_a_catalogued_over_rejection(
134162
assert_rule: Callable[[dict[str, Any]], str],
135163
) -> None:

‎tests/integration/refusals.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,13 @@
3636
in four of them. Only a `LIMITATIONS` row leaves both empty, and only where the limit really
3737
is the short form's rather than podman's.
3838
39-
Four claims, four experiments. `network_mode` alone has no row: it is refused under
40-
ADR-0003, not rule two, and podman honours it (#115). The gate that every rule-two site
41-
has a row is issue #109 phase 3, and that is the exemption it has to know about.
39+
Four claims, four experiments. The keys refused under ADR-0003 have no rows, because
40+
their reason is the pod model rather than rule two: podman honours `network_mode` for a
41+
container that joined a pod (#115), and `external_links` names a container the generated
42+
script never creates, which is a fact about the script. `links` has none either, for a
43+
third reason -- it is a form compose2pod does not read yet (#120). The gate that every
44+
rule-two site has a row is issue #109 phase 3, and those are the exemptions it has to
45+
know about.
4246
4347
A `subpath` row measures the floor, not podman as such: podman gained the option above
4448
the supported minimum (ADR-0006), so the row goes red on a runner newer than the floor,

‎tests/test_parsing.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,41 @@ def test_network_mode_is_refused_for_leaving_the_pod_rather_than_as_an_unknown_k
7272
with pytest.raises(UnsupportedComposeError, match=r"out of the pod's shared network namespace"):
7373
validate({"services": {"app": {"image": "x", "network_mode": mode}}})
7474

75+
def test_links_is_refused_as_a_key_compose2pod_does_not_read_yet(self) -> None:
76+
# `docker compose config` v5.1.2 normalises links into a depends_on edge plus an
77+
# alias, so it is neither inert nor a namespace escape -- ADR-0003 swept it up with
78+
# network_mode, and the measurement in issue 120 says otherwise.
79+
for entry in (["db"], ["db:database"]):
80+
with pytest.raises(UnsupportedComposeError, match=r"'links' is not supported: docker reads it as"):
81+
validate({"services": {"db": {"image": "x"}, "app": {"image": "x", "links": entry}}})
82+
83+
def test_links_refusal_names_the_two_keys_that_replace_it(self) -> None:
84+
with pytest.raises(UnsupportedComposeError, match=r"'depends_on'.*aliases") as refusal:
85+
validate({"services": {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:database"]}}})
86+
87+
assert "podman" not in str(refusal.value)
88+
89+
def test_external_links_is_refused_for_naming_a_container_the_script_never_creates(self) -> None:
90+
with pytest.raises(UnsupportedComposeError, match=r"'external_links' is not supported:.*extra_hosts"):
91+
validate({"services": {"app": {"image": "x", "external_links": ["other:alias"]}}})
92+
93+
def test_expose_is_ignored_with_a_warning_rather_than_refused(self) -> None:
94+
# Inert in a shared namespace: no dependency edge, never published, and docker
95+
# validates nothing beyond the list shape (it keeps `banana`, measured v5.1.2).
96+
warnings = validate({"services": {"app": {"image": "x", "expose": [8080, "9000/udp", "banana", ""]}}})
97+
98+
assert any("expose" in warning for warning in warnings)
99+
100+
def test_expose_shape_is_checked_even_though_it_is_ignored(self) -> None:
101+
for bad in ("8080", 8080, {"a": 1}, [None], [True], [1.5], [{"target": 80}], [[80]]):
102+
with pytest.raises(UnsupportedComposeError, match=r"'expose'"):
103+
validate({"services": {"app": {"image": "x", "expose": bad}}})
104+
105+
def test_expose_accepts_the_empty_list(self) -> None:
106+
# Warned like any ignored key, as `ports: []` is -- docker takes it, so the shape
107+
# check must not treat an empty list as a missing one.
108+
assert validate({"services": {"app": {"image": "x", "expose": []}}}) == ["service 'app': ignoring 'expose'"]
109+
75110
def test_unsupported_healthcheck_key_raises(self) -> None:
76111
compose = {"services": {"app": {"image": "x", "healthcheck": {"test": "true", "start_interval": "1s"}}}}
77112
with pytest.raises(UnsupportedComposeError, match="start_interval"):

0 commit comments

Comments
 (0)