diff --git a/README.md b/README.md index 6383758..1d5c21b 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,9 @@ it: `image`/`build`, `command`, `environment`/`env_file`, short-form bind `volumes`, `healthcheck` (CMD/CMD-SHELL), `depends_on` (all conditions), and network `aliases`. Compose extension fields (any `x-`-prefixed key) and YAML anchors are accepted as-is, so a top-level `x-*` anchor block for shared -config just works. `${VAR}`-style variable interpolation is resolved against -the process environment (no `.env` file support). See +config just works. `${VAR}`-style variable interpolation is left live in the +generated script, resolved by its shell against the environment present +when the script runs (no `.env` file support). See `architecture/supported-subset.md` for the full accept/ignore/reject matrix. ## Status diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index e27aeaa..c4b9df7 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -80,18 +80,32 @@ colon-less entry that is not absolute (e.g. `./cache`) is malformed and raises. ## Variable interpolation -`interpolate()` (`compose2pod/interpolate.py`) resolves Compose Spec -`${VAR}` references against the process environment (`os.environ` in the -CLI) before `validate()` sees the document, in every string leaf of the -compose document — dict keys are left untouched. Supported forms: -`$VAR`, `${VAR}`, `${VAR:-default}`, `${VAR-default}`, `${VAR:?msg}`, -`${VAR?msg}`, `${VAR:+alt}`, `${VAR+alt}`, and `$$` for a literal `$`. An -unset `$VAR`/`${VAR}` with no default resolves to an empty string and -emits a warning rather than failing; `${VAR:?msg}`/`${VAR?msg}` raises -`UnsupportedComposeError(msg)` instead. There is no `.env` file support — -only the environment the caller already has is consulted; export the -values first (`set -a; . .env; set +a`) if a project relies on a `.env` -file. +compose2pod does not resolve Compose Spec `${VAR}` references at generation +time. `to_shell()` (`compose2pod/shell.py`) instead re-encodes every +compose-derived string leaf (`environment`, `image`, `command`, `volumes`, +`tmpfs`, `env_file`, healthcheck `test`) into a double-quoted POSIX-shell +fragment with the variable references left live, so the generated script's +own shell expands them against its runtime environment when the script +runs. Only those field values pass through `to_shell()`; other document +fields (service and network names, ports, and the like) are never +interpolated. Supported forms: `$VAR`, `${VAR}`, +`${VAR:-default}`, `${VAR-default}`, `${VAR:?msg}`, `${VAR?msg}`, +`${VAR:+alt}`, `${VAR+alt}`, and `$$` for a literal `$`. The operator forms +map onto identical POSIX `sh` parameter expansion; bare `$VAR`/`${VAR}` is +emitted as `${VAR-}` so an unset variable expands to empty under the +script's `set -eu` (matching Compose semantics) instead of aborting on +`nounset`. `${VAR:?msg}`/`${VAR?msg}` fails the script at run time — with +`msg` — if the variable is unset or empty; there is no generation-time +check. Tool/CLI-supplied values (`--project-dir`, `--image`, the pod name, +the `--command` override) are literal and never interpolated. The CLI +prints one informational stderr note listing the variable names the +generated script actually expands at run time — `referenced_variables()` +(`compose2pod/emit.py`) collects these from the same tokens `to_shell()` +renders, so a `${VAR}` sitting in a literally-emitted field (or in the +`--command` override) never appears in the note. There is no `.env` file +support — only the environment present when the generated script runs is +consulted; export the values first (`set -a; . .env; set +a`) if a project +relies on a `.env` file. ## depends_on diff --git a/compose2pod/__init__.py b/compose2pod/__init__.py index 6fd5f41..a7cd8e1 100644 --- a/compose2pod/__init__.py +++ b/compose2pod/__init__.py @@ -2,14 +2,14 @@ from compose2pod.emit import EmitOptions, emit_script from compose2pod.exceptions import UnsupportedComposeError -from compose2pod.interpolate import interpolate from compose2pod.parsing import validate +from compose2pod.shell import to_shell __all__ = [ "EmitOptions", "UnsupportedComposeError", "emit_script", - "interpolate", + "to_shell", "validate", ] diff --git a/compose2pod/cli.py b/compose2pod/cli.py index 56d6d1b..fa80343 100644 --- a/compose2pod/cli.py +++ b/compose2pod/cli.py @@ -3,16 +3,14 @@ import argparse import contextlib import json -import os import re import sys from pathlib import Path from types import ModuleType from typing import Any -from compose2pod.emit import EmitOptions, emit_script +from compose2pod.emit import EmitOptions, emit_script, referenced_variables from compose2pod.exceptions import UnsupportedComposeError -from compose2pod.interpolate import interpolate from compose2pod.parsing import validate @@ -89,24 +87,24 @@ def main(argv: list[str] | None = None) -> int: except (json.JSONDecodeError, UnsupportedComposeError) as error: sys.stderr.write(f"compose2pod: error: could not parse compose input: {error}\n") return 2 + options = EmitOptions( + target=args.target, + ci_image=args.image, + command=args.command, + pod=args.pod_name, + project_dir=args.project_dir, + artifacts=args.artifact, + allow_exit_codes=args.allow_exit_code, + ) try: - compose, interpolation_warnings = interpolate(compose, os.environ) - warnings = [*interpolation_warnings, *validate(compose)] - script = emit_script( - compose=compose, - options=EmitOptions( - target=args.target, - ci_image=args.image, - command=args.command, - pod=args.pod_name, - project_dir=args.project_dir, - artifacts=args.artifact, - allow_exit_codes=args.allow_exit_code, - ), - ) + warnings = validate(compose) + script = emit_script(compose=compose, options=options) except UnsupportedComposeError as error: sys.stderr.write(f"compose2pod: error: {error}\n") return 2 + referenced = referenced_variables(compose, options) + if referenced: + sys.stderr.write("compose2pod: note: script references variables at run time: " + ", ".join(referenced) + "\n") for warning in warnings: sys.stderr.write(f"compose2pod: {warning}\n") sys.stdout.write(script) diff --git a/compose2pod/emit.py b/compose2pod/emit.py index e466e28..57cf4aa 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -7,33 +7,44 @@ from compose2pod.graph import depends_on, hostnames, startup_order from compose2pod.healthcheck import health_cmd, interval_seconds +from compose2pod.shell import to_shell, variable_names HEALTHY_WAIT_BUDGET_SECONDS = 120 -def image_for(svc: dict[str, Any], ci_image: str) -> str: +@dataclasses.dataclass(frozen=True) +class _Expand: + """A token whose Compose variable references expand at script-run time.""" + + value: str + + +Token = str | _Expand + + +def image_for(svc: dict[str, Any], ci_image: str) -> Token: """Services with a build section run the freshly built CI image.""" if "build" in svc: return ci_image - return svc["image"] + return _Expand(svc["image"]) -def command_tokens(svc: dict[str, Any]) -> list[str]: +def command_tokens(svc: dict[str, Any]) -> list[Token]: """Service command as argv tokens; compose string form means shell form.""" command = svc.get("command") if command is None: return [] if isinstance(command, str): - return ["/bin/sh", "-c", command] - return list(command) + return ["/bin/sh", "-c", _Expand(command)] + return [_Expand(str(token)) for token in command] -def _add_health_flags(flags: list[str], healthcheck: dict[str, Any]) -> None: +def _add_health_flags(flags: list[Token], healthcheck: dict[str, Any]) -> None: """Add healthcheck flags to the flags list.""" cmd = health_cmd(healthcheck.get("test")) if cmd is not None: - flags += ["--health-cmd", cmd] + flags += ["--health-cmd", _Expand(cmd)] if "timeout" in healthcheck: flags += ["--health-timeout", str(healthcheck["timeout"])] if "start_period" in healthcheck: @@ -42,24 +53,24 @@ def _add_health_flags(flags: list[str], healthcheck: dict[str, Any]) -> None: flags += ["--health-retries", str(healthcheck["retries"])] -def run_flags(name: str, svc: dict[str, Any], pod: str, hosts: list[str], project_dir: str) -> list[str]: +def run_flags(name: str, svc: dict[str, Any], pod: str, hosts: list[str], project_dir: str) -> list[Token]: """Flag tokens (unquoted) for `podman run` of one service.""" - flags = ["--pod", pod, "--name", f"{pod}-{name}"] + flags: list[Token] = ["--pod", pod, "--name", f"{pod}-{name}"] for host in hosts: flags += ["--add-host", f"{host}:127.0.0.1"] environment = svc.get("environment") or {} pairs = environment if isinstance(environment, list) else [f"{k}={v}" for k, v in environment.items()] for pair in pairs: - flags += ["-e", pair] + flags += ["-e", _Expand(str(pair))] env_files = svc.get("env_file") or [] if isinstance(env_files, str): env_files = [env_files] for env_file in env_files: - flags += ["--env-file", str(Path(project_dir, env_file))] + flags += ["--env-file", _Expand(str(Path(project_dir, env_file)))] for volume in svc.get("volumes") or []: if ":" not in volume: # Anonymous volume: a bare container path, no host source to translate. - flags += ["-v", volume] + flags += ["-v", _Expand(volume)] continue source, destination = volume.split(":", 1) if source.startswith("."): @@ -67,12 +78,12 @@ def run_flags(name: str, svc: dict[str, Any], pod: str, hosts: list[str], projec source = str(Path(project_dir, source)) # Absolute bind mount (starts with "/") and named volume (bare # identifier) are both kept as-is — neither is a path to translate. - flags += ["-v", f"{source}:{destination}"] + flags += ["-v", _Expand(f"{source}:{destination}")] tmpfs = svc.get("tmpfs") or [] if isinstance(tmpfs, str): tmpfs = [tmpfs] for mount in tmpfs: - flags += ["--tmpfs", mount] + flags += ["--tmpfs", _Expand(mount)] healthcheck = svc.get("healthcheck") or {} _add_health_flags(flags, healthcheck) return flags @@ -115,11 +126,11 @@ class EmitOptions: allow_exit_codes: list[int] -def _render(tokens: list[str]) -> str: - return " ".join(shlex.quote(token) for token in tokens) +def _render(tokens: list[Token]) -> str: + return " ".join(to_shell(token.value) if isinstance(token, _Expand) else shlex.quote(token) for token in tokens) -def _run_tokens(name: str, services: dict[str, Any], options: EmitOptions, hosts: list[str]) -> list[str]: +def _run_tokens(name: str, services: dict[str, Any], options: EmitOptions, hosts: list[str]) -> list[Token]: svc = services[name] tokens = run_flags(name, svc, options.pod, hosts, options.project_dir) tokens.append(image_for(svc, options.ci_image)) @@ -130,7 +141,7 @@ def _run_tokens(name: str, services: dict[str, Any], options: EmitOptions, hosts return tokens -def _emit_target(lines: list[str], run_tokens: list[str], options: EmitOptions) -> None: +def _emit_target(lines: list[str], run_tokens: list[Token], options: EmitOptions) -> None: target_ctr = shlex.quote(f"{options.pod}-{options.target}") lines.append("rc=0") lines.append(f"podman run {_render(run_tokens)} || rc=$?") @@ -181,3 +192,21 @@ def emit_script(compose: dict[str, Any], options: EmitOptions) -> str: else: lines.append(f"podman run -d {_render(run_tokens)}") return "\n".join(lines) + "\n" + + +def referenced_variables(compose: dict[str, Any], options: EmitOptions) -> list[str]: + """Variable names the emitted script expands at run time (sorted, unique). + + Reuses the same token construction as `emit_script`, so it counts exactly + the values that pass through `to_shell` -- never a `${VAR}` in a field that + is emitted literally, and never the `--command` override (a literal CLI value). + """ + services = compose["services"] + hosts = hostnames(services) + order = startup_order(services, options.target) + names: set[str] = set() + for name in order: + for token in _run_tokens(name, services, options, hosts): + if isinstance(token, _Expand): + names.update(variable_names(token.value)) + return sorted(names) diff --git a/compose2pod/interpolate.py b/compose2pod/interpolate.py deleted file mode 100644 index 5619d08..0000000 --- a/compose2pod/interpolate.py +++ /dev/null @@ -1,72 +0,0 @@ -"""Resolve Compose-spec `${VAR}` interpolation against a given environment.""" - -import re -from collections.abc import Mapping -from typing import Any - -from compose2pod.exceptions import UnsupportedComposeError - - -_PATTERN = re.compile( - r""" - \$(?: - (?P\$) - |(?P[a-zA-Z_][a-zA-Z0-9_]*) - |\{(?P[a-zA-Z_][a-zA-Z0-9_]*)(?P:-|-|:\?|\?|:\+|\+)?(?P[^}]*)\} - ) - """, - re.VERBOSE, -) - - -def _resolve(name: str, op: str | None, arg: str, environ: Mapping[str, str], warnings: list[str]) -> str: - is_set = name in environ - is_set_and_nonempty = is_set and environ[name] != "" - if op is None: - if is_set: - return environ[name] - warnings.append(f"variable '{name}' is not set, defaulting to blank string") - return "" - if op in {"-", ":-"}: - condition = is_set_and_nonempty if op == ":-" else is_set - return environ[name] if condition else arg - if op in {"?", ":?"}: - condition = is_set_and_nonempty if op == ":?" else is_set - if condition: - return environ[name] - raise UnsupportedComposeError(arg) - condition = is_set_and_nonempty if op == ":+" else is_set - return arg if condition else "" - - -def _interpolate_string(text: str, environ: Mapping[str, str], warnings: list[str]) -> str: - def substitute(match: re.Match[str]) -> str: - if match.group("escaped"): - return "$" - if match.group("named") is not None: - return _resolve(match.group("named"), None, "", environ, warnings) - return _resolve(match.group("braced"), match.group("op"), match.group("arg"), environ, warnings) - - return _PATTERN.sub(substitute, text) - - -def _walk(node: Any, environ: Mapping[str, str], warnings: list[str]) -> Any: # noqa: ANN401 - arbitrary compose value - if isinstance(node, str): - return _interpolate_string(node, environ, warnings) - if isinstance(node, dict): - return {key: _walk(value, environ, warnings) for key, value in node.items()} - if isinstance(node, list): - return [_walk(item, environ, warnings) for item in node] - return node - - -def interpolate(document: Any, environ: Mapping[str, str]) -> tuple[Any, list[str]]: # noqa: ANN401 - arbitrary compose value - """Resolve `${VAR}`-style references in every string leaf of `document`. - - Dict keys are left untouched. Returns the resolved document and any - "variable not set" warnings; raises `UnsupportedComposeError` for a - `${VAR:?msg}`/`${VAR?msg}` reference whose condition is not met. - """ - warnings: list[str] = [] - resolved = _walk(document, environ, warnings) - return resolved, warnings diff --git a/compose2pod/shell.py b/compose2pod/shell.py new file mode 100644 index 0000000..8996feb --- /dev/null +++ b/compose2pod/shell.py @@ -0,0 +1,62 @@ +"""Encode Compose string values as POSIX-shell fragments the runtime shell expands.""" + +import re + + +_PATTERN = re.compile( + r""" + \$(?: + (?P\$) + |(?P[a-zA-Z_][a-zA-Z0-9_]*) + |\{(?P[a-zA-Z_][a-zA-Z0-9_]*)(?P:-|-|:\?|\?|:\+|\+)?(?P[^}]*)\} + ) + """, + re.VERBOSE, +) + +_DQUOTE_ESCAPES = {"\\": "\\\\", '"': '\\"', "$": "\\$", "`": "\\`"} + + +def _escape_literal(text: str) -> str: + """Escape `text` so a POSIX shell treats it literally inside double quotes.""" + return "".join(_DQUOTE_ESCAPES.get(char, char) for char in text) + + +def _encode_match(match: re.Match[str]) -> str: + if match.group("escaped"): + return "\\$" # Compose `$$` -> a literal `$` + if match.group("named") is not None: + return "${" + match.group("named") + "-}" # unset -> empty, survives `set -u` + name = match.group("braced") + op = match.group("op") + if op is None: + return "${" + name + "-}" + return "${" + name + op + _escape_literal(match.group("arg")) + "}" + + +def to_shell(value: str) -> str: + """Return `value` as a double-quoted shell fragment. + + Compose variable references (`$VAR`, `${VAR:-d}`, ...) stay live so the + shell running the generated script resolves them against its own + environment; every other character is inert (no command substitution, no + accidental expansion). `$$` becomes a literal `$`. + """ + out: list[str] = [] + pos = 0 + for match in _PATTERN.finditer(value): + out.append(_escape_literal(value[pos : match.start()])) + out.append(_encode_match(match)) + pos = match.end() + out.append(_escape_literal(value[pos:])) + return '"' + "".join(out) + '"' + + +def variable_names(value: str) -> set[str]: + """Variable names `value` references (excluding `$$`).""" + names: set[str] = set() + for match in _PATTERN.finditer(value): + if match.group("escaped"): + continue + names.add(match.group("named") or match.group("braced")) + return names diff --git a/planning/changes/2026-07-08.07-runtime-variable-interpolation.md b/planning/changes/2026-07-08.07-runtime-variable-interpolation.md new file mode 100644 index 0000000..6b4f382 --- /dev/null +++ b/planning/changes/2026-07-08.07-runtime-variable-interpolation.md @@ -0,0 +1,155 @@ +--- +summary: Replaced generation-time `${VAR}` resolution with runtime shell expansion — emit re-encodes compose string values as double-quoted shell fragments so the generated `.sh` interpolates against the environment present when it runs, not compose2pod's. +--- + +# Design: Interpolate variables at script-run time + +## Summary + +`compose2pod` 0.1.7 (#18) added `interpolate()`, which resolves Compose +`${VAR}` references against `os.environ` **when the script is generated**. +For the tool's actual use case — generate a script now, run it later in CI +where the secrets/tokens live — that is the wrong time: any variable unset in +compose2pod's own environment is baked to an empty string, so `${VAR}` reaches +the container blank. This change removes generation-time resolution and instead +emits the references so the **runtime POSIX shell** expands them against the +environment present when the generated `.sh` runs. POSIX `sh` parameter +expansion already matches the Compose operator forms exactly, so the emitted +script reproduces Compose interpolation semantics, just evaluated at run time. + +## Motivation + +```yaml +services: + app: + environment: + API_KEY: ${API_KEY} +``` + +`API_KEY` is injected by CI when the pod runs, and is **not** set in the shell +that invokes `compose2pod` to produce the script. #18 resolves `${API_KEY}` +against that generation-time environment, finds it unset, and emits +`-e 'API_KEY='` (empty) plus a "variable not set" warning. The container gets +an empty value. Because `emit` `shlex.quote`s every token, there is no +shell-runtime fallback either — the baked value is final. + +The generation-time warning and the generation-time `${VAR:?msg}` failure are +both the same error in another guise: checking variables against the wrong +environment. In this flow the variable is *supposed* to be unset at generation +time, so both fire as false alarms. + +## Design + +### 1. Pipeline (`cli.py`) + +Remove the `interpolate(compose, os.environ)` resolution step. The parsed +document flows to `validate()`/`emit` with `${VAR}` references intact. After +building the script, write one informational stderr line naming the variables +the script references at run time (see §4). + +### 2. New module `compose2pod/shell.py` (replaces `interpolate.py`) + +Two pure functions; keys are never interpolated (Compose doesn't template +mapping keys). + +`to_shell(value: str) -> str` — re-encode one compose string into a +**double-quoted** shell fragment where variable references stay live and +everything else is inert. Walks the string with the existing interpolation +regex and, per span: + +- `${VAR:-d}`, `${VAR-d}`, `${VAR:?m}`, `${VAR?m}`, `${VAR:+a}`, `${VAR+a}` + → emitted **unchanged**; POSIX `sh` parameter expansion has identical + set/unset/empty semantics to Compose. The literal `d`/`m`/`a` text is escaped. +- bare `$VAR` / `${VAR}` → **`${VAR-}`**. The script header is `set -eu`; a + bare `${VAR}` on an unset variable would abort under `nounset`. Compose + semantics are unset → empty, so `${VAR-}` both reproduces that and sidesteps + `nounset`. Authors wanting fail-fast write `${VAR:?...}` explicitly. +- `$$` → `\$` — literal `$`, matching Compose (shell's `$$` = PID is avoided). +- literal `$(`, backticks, `"`, `\`, and any `$` not starting a recognized + reference → escaped. No command substitution; Compose does variable + interpolation only. + +`referenced_variables(document: Any) -> list[str]` — walk string leaves, return +the sorted unique variable names referenced (excluding `$$`), for the note. + +### 3. `emit` renders two token classes + +A whole token is either literal or expandable — there is no intra-token +mixing. `emit` gains a tiny wrapper, `_Expand(value: str)`; `_render` +dispatches: `_Expand` → `to_shell(value)`, plain `str` → `shlex.quote` +(unchanged literal behavior). Token lists become `list[str | _Expand]`. + +Compose-*content* leaves become `_Expand`: `environment` entries (the whole +`KEY=value`, list or mapping form — valid env names contain no `$`, so keys +are unaffected), the service `image`, `command` argument (`/bin/sh -c ` +keeps `/bin/sh`/`-c` literal, wraps ``; list form wraps each arg), +`volumes` (`source:dest`, anonymous path), `tmpfs` mounts, and the +healthcheck `test` command. Tool/CLI-generated tokens stay plain `str` +(literal): pod name (regex-validated `$`-free), `{pod}-{name}` container +names, `--add-host` host names, health timeouts, artifact paths, the +`--command` override (`shlex.split`, already shell-final), and the `--image` +value used for `build` services. + +`image_for`, `command_tokens`, and `run_flags` therefore return +`str | _Expand` tokens rather than raw strings; their unit tests wrap the +compose-derived expectations in `_Expand(...)`. + +### 4. Informational note (`cli.py`) + +If `referenced_variables(compose)` is non-empty, write one line: + +``` +compose2pod: note: script references variables at run time: API_KEY, DATABASE_URL +``` + +It needs no runtime values and tells CI authors what the environment must +provide. + +### 5. Public API (`__init__.py`) + +Remove the `interpolate` export added in 0.1.7; export `to_shell`. +`referenced_variables` stays internal. This breaks the just-shipped 0.1.7 API, +which is acceptable — we are correcting that API, not extending it. + +## Non-goals + +- **No generation-time resolution of any field.** Every `${VAR}` in the emitted + script expands at run time; there is no generation-time pinning. The emitted + script is therefore not self-contained — its effect depends on the runtime + environment. This is the intent. +- **No command substitution.** `$(...)`/backticks in compose values are emitted + literally, matching Compose. +- **No interpolation of the `--project-dir` / `--image` CLI values.** They are a + path and an image reference, not templates; a literal `$` in them is a + pathological input and would pass through un-escaped only where a volume + source is joined onto `--project-dir`. Don't put `$` in those flags. +- **No `.env` file loading** (unchanged from #18): the runtime shell's own + environment is what's consulted; `set -a; . .env; set +a` before running the + generated script if a project relies on a `.env` file. + +## Testing + +TDD, `just test-ci` at 100% line coverage. + +- Unit (`test_shell.py`, replaces `test_interpolate.py`): one case per form + asserting the emitted fragment — `$FOO`/`${FOO}` → `"${FOO-}"`, + `${FOO:-d}` → `"${FOO:-d}"`, each operator form passed through, `$$` → `\$`, + `a$(rm -rf x)b` and backticks/`"`/`\` escaped, multiple refs in one string, + non-string leaves untouched. `referenced_variables` returns sorted unique + names and excludes `$$`. +- Integration (`test_cli.py`): a compose with `${VAR}` in an `environment` + value emits a script whose `-e` flag carries the live `${VAR-}` reference + (not a resolved value); running that emitted fragment through `sh` with the + variable set vs. unset yields the value vs. empty. The informational note + lists the referenced names. `${VAR:?msg}` in a value survives into the script + rather than failing generation. +- `test_exports` updated for `to_shell` replacing `interpolate`. + +## Risk + +Low–moderate. The re-encoder's escaping is the delicate part: a missed literal +`$`/backtick/quote either breaks the emitted shell word or opens command +substitution. Mitigated by the per-form/escaping unit matrix and by an +integration test that actually executes the emitted fragment under `sh`. The +`set -eu` × bare-`${VAR-}` interaction is covered by an explicit unset-variable +run. Public-API break is contained to a one-release-old export. diff --git a/tests/test_cli.py b/tests/test_cli.py index f66ec53..1c4347c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,10 +25,11 @@ def test_exports(self) -> None: "EmitOptions", "UnsupportedComposeError", "emit_script", - "interpolate", + "to_shell", "validate", } assert compose2pod.validate({"services": {"a": {"image": "x"}}}) == [] + assert compose2pod.to_shell("$FOO") == '"${FOO-}"' class TestMain: @@ -134,37 +135,49 @@ def test_missing_file_returns_2(self, capsys: pytest.CaptureFixture[str]) -> Non assert rc == EXIT_USAGE_ERROR assert "compose2pod: error:" in capsys.readouterr().err - def test_environment_variable_reference_is_resolved( + def test_environment_reference_is_emitted_live_not_resolved( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv("LLM_MODEL", "gpt-4") + monkeypatch.setenv("LLM_MODEL", "gpt-4") # set at generation time -> must be ignored yaml_text = "services:\n app:\n image: x\n environment:\n MODEL: ${LLM_MODEL}\n" rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) out = capsys.readouterr() assert rc == 0 - assert "MODEL=gpt-4" in out.out - assert "LLM_MODEL" not in out.out + assert "MODEL=gpt-4" not in out.out # NOT baked in at generation + assert "${LLM_MODEL-}" in out.out # left live for the runtime shell + assert "note: script references variables at run time: LLM_MODEL" in out.err - def test_unset_environment_variable_reference_warns( + def test_required_reference_survives_generation( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.delenv("UNSET_VAR", raising=False) - yaml_text = "services:\n app:\n image: x\n environment:\n MODEL: ${UNSET_VAR}\n" + monkeypatch.delenv("REQUIRED_VAR", raising=False) + yaml_text = "services:\n app:\n image: x\n environment:\n MODEL: ${REQUIRED_VAR:?must be set}\n" rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) out = capsys.readouterr() - assert rc == 0 - assert "MODEL=" in out.out - assert "UNSET_VAR" in out.err - assert "not set" in out.err + assert rc == 0 # no longer fails at generation + assert "${REQUIRED_VAR:?must be set}" in out.out # shell enforces it at run time - def test_required_environment_variable_missing_returns_2( + def test_note_lists_multiple_referenced_variables_sorted( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.delenv("REQUIRED_VAR", raising=False) - yaml_text = "services:\n app:\n image: x\n environment:\n MODEL: ${REQUIRED_VAR:?must be set}\n" + yaml_text = "services:\n app:\n image: x\n environment:\n A: ${ZED}\n B: ${ALPHA}\n" rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) - assert rc == EXIT_USAGE_ERROR - assert "must be set" in capsys.readouterr().err + out = capsys.readouterr() + assert rc == 0 + assert "note: script references variables at run time: ALPHA, ZED" in out.err + + def test_env_file_path_variable_is_live_and_noted( + self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch + ) -> None: + yaml_text = "services:\n app:\n image: x\n env_file: ${ENV_DIR}/app.env\n" + rc = run_main( + yaml_text, ["--target", "app", "--image", "i", "--project-dir", "/p", "--format", "yaml"], monkeypatch + ) + out = capsys.readouterr() + assert rc == 0 + assert "--env-file" in out.out + assert "${ENV_DIR-}" in out.out + assert "ENV_DIR" in out.err def test_yaml_anchor_extension_fields_convert( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch diff --git a/tests/test_emit.py b/tests/test_emit.py index 8362013..0fa6ad2 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -1,8 +1,10 @@ from compose2pod.emit import ( EmitOptions, + _Expand, command_tokens, emit_script, image_for, + referenced_variables, run_flags, ) from compose2pod.parsing import validate @@ -17,8 +19,8 @@ def test_db_flags(self, chats_compose: dict) -> None: assert flags[:4] == ["--pod", "test-pod", "--name", "test-pod-db"] assert flags[4:6] == ["--add-host", "db:127.0.0.1"] assert flags[6:8] == ["--add-host", "keydb:127.0.0.1"] - assert flags[8:10] == ["-e", "POSTGRES_PASSWORD=password"] - assert flags[10:12] == ["--health-cmd", "pg_isready -U database -d database"] + assert flags[8:10] == ["-e", _Expand("POSTGRES_PASSWORD=password")] + assert flags[10:12] == ["--health-cmd", _Expand("pg_isready -U database -d database")] assert flags[12:14] == ["--health-timeout", "5s"] assert flags[14:16] == ["--health-retries", "15"] # fix #2 @@ -31,47 +33,51 @@ def test_start_period_is_passed_through(self) -> None: def test_env_map_form(self) -> None: svc = {"image": "x", "environment": {"A": "1", "B": "two words"}} flags = run_flags("app", svc, "p", [], "/builds/x") - assert flags[4:6] == ["-e", "A=1"] - assert flags[6:8] == ["-e", "B=two words"] + assert flags[4:8] == ["-e", _Expand("A=1"), "-e", _Expand("B=two words")] def test_env_file_and_volume_resolved_against_project_dir(self) -> None: svc = {"image": "x", "env_file": "tests.env", "volumes": [".:/srv/www/"]} flags = run_flags("app", svc, "p", [], "/builds/chats") - assert flags[4:6] == ["--env-file", "/builds/chats/tests.env"] - assert flags[6:8] == ["-v", "/builds/chats:/srv/www/"] + assert flags[4:6] == ["--env-file", _Expand("/builds/chats/tests.env")] + assert flags[6:8] == ["-v", _Expand("/builds/chats:/srv/www/")] def test_env_file_list_form(self) -> None: svc = {"image": "x", "env_file": ["a.env", "b.env"]} flags = run_flags("app", svc, "p", [], "/builds/x") - assert flags[4:8] == ["--env-file", "/builds/x/a.env", "--env-file", "/builds/x/b.env"] + assert flags[4:8] == [ + "--env-file", + _Expand("/builds/x/a.env"), + "--env-file", + _Expand("/builds/x/b.env"), + ] def test_tmpfs_string_form(self) -> None: # S108 flags "/tmp" as an insecure hardcoded temp path; this is a # pass-through string being tested, not a file write. flags = run_flags("app", {"image": "x", "tmpfs": "/tmp:mode=1777"}, "p", [], "/builds/x") # noqa: S108 - assert flags[4:6] == ["--tmpfs", "/tmp:mode=1777"] # noqa: S108 + assert flags[4:6] == ["--tmpfs", _Expand("/tmp:mode=1777")] # noqa: S108 def test_tmpfs_list_form(self) -> None: svc = {"image": "x", "tmpfs": ["/tmp:mode=1777", "/run"]} # noqa: S108 flags = run_flags("app", svc, "p", [], "/builds/x") - assert flags[4:8] == ["--tmpfs", "/tmp:mode=1777", "--tmpfs", "/run"] # noqa: S108 + assert flags[4:8] == ["--tmpfs", _Expand("/tmp:mode=1777"), "--tmpfs", _Expand("/run")] # noqa: S108 def test_absolute_volume_source_is_kept_as_is(self) -> None: flags = run_flags("app", {"image": "x", "volumes": ["/data/app:/srv/www/"]}, "p", [], "/builds/x") - assert flags[4:6] == ["-v", "/data/app:/srv/www/"] + assert flags[4:6] == ["-v", _Expand("/data/app:/srv/www/")] def test_anonymous_volume_emitted_as_single_path(self) -> None: flags = run_flags("app", {"image": "x", "volumes": ["/var/cache/models"]}, "p", [], "/builds/x") - assert flags[4:6] == ["-v", "/var/cache/models"] + assert flags[4:6] == ["-v", _Expand("/var/cache/models")] def test_named_volume_emitted_without_project_dir_translation(self) -> None: svc = {"image": "x", "volumes": ["pgdata:/var/lib/postgresql/data"]} flags = run_flags("db", svc, "p", [], "/builds/x") - assert flags[4:6] == ["-v", "pgdata:/var/lib/postgresql/data"] + assert flags[4:6] == ["-v", _Expand("pgdata:/var/lib/postgresql/data")] def test_healthcheck_without_timeout_omits_health_timeout_flag(self) -> None: flags = run_flags("app", {"image": "x", "healthcheck": {"test": "true"}}, "p", [], "/builds/x") - assert flags[4:6] == ["--health-cmd", "true"] + assert flags[4:6] == ["--health-cmd", _Expand("true")] assert "--health-timeout" not in flags @@ -80,13 +86,17 @@ def test_build_service_uses_ci_image(self, chats_compose: dict) -> None: assert image_for(chats_compose["services"]["application"], "reg/ci:abc") == "reg/ci:abc" def test_plain_service_keeps_image(self, chats_compose: dict) -> None: - assert image_for(chats_compose["services"]["db"], "reg/ci:abc") == "postgres:13.5-alpine" + assert image_for(chats_compose["services"]["db"], "reg/ci:abc") == _Expand("postgres:13.5-alpine") def test_command_list_passes_through(self, chats_compose: dict) -> None: - assert command_tokens(chats_compose["services"]["migrations"]) == ["alembic", "upgrade", "head"] + assert command_tokens(chats_compose["services"]["migrations"]) == [ + _Expand("alembic"), + _Expand("upgrade"), + _Expand("head"), + ] def test_command_string_becomes_shell(self) -> None: - assert command_tokens({"command": "echo hi"}) == ["/bin/sh", "-c", "echo hi"] + assert command_tokens({"command": "echo hi"}) == ["/bin/sh", "-c", _Expand("echo hi")] def test_missing_command_is_empty(self) -> None: assert command_tokens({"image": "x"}) == [] @@ -129,7 +139,7 @@ def test_db_and_keydb_detached_migrations_foreground(self, chats_compose: dict) assert line.startswith("podman run -d ") if "--name test-pod-migrations" in line: assert line.startswith("podman run --rm ") - assert line.rstrip().endswith("alembic upgrade head") + assert line.rstrip().endswith('"alembic" "upgrade" "head"') def test_target_command_is_overridden_and_rc_gated(self, chats_compose: dict) -> None: script = self.make_script(chats_compose) @@ -216,7 +226,7 @@ def test_named_volume_round_trips_through_validate_and_emit(self) -> None: ) assert validate(compose) == ["ignoring top-level 'volumes' (podman creates named volumes on first reference)"] script = emit_script(compose=compose, options=options) - assert "-v calutrondb:/var/lib/postgresql/data" in script + assert '-v "calutrondb:/var/lib/postgresql/data"' in script def test_target_without_command_uses_service_command(self, chats_compose: dict) -> None: options = EmitOptions( @@ -230,4 +240,39 @@ def test_target_without_command_uses_service_command(self, chats_compose: dict) ) script = emit_script(compose=chats_compose, options=options) target_line = next(line for line in script.splitlines() if "--name test-pod-application" in line) - assert target_line.rstrip().endswith("python -m chats.api || rc=$?") + assert target_line.rstrip().endswith('"python" "-m" "chats.api" || rc=$?') + + +class TestReferencedVariables: + def _options(self, command: str = "") -> EmitOptions: + return EmitOptions( + target="app", + ci_image="i", + command=command, + pod="p", + project_dir="/p", + artifacts=[], + allow_exit_codes=[], + ) + + def test_collects_from_interpolated_fields_sorted(self) -> None: + compose = {"services": {"app": {"image": "${IMG}", "environment": {"A": "${AVAR}", "B": "${BVAR}"}}}} + assert referenced_variables(compose, self._options()) == ["AVAR", "BVAR", "IMG"] + + def test_includes_env_file_excludes_non_interpolated_fields(self) -> None: + compose = { + "services": { + "app": { + "image": "x", + "environment": {"K": "${LIVE}"}, + "env_file": "${EDIR}/a.env", + "x-note": "${XVAR}", + } + } + } + assert referenced_variables(compose, self._options()) == ["EDIR", "LIVE"] + + def test_command_override_vars_are_excluded(self) -> None: + compose = {"services": {"app": {"image": "x", "command": "run ${CMDVAR}"}}} + options = self._options(command="override ${SHELLVAR}") + assert referenced_variables(compose, options) == [] diff --git a/tests/test_interpolate.py b/tests/test_interpolate.py deleted file mode 100644 index c85f1f7..0000000 --- a/tests/test_interpolate.py +++ /dev/null @@ -1,102 +0,0 @@ -import pytest - -from compose2pod.exceptions import UnsupportedComposeError -from compose2pod.interpolate import interpolate - - -class TestInterpolate: - def test_bare_dollar_var_substitutes(self) -> None: - document, warnings = interpolate({"k": "$FOO"}, {"FOO": "bar"}) - assert document == {"k": "bar"} - assert warnings == [] - - def test_braced_var_substitutes(self) -> None: - document, warnings = interpolate({"k": "${FOO}"}, {"FOO": "bar"}) - assert document == {"k": "bar"} - assert warnings == [] - - def test_unset_var_defaults_to_blank_and_warns(self) -> None: - document, warnings = interpolate({"k": "${FOO}"}, {}) - assert document == {"k": ""} - assert "FOO" in warnings[0] - assert "not set" in warnings[0] - - def test_dollar_dollar_is_literal_dollar(self) -> None: - document, warnings = interpolate({"k": "$$FOO"}, {"FOO": "bar"}) - assert document == {"k": "$FOO"} - assert warnings == [] - - def test_colon_dash_uses_default_when_unset(self) -> None: - document, _ = interpolate({"k": "${FOO:-fallback}"}, {}) - assert document == {"k": "fallback"} - - def test_colon_dash_uses_default_when_empty(self) -> None: - document, _ = interpolate({"k": "${FOO:-fallback}"}, {"FOO": ""}) - assert document == {"k": "fallback"} - - def test_colon_dash_uses_value_when_set_and_nonempty(self) -> None: - document, _ = interpolate({"k": "${FOO:-fallback}"}, {"FOO": "bar"}) - assert document == {"k": "bar"} - - def test_dash_uses_default_only_when_unset(self) -> None: - document, _ = interpolate({"k": "${FOO-fallback}"}, {}) - assert document == {"k": "fallback"} - - def test_dash_keeps_empty_value_when_set_empty(self) -> None: - document, _ = interpolate({"k": "${FOO-fallback}"}, {"FOO": ""}) - assert document == {"k": ""} - - def test_colon_question_raises_when_unset(self) -> None: - with pytest.raises(UnsupportedComposeError, match="FOO is required"): - interpolate({"k": "${FOO:?FOO is required}"}, {}) - - def test_colon_question_raises_when_empty(self) -> None: - with pytest.raises(UnsupportedComposeError, match="FOO is required"): - interpolate({"k": "${FOO:?FOO is required}"}, {"FOO": ""}) - - def test_colon_question_passes_when_set_and_nonempty(self) -> None: - document, _ = interpolate({"k": "${FOO:?FOO is required}"}, {"FOO": "bar"}) - assert document == {"k": "bar"} - - def test_question_raises_only_when_unset(self) -> None: - with pytest.raises(UnsupportedComposeError, match="FOO is required"): - interpolate({"k": "${FOO?FOO is required}"}, {}) - - def test_question_passes_when_set_empty(self) -> None: - document, _ = interpolate({"k": "${FOO?FOO is required}"}, {"FOO": ""}) - assert document == {"k": ""} - - def test_colon_plus_substitutes_alt_when_set_and_nonempty(self) -> None: - document, _ = interpolate({"k": "${FOO:+alt}"}, {"FOO": "bar"}) - assert document == {"k": "alt"} - - def test_colon_plus_blank_when_unset_or_empty(self) -> None: - assert interpolate({"k": "${FOO:+alt}"}, {})[0] == {"k": ""} - assert interpolate({"k": "${FOO:+alt}"}, {"FOO": ""})[0] == {"k": ""} - - def test_plus_substitutes_alt_when_set_even_empty(self) -> None: - document, _ = interpolate({"k": "${FOO+alt}"}, {"FOO": ""}) - assert document == {"k": "alt"} - - def test_plus_blank_when_unset(self) -> None: - document, _ = interpolate({"k": "${FOO+alt}"}, {}) - assert document == {"k": ""} - - def test_recurses_through_nested_dict_and_list(self) -> None: - document, _ = interpolate( - {"services": {"app": {"environment": ["A=$FOO", "B=${BAR}"]}}}, - {"FOO": "1", "BAR": "2"}, - ) - assert document == {"services": {"app": {"environment": ["A=1", "B=2"]}}} - - def test_keys_are_left_untouched(self) -> None: - document, _ = interpolate({"$FOO": "literal"}, {"FOO": "bar"}) - assert document == {"$FOO": "literal"} - - def test_non_string_leaves_pass_through(self) -> None: - document, _ = interpolate({"k": 1, "n": None, "b": True}, {}) - assert document == {"k": 1, "n": None, "b": True} - - def test_multiple_variables_in_one_string(self) -> None: - document, _ = interpolate({"k": "$FOO-${BAR}"}, {"FOO": "a", "BAR": "b"}) - assert document == {"k": "a-b"} diff --git a/tests/test_shell.py b/tests/test_shell.py new file mode 100644 index 0000000..2207e57 --- /dev/null +++ b/tests/test_shell.py @@ -0,0 +1,73 @@ +import os +import subprocess + +from compose2pod.shell import to_shell, variable_names + + +def _run_fragment(fragment: str, env: dict[str, str]) -> subprocess.CompletedProcess[str]: + # Execute the emitted fragment under the same `set -eu` the generated + # script uses, proving refs expand (or fail) at run time, not generation. + return subprocess.run( # noqa: S603 - fixed argv, test-controlled fragment + ["sh", "-euc", f"printf %s {fragment}"], # noqa: S607 - sh from PATH is intentional + env={"PATH": os.environ["PATH"], **env}, + capture_output=True, + text=True, + check=False, + ) + + +class TestToShell: + def test_plain_literal_is_double_quoted(self) -> None: + assert to_shell("python:3.12") == '"python:3.12"' + + def test_named_ref_becomes_dash_default(self) -> None: + assert to_shell("$FOO") == '"${FOO-}"' + + def test_braced_ref_becomes_dash_default(self) -> None: + assert to_shell("${FOO}") == '"${FOO-}"' + + def test_operator_forms_pass_through(self) -> None: + assert to_shell("${FOO:-d}") == '"${FOO:-d}"' + assert to_shell("${FOO-d}") == '"${FOO-d}"' + assert to_shell("${FOO:?m}") == '"${FOO:?m}"' + assert to_shell("${FOO?m}") == '"${FOO?m}"' + assert to_shell("${FOO:+a}") == '"${FOO:+a}"' + assert to_shell("${FOO+a}") == '"${FOO+a}"' + + def test_dollar_dollar_is_literal_dollar(self) -> None: + assert to_shell("$$") == '"\\$"' + + def test_specials_in_literal_are_escaped(self) -> None: + assert to_shell('a"b`c\\d') == '"a\\"b\\`c\\\\d"' + + def test_bare_dollar_not_a_ref_is_escaped(self) -> None: + assert to_shell("cost $5") == '"cost \\$5"' + + def test_default_text_is_escaped(self) -> None: + assert to_shell("${FOO:-a`b}") == '"${FOO:-a\\`b}"' + + def test_multiple_refs_and_literals(self) -> None: + assert to_shell("$A-${B}") == '"${A-}-${B-}"' + + def test_named_ref_expands_at_runtime(self) -> None: + frag = to_shell("MODEL=${LLM_MODEL}") + assert _run_fragment(frag, {}).stdout == "MODEL=" + assert _run_fragment(frag, {"LLM_MODEL": "gpt-4"}).stdout == "MODEL=gpt-4" + + def test_required_form_fails_under_shell_when_unset(self) -> None: + result = _run_fragment(to_shell("${VAR:?must be set}"), {}) + assert result.returncode != 0 + assert "must be set" in result.stderr + + def test_command_substitution_is_inert(self) -> None: + result = _run_fragment(to_shell("a$(echo pwned)b"), {}) + assert result.returncode == 0 + assert result.stdout == "a$(echo pwned)b" + + +class TestVariableNames: + def test_collects_names_excluding_escaped(self) -> None: + assert variable_names("A=$FOO ${BAR} $$X ${BAZ:-d}") == {"FOO", "BAR", "BAZ"} + + def test_empty_when_no_refs(self) -> None: + assert variable_names("plain") == set()