Author SHA1 Message Date
Evelyn Chen 328d94c44f fix(site-publish): close split migration boundaries
Test / contract (pull_request) Successful in 6s
Authored-By: OpenAI (GPT-5) <noreply@openai.com>
2026-08-29 22:47:24 +00:00
5 changed files with 111 additions and 11 deletions
+4 -1
View File
@@ -133,10 +133,13 @@ any route's mutable objects change.
Mutable default and override partitions receive their final cache policy before Mutable default and override partitions receive their final cache policy before
the matching prefix-scoped stale deletion, so publication never exposes a the matching prefix-scoped stale deletion, so publication never exposes a
provisional cache policy or a pointer to a missing immutable target. provisional cache policy or a pointer to a missing immutable target.
Generated Ingress annotations retain each artifact's prior route. When a move
places that retired prefix inside the new sync scope, only its declared
immutable subtrees are excluded; a current-file collision fails publication.
Artifact input directories must be pairwise disjoint after filesystem Artifact input directories must be pairwise disjoint after filesystem
resolution. Publication stops before build or upload if one contains another or resolution. Publication stops before build or upload if one contains another or
escapes the repository. Descendant symlinks are also rejected, preventing escapes the repository. Symlinked roots, components, and descendants are also rejected, preventing
protected input from entering a public artifact through dereference. Split protected input from entering a public artifact through dereference. Split
storage endpoints are pinned to Garage, and each website storage endpoints are pinned to Garage, and each website
authority is derived from its bucket; a site cannot expose an arbitrary backend. authority is derived from its bucket; a site cannot expose an arbitrary backend.
+60 -8
View File
@@ -12,6 +12,8 @@ from pathlib import Path
from urllib.error import HTTPError, URLError from urllib.error import HTTPError, URLError
from urllib.request import Request, urlopen from urllib.request import Request, urlopen
import yaml
from utils import ( from utils import (
NAMESPACE, NAMESPACE,
clone_apps, clone_apps,
@@ -176,7 +178,32 @@ def publish_route_immutables(artifact, route, site_dir, credential_env_names=Non
publish_immutable_rule(artifact, route, rule, html_dir, aws_env) publish_immutable_rule(artifact, route, rule, html_dir, aws_env)
def s3_sync(artifact, route, site_dir, credential_env_names=None): def retired_immutable_filters(artifact, route, html_dir, previous_path):
"""Protect immutable keys only when an old route falls inside the new scope."""
if not previous_path or previous_path == route["path"]:
return []
current_prefix = route["path"].strip("/")
previous_prefix = previous_path.strip("/")
if current_prefix:
marker = f"{current_prefix}/"
if not previous_prefix.startswith(marker):
return []
previous_prefix = previous_prefix[len(marker):]
filters = []
for rule in artifact["cache_rules"]:
if not _is_immutable(rule):
continue
retired_path = "/".join(part for part in (previous_prefix, rule["path"]) if part)
collision = html_dir / retired_path
if collision.exists() and any(path.is_file() for path in collision.rglob("*")):
raise RuntimeError(
f"current artifact collides with retired immutable partition: {retired_path}"
)
filters.extend(("--exclude", f"{retired_path}/*"))
return filters
def s3_sync(artifact, route, site_dir, credential_env_names=None, previous_path=None):
endpoint = artifact["s3_endpoint"] endpoint = artifact["s3_endpoint"]
html_dir = site_dir / artifact["build_dir"] html_dir = site_dir / artifact["build_dir"]
aws_env = publication_aws_env(artifact, credential_env_names) aws_env = publication_aws_env(artifact, credential_env_names)
@@ -204,12 +231,13 @@ def s3_sync(artifact, route, site_dir, credential_env_names=None):
# so a fresh upload always carries the right MIME type. # so a fresh upload always carries the right MIME type.
specific_paths = [rule["path"] for rule in artifact["cache_rules"] if rule["path"]] specific_paths = [rule["path"] for rule in artifact["cache_rules"] if rule["path"]]
default_filters = [arg for path in specific_paths for arg in ("--exclude", f"{path}/*")] default_filters = [arg for path in specific_paths for arg in ("--exclude", f"{path}/*")]
retired_filters = retired_immutable_filters(artifact, route, html_dir, previous_path)
run(["aws", "--endpoint-url", endpoint, "s3", "cp", f"{html_dir}/", destination, run(["aws", "--endpoint-url", endpoint, "s3", "cp", f"{html_dir}/", destination,
"--recursive", "--only-show-errors", "--cache-control", default_cache, "--recursive", "--only-show-errors", "--cache-control", default_cache,
*default_filters, *exclude_args], env=aws_env) *default_filters, *retired_filters, *exclude_args], env=aws_env)
run(["aws", "--endpoint-url", endpoint, "s3", "sync", f"{html_dir}/", destination, run(["aws", "--endpoint-url", endpoint, "s3", "sync", f"{html_dir}/", destination,
"--delete", "--only-show-errors", "--cache-control", default_cache, "--delete", "--only-show-errors", "--cache-control", default_cache,
*default_filters, *exclude_args], env=aws_env) *default_filters, *retired_filters, *exclude_args], env=aws_env)
for rule in artifact["cache_rules"]: for rule in artifact["cache_rules"]:
if not rule["path"]: if not rule["path"]:
continue continue
@@ -297,6 +325,27 @@ def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg):
render_templates(action_dir, template_vars, app_dir, manifests_dir) render_templates(action_dir, template_vars, app_dir, manifests_dir)
def previous_route_paths(app_dir):
"""Read artifact-to-route history from generated Ingress annotations."""
paths = {}
manifests = app_dir / "manifests"
if not manifests.exists():
return paths
for path in sorted(manifests.glob("ingress*.yaml")):
document = yaml.safe_load(path.read_text()) or {}
annotations = document.get("metadata", {}).get("annotations", {})
artifact = annotations.get("site-publish.fritzlab.net/artifact")
route_path = annotations.get("site-publish.fritzlab.net/route-path")
if artifact is None and route_path is None:
continue
if not isinstance(artifact, str) or not isinstance(route_path, str) or not route_path.startswith("/"):
raise RuntimeError(f"invalid site-publish route history in {path}")
if artifact in paths:
raise RuntimeError(f"duplicate site-publish route history for artifact {artifact}")
paths[artifact] = route_path
return paths
def deploy_static(site_name, site_dir, action_dir, token, cfg): def deploy_static(site_name, site_dir, action_dir, token, cfg):
artifact_by_name = {artifact["name"]: artifact for artifact in cfg["artifacts"]} artifact_by_name = {artifact["name"]: artifact for artifact in cfg["artifacts"]}
credential_env_names = { credential_env_names = {
@@ -305,6 +354,10 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
validate_publication_environment(cfg) validate_publication_environment(cfg)
for artifact in cfg["artifacts"]: for artifact in cfg["artifacts"]:
validate_artifact_output(site_dir, artifact) validate_artifact_output(site_dir, artifact)
apps_dir = clone_apps(token)
app_dir = apps_dir / "sjc001" / "websites" / site_name
manifests_dir = app_dir / "manifests"
previous_paths = previous_route_paths(app_dir)
# Complete immutable work across the whole publication before any route's # Complete immutable work across the whole publication before any route's
# mutable pointers can change. Partial immutable success is safe; mixing a # mutable pointers can change. Partial immutable success is safe; mixing a
# new route with an old route after a later immutable failure is not. # new route with an old route after a later immutable failure is not.
@@ -313,14 +366,13 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
artifact_by_name[route["artifact"]], route, site_dir, credential_env_names, artifact_by_name[route["artifact"]], route, site_dir, credential_env_names,
) )
for route in cfg["routes"]: for route in cfg["routes"]:
s3_sync(artifact_by_name[route["artifact"]], route, site_dir, credential_env_names) s3_sync(
artifact_by_name[route["artifact"]], route, site_dir, credential_env_names,
previous_paths.get(route["artifact"]),
)
if cfg["compatibility"]: if cfg["compatibility"]:
ensure_bucket_aliases(site_name, cfg["aliases"], os.environ.get("GARAGE_ADMIN_TOKEN")) ensure_bucket_aliases(site_name, cfg["aliases"], os.environ.get("GARAGE_ADMIN_TOKEN"))
apps_dir = clone_apps(token)
app_dir = apps_dir / "sjc001" / "websites" / site_name
manifests_dir = app_dir / "manifests"
render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg) render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg)
commit_and_push(apps_dir, f"Deploy {site_name}", token) commit_and_push(apps_dir, f"Deploy {site_name}", token)
+9 -1
View File
@@ -432,7 +432,15 @@ def validate_artifact_inputs(site_dir, cfg):
root = Path(site_dir).resolve() root = Path(site_dir).resolve()
sources = [] sources = []
for artifact in cfg["artifacts"]: for artifact in cfg["artifacts"]:
source = (root / artifact["content_dir"]).resolve() declared = root
for component in Path(artifact["content_dir"]).parts:
declared /= component
if declared.is_symlink():
raise ConfigError(
f"artifact {artifact['name']} content_dir contains symlink component: "
f"{declared.relative_to(root)}"
)
source = declared.resolve()
if source != root and root not in source.parents: if source != root and root not in source.parents:
raise ConfigError( raise ConfigError(
f"artifact {artifact['name']} content_dir resolves outside the repository" f"artifact {artifact['name']} content_dir resolves outside the repository"
+2
View File
@@ -4,6 +4,8 @@ metadata:
name: {{ route.resource_name }} name: {{ route.resource_name }}
namespace: {{ namespace }} namespace: {{ namespace }}
annotations: annotations:
site-publish.fritzlab.net/artifact: {{ route.artifact }}
site-publish.fritzlab.net/route-path: {{ route.path | tojson }}
traefik.ingress.kubernetes.io/router.middlewares: https-redirect@file,retry-upstream@file{% if route.access_middleware %},{{ route.access_middleware }}@file{% endif %}{% for m in route.middlewares %},{{ m }}@file{% endfor %} traefik.ingress.kubernetes.io/router.middlewares: https-redirect@file,retry-upstream@file{% if route.access_middleware %},{{ route.access_middleware }}@file{% endif %}{% for m in route.middlewares %},{{ m }}@file{% endfor %}
spec: spec:
ingressClassName: traefik ingressClassName: traefik
+36 -1
View File
@@ -230,6 +230,16 @@ class ConfigContractTests(unittest.TestCase):
with self.assertRaisesRegex(ConfigError, "build input contains symlink"): with self.assertRaisesRegex(ConfigError, "build input contains symlink"):
validate_artifact_inputs(root, cfg) validate_artifact_inputs(root, cfg)
def test_artifact_root_symlink_is_rejected_before_resolution(self):
cfg = normalize_site_config(self.raw, "baseline.fritzlab.net")
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
(root / "dist-real").mkdir()
(root / "dist").symlink_to(root / "dist-real")
(root / "portal" / "build").mkdir(parents=True)
with self.assertRaisesRegex(ConfigError, "content_dir contains symlink component"):
validate_artifact_inputs(root, cfg)
class GenerationTests(unittest.TestCase): class GenerationTests(unittest.TestCase):
def render(self, raw): def render(self, raw):
@@ -429,12 +439,37 @@ class PublishingTests(unittest.TestCase):
immutable_publish.call_args.args[2]["cache_control"], immutable_publish.call_args.args[2]["cache_control"],
) )
def test_root_move_preserves_only_actual_retired_immutable_prefix(self):
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
artifact = next(item for item in cfg["artifacts"] if item["name"] == "distributions")
route = {**next(item for item in cfg["routes"] if item["artifact"] == "distributions"),
"path": "/"}
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
html = root / artifact["build_dir"]
(html / "releases").mkdir(parents=True)
(html / "channels").mkdir()
(html / "docs" / "releases").mkdir(parents=True)
(html / "channels" / "stable.json").write_text("channel")
(html / "docs" / "releases" / "index.html").write_text("mutable")
commands = []
with patch.dict(os.environ, {
"DIST_S3_ACCESS_KEY": "dist-key", "DIST_S3_SECRET_KEY": "dist-secret"
}, clear=False), patch.object(
deploy, "run", side_effect=lambda command, **_: commands.append(command)
):
deploy.s3_sync(artifact, route, root, previous_path="/foo")
rendered = [" ".join(command) for command in commands]
self.assertTrue(all("foo/releases/*" in command for command in rendered[:2]))
self.assertTrue(all("*/releases/*" not in command for command in rendered))
def test_later_route_immutable_failure_stops_all_mutable_publication(self): def test_later_route_immutable_failure_stops_all_mutable_publication(self):
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net") cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
with tempfile.TemporaryDirectory() as tmp: with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp) root = Path(tmp)
with patch.object(deploy, "validate_publication_environment"), \ with patch.object(deploy, "validate_publication_environment"), \
patch.object(deploy, "validate_artifact_output"), patch.object( patch.object(deploy, "validate_artifact_output"), \
patch.object(deploy, "clone_apps", return_value=root / "apps"), patch.object(
deploy, "publish_route_immutables", deploy, "publish_route_immutables",
side_effect=[None, RuntimeError("immutable failed")], side_effect=[None, RuntimeError("immutable failed")],
) as immutable_publish, patch.object(deploy, "s3_sync") as mutable_sync, \ ) as immutable_publish, patch.object(deploy, "s3_sync") as mutable_sync, \