diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 71387f3..1fb8a52 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -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, @@ -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..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`. @@ -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}'" diff --git a/compose2pod/values.py b/compose2pod/values.py index 9d5c3d5..e9593d7 100644 --- a/compose2pod/values.py +++ b/compose2pod/values.py @@ -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): diff --git a/docs/adr/0003-the-shared-namespace-decides-key-classification.md b/docs/adr/0003-the-shared-namespace-decides-key-classification.md index 4e4f9f9..e887e8a 100644 --- a/docs/adr/0003-the-shared-namespace-decides-key-classification.md +++ b/docs/adr/0003-the-shared-namespace-decides-key-classification.md @@ -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. diff --git a/tests/conformance/corpus/service_external_links.yaml b/tests/conformance/corpus/service_external_links.yaml new file mode 100644 index 0000000..58e4c7f --- /dev/null +++ b/tests/conformance/corpus/service_external_links.yaml @@ -0,0 +1,5 @@ +services: + app: + image: nginx + external_links: + - other:alias diff --git a/tests/conformance/corpus/service_links_alias.yaml b/tests/conformance/corpus/service_links_alias.yaml new file mode 100644 index 0000000..9b145b4 --- /dev/null +++ b/tests/conformance/corpus/service_links_alias.yaml @@ -0,0 +1,7 @@ +services: + db: + image: nginx + app: + image: nginx + links: + - db:database diff --git a/tests/conformance/test_corpus.py b/tests/conformance/test_corpus.py index c06d43a..9fbf77c 100644 --- a/tests/conformance/test_corpus.py +++ b/tests/conformance/test_corpus.py @@ -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: diff --git a/tests/integration/refusals.py b/tests/integration/refusals.py index 8a6192d..ec69e35 100644 --- a/tests/integration/refusals.py +++ b/tests/integration/refusals.py @@ -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, diff --git a/tests/test_parsing.py b/tests/test_parsing.py index b6d368a..6fe0381 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -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"):