Skip to content

Commit f7a3d45

Browse files
authored
fix: scope --add-host to the target's dependency closure (#55)
1 parent ac21b06 commit f7a3d45

4 files changed

Lines changed: 139 additions & 3 deletions

File tree

architecture/supported-subset.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,12 +281,20 @@ flags. compose2pod hoists them onto `podman pod create` instead
281281
unioned by key, and two closure services setting the same key to
282282
different values is refused (`conflicting sysctl ...`) rather than
283283
resolved last-writer-wins. `--add-host` is seeded from the alias/hostname
284-
set (document-wide, not closure-scoped — see `hostname`/`container_name`/
285-
`networks`, above), then layered with each closure service's
284+
set of the closure's services, then layered with each closure service's
286285
`extra_hosts`; a host name landing on two different addresses is refused
287286
the same way (`conflicting host ...`). An alias/hostname `--add-host`
288287
entry stays a plain unquoted token; an `extra_hosts` entry is
289288
`${VAR}`-live.
289+
290+
Only the closure joins the pod, so only the closure is resolvable: a
291+
service outside it contributes no name and cannot conflict with an
292+
`extra_hosts` entry. Resolving a never-run service's name to `127.0.0.1`
293+
would point it at a port where nothing listens, turning an honest
294+
resolution failure into a connection-refused. Shape validation of
295+
`hostname`/`container_name`/`networks` stays document-wide at the gate
296+
(`hostnames` in `parsing.py`), so a malformed value is still rejected on a
297+
service the target never reaches.
290298
- **Pod-wide divergence:** unlike every other service key, these apply to
291299
every container in the pod once emitted — including services that never
292300
declared them — because the pod shares one `/etc/resolv.conf`, sysctl

