From c1ad54b095d851048884198c2cb54b27179eda23 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 20 Sep 2026 20:50:54 +0300 Subject: [PATCH] feat: read links as a dependency edge plus a hostname alias `docker compose config` v5.1.2 normalises `links: [db:database]` into a `depends_on` entry with `condition: service_started` and an alias, and both are mechanisms compose2pod already had. `graph._link_entries` splits an entry on a single colon and not otherwise, which is how docker reads it: `db:a:b` and `:db` name a service nothing defines, and `validate_graph` refuses them with docker's own verdict. `depends_on` merges the edges into the mapping the closure and the graph check are built from, leaving an explicit entry's condition alone. `_link_aliases` feeds `hostnames`, so the alias lands in the pod's hosts file at 127.0.0.1 like every other pod-internal name. `links` joins STRUCTURAL_KEYS, which puts it under the generated conformance probe matrix, and `service_links_alias.yaml` flips from over-reject to both-accept. Closes #132 --- CONTEXT.md | 18 +-- README.md | 3 +- compose2pod/emit.py | 6 +- compose2pod/graph.py | 77 ++++++++++- compose2pod/keys.py | 1 + compose2pod/parsing.py | 18 +-- ...ed-namespace-decides-key-classification.md | 14 +- .../corpus/service_links_empty_alias.yaml | 10 ++ .../corpus/service_links_multi_colon.yaml | 9 ++ .../corpus/service_links_self.yaml | 7 + tests/conformance/test_corpus.py | 28 ++-- tests/integration/refusals.py | 6 +- tests/test_emit.py | 57 ++++++++ tests/test_graph.py | 127 +++++++++++++++--- tests/test_keys.py | 1 + tests/test_parsing.py | 23 ++-- 16 files changed, 327 insertions(+), 78 deletions(-) create mode 100644 tests/conformance/corpus/service_links_empty_alias.yaml create mode 100644 tests/conformance/corpus/service_links_multi_colon.yaml create mode 100644 tests/conformance/corpus/service_links_self.yaml diff --git a/CONTEXT.md b/CONTEXT.md index b39118c..6a1abe7 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -43,15 +43,15 @@ variable's value is a fact about the machine the script runs on, which is not th machine that read the compose file. **Closure**: -The target service and everything reachable from it through `depends_on` -(`graph.startup_order`). It is the unit of scope for almost everything: only the -closure joins the pod, so only the closure is emitted, only its hostnames are -resolvable, and pod-level options (`dns`, `sysctls`, `extra_hosts`) are unioned and -conflict-checked across it and nothing else. A service outside the closure never runs, -which is why `profiles` is inert. Validation is not scoped to it, though: the dependency -graph is checked document-wide (`graph.validate_graph`), because whether Docker rejects a -document cannot depend on which service the caller happened to target -([#87](https://github.com/modern-python/compose2pod/issues/87)). +The target service and everything reachable from it through `depends_on` or `links` +(`graph.startup_order`, over the one graph `graph.depends_on` returns for both keys). It is +the unit of scope for almost everything: only the closure joins the pod, so only the closure +is emitted, only its hostnames are resolvable, and pod-level options (`dns`, `sysctls`, +`extra_hosts`) are unioned and conflict-checked across it and nothing else. A service outside +the closure never runs, which is why `profiles` is inert. Validation is not scoped to it, +though: the dependency graph is checked document-wide (`graph.validate_graph`), because +whether Docker rejects a document cannot depend on which service the caller happened to +target ([#87](https://github.com/modern-python/compose2pod/issues/87)). **Rule one / rule two**: The two directions of the Docker-rejection parity rule diff --git a/README.md b/README.md index 549f1fa..886c349 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,8 @@ Within that boundary it covers most of what real compose files use: - **Services** — `image`/`build`, `command`/`entrypoint`, `environment` and `env_file` (string and long-form `{path, required, format}`), `volumes` (short-form and long-form `--mount`, including the `bind` and `tmpfs` - option maps), `tmpfs`, `healthcheck`, `depends_on` (all conditions), network + option maps), `tmpfs`, `healthcheck`, `depends_on` (all conditions), `links` + (read as a dependency plus a hostname alias, as Docker reads it), network `aliases`, `hostname`/`container_name`. - **Confinement & metadata** — `user`, `working_dir`, `read_only`, `init`, `privileged`, `cap_add`/`cap_drop`, `security_opt`, `devices`, `group_add`, diff --git a/compose2pod/emit.py b/compose2pod/emit.py index 9714b8b..e1e21c6 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -347,8 +347,8 @@ def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript: host_tokens = hosts_file_tokens(services, order, hosts) completion_gated = { dep - for svc in services.values() - for dep, condition in depends_on(svc).items() + for svc_name, svc in services.items() + for dep, condition in depends_on(svc_name, svc).items() if condition == "service_completed_successfully" } names: set[str] = set() @@ -381,7 +381,7 @@ def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript: names |= stores.referenced_variables(compose, order, options.project_dir) waited: set[str] = set() for name in order: - for dep, condition in depends_on(services[name]).items(): + for dep, condition in depends_on(name, services[name]).items(): if condition == "service_healthy" and dep not in waited: interval = interval_seconds((services[dep].get("healthcheck") or {}).get("interval")) attempts = max(HEALTHY_WAIT_BUDGET_SECONDS // interval, 1) diff --git a/compose2pod/graph.py b/compose2pod/graph.py index 98a8e9a..a202be4 100644 --- a/compose2pod/graph.py +++ b/compose2pod/graph.py @@ -80,8 +80,8 @@ def _depends_on_entry_condition(dep: str, spec: dict[str, Any]) -> str: return condition -def depends_on(svc: dict[str, Any]) -> dict[str, str]: - """Normalize dependencies of a service to a name -> condition mapping.""" +def _declared_depends_on(svc: dict[str, Any]) -> dict[str, str]: + """Normalize the `depends_on` key alone to a name -> condition mapping.""" deps = svc.get("depends_on") if deps is None: # Explicitly absent, not merely falsy. `or {}` treated `depends_on: ""` @@ -101,6 +101,54 @@ def depends_on(svc: dict[str, Any]) -> dict[str, str]: return {dep: _depends_on_entry_condition(dep, spec) for dep, spec in deps.items()} +def _link_entries(name: str, svc: dict[str, Any]) -> list[tuple[str, str]]: + """Split each `links` entry into the service it names and the alias it adds. + + Measured against `docker compose config` v5.1.2. An entry splits into + `service:alias` on a single colon and not otherwise: `db:a:b` is refused as + `undefined service "db:a:b"`, so the whole entry is the name, and `:db` is + refused as `undefined service ""`, so the empty half is the name too. Both + verdicts fall out of returning the string unsplit or split as measured and + letting `validate_graph` refuse a name no service answers to. + + The alias half is empty for the plain form (`links: [db]`, which carries the + dependency and no new name, `db` already resolving) and for the measured + `links: ['db:']`, which docker ACCEPTS with a blank alias -- a name with no + characters cannot be written into a hosts file, so it contributes none. + """ + links = svc.get("links") + if links is None: + return [] + if not isinstance(links, list): + msg = f"service {name!r}: 'links' must be a list" + raise UnsupportedComposeError(msg) + entries: list[tuple[str, str]] = [] + for entry in links: + # A bool is an int and an int is not a string, so `isinstance(entry, str)` + # is the whole check -- docker refuses `links: [1]` and `links: [true]` alike. + if not isinstance(entry, str): + msg = f"service {name!r}: 'links' entry {entry!r} must be a string" + raise UnsupportedComposeError(msg) + service, _sep, alias = entry.partition(":") if entry.count(":") == 1 else (entry, "", "") + entries.append((service, alias)) + return entries + + +def depends_on(name: str, svc: dict[str, Any]) -> dict[str, str]: + """Dependencies of a service as a name -> condition mapping, from `depends_on` and `links`. + + `docker compose config` v5.1.2 normalises `links: [db]` into a `depends_on` + entry on `db` with `condition: service_started`, so the two keys feed one + graph -- which is why a self-link and a `links` cycle are both refused as + cycles, exactly as docker refuses them. Where both keys name the same + service the declared entry keeps its own condition, which is what docker's + normalised output carries: `links: [db]` beside a `service_healthy` + `depends_on` leaves the condition `service_healthy`. + """ + declared = _declared_depends_on(svc) + return {service: "service_started" for service, _alias in _link_entries(name, svc)} | declared + + # Docker validates container_name against this exact pattern (measured: # `container_name '' does not match pattern '[a-zA-Z0-9][a-zA-Z0-9_.-]+'`). # It is a *search*, not a fullmatch -- JSON-schema `pattern` semantics, which is @@ -125,6 +173,19 @@ def _validated_name(name: str, key: str, svc: dict[str, Any]) -> str | None: return value +def _link_aliases(name: str, svc: dict[str, Any]) -> list[str]: + """Names this service's `links` add, each belonging to the service it links to. + + Kept out of `_host_names`, which answers a different question: the names one + service is reachable by, declared on that service. A `links` alias is declared + on the service doing the linking and names another one. It needs no address of + its own -- every name compose2pod writes into the pod's hosts file resolves to + 127.0.0.1 -- only for the service it names to be running, which is what the + edge `depends_on` takes from the same entry guarantees. + """ + return [alias for _service, alias in _link_entries(name, svc) if alias] + + def _host_names(name: str, svc: dict[str, Any]) -> list[str]: """Names one service is reachable by: hostname, container_name, and network aliases.""" result: list[str] = [value for key in ("hostname", "container_name") if (value := _validated_name(name, key, svc))] @@ -147,10 +208,18 @@ def _host_names(name: str, svc: dict[str, Any]) -> list[str]: def hostnames(services: dict[str, Any]) -> list[str]: - """All names other services may use to reach a service: names, hostnames/container names, then aliases.""" + """All names other services may use to reach a service: names, hostnames/container names, then aliases. + + `links` aliases are collected here too, and so are scoped to whichever + services are passed in: at emit time that is the closure, so an alias + declared by a service that never runs never reaches the pod's hosts file. + A closure service's own alias always names a closure service, because the + same entry put it there. + """ names = list(services) for name, svc in services.items(): names.extend(_host_names(name, svc)) + names.extend(_link_aliases(name, svc)) return names @@ -181,7 +250,7 @@ def visit(name: str, declared_by: str) -> None: msg = f"service {declared_by!r}: unknown dependency {name!r}" raise UnsupportedComposeError(msg) state[name] = "visiting" - for dep in depends_on(services[name]): + for dep in depends_on(name, services[name]): visit(dep, name) state[name] = "done" order.append(name) diff --git a/compose2pod/keys.py b/compose2pod/keys.py index 22b6078..cf4e331 100644 --- a/compose2pod/keys.py +++ b/compose2pod/keys.py @@ -448,6 +448,7 @@ def _emit_ulimits(value: Any) -> list[Token]: # noqa: ANN401 - Compose values a # "tmpfs" removed — now a SERVICE_KEYS registry key (_scalar_or_list("--tmpfs")). "healthcheck", "depends_on", + "links", "networks", "hostname", "container_name", diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 94f198d..61b1aa0 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -65,18 +65,6 @@ def _validate_string_list(name: str, key: str, value: Any) -> None: # noqa: ANN "'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`. @@ -815,8 +803,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 _REFUSAL_REASONS: - msg = f"service {name!r}: {key!r} is not supported: {_REFUSAL_REASONS[key]}" + elif key in _POD_MODEL_REFUSALS: + msg = f"service {name!r}: {key!r} is not supported: {_POD_MODEL_REFUSALS[key]}" raise UnsupportedComposeError(msg) elif key not in SUPPORTED_SERVICE_KEYS: msg = f"service {name!r}: unsupported key '{key}'" @@ -1030,7 +1018,7 @@ def _validate_depends_on(services: dict[str, Any]) -> None: """ validate_graph(services) for name, svc in services.items(): - for dep, condition in depends_on(svc).items(): + for dep, condition in depends_on(name, svc).items(): if condition not in DEPENDS_ON_CONDITIONS: msg = f"service {name!r}: depends_on {dep!r} has unsupported condition {condition!r}" raise UnsupportedComposeError(msg) 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 e887e8a..7566add 100644 --- a/docs/adr/0003-the-shared-namespace-decides-key-classification.md +++ b/docs/adr/0003-the-shared-namespace-decides-key-classification.md @@ -10,11 +10,15 @@ supported way to name one. This clause once also swept up `links` and `expose`, 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 +-- docker refuses `links: [ghost]` exactly as it refuses a ghost `depends_on`, and a self-link as +a cycle -- so it neither escapes the namespace nor is satisfied by it, and ignoring it would drop +a dependency the `--target` closure is built from. Both halves were mechanisms compose2pod already +had, which is why the tracked limitation was expected to shrink and did +([#132](https://github.com/modern-python/compose2pod/issues/132)): the edge joins the mapping +`graph.depends_on` returns, so one graph is closed over and validated whichever key declared it, +and the alias joins `graph.hostnames`, landing in the pod's hosts file at `127.0.0.1` like every +other name in it. An alias needs no address of its own for exactly the reason `external_links` +cannot have one. `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 diff --git a/tests/conformance/corpus/service_links_empty_alias.yaml b/tests/conformance/corpus/service_links_empty_alias.yaml new file mode 100644 index 0000000..7aaf050 --- /dev/null +++ b/tests/conformance/corpus/service_links_empty_alias.yaml @@ -0,0 +1,10 @@ +# Measured ACCEPT by `docker compose config` v5.1.2: `links: ['db:']` keeps the entry +# verbatim, blank alias and all. The edge stands; a name with no characters cannot be +# written into the pod's hosts file, so it contributes none (issue 132). +services: + db: + image: nginx + app: + image: nginx + links: + - "db:" diff --git a/tests/conformance/corpus/service_links_multi_colon.yaml b/tests/conformance/corpus/service_links_multi_colon.yaml new file mode 100644 index 0000000..399927f --- /dev/null +++ b/tests/conformance/corpus/service_links_multi_colon.yaml @@ -0,0 +1,9 @@ +# Docker rejects this as `undefined service "db:a:b"`: an entry splits into service and +# alias on a single colon and not otherwise, so the whole string is the service name. +services: + db: + image: nginx + app: + image: nginx + links: + - db:a:b diff --git a/tests/conformance/corpus/service_links_self.yaml b/tests/conformance/corpus/service_links_self.yaml new file mode 100644 index 0000000..95c2f0d --- /dev/null +++ b/tests/conformance/corpus/service_links_self.yaml @@ -0,0 +1,7 @@ +# Docker rejects this as "dependency cycle detected: app -> app". `links` feeds the same +# dependency graph `depends_on` does, so a self-link is a self-edge. +services: + app: + image: nginx + links: + - app diff --git a/tests/conformance/test_corpus.py b/tests/conformance/test_corpus.py index 9fbf77c..792ce6f 100644 --- a/tests/conformance/test_corpus.py +++ b/tests/conformance/test_corpus.py @@ -130,19 +130,31 @@ 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( +def test_service_links_is_no_longer_an_over_rejection( assert_rule: Callable[[dict[str, Any]], str], ) -> None: - """Docker accepts `links: [db:database]`; compose2pod does not read the key yet. + """Docker accepts `links: [db:database]`, and since issue 132 so does compose2pod. - 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. + This assertion is the one the issue said would report when the limitation closed: it + asserted `over-reject` while the key was refused, and the verdict it asserts now is the + measurement that replaced it. Left to the generic corpus run it would stay green either + way, `over-reject` being an allowed verdict -- which is exactly why the flip needs saying. """ path = Path(__file__).parent / "corpus" / "service_links_alias.yaml" - assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject" + assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept" + + +def test_service_links_empty_alias_is_accepted_by_both( + assert_rule: Callable[[dict[str, Any]], str], +) -> None: + """`links: ['db:']` is accepted by docker with a blank alias, so it cannot be refused here. + + Asserted on the verdict for the same reason as the row above: an over-rejection of this + shape would pass the generic run silently, and refusing a document docker runs over an + alias that names nothing would be a limitation invented rather than measured. + """ + path = Path(__file__).parent / "corpus" / "service_links_empty_alias.yaml" + assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept" def test_service_external_links_is_a_catalogued_over_rejection( diff --git a/tests/integration/refusals.py b/tests/integration/refusals.py index ec69e35..f2e9109 100644 --- a/tests/integration/refusals.py +++ b/tests/integration/refusals.py @@ -39,10 +39,8 @@ 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. +script never creates, which is a fact about the script. 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_emit.py b/tests/test_emit.py index 1050c86..e1cd73d 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -1195,6 +1195,63 @@ def test_dependency_hostnames_and_aliases_still_resolve(self) -> None: assert f"127.0.0.1 {host}" in script +class TestLinksInTheEmittedScript: + """`links` supplies a startup edge and a hosts-file name, nothing else (issue 132).""" + + def _options(self, target: str) -> EmitOptions: + return EmitOptions( + target=target, + ci_image="ci:latest", + command="", + pod="test-pod", + project_dir=".", + artifacts=[], + allow_exit_codes=[], + ) + + def test_a_linked_service_starts_before_the_target(self) -> None: + compose = {"services": {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:database"]}}} + script = emit_script(compose=compose, options=self._options("app")) + assert "test-pod-db" in script + assert script.index("test-pod-db") < script.index("test-pod-app") + + def test_the_alias_resolves_and_so_does_the_linked_service_name(self) -> None: + compose = {"services": {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:database"]}}} + script = emit_script(compose=compose, options=self._options("app")) + for host in ("db", "database", "app"): + assert f"127.0.0.1 {host}" in script + + def test_the_plain_form_adds_the_dependency_and_no_name(self) -> None: + compose = {"services": {"db": {"image": "x"}, "app": {"image": "x", "links": ["db"]}}} + script = emit_script(compose=compose, options=self._options("app")) + assert "test-pod-db" in script + assert "127.0.0.1 db" in script + + def test_an_alias_colliding_with_extra_hosts_is_refused(self) -> None: + # A links alias is fixed at 127.0.0.1 like every other pod-internal name, so an + # `extra_hosts` entry giving the same name a real address is the same conflict + # `hosts_file_tokens` already refuses for a hostname or a network alias. + compose = { + "services": { + "db": {"image": "x"}, + "app": {"image": "x", "links": ["db:database"], "extra_hosts": ["database:1.2.3.4"]}, + } + } + with pytest.raises(UnsupportedComposeError, match=r"conflicting host 'database'"): + emit_script(compose=compose, options=self._options("app")) + + def test_an_alias_declared_outside_the_closure_does_not_reach_the_hosts_file(self) -> None: + compose = { + "services": { + "app": {"image": "x"}, + "db": {"image": "x"}, + "other": {"image": "x", "links": ["db:unreachable"]}, + } + } + script = emit_script(compose=compose, options=self._options("app")) + assert "unreachable" not in script + + class TestGuardedEnvFileDependencyWiring: def test_env_file_guarded_on_dependency_service(self) -> None: # Proves the prelude wiring on the `-d` dependency branch (not just the target), diff --git a/tests/test_graph.py b/tests/test_graph.py index b3d13e9..f1c3a8c 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -7,10 +7,10 @@ class TestDependsOn: def test_list_form_normalizes_to_service_started(self) -> None: - assert depends_on({"depends_on": ["db"]}) == {"db": "service_started"} + assert depends_on("app", {"depends_on": ["db"]}) == {"db": "service_started"} def test_map_form_keeps_conditions(self) -> None: - assert depends_on({"depends_on": {"db": {"condition": "service_healthy"}}}) == {"db": "service_healthy"} + assert depends_on("app", {"depends_on": {"db": {"condition": "service_healthy"}}}) == {"db": "service_healthy"} def test_long_form_missing_condition_raises(self) -> None: # Measured against `docker compose config` v5.1.2: a long-form entry @@ -19,24 +19,24 @@ def test_long_form_missing_condition_raises(self) -> None: # service_started -- Docker's own default, kept as-is. This default # was ours, not Docker's, and is the false green Task 13 closes. with pytest.raises(UnsupportedComposeError, match=r"depends_on entry 'db': missing required key 'condition'"): - depends_on({"depends_on": {"db": {}}}) + depends_on("app", {"depends_on": {"db": {}}}) def test_long_form_with_other_sub_keys_but_no_condition_still_raises(self) -> None: # A populated but condition-less entry is refused the same way -- # `restart`/`required` are no substitute for the required key. with pytest.raises(UnsupportedComposeError, match=r"depends_on entry 'db': missing required key 'condition'"): - depends_on({"depends_on": {"db": {"restart": True}}}) + depends_on("app", {"depends_on": {"db": {"restart": True}}}) def test_missing_depends_on_is_empty(self) -> None: - assert depends_on({"image": "x"}) == {} + assert depends_on("app", {"image": "x"}) == {} def test_non_list_or_mapping_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="'depends_on' must be a list or mapping"): - depends_on({"depends_on": "db"}) + depends_on("app", {"depends_on": "db"}) def test_mapping_entry_not_a_mapping_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="depends_on entry 'db' must be a mapping"): - depends_on({"depends_on": {"db": "service_healthy"}}) + depends_on("app", {"depends_on": {"db": "service_healthy"}}) def test_list_entry_not_a_string_raises(self) -> None: # Same YAML slip as `environment`/`command`: `- db: {condition: ...}` @@ -44,10 +44,10 @@ def test_list_entry_not_a_string_raises(self) -> None: # (TypeError: unhashable type: 'dict') from dict.fromkeys inside # validate() itself, instead of a clean UnsupportedComposeError. with pytest.raises(UnsupportedComposeError, match=r"depends_on entry .* must be a string"): - depends_on({"depends_on": [{"db": {"condition": "service_healthy"}}]}) + depends_on("app", {"depends_on": [{"db": {"condition": "service_healthy"}}]}) def test_list_form_string_entries_still_accepted(self) -> None: - assert depends_on({"depends_on": ["db", "keydb"]}) == { + assert depends_on("app", {"depends_on": ["db", "keydb"]}) == { "db": "service_started", "keydb": "service_started", } @@ -57,17 +57,17 @@ def test_unhashable_condition_raises_cleanly(self) -> None: # hashes `x` -- an unhashable condition (dict/list) used to crash # raw (TypeError: unhashable type) instead of failing clean. with pytest.raises(UnsupportedComposeError, match=r"depends_on entry 'db': condition must be a string"): - depends_on({"depends_on": {"db": {"condition": {"a": 1}}}}) + depends_on("app", {"depends_on": {"db": {"condition": {"a": 1}}}}) def test_list_condition_also_raises_cleanly(self) -> None: with pytest.raises(UnsupportedComposeError, match=r"depends_on entry 'db': condition must be a string"): - depends_on({"depends_on": {"db": {"condition": ["x"]}}}) + depends_on("app", {"depends_on": {"db": {"condition": ["x"]}}}) def test_int_condition_raises_cleanly(self) -> None: # Hashable but still not a valid condition shape -- must not slip # past this check only to fail confusingly deeper in. with pytest.raises(UnsupportedComposeError, match=r"depends_on entry 'db': condition must be a string"): - depends_on({"depends_on": {"db": {"condition": 1}}}) + depends_on("app", {"depends_on": {"db": {"condition": 1}}}) def test_depends_on_rejects_a_bare_string(self) -> None: with pytest.raises(UnsupportedComposeError, match="depends_on"): @@ -77,10 +77,10 @@ def test_unknown_sub_key_rejected(self) -> None: # Strict schema, measured against `docker compose config` v5.1.2: # "additional properties 'bogus' not allowed". with pytest.raises(UnsupportedComposeError, match="unsupported keys"): - depends_on({"depends_on": {"db": {"condition": "service_started", "bogus": 1}}}) + depends_on("app", {"depends_on": {"db": {"condition": "service_started", "bogus": 1}}}) def test_x_prefixed_extension_key_accepted(self) -> None: - assert depends_on({"depends_on": {"db": {"condition": "service_started", "x-custom": 1}}}) == { + assert depends_on("app", {"depends_on": {"db": {"condition": "service_started", "x-custom": 1}}}) == { "db": "service_started" } @@ -90,20 +90,20 @@ def test_restart_must_be_a_boolean(self) -> None: # refused). `condition` is present so this exercises the `restart` # check specifically, not the missing-condition one. with pytest.raises(UnsupportedComposeError, match=r"'restart' must be a boolean"): - depends_on({"depends_on": {"db": {"condition": "service_started", "restart": 5}}}) + depends_on("app", {"depends_on": {"db": {"condition": "service_started", "restart": 5}}}) def test_required_must_be_a_boolean(self) -> None: with pytest.raises(UnsupportedComposeError, match=r"'required' must be a boolean"): - depends_on({"depends_on": {"db": {"condition": "service_started", "required": "notabool"}}}) + depends_on("app", {"depends_on": {"db": {"condition": "service_started", "required": "notabool"}}}) def test_restart_and_required_true_and_false_accepted(self) -> None: assert depends_on( - {"depends_on": {"db": {"condition": "service_started", "restart": True, "required": False}}} + "app", {"depends_on": {"db": {"condition": "service_started", "restart": True, "required": False}}} ) == {"db": "service_started"} def test_restart_quoted_boolean_accepted(self) -> None: # Measured (docker compose config v5.1.2): a YAML-1.1 boolean string runs. - assert depends_on({"depends_on": {"db": {"condition": "service_started", "restart": "yes"}}}) == { + assert depends_on("app", {"depends_on": {"db": {"condition": "service_started", "restart": "yes"}}}) == { "db": "service_started" } @@ -113,12 +113,12 @@ def test_restart_variable_reference_passes_through(self) -> None: # failed to cast to expected type"), so its verdict is a fact about # the reading shell's environment, not the document -- the same # carve-out as `_validate_build_bool`. - assert depends_on({"depends_on": {"db": {"condition": "service_started", "restart": "${MYVAR}"}}}) == { + assert depends_on("app", {"depends_on": {"db": {"condition": "service_started", "restart": "${MYVAR}"}}}) == { "db": "service_started" } def test_required_variable_reference_passes_through(self) -> None: - assert depends_on({"depends_on": {"db": {"condition": "service_started", "required": "${MYVAR}"}}}) == { + assert depends_on("app", {"depends_on": {"db": {"condition": "service_started", "required": "${MYVAR}"}}}) == { "db": "service_started" } @@ -258,3 +258,90 @@ def test_a_service_reached_from_two_roots_is_walked_once(self) -> None: "b": {"image": "x", "depends_on": ["shared"]}, } assert validate_graph(services) is None + + +class TestLinks: + """A `depends_on` edge on the linked service, plus a hostname alias. + + Which is what `docker compose config` v5.1.2 normalises the key into (issue 132). + """ + + def test_plain_form_creates_the_edge_and_no_alias(self) -> None: + assert depends_on("app", {"links": ["db"]}) == {"db": "service_started"} + assert hostnames({"db": {"image": "x"}, "app": {"image": "x", "links": ["db"]}}) == ["db", "app"] + + def test_alias_form_creates_the_edge_and_the_alias(self) -> None: + assert depends_on("app", {"links": ["db:database"]}) == {"db": "service_started"} + services = {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:database"]}} + assert hostnames(services) == ["db", "app", "database"] + + def test_an_explicit_depends_on_entry_keeps_its_condition(self) -> None: + # Measured: with both keys present docker's normalised output carries + # `condition: service_healthy`, not the `service_started` links implies. + svc = {"links": ["db"], "depends_on": {"db": {"condition": "service_healthy"}}} + assert depends_on("app", svc) == {"db": "service_healthy"} + + def test_a_second_link_to_the_same_service_adds_one_edge_and_both_aliases(self) -> None: + assert depends_on("app", {"links": ["db:a", "db:b"]}) == {"db": "service_started"} + services = {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:a", "db:b"]}} + assert hostnames(services) == ["db", "app", "a", "b"] + + def test_an_empty_alias_carries_the_edge_and_contributes_no_name(self) -> None: + # Measured: `links: ['db:']` is ACCEPTED by docker, alias and all. A blank + # name would render as a hosts-file line with an address and nothing else. + assert depends_on("app", {"links": ["db:"]}) == {"db": "service_started"} + assert hostnames({"db": {"image": "x"}, "app": {"image": "x", "links": ["db:"]}}) == ["db", "app"] + + def test_more_than_one_colon_is_not_split(self) -> None: + # Measured: docker refuses `links: ['db:a:b']` as `undefined service "db:a:b"`, + # so the whole entry is the service name and the existence check refuses it. + assert depends_on("app", {"links": ["db:a:b"]}) == {"db:a:b": "service_started"} + + def test_a_leading_colon_names_the_empty_service(self) -> None: + # Measured: docker refuses `links: [':db']` as `undefined service ""`. + assert depends_on("app", {"links": [":db"]}) == {"": "service_started"} + + def test_duplicate_entries_collapse(self) -> None: + assert depends_on("app", {"links": ["db", "db"]}) == {"db": "service_started"} + + def test_links_must_be_a_list(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"service 'app': 'links' must be a list"): + depends_on("app", {"links": "db"}) + + def test_links_entry_must_be_a_string(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"service 'app': 'links' entry 1 must be a string"): + depends_on("app", {"links": [1]}) + + def test_links_entry_must_not_be_a_bool(self) -> None: + # Measured: docker refuses `links: [true]` ("unexpected type bool"). A bool is + # an int in Python, so a numeric check alone would let this through. + with pytest.raises(UnsupportedComposeError, match=r"service 'app': 'links' entry True must be a string"): + depends_on("app", {"links": [True]}) + + def test_an_alias_declared_by_a_service_outside_the_closure_is_not_collected(self) -> None: + # `hostnames` is called on the closure at emit time, so the alias of a + # service that never runs never reaches the pod's hosts file. + services = {"db": {"image": "x"}, "other": {"image": "x", "links": ["db:ghostalias"]}} + assert hostnames({"db": services["db"]}) == ["db"] + + +class TestLinksInTheGraph: + def test_a_link_to_an_undefined_service_is_refused(self) -> None: + services = {"app": {"image": "x", "links": ["ghost"]}} + with pytest.raises(UnsupportedComposeError, match=r"service 'app': unknown dependency 'ghost'"): + validate_graph(services) + + def test_a_self_link_is_refused_as_a_cycle(self) -> None: + # Measured: docker refuses `links: [app]` on `app` with "dependency cycle + # detected: app -> app", the same verdict a self-referencing depends_on gets. + with pytest.raises(UnsupportedComposeError, match=r"dependency cycle involving 'app'"): + validate_graph({"app": {"image": "x", "links": ["app"]}}) + + def test_a_links_cycle_is_refused(self) -> None: + services = {"a": {"image": "x", "links": ["b"]}, "b": {"image": "x", "links": ["a"]}} + with pytest.raises(UnsupportedComposeError, match=r"dependency cycle involving 'a'"): + validate_graph(services) + + def test_a_linked_service_joins_the_startup_closure(self) -> None: + services = {"db": {"image": "x"}, "app": {"image": "x", "links": ["db:database"]}} + assert startup_order(services, "app") == ["db", "app"] diff --git a/tests/test_keys.py b/tests/test_keys.py index abcb07f..082bee0 100644 --- a/tests/test_keys.py +++ b/tests/test_keys.py @@ -69,6 +69,7 @@ def test_supported_service_keys_snapshot() -> None: "tmpfs", "healthcheck", "depends_on", + "links", "networks", "hostname", "container_name", diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 4f1ae88..412fd0f 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -72,19 +72,24 @@ 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: + def test_links_is_accepted_in_both_forms_without_a_warning(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. + # alias, both of which compose2pod reads (issue 132). It is not inert, so unlike + # `expose` it is accepted silently rather than ignored with a warning. 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}}}) + assert 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"]}}}) + def test_links_naming_an_undefined_service_is_refused_at_the_gate(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"service 'app': unknown dependency 'ghost'"): + validate({"services": {"app": {"image": "x", "links": ["ghost"]}}}) - assert "podman" not in str(refusal.value) + def test_a_self_link_is_refused_at_the_gate(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"dependency cycle involving 'app'"): + validate({"services": {"app": {"image": "x", "links": ["app"]}}}) + + def test_malformed_links_is_refused_at_the_gate(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"service 'app': 'links' must be a list"): + validate({"services": {"db": {"image": "x"}, "app": {"image": "x", "links": "db"}}}) 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"):