fix(site-publish): close split migration boundaries

Authored-By: OpenAI (GPT-5) <noreply@openai.com>

README.md

@@ -133,12 +133,11 @@ any route's mutable objects change.
 Mutable default and override partitions receive their final cache policy before
 the matching prefix-scoped stale deletion, so publication never exposes a
 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
-class and absolute immutable prefixes, including removed routes and rules. When a route move places
-a retired prefix inside the new sync scope, that subtree is excluded; a current-file collision
-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.
+Generated Ingress annotations retain each bucket'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.
+The bucket-keyed history rejects a protected-to-public transition even when the
+artifact is renamed; publishing that artifact publicly requires a new bucket.
 
 Artifact input directories must be pairwise disjoint after filesystem
 resolution. Publication stops before build or upload if one contains another or

scripts/deploy.py

@@ -8,10 +8,12 @@ import os
 import re
 import shutil
 import subprocess
-from pathlib import Path, PurePosixPath
+from pathlib import Path
 from urllib.error import HTTPError, URLError
 from urllib.request import Request, urlopen
 
+import yaml
+
 from utils import (
     NAMESPACE,
     clone_apps,
@@ -28,7 +30,6 @@ from utils import (
 GARAGE_ADMIN_ENDPOINT = os.environ.get(
     "GARAGE_ADMIN_ENDPOINT", "http://garage.storage.svc:3903"
 )
-HISTORY_FILE = "site-publish-history.json"
 
 
 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)
 
 
-def retired_immutable_filters(artifact, route, html_dir, previous_contract):
-    """Protect every historical immutable prefix inside the current sync scope."""
-    if not previous_contract:
+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("/")
-    filters = []
-    current_immutable = set(immutable_prefixes(artifact, route))
-    for immutable_prefix in previous_contract["immutable_prefixes"]:
+    previous_prefix = previous_path.strip("/")
     if 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
-            relative_path = immutable_prefix[len(marker):]
-        else:
-            relative_path = immutable_prefix
-        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("*"))):
+        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: {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
 
 
-def immutable_prefixes(artifact, route):
-    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):
+def s3_sync(artifact, route, site_dir, credential_env_names=None, previous_path=None):
     endpoint = artifact["s3_endpoint"]
     html_dir = site_dir / artifact["build_dir"]
     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.
     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}/*")]
-    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,
          "--recursive", "--only-show-errors", "--cache-control", default_cache,
          *default_filters, *retired_filters, *exclude_args], env=aws_env)
@@ -333,82 +326,28 @@ def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg):
 
 
 def previous_route_contracts(app_dir):
-    """Read the append-only bucket history kept beside generated manifests."""
-    path = app_dir / HISTORY_FILE
-    if not path.exists():
-        return {}
-    try:
-        document = json.loads(path.read_text())
-    except (OSError, json.JSONDecodeError) as exc:
-        raise RuntimeError(f"invalid site-publish route history in {path}") from exc
-    if (not isinstance(document, dict) or set(document) != {"schemaVersion", "buckets"}
-            or document["schemaVersion"] != 1 or not isinstance(document["buckets"], dict)):
+    """Read bucket-keyed route history from generated Ingresses."""
+    contracts = {}
+    manifests = app_dir / "manifests"
+    if not manifests.exists():
+        return contracts
+    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")
+        access = annotations.get("site-publish.fritzlab.net/access")
+        bucket = annotations.get("site-publish.fritzlab.net/bucket")
+        route_path = annotations.get("site-publish.fritzlab.net/route-path")
+        values = (artifact, access, bucket, route_path)
+        if all(value is None for value in values):
+            continue
+        if (not all(isinstance(value, str) for value in values)
+                or access not in {"public", "protected"} or not route_path.startswith("/")):
             raise RuntimeError(f"invalid site-publish route history in {path}")
-    for bucket, contract in document["buckets"].items():
-        if (not isinstance(bucket, str) or not isinstance(contract, dict)
-                or set(contract) != {"access", "artifact", "immutablePrefixes", "routePath"}
-                or contract["access"] not in {"legacy", "protected", "public"}
-                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"],
-        }
+        if bucket in contracts:
+            raise RuntimeError(f"duplicate site-publish route history for bucket {bucket}")
+        contracts[bucket] = {"path": route_path, "access": access, "artifact": artifact}
     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):
@@ -416,8 +355,7 @@ def validate_route_migrations(cfg, previous_contracts):
     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"}
+        if (previous and previous["access"] == "protected" and route["access"] == "public"
         ):
             raise RuntimeError(
                 f"artifact {route['artifact']} cannot become public while reusing protected "
@@ -448,12 +386,11 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
     for route in cfg["routes"]:
         s3_sync(
             artifact_by_name[route["artifact"]], route, site_dir, credential_env_names,
-            previous_contracts.get(artifact_by_name[route["artifact"]]["bucket"]),
+            (previous_contracts.get(artifact_by_name[route["artifact"]]["bucket"]) or {}).get("path"),
         )
     if cfg["compatibility"]:
         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)
 
     commit_and_push(apps_dir, f"Deploy {site_name}", token)
@@ -466,12 +403,7 @@ def decommission(site_name, token, buckets=None):
     if not site_path.exists():
         print(f"No manifests for {site_name} — nothing to remove")
         return
-    history_path = site_path / HISTORY_FILE
-    history = history_path.read_bytes() if history_path.exists() else None
     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)
     for bucket in buckets or [site_name]:
         print(f"Bucket {bucket} and its objects are NOT purged automatically.")

templates/ingress.yaml.j2

@@ -4,6 +4,12 @@ metadata:
   name: {{ route.resource_name }}
   namespace: {{ namespace }}
   annotations:
+{%- if not compatibility %}
+    site-publish.fritzlab.net/artifact: {{ route.artifact | tojson }}
+    site-publish.fritzlab.net/access: {{ route.access | tojson }}
+    site-publish.fritzlab.net/bucket: {{ route.artifact_config.bucket | tojson }}
+    site-publish.fritzlab.net/route-path: {{ route.path | tojson }}
+{%- endif %}
     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:
   ingressClassName: traefik

tests/test_contract.py

@@ -273,17 +273,16 @@ class GenerationTests(unittest.TestCase):
         self.assertNotIn("passhostheader", files["manifests/ingress-portal.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):
+    def test_yaml_ambiguous_artifact_name_stays_a_string_annotation(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"])
+        tmp, _, files = self.render(raw)
+        self.addCleanup(tmp.cleanup)
+        ingress = yaml.safe_load(files["manifests/ingress-portal.yaml"])
+        self.assertEqual(
+            "yes", ingress["metadata"]["annotations"]["site-publish.fritzlab.net/artifact"],
+        )
 
     def test_generation_is_deterministic_when_input_lists_are_reversed(self):
         raw = fixture("split-site.yaml")
@@ -472,10 +471,7 @@ class PublishingTests(unittest.TestCase):
             }, clear=False), patch.object(
                 deploy, "run", side_effect=lambda command, **_: commands.append(command)
             ):
-                deploy.s3_sync(artifact, route, root, previous_contract={
-                    "path": "/foo", "access": "public", "artifact": "distributions",
-                    "immutable_prefixes": ["foo/releases"],
-                })
+                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))
@@ -485,7 +481,6 @@ class PublishingTests(unittest.TestCase):
         previous = {
             "baseline-dist": {
                 "path": "/dist", "access": "protected", "artifact": "old-name",
-                "immutable_prefixes": ["dist/releases"],
             }
         }
         with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
@@ -500,82 +495,6 @@ class PublishingTests(unittest.TestCase):
         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):
         cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
         with tempfile.TemporaryDirectory() as tmp: