fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(hierarchies): cross-repo template expansion (M7)

Browse Source 
Route/trigger templates declared at a group now fan out to EVERY descendant
app in the DB subtree, not just the app nodes present in the current
`pic apply --dir`. A template change at group N reaches subtree(N) across
repos, and a removed/disabled template reaps its expansions on those
descendants too.

- group_repo: recursive `descendant_app_ids` (+ `_tx`) — inverse of the
  ancestor chain CTE, app_id-ordered, depth-bounded.
- apply_service: Phase B2 expands templates into descendants of every in-tree
  group not already handled in Phase B. All descendant locks are taken up
  front in the single sorted batch (their union is invariant under reparenting
  in-tree groups), so there is no out-of-order mid-tx locking / deadlock.
  Per-recipient write caps (AppWriteRoute / AppManageTriggers /
  AppSecretsRead-for-email) are required AUTHORITATIVELY IN-TX against the
  post-reparent, already-locked app — checked set == written set, no TOCTOU.
  expand_*_templates_tx are now self-contained (load hand-declared identities
  from the tx), so collisions are caught on descendants too. Blast radius now
  counts the true DB subtree. Descendant expansion errors are tagged with the
  app slug (e.g. an {env} template on a NULL-environment descendant names it).
- apply_api: the pre-tx descendant authz pass is removed in favour of the
  in-tx gate (sound via the hierarchy-aware effective_app_role).
- templates journey: out-of-tree descendant at depth 1 AND 2 receives the
  expansion and is reaped on template removal.
- doc §4.5: strike the in-apply-only scope limitation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-28 18:38:33 +02:00
parent 7c17d6e363
commit c04c684a2e
5 changed files with 392 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
crates/manager-core/src/apply_api.rs
View File

@@ -428,6 +428,13 @@ async fn authz_tree(
}
}
}
// NOTE (§4.5, M7): templates also fan out to descendant apps NOT declared in
// this bundle (the cross-repo subtree). Those per-recipient write caps are
// gated AUTHORITATIVELY IN-TRANSACTION in `apply_tree` Phase B2 — against the
// post-reparent app set, each app already locked — so the checked set is by
// construction the written set (no pre-tx TOCTOU, and a reparent that moves
// apps under a templated group in the same apply can't slip the gate).
Ok(())
}

254
crates/manager-core/src/apply_service.rs
View File

