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

feat(apply): reconcile app-owned vars in the declarative engine

Browse Source 
Extends the Phase-1 apply engine to manage app `vars` declaratively,
alongside scripts/routes/triggers/secrets. The `Bundle` gains a `vars`
map (key → JSON value; values are non-secret, so they live inline, unlike
secret names). `diff_vars` is value-sensitive: a changed value is an
Update, a live var the manifest dropped is a Delete (applied only under
`--prune` — vars are prunable config, unlike never-pruned secrets).

Writes go through `set_app_var_tx`/`delete_app_var_tx` inside the apply
transaction (mirroring `PostgresVarsRepo::set` for `VarOwner::App`, scope
`*`), so they commit atomically with the rest of the apply and roll back
together on failure. `CurrentState` loads the app's OWN scope-`*`,
non-tombstone vars (inherited group vars and tombstones are out of scope
for the manifest); `state_token` folds them in so the bound-plan check
catches an out-of-band `pic vars set` between plan and apply. Keys are
validated against the same kebab rule as the vars admin API, and the
apply handler now requires `AppVarsWrite` when vars are touched or
`--prune` is set.

Scope: app-owned vars at scope `*` (an app *is* an environment). Group
vars via manifest and tombstone-via-manifest wait for the nested-manifest
model. Unit tests cover the create/update/noop/delete diff and the
state-token value sensitivity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-25 07:35:12 +02:00
parent b2822c8435
commit e517f6dc8c
3 changed files with 259 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -81,6 +81,15 @@ async fn apply_handler(
.await
.map_err(map_authz)?;
}
if req.prune || !req.bundle.vars.is_empty() {
require(
svc.authz.as_ref(),
&principal,
Capability::AppVarsWrite(app_id),
)
.await
.map_err(map_authz)?;
}
// Email triggers resolve and decrypt a stored secret by name server-side,
// which the secrets API guards with `AppSecretsRead`. Require it here too
// so apply can't bind a secret a principal couldn't otherwise read — the

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

