-
Notifications
You must be signed in to change notification settings - Fork 0
fix(deploy): isolate Quadlet environments safely #349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e4e5780
919f22a
9c05821
d1fca46
cfc95e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,9 @@ | ||
| """Static deployment-contract tests for the systemd Quadlet templates.""" | ||
| from pathlib import Path | ||
| import os | ||
| import re | ||
| import subprocess | ||
| import tempfile | ||
|
|
||
|
|
||
| ROOT = Path(__file__).resolve().parent.parent | ||
|
|
@@ -44,7 +47,8 @@ def test_rendered_quadlets_are_owner_only(): | |
| script = (ROOT / "start-stack.sh").read_text() | ||
| assert 'chmod 700 "$QUADLET_DIR"' in script | ||
| assert "os.chmod(staged_path, 0o600)" in script | ||
| assert 'LLM_ROUTING_POD_UNIT="llm-routing-pod.service"' in script | ||
| assert 'LLM_ROUTING_POD_UNIT="${QUADLET_NAMESPACE}-pod.service"' in script | ||
| assert 'QUADLET_DIR="${HOME}/.config/containers/systemd/${QUADLET_NAMESPACE}"' in script | ||
| assert "systemctl --user daemon-reload" in script | ||
| assert "stack_ownership()" in script | ||
| assert "PODMAN_SYSTEMD_UNIT" in script | ||
|
|
@@ -87,3 +91,71 @@ def test_quadlet_deployment_enforces_prerequisite_and_restart_failures(): | |
| assert "require_user_systemd || exit 1" in script | ||
| assert "failed to derive external service URLs for Quadlet rendering" in script | ||
| assert 'if ! podman pod restart "${POD_NAME}"; then' in script | ||
|
|
||
|
|
||
| def test_quadlet_namespace_is_environment_specific(): | ||
| dev_env = (ROOT / ".env.dev").read_text() | ||
| prod_namespace = "llm-routing-prod" | ||
| assert 'QUADLET_NAMESPACE="llm-routing-dev"' in dev_env | ||
| assert 'QUADLET_NAMESPACE="${QUADLET_NAMESPACE:-llm-routing-prod}"' in (ROOT / "start-stack.sh").read_text() | ||
| assert prod_namespace in (ROOT / "start-stack.sh").read_text() | ||
| script = (ROOT / "start-stack.sh").read_text() | ||
| assert 'def namespace_identifier(match):' in script | ||
| assert 'identifier_suffixes = (' in script | ||
| assert 'identifier_prefix.sub(namespace + "-", value)' in script | ||
| assert 'rendered_name = os.path.basename(tpl).replace("llm-routing", namespace)' in script | ||
|
Comment on lines
+96
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Strengthen environment-specific behavior coverage by exercising rendering results rather than only script text. This test only checks for specific strings in
Suggested implementation: def test_quadlet_namespace_is_environment_specific():
dev_env = (ROOT / ".env.dev").read_text()
script = (ROOT / "start-stack.sh").read_text()
# Structural checks: ensure environment variables and script wiring are correct.
dev_namespace = "llm-routing-dev"
prod_namespace = "llm-routing-prod"
assert f'QUADLET_NAMESPACE="{dev_namespace}"' in dev_env
assert 'QUADLET_NAMESPACE="${QUADLET_NAMESPACE:-llm-routing-prod}"' in script
assert prod_namespace in script
assert 'r"llm-routing-(?=(?:pod|clickhouse|langfuse|litellm|minio|postgres|router|valkey))"' in script
assert 'text = text.replace("llm-routing.pod", namespace + ".pod")' in script
assert 'rendered_name = os.path.basename(tpl).replace("llm-routing", namespace)' in script
# Behavioral checks: exercise template rendering for both dev and prod namespaces
# and assert on the rendered unit names.
#
# Use the same helper that start-stack.sh uses to render Quadlet templates; this
# should return a mapping of template path -> rendered unit name (or similar).
dev_rendered_units = render_quadlet_templates(namespace=dev_namespace)
prod_rendered_units = render_quadlet_templates(namespace=prod_namespace)
# 1. Dev units use llm-routing-dev-* and prod units llm-routing-prod-*
for name in dev_rendered_units.values():
if "llm-routing-" in name:
assert name.startswith(f"{dev_namespace}-"), f"dev unit not namespaced: {name}"
for name in prod_rendered_units.values():
if "llm-routing-" in name:
assert name.startswith(f"{prod_namespace}-"), f"prod unit not namespaced: {name}"
# 2. The regex r"llm-routing-(?=(?:pod|clickhouse|langfuse|litellm|minio|postgres|router|valkey))"
# only affects the intended identifiers.
expected_suffixes = {
"pod",
"clickhouse",
"langfuse",
"litellm",
"minio",
"postgres",
"router",
"valkey",
}
for name in dev_rendered_units.values():
if name.startswith("llm-routing-"):
suffix = name.removeprefix("llm-routing-").split(".")[0]
assert suffix in expected_suffixes, f"unexpected dev identifier rewritten by regex: {name}"
for name in prod_rendered_units.values():
if name.startswith("llm-routing-"):
suffix = name.removeprefix("llm-routing-").split(".")[0]
assert suffix in expected_suffixes, f"unexpected prod identifier rewritten by regex: {name}"
# 3. Legacy/non-namespaced identifiers remain unchanged.
# Assuming we have some known legacy unit(s) that should keep their original name.
# Adjust the expected_legacy_units list to match your actual legacy units.
expected_legacy_units = {
"llm-routing.pod", # example; replace with real legacy identifiers
}
for legacy in expected_legacy_units:
assert legacy in dev_rendered_units.values(), f"legacy unit missing or renamed in dev: {legacy}"
assert legacy in prod_rendered_units.values(), f"legacy unit missing or renamed in prod: {legacy}"The above test assumes the existence of a helper function
|
||
|
|
||
|
|
||
| def test_namespace_rendering_preserves_non_identifiers(): | ||
| script = (ROOT / "start-stack.sh").read_text() | ||
| embedded = script.split("python3 - \"$src_dir\" \"$QUADLET_DIR\" <<'PY'\n", 1)[1].split("\nPY\n", 1)[0] | ||
| env = os.environ.copy() | ||
| values = { | ||
| "POSTGRES_PASSWORD": "pg", "WORKDIR": str(ROOT), "HOME": str(ROOT), | ||
| "LITELLM_MASTER_KEY": "master", "NEXTAUTH_SECRET": "next", "SALT": "salt", | ||
| "ENCRYPTION_KEY": "encrypt", "OLLAMA_API_KEY": "ollama", "OPENROUTER_API_KEY": "openrouter", | ||
| "LANGFUSE_PUBLIC_KEY": "public", "LANGFUSE_SECRET_KEY": "secret", "MINIO_ROOT_USER": "minio", | ||
| "MINIO_ROOT_PASSWORD": "minio-pass", "LANGFUSE_INIT_USER_PASSWORD": "lf-pass", | ||
| "REDIS_AUTH": "redis", "CLICKHOUSE_PASSWORD": "click", "PROXY_BASE_URL_DERIVED": "https://proxy", | ||
| "NEXTAUTH_URL_DERIVED": "https://next", "PUBLIC_BASE_URL": "https://host/llm-routing", | ||
| "ROUTING_DOMAIN": "vendeuvre.lan", "LLAMA_CLASSIFIER_URL": "http://127.0.0.1:8083/v1", | ||
| "LLAMA_SERVER_URL": "http://127.0.0.1:8083", "POD_NAME": "dev-router-pod", | ||
| "DATA_ROOT": str(ROOT / "data"), "ROUTER_IMAGE": "registry/llm-routing-router:latest", | ||
| "ROUTER_PORT": "5010", "LITELLM_PORT": "4010", "LANGFUSE_WEB_PORT": "3011", | ||
| "LANGFUSE_WORKER_PORT": "3030", "POSTGRES_PORT": "5442", "VALKEY_CACHE_PORT": "6389", | ||
| "VALKEY_LF_PORT": "6390", "CLICKHOUSE_HTTP_PORT": "8123", "CLICKHOUSE_TCP_PORT": "9003", | ||
| "MINIO_S3_PORT": "9002", "MINIO_CONSOLE_PORT": "9001", "QUADLET_NAMESPACE": "llm-routing-dev", | ||
| } | ||
| env.update(values) | ||
| with tempfile.TemporaryDirectory() as tmp: | ||
| src, out = Path(tmp) / "src", Path(tmp) / "out" | ||
| src.mkdir(); out.mkdir() | ||
| (src / "llm-routing.pod").write_text("[Pod]\nPodName=llm-routing.pod\n") | ||
| (src / "llm-routing-router.container").write_text( | ||
| "[Unit]\nAfter=llm-routing-litellm.service\n[Container]\n" | ||
| "Image=registry/llm-routing-router:latest\n" | ||
| "Environment=PUBLIC_BASE_URL=https://host/llm-routing-router\nPod=llm-routing.pod\n" | ||
| ) | ||
| subprocess.run(["python3", "-c", embedded, str(src), str(out)], env=env, check=True, capture_output=True, text=True) | ||
| rendered = (out / "llm-routing-dev-router.container").read_text() | ||
| assert "After=llm-routing-dev-litellm.service" in rendered | ||
| assert "Image=registry/llm-routing-router:latest" in rendered | ||
| assert "PUBLIC_BASE_URL=https://host/llm-routing-router" in rendered | ||
| assert "Pod=llm-routing-dev.pod" in rendered | ||
|
|
||
|
|
||
| def test_namespace_is_validated_and_ownership_preserves_exact_unit(): | ||
| script = (ROOT / "start-stack.sh").read_text() | ||
| assert '[[ ! "$QUADLET_NAMESPACE" =~ ^[a-z0-9][a-z0-9-]*$ ]]' in script | ||
| assert "printf 'quadlet:%s\\n' \"$infra_unit\"" in script | ||
| assert 'owner_unit="${STACK_OWNERSHIP#quadlet:}"' in script | ||
|
Comment on lines
+147
to
+151
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (testing): The namespace validation assertion seems to miss the full expected expression, which could make the test misleading. In assert 'QUADLET_NAMESPACE" =~ ^[a-z0-9][a-z0-9-]*$' in scriptmisses the leading |
||
| assert 'failed to restart ${owner_unit}' in script | ||
| assert 'status ${owner_unit} --no-pager' in script | ||
|
|
||
|
|
||
| def test_documentation_uses_environment_specific_units(): | ||
| readme = (ROOT / "README.md").read_text() | ||
| scripts_readme = (ROOT / "scripts" / "README.md").read_text() | ||
| assert "systemctl --user status llm-routing-prod-pod.service" in readme | ||
| assert "llm-routing-dev-pod.service" in readme | ||
| assert "llm-routing-pod.service" not in scripts_readme | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Failure diagnostics still reference the non-legacy unit even when restarting a legacy one.
Here you restart
owner_unit, but the error and hint messages still useLLM_ROUTING_POD_UNIT. Ifowner_unitis a legacy unit, diagnostics will point to the wrong unit. Useowner_unitin these messages so they accurately reflect the unit being restarted.