1 Commits
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
4 changed files with 91 additions and 16 deletions
+3
View File
@@ -133,6 +133,9 @@ 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
+60 -14
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,18 +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}/*")]
# A move from a nested route to `/` makes the old partition fall inside the retired_filters = retired_immutable_filters(artifact, route, html_dir, previous_path)
# new sync scope. Preserve declared immutable subtrees at every retired
# prefix without enumerating the bucket.
retired_immutable_filters = [
arg for path in immutable_paths for arg in ("--exclude", f"*/{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, *retired_immutable_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, *retired_immutable_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
@@ -303,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 = {
@@ -311,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.
@@ -319,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)
+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
+26 -2
View File
@@ -427,7 +427,6 @@ class PublishingTests(unittest.TestCase):
rendered = [" ".join(command) for command, _ in commands] rendered = [" ".join(command) for command, _ in commands]
self.assertIn("s3://baseline-dist/dist/", rendered[0]) self.assertIn("s3://baseline-dist/dist/", rendered[0])
self.assertIn("releases/*", rendered[0]) self.assertIn("releases/*", rendered[0])
self.assertIn("*/releases/*", rendered[0])
self.assertNotIn("--delete", rendered[0]) self.assertNotIn("--delete", rendered[0])
self.assertIn("--delete", rendered[1]) self.assertIn("--delete", rendered[1])
self.assertTrue(all("s3://baseline-dist/dist/" in command for command in rendered[1:])) self.assertTrue(all("s3://baseline-dist/dist/" in command for command in rendered[1:]))
@@ -440,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, \