@@ -52,6 +52,7 @@ use crate::trigger_repo::{
delete_trigger_tx, insert_email_trigger_tx, insert_trigger_tx, Trigger, TriggerDetails,
TriggerDispatchMode, TriggerRepo, TriggerRepoError,
};
use crate::vars_repo::{VarOwner, VarsRepo};
/// Reserved path prefixes that user routes may not claim (mirrors the
/// route-create rejection — see CLAUDE.md "Reserved path prefixes").
@@ -74,6 +75,13 @@ pub struct Bundle {
/// Declared secret *names* only; values are pushed out-of-band.
#[serde(default)]
pub secrets: Vec<String>,
/// Declared app-owned config `vars` — key → JSON value. Unlike secrets,
/// var values are non-secret and live inline in the manifest. Written at
/// app scope `*` (an app *is* an environment, §2 — per-env scoping is a
/// group concern, deferred to the nested-manifest model). Tombstones are
/// not expressible here; use `pic vars set --tombstone`.
#[serde(default)]
pub vars: std::collections::BTreeMap<String, serde_json::Value>,
}
#[derive(Debug, Clone, Deserialize)]
@@ -304,6 +312,8 @@ pub struct Plan {
pub routes: Vec<ResourceChange>,
pub triggers: Vec<ResourceChange>,
pub secrets: Vec<ResourceChange>,
#[serde(default)]
pub vars: Vec<ResourceChange>,
}
impl Plan {
@@ -315,6 +325,7 @@ impl Plan {
.chain(&self.routes)
.chain(&self.triggers)
.chain(&self.secrets)
.chain(&self.vars)
.all(|c| c.op == Op::NoOp)
}
}
@@ -340,6 +351,10 @@ pub struct CurrentState {
pub routes: Vec<Route>,
pub triggers: Vec<Trigger>,
pub secret_names: Vec<String>,
/// App's OWN `vars` at scope `*` (key, value), non-tombstone — the set
/// the manifest reconciles. Inherited group vars and tombstones are
/// excluded (the manifest manages only the app's own real values).
pub vars: Vec<(String, serde_json::Value)>,
}
// ----------------------------------------------------------------------------
@@ -380,6 +395,9 @@ pub struct ApplyService {
pub routes: Arc<dyn RouteRepository>,
pub triggers: Arc<dyn TriggerRepo>,
pub secrets: Arc<dyn SecretsRepo>,
/// App-owned `vars` reconciliation (read/diff path; the transactional
/// write goes through `set_app_var_tx` / `delete_app_var_tx`).
pub vars: Arc<dyn VarsRepo>,
pub apps: Arc<dyn AppRepository>,
/// App domain claims — validates a route's host belongs to the app.
pub domains: Arc<dyn AppDomainRepository>,
@@ -650,6 +668,22 @@ impl ApplyService {
report.triggers_created += 1;
}
// 3b. Vars — upsert every Create/Update at app scope `*`, in-tx so a
// later failure rolls them back with everything else.
for ch in &plan.vars {
match ch.op {
Op::Create => {
set_app_var_tx(&mut tx, app_id, &ch.key, &bundle.vars[&ch.key]).await?;
report.vars_created += 1;
}
Op::Update => {
set_app_var_tx(&mut tx, app_id, &ch.key, &bundle.vars[&ch.key]).await?;
report.vars_updated += 1;
}
Op::NoOp | Op::Delete => {}
}
}
// 4. Prune (only with --prune): delete stale triggers, then scripts
// (route removals already happened in the route delete-pass above).
// Secret pruning is deliberately deferred (destructive + irreversible):
@@ -712,6 +746,15 @@ impl ApplyService {
}
}
}
// Vars are prunable config (unlike secrets, which are never
// pruned): a live app var the manifest dropped is deleted.
for ch in &plan.vars {
if ch.op == Op::Delete {
delete_app_var_tx(&mut tx, app_id, &ch.key).await?;
report.vars_deleted += 1;
}
}
}
tx.commit()
@@ -984,6 +1027,12 @@ impl ApplyService {
return Err(ApplyError::Invalid(format!("duplicate trigger `{id}`")));
}
}
// Vars: kebab keys (same rule the vars admin API enforces), so a
// manifest can't write a key the interactive surface would reject.
for key in bundle.vars.keys() {
validate_var_key(key)?;
}
Ok(())
}
@@ -1004,11 +1053,25 @@ impl ApplyService {
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
let secret_names = self.all_secret_names(app_id).await?;
// App's OWN vars, narrowed to scope `*` and real values: the manifest
// reconciles app-own env-agnostic config, not inherited group vars
// (those aren't in this owner's rows at all) nor tombstones (an
// imperative suppress the manifest can't express).
let vars = self
.vars
.list_for_owner(VarOwner::App(app_id))
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?
.into_iter()
.filter(|r| r.environment_scope == "*" && !r.is_tombstone)
.map(|r| (r.key, r.value))
.collect();
Ok(CurrentState {
scripts,
routes,
triggers,
secret_names,
vars,
})
}
@@ -1053,9 +1116,112 @@ pub fn compute_diff(current: &CurrentState, bundle: &Bundle) -> Plan {
routes: diff_routes(current, bundle, &script_name_by_id),
triggers: diff_triggers(current, bundle, &script_name_by_id),
secrets: diff_secrets(current, bundle),
vars: diff_vars(current, bundle),
}
}
/// Diff app-owned vars by key. Unlike secrets (whose values live out-of-band),
/// a var's value IS in the manifest, so equality is value-sensitive: a changed
/// value is an `Update`. Live vars absent from the manifest are `Delete`
/// (applied only under `--prune`).
fn diff_vars(current: &CurrentState, bundle: &Bundle) -> Vec<ResourceChange> {
let live: HashMap<&str, &serde_json::Value> =
current.vars.iter().map(|(k, v)| (k.as_str(), v)).collect();
let mut out = Vec::new();
for (key, value) in &bundle.vars {
match live.get(key.as_str()) {
Some(cur) if *cur == value => out.push(ResourceChange {
op: Op::NoOp,
key: key.clone(),
detail: None,
}),
Some(_) => out.push(ResourceChange {
op: Op::Update,
key: key.clone(),
detail: Some("value changed".into()),
}),
None => out.push(ResourceChange {
op: Op::Create,
key: key.clone(),
detail: None,
}),
}
}
for (key, _) in &current.vars {
if !bundle.vars.contains_key(key) {
out.push(ResourceChange {
op: Op::Delete,
key: key.clone(),
detail: Some("on server, not declared".into()),
});
}
}
out
}
/// Kebab var key (`^[a-z0-9][a-z0-9-]{0,127}$`) — mirrors `vars_api`'s
/// `validate_key` so the manifest and the interactive surface agree.
fn validate_var_key(key: &str) -> Result<(), ApplyError> {
let ok = !key.is_empty()
&& key.len() <= 128
&& key
.chars()
.next()
.is_some_and(|c| c.is_ascii_lowercase() || c.is_ascii_digit())
&& key
.chars()
.all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-');
if ok {
Ok(())
} else {
Err(ApplyError::Invalid(format!(
"var key `{key}` must be 1–128 chars of lowercase letters, digits, and hyphens, \
starting with a letter or digit"
)))
}
}
/// Upsert one app-owned var at scope `*`, in the apply transaction. Mirrors
/// `PostgresVarsRepo::set` for `VarOwner::App` (the partial-index predicate is
/// restated in the conflict target) but runs against `&mut tx` so it commits
/// atomically with the rest of the apply. Clears any tombstone.
async fn set_app_var_tx(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
app_id: AppId,
key: &str,
value: &serde_json::Value,
) -> Result<(), ApplyError> {
sqlx::query(
"INSERT INTO vars (app_id, environment_scope, key, value, is_tombstone) \
VALUES ($1, '*', $2, $3, FALSE) \
ON CONFLICT (app_id, environment_scope, key) WHERE app_id IS NOT NULL DO UPDATE \
SET value = EXCLUDED.value, is_tombstone = FALSE, updated_at = NOW()",
)
.bind(app_id.into_inner())
.bind(key)
.bind(value)
.execute(&mut **tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
Ok(())
}
/// Delete one app-owned var at scope `*`, in the apply transaction.
async fn delete_app_var_tx(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
app_id: AppId,
key: &str,
) -> Result<(), ApplyError> {
sqlx::query("DELETE FROM vars WHERE app_id = $1 AND environment_scope = '*' AND key = $2")
.bind(app_id.into_inner())
.bind(key)
.execute(&mut **tx)
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?;
Ok(())
}
fn diff_scripts(current: &CurrentState, bundle: &Bundle) -> Vec<ResourceChange> {
let by_name: HashMap<String, &Script> = current
.scripts
@@ -1608,6 +1774,12 @@ pub struct ApplyReport {
pub routes_deleted: u32,
pub triggers_created: u32,
pub triggers_deleted: u32,
#[serde(default)]
pub vars_created: u32,
#[serde(default)]
pub vars_updated: u32,
#[serde(default)]
pub vars_deleted: u32,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub warnings: Vec<String>,
}
@@ -1731,7 +1903,8 @@ pub fn state_token(current: &CurrentState) -> String {
current.scripts.len()
+ current.routes.len()
+ current.triggers.len()
+ current.secret_names.len(),
+ current.secret_names.len()
+ current.vars.len(),
);
for s in &current.scripts {
parts.push(format!(
@@ -1775,6 +1948,16 @@ pub fn state_token(current: &CurrentState) -> String {
for n in &current.secret_names {
parts.push(format!("k|{n}"));
}
// Vars are value-sensitive (the value lives in the manifest), so the token
// moves when a value changes — the bound-plan check then catches an edit
// between plan and apply. `to_string` on a serde_json::Value is stable
// (serde_json sorts object keys by default).
for (key, value) in &current.vars {
parts.push(format!(
"v|{key}|{}",
serde_json::to_string(value).unwrap_or_default()
));
}
// Order-independent: sort the per-resource tokens before hashing.
parts.sort_unstable();
let mut h: u64 = 0xcbf2_9ce4_8422_2325;
@@ -1843,6 +2026,7 @@ mod tests {
routes: vec![],
triggers: vec![],
secrets: vec!["S".into()],
vars: std::collections::BTreeMap::new(),
};
let plan = compute_diff(&current, &bundle);
assert_eq!(plan.scripts.len(), 1);
@@ -1864,6 +2048,7 @@ mod tests {
routes: vec![],
triggers: vec![],
secrets: vec!["S".into()],
vars: std::collections::BTreeMap::new(),
};
let plan = compute_diff(&current, &bundle);
assert!(plan.is_noop(), "expected all no-op, got {plan:?}");
@@ -1880,6 +2065,7 @@ mod tests {
routes: vec![],
triggers: vec![],
secrets: vec![],
vars: std::collections::BTreeMap::new(),
};
let plan = compute_diff(&current, &bundle);
assert_eq!(plan.scripts[0].op, Op::Update);
@@ -1897,6 +2083,7 @@ mod tests {
routes: vec![],
triggers: vec![],
secrets: vec![],
vars: std::collections::BTreeMap::new(),
};
let plan = compute_diff(&current, &bundle);
assert_eq!(plan.scripts[0].op, Op::Delete);
@@ -1942,6 +2129,7 @@ mod tests {
}],
triggers: vec![],
secrets: vec![],
vars: std::collections::BTreeMap::new(),
};
let plan = compute_diff(&current, &bundle);
assert_eq!(plan.routes[0].op, Op::Update);
@@ -1953,6 +2141,7 @@ mod tests {
routes: vec![],
triggers: vec![],
secrets: vec![],
vars: std::collections::BTreeMap::new(),
}
}
@@ -2140,6 +2329,65 @@ mod tests {
assert_ne!(state_token(&base), state_token(&with_secret));
}
#[test]
fn diff_vars_create_update_noop_delete() {
let current = CurrentState {
vars: vec![
("region".into(), serde_json::json!("eu")),
("keep".into(), serde_json::json!(1)),
("stale".into(), serde_json::json!(true)),
],
..CurrentState::default()
};
let mut vars = std::collections::BTreeMap::new();
vars.insert("region".to_string(), serde_json::json!("us")); // value changed
vars.insert("keep".to_string(), serde_json::json!(1)); // identical
vars.insert("new".to_string(), serde_json::json!("x")); // not live
// "stale" is live but undeclared → Delete.
let bundle = Bundle {
vars,
..empty_bundle()
};
let plan = compute_diff(&current, &bundle);
let op = |k: &str| plan.vars.iter().find(|c| c.key == k).unwrap().op;
assert_eq!(op("region"), Op::Update);
assert_eq!(op("keep"), Op::NoOp);
assert_eq!(op("new"), Op::Create);
assert_eq!(op("stale"), Op::Delete);
assert!(!plan.is_noop());
}
#[test]
fn state_token_is_sensitive_to_var_values_and_order_independent() {
let base = CurrentState {
vars: vec![("region".into(), serde_json::json!("eu"))],
..CurrentState::default()
};
// A value change must flip the token (covers the bound-plan check for
// an out-of-band `pic vars set` between plan and apply).
let changed = CurrentState {
vars: vec![("region".into(), serde_json::json!("us"))],
..CurrentState::default()
};
assert_ne!(state_token(&base), state_token(&changed));
// Order-independent across multiple vars.
let ab = CurrentState {
vars: vec![
("a".into(), serde_json::json!(1)),
("b".into(), serde_json::json!(2)),
],
..CurrentState::default()
};
let ba = CurrentState {
vars: vec![
("b".into(), serde_json::json!(2)),
("a".into(), serde_json::json!(1)),
],
..CurrentState::default()
};
assert_eq!(state_token(&ab), state_token(&ba));
}
#[test]
fn state_token_ignores_dead_letter_triggers() {
// The diff ignores dead-letter triggers (not manifest-representable),

1
crates/picloud/src/lib.rs
View File

@@ -471,6 +471,7 @@ pub async fn build_app(
routes: route_repo.clone(),
triggers: trigger_repo.clone(),
secrets: secrets_repo.clone(),
vars: Arc::new(PostgresVarsRepo::new(pool.clone())),
apps: apps_repo.clone(),
domains: domains_repo.clone(),
authz: authz.clone(),