fix(#3044): stop mounting a child's harness dir into its parent
bind_child_agent_dirs looped over state, harness and config alike and mounted all three read-write, while the doc comment above it defended only state. The rationale covered one dir, the loop covered three — the uniformity is what erased the fact that the three have three different answers. harness holds the child's own runtime material (bash-tasks, the turn-stats and event sqlite dbs) and nothing argues for a parent touching it. The only other reader is stats::hive_stats, which reads the host path directly and needs no mount into anyone. config stays read-write here on purpose. The ruling is that it becomes read-only, but request_init_config still has the manager seed a new child's config in place, so flipping the mount before relocating that step breaks agent creation hive-wide. That ordering now lives in the doc comment, where someone about to finish the job in one line will see it. docs/persistence.md justified all three dirs as RW; it now states the boundary as three answers and names the right source file.
This commit is contained in:
parent
66a3c2409c
commit
609035961f
2 changed files with 94 additions and 19 deletions
|
|
@ -314,19 +314,29 @@ Under `/var/lib/hyperhive/agents/<name>/`:
|
||||||
|
|
||||||
### Parent access to child state
|
### Parent access to child state
|
||||||
|
|
||||||
A parent agent gets each direct child's `state`, `harness`, and
|
A parent agent gets each direct child's `state` and `config` dirs
|
||||||
`config` dirs bind-mounted **read-write** (`bind_child_agent_dirs` in
|
bind-mounted **read-write** (`bind_child_agent_dirs` in
|
||||||
`lifecycle.rs`). The RW on `state` is deliberate, not an oversight: a
|
`lifecycle/host_config.rs`). The RW on `state` is deliberate, not an
|
||||||
parent manages its children, which includes writing into a child's
|
oversight: a parent manages its children, which includes writing into a
|
||||||
state for recovery (e.g. seeding notes, clearing a stuck sentinel) as
|
child's state for recovery (e.g. seeding notes, clearing a stuck
|
||||||
well as reading it. `config` and `harness` are RW too, but **nothing
|
sentinel) as well as reading it.
|
||||||
justifies that for `config`**: a config change is a PR on the child's
|
|
||||||
config repo, made from a clone, so the bind-mounted `config` dir is a
|
**`harness` is not mounted at all.** It holds the child's own runtime
|
||||||
read-only *copy* for reading a child's config — not a tree anyone edits
|
material — `bash-tasks/`, the turn-stats and event sqlite dbs — and
|
||||||
in place. Narrowing it is tracked separately, and depends on relocating
|
nothing argues for a parent reading it, let alone writing it. It used to
|
||||||
where `InitConfig` seeds. Per-child isolation still holds: a container only ever has
|
be mounted RW for "the same management reasons" as `state`, which was
|
||||||
its *own* dirs plus its direct children's bind-mounted, never a
|
never an argument so much as the side-effect of one loop treating all
|
||||||
sibling's.
|
three dirs alike. hive-c0re reads a child's harness dir **directly on the
|
||||||
|
host** when it wants those stats, which needs no mount into the parent.
|
||||||
|
|
||||||
|
`config` is still RW, and **nothing justifies that**: a config change is
|
||||||
|
a PR on the child's config repo, made from a clone, so the bind-mounted
|
||||||
|
`config` dir is a read-only *copy* for reading a child's config — not a
|
||||||
|
tree anyone edits in place. Narrowing it is tracked separately, and
|
||||||
|
depends on relocating where `InitConfig` seeds.
|
||||||
|
|
||||||
|
Per-child isolation still holds: a container only ever has its *own*
|
||||||
|
dirs plus its direct children's bind-mounted, never a sibling's.
|
||||||
|
|
||||||
Under `/var/lib/hyperhive/applied/<name>/` — the hive-c0re-only
|
Under `/var/lib/hyperhive/applied/<name>/` — the hive-c0re-only
|
||||||
applied repo. Tracks `flake.nix` (module-only boilerplate; never
|
applied repo. Tracks `flake.nix` (module-only boilerplate; never
|
||||||
|
|
|
||||||
|
|
@ -71,18 +71,31 @@ async fn systemd_daemon_reload() -> Result<()> {
|
||||||
/// inside the container.
|
/// inside the container.
|
||||||
pub const CONTAINER_MANAGER_APPLIED_MOUNT: &str = "/applied";
|
pub const CONTAINER_MANAGER_APPLIED_MOUNT: &str = "/applied";
|
||||||
|
|
||||||
/// Append bind flags for `child`'s state, harness, and config dirs into
|
/// Append bind flags for `child`'s state and config dirs into `binds`,
|
||||||
/// `binds`, all read-write. The RW on `state` is deliberate (recovery),
|
/// read-write. See docs/persistence.md ("Parent access to child state")
|
||||||
/// not an oversight; see docs/persistence.md ("Parent access to child
|
/// for what a parent may touch and why. Creates missing host-side
|
||||||
/// state") for the rationale. Creates missing host-side directories so
|
/// directories so nspawn doesn't refuse to start; missing dirs are
|
||||||
/// nspawn doesn't refuse to start; missing dirs are non-fatal.
|
/// non-fatal.
|
||||||
|
///
|
||||||
|
/// **`harness` is deliberately absent.** It holds the child's own runtime
|
||||||
|
/// material — `bash-tasks/`, turn-stats and event sqlite dbs — none of
|
||||||
|
/// which a parent has a stated reason to read, let alone write. It was
|
||||||
|
/// mounted only because the loop treated all three dirs alike; the three
|
||||||
|
/// have three different answers, and the uniformity is what hid that.
|
||||||
|
/// hive-c0re still reads a child's harness dir directly on the host
|
||||||
|
/// (`stats::hive_stats`), which needs no bind mount into the parent.
|
||||||
|
///
|
||||||
|
/// `config` is RW here **pending** the sequenced change: the ruling is
|
||||||
|
/// that it becomes read-only, but `request_init_config` still has the
|
||||||
|
/// manager seed a new child's config in place, so flipping the mount
|
||||||
|
/// before relocating that step breaks agent creation hive-wide.
|
||||||
fn bind_child_agent_dirs(child: &str, binds: &mut Vec<BindMount>) {
|
fn bind_child_agent_dirs(child: &str, binds: &mut Vec<BindMount>) {
|
||||||
let Ok(child) = hive_types::Ident::parse(child) else {
|
let Ok(child) = hive_types::Ident::parse(child) else {
|
||||||
tracing::warn!(%child, "skipping child bind: invalid agent name");
|
tracing::warn!(%child, "skipping child bind: invalid agent name");
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
let child_root = crate::paths::agent_state_dir(&child);
|
let child_root = crate::paths::agent_state_dir(&child);
|
||||||
for sub in ["state", "harness", "config"] {
|
for sub in ["state", "config"] {
|
||||||
let host = child_root.join(sub);
|
let host = child_root.join(sub);
|
||||||
let _ = std::fs::create_dir_all(&host);
|
let _ = std::fs::create_dir_all(&host);
|
||||||
binds.push(BindMount {
|
binds.push(BindMount {
|
||||||
|
|
@ -338,3 +351,55 @@ async fn set_nspawn_flags(
|
||||||
// Delegate the actual conf-file rewrite to hive-priv (runs as root).
|
// Delegate the actual conf-file rewrite to hive-priv (runs as root).
|
||||||
crate::priv_client::write_nspawn_flags(container, &binds, isolation, &load_creds).await
|
crate::priv_client::write_nspawn_flags(container, &binds, isolation, &load_creds).await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::{BindMount, bind_child_agent_dirs};
|
||||||
|
|
||||||
|
fn child_binds() -> Vec<BindMount> {
|
||||||
|
let mut binds = Vec::new();
|
||||||
|
bind_child_agent_dirs("kiddo", &mut binds);
|
||||||
|
binds
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The boundary, asserted as a whole rather than per-dir: a parent
|
||||||
|
/// sees a child's `state` and `config`, and nothing else.
|
||||||
|
#[test]
|
||||||
|
fn parent_sees_only_child_state_and_config() {
|
||||||
|
let paths: Vec<String> = child_binds()
|
||||||
|
.into_iter()
|
||||||
|
.map(|b| b.container_path)
|
||||||
|
.collect();
|
||||||
|
assert_eq!(paths, ["/agents/kiddo/state", "/agents/kiddo/config"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The regression this exists for. `harness` holds the child's own
|
||||||
|
/// runtime material and was only ever mounted because one loop
|
||||||
|
/// treated all three dirs alike — re-adding it to that loop is a
|
||||||
|
/// one-word change that nothing else would catch.
|
||||||
|
#[test]
|
||||||
|
fn parent_never_sees_a_child_harness_dir() {
|
||||||
|
for bind in child_binds() {
|
||||||
|
assert!(
|
||||||
|
!bind.container_path.contains("harness"),
|
||||||
|
"child harness must not be bound into a parent: {}",
|
||||||
|
bind.container_path
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!bind.host_path.contains("harness"),
|
||||||
|
"child harness must not be bound into a parent: {}",
|
||||||
|
bind.host_path
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An unparseable name yields no mounts at all — the guard must fail
|
||||||
|
/// closed, since the alternative is a path built from unvalidated
|
||||||
|
/// input.
|
||||||
|
#[test]
|
||||||
|
fn an_invalid_child_name_binds_nothing() {
|
||||||
|
let mut binds = Vec::new();
|
||||||
|
bind_child_agent_dirs("../escape", &mut binds);
|
||||||
|
assert!(binds.is_empty());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue