diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index b1d8db3..b42990e 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -59,16 +59,22 @@ blocks (top-level and per-service — user payload by design, though the `x-` key itself is still checked); `build`'s own contents (never read — see `build`, below); and the ignored top-level `networks`/`volumes` blocks. -Rejecting a non-string key is a deliberate divergence from Docker, for keys -that *are* swept: `environment: {3306: db}` is valid Compose and Docker -accepts it, but compose2pod refuses it. Docker parses Compose as YAML 1.2, -where a bare `3306`/`on`/`off` stays the string it looks like, so Docker -never observes a non-string key at all; a boolean or int *key* has no -single correct string form to normalize to the way a boolean *value* does -(`DEBUG: true` → `"true"`, matching Docker — see `_render_scalar`, below) — -normalizing would give `true=1` where Docker gives `on=1`. Docker also -accepts a non-string key in `build`'s `args` or a top-level `volumes` -block's `driver_opts`; compose2pod, never reading either, accepts them too. +Rejecting a non-string key **matches Docker**, which refuses one too +(`non-string key in services.app.environment: 3306`). So `environment: +{3306: db}` and `{true: x}` are refused by both. + +What Docker does *not* see is a bare `on:` / `off:` / `yes:` / `no:` as a +non-string key, because it parses **YAML 1.2**, where each of those is an +ordinary string. PyYAML implements YAML **1.1**, where each is a *boolean* — +so the CLI loads YAML with a 1.2-style boolean resolver (`_build_yaml_loader`, +`compose2pod/cli.py`), and only `true`/`false` resolve as booleans. Without it, +`environment: {on: 1}` would arrive as the key `True` and be refused — a file +Docker runs — and the *value* `SSL: on` would reach the container as `SSL=true` +rather than `SSL=on`. The spelling cannot be recovered downstream: once the +loader has resolved `on` to `True`, `"on"` is gone. + +A genuine boolean *value* still renders lowercase (`DEBUG: true` → `DEBUG=true`, +see `_render_scalar`), which is what Docker renders too. The rejection runs up front, not key by key, because some downstream readers crash raw on a non-string key (`sorted()`, `str.startswith`, the diff --git a/compose2pod/cli.py b/compose2pod/cli.py index 5668fc7..e564522 100644 --- a/compose2pod/cli.py +++ b/compose2pod/cli.py @@ -3,6 +3,7 @@ import argparse import contextlib import json +import re import sys from pathlib import Path from types import ModuleType @@ -19,12 +20,41 @@ import yaml as _yaml +# YAML 1.2's boolean set: `true`/`false` only. PyYAML implements YAML *1.1*, +# where a bare `on`/`off`/`yes`/`no` is also a boolean -- but Docker's parser is +# YAML 1.2, so to `docker compose` each of those is an ordinary string. The +# difference is not cosmetic: `SSL: on` reaches the container as `SSL=true` +# instead of `SSL=on`, and `on:` used as a *key* resolves to the bool `True` and +# is then refused by the string-key rule -- rejecting a file Docker runs. +# +# It can only be fixed here. Once PyYAML has resolved `on` to `True`, the +# spelling is gone and no downstream pass can recover it. +_YAML_12_BOOL = r"^(?:true|True|TRUE|false|False|FALSE)$" + + +def _build_yaml_loader(yaml_module: ModuleType) -> type: + """Build a SafeLoader that resolves booleans the way YAML 1.2 (and so Docker) does.""" + + class Loader(yaml_module.SafeLoader): + pass + + # Drop PyYAML's YAML 1.1 bool resolver, then install the 1.2 one. Rebuilding + # the table is what removes `on`/`off`/`yes`/`no` from the boolean set; they + # fall through to the plain-string resolver. + Loader.yaml_implicit_resolvers = { + first_char: [(tag, regexp) for tag, regexp in resolvers if tag != "tag:yaml.org,2002:bool"] + for first_char, resolvers in yaml_module.SafeLoader.yaml_implicit_resolvers.items() + } + Loader.add_implicit_resolver("tag:yaml.org,2002:bool", re.compile(_YAML_12_BOOL), list("tTfF")) + return Loader + + def _load_yaml(text: str) -> Any: # noqa: ANN401 - returns arbitrary parsed compose data if _yaml is None: msg = "YAML input requires the 'yaml' extra: pip install compose2pod[yaml] (or pipe JSON via yq)" raise UnsupportedComposeError(msg) try: - return _yaml.safe_load(text) + return _yaml.load(text, Loader=_build_yaml_loader(_yaml)) # noqa: S506 - SafeLoader subclass, not full load except _yaml.YAMLError as error: msg = f"invalid YAML: {error}" raise UnsupportedComposeError(msg) from error diff --git a/planning/changes/2026-07-14.09-yaml-12-booleans.md b/planning/changes/2026-07-14.09-yaml-12-booleans.md new file mode 100644 index 0000000..3d70ebb --- /dev/null +++ b/planning/changes/2026-07-14.09-yaml-12-booleans.md @@ -0,0 +1,94 @@ +--- +summary: The CLI reads YAML with a 1.2-style boolean resolver, so on/off/yes/no stay strings as they do for docker compose, fixing both a rejected-file divergence and silent value corruption. +--- + +# Design: Read YAML the way Docker reads it + +## Summary + +PyYAML implements **YAML 1.1**, where a bare `on` / `off` / `yes` / `no` is a +**boolean**. Docker Compose parses **YAML 1.2**, where each is an ordinary +**string**. That single difference produces two bugs at once — one file +rejected that Docker runs, and one file silently mis-emitted. + +Fix it at the only place it can be fixed: the loader. + +## Motivation + +`compose2pod` is a drop-in replacement for `docker compose` on rootless runners +(`2026-07-14.08`), so the same file must behave the same way. It does not. + +**Silent corruption, exit 0.** A value: + +```yaml +environment: + SSL: on + DEBUG: yes +``` +``` +podman run ... -e "SSL=true" -e "DEBUG=true" # docker: -e SSL=on -e DEBUG=yes +``` + +The application reads `"true"` where its author wrote `on`. Nothing is reported. + +**A working file rejected.** The same words as a *key*: + +```yaml +environment: + on: 1 # docker compose: renders `on: 1`, runs fine +``` +``` +compose2pod: error: ... key True must be a string +``` + +PyYAML resolves the key to the boolean `True`, and the gate's string-key rule +refuses it. Refusing a file Docker runs is the one direction that must never +happen. + +Both come from the same place, and neither can be repaired downstream: by the +time compose2pod sees the document, `on` is already `True` and the original +spelling is gone. The string `"on"` is not recoverable from `True`. + +## Design + +`cli.py` loads YAML with a `SafeLoader` subclass whose boolean resolver matches +only `true`/`false` (and their case variants) — the YAML 1.2 core schema, which +is what Docker's parser implements. `on`/`off`/`yes`/`no` resolve as plain +strings. + +Nothing else about the schema changes. This is the one divergence Compose +actually trips over, and widening the fix into a general YAML-1.2 port would +change resolution rules (octal, `~`, timestamps) that nothing here depends on. + +The JSON path is untouched and needs nothing: JSON has no such ambiguity, and +its keys are strings by construction. + +## The non-string-key rule stands + +The gate refuses a non-string mapping key (`3306:`, `true:`, `1.5:`). **So does +Docker** — `non-string key in services.app.environment: 3306`. Measured, not +assumed. `architecture/supported-subset.md` claimed the opposite ("valid Compose +and Docker accepts it, but compose2pod refuses it"); that claim is false and is +corrected here. + +After this change the rule agrees with Docker exactly: `on:` is a *string* key +and is accepted; `3306:` is a non-string key and is refused, by both. + +## Non-goals + +- Not a general YAML 1.2 port. Only the boolean resolver moves. +- Not changing `_render_scalar`: a genuine boolean (`DEBUG: true`) still renders + lowercase `true`, matching `docker compose config`. + +## Testing + +`just test-ci` at 100%. The loader is tested directly (`on`/`off`/`yes`/`no` +parse as strings; `true`/`false` still parse as booleans, in both key and value +position), and end-to-end through the CLI: a document with `SSL: on` must emit +`-e SSL=on`, and one with `on: 1` as a key must be accepted. + +## Risk + +- **A document relying on `on` meaning boolean-true.** It never meant that to + Docker, so such a file was already being converted wrongly here. The change + makes it match the tool it replaces. diff --git a/tests/test_cli.py b/tests/test_cli.py index 46a0123..a39c296 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -245,22 +245,22 @@ def test_non_string_key_survives_extends_and_is_rejected_cleanly( yaml_text = ( "services:\n" " base:\n image: j\n environment:\n A: '1'\n" - " app:\n extends: {service: base}\n environment:\n on: '2'\n" + " app:\n extends: {service: base}\n environment:\n 3306: '2'\n" ) rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) assert rc == EXIT_USAGE_ERROR - assert "key True must be a string" in capsys.readouterr().err + assert "key 3306 must be a string" in capsys.readouterr().err def test_non_string_service_name_rejected_cleanly( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: - # F2: a non-string service name reached --add-host/--name verbatim - # (e.g. `podman run --name test-pod-True ...`) instead of being - # rejected at the gate. - yaml_text = "services:\n app:\n image: i\n on:\n image: j\n" + # A non-string service name reached --add-host/--name verbatim + # (e.g. `podman run --name test-pod-3306 ...`) instead of being + # rejected at the gate. Docker refuses a non-string key too. + yaml_text = "services:\n app:\n image: i\n 3306:\n image: j\n" rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) assert rc == EXIT_USAGE_ERROR - assert "compose document.services: key True must be a string" in capsys.readouterr().err + assert "compose document.services: key 3306 must be a string" in capsys.readouterr().err def test_yaml_anchor_extension_fields_convert( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch @@ -280,11 +280,11 @@ def test_non_string_key_from_x_block_anchor_merged_into_service_is_rejected( # *source* lived in a skipped x- block lands in the service body as # ordinary content once merged -- it must still be swept there. yaml_text = ( - "x-common: &common\n environment:\n on: 1\nservices:\n app:\n image: alpine\n <<: *common\n" + "x-common: &common\n environment:\n 3306: 1\nservices:\n app:\n image: alpine\n <<: *common\n" ) rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) assert rc == EXIT_USAGE_ERROR - assert "key True must be a string" in capsys.readouterr().err + assert "key 3306 must be a string" in capsys.readouterr().err class TestModuleEntrypoint: @@ -299,3 +299,50 @@ def test_python_m_runs(self, chats_compose: dict) -> None: ) assert result.returncode == 0, result.stderr assert result.stdout.startswith("#!/bin/sh") + + +class TestYaml12Booleans: + """`on`/`off`/`yes`/`no` are strings, as they are for `docker compose`. + + PyYAML implements YAML 1.1, where each is a boolean. Docker parses YAML 1.2, + where each is an ordinary string -- so a bare `on` must survive as the string + it was written as, both as a key and as a value. `true`/`false` are booleans + in both schemas and stay so. + """ + + def test_yaml_11_booleans_load_as_strings(self) -> None: + loaded = cli._load_yaml( # noqa: SLF001 - the loader is the unit under test + "k:\n a: on\n b: off\n c: yes\n d: no\n" + ) + assert loaded["k"] == {"a": "on", "b": "off", "c": "yes", "d": "no"} + + def test_real_booleans_still_load_as_booleans(self) -> None: + loaded = cli._load_yaml( # noqa: SLF001 - the loader is the unit under test + "k:\n t: true\n f: false\n T: True\n F: FALSE\n" + ) + assert loaded["k"] == {"t": True, "f": False, "T": True, "F": False} + + def test_on_as_a_key_stays_a_string(self) -> None: + # PyYAML 1.1 resolves this key to the bool True, and the gate's + # string-key rule then refuses a document `docker compose` runs fine. + loaded = cli._load_yaml( # noqa: SLF001 - the loader is the unit under test + "environment:\n on: 1\n off: 2\n" + ) + assert list(loaded["environment"]) == ["on", "off"] + + def test_value_on_reaches_the_script_as_on( + self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch + ) -> None: + # Used to emit -e "SSL=true": the app read "true" where its author wrote `on`. + compose = "services:\n app:\n image: alpine\n environment:\n SSL: on\n DEBUG: yes\n" + assert run_main(compose, ["--target", "app", "--image", "ci:1", "--format", "yaml"], monkeypatch) == 0 + script = capsys.readouterr().out + assert '-e "SSL=on"' in script + assert '-e "DEBUG=yes"' in script + + def test_on_as_a_key_is_accepted_end_to_end( + self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch + ) -> None: + compose = "services:\n app:\n image: alpine\n environment:\n on: 1\n" + assert run_main(compose, ["--target", "app", "--image", "ci:1", "--format", "yaml"], monkeypatch) == 0 + assert '-e "on=1"' in capsys.readouterr().out