fix(site-publish): refuse to record a contract a scoped run did not publish
Test / contract (pull_request) Successful in 7s
Test / contract (pull_request) Successful in 7s
Review found the hole in the first commit's claim. render_site_manifests advances the stored route contract for every route in site.yaml, and `access` there is a replacement, not a union the way immutable_paths is. So a catalogue-only publish could write `protected` for the distributions bucket that nothing published — and validate_route_migrations then refuses to put that bucket back public. Unpublished intent became an irreversible fact. Reproduced from the repo's own fixture: after a whole publish the record reads public; after a catalogue-only publish with the route flipped it reads protected, with nothing written to baseline-dist, and reverting fails with "artifact distributions cannot become public while reusing protected bucket baseline-dist". A scoped run now refuses before the first bucket is touched when an unselected artifact's path, access or artifact name differs from what is recorded, naming both contracts. It also refuses an unselected artifact with no published history, which is the same defect at time zero. Publish the artifact in the same run. Three tests: the reviewer's flip scenario (and the same change published in the same run, which proceeds), the no-history case, and one proving deploy_static reaches the guard before publish_route_immutables, reconcile_artifact_cors or s3_sync. Disabling the call site alone turns the last one red. The README sentence is narrowed to what the code actually guarantees, and gains the CORS consequence: a scoped run holds no credential for the other bucket, so a cors_origins change lands with that artifact's next publish rather than on the merge that edits site.yaml. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjQqc4qFmdpAWaYfy2Aypb
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
3dfee64335
commit
b14f6a856b
@@ -255,9 +255,24 @@ Selection scopes the build and the S3 publication only. Ingresses, Services,
|
|||||||
Certificates and the immutable-path history are always rendered from the whole
|
Certificates and the immutable-path history are always rendered from the whole
|
||||||
`site.yaml`, so a scoped run cannot retire another artifact's route or delete
|
`site.yaml`, so a scoped run cannot retire another artifact's route or delete
|
||||||
its bucket contents. Credentials are resolved for the selected artifacts alone,
|
its bucket contents. Credentials are resolved for the selected artifacts alone,
|
||||||
so a workflow need not carry secrets for artifacts it does not publish. An
|
so a workflow need not carry secrets for artifacts it does not publish.
|
||||||
undeclared name fails before the first bucket is touched, and `enabled: false`
|
|
||||||
refuses a selection because decommissioning is whole-site.
|
What a scoped run may **not** do is move an artifact it is not publishing. The
|
||||||
|
generated route contract in `site-publish-history.yaml` is written from the
|
||||||
|
whole `site.yaml`, and `access` there is a replacement rather than a union — so
|
||||||
|
recording a change nothing published would turn intent into a fact that
|
||||||
|
`validate_route_migrations` then refuses to undo. A run whose `site.yaml`
|
||||||
|
changes an unselected artifact's route path, access or artifact name is refused
|
||||||
|
before the first bucket is touched, naming both contracts: publish that artifact
|
||||||
|
in the same run. For the same reason a scoped run cannot introduce an artifact
|
||||||
|
that has no published history yet.
|
||||||
|
|
||||||
|
Two more refusals, both before any bucket changes: an undeclared name, and
|
||||||
|
`enabled: false` with a selection, because decommissioning is whole-site.
|
||||||
|
|
||||||
|
One consequence to know: CORS reconciliation is scoped too, since a scoped run
|
||||||
|
holds no credential for the other bucket. A `cors_origins:` change lands with
|
||||||
|
that artifact's next publish, not on the merge that edits `site.yaml`.
|
||||||
|
|
||||||
## Tools
|
## Tools
|
||||||
|
|
||||||
|
|||||||
@@ -535,6 +535,43 @@ def validate_route_migrations(cfg, previous_contracts):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def validate_scoped_history(cfg, previous_contracts):
|
||||||
|
"""A scoped run may not record a route contract it did not publish.
|
||||||
|
|
||||||
|
render_site_manifests advances the stored contract for every route in
|
||||||
|
site.yaml, and `access` is overwritten rather than unioned the way
|
||||||
|
immutable_paths is. Without this, a catalogue-only publish could write a
|
||||||
|
protected access for the distributions bucket that nothing published, and
|
||||||
|
validate_route_migrations would then refuse to put that bucket back —
|
||||||
|
unpublished intent turned into an irreversible fact.
|
||||||
|
"""
|
||||||
|
chosen = set(cfg["selected"])
|
||||||
|
if len(chosen) == len(cfg["artifacts"]):
|
||||||
|
return
|
||||||
|
artifact_by_name = {artifact["name"]: artifact for artifact in cfg["artifacts"]}
|
||||||
|
for route in cfg["routes"]:
|
||||||
|
if route["artifact"] in chosen:
|
||||||
|
continue
|
||||||
|
artifact = artifact_by_name[route["artifact"]]
|
||||||
|
previous = previous_contracts.get(artifact["bucket"])
|
||||||
|
if previous is None:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"a scoped run cannot introduce artifact {route['artifact']}; "
|
||||||
|
f"publish it in the same run"
|
||||||
|
)
|
||||||
|
current = {
|
||||||
|
"path": route["path"],
|
||||||
|
"access": "public" if route["access"] == "legacy" else route["access"],
|
||||||
|
"artifact": route["artifact"],
|
||||||
|
}
|
||||||
|
recorded = {key: previous[key] for key in current}
|
||||||
|
if recorded != current:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"a scoped run may not change unselected artifact {route['artifact']}'s route "
|
||||||
|
f"contract ({recorded} -> {current}); publish it in the same run"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
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 = {
|
||||||
@@ -552,6 +589,7 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
|
|||||||
manifests_dir = app_dir / "manifests"
|
manifests_dir = app_dir / "manifests"
|
||||||
previous_contracts = previous_route_contracts(app_dir)
|
previous_contracts = previous_route_contracts(app_dir)
|
||||||
validate_route_migrations(cfg, previous_contracts)
|
validate_route_migrations(cfg, previous_contracts)
|
||||||
|
validate_scoped_history(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.
|
||||||
|
|||||||
@@ -475,6 +475,22 @@ class ArtifactSelectionTests(unittest.TestCase):
|
|||||||
apps_dir = root / "apps"
|
apps_dir = root / "apps"
|
||||||
app_dir = apps_dir / "sjc001" / "websites" / "baseline.fritzlab.net"
|
app_dir = apps_dir / "sjc001" / "websites" / "baseline.fritzlab.net"
|
||||||
app_dir.mkdir(parents=True)
|
app_dir.mkdir(parents=True)
|
||||||
|
# A scoped run refuses an artifact it has no published history for, so
|
||||||
|
# the fixture starts from the whole publication this one narrows.
|
||||||
|
(app_dir / "site-publish-history.yaml").write_text(yaml.safe_dump({
|
||||||
|
"version": 1,
|
||||||
|
"buckets": {
|
||||||
|
artifact["bucket"]: {
|
||||||
|
"path": route["path"],
|
||||||
|
"access": "public" if route["access"] == "legacy" else route["access"],
|
||||||
|
"artifact": route["artifact"],
|
||||||
|
"immutable_paths": deploy.immutable_key_prefixes(artifact, route),
|
||||||
|
}
|
||||||
|
for route in cfg["routes"]
|
||||||
|
for artifact in [next(item for item in cfg["artifacts"]
|
||||||
|
if item["name"] == route["artifact"])]
|
||||||
|
},
|
||||||
|
}, sort_keys=True))
|
||||||
for artifact in cfg["artifacts"]:
|
for artifact in cfg["artifacts"]:
|
||||||
if artifact["name"] not in set(selection):
|
if artifact["name"] not in set(selection):
|
||||||
continue
|
continue
|
||||||
@@ -520,6 +536,72 @@ class ArtifactSelectionTests(unittest.TestCase):
|
|||||||
["dist/releases"], history["buckets"]["baseline-dist"]["immutable_paths"]
|
["dist/releases"], history["buckets"]["baseline-dist"]["immutable_paths"]
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_scoped_run_refuses_to_record_a_contract_it_did_not_publish(self):
|
||||||
|
"""The reviewer's scenario: distributions goes protected in site.yaml while
|
||||||
|
a catalogue-only run publishes. Recording that access would make the
|
||||||
|
bucket unreturnable through validate_route_migrations."""
|
||||||
|
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
|
||||||
|
published = {
|
||||||
|
"baseline-portal": {
|
||||||
|
"path": "/", "access": "protected", "artifact": "portal", "immutable_paths": [],
|
||||||
|
},
|
||||||
|
"baseline-dist": {
|
||||||
|
"path": "/dist", "access": "public", "artifact": "distributions",
|
||||||
|
"immutable_paths": ["dist/releases"],
|
||||||
|
},
|
||||||
|
}
|
||||||
|
# A whole publish agrees with what is recorded, scoped or not.
|
||||||
|
deploy.validate_scoped_history(cfg, published)
|
||||||
|
cfg["selected"] = ["portal"]
|
||||||
|
deploy.validate_scoped_history(cfg, published)
|
||||||
|
|
||||||
|
moved = copy.deepcopy(cfg)
|
||||||
|
route = next(item for item in moved["routes"] if item["artifact"] == "distributions")
|
||||||
|
route["access"] = "protected"
|
||||||
|
route["access_middleware"] = "authentik-forwardauth"
|
||||||
|
with self.assertRaises(RuntimeError) as refused:
|
||||||
|
deploy.validate_scoped_history(moved, published)
|
||||||
|
self.assertIn("may not change unselected artifact distributions", str(refused.exception))
|
||||||
|
# The same change published in the same run is allowed to proceed.
|
||||||
|
moved["selected"] = ["distributions", "portal"]
|
||||||
|
deploy.validate_scoped_history(moved, published)
|
||||||
|
|
||||||
|
def test_scoped_run_refuses_an_artifact_with_no_published_history(self):
|
||||||
|
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
|
||||||
|
cfg["selected"] = ["portal"]
|
||||||
|
with self.assertRaises(RuntimeError) as refused:
|
||||||
|
deploy.validate_scoped_history(cfg, {})
|
||||||
|
self.assertIn("cannot introduce artifact distributions", str(refused.exception))
|
||||||
|
|
||||||
|
def test_scoped_deploy_runs_the_history_guard_before_touching_a_bucket(self):
|
||||||
|
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
|
||||||
|
cfg["selected"] = ["portal"]
|
||||||
|
tmp = tempfile.TemporaryDirectory()
|
||||||
|
self.addCleanup(tmp.cleanup)
|
||||||
|
root = Path(tmp.name)
|
||||||
|
apps_dir = root / "apps"
|
||||||
|
app_dir = apps_dir / "sjc001" / "websites" / "baseline.fritzlab.net"
|
||||||
|
app_dir.mkdir(parents=True)
|
||||||
|
html = root / next(
|
||||||
|
item for item in cfg["artifacts"] if item["name"] == "portal"
|
||||||
|
)["build_dir"]
|
||||||
|
html.mkdir(parents=True)
|
||||||
|
(html / "index.html").write_text("portal")
|
||||||
|
with patch.dict(os.environ, {
|
||||||
|
"PORTAL_S3_ACCESS_KEY": "portal-key", "PORTAL_S3_SECRET_KEY": "portal-secret",
|
||||||
|
}, clear=False), \
|
||||||
|
patch.object(deploy, "clone_apps", return_value=apps_dir), \
|
||||||
|
patch.object(deploy, "commit_and_push"), \
|
||||||
|
patch.object(deploy, "publish_route_immutables") as immutables, \
|
||||||
|
patch.object(deploy, "reconcile_artifact_cors") as cors, \
|
||||||
|
patch.object(deploy, "s3_sync") as sync, \
|
||||||
|
redirect_stdout(io.StringIO()), \
|
||||||
|
self.assertRaises(RuntimeError):
|
||||||
|
deploy.deploy_static("baseline.fritzlab.net", root, ROOT, "token", cfg)
|
||||||
|
immutables.assert_not_called()
|
||||||
|
cors.assert_not_called()
|
||||||
|
sync.assert_not_called()
|
||||||
|
|
||||||
def test_disabled_site_refuses_a_partial_selection(self):
|
def test_disabled_site_refuses_a_partial_selection(self):
|
||||||
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
|
cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
|
||||||
cfg["enabled"] = False
|
cfg["enabled"] = False
|
||||||
|
|||||||
Reference in New Issue
Block a user