diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 0d7d442..75ed00d 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -21,7 +21,8 @@ warns (ignored, behavior-neutral inside a single pod) or raises - **Supported:** `image`, `build`, `command`, `entrypoint`, `environment`, `env_file`, `volumes`, `healthcheck`, `depends_on`, `networks`, `hostname`, - `container_name`, `tmpfs`, `user`, `working_dir`, `group_add`, `labels`. + `container_name`, `tmpfs`, `user`, `working_dir`, `group_add`, `labels`, + `read_only`, `init`, `privileged`, `cap_add`, `cap_drop`, `security_opt`. compose2pod never builds: a `build` section is accepted but its contents (context, dockerfile, args) are not read — `image_for` (`compose2pod/emit.py`) runs the CI image supplied via `--image` for any service that has one. @@ -43,6 +44,12 @@ warns (ignored, behavior-neutral inside a single pod) or raises if the override needs to actually run. - **`user` / `working_dir`:** strings, emitted verbatim as `--user` / `--workdir`. - **`group_add`:** a list, emitted as repeated `--group-add`. +- **`read_only` / `init` / `privileged`:** booleans, emitted as the bare + `--read-only` / `--init` / `--privileged` flag only when true (nothing when + false or absent). +- **`cap_add` / `cap_drop` / `security_opt`:** lists, emitted as repeated + `--cap-add` / `--cap-drop` / `--security-opt`. Item contents pass through + verbatim (no content validation), like `tmpfs` and named volumes. - **`labels`:** list (`- KEY=value` / `- KEY`) or mapping (`KEY: value` / `KEY:`), emitted as repeated `--label`. A null value means an empty label (`--label KEY`) -- the same emitted shape as `environment`'s null but a @@ -61,8 +68,11 @@ warns (ignored, behavior-neutral inside a single pod) or raises (used internally for `podman cp`, healthcheck polling, and target-container diagnostics) — only name *resolution* is meaningful to other services, and no per-container `--hostname` or renamed `--name` is emitted. -- **Ignored (warns):** `ports`, `restart`, `stdin_open`, `tty` — meaningless - or irrelevant inside a single shared-namespace pod. +- **Ignored (warns):** `ports`, `restart`, `stdin_open`, `tty`, `stop_signal`, + `stop_grace_period` — meaningless or irrelevant inside a single + shared-namespace pod. `stop_signal`/`stop_grace_period` are inert because the + script force-removes the pod (`podman pod rm -f`) and never gracefully stops a + container. - **Extension fields:** any `x-`-prefixed service key is accepted and ignored silently. - Everything else raises. diff --git a/compose2pod/emit.py b/compose2pod/emit.py index d966b88..c301d07 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -14,7 +14,14 @@ _SCALAR_FLAGS: dict[str, str] = {"user": "--user", "working_dir": "--workdir"} -_LIST_FLAGS: dict[str, str] = {"group_add": "--group-add"} +_BOOL_FLAGS: dict[str, str] = {"init": "--init", "read_only": "--read-only", "privileged": "--privileged"} + +_LIST_FLAGS: dict[str, str] = { + "group_add": "--group-add", + "cap_add": "--cap-add", + "cap_drop": "--cap-drop", + "security_opt": "--security-opt", +} @dataclasses.dataclass(frozen=True) @@ -112,10 +119,13 @@ def _add_volume_flags(flags: list[Token], svc: dict[str, Any], project_dir: str) def _add_declarative_flags(flags: list[Token], svc: dict[str, Any]) -> None: - """Add the scalar-, list-, and label-driven flags to the flags list.""" + """Add the scalar-, boolean-, list-, and label-driven flags to the flags list.""" for key, flag in _SCALAR_FLAGS.items(): if key in svc: flags += [flag, _Expand(str(svc[key]))] + for key, flag in _BOOL_FLAGS.items(): + if svc.get(key): + flags.append(flag) for key, flag in _LIST_FLAGS.items(): for item in svc.get(key) or []: flags += [flag, _Expand(str(item))] diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 2113e89..d1c6b54 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -25,8 +25,14 @@ "working_dir", "group_add", "labels", + "read_only", + "init", + "privileged", + "cap_add", + "cap_drop", + "security_opt", } -IGNORED_SERVICE_KEYS = {"ports", "restart", "stdin_open", "tty"} +IGNORED_SERVICE_KEYS = {"ports", "restart", "stdin_open", "tty", "stop_signal", "stop_grace_period"} SUPPORTED_HEALTHCHECK_KEYS = {"test", "interval", "timeout", "retries", "start_period"} SUPPORTED_TOP_LEVEL_KEYS = {"services", "version", "name", "networks", "volumes"} DEPENDS_ON_CONDITIONS = {"service_started", "service_healthy", "service_completed_successfully"} @@ -65,15 +71,20 @@ def _validate_service_forms(name: str, svc: dict[str, Any]) -> None: if key in svc and not isinstance(svc[key], str): msg = f"service {name!r}: '{key}' must be a string" raise UnsupportedComposeError(msg) - if "group_add" in svc and not isinstance(svc["group_add"], list): - msg = f"service {name!r}: 'group_add' must be a list" - raise UnsupportedComposeError(msg) + for key in ("group_add", "cap_add", "cap_drop", "security_opt"): + if key in svc and not isinstance(svc[key], list): + msg = f"service {name!r}: '{key}' must be a list" + raise UnsupportedComposeError(msg) if "labels" in svc and not isinstance(svc["labels"], list | dict): msg = f"service {name!r}: 'labels' must be a list or mapping" raise UnsupportedComposeError(msg) if "entrypoint" in svc and not isinstance(svc["entrypoint"], str | list): msg = f"service {name!r}: 'entrypoint' must be a string or list" raise UnsupportedComposeError(msg) + for key in ("read_only", "init", "privileged"): + if key in svc and not isinstance(svc[key], bool): + msg = f"service {name!r}: '{key}' must be a boolean" + raise UnsupportedComposeError(msg) def _validate_service(name: str, svc: dict[str, Any]) -> list[str]: diff --git a/planning/audits/2026-07-09-compose-spec-coverage.md b/planning/audits/2026-07-09-compose-spec-coverage.md index adddb40..32292d0 100644 --- a/planning/audits/2026-07-09-compose-spec-coverage.md +++ b/planning/audits/2026-07-09-compose-spec-coverage.md @@ -43,8 +43,6 @@ network namespace, so none conflicts with the pod. | `extra_hosts` | `--add-host` | Merges into the existing add-host set | | `init` | `--init` | boolean | | `read_only` | `--read-only` | boolean | -| `stop_signal` | `--stop-signal` | Direct | -| `stop_grace_period` | `--stop-timeout` | duration parse | | `cap_add` / `cap_drop` | `--cap-add` / `--cap-drop` | lists | | `privileged` | `--privileged` | boolean | | `security_opt` | `--security-opt` | list | @@ -56,6 +54,15 @@ network namespace, so none conflicts with the pod. | `annotations` | `--annotation` | Direct | | `sysctls` | `--sysctl` | net-namespace sysctls are pod-level; non-net ones are per-container | +**Shipped:** `entrypoint`, `user`, `working_dir`, `labels`, `group_add` +(`changes/2026-07-09.03`); `read_only`, `init`, `privileged`, `cap_add`, +`cap_drop`, `security_opt` (`changes/2026-07-09.04`). + +**Moved out:** `stop_signal`/`stop_grace_period` were listed here but are inert +under the `podman pod rm -f` force teardown (no graceful `podman stop` is ever +emitted), so they are reclassified accepted-but-inert — see +`decisions/2026-07-09-stop-lifecycle-keys-inert.md`. + ## Bucket B — meaningful, needs translation or pod-wide reconciliation - **Resource limits.** Modern `deploy.resources.limits/reservations` plus legacy diff --git a/planning/changes/2026-07-09.04-container-confinement-service-keys.md b/planning/changes/2026-07-09.04-container-confinement-service-keys.md new file mode 100644 index 0000000..47ae8c7 --- /dev/null +++ b/planning/changes/2026-07-09.04-container-confinement-service-keys.md @@ -0,0 +1,85 @@ +--- +summary: Support the container-confinement service keys read_only, init, privileged, cap_add, cap_drop, security_opt; emit them as podman run flags via a new boolean-flag table and the existing list-flag table. +--- + +# Design: Container confinement service keys + +## Summary + +Add six Compose service keys that control how a container is sandboxed: +`read_only`, `init`, `privileged` (booleans) and `cap_add`, `cap_drop`, +`security_opt` (lists). Each is an invariant-preserving per-container `podman +run` flag (Bucket A of `audits/2026-07-09-compose-spec-coverage.md`). This is +the second Bucket A bundle, after +`changes/2026-07-09.03-core-process-identity-service-keys.md`, and it introduces +one new emission mechanism (a boolean-flag table) while reusing the existing +list-flag table. + +## Motivation + +These are the common keys for confining a CI container: dropping capabilities, +running read-only, reaping zombies via an init, or (rarely) going privileged. +All map to plain podman flags with no pod-level conflict, so refusing them is a +gap rather than an honest boundary. + +## Design + +**Emission (`emit.py`, in `_add_declarative_flags`).** A new table drives the +booleans; the bare flag is emitted only when the value is truthy: + +```python +_BOOL_FLAGS = {"init": "--init", "read_only": "--read-only", "privileged": "--privileged"} +... +for key, flag in _BOOL_FLAGS.items(): + if svc.get(key): + flags.append(flag) +``` + +The three list keys are added to the existing `_LIST_FLAGS` +(`cap_add` -> `--cap-add`, `cap_drop` -> `--cap-drop`, `security_opt` -> +`--security-opt`); the existing loop emits them as repeated flags with each item +wrapped in `_Expand`, so `${VAR}` interpolation and the `referenced_variables()` +stderr note cover them for free -- identical to `group_add`. Boolean flags are +single plain tokens (no value, no interpolation). + +**Validation (`parsing.py`, `_validate_service_forms`).** Add the six keys to +`SUPPORTED_SERVICE_KEYS`; require `read_only`/`init`/`privileged` to be `bool` +and `cap_add`/`cap_drop`/`security_opt` to be lists, rejecting other forms +loudly in the existing style. `security_opt`/`cap_*` item *contents* pass +through verbatim (no content validation), consistent with `tmpfs` and named +volumes. + +**Test retarget.** `tests/test_parsing.py::test_unsupported_service_key_raises` +currently uses `privileged` as its example of a rejected key. Since `privileged` +becomes supported, retarget that test to a still-unsupported key -- +`network_mode` (a permanent-reject key per +`decisions/2026-07-09-reject-namespace-network-keys.md`, so it stays stable). + +**Promote** the six keys into `architecture/supported-subset.md` in the same PR. + +## Non-goals + +- `stop_signal`/`stop_grace_period` -- reclassified out of Bucket A as inert + under the force teardown; see + `decisions/2026-07-09-stop-lifecycle-keys-inert.md`. +- Remaining Bucket A keys (`extra_hosts`, `devices`, `pull_policy`, `platform`, + `ulimits`, `annotations`, `sysctls`) -- later bundles. +- Content validation of `security_opt`/`cap_*` values -- passed through verbatim. + +## Testing + +`just test-ci` at 100% line coverage. Per key: happy-path flag emission +(booleans emit only when true; the three list keys emit repeated flags); +form rejection (`UnsupportedComposeError` for a non-bool boolean, a non-list +list); `${VAR}` interpolation on a `cap_add`/`security_opt` item reaching both +the emitted fragment and the `referenced_variables()` note. Plus the retargeted +`test_unsupported_service_key_raises`. `just lint-ci` clean. + +## Risk + +- **Boolean truthiness vs. form** (low x low): a stray non-bool (`read_only: + "false"`, a string) is truthy and would wrongly emit `--read-only`; mitigated + by the `bool` form check in validation, which rejects it before emission. +- **`test_unsupported_service_key_raises` retarget** (certain x trivial): a + known required test edit, called out above so it is not mistaken for a + regression. diff --git a/planning/changes/2026-07-09.05-ignore-stop-lifecycle-keys.md b/planning/changes/2026-07-09.05-ignore-stop-lifecycle-keys.md new file mode 100644 index 0000000..a832720 --- /dev/null +++ b/planning/changes/2026-07-09.05-ignore-stop-lifecycle-keys.md @@ -0,0 +1,41 @@ +--- +summary: Add stop_signal and stop_grace_period to IGNORED_SERVICE_KEYS so they are accepted with a warning instead of rejected, matching their accepted-but-inert classification. +--- + +# Change: Ignore stop_signal / stop_grace_period with a warning + +**Lane:** lightweight — one set addition, one test, doc promotion. + +## Goal + +`stop_signal` and `stop_grace_period` are inert under the pod's force teardown +(`decisions/2026-07-09-stop-lifecycle-keys-inert.md`), but the gate currently +*rejects* them (they are in neither `SUPPORTED_SERVICE_KEYS` nor +`IGNORED_SERVICE_KEYS`, so `validate()` raises `unsupported key`). Rejecting a +valid-but-behavior-neutral key is stricter than the tool's norm. Move them into +the warn-and-ignore set alongside `ports`/`restart`/`stdin_open`/`tty`. + +## Approach + +Add `"stop_signal"` and `"stop_grace_period"` to `IGNORED_SERVICE_KEYS` +(`parsing.py`). The existing `_validate_service` loop then emits +`service '': ignoring ''` for them and does not raise. Promote into +`architecture/supported-subset.md` (the Ignored-warns list) and finalize the +decision doc's wording (they are now genuinely accepted-but-inert, not a +follow-up). + +## Files + +- `compose2pod/parsing.py` — add the two keys to `IGNORED_SERVICE_KEYS`. +- `tests/test_parsing.py` — test that both keys are accepted with a warning. +- `architecture/supported-subset.md` — add them to the Ignored-warns list. +- `planning/decisions/2026-07-09-stop-lifecycle-keys-inert.md` — drop the + "not yet wired / follow-up" parenthetical. + +## Verification + +- [ ] Failing test first — `just test tests/test_parsing.py::TestValidate::test_stop_lifecycle_keys_are_ignored_with_warning -v` (raises `unsupported key 'stop_signal'` before the change). +- [ ] Apply the change. +- [ ] Test passes — same command. +- [ ] `just test-ci` — full suite green at 100%. +- [ ] `just lint-ci` — clean. diff --git a/planning/decisions/2026-07-09-stop-lifecycle-keys-inert.md b/planning/decisions/2026-07-09-stop-lifecycle-keys-inert.md new file mode 100644 index 0000000..b52a90f --- /dev/null +++ b/planning/decisions/2026-07-09-stop-lifecycle-keys-inert.md @@ -0,0 +1,52 @@ +--- +status: accepted +summary: stop_signal and stop_grace_period are reclassified out of Bucket A as accepted-but-inert, because the generated script force-removes the pod (podman pod rm -f) and never gracefully stops a container. +supersedes: null +superseded_by: null +--- + +# 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 (`audits/2026-07-09-compose-spec-coverage.md`) listed +`stop_signal`/`stop_grace_period` in Bucket A as "clean per-container flag +mappings." Revisiting during the container-confinement bundle +(`changes/2026-07-09.04-container-confinement-service-keys.md`) 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 + (`changes/2026-07-09.05-ignore-stop-lifecycle-keys.md`). +- This is the same reasoning shape as the `dns` reclassification + (`deferred.md`): 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/tests/test_cli.py b/tests/test_cli.py index d453870..0533408 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -103,12 +103,12 @@ def test_unsupported_compose_returns_2( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: rc = run_main( - json.dumps({"services": {"app": {"image": "x", "privileged": True}}}), + json.dumps({"services": {"app": {"image": "x", "network_mode": "host"}}}), ["--target", "app", "--image", "i"], monkeypatch, ) assert rc == EXIT_USAGE_ERROR - assert "privileged" in capsys.readouterr().err + assert "network_mode" in capsys.readouterr().err def test_invalid_pod_name_returns_2( self, chats_compose: dict, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch diff --git a/tests/test_emit.py b/tests/test_emit.py index e48e510..53e2b81 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -104,6 +104,15 @@ def test_group_add_flag(self) -> None: flags = run_flags("app", {"image": "x", "group_add": ["docker", 1000]}, "p", [], "/b") assert flags[4:8] == ["--group-add", _Expand("docker"), "--group-add", _Expand("1000")] + def test_cap_add_flag(self) -> None: + flags = run_flags("app", {"image": "x", "cap_add": ["NET_ADMIN", "SYS_TIME"]}, "p", [], "/b") + assert flags[4:8] == ["--cap-add", _Expand("NET_ADMIN"), "--cap-add", _Expand("SYS_TIME")] + + def test_cap_drop_and_security_opt_flags(self) -> None: + svc = {"image": "x", "cap_drop": ["ALL"], "security_opt": ["label=disable"]} + flags = run_flags("app", svc, "p", [], "/b") + assert flags[4:8] == ["--cap-drop", _Expand("ALL"), "--security-opt", _Expand("label=disable")] + def test_labels_map_form(self) -> None: svc = {"image": "x", "labels": {"team": "api", "tier": "backend"}} flags = run_flags("app", svc, "p", [], "/b") @@ -120,6 +129,20 @@ def test_labels_null_value_is_empty_label(self) -> None: flags = run_flags("app", {"image": "x", "labels": {"empty": None}}, "p", [], "/b") assert flags[4:6] == ["--label", _Expand("empty")] + def test_read_only_flag(self) -> None: + flags = run_flags("app", {"image": "x", "read_only": True}, "p", [], "/b") + assert flags[4:5] == ["--read-only"] + + def test_all_boolean_flags_emit_when_true(self) -> None: + svc = {"image": "x", "init": True, "read_only": True, "privileged": True} + flags = run_flags("app", svc, "p", [], "/b") + assert flags[4:7] == ["--init", "--read-only", "--privileged"] + + def test_boolean_flag_false_or_absent_is_omitted(self) -> None: + flags = run_flags("app", {"image": "x", "read_only": False}, "p", [], "/b") + assert "--read-only" not in flags + assert flags == ["--pod", "p", "--name", "p-app"] + class TestImageAndCommand: def test_build_service_uses_ci_image(self, chats_compose: dict) -> None: @@ -357,6 +380,25 @@ def test_all_process_identity_keys_compose_on_one_service(self) -> None: assert '--label "team=api"' in script assert '--entrypoint "serve"' in script + def test_confinement_keys_compose_on_one_service(self) -> None: + svc = { + "image": "x", + "read_only": True, + "init": True, + "cap_add": ["NET_ADMIN"], + "cap_drop": ["ALL"], + "security_opt": ["label=disable"], + } + script = self._single(svc) + for fragment in ( + "--read-only", + "--init", + '--cap-add "NET_ADMIN"', + '--cap-drop "ALL"', + '--security-opt "label=disable"', + ): + assert fragment in script + class TestReferencedVariables: def _options(self, command: str = "") -> EmitOptions: @@ -409,3 +451,7 @@ def test_collects_from_new_process_identity_fields(self) -> None: def test_collects_from_entrypoint(self) -> None: compose = {"services": {"app": {"image": "x", "entrypoint": ["${EP}", "run"]}}} assert referenced_variables(compose, self._options()) == ["EP"] + + def test_collects_from_capability_and_security_lists(self) -> None: + compose = {"services": {"app": {"image": "x", "cap_add": ["${CAP}"], "security_opt": ["${OPT}"]}}} + assert referenced_variables(compose, self._options()) == ["CAP", "OPT"] diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 5538113..da84614 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -12,14 +12,21 @@ def test_chats_compose_is_accepted_with_warnings_for_ignored_keys(self, chats_co assert "'stdin_open'" in joined assert "'tty'" in joined + def test_stop_lifecycle_keys_are_ignored_with_warning(self) -> None: + # Inert under the pod force teardown -> accepted with a warning, not rejected. + svc = {"image": "x", "stop_signal": "SIGINT", "stop_grace_period": "30s"} + warnings = validate({"services": {"app": svc}}) + assert any("stop_signal" in w for w in warnings) + assert any("stop_grace_period" in w for w in warnings) + def test_non_dict_document_raises(self) -> None: for bad in (None, [], "compose", 42): with pytest.raises(UnsupportedComposeError, match="must be a mapping"): validate(bad) # ty: ignore[invalid-argument-type] def test_unsupported_service_key_raises(self) -> None: - with pytest.raises(UnsupportedComposeError, match="privileged"): - validate({"services": {"app": {"image": "x", "privileged": True}}}) + with pytest.raises(UnsupportedComposeError, match="network_mode"): + validate({"services": {"app": {"image": "x", "network_mode": "host"}}}) def test_unsupported_healthcheck_key_raises(self) -> None: compose = {"services": {"app": {"image": "x", "healthcheck": {"test": "true", "start_interval": "1s"}}}} @@ -150,6 +157,14 @@ def test_group_add_non_list_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match=r"'group_add' must be a list"): validate({"services": {"app": {"image": "x", "group_add": "docker"}}}) + def test_capability_list_keys_accepted(self) -> None: + svc = {"image": "x", "cap_add": ["NET_ADMIN"], "cap_drop": ["ALL"], "security_opt": ["label=disable"]} + assert validate({"services": {"app": svc}}) == [] + + def test_security_opt_non_list_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"'security_opt' must be a list"): + validate({"services": {"app": {"image": "x", "security_opt": "label=disable"}}}) + def test_labels_accepted(self) -> None: assert validate({"services": {"app": {"image": "x", "labels": {"team": "api"}}}}) == [] @@ -167,3 +182,11 @@ def test_entrypoint_non_string_or_list_raises(self) -> None: def test_string_entrypoint_with_command_warns(self) -> None: warnings = validate({"services": {"app": {"image": "x", "entrypoint": "serve", "command": ["x"]}}}) assert any("command' is ignored" in w for w in warnings) + + def test_boolean_confinement_keys_accepted(self) -> None: + svc = {"image": "x", "read_only": True, "init": True, "privileged": False} + assert validate({"services": {"app": svc}}) == [] + + def test_read_only_non_bool_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"'read_only' must be a boolean"): + validate({"services": {"app": {"image": "x", "read_only": "yes"}}})