From e4bc89da9080ca0df1448ce3b954cc99c4b9aa86 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 19 Sep 2026 14:00:43 +0300 Subject: [PATCH] docs(adr): compress to 6 records in the domain-modeling format Drop 0004, 0005, 0007, 0010, 0011, 0012, 0013 as separate records and fold what still holds into the survivors; merge 0003/0004/0005 into the shared-namespace record, 0006/0007 into the raw-dict seam record, 0008/0010/0013 into the owning-modules record, 0011/0012 into the parity record. Renumber the six survivors 0001-0006 in their original order and move every citation (CONTEXT.md, graph.py, values.py, three tests). --- CONTEXT.md | 6 +- compose2pod/graph.py | 2 +- compose2pod/values.py | 2 +- ...hcheck-start-period-retries-passthrough.md | 55 ------- ...k-wait-budget-is-not-coupled-to-retries.md | 9 ++ docs/adr/0002-zero-dependency-core.md | 62 +------- .../adr/0003-reject-namespace-network-keys.md | 52 ------- ...ed-namespace-decides-key-classification.md | 12 ++ docs/adr/0004-stop-lifecycle-keys-inert.md | 40 ------ ...alidate-and-emit-both-read-the-raw-dict.md | 11 ++ ...validators-stay-in-their-owning-modules.md | 17 +++ docs/adr/0005-sysctls-pod-level.md | 49 ------- docs/adr/0006-docker-rejection-parity.md | 16 +++ docs/adr/0006-reject-parse-dont-validate.md | 88 ------------ .../adr/0007-keep-graph-query-as-validator.md | 49 ------- .../0008-reject-structural-key-registry.md | 91 ------------ docs/adr/0009-docker-rejection-parity.md | 136 ------------------ ...ect-strict-schema-validator-unification.md | 82 ----------- docs/adr/0011-list-of-str-refusals.md | 41 ------ ...tive-numeric-values-deferred-to-runtime.md | 42 ------ docs/adr/0013-volumes-stays-hand-rolled.md | 27 ---- tests/conformance/conftest.py | 2 +- tests/test_cli.py | 2 +- tests/test_values.py | 2 +- 24 files changed, 79 insertions(+), 816 deletions(-) delete mode 100644 docs/adr/0001-healthcheck-start-period-retries-passthrough.md create mode 100644 docs/adr/0001-healthcheck-wait-budget-is-not-coupled-to-retries.md delete mode 100644 docs/adr/0003-reject-namespace-network-keys.md create mode 100644 docs/adr/0003-the-shared-namespace-decides-key-classification.md delete mode 100644 docs/adr/0004-stop-lifecycle-keys-inert.md create mode 100644 docs/adr/0004-validate-and-emit-both-read-the-raw-dict.md create mode 100644 docs/adr/0005-structural-keys-and-schema-validators-stay-in-their-owning-modules.md delete mode 100644 docs/adr/0005-sysctls-pod-level.md create mode 100644 docs/adr/0006-docker-rejection-parity.md delete mode 100644 docs/adr/0006-reject-parse-dont-validate.md delete mode 100644 docs/adr/0007-keep-graph-query-as-validator.md delete mode 100644 docs/adr/0008-reject-structural-key-registry.md delete mode 100644 docs/adr/0009-docker-rejection-parity.md delete mode 100644 docs/adr/0010-reject-strict-schema-validator-unification.md delete mode 100644 docs/adr/0011-list-of-str-refusals.md delete mode 100644 docs/adr/0012-negative-numeric-values-deferred-to-runtime.md delete mode 100644 docs/adr/0013-volumes-stays-hand-rolled.md diff --git a/CONTEXT.md b/CONTEXT.md index e8959c2..0c85ed7 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -26,8 +26,8 @@ A supported service key handled *outside* the service-key registry, listed in `project_dir` (`env_file`, `volumes`), spans keys, or occupies the image/command slot (`entrypoint`). Structural keys keep their own validate/emit machinery, in the module that owns the concern. Which side of this line a key falls on is a design ruling, not -a convenience: see [ADR-0008](docs/adr/0008-reject-structural-key-registry.md) and -[ADR-0013](docs/adr/0013-volumes-stays-hand-rolled.md). +a convenience: see +[ADR-0005](docs/adr/0005-structural-keys-and-schema-validators-stay-in-their-owning-modules.md). **Token**: The result of rendering one Compose value into a `podman run`/`pod create` argument: @@ -53,7 +53,7 @@ document-wide. **Rule one / rule two**: The two directions of the Docker-rejection parity rule -([ADR-0009](docs/adr/0009-docker-rejection-parity.md)). "Rule two" is named bare +([ADR-0006](docs/adr/0006-docker-rejection-parity.md)). "Rule two" is named bare in `parsing.py` comments and in tests, with no restatement at the call site. **Rule one**: a document `docker compose config` rejects, compose2pod rejects too — hard, no exceptions. **Rule two**: a document Docker accepts, diff --git a/compose2pod/graph.py b/compose2pod/graph.py index 4ba2f72..2678fd4 100644 --- a/compose2pod/graph.py +++ b/compose2pod/graph.py @@ -58,7 +58,7 @@ def _depends_on_entry_condition(dep: str, spec: dict[str, Any]) -> str: # defaults to service_started -- that default is Docker's own and # stays. This one was compose2pod's own invention (`spec.get(..., # "service_started")`) and was a false green against the hard rule - # in `docs/adr/0009-docker-rejection-parity.md`. + # in `docs/adr/0006-docker-rejection-parity.md`. msg = f"depends_on entry {dep!r}: missing required key 'condition'" raise UnsupportedComposeError(msg) condition = spec["condition"] diff --git a/compose2pod/values.py b/compose2pod/values.py index 1eea5c8..9d5c3d5 100644 --- a/compose2pod/values.py +++ b/compose2pod/values.py @@ -1,7 +1,7 @@ """Value grammars: the shapes `docker compose config` accepts for a scalar key. compose2pod refuses every document Docker refuses -(`docs/adr/0009-docker-rejection-parity.md`), which means +(`docs/adr/0006-docker-rejection-parity.md`), which means matching Docker's *value* grammars, not just its types: `mem_limit: ""` and `cpus: somevalue` are documents Docker will not run. diff --git a/docs/adr/0001-healthcheck-start-period-retries-passthrough.md b/docs/adr/0001-healthcheck-start-period-retries-passthrough.md deleted file mode 100644 index 98d3646..0000000 --- a/docs/adr/0001-healthcheck-start-period-retries-passthrough.md +++ /dev/null @@ -1,55 +0,0 @@ -# Healthcheck start_period/retries pass-through without shortening the wait budget - -**Decision:** `run_flags` passes a service's healthcheck `start_period` and -`retries` through to `podman run` as `--health-start-period` and -`--health-retries` respectively (alongside the existing -`--health-timeout`). The `wait_healthy` polling budget in the emitted -script stays `HEALTHY_WAIT_BUDGET_SECONDS // interval` attempts — -it is **not** shortened to `retries × interval`. - -## Context - -The chats prototype validated `start_period` and `retries` as recognized -healthcheck keys but silently ignored both when building `podman run` -flags and when computing the `wait_healthy` poll budget — a -review-identified gap (deviation #2 in the extraction design). Two things -needed fixing: recording the author's intent on the container itself, and -deciding whether the emitted script's own wait loop should account for -`retries`/`start_period`. - -The options considered for the wait loop: -1. Keep the loop's fixed `HEALTHY_WAIT_BUDGET_SECONDS` budget, independent - of the podman-level `--health-retries`/`--health-start-period` values. -2. Recompute the loop's attempt count from `retries × interval` (plus - `start_period`), so the script-level wait tracks the container's own - healthcheck retry policy more precisely. - -## Decision & rationale - -- **Pass `start_period`/`retries` to `podman run`** so the container's own - healthcheck scheduling (as podman understands it) matches what the - compose file declares — this is the straightforward, low-risk half of - the fix, and was uncontroversial. -- **Do not shorten the wait budget to `retries × interval`.** `wait_healthy` - polls `podman healthcheck run ` directly in a loop and returns as - soon as the first successful check is observed — it does not wait for the - full budget on a healthy service. Coupling the budget to `retries × - interval` would risk **premature failure** for a service with a long - `start_period`: `retries` counts consecutive *failures* podman tolerates - before marking a container unhealthy, not the time before the first - check is meaningful, and a short `retries × interval` product could expire - the script's wait loop before `start_period` has even elapsed. The fixed, - generous `HEALTHY_WAIT_BUDGET_SECONDS` (120s) is a safer default that - errs toward giving slow-starting services enough time, at the cost of a - slower failure signal for services that are genuinely broken. - -**Revisit trigger:** any of — - -- A real service in the fleet needs a `start_period` long enough that the - fixed 120s `HEALTHY_WAIT_BUDGET_SECONDS` is insufficient (i.e. the - container needs more than 120s from first `podman run` to first healthy - check), which would call for making the budget configurable rather than - coupling it to `retries × interval`. -- Repeated false-positive "did not become healthy" failures in CI point at - the fixed budget being too tight for a class of services, prompting a - reconsideration of how the budget is derived. diff --git a/docs/adr/0001-healthcheck-wait-budget-is-not-coupled-to-retries.md b/docs/adr/0001-healthcheck-wait-budget-is-not-coupled-to-retries.md new file mode 100644 index 0000000..d4a11c8 --- /dev/null +++ b/docs/adr/0001-healthcheck-wait-budget-is-not-coupled-to-retries.md @@ -0,0 +1,9 @@ +# The healthcheck wait budget is not coupled to `retries` + +`run_flags` passes a healthcheck's `start_period` and `retries` through as `--health-start-period` +and `--health-retries`, but the emitted script's `wait_healthy` loop keeps its fixed +`HEALTHY_WAIT_BUDGET_SECONDS` rather than deriving `retries × interval`. `retries` counts the +consecutive failures podman tolerates, not the time until the first check is meaningful, so a +budget derived from it can expire before a long `start_period` has elapsed. The fixed 120 s errs +toward slow starters at the cost of a slower failure signal; a service that needs more makes the +budget configurable, not coupled. diff --git a/docs/adr/0002-zero-dependency-core.md b/docs/adr/0002-zero-dependency-core.md index ee1b0d4..2258567 100644 --- a/docs/adr/0002-zero-dependency-core.md +++ b/docs/adr/0002-zero-dependency-core.md @@ -1,57 +1,7 @@ -# Zero-dependency core, no compose-parser dependency +# Zero-dependency core -**Decision:** The `compose2pod` core package has zero runtime dependencies -(stdlib only). PyYAML is shipped only behind the optional `[yaml]` extra. -The core does not depend on any compose-spec parser library. - -## Context - -`compose2pod` reads a Docker Compose document and must decide how it is -loaded (JSON/YAML) and validated against the supported subset. Two -dependency questions came up while designing the package: - -1. Should YAML parsing be a hard dependency, so the CLI always accepts - `docker-compose.yml` directly? -2. Should the core adopt an existing compose-spec parser library - (`compose-spec`, `compose-pydantic`) instead of hand-rolled subset - validation, to get broader spec coverage for free? - -The primary differentiator for `compose2pod` is that it installs with no -compiled wheels and runs in minimal Python images (the same CI containers -that motivate the tool in the first place — see the design's "Why this -exists"). Any hard dependency, especially one with a compiled wheel, -undermines that. - -## Decision & rationale - -- **YAML stays optional.** JSON parsing via stdlib `json` is always - available. YAML parsing via PyYAML is the optional `[yaml]` extra; without - it, `--format yaml` errors with an actionable message pointing at `pip - install compose2pod[yaml]` or piping through `yq -o=json`. This keeps the - dependency-constrained-CI path (where even PyYAML may not be installable) - fully functional. -- **No compose-spec parser dependency.** Researched candidates - (`compose-spec`, `compose-pydantic`) both require pydantic v2 (a compiled - `pydantic-core` wheel) and are early-stage single-maintainer 0.x projects. - Adopting one would: - (a) break the zero-dependency differentiator for the core, - (b) not actually remove the subset validation — full-spec parsers - permissively accept constructs (`configs`, `secrets`, `profiles`, - `deploy`, long-form volumes, ...) that `compose2pod` cannot turn into a - single pod, so a subset gate is still required regardless of what parses - the document, and - (c) add supply-chain risk (compiled wheel, single maintainer, early 0.x) - to a package whose whole pitch is minimal-footprint installability. -- A future optional `[strict]` extra could cross-validate against - `compose-spec` for users who want full-spec checking, but that is - deliberately out of scope for v1 (YAGNI) — no user need has been - identified yet. - -**Revisit trigger:** any of — - -- A mature, pure-Python (no compiled wheel) compose-spec parser reaches a - stable 1.x release with more than one maintainer, and a concrete user - need for full-spec validation (beyond the subset `compose2pod` supports) - emerges. -- PyYAML itself becomes uninstallable in a target CI environment that - `compose2pod` needs to support, forcing a rethink of the YAML story. +The core has no runtime dependencies: JSON via the stdlib always works and PyYAML sits behind the +`[yaml]` extra, because the tool exists for minimal CI images where even a compiled wheel may not +install. No compose-spec parser library is used either. The candidates require `pydantic-core`, +are early single-maintainer projects, and would not remove the subset gate anyway, since a +full-spec parser accepts constructs a single pod cannot express. diff --git a/docs/adr/0003-reject-namespace-network-keys.md b/docs/adr/0003-reject-namespace-network-keys.md deleted file mode 100644 index 1253c82..0000000 --- a/docs/adr/0003-reject-namespace-network-keys.md +++ /dev/null @@ -1,52 +0,0 @@ -# Reject network- and namespace-mode service keys, but only some permanently - -**Decision:** The Compose keys that touch a container's network or namespace -mode are rejected, but for two distinct reasons that must not be conflated: -one permanent, one contingent on the pod model. `dns` and `pid`, previously -grouped with them, are feasible and are *deferred*, not rejected. - -## Context - -While auditing Compose spec coverage the reject bucket was justified -with a single reason — "namespaces shared at pod level — conflict." That reason -is imprecise. `compose2pod` runs every service in one Podman pod that shares -`net`, `uts`, `ipc` (and `cgroup`) by default; `pid` is **not** shared by -default. So "shared namespace" does not uniformly explain the rejects, and the -tool risks treating a feasible key as impossible. - -The tool's one load-bearing invariant is the **shared network namespace**: -services talk over `127.0.0.1` and resolve names via per-container `--add-host`. -That invariant is the whole reason the tool exists (bridge-less CI). It is the -correct axis for deciding acceptance. - -## Decision & rationale - -- **Permanent reject (invariant-violating):** `network_mode`, `links`, - `external_links`, `expose`. Honoring any of these pulls a container out of the - shared netns or implies bridge/link semantics — silently breaking localhost - service discovery. Refusing is the contract, not a limitation. -- **Reject while the pod keeps default `--share` (fights a shared namespace):** - `ipc`, `uts`, `domainname`, `cgroup`, `userns_mode`. A per-container override - conflicts with the pod's shared namespace; supporting it means reshaping how - the pod is created for a need no CI user has raised. -- **Deferred, not rejected (feasible):** - - `dns` / `dns_search` / `dns_opt` are **pod-wide, not per-container**. Podman - rejects `--dns` on a container that has joined a pod's netns (invalid when - the netns is `container:`), while `--add-host` edits the per-container - `/etc/hosts` and is allowed — hence the tool already emits add-host per - container. `dns` is expressible on `podman pod create` and reconciled across - services. Tracked as unscheduled work. - - `pid` is not pod-shared, so `pid: host` / `pid: service:x` map to clean - per-container `--pid` flags with no pod conflict. Feasible; low demand. - -The per-container-`--dns`-invalid-in-pod behavior is documented, not yet -observed on a live podman; validate it before building `dns` support. - -**Revisit trigger:** any of — - -- A concrete CI need appears for a per-service network/namespace mode - (`ipc: host`, a private `pid` namespace, a service-specific resolver), with a - design that preserves the shared-network invariant — e.g. hoisting `dns` to - `podman pod create`, or selectively narrowing the pod's `--share`. -- A live podman run contradicts the documented `--dns`-in-pod behavior this - decision relies on. diff --git a/docs/adr/0003-the-shared-namespace-decides-key-classification.md b/docs/adr/0003-the-shared-namespace-decides-key-classification.md new file mode 100644 index 0000000..4e4f9f9 --- /dev/null +++ b/docs/adr/0003-the-shared-namespace-decides-key-classification.md @@ -0,0 +1,12 @@ +# 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. diff --git a/docs/adr/0004-stop-lifecycle-keys-inert.md b/docs/adr/0004-stop-lifecycle-keys-inert.md deleted file mode 100644 index d08fc4b..0000000 --- a/docs/adr/0004-stop-lifecycle-keys-inert.md +++ /dev/null @@ -1,40 +0,0 @@ -# stop_signal / stop_grace_period are inert under the force teardown - -**Decision:** `stop_signal` and `stop_grace_period` are not supported and are -reclassified out of Bucket A (clean per-container flags) into -accepted-but-inert. They map to `podman run --stop-signal` / `--stop-timeout`, -which only take effect during a graceful `podman stop` -- something the -generated script never performs. - -## Context - -The spec-coverage audit listed -`stop_signal`/`stop_grace_period` in Bucket A as "clean per-container flag -mappings." Revisiting during the container-confinement bundle showed the -teardown model makes them inert. - -The generated script (`emit.py` `emit_script`) starts services -(`podman run -d`, `--rm` for completion-gated deps, foreground for the target) -and cleans up with a single `trap 'podman pod rm -f ' EXIT`. There is no -`podman stop` anywhere: the pod is force-removed (SIGKILL), which bypasses the -per-container stop signal and grace period entirely. So emitting -`--stop-signal`/`--stop-timeout` would set container metadata that nothing in -the script's lifecycle ever consults. - -## Decision & rationale - -- Supporting them would emit flags with **no observable effect** -- against the - tool's honest-subset principle, which prefers to refuse (or leave - behavior-neutral constructs ignored) rather than imply behavior it does not - deliver. -- They join the "accepted-but-inert" category alongside `ports`, `restart`, - `stdin_open`, `tty` -- valid Compose that is meaningless in this pod's run + - force-teardown model. They are in `IGNORED_SERVICE_KEYS`, so a document that - uses them is accepted with a warning rather than rejected. -- This is the same reasoning shape as the `dns` reclassification: a key looked - like a clean flag until the pod/runtime model was examined. - -**Revisit trigger:** the generated script gains a graceful-stop phase (an -explicit `podman stop` with a timeout before `pod rm`, or a per-service stop -sequence), at which point `--stop-signal`/`--stop-timeout` would become -effective and worth emitting. diff --git a/docs/adr/0004-validate-and-emit-both-read-the-raw-dict.md b/docs/adr/0004-validate-and-emit-both-read-the-raw-dict.md new file mode 100644 index 0000000..40baf49 --- /dev/null +++ b/docs/adr/0004-validate-and-emit-both-read-the-raw-dict.md @@ -0,0 +1,11 @@ +# `validate` and `emit` both read the raw dict + +`validate(compose)` and `emit_script(compose, options)` both take the raw compose dict. There is +no typed `CheckedDocument` produced by one and consumed by the other, and the graph queries in +`graph.py` (`depends_on`, `hostnames`, `startup_order`) normalise and raise on bad shape wherever +they are called rather than being computed once and threaded through. The gate is +target-agnostic and emit is target-scoped, so one shared model cannot span them, and the typed +model would cost a registry rewrite plus a breaking API for a gap that is closed differently: +`emit._plan`, the single traversal both `emit_script` and `referenced_variables` project from, +calls `validate(compose)` itself, so a library caller cannot reach a raw `KeyError` or a corrupted +flag. A second emit consumer is what would make the typed model pay. diff --git a/docs/adr/0005-structural-keys-and-schema-validators-stay-in-their-owning-modules.md b/docs/adr/0005-structural-keys-and-schema-validators-stay-in-their-owning-modules.md new file mode 100644 index 0000000..8e22ecf --- /dev/null +++ b/docs/adr/0005-structural-keys-and-schema-validators-stay-in-their-owning-modules.md @@ -0,0 +1,17 @@ +# Structural keys and schema validators stay in their owning modules + +`SERVICE_KEYS` holds the keys that share one `emit(value) -> list[Token]` interface. The +structural keys in `STRUCTURAL_KEYS` stay in the module that owns their concern (`emit.py`, +`graph.py`, `pod.py`, `stores.py`, `resources.py`, `healthcheck.py`) because they have at least +six emit shapes: `entrypoint` emits on both sides of the image token and its string form cancels +`command`, `volumes` and `env_file` need `project_dir`, `depends_on` drives ordering, `dns` and +`sysctls` aggregate onto the pod, `secrets` and `deploy` need document scope. A registry over +them would be `Any`-typed callbacks, a false seam, and widening `KeySpec.emit` to carry a context +for the two keys that need it would tax the ~29 that do not, which is why `tmpfs` joined the +registry and `volumes` did not. The same holds for the ~14 strict-schema validators: the shareable +core is a three-line unknown-key check and everything around it differs per site (`x-` policy, +pre-check shape, required keys, message text), so only the two identical definition validators +share `parsing._validate_top_level_definition`. Reopened 2026-07-15 with two reviewers blind to +this record arguing opposite sides; both converged on it. A third reader of the key-name list, or +a cluster of new keys sharing one new shape, earns a narrow sub-registry for that shape, never a +universal one. diff --git a/docs/adr/0005-sysctls-pod-level.md b/docs/adr/0005-sysctls-pod-level.md deleted file mode 100644 index df6d935..0000000 --- a/docs/adr/0005-sysctls-pod-level.md +++ /dev/null @@ -1,49 +0,0 @@ -# sysctls is pod-level, not per-container - -**Decision:** `sysctls` is not supported as a per-container flag and is -reclassified out of Bucket A. The only sysctls podman lets a container set are -namespaced to the network or IPC namespace, and a Podman pod owns both, so -sysctls belong on `podman pod create --sysctl` (pod-wide) -- the same shape as -`dns`. It stays refused (raises), and pod-level support is deferred alongside -`dns`. - -## Context - -The spec-coverage audit listed -`sysctls` in Bucket A as a per-container `--sysctl` flag, with a note that -`net.*` are pod-level. Revisiting during the `ulimits` bundle, the podman docs show the note -understated it: - -- For the **network** namespace, only `net.*` sysctls are allowed, and only if - that namespace is owned by the container. -- For the **IPC** namespace, only a fixed set (`kernel.msgmax`, `kernel.sem`, - `kernel.shm*`, `fs.mqueue.*`, ...) is allowed, and only if owned. -- Non-namespaced sysctls cannot be set in a container at all. - -A Podman pod shares net + ipc by default, so a container joining the pod owns -neither namespace. Every settable sysctl is therefore pod-level in this model; -a per-container `podman run --sysctl` would be rejected or wrong. - -## Decision & rationale - -- **Not per-container.** Emitting `--sysctl` on `podman run` cannot honor the - request in a shared-namespace pod. The honest home is - `podman pod create --sysctl`, unioned/conflict-checked across services -- the - same pod-level-aggregation design `dns` needs. -- **Refuse, do not ignore.** Unlike `stop_signal`/`stop_grace_period` (inert, so - warn-and-ignore), a `sysctls` request is *not* behavior-neutral -- silently - dropping it would lose behavior the user asked for. So `sysctls` keeps raising - as unsupported, matching `dns`. -- The behavior is documented, not observed on a live podman in a pod; validate - before building pod-level support. - -**Resolved.** The trigger below was met: the pod-level-aggregation pattern was -built, so `sysctls` (and `dns`/`dns_search`/`dns_opt`) are now supported via -`podman pod create --sysctl`/`--dns`, unioned and conflict-checked across the -target's closure. This decision's reasoning (sysctls is pod-level, not -per-container) was realized, not reversed, so this record stands as the account -of why the pod-level home was chosen. - -**Revisit trigger:** the pod-level-aggregation pattern is built (for `dns` or -otherwise), giving `sysctls` a `podman pod create --sysctl` home; or a live -podman run contradicts the documented per-container-`--sysctl`-in-pod behavior. diff --git a/docs/adr/0006-docker-rejection-parity.md b/docs/adr/0006-docker-rejection-parity.md new file mode 100644 index 0000000..2699373 --- /dev/null +++ b/docs/adr/0006-docker-rejection-parity.md @@ -0,0 +1,16 @@ +# Docker's refusals bind; podman decides what we can accept + +Two rules, one direction each. A document `docker compose config` rejects, compose2pod rejects: +`accepted(compose2pod) ⊆ accepted(docker)`, because the tool is a drop-in for `docker compose` on +rootless runners and accepting a file Docker refuses turns a hard error into a green CI run. A +document Docker accepts, compose2pod accepts whenever podman can express it inside a pod. Where +podman cannot, that is a legitimate refusal (`network_mode`; `sysctls: ["a"]` with no value; +`volumes: ["a"]`, which podman rejects as a relative mount target), and where compose2pod merely +does not parse a form yet, that is a tracked limitation, never a design position. Docker's +verdict binds only on the document, not the host: `env_file` existence, `${VAR:?}`, and a +negative on a top-level numeric key are facts about the machine that runs the script and are +deferred to it. `tests/conformance/` runs both oracles for real over a probe matrix generated +from `SERVICE_KEYS | STRUCTURAL_KEYS | IGNORED_SERVICE_KEYS`, so a new key is probed the moment +it is added. One residual is open by design: `depends_on` errors among services outside the +target's closure are accepted here and rejected by Docker +([#87](https://github.com/modern-python/compose2pod/issues/87)). diff --git a/docs/adr/0006-reject-parse-dont-validate.md b/docs/adr/0006-reject-parse-dont-validate.md deleted file mode 100644 index 76ba704..0000000 --- a/docs/adr/0006-reject-parse-dont-validate.md +++ /dev/null @@ -1,88 +0,0 @@ -# Reject parse-don't-validate for the validate -> emit seam - -**Decision:** Keep `validate(compose) -> warnings` and `emit_script(compose, -options) -> str` both taking the raw compose dict. Do not introduce a typed -`CheckedDocument` that `validate`/`parse` produces and `emit` consumes (the -"parse, don't validate" restructuring — Candidate 3 of the architecture review). - -## Context - -The architecture review proposed type-enforcing the seam: `validate` would -return a normalized, typed model that `emit` consumes, so the type system -guarantees `emit` only ever sees checked input, rather than relying on -`cli.py`'s call-order convention (`validate` then `emit_script`). It was flagged -Speculative at the time. - -Two of the review's candidates then shipped and changed the calculus: - -- The **service-key registry** single-sourced each - declarative key's validate + emit, so `emit` no longer re-derives shape - knowledge. -- **validate() owning every shape emit reads** was - believed, at the time this decision was first written, to make the - shape-reading functions robust enough that a direct `emit_script(dict)` - call on a malformed document would fail with `UnsupportedComposeError`, - not a raw crash. That belief was wrong — see the note below. - -**Correction, added when the gap this decision names was actually closed** -(the structural-key gate work, "Round 8"): the claim above was false when -written, and stayed false through seven further review rounds that each -hardened another shape-reading function and re-asserted it — because every -round hardened callers reached *through* `validate()`, and none checked -whether `validate()` itself was reachable from `emit_script`/ -`referenced_variables`'s own call graph. It was not: `emit_script` is -exported from `compose2pod`, `referenced_variables` is public as -`compose2pod.emit.referenced_variables`, and a library caller can call -either directly, and -doing so on a malformed document reached a raw `KeyError`/`TypeError`, or -worse, silently emitted a corrupted flag value (e.g. `--user "{'a': 1}"` -for `user: {a: 1}`) — identical to what this decision assumed was already -fixed. The gap was closed not by further hardening individual readers, but -by giving `emit._plan` — the single traversal both public entry points -project from — its own call to `validate(compose)`, discarding the returned -warnings (the CLI already prints its own copy from its own `validate()` -call). This is a *mechanism* difference from what this decision originally -described, not a reopening of it: see below. - -## Decision & rationale - -After those two changes — and now, after the Round 8 correction above — -parse-don't-validate's *unique* remaining benefit is narrow: preventing a -library caller from calling `emit_script` on a **valid-but-unvalidated** -dict (skipping warnings/normalization). That gap is: - -- **CLI-unreachable** — the only product entry point always calls `validate()` - before `emit_script()`. -- **Already de-risked for malformed input** — not because the shape-reading - functions are individually robust against every malformed input (that - claim was false, per the correction above), but because `emit._plan` - (`compose2pod/emit.py`) calls `validate(compose)` itself, before reading - anything else out of `compose`. Both public entry points that project - from `_plan` — `emit_script` and `referenced_variables` — are safe by - construction of that one call site, not by relying on `cli.py`'s - call-order convention or on every reader being individually hardened. - -Against that marginal gain, the cost is real: a typed `CheckedDocument` for the -whole ~30-key subset, rewriting the just-built, 100%-covered registry so its -specs produce/consume typed fields, and a **breaking** public-API change -(`validate` -> `parse`, `emit_script` signature). A thin "branded" wrapper -(`frozen CheckedDocument` holding the dict, constructible only via `parse`) -avoids the model rewrite but still carries the breaking API for a near-zero real -gain, since `emit` would still read the wrapped dict. - -Parse-don't-validate is the architecturally pure pattern, but it does not earn -its keep at this codebase's size and stage — it is the over-engineering the -review itself flagged, more so now that Candidates 1-2 shrank its payoff. This -also fits the zero-dependency, minimal-footprint ethos -([ADR-0002](0002-zero-dependency-core.md)). - -**Revisit trigger:** any of — - -- A **second `emit` consumer** appears — a distinct output format alongside the - pod script, or another module that renders from the compose model — so the - typed model would pay back across more than one consumer; or -- `emit._plan`'s own `validate()` call (added to close this decision's gap — - see the Round 8 correction above) is bypassed or removed, and a real caller - emits unvalidated input and ships wrong output as a result — not a - hypothetical convention violation, an actual regression in the enforced - invariant. diff --git a/docs/adr/0007-keep-graph-query-as-validator.md b/docs/adr/0007-keep-graph-query-as-validator.md deleted file mode 100644 index d880138..0000000 --- a/docs/adr/0007-keep-graph-query-as-validator.md +++ /dev/null @@ -1,49 +0,0 @@ -# Keep graph queries as validate-on-read; do not thread a shared graph - -**Decision:** Keep `depends_on`, `hostnames`, and `startup_order` -(`compose2pod/graph.py`) as query functions that normalize *and* raise on bad -shape, recomputed independently wherever they are called. Do not compute a -normalized graph once and thread it from `validate` into emit, and do not split -each query's normalization from its shape-validation. - -## Context - -Architecture-review candidate 4 (Speculative) flagged a "query-as-validator" -smell: `hostnames(services)` is called at the gate (`parsing.py:135`) purely to -trigger its shape-raise and its result discarded, then recomputed in `_plan` -for real use; `depends_on` normalizes-and-raises and is re-run at several sites -(inside `startup_order`, at the gate, and twice in `_plan`). Two fixes were -weighed: thread a normalized graph computed once and shared between `validate` -and emit; or split each query into a pure normalizer plus a separate validator. - -## Decision & rationale - -- **The gate and emit operate at different scopes, so a shared graph cannot - span them.** `validate(compose)` is target-agnostic; emit is target-scoped - (`_plan` calls `startup_order(services, target)`). The gate cannot compute the - dependency closure at all without a target, and cycle/unknown-dep detection is - inherently target-scoped. The gate does shape-validation; emit does - target-scoped assembly. That divide is inherent, not incidental. -- **The independent recompute is the accepted price of parse-don't-validate** - ([ADR-0006](0006-reject-parse-dont-validate.md)): `validate` and emit are - independent readers of the raw dict with no shared computed model. Threading a - normalized graph reintroduces exactly the coupling that decision declined. -- **Query-as-validator is the "validate owns emit shapes" pattern**: the gate calls - `hostnames`/`depends_on` to validate the shapes emit later reads. Splitting the - raise out of the query would leave emit's reader non-validating — a direct - `emit_script(malformed_dict)` would crash instead of raising - `UnsupportedComposeError`, regressing that robustness. -- **The recompute is cheap.** Normalizing `depends_on` and walking `hostnames` - are small in-memory passes over the service dict; the only non-test caller - (cli) runs once per process. -- **The one ADR-neutral change — deduping `depends_on` within `_plan` — is - marginal and partial** (`startup_order` normalizes independently regardless), - not worth the added state. - -**Revisit trigger:** any of — - -- the gate becomes **target-aware** (e.g. `validate` gains a target-scoped mode), - dissolving the scope mismatch that blocks a shared graph; or -- the graph traversal shows up as a **real hotspot in a profiled run** on large - compose documents — at which point the fix is memoizing or threading the - normalized graph *within emit*, still never a shared `validate`↔emit model. diff --git a/docs/adr/0008-reject-structural-key-registry.md b/docs/adr/0008-reject-structural-key-registry.md deleted file mode 100644 index 4783a7b..0000000 --- a/docs/adr/0008-reject-structural-key-registry.md +++ /dev/null @@ -1,91 +0,0 @@ -# Reject a structural-key registry; keep behavior in the owning modules - -**Decision:** Do not introduce a uniform structural-key registry (a table of -`emit(value, ctx)` callbacks covering `image`/`command`/`depends_on`/`dns`/ -`secrets`/`deploy`/...). Keep the split as it is: the `SERVICE_KEYS` registry -holds the keys that share one `emit(value) -> list[Token]` interface, and each -*structural* key's behavior stays in the module that owns its concern -(`emit.py`, `graph.py`, `pod.py`, `stores.py`, `resources.py`, -`healthcheck.py`), with `keys.STRUCTURAL_KEYS` as the gate's accept-list. - -## Context - -Architecture-review candidate 3 flagged a "split-brain": `SERVICE_KEYS` -single-sources validate+emit per key, but the 20 `STRUCTURAL_KEYS` are a bare -name set whose behavior lives across six modules, and the supported-key name -list is echoed again in `parsing.SUPPORTED_SERVICE_KEYS` and the 49-name -`test_keys.py` snapshot. Two fixes were on the table: (1) a uniform structural -registry with an `emit(value, ctx)` interface; (2) a narrow single-sourcing of -the key-*name* list from each owning module. - -## Decision & rationale - -- **Structural keys are heterogeneous — at least six distinct emit shapes**, so - they share no single interface: - - *slot-occupiers* — `image`/`build` (image token), `command`/`entrypoint` - (argv tokens): they don't produce `--flags` at all; - - *project_dir flag-producers* — `environment`/`env_file`, `volumes`/`tmpfs`; - - *healthcheck* — a sub-mapping → `--health-*`, also driving `wait_healthy`; - - *graph keys* — `depends_on`/`networks`/`hostname`/`container_name`: drive - ordering and pod-wide `--add-host`, not per-service flags; - - *pod-level aggregated* — `dns`/`dns_search`/`dns_opt`/`sysctls`: unioned - across all services onto `podman pod create`; - - *document-scoped* — `secrets`/`configs`/`deploy`: need compose defs + - closure order + `project_dir`, and emit create/teardown lines too. -- `SERVICE_KEYS` is a real registry precisely because its ~29 keys all fit one - shape ("two adapters = a real seam"). A structural registry spanning the six - shapes above would need `Any`-typed / variadic callbacks — **a false seam**. -- It would also **scatter cohesive logic** away from its concern: `depends_on` - belongs with the graph, `dns` with the pod, `secrets` with stores, `deploy` - with resources. Centralizing them behind a dispatcher trades locality for a - lookup table — the opposite of a deep module. -- `STRUCTURAL_KEYS` holds **no behavior** (deletion test): delete it and only - the gate's accept-list and the snapshot test break, never an emit path. The - sole duplicated knowledge is the key-*name* list. -- **The narrow single-sourcing (option 2) was also weighed and declined now.** - Deriving the accept-list from per-module declared key sets would distribute - six small sets plus import edges to remove a minor duplication that the - snapshot + disjoint tests already guard loudly (no silent-bug risk). Not worth - it at this size and stage. - -## Reopened 2026-07-15 — reaffirmed - -Reopened at the maintainer's request during an architecture review. The registry -question was handed to **two independent reviewers, each blind to this decision** -— one mandated to build the strongest case *for* a structural-key registry, one -*against*. They converged on this decision's holding: a universal registry is a -false seam, and `secrets`/`configs` (`stores.py`), `dns`/`sysctls`/`extra_hosts` -(`pod.py`), `deploy` (`resources.py`), `depends_on`/`networks`/`hostname`/ -`container_name` (`graph.py`), and the positional `command`/`entrypoint`/`image` -argv keys all belong in their owning module. The clinching disproof of the -universal case: a single `entrypoint` value emits tokens on *both sides* of the -`image` token and a string `entrypoint` silently cancels `command` -(`emit.py:199-207`) — no `emit(value) -> list[Token]` slot can express that. - -The reopen surfaced exactly one crack, and it does **not** reach this rejection. -Four per-service keys have no owning noun — `environment`, `env_file`, `volumes`, -`tmpfs` — and of those, `environment` is provably `_map("-e")` (same -`validate_map`, same `["-e", Expand(str(pair))]` emit loop as `labels`/ -`annotations`, same `pairs_to_mapping` map-merge), fitting the *existing* -`emit(value)` signature at zero interface cost. `tmpfs` is nearly `_list("--tmpfs")`. -But `env_file`/`volumes` need `project_dir`, which would force widening -`KeySpec.emit` to `emit(value, ctx)` across all ~29 current keys to house two — -a real cost this decision still declines. So the strongest "reopen" collapses -from *"structural-key registry"* all the way down to *"optionally move -`environment` (± `tmpfs`) into `SERVICE_KEYS`"*, which is this decision's own -narrow revisit-trigger #2, not an overturn of it. That narrow move was examined -and left unscheduled (same category as the option-2 single-sourcing declined -above); it is recorded here so a third review does not re-derive it from scratch. - -**Revisit trigger:** reopen — and when reopening, reach for the **narrow name -single-sourcing**, not a registry — if either holds: - -- a **third reader** of the supported-key name set appears beyond the parsing - gate and the owning-module handlers (e.g. a `--list-supported-keys` feature, - or docs generated from the key set), so single-sourcing the list pays back - across more than the gate; or -- **several new structural keys arrive that share one new uniform shape** (e.g. - a cluster of new pod-level aggregated keys), at which point a **narrow - sub-registry for that one shape** — a `SERVICE_KEYS`-analog for the new emit - signature — is warranted, never a universal structural registry spanning all - shapes. diff --git a/docs/adr/0009-docker-rejection-parity.md b/docs/adr/0009-docker-rejection-parity.md deleted file mode 100644 index f14f1ce..0000000 --- a/docs/adr/0009-docker-rejection-parity.md +++ /dev/null @@ -1,136 +0,0 @@ -# Docker's refusals bind; podman decides what we can accept - -**Decision:** two rules, in one direction each. - -- **Docker rejects ⇒ compose2pod rejects.** Hard, no exceptions. -- **Docker accepts ⇒ compose2pod accepts, when podman can express it and it - means something inside a pod.** Where it cannot yet, that is a **current - limitation** — a deferred piece of the subset, tracked as a GitHub issue — - not a bug, and not a licence to refuse on taste. - -The rule binds only on what the *document* says, never on the host it is read -from. - -## Context - -The rule was already the project's working belief, but it had never been -written down, and the two places that stated it disagreed: - -- The subset write-up — "Parity on *refusal* is what the drop-in role demands; - it is not parity for its own sake, and the package keeps its documented - divergences elsewhere." -- The YAML-1.1-booleans change — "Refusing a file Docker runs is the one - direction that must never happen." - -Read literally the second forbids the honest subset, which refuses -`network_mode`, long-form volumes and `1h30m` healthcheck intervals — all -files Docker runs. Neither statement was wrong; neither was precise. - -Nothing checked either of them. Five consecutive changes each hand-found a -single divergence and fixed it, which is what a missing invariant looks like -from the outside. A measured sweep then found **113 more** across 672 probes. - -## Decision & rationale - -**The hard rule (soundness).** `accepted(compose2pod) ⊆ accepted(docker)`. -compose2pod is a drop-in replacement for `docker compose` on rootless runners: -the file it converts is the file the developer runs locally. Accepting a document -Docker refuses emits a script for a file that is already broken upstream, turning -a hard error into a green CI run. That false green is the single failure the gate -exists to prevent. This is the direction that must never regress, and the -conformance harness (`tests/conformance/`) exists to keep it from doing so. - -**What the rule covers, and its one known residual.** The audit was explicit -that its 113 measured divergences were "a floor, not a total" — a single-key matrix cannot -reach every nested or cross-document position. The closure work took that floor -down to zero across every position the harness and a series of deep controller -sweeps could reach: the value grammars, the ignored and -structural keys, `deploy.resources`, healthcheck scalars, `build` values, the -`ports`/`networks`/`extra_hosts`/secret long-form and nested schemas, the -top-level `networks`/`volumes` definitions, `depends_on` long-form, top-level -scalar-key types, and named-volume references. The value-grammar surface is -soundness-complete — an adversarial review of ~60 hostile grammar probes found -nothing. - -**One residual is left open by design, not oversight:** the **non-target -dependency graph**. `docker compose config` validates `depends_on` existence and -cycles across the *whole document*; compose2pod validates only the **target's -dependency closure** (`startup_order`), because a service outside the closure -never joins the pod and never runs — the same closure-scoping that governs -add-host, stores, and pod options. So a -`depends_on` naming a missing service, or a dependency cycle, *among services the -target never reaches* is accepted here and rejected by Docker. This is the honest -boundary between "reject every broken document" and "don't validate what never -runs." It is catalogued as -modern-python/compose2pod#87 with a revisit trigger (a document-wide -pre-validation pass, independent of the closure, would close it), -and it is the single known place the hard rule does not hold — named here rather -than papered over. - -**The second rule: podman decides, not documentation.** An earlier draft of this -decision said an over-rejection was fine "as long as it is declared". That test -was worthless — it is a standard met by typing a sentence, and it let taste -masquerade as design. The test is **podman**: - -- **Legitimate refusal — the capability cannot work.** `network_mode` (every - service shares the pod's namespace), per-service `dns` (one `/etc/resolv.conf` - per pod), `stop_signal` (the script force-removes the pod and never stops a - container gracefully), `sysctls: ["a"]` (no `=`, so there is no value to put in - a `--sysctl` flag). These stay refused, permanently, and the reason is podman's, - not ours. -- **Not a licence to refuse a *form*.** Where compose2pod supports the - capability, an unusual spelling of it that podman can honor must be accepted. - A quoted boolean (`tty: "true"` — podman only ever sees `--tty` or nothing) and - a compound duration (`interval: 1h30m` — 5400s, and the value only paces a - polling loop) are forms, not capabilities. - -**An over-rejection is a limitation, not a bug.** Where we refuse a form podman -could express, we are behind, not wrong: it is a deferred part of the subset, -recorded as a GitHub issue with a revisit trigger, and workable later — the live -instance is modern-python/compose2pod#86. -This distinction is what keeps the rule bounded — it forbids refusing a spelling -on taste, without obliging compose2pod to implement every Compose feature podman -happens to support. A key outside the subset entirely is still refused loudly; -that is the honest subset, and it is unaffected. - -**The carve-out: the document, not the host.** Docker's verdict binds only when -it is a property of the document alone. `docker compose config` also rejects on -host state — `env_file: app.env` fails with `env file not found` when the file -is absent, and `${VAR:?msg}` fails when `VAR` is unset in the *reading* shell. -compose2pod generates a script that runs somewhere else, where that file is -checked out and that variable is set; deferring both to script-run time is -deliberate. -Docker's rejection there is a fact about the developer's laptop, not about the -document, so it cannot bind — and a harness that enforced it would demand -checks that are actively wrong. - -**Enforcement is executable, not asserted.** A hand-measured table is a claim -typed into a file: it cannot catch a construct nobody thought to enumerate, -which is precisely how the last five divergences survived. `tests/conformance/` -instead runs both oracles for real, and generates its probe matrix from -`SERVICE_KEYS | STRUCTURAL_KEYS | IGNORED_SERVICE_KEYS` — so **a new key in the -registry is probed the moment it is added**, and the rule cannot decay as the -subset grows. `docker compose config` needs no daemon, so the harness is a -plain CI job. - -**Revisit trigger:** any of — - -- **Docker's own validation changes** such that a construct compose2pod - correctly accepts starts being refused by `docker compose config` — the - harness will fail, and the question becomes which version of Compose the - invariant tracks (it currently tracks whatever the CI runner ships). -- **The carve-out grows.** If a third class of host-dependent rejection appears - beyond `env_file` existence and `${VAR:?}` interpolation, the "document, not - the host" line is doing more work than one sentence can carry and needs its - own rule. -- **A grammar validator drifts from Docker's** — the port or size grammar - starts refusing a value Docker accepts — turning the soundness fix into an - over-rejection. The harness catches this too, in the other direction. -- **A refusal is justified by "we don't parse that yet" rather than by podman.** - That is the failure mode the second rule exists to catch: the refusal is a - limitation to be tracked and worked off, not a design position to be defended. -- **A user hits the non-target dependency-graph residual** — a real document - whose `depends_on` names a missing service, or forms a cycle, outside the - target's closure, and who is surprised compose2pod ran it green. That is the - signal to add the document-wide `depends_on` existence + cycle pass and retire - the residual. diff --git a/docs/adr/0010-reject-strict-schema-validator-unification.md b/docs/adr/0010-reject-strict-schema-validator-unification.md deleted file mode 100644 index 7d6f54a..0000000 --- a/docs/adr/0010-reject-strict-schema-validator-unification.md +++ /dev/null @@ -1,82 +0,0 @@ -# Reject a unified strict-schema validator - -**Decision:** Do not introduce a shared `validate_schema(where, mapping, fields)` -helper to unify the ~14 hand-written "known-key set, reject unknown, run each -present key's grammar" sites across `parsing.py`, `values.py`, `stores.py`, -`resources.py`, and `graph.py`. Keep each block's validator as it is. The only -shared shape that earns its keep — the top-level `networks`/`volumes` definition -pair — is already factored into `parsing._validate_top_level_definition`. - -## Context - -Architecture-review candidate 4 flagged that the strict-schema pattern is -re-implemented five-plus times: `_validate_network_entry_value` -(`_DOCKER_NETWORK_ENTRY_KEYS`), `_validate_top_level_definition` -(`_NETWORK_DEFINITION_KEYS`/`_VOLUME_DEFINITION_KEYS`), `_validate_port_long_form` -(`_PORT_LONG_FORM_KEYS`), the two ipam sub-schemas, `stores._ref_source` -(`_LONG_FORM_KEYS`), `_validate_build` (`_DOCKER_BUILD_KEYS`), the deploy blocks, -and `graph._depends_on_entry_condition`. The proposed deepening was one -`validate_schema(where, mapping, fields)` deep module every caller feeds its own -field table. - -## Decision & rationale - -The candidate was walked down its design tree (grilling) against the actual code. -The unifiable core is genuinely small — `unknown = ; if unknown: raise -f"{where}: unsupported keys {sorted(...)}"`, sometimes preceded by -`require_string_keys` — 2 to 4 lines. Everything wrapped around it diverges per -site, so a single signature is a **false seam** that would need a flag for every -divergence: - -- **`x-` extension policy is not uniform.** Only 4 sites skip `x-` keys (build, - the per-service network entry, `depends_on`, and the top-level definition); the - other ~8 (both ipam sub-schemas, both store schemas, port long-form, the - `external` map, build-secrets, the four deploy blocks) deliberately do **not** — - `x-` is not legal there. A shared helper cannot hold a fixed `x-` policy; it - would have to take `allow_extensions` per call. This divergence alone forecloses - one signature. -- **Two non-string-key idioms with different messages.** The comprehension form - runs `require_string_keys` first (precise `"key ... must be a string"`); the - bare `set(m) - KEYS` form lets a non-string key fall into `"unsupported keys"`. - Unifying would change one site's error text or the other's. -- **Pre-check shape differs:** null-or-dict (definitions, network entry) vs - str-or-dict (build, store ref) vs dict-only (ipam, port). -- **Required keys are bespoke and site-specific:** port needs `target`, store ref - needs `source`, `depends_on` needs `condition` — each checked around the unknown - check, not by it. -- **Field-dispatch context differs:** definition validators receive `ident` with - the label closured in (`_validate_definition_string("network")`); network-entry - validators receive the service `name` and drop the network name from their - messages. Same loop, different error text. -- **Nesting:** ipam's `config` is a list of per-subnet sub-schemas; the rest are - flat. - -To span all of that, `validate_schema` would carry `allow_null`, `allow_str`, -`allow_extensions`, `required_keys`, a message prefix *and* a separate field -context — a wide, flag-laden interface whose body is still a 3-line loop. That is -the same over-abstraction-across-divergent-shapes error already rejected for the -structural keys ([ADR-0008](0008-reject-structural-key-registry.md)), at -smaller scale. Depth here is illusory: the interface would be nearly as complex as -the implementation. - -`_validate_top_level_definition` is the right amount of sharing — it unifies the -two cases (network and volume definitions) that match shape exactly (null-or-dict, -`x-` skip, `(ident, key, value)` field dispatch, identical message frame). Pushing -past those two trades locality for a lookup table. - -A narrower `reject_unknown_keys(where, mapping, allowed, *, allow_extensions)` -helper — mirroring the existing `require_string_keys` — was weighed and also -declined: it would concentrate only the message format and the `x-` flag across -~12 one-line call sites, a modest churn for a check the per-site tests already -guard, and the two non-string-key idioms would still have to be reconciled first. -Not worth it at this size and stage — the same call -[ADR-0008](0008-reject-structural-key-registry.md) made for its own narrow -single-sourcing. - -**Revisit trigger:** reopen if **three or more new strict-schema blocks arrive -that share one exact shape** — same pre-check, same `x-` policy, same -field-dispatch convention, same message frame — at which point a narrow helper -for *that one shape* (a -`_validate_top_level_definition` analog), not a universal `validate_schema`, is -warranted. A new block that merely resembles an existing one at the 3-line-loop -level is not a trigger. diff --git a/docs/adr/0011-list-of-str-refusals.md b/docs/adr/0011-list-of-str-refusals.md deleted file mode 100644 index c1a7714..0000000 --- a/docs/adr/0011-list-of-str-refusals.md +++ /dev/null @@ -1,41 +0,0 @@ -# list-of-str sysctls/volumes entries are legitimate refusals - -**Decision:** compose2pod refuses `sysctls: ["a"]` (a list entry with no `=`) and -`volumes: ["a"]` (a colon-less relative entry) even though `docker compose config` -accepts both, because podman cannot form the corresponding flag. These are -legitimate refusals under [ADR-0009](0009-docker-rejection-parity.md) rule two, -not unfinished parsers to be closed later. - -## Context - -The conformance harness reports both as `over-reject` (Docker accepts, compose2pod -refuses). The deferred list had catalogued `volumes: ["a"]` as "worth re-measuring … -legitimate refusal or unfinished form?". Re-measured against `docker compose config` -v5.1.2 and podman 6.0.1: - -- **`sysctls: ["a"]`** — Docker normalizes it to `{a: ""}` (the sysctl `a` set to - the empty string). The equivalent podman flag is refused: `--sysctl a=` → - `sysctl 'a' is not allowed`; `--sysctl a` → `sysctl values must be in the form - of KEY=VALUE`. compose2pod accepts the useful `sysctls: ["key=value"]` list form - (`pod._sysctl_pairs`); only the valueless entry is refused. -- **`volumes: ["a"]`** — Docker normalizes it to an anonymous volume with the - *relative* target `a`. podman refuses a relative mount target: both `-v a` and - `--mount type=volume,target=a` → `invalid container path "a", must be an - absolute path` (an absolute target such as `/a` is accepted). compose2pod's own - error — "anonymous volume 'a' must be an absolute path" — mirrors podman's - constraint exactly. - -## Decision & rationale - -Refuse both, permanently. Rule two accepts a form only when podman can express it -and it means something in a pod; here podman rejects the flag outright, so -accepting the form would emit a script that cannot run. The rejected alternative — -accept at the gate and let podman fail at run time — trades a clean generate-time -refusal for an opaque runtime crash, which the docker-rejection-parity design -explicitly avoids. Neither is an unfinished parser: the useful forms -(`sysctls: ["k=v"]`, absolute anonymous volumes) are already supported; only the -inexpressible edge is refused. - -**Revisit trigger:** a future podman accepts a relative mount target, or a -valueless / bare-`KEY` sysctl. Then the corresponding form becomes expressible -and this decision reopens. diff --git a/docs/adr/0012-negative-numeric-values-deferred-to-runtime.md b/docs/adr/0012-negative-numeric-values-deferred-to-runtime.md deleted file mode 100644 index a8fab38..0000000 --- a/docs/adr/0012-negative-numeric-values-deferred-to-runtime.md +++ /dev/null @@ -1,42 +0,0 @@ -# Negative numeric values are config-accepted and deferred to runtime - -**Decision:** compose2pod does **not** refuse a negative native number on the -top-level numeric keys. `docker compose config` v5.1.2 accepts one there and -defers the negative to run time; compose2pod matches, per the config-level parity -rule. Only the volume mount sub-schema (`tmpfs.size`/`tmpfs.mode`) is -config-validated as unsigned and refuses a negative. - -## Context - -The final review of the nested-volume-options work found that -`tmpfs.size: -5` / `tmpfs.mode: -1` were a false green (docker rejects the -unsigned mount fields; compose2pod accepted) and suggested the same might hold -for every `values.validate_size` caller. Measured against `docker compose config` -v5.1.2: - -- **Top-level keys ACCEPT a negative native** — `mem_limit: -5`, `cpus: -1.5`, - `cpu_shares: -5`, `cpu_quota`/`cpu_period`/`pids_limit: -5`, `oom_score_adj: - -5`, `ulimits.nofile: -5`, `mem_reservation`/`memswap_limit`/`mem_swappiness`/ - `shm_size: -5` — all accepted. compose2pod accepts them too → both-accept, no - violation. (`memswap_limit: -1` and `oom_score_adj` are even legitimately - signed.) -- **The tmpfs mount sub-schema REJECTS a negative** — `{type: tmpfs, tmpfs: - {size: -5}}` / `{mode: -1}` raise (`overflows uint`). This is a *config-level* - refusal, so accepting it was a genuine hard-rule false green — since fixed. - -## Decision & rationale - -Leave the top-level keys as-is. Refusing a negative there would **introduce** -over-rejections: compose2pod would reject a document `docker compose config` -accepts, diverging from Docker for a purely run-time concern the project already -defers (like env-file existence or a `${VAR}`'s host value). The hard rule -`accepted(compose2pod) ⊆ accepted(docker)` -([ADR-0009](0009-docker-rejection-parity.md)) is already satisfied — a -negative native is inside Docker's config-accept set. The rejected alternative — a -rule-two "refuse anything podman can't run" stance for negatives — is inconsistent -with the config-parity + defer-runtime line the whole subset draws, and would -special-case negatives among the many run-time-invalid-but-config-valid values. - -**Revisit trigger:** `docker compose config` starts rejecting a negative on a -top-level numeric key (i.e. the negative becomes a config-level, not run-time, -error) — then matching it would be a parity fix, not an over-rejection. diff --git a/docs/adr/0013-volumes-stays-hand-rolled.md b/docs/adr/0013-volumes-stays-hand-rolled.md deleted file mode 100644 index c40971c..0000000 --- a/docs/adr/0013-volumes-stays-hand-rolled.md +++ /dev/null @@ -1,27 +0,0 @@ -# volumes stays hand-rolled, outside the service-key registry - -**Decision:** `tmpfs` moved into `SERVICE_KEYS`; `volumes` did not, and stays hand-rolled. - -## Context - -The registry-unification refactor considered moving both `volumes` and `tmpfs`. -`tmpfs` is a uniform scalar-or-list flag key and fits cleanly. `volumes` does not: -its emit (`emit._volume_flags`/`_mount_flag`) needs `project_dir` (relative bind -resolution), which `KeySpec.emit(value)` cannot supply without widening the -signature for all ~30 keys; and its validation is partly document-level -(`_validate_volume_references` cross-checks named-volume sources against the -top-level `volumes:` block), which cannot live in a per-service -`KeySpec.validate(name, key, value)`. - -## Decision & rationale - -Leave `volumes` hand-rolled. A registry `volumes` would be a thin `KeySpec` -wrapper around three still-custom, still-split pieces (custom validate, custom -project_dir emit, separate document-level reference pass), bought by degrading -the clean `emit(value) -> tokens` interface for the 28 keys that do not need -`project_dir`. The common case would pay for the uncommon one. The long-form -`--mount` work made `volumes` an even worse fit. - -**Revisit trigger:** `KeySpec.emit` is widened to carry a context -(`project_dir`) for another, independent reason — then moving -`volumes`/`env_file` in costs nothing extra and should be reconsidered. diff --git a/tests/conformance/conftest.py b/tests/conformance/conftest.py index 0ff5e49..26afc01 100644 --- a/tests/conformance/conftest.py +++ b/tests/conformance/conftest.py @@ -1,6 +1,6 @@ """Conformance harness: compose2pod must refuse every document `docker compose config` refuses. -The rule is one-way (docs/adr/0009-docker-rejection-parity.md): +The rule is one-way (docs/adr/0006-docker-rejection-parity.md): Docker rejecting a document binds; Docker accepting one does not oblige us to, because compose2pod converts an honest subset. diff --git a/tests/test_cli.py b/tests/test_cli.py index 33e09e1..94d55a7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -358,7 +358,7 @@ class TestYaml12Floats: not cosmetic: `cpuset: 1e3` is a *string* to compose2pod today, so it slides past the "cpuset must be a string" rule Docker enforces on the float 1000.0 -- a false green, the one thing the rejection-parity gate - exists to prevent (`docs/adr/0009-docker-rejection-parity.md`). + exists to prevent (`docs/adr/0006-docker-rejection-parity.md`). """ def test_bare_exponent_loads_as_float(self) -> None: diff --git a/tests/test_values.py b/tests/test_values.py index 9e63f6e..f3edc14 100644 --- a/tests/test_values.py +++ b/tests/test_values.py @@ -155,7 +155,7 @@ def test_every_value_grammar_in_values_py_ends_at_the_true_end_of_string() -> No grammar silently accepts a value carrying one -- reachable from any YAML block scalar (`mem_limit: |` resolves to `"512m\n"`) -- and `docker compose config` refuses that value. Accepting it is a false green - against the hard rule in `docs/adr/0009-docker-rejection-parity.md`, and it + against the hard rule in `docs/adr/0006-docker-rejection-parity.md`, and it is invisible from the call site: every hand-written probe passes a newline-free value, which is exactly how the gap survived until it was measured. Asserting the anchor over the whole module, rather than per