@@ -1329,6 +1329,22 @@ impl ApplyService {
// Lock every node's apply key, in sorted order, so concurrent applies
// touching any shared node serialize and the ordering can't deadlock.
let mut lock_keys: Vec<i64> = prepared.iter().map(|p| apply_lock_key(p.owner)).collect();
// Also lock every descendant app of an in-tree group up front (M7):
// Phase B2 expands templates into them. Their union is invariant under
// reparenting in-tree groups (a reparent changes a group's ancestors,
// not its descendants; created groups are empty), so this pool-computed
// set equals the post-Phase-0 set Phase B2 touches — one sorted batch,
// so there is no out-of-order mid-tx locking and no deadlock.
for p in &prepared {
if let ApplyOwner::Group(gid) = p.owner {
for aid in crate::group_repo::descendant_app_ids(&self.pool, gid)
.await
.map_err(map_group_err)?
{
lock_keys.push(apply_lock_key(ApplyOwner::App(aid)));
}
}
}
lock_keys.sort_unstable();
lock_keys.dedup();
for k in lock_keys {
@@ -1471,7 +1487,6 @@ impl ApplyService {
// routes are reconciled (so a template can bind the app's own
// script and collide-check against hand-declared routes).
if let ApplyOwner::App(app_id) = p.owner {
let hand_declared_keys = bundle_route_keys(&p.node.bundle);
self.expand_route_templates_tx(
&mut tx,
app_id,
@@ -1480,11 +1495,9 @@ impl ApplyService {
&p.app_chain,
&name_to_id,
&group_script_index,
&hand_declared_keys,
&mut report,
)
.await?;
let hand_trigger_ids = bundle_trigger_identities(&p.node.bundle);
self.expand_trigger_templates_tx(
&mut tx,
app_id,
@@ -1494,7 +1507,6 @@ impl ApplyService {
&p.app_chain,
&name_to_id,
&group_script_index,
&hand_trigger_ids,
&mut report,
)
.await?;
@@ -1502,6 +1514,127 @@ impl ApplyService {
}
}
// Phase B2 — cross-repo descendant expansion (§4.5, M7): a template at a
// group in this apply fans out to EVERY descendant app in the DB subtree,
// not only the app nodes present in the tree. Gather the descendants of
// every in-tree group, drop the ones already expanded in Phase B, and
// expand each remaining app once (app_id-ordered, deterministic locking).
// Each `expand_*` reads the app's own chain/vars/hand-declared rows from
// the tx, so a removed template reaps its expansions on these apps too.
let in_tree_apps: HashSet<AppId> = prepared
.iter()
.filter_map(|p| match p.owner {
ApplyOwner::App(a) => Some(a),
ApplyOwner::Group(_) => None,
})
.collect();
let mut descendant_apps: BTreeSet<AppId> = BTreeSet::new();
for p in &prepared {
if let ApplyOwner::Group(gid) = p.owner {
for aid in crate::group_repo::descendant_app_ids_tx(&mut tx, gid)
.await
.map_err(map_group_err)?
{
if !in_tree_apps.contains(&aid) {
descendant_apps.insert(aid);
}
}
}
}
for app_id in descendant_apps {
any_app = true;
// Already locked up front (in the initial sorted batch). Slug + own
// environment (for `{env}`) + parent group (for the chain).
let (slug, app_env, group_id): (String, Option<String>, Uuid) =
sqlx::query_as("SELECT slug, environment, group_id FROM apps WHERE id = $1")
.bind(app_id.into_inner())
.fetch_one(&mut *tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
let chain: Vec<GroupId> = self
.groups
.ancestors(group_id.into())
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?
.iter()
.map(|g| g.id)
.collect();
// Authoritative IN-TX per-recipient authz (§7.4, M7): the actor needs
// the matching write cap on THIS descendant before any expansion is
// written into it. Checked here (post-reparent, already locked) so the
// checked set is by construction the written set — no pre-tx TOCTOU
// and a reparent-into-a-templated-group can't slip the gate. Sound via
// the hierarchy-aware effective_app_role: a group admin/editor is
// implicitly authorized on its subtree; a narrow principal is refused.
let route_tmpls = crate::template_repo::list_for_groups_tx(&mut tx, &chain)
.await
.map_err(map_group_err)?;
let trig_tmpls = crate::template_repo::list_triggers_for_groups_tx(&mut tx, &chain)
.await
.map_err(map_group_err)?;
if !route_tmpls.is_empty() {
require(
self.authz.as_ref(),
principal,
Capability::AppWriteRoute(app_id),
)
.await
.map_err(map_authz_denied)?;
}
if !trig_tmpls.is_empty() {
require(
self.authz.as_ref(),
principal,
Capability::AppManageTriggers(app_id),
)
.await
.map_err(map_authz_denied)?;
// Email-trigger expansion resolves + seals the app's secret —
// same AppSecretsRead gate a hand-declared email trigger needs.
if trig_tmpls.iter().any(|t| {
t.spec.get("kind").and_then(serde_json::Value::as_str) == Some("email")
}) {
require(
self.authz.as_ref(),
principal,
Capability::AppSecretsRead(app_id),
)
.await
.map_err(map_authz_denied)?;
}
}
// No own/in-tx group scripts for a descendant not in this apply: its
// template scripts resolve via in-tx group scripts (group_script_index)
// then committed inherited scripts (resolve_template_script fallback).
// Wrap errors with the descendant slug so e.g. an `{env}` template on a
// NULL-environment descendant names the offending app, not just "{env}".
self.expand_route_templates_tx(
&mut tx,
app_id,
&slug,
app_env.as_deref(),
&chain,
&HashMap::new(),
&group_script_index,
&mut report,
)
.await
.map_err(|e| with_descendant_ctx(e, &slug))?;
self.expand_trigger_templates_tx(
&mut tx,
app_id,
&slug,
app_env.as_deref(),
actor,
&chain,
&HashMap::new(),
&group_script_index,
&mut report,
)
.await
.map_err(|e| with_descendant_ctx(e, &slug))?;
}
// Phase C — structural prune (§7, M3): with `--prune`, delete groups
// THIS project owns that the manifest no longer declares, leaf-first
// (delete_tx is RESTRICT, so a group still holding apps/child-groups
@@ -1519,18 +1652,6 @@ impl ApplyService {
}
}
// §4.5 M4a scope: expansion reaping only runs for app nodes IN this
// apply. A deleted template's expansions in descendant apps NOT in this
// tree survive until those apps are re-applied — warn so the operator
// isn't surprised that a pruned template left live routes elsewhere.
if report.route_templates_deleted > 0 || report.trigger_templates_deleted > 0 {
report.warnings.push(
"deleted template(s): expansions in descendant apps not included in this apply \
remain until those apps are re-applied (`pic apply --dir` over them)"
.into(),
);
}
tx.commit()
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
@@ -1966,10 +2087,12 @@ impl ApplyService {
!n.bundle.route_templates.is_empty() || !n.bundle.trigger_templates.is_empty();
if n.kind == NodeKind::Group && has_templates {
if let Some(gid) = group_id_by_slug.get(&n.slug.to_lowercase()).copied() {
let affected_apps = prepared
.iter()
.filter(|p| p.app_chain.contains(&gid))
.count();
// True blast radius (§4.2, M7): every descendant app in the DB
// subtree, not just the app nodes present in this apply.
let affected_apps = crate::group_repo::descendant_app_ids(&self.pool, gid)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?
.len();
template_blast_radius.push(TemplateBlastRadius {
group: n.slug.clone(),
affected_apps,
@@ -2157,7 +2280,6 @@ impl ApplyService {
app_chain: &[GroupId],
own_name_to_id: &HashMap<String, ScriptId>,
group_script_index: &HashMap<GroupId, HashMap<String, ScriptId>>,
hand_declared_keys: &HashSet<String>,
report: &mut ApplyReport,
) -> Result<(), ApplyError> {
// Visible templates: nearest-wins by name across the ancestor chain.
@@ -2194,6 +2316,12 @@ impl ApplyService {
crate::config_resolver::resolve(cands).0
};
// Hand-declared routes (`from_template IS NULL`) of THIS app, read in-tx
// so an in-tree app's just-reconciled routes are included. A template
// expanding onto one of these is a hard collision — and for a descendant
// app not in this apply, these are its committed routes.
let hand_declared_keys = self.load_hand_declared_route_keys_tx(tx, app_id).await?;
// Build the desired expansion set (identity → resolved route + source).
let mut desired: HashMap<String, (Uuid, ScriptId, BundleRoute)> = HashMap::new();
for t in by_name.values() {
@@ -2347,6 +2475,27 @@ impl ApplyService {
Ok(rows.into_iter().map(|r| (r.identity(), r)).collect())
}
/// Route identities the app declares BY HAND (`from_template IS NULL`), read
/// in-tx. A template expansion colliding with one of these is rejected. For
/// an in-tree app this reflects its just-reconciled routes; for a descendant
/// not in the apply, its committed routes.
async fn load_hand_declared_route_keys_tx(
&self,
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
app_id: AppId,
) -> Result<HashSet<String>, ApplyError> {
let rows: Vec<ExpansionRow> = sqlx::query_as(
"SELECT id, script_id, host_kind, host, host_param_name, path_kind, path, \
method, dispatch_mode, enabled \
FROM routes WHERE app_id = $1 AND from_template IS NULL",
)
.bind(app_id.into_inner())
.fetch_all(&mut **tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
Ok(rows.into_iter().map(|r| r.identity()).collect())
}
/// Expand the TRIGGER templates visible to one descendant app (§4.5, M4b),
/// the trigger analogue of `expand_route_templates_tx`. Each chain template
/// fans into one concrete trigger, keyed by `from_template` (one trigger per
@@ -2365,7 +2514,6 @@ impl ApplyService {
app_chain: &[GroupId],
own_name_to_id: &HashMap<String, ScriptId>,
group_script_index: &HashMap<GroupId, HashMap<String, ScriptId>>,
hand_declared_trigger_ids: &HashSet<String>,
report: &mut ApplyReport,
) -> Result<(), ApplyError> {
let templates = crate::template_repo::list_triggers_for_groups_tx(tx, app_chain)
@@ -2392,6 +2540,17 @@ impl ApplyService {
.iter()
.map(|(n, id)| (*id, n.clone()))
.collect();
// App-own scripts too, so a hand-declared trigger bound to the app's own
// script (a descendant app's committed scripts aren't in own_name_to_id)
// still resolves an identity for the collision check below.
for s in self
.scripts
.list_for_app(app_id)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?
{
id_to_name.entry(s.id).or_insert_with(|| s.name.clone());
}
for gid in app_chain {
for s in self
.scripts
@@ -2421,11 +2580,18 @@ impl ApplyService {
.list_for_app(app_id)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
// template_id → (trigger_id, current semantic identity)
// template_id → (trigger_id, current semantic identity); and the
// identities the app declares BY HAND (every trigger NOT sourced from a
// template), so an expansion colliding with one is rejected. For an
// in-tree app these are its just-reconciled triggers; for a descendant
// not in this apply, its committed triggers.
let mut existing: HashMap<Uuid, (TriggerId, Option<String>)> = HashMap::new();
let mut hand_declared_trigger_ids: HashSet<String> = HashSet::new();
for trg in &all_triggers {
if let Some(tmpl) = by_trigger_id.get(&trg.id) {
existing.insert(*tmpl, (trg.id, current_trigger_identity(trg, &id_to_name)));
} else if let Some(id) = current_trigger_identity(trg, &id_to_name) {
hand_declared_trigger_ids.insert(id);
}
}
@@ -4203,34 +4369,6 @@ fn current_trigger_identity(t: &Trigger, name_by_id: &HashMap<ScriptId, String>)
}
}
/// The semantic identities of a bundle's HAND-declared triggers — the set a
/// trigger-template expansion is collision-checked against (§4.5, M4b).
fn bundle_trigger_identities(bundle: &Bundle) -> HashSet<String> {
bundle
.triggers
.iter()
.map(BundleTrigger::identity)
.collect()
}
/// The route-identity keys of a bundle's HAND-declared routes — the set a
/// template expansion is collision-checked against (§4.5, M4a).
fn bundle_route_keys(bundle: &Bundle) -> HashSet<String> {
bundle
.routes
.iter()
.map(|r| {
route_key(
r.method.as_deref(),
r.host_kind,
&r.host,
r.path_kind,
&r.path,
)
})
.collect()
}
fn route_key(
method: Option<&str>,
host_kind: HostKind,
@@ -4681,6 +4819,20 @@ fn map_authz_denied(e: AuthzDenied) -> ApplyError {
}
}
/// Tag a template-expansion error from a cross-repo DESCENDANT (an app not
/// declared in this apply) with the app slug, so e.g. an `{env}` template on a
/// NULL-environment descendant points at the offending app rather than reading
/// like a missing `--env` flag on the apply itself. Atomicity is preserved (the
/// error still aborts the whole apply); this only improves diagnosability.
fn with_descendant_ctx(e: ApplyError, slug: &str) -> ApplyError {
match e {
ApplyError::Invalid(m) => {
ApplyError::Invalid(format!("descendant app `{slug}` (not in this apply): {m}"))
}
other => other,
}
}
fn map_group_err(e: crate::group_repo::GroupRepositoryError) -> ApplyError {
use crate::group_repo::GroupRepositoryError as E;
// Conflict (cycle, slug clash, non-empty delete) and not-found are caller

48
crates/manager-core/src/group_repo.rs
View File

@@ -14,7 +14,7 @@
//! future CLI/orchestrator can detect structural drift (§6).
use async_trait::async_trait;
use picloud_shared::{Group, GroupId};
use picloud_shared::{AppId, Group, GroupId};
use sqlx::PgPool;
use uuid::Uuid;
@@ -523,6 +523,52 @@ pub(crate) async fn list_owned_groups_tx(
.collect())
}
/// Recursive descendant-app query, the inverse of the ancestor `CHAIN_LEVELS_CTE`
/// in `config_resolver`: walk `groups.parent_id` DOWN from `$1` to collect the
/// whole subtree of group ids, then every app whose `group_id` falls in it.
/// Ordered by `app_id` so callers lock/iterate deterministically. Depth-bounded
/// `< 64` as a runaway guard (the cycle guard already forbids real cycles).
const DESCENDANT_APPS_SQL: &str = "\
WITH RECURSIVE subtree AS ( \
SELECT id, 0 AS depth FROM groups WHERE id = $1 \
UNION ALL \
SELECT g.id, s.depth + 1 FROM groups g JOIN subtree s ON g.parent_id = s.id \
WHERE s.depth < 64 \
) \
SELECT a.id FROM apps a WHERE a.group_id IN (SELECT id FROM subtree) ORDER BY a.id";
/// Every app in the subtree rooted at `group_id` (the group itself and all
/// descendant groups), `app_id`-ordered. The read-only pool variant — used by
/// `plan` to size a template's true blast radius across the whole DB subtree,
/// not just the apps present in the current apply.
///
/// # Errors
/// Propagates sqlx errors.
pub async fn descendant_app_ids(
pool: &PgPool,
group_id: GroupId,
) -> Result<Vec<AppId>, GroupRepositoryError> {
let rows: Vec<(Uuid,)> = sqlx::query_as(DESCENDANT_APPS_SQL)
.bind(group_id.into_inner())
.fetch_all(pool)
.await?;
Ok(rows.into_iter().map(|(id,)| id.into()).collect())
}
/// The same subtree enumeration inside the apply tx, so groups/apps created
/// earlier in this transaction (M2 structural reconcile) are included when
/// fanning a template out to its descendants (§4.5, M7).
pub(crate) async fn descendant_app_ids_tx(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
group_id: GroupId,
) -> Result<Vec<AppId>, GroupRepositoryError> {
let rows: Vec<(Uuid,)> = sqlx::query_as(DESCENDANT_APPS_SQL)
.bind(group_id.into_inner())
.fetch_all(&mut **tx)
.await?;
Ok(rows.into_iter().map(|(id,)| id.into()).collect())
}
#[derive(sqlx::FromRow)]
struct GroupRow {
id: Uuid,