feat(enabled): apply-time reachability warning + end-to-end journey
- §4.7 warning: `apply` now reports when an enabled route or trigger is
bound to a disabled script — deployed but unreachable (route 404s,
trigger won't fire). A warning, not an error: it's valid desired state,
the operator just shouldn't be surprised by the silent 404.
- E2E journey (`tests/enabled.rs`): apply an active script and confirm
`/api/v1/execute/{id}` 200s; flip `enabled = false` in the manifest and
re-apply → 404; re-enable → 200. Exercises the whole path: manifest →
diff → apply → runtime honoring.
Tested: manager-core lib 367 + cli bins 31 + 15 project-tool journeys
(incl. the new enabled e2e) green; clippy -D warnings clean; versioning
gate passes (migration 0045).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -469,6 +469,10 @@ impl ApplyService {
|
|||||||
validate_email_secrets_present(bundle, ¤t.secret_names)?;
|
validate_email_secrets_present(bundle, ¤t.secret_names)?;
|
||||||
let plan = compute_diff(¤t, bundle);
|
let plan = compute_diff(¤t, bundle);
|
||||||
let mut report = ApplyReport::default();
|
let mut report = ApplyReport::default();
|
||||||
|
// §4.7 warning: an enabled binding pointing at a disabled script is
|
||||||
|
// deployed-but-unreachable. Not an error (it's valid desired state),
|
||||||
|
// but surfaced so the operator isn't surprised by a silent 404.
|
||||||
|
report.warnings.extend(disabled_target_warnings(bundle));
|
||||||
|
|
||||||
let bundle_scripts: HashMap<String, &BundleScript> = bundle
|
let bundle_scripts: HashMap<String, &BundleScript> = bundle
|
||||||
.scripts
|
.scripts
|
||||||
@@ -1422,6 +1426,42 @@ fn reject_reserved_path(path: &str) -> Result<(), ApplyError> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// §4.7 reachability warnings: an *enabled* route or trigger bound to a
|
||||||
|
/// script the manifest marks *disabled* is deployed but unreachable (the
|
||||||
|
/// route 404s, the trigger won't fire). Valid desired state, so a warning —
|
||||||
|
/// not an error — keyed on the bundle alone.
|
||||||
|
fn disabled_target_warnings(bundle: &Bundle) -> Vec<String> {
|
||||||
|
let disabled: HashSet<&str> = bundle
|
||||||
|
.scripts
|
||||||
|
.iter()
|
||||||
|
.filter(|s| !s.enabled)
|
||||||
|
.map(|s| s.name.as_str())
|
||||||
|
.collect();
|
||||||
|
if disabled.is_empty() {
|
||||||
|
return Vec::new();
|
||||||
|
}
|
||||||
|
let mut out = Vec::new();
|
||||||
|
for r in &bundle.routes {
|
||||||
|
if r.enabled && disabled.contains(r.script.as_str()) {
|
||||||
|
out.push(format!(
|
||||||
|
"route `{}` is enabled but its script `{}` is disabled — it will 404",
|
||||||
|
route_key(r.method.as_deref(), r.host_kind, &r.host, r.path_kind, &r.path),
|
||||||
|
r.script
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for t in &bundle.triggers {
|
||||||
|
if disabled.contains(t.script()) {
|
||||||
|
out.push(format!(
|
||||||
|
"{} trigger targets disabled script `{}` — it will not fire",
|
||||||
|
t.kind_str(),
|
||||||
|
t.script()
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
/// Per-kind structural validation for one desired trigger — kept at parity
|
/// Per-kind structural validation for one desired trigger — kept at parity
|
||||||
/// with the interactive trigger API so `apply` can't write a trigger the
|
/// with the interactive trigger API so `apply` can't write a trigger the
|
||||||
/// dashboard would have rejected. Pure (no live state), so it's unit-tested
|
/// dashboard would have rejected. Pure (no live state), so it's unit-tested
|
||||||
@@ -2105,6 +2145,33 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn warns_on_enabled_binding_to_disabled_script() {
|
||||||
|
let mut b = empty_bundle();
|
||||||
|
let mut s = bundle_script("h", "x");
|
||||||
|
s.enabled = false;
|
||||||
|
b.scripts = vec![s];
|
||||||
|
b.routes = vec![BundleRoute {
|
||||||
|
script: "h".into(),
|
||||||
|
method: None,
|
||||||
|
host_kind: HostKind::Any,
|
||||||
|
host: String::new(),
|
||||||
|
host_param_name: None,
|
||||||
|
path_kind: PathKind::Exact,
|
||||||
|
path: "/h".into(),
|
||||||
|
dispatch_mode: DispatchMode::Sync,
|
||||||
|
enabled: true,
|
||||||
|
}];
|
||||||
|
let w = disabled_target_warnings(&b);
|
||||||
|
assert!(
|
||||||
|
w.iter().any(|m| m.contains("will 404")),
|
||||||
|
"expected a 404 reachability warning: {w:?}"
|
||||||
|
);
|
||||||
|
// No warning when the script is active.
|
||||||
|
b.scripts[0].enabled = true;
|
||||||
|
assert!(disabled_target_warnings(&b).is_empty());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn trigger_diff_create_noop_delete() {
|
fn trigger_diff_create_noop_delete() {
|
||||||
let s = script("h", "x");
|
let s = script("h", "x");
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ mod apps;
|
|||||||
mod auth;
|
mod auth;
|
||||||
mod dead_letters;
|
mod dead_letters;
|
||||||
mod email_queue;
|
mod email_queue;
|
||||||
|
mod enabled;
|
||||||
mod init;
|
mod init;
|
||||||
mod invoke;
|
mod invoke;
|
||||||
mod logs;
|
mod logs;
|
||||||
|
|||||||
100
crates/picloud-cli/tests/enabled.rs
Normal file
100
crates/picloud-cli/tests/enabled.rs
Normal file
@@ -0,0 +1,100 @@
|
|||||||
|
//! `enabled` three-state lifecycle, end to end: disabling a script via the
|
||||||
|
//! manifest makes it non-invocable (404 on the execute-by-id bypass), and
|
||||||
|
//! re-enabling restores it — proving the data path + runtime honoring.
|
||||||
|
|
||||||
|
use std::fs;
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
|
use serde_json::Value;
|
||||||
|
use tempfile::TempDir;
|
||||||
|
|
||||||
|
use crate::common;
|
||||||
|
use crate::common::cleanup::AppGuard;
|
||||||
|
|
||||||
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
||||||
|
#[test]
|
||||||
|
fn disabling_a_script_makes_it_uninvocable_then_reenable() {
|
||||||
|
let Some(fx) = common::fixture_or_skip() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let env = common::admin_env(fx);
|
||||||
|
let slug = common::unique_slug("enabled");
|
||||||
|
common::pic_as(&env)
|
||||||
|
.args(["apps", "create", &slug])
|
||||||
|
.assert()
|
||||||
|
.success();
|
||||||
|
let _guard = AppGuard::new(&env.url, &env.token, &slug);
|
||||||
|
|
||||||
|
let dir = TempDir::new().unwrap();
|
||||||
|
fs::create_dir_all(dir.path().join("scripts")).unwrap();
|
||||||
|
fs::write(dir.path().join("scripts/hello.rhai"), "\"hi\"").unwrap();
|
||||||
|
let manifest_path = dir.path().join("picloud.toml");
|
||||||
|
let manifest = |enabled_line: &str| {
|
||||||
|
format!(
|
||||||
|
"[app]\nslug = \"{slug}\"\nname = \"Enabled\"\n\n\
|
||||||
|
[[scripts]]\nname = \"hello\"\nfile = \"scripts/hello.rhai\"\n{enabled_line}"
|
||||||
|
)
|
||||||
|
};
|
||||||
|
|
||||||
|
// Active → invocable.
|
||||||
|
fs::write(&manifest_path, manifest("")).unwrap();
|
||||||
|
apply(&env, &manifest_path);
|
||||||
|
let id = script_id(&env, &slug, "hello");
|
||||||
|
assert_eq!(invoke_status(&env, &id), 200, "active script must invoke");
|
||||||
|
|
||||||
|
// Disabled → 404 (not invocable), but still deployed (re-pull would show it).
|
||||||
|
fs::write(&manifest_path, manifest("enabled = false\n")).unwrap();
|
||||||
|
apply(&env, &manifest_path);
|
||||||
|
assert_eq!(
|
||||||
|
invoke_status(&env, &id),
|
||||||
|
404,
|
||||||
|
"disabled script must 404 on execute-by-id"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Re-enabled → invocable again.
|
||||||
|
fs::write(&manifest_path, manifest("enabled = true\n")).unwrap();
|
||||||
|
apply(&env, &manifest_path);
|
||||||
|
assert_eq!(
|
||||||
|
invoke_status(&env, &id),
|
||||||
|
200,
|
||||||
|
"re-enabled script must invoke"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn apply(env: &common::TestEnv, manifest_path: &Path) {
|
||||||
|
common::pic_as(env)
|
||||||
|
.args(["apply", "--file"])
|
||||||
|
.arg(manifest_path)
|
||||||
|
.assert()
|
||||||
|
.success();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Resolve a script's id via the admin API (the manifest carries no ids).
|
||||||
|
fn script_id(env: &common::TestEnv, slug: &str, name: &str) -> String {
|
||||||
|
let client = reqwest::blocking::Client::new();
|
||||||
|
let scripts: Vec<Value> = client
|
||||||
|
.get(format!("{}/api/v1/admin/scripts?app={slug}", env.url))
|
||||||
|
.bearer_auth(&env.token)
|
||||||
|
.send()
|
||||||
|
.unwrap()
|
||||||
|
.json()
|
||||||
|
.unwrap();
|
||||||
|
scripts
|
||||||
|
.into_iter()
|
||||||
|
.find(|s| s["name"] == name)
|
||||||
|
.and_then(|s| s["id"].as_str().map(String::from))
|
||||||
|
.expect("script id")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// POST the execute-by-id bypass and return the HTTP status code.
|
||||||
|
fn invoke_status(env: &common::TestEnv, id: &str) -> u16 {
|
||||||
|
let client = reqwest::blocking::Client::new();
|
||||||
|
client
|
||||||
|
.post(format!("{}/api/v1/execute/{id}", env.url))
|
||||||
|
.bearer_auth(&env.token)
|
||||||
|
.body("{}")
|
||||||
|
.send()
|
||||||
|
.unwrap()
|
||||||
|
.status()
|
||||||
|
.as_u16()
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user