compose2pod/emit.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,11 @@ def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript:
272272
msg = f"invalid pod name {options.pod!r}"
273273
raise UnsupportedComposeError(msg)
274274
services = compose["services"]
275-
hosts = hostnames(services)
276275
order = startup_order(services, options.target)
276+
# Only the closure joins the pod, so only the closure is resolvable. A name
277+
# pointing at 127.0.0.1 for a service that never runs would turn an honest
278+
# resolution failure into a connection-refused.
279+
hosts = hostnames({name: services[name] for name in order})
277280
completion_gated = {
278281
dep
279282
for svc in services.values()
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
summary: The emit-side --add-host set is scoped to the target's dependency closure, so a service that never runs no longer resolves to 127.0.0.1 and can no longer veto another service's extra_hosts.
3+
---
4+
5+
# Change: Scope --add-host to the target's closure
6+
7+
**Lane:** lightweight — one-line change in `emit._plan`, plus tests.
8+
9+
## Goal
10+
11+
`--add-host` is the one aggregate in the emit path that is built document-wide
12+
instead of closure-scoped, and the seam shows. `emit._plan` seeds hosts from
13+
`graph.hostnames(services)` — every service in the *document* — while
14+
`extra_hosts` is layered per service in `order`, the target's dependency
15+
closure. `pod._add_host_flags` then conflict-checks the two against each other,
16+
so a service that **never runs** can veto a valid configuration:
17+
18+
```yaml
19+
services:
20+
app: {image: i, extra_hosts: ["db:1.2.3.4"]}
21+
other: {image: i, hostname: db} # not in app's closure; never started
22+
```
23+
`UnsupportedComposeError: service 'app': conflicting host 'db'
24+
('127.0.0.1' vs '1.2.3.4')`
25+
26+
The second symptom is quieter: a never-run service still gets an `--add-host`
27+
entry pointing its name at `127.0.0.1`, where nothing is listening. That turns
28+
an honest name-resolution failure into a connection-refused.
29+
30+
Every other aggregate in the emit path — `dns`, `dns_search`, `dns_opt`,
31+
`sysctls`, secrets, configs — is closure-scoped. This makes `--add-host` agree.
32+
33+
## Approach
34+
35+
`emit._plan` passes only the closure's services to `hostnames()`:
36+
37+
```python
38+
hosts = hostnames({name: services[name] for name in order})
39+
```
40+
41+
`hostnames()` has exactly two callers, and only this one changes:
42+
43+
- `parsing.py` calls it document-wide to shape-check every service's
44+
`hostname`/`container_name`/`networks` at the gate. That stays — validation
45+
is target-agnostic, and scoping it would stop rejecting a malformed
46+
`hostname` on a service outside the closure.
47+
- `emit.py` calls it to build the `--add-host` set. That is the one that must
48+
match the pod's actual contents.
49+
50+
Truth home: `architecture/supported-subset.md`'s Pod-level options section,
51+
which currently documents the document-wide behavior as "pre-existing,
52+
orthogonal".
53+
54+
## Files
55+
56+
- `compose2pod/emit.py` — scope the `hostnames()` argument to `order`
57+
- `architecture/supported-subset.md` — Pod-level options: `--add-host` is
58+
closure-scoped like the rest
59+
- `tests/test_emit.py` — tests added
60+
61+
## Verification
62+
63+
- [ ] Failing test first: an out-of-closure `hostname` colliding with an
64+
in-closure `extra_hosts` must NOT raise; a never-run service must NOT
65+
appear in `--add-host`.
66+
- [ ] Apply the change.
67+
- [ ] Tests pass.
68+
- [ ] `just test-ci` — full suite green at 100%.
69+
- [ ] `just lint-ci`, `just check-planning` — clean.

tests/test_emit.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -995,3 +995,59 @@ def test_referenced_variables_is_equally_guarded(self) -> None:
995995
# and must reject malformed input identically.
996996
with pytest.raises(UnsupportedComposeError):
997997
referenced_variables({}, self._options())
998+
999+
1000+
class TestAddHostClosureScope:
1001+
"""--add-host covers the target's closure, like every other emit-path aggregate."""
1002+
1003+
def _options(self, target: str) -> EmitOptions:
1004+
return EmitOptions(
1005+
target=target,
1006+
ci_image="ci:latest",
1007+
command="",
1008+
pod="test-pod",
1009+
project_dir=".",
1010+
artifacts=[],
1011+
allow_exit_codes=[],
1012+
)
1013+
1014+
def test_service_outside_the_closure_is_not_resolvable(self) -> None:
1015+
# A never-run service pointed its name at 127.0.0.1, where nothing listens.
1016+
compose = {"services": {"app": {"image": "x"}, "never_run": {"image": "x"}}}
1017+
script = emit_script(compose=compose, options=self._options("app"))
1018+
assert "--add-host app:127.0.0.1" in script
1019+
assert "never_run" not in script
1020+
1021+
def test_out_of_closure_hostname_does_not_veto_extra_hosts(self) -> None:
1022+
# 'other' is not in app's closure, so its hostname cannot conflict with app's extra_hosts.
1023+
compose = {
1024+
"services": {
1025+
"app": {"image": "x", "extra_hosts": ["db:1.2.3.4"]},
1026+
"other": {"image": "x", "hostname": "db"},
1027+
}
1028+
}
1029+
script = emit_script(compose=compose, options=self._options("app"))
1030+
assert '--add-host "db:1.2.3.4"' in script
1031+
1032+
def test_in_closure_hostname_still_conflicts_with_extra_hosts(self) -> None:
1033+
# The conflict rule still holds for services that actually run.
1034+
compose = {
1035+
"services": {
1036+
"app": {"image": "x", "extra_hosts": ["db:1.2.3.4"], "depends_on": ["db_svc"]},
1037+
"db_svc": {"image": "x", "hostname": "db"},
1038+
}
1039+
}
1040+
with pytest.raises(UnsupportedComposeError, match="conflicting host"):
1041+
emit_script(compose=compose, options=self._options("app"))
1042+
1043+
def test_dependency_hostnames_and_aliases_still_resolve(self) -> None:
1044+
# Everything inside the closure keeps its add-host entry.
1045+
compose = {
1046+
"services": {
1047+
"app": {"image": "x", "depends_on": ["db"]},
1048+
"db": {"image": "x", "hostname": "db-host", "networks": {"default": {"aliases": ["db-alias"]}}},
1049+
}
1050+
}
1051+
script = emit_script(compose=compose, options=self._options("app"))
1052+
for host in ("app", "db", "db-host", "db-alias"):
1053+
assert f"--add-host {host}:127.0.0.1" in script

0 commit comments

Comments
 (0)