fix(config): scoped (PATCH) writes for deployment/program edits
Root-cause hardening for the dropped-globals bug: config edits went through the full-document save_config, which rewrote castle.yaml globals (+ every resource file) on every deployment/program edit — so an unmodeled global (role, secrets) was dropped. - add write_deployment_file / write_program_file: persist ONE resource file, never touching globals or other resources - config_editor deployment/program save/delete/enable now use those instead of save_config, so a deployment edit can't rewrite castle.yaml - save_config now preserves ANY unmanaged top-level global (not just secrets) - _aggregate_yaml surfaces role + secrets (raw-yaml round-trip completeness) - tests: scoped write leaves globals byte-identical; unmanaged global survives Verified live: editing lakehouse via the endpoint leaves castle.yaml unchanged.
This commit is contained in:
@@ -19,6 +19,8 @@ from castle_core.config import (
|
|||||||
load_config,
|
load_config,
|
||||||
parse_gateway,
|
parse_gateway,
|
||||||
save_config,
|
save_config,
|
||||||
|
write_deployment_file,
|
||||||
|
write_program_file,
|
||||||
)
|
)
|
||||||
from castle_core.manifest import ProgramSpec, kind_for
|
from castle_core.manifest import ProgramSpec, kind_for
|
||||||
|
|
||||||
@@ -78,6 +80,16 @@ def _aggregate_yaml(config: CastleConfig) -> str:
|
|||||||
data: dict = {"gateway": {"port": config.gateway.port}}
|
data: dict = {"gateway": {"port": config.gateway.port}}
|
||||||
if config.repo:
|
if config.repo:
|
||||||
data["repo"] = str(config.repo)
|
data["repo"] = str(config.repo)
|
||||||
|
if config.role and config.role != "follower":
|
||||||
|
data["role"] = config.role
|
||||||
|
# `secrets:` isn't modeled on CastleConfig — surface it from the raw file so the
|
||||||
|
# aggregate view/round-trip includes it.
|
||||||
|
try:
|
||||||
|
raw = yaml.safe_load((config.root / "castle.yaml").read_text()) or {}
|
||||||
|
if raw.get("secrets"):
|
||||||
|
data["secrets"] = raw["secrets"]
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
if config.programs:
|
if config.programs:
|
||||||
data["programs"] = {
|
data["programs"] = {
|
||||||
n: _program_to_yaml_dict(s, config) for n, s in config.programs.items()
|
n: _program_to_yaml_dict(s, config) for n, s in config.programs.items()
|
||||||
@@ -221,7 +233,7 @@ def save_program(name: str, request: ProgramConfigRequest) -> dict:
|
|||||||
)
|
)
|
||||||
|
|
||||||
config.programs[name] = spec
|
config.programs[name] = spec
|
||||||
save_config(config)
|
write_program_file(config, name) # PATCH: only this program file
|
||||||
return {"ok": True, "program": name}
|
return {"ok": True, "program": name}
|
||||||
|
|
||||||
|
|
||||||
@@ -264,10 +276,11 @@ async def delete_program(name: str, cascade: bool = False) -> dict:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
del config.store_for(kind)[ref]
|
del config.store_for(kind)[ref]
|
||||||
|
write_deployment_file(config, kind, ref) # unlinks the removed deployment
|
||||||
removed.append(ref)
|
removed.append(ref)
|
||||||
|
|
||||||
del config.programs[name]
|
del config.programs[name]
|
||||||
save_config(config)
|
write_program_file(config, name) # unlinks the program file only
|
||||||
|
|
||||||
if removed:
|
if removed:
|
||||||
# Converge the runtime: prune any orphan units and regenerate the Caddyfile
|
# Converge the runtime: prune any orphan units and regenerate the Caddyfile
|
||||||
@@ -350,8 +363,9 @@ def _save_deployment(name: str, config_dict: dict, kind: str | None = None) -> d
|
|||||||
# spec to the new store; drop the stale entry under the requested kind.
|
# spec to the new store; drop the stale entry under the requested kind.
|
||||||
if kind is not None and target_kind != kind:
|
if kind is not None and target_kind != kind:
|
||||||
config.store_for(kind).pop(name, None)
|
config.store_for(kind).pop(name, None)
|
||||||
|
write_deployment_file(config, kind, name) # spec now absent → unlinks old file
|
||||||
config.store_for(target_kind)[name] = dep
|
config.store_for(target_kind)[name] = dep
|
||||||
save_config(config)
|
write_deployment_file(config, target_kind, name) # PATCH: only this file
|
||||||
return {"ok": True, "deployment": name}
|
return {"ok": True, "deployment": name}
|
||||||
|
|
||||||
|
|
||||||
@@ -359,18 +373,19 @@ def _delete_deployment(name: str, kind: str | None = None) -> dict:
|
|||||||
"""Remove a deployment. A kind-scoped delete drops only that twin; the
|
"""Remove a deployment. A kind-scoped delete drops only that twin; the
|
||||||
kind-agnostic path removes every kind sharing the name."""
|
kind-agnostic path removes every kind sharing the name."""
|
||||||
config = get_config()
|
config = get_config()
|
||||||
removed = False
|
removed_kinds = []
|
||||||
kinds = (kind,) if kind is not None else KINDS
|
kinds = (kind,) if kind is not None else KINDS
|
||||||
for k in kinds:
|
for k in kinds:
|
||||||
if name in config.store_for(k):
|
if name in config.store_for(k):
|
||||||
del config.store_for(k)[name]
|
del config.store_for(k)[name]
|
||||||
removed = True
|
removed_kinds.append(k)
|
||||||
if not removed:
|
if not removed_kinds:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_404_NOT_FOUND,
|
status_code=status.HTTP_404_NOT_FOUND,
|
||||||
detail=f"Deployment '{name}' not found",
|
detail=f"Deployment '{name}' not found",
|
||||||
)
|
)
|
||||||
save_config(config)
|
for k in removed_kinds:
|
||||||
|
write_deployment_file(config, k, name) # spec absent → unlinks
|
||||||
return {"ok": True, "deployment": name, "action": "deleted"}
|
return {"ok": True, "deployment": name, "action": "deleted"}
|
||||||
|
|
||||||
|
|
||||||
@@ -406,9 +421,9 @@ def set_deployment_enabled(name: str, request: EnabledRequest) -> dict:
|
|||||||
detail=f"Deployment '{name}' not found",
|
detail=f"Deployment '{name}' not found",
|
||||||
)
|
)
|
||||||
# A name may span kinds — toggle all of them together.
|
# A name may span kinds — toggle all of them together.
|
||||||
for _kind, dep in deps:
|
for kind, dep in deps:
|
||||||
dep.enabled = request.enabled
|
dep.enabled = request.enabled
|
||||||
save_config(config)
|
write_deployment_file(config, kind, name)
|
||||||
return {"ok": True, "deployment": name, "enabled": request.enabled}
|
return {"ok": True, "deployment": name, "enabled": request.enabled}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -644,6 +644,44 @@ def _write_resource_dir(directory: Path, specs: dict[str, dict]) -> None:
|
|||||||
path.unlink()
|
path.unlink()
|
||||||
|
|
||||||
|
|
||||||
|
# Top-level castle.yaml keys that save_config owns and rewrites; anything else
|
||||||
|
# (e.g. `secrets:`) is preserved verbatim so a rewrite can't drop it.
|
||||||
|
_MANAGED_GLOBALS = {
|
||||||
|
"gateway", "repo", "data_dir", "repos_dir", "agents", "role",
|
||||||
|
"programs", "services", "jobs", "tools", "statics", "references", "deployments",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def write_deployment_file(config: CastleConfig, kind: str, name: str) -> None:
|
||||||
|
"""Write (or remove) a single deployment file — globals and other resources are
|
||||||
|
left untouched. This is the PATCH primitive: a deployment edit persists only
|
||||||
|
that deployment, never rewriting castle.yaml globals."""
|
||||||
|
directory = config.root / "deployments" / _KIND_STORE[kind]
|
||||||
|
path = directory / f"{name}.yaml"
|
||||||
|
spec = config.store_for(kind).get(name)
|
||||||
|
if spec is None:
|
||||||
|
path.unlink(missing_ok=True)
|
||||||
|
return
|
||||||
|
directory.mkdir(parents=True, exist_ok=True)
|
||||||
|
with open(path, "w") as f:
|
||||||
|
yaml.dump(_spec_to_yaml_dict(spec), f, default_flow_style=False, sort_keys=False)
|
||||||
|
|
||||||
|
|
||||||
|
def write_program_file(config: CastleConfig, name: str) -> None:
|
||||||
|
"""Write (or remove) a single program file — nothing else is touched."""
|
||||||
|
directory = config.root / "programs"
|
||||||
|
path = directory / f"{name}.yaml"
|
||||||
|
spec = config.programs.get(name)
|
||||||
|
if spec is None:
|
||||||
|
path.unlink(missing_ok=True)
|
||||||
|
return
|
||||||
|
directory.mkdir(parents=True, exist_ok=True)
|
||||||
|
with open(path, "w") as f:
|
||||||
|
yaml.dump(
|
||||||
|
_program_to_yaml_dict(spec, config), f, default_flow_style=False, sort_keys=False
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def save_config(config: CastleConfig) -> None:
|
def save_config(config: CastleConfig) -> None:
|
||||||
"""Save castle config: global castle.yaml + programs/ and deployments/ dirs."""
|
"""Save castle config: global castle.yaml + programs/ and deployments/ dirs."""
|
||||||
gateway_data: dict = {"port": config.gateway.port}
|
gateway_data: dict = {"port": config.gateway.port}
|
||||||
@@ -687,10 +725,13 @@ def save_config(config: CastleConfig) -> None:
|
|||||||
# from the existing file.
|
# from the existing file.
|
||||||
if config.role and config.role != "follower":
|
if config.role and config.role != "follower":
|
||||||
data["role"] = config.role
|
data["role"] = config.role
|
||||||
|
# Preserve any top-level castle.yaml keys this writer doesn't model (e.g.
|
||||||
|
# `secrets:`) — a full rewrite must never silently drop an unmanaged global.
|
||||||
try:
|
try:
|
||||||
existing = yaml.safe_load((config.root / "castle.yaml").read_text()) or {}
|
existing = yaml.safe_load((config.root / "castle.yaml").read_text()) or {}
|
||||||
if existing.get("secrets"):
|
for k, v in existing.items():
|
||||||
data["secrets"] = existing["secrets"]
|
if k not in _MANAGED_GLOBALS and k not in data:
|
||||||
|
data[k] = v
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|||||||
@@ -52,6 +52,40 @@ def test_save_config_round_trips_role_and_secrets(tmp_path: Path) -> None:
|
|||||||
assert reloaded.get("secrets", {}).get("backend") == "openbao"
|
assert reloaded.get("secrets", {}).get("backend") == "openbao"
|
||||||
|
|
||||||
|
|
||||||
|
def test_save_config_preserves_arbitrary_unmanaged_global(tmp_path: Path) -> None:
|
||||||
|
"""Any top-level key save_config doesn't model must survive a rewrite."""
|
||||||
|
from castle_core.config import load_config, save_config
|
||||||
|
|
||||||
|
(tmp_path / "castle.yaml").write_text(
|
||||||
|
yaml.safe_dump(
|
||||||
|
{"gateway": {"port": 18000}, "role": "authority", "future_thing": {"x": 1}}
|
||||||
|
)
|
||||||
|
)
|
||||||
|
save_config(load_config(tmp_path))
|
||||||
|
reloaded = yaml.safe_load((tmp_path / "castle.yaml").read_text())
|
||||||
|
assert reloaded.get("future_thing") == {"x": 1}
|
||||||
|
assert reloaded.get("role") == "authority"
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_deployment_file_leaves_globals_untouched(castle_root: Path) -> None:
|
||||||
|
"""A scoped deployment write must not rewrite castle.yaml globals (the PATCH
|
||||||
|
guarantee that stops a deployment edit from dropping role/secrets)."""
|
||||||
|
from castle_core.config import load_config, write_deployment_file
|
||||||
|
|
||||||
|
cy = castle_root / "castle.yaml"
|
||||||
|
data = yaml.safe_load(cy.read_text())
|
||||||
|
data["role"] = "authority"
|
||||||
|
data["secrets"] = {"backend": "openbao"}
|
||||||
|
cy.write_text(yaml.safe_dump(data))
|
||||||
|
before = cy.read_text()
|
||||||
|
|
||||||
|
config = load_config(castle_root)
|
||||||
|
kind, name, _dep = next(iter(config.all_deployments()))
|
||||||
|
write_deployment_file(config, kind, name)
|
||||||
|
|
||||||
|
assert cy.read_text() == before # globals byte-identical — nothing touched them
|
||||||
|
|
||||||
|
|
||||||
def test_registry_role_round_trip(tmp_path: Path) -> None:
|
def test_registry_role_round_trip(tmp_path: Path) -> None:
|
||||||
reg = NodeRegistry(
|
reg = NodeRegistry(
|
||||||
node=NodeConfig(hostname="civil", role="authority"), deployed={}
|
node=NodeConfig(hostname="civil", role="authority"), deployed={}
|
||||||
|
|||||||
Reference in New Issue
Block a user