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 49 additions and 249 deletions
+3 -6
View File
@@ -133,12 +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.
An append-only `site-publish-history.json` beside each generated site retains every bucket's access Generated Ingress annotations retain each artifact's prior route. When a move
class and absolute immutable prefixes, including removed routes and rules. When a route move places places that retired prefix inside the new sync scope, only its declared
a retired prefix inside the new sync scope, that subtree is excluded; a current-file collision immutable subtrees are excluded; a current-file collision fails publication.
fails publication. Protected access remains sticky across artifact renames and legacy mode, so
publishing the same artifact publicly requires a new bucket. Decommissioning removes the live
application and manifests while retaining this history because its Garage bucket is not purged.
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
+40 -126
View File
@@ -8,10 +8,12 @@ import os
import re import re
import shutil import shutil
import subprocess import subprocess
from pathlib import Path, PurePosixPath 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,
@@ -28,7 +30,6 @@ from utils import (
GARAGE_ADMIN_ENDPOINT = os.environ.get( GARAGE_ADMIN_ENDPOINT = os.environ.get(
"GARAGE_ADMIN_ENDPOINT", "http://garage.storage.svc:3903" "GARAGE_ADMIN_ENDPOINT", "http://garage.storage.svc:3903"
) )
HISTORY_FILE = "site-publish-history.json"
def validate_artifact_output(site_dir, artifact): def validate_artifact_output(site_dir, artifact):
@@ -177,40 +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 retired_immutable_filters(artifact, route, html_dir, previous_contract): def retired_immutable_filters(artifact, route, html_dir, previous_path):
"""Protect every historical immutable prefix inside the current sync scope.""" """Protect immutable keys only when an old route falls inside the new scope."""
if not previous_contract: if not previous_path or previous_path == route["path"]:
return [] return []
current_prefix = route["path"].strip("/") current_prefix = route["path"].strip("/")
filters = [] previous_prefix = previous_path.strip("/")
current_immutable = set(immutable_prefixes(artifact, route))
for immutable_prefix in previous_contract["immutable_prefixes"]:
if current_prefix: if current_prefix:
marker = f"{current_prefix}/" marker = f"{current_prefix}/"
if not immutable_prefix.startswith(marker): 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 continue
relative_path = immutable_prefix[len(marker):] retired_path = "/".join(part for part in (previous_prefix, rule["path"]) if part)
else: collision = html_dir / retired_path
relative_path = immutable_prefix if collision.exists() and any(path.is_file() for path in collision.rglob("*")):
collision = html_dir / relative_path
if (immutable_prefix not in current_immutable and collision.exists()
and any(path.is_file() for path in collision.rglob("*"))):
raise RuntimeError( raise RuntimeError(
f"current artifact collides with retired immutable partition: {relative_path}" f"current artifact collides with retired immutable partition: {retired_path}"
) )
filters.extend(("--exclude", f"{relative_path}/*")) filters.extend(("--exclude", f"{retired_path}/*"))
return filters return filters
def immutable_prefixes(artifact, route): def s3_sync(artifact, route, site_dir, credential_env_names=None, previous_path=None):
route_prefix = route["path"].strip("/")
return sorted({
"/".join(part for part in (route_prefix, rule["path"]) if part)
for rule in artifact["cache_rules"] if _is_immutable(rule)
})
def s3_sync(artifact, route, site_dir, credential_env_names=None, previous_contract=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)
@@ -238,7 +231,7 @@ def s3_sync(artifact, route, site_dir, credential_env_names=None, previous_contr
# 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_contract) 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, *retired_filters, *exclude_args], env=aws_env) *default_filters, *retired_filters, *exclude_args], env=aws_env)
@@ -332,97 +325,25 @@ 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_contracts(app_dir): def previous_route_paths(app_dir):
"""Read the append-only bucket history kept beside generated manifests.""" """Read artifact-to-route history from generated Ingress annotations."""
path = app_dir / HISTORY_FILE paths = {}
if not path.exists(): manifests = app_dir / "manifests"
return {} if not manifests.exists():
try: return paths
document = json.loads(path.read_text()) for path in sorted(manifests.glob("ingress*.yaml")):
except (OSError, json.JSONDecodeError) as exc: document = yaml.safe_load(path.read_text()) or {}
raise RuntimeError(f"invalid site-publish route history in {path}") from exc annotations = document.get("metadata", {}).get("annotations", {})
if (not isinstance(document, dict) or set(document) != {"schemaVersion", "buckets"} artifact = annotations.get("site-publish.fritzlab.net/artifact")
or document["schemaVersion"] != 1 or not isinstance(document["buckets"], dict)): 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}") raise RuntimeError(f"invalid site-publish route history in {path}")
for bucket, contract in document["buckets"].items(): if artifact in paths:
if (not isinstance(bucket, str) or not isinstance(contract, dict) raise RuntimeError(f"duplicate site-publish route history for artifact {artifact}")
or set(contract) != {"access", "artifact", "immutablePrefixes", "routePath"} paths[artifact] = route_path
or contract["access"] not in {"legacy", "protected", "public"} return paths
or not isinstance(contract["artifact"], str)
or not isinstance(contract["routePath"], str)
or not contract["routePath"].startswith("/")
or not isinstance(contract["immutablePrefixes"], list)
or any(not _valid_immutable_prefix(value)
for value in contract["immutablePrefixes"])
or len(contract["immutablePrefixes"]) != len(set(contract["immutablePrefixes"]))):
raise RuntimeError(f"invalid site-publish route history in {path}")
return {
bucket: {
"access": contract["access"],
"artifact": contract["artifact"],
"immutable_prefixes": sorted(contract["immutablePrefixes"]),
"path": contract["routePath"],
}
for bucket, contract in document["buckets"].items()
}
def next_route_contracts(cfg, previous_contracts):
"""Carry protected access and immutable prefixes forward for every known bucket."""
contracts = json.loads(json.dumps(previous_contracts))
artifacts = {artifact["name"]: artifact for artifact in cfg["artifacts"]}
for route in cfg["routes"]:
artifact = artifacts[route["artifact"]]
bucket = artifact["bucket"]
previous = previous_contracts.get(bucket)
access = "protected" if (
route["access"] == "protected" or previous and previous["access"] == "protected"
) else route["access"]
contracts[bucket] = {
"access": access,
"artifact": route["artifact"],
"immutable_prefixes": sorted(set(
(previous or {}).get("immutable_prefixes", []) + immutable_prefixes(artifact, route)
)),
"path": route["path"],
}
return contracts
def write_route_contracts(app_dir, contracts):
app_dir.mkdir(parents=True, exist_ok=True)
document = {
"schemaVersion": 1,
"buckets": {
bucket: {
"access": contract["access"],
"artifact": contract["artifact"],
"immutablePrefixes": contract["immutable_prefixes"],
"routePath": contract["path"],
}
for bucket, contract in sorted(contracts.items())
},
}
(app_dir / HISTORY_FILE).write_text(f"{json.dumps(document, indent=2, sort_keys=True)}\n")
def _valid_immutable_prefix(value):
return (isinstance(value, str) and value and not value.startswith("/")
and not value.endswith("/") and ".." not in PurePosixPath(value).parts)
def validate_route_migrations(cfg, previous_contracts):
artifacts = {artifact["name"]: artifact for artifact in cfg["artifacts"]}
for route in cfg["routes"]:
artifact = artifacts[route["artifact"]]
previous = previous_contracts.get(artifact["bucket"])
if (previous and previous["access"] == "protected"
and route["access"] in {"public", "legacy"}
):
raise RuntimeError(
f"artifact {route['artifact']} cannot become public while reusing protected "
f"bucket {artifact['bucket']}"
)
def deploy_static(site_name, site_dir, action_dir, token, cfg): def deploy_static(site_name, site_dir, action_dir, token, cfg):
@@ -436,8 +357,7 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
apps_dir = clone_apps(token) apps_dir = clone_apps(token)
app_dir = apps_dir / "sjc001" / "websites" / site_name app_dir = apps_dir / "sjc001" / "websites" / site_name
manifests_dir = app_dir / "manifests" manifests_dir = app_dir / "manifests"
previous_contracts = previous_route_contracts(app_dir) previous_paths = previous_route_paths(app_dir)
validate_route_migrations(cfg, previous_contracts)
# 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.
@@ -448,12 +368,11 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
for route in cfg["routes"]: for route in cfg["routes"]:
s3_sync( s3_sync(
artifact_by_name[route["artifact"]], route, site_dir, credential_env_names, artifact_by_name[route["artifact"]], route, site_dir, credential_env_names,
previous_contracts.get(artifact_by_name[route["artifact"]]["bucket"]), 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"))
write_route_contracts(app_dir, next_route_contracts(cfg, previous_contracts))
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)
@@ -466,12 +385,7 @@ def decommission(site_name, token, buckets=None):
if not site_path.exists(): if not site_path.exists():
print(f"No manifests for {site_name} — nothing to remove") print(f"No manifests for {site_name} — nothing to remove")
return return
history_path = site_path / HISTORY_FILE
history = history_path.read_bytes() if history_path.exists() else None
shutil.rmtree(site_path) shutil.rmtree(site_path)
if history is not None:
site_path.mkdir(parents=True)
(site_path / HISTORY_FILE).write_bytes(history)
commit_and_push(apps_dir, f"Decommission {site_name}", token) commit_and_push(apps_dir, f"Decommission {site_name}", token)
for bucket in buckets or [site_name]: for bucket in buckets or [site_name]:
print(f"Bucket {bucket} and its objects are NOT purged automatically.") print(f"Bucket {bucket} and its objects are NOT purged automatically.")
+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
+2 -115
View File
@@ -273,18 +273,6 @@ class GenerationTests(unittest.TestCase):
self.assertNotIn("passhostheader", files["manifests/ingress-portal.yaml"]) self.assertNotIn("passhostheader", files["manifests/ingress-portal.yaml"])
self.assertIn("baseline-dist.web.sjc001.fritzlab.net", files["manifests/service-distributions.yaml"]) self.assertIn("baseline-dist.web.sjc001.fritzlab.net", files["manifests/service-distributions.yaml"])
def test_history_keeps_yaml_ambiguous_artifact_names_as_strings(self):
raw = fixture("split-site.yaml")
raw["artifacts"][0]["name"] = "yes"
raw["routes"][0]["artifact"] = "yes"
cfg = normalize_site_config(raw, "baseline.fritzlab.net")
contracts = deploy.next_route_contracts(cfg, {})
with tempfile.TemporaryDirectory() as tmp:
app_dir = Path(tmp)
deploy.write_route_contracts(app_dir, contracts)
restored = deploy.previous_route_contracts(app_dir)
self.assertEqual("yes", restored["baseline-portal"]["artifact"])
def test_generation_is_deterministic_when_input_lists_are_reversed(self): def test_generation_is_deterministic_when_input_lists_are_reversed(self):
raw = fixture("split-site.yaml") raw = fixture("split-site.yaml")
first_tmp, _, first = self.render(raw) first_tmp, _, first = self.render(raw)
@@ -308,14 +296,12 @@ class GenerationTests(unittest.TestCase):
self.assertFalse(stale.exists()) self.assertFalse(stale.exists())
def test_legacy_names_and_garage_s3_target_are_preserved(self): def test_legacy_names_and_garage_s3_target_are_preserved(self):
tmp, app_dir, files = self.render(fixture("legacy-site.yaml")) tmp, _, files = self.render(fixture("legacy-site.yaml"))
self.addCleanup(tmp.cleanup) self.addCleanup(tmp.cleanup)
self.assertIn("manifests/service.yaml", files) self.assertIn("manifests/service.yaml", files)
self.assertIn("manifests/ingress.yaml", files) self.assertIn("manifests/ingress.yaml", files)
self.assertIn("garage-s3.storage.svc.k8s.sjc001.fritzlab.net", files["manifests/service.yaml"]) self.assertIn("garage-s3.storage.svc.k8s.sjc001.fritzlab.net", files["manifests/service.yaml"])
self.assertNotIn("passhostheader", files["manifests/ingress.yaml"]) self.assertNotIn("passhostheader", files["manifests/ingress.yaml"])
self.assertNotIn("site-publish.fritzlab.net", files["manifests/ingress.yaml"])
self.assertEqual({}, deploy.previous_route_contracts(app_dir))
class BuildTests(unittest.TestCase): class BuildTests(unittest.TestCase):
@@ -472,110 +458,11 @@ class PublishingTests(unittest.TestCase):
}, clear=False), patch.object( }, clear=False), patch.object(
deploy, "run", side_effect=lambda command, **_: commands.append(command) deploy, "run", side_effect=lambda command, **_: commands.append(command)
): ):
deploy.s3_sync(artifact, route, root, previous_contract={ deploy.s3_sync(artifact, route, root, previous_path="/foo")
"path": "/foo", "access": "public", "artifact": "distributions",
"immutable_prefixes": ["foo/releases"],
})
rendered = [" ".join(command) for command in commands] rendered = [" ".join(command) for command in commands]
self.assertTrue(all("foo/releases/*" in command for command in rendered[:2])) self.assertTrue(all("foo/releases/*" in command for command in rendered[:2]))
self.assertTrue(all("*/releases/*" not in command for command in rendered)) self.assertTrue(all("*/releases/*" not in command for command in rendered))
def test_protected_bucket_cannot_become_public_across_deployments(self):
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
previous = {
"baseline-dist": {
"path": "/dist", "access": "protected", "artifact": "old-name",
"immutable_prefixes": ["dist/releases"],
}
}
with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
deploy.validate_route_migrations(cfg, previous)
renamed = copy.deepcopy(cfg)
artifact = next(item for item in renamed["artifacts"] if item["name"] == "distributions")
artifact["name"] = "downloads"
route = next(item for item in renamed["routes"] if item["artifact"] == "distributions")
route["artifact"] = "downloads"
with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
deploy.validate_route_migrations(renamed, previous)
previous = {"retired-protected-bucket": previous["baseline-dist"]}
deploy.validate_route_migrations(cfg, previous)
def test_protected_split_bucket_cannot_become_legacy_public(self):
cfg = normalize_site_config(fixture("legacy-site.yaml"), "baseline.fritzlab.net")
previous = {
"baseline.fritzlab.net": {
"path": "/portal", "access": "protected", "artifact": "portal",
"immutable_prefixes": [],
}
}
with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
deploy.validate_route_migrations(cfg, previous)
def test_removed_immutable_rule_preserves_prior_keys(self):
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
artifact = next(item for item in cfg["artifacts"] if item["name"] == "distributions")
artifact["cache_rules"] = [
rule for rule in artifact["cache_rules"] if rule["path"] != "releases"
]
route = next(item for item in cfg["routes"] if item["artifact"] == "distributions")
with tempfile.TemporaryDirectory() as tmp:
html = Path(tmp)
filters = deploy.retired_immutable_filters(artifact, route, html, {
"path": "/dist", "access": "public", "artifact": "distributions",
"immutable_prefixes": ["dist/releases"],
})
self.assertEqual(["--exclude", "releases/*"], filters)
previous = {
"baseline-dist": {
"path": "/dist", "access": "public", "artifact": "distributions",
"immutable_prefixes": ["dist/releases"],
}
}
next_contracts = deploy.next_route_contracts(cfg, previous)
self.assertEqual(
["dist/releases"], next_contracts["baseline-dist"]["immutable_prefixes"],
)
self.assertEqual(
["--exclude", "releases/*"],
deploy.retired_immutable_filters(
artifact, route, html, next_contracts["baseline-dist"],
),
)
(html / "releases").mkdir()
(html / "releases" / "replacement.js").write_text("mutable")
with self.assertRaisesRegex(RuntimeError, "collides with retired immutable"):
deploy.retired_immutable_filters(artifact, route, html, {
"path": "/dist", "access": "public", "artifact": "distributions",
"immutable_prefixes": ["dist/releases"],
})
def test_removed_route_history_remains_append_only(self):
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
previous = {
"retired-bucket": {
"path": "/retired", "access": "protected", "artifact": "retired",
"immutable_prefixes": ["retired/releases"],
}
}
contracts = deploy.next_route_contracts(cfg, previous)
self.assertEqual(previous["retired-bucket"], contracts["retired-bucket"])
def test_decommission_preserves_history_for_an_unpurged_bucket(self):
with tempfile.TemporaryDirectory() as tmp:
apps = Path(tmp)
site = apps / "sjc001/websites/baseline"
site.mkdir(parents=True)
history = b'{"schemaVersion":1,"buckets":{}}\n'
(site / deploy.HISTORY_FILE).write_bytes(history)
(site / "app.yaml").write_text("live\n")
with patch.object(deploy, "clone_apps", return_value=apps), patch.object(
deploy, "commit_and_push"
) as commit:
deploy.decommission("baseline", "token", ["baseline-dist"])
self.assertEqual(history, (site / deploy.HISTORY_FILE).read_bytes())
self.assertFalse((site / "app.yaml").exists())
commit.assert_called_once_with(apps, "Decommission baseline", "token")
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: