diff --git a/crates/alien-core/src/lib.rs b/crates/alien-core/src/lib.rs index 020965810..76c91d130 100644 --- a/crates/alien-core/src/lib.rs +++ b/crates/alien-core/src/lib.rs @@ -12,6 +12,8 @@ pub use permissions::*; mod platform; pub use platform::*; +mod resource_links; +pub use resource_links::*; mod secret_delivery; pub use secret_delivery::*; diff --git a/crates/alien-core/src/resource_links.rs b/crates/alien-core/src/resource_links.rs new file mode 100644 index 000000000..d4bf441a0 --- /dev/null +++ b/crates/alien-core/src/resource_links.rs @@ -0,0 +1,305 @@ +//! Resources that own links to other resources. +//! +//! A link produces an `ALIEN__BINDING` and is the one reference kind a declined gate can +//! remove; structural edges cannot. Resolution is by concrete type, not by tag string. + +use crate::resource::{Resource, ResourceRef}; +use crate::resources::{Build, Container, Daemon, Worker}; + +/// A resource definition that owns resource links. +pub trait ResourceLinks { + /// The links this definition owns, excluding triggers and ordering edges. + fn links(&self) -> &[ResourceRef]; + + /// Mutable access, for dropping links to resources leaving the stack. + fn links_mut(&mut self) -> &mut Vec; +} + +macro_rules! impl_resource_links { + ($($ty:ty),+ $(,)?) => {$( + impl ResourceLinks for $ty { + fn links(&self) -> &[ResourceRef] { + &self.links + } + + fn links_mut(&mut self) -> &mut Vec { + &mut self.links + } + } + )+}; +} + +impl_resource_links!(Worker, Container, Daemon, Build); + + + +/// The link-owning view of a resource, or `None` when it owns no links. +/// +/// `None` is ordinary and callers walking every resource skip it; a caller that has already +/// established it holds a link owner should fail loudly instead. +pub fn resource_links(resource: &Resource) -> Option<&dyn ResourceLinks> { + if let Some(worker) = resource.downcast_ref::() { + return Some(worker); + } + if let Some(container) = resource.downcast_ref::() { + return Some(container); + } + if let Some(daemon) = resource.downcast_ref::() { + return Some(daemon); + } + // Build is not a compute kind, but it owns author-declared links producing the same + // bindings, so a declined gate must reach them too. Strip timing follows the target's + // lifecycle, not Build's, so a Build linking a live-gated target takes the late strip. + if let Some(build) = resource.downcast_ref::() { + return Some(build); + } + None +} + +/// Mutable counterpart of [`resource_links`]. +pub fn resource_links_mut(resource: &mut Resource) -> Option<&mut dyn ResourceLinks> { + // Probed immutably first: returning a `&mut` borrow out of a conditional keeps the + // borrow alive across the whole function, which rejects a downcast chain. + if resource.downcast_ref::().is_some() { + return resource + .downcast_mut::() + .map(|worker| worker as &mut dyn ResourceLinks); + } + if resource.downcast_ref::().is_some() { + return resource + .downcast_mut::() + .map(|container| container as &mut dyn ResourceLinks); + } + if resource.downcast_ref::().is_some() { + return resource + .downcast_mut::() + .map(|daemon| daemon as &mut dyn ResourceLinks); + } + if resource.downcast_ref::().is_some() { + return resource + .downcast_mut::() + .map(|build| build as &mut dyn ResourceLinks); + } + None +} + +/// The links a resource owns, empty when it owns none. +pub fn links_of(resource: &Resource) -> &[ResourceRef] { + resource_links(resource).map_or(&[], |owner| owner.links()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::resources::{ContainerCode, DaemonCode, Kv, ResourceSpec, WorkerCode}; + use serde::Deserialize; + + fn kv(id: &str) -> Kv { + Kv::new(id.to_string()).build() + } + + fn worker() -> Resource { + Resource::new( + Worker::new("api".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/api:latest".to_string(), + }) + .link(&kv("cache")) + .link(&kv("store")) + .build(), + ) + } + + fn container() -> Resource { + Resource::new( + Container::new("api".to_string()) + .code(ContainerCode::Image { + image: "example.com/api:latest".to_string(), + }) + .cpu(ResourceSpec { + min: "0.25".to_string(), + desired: "0.5".to_string(), + }) + .memory(ResourceSpec { + min: "256Mi".to_string(), + desired: "512Mi".to_string(), + }) + .replicas(1) + .permissions("execution".to_string()) + .port(8080) + .link(&kv("cache")) + .link(&kv("store")) + .build(), + ) + } + + fn daemon() -> Resource { + Resource::new( + Daemon::new("agent".to_string()) + .code(DaemonCode::Image { + image: "example.com/agent:latest".to_string(), + }) + .cluster("runtime".to_string()) + .permissions("execution".to_string()) + .link(&kv("cache")) + .link(&kv("store")) + .build(), + ) + } + + fn build() -> Resource { + Resource::new( + Build::new("builder".to_string()) + .permissions("build".to_string()) + .link(&kv("cache")) + .link(&kv("store")) + .build(), + ) + } + + /// One entry per wired link owner. The resolve, scrub and drift tests all read this, so + /// adding a type is a single edit rather than several independently-trusted ones. + const FIXTURES: &[(&str, fn() -> Resource)] = &[ + ("worker", worker), + ("container", container), + ("daemon", daemon), + ("build", build), + ]; + + /// Every link owner must resolve, expose its links, and drop exactly the named one. + /// Parametrised because a type that silently stops participating would otherwise only + /// surface as a dangling reference in a customer's account. + #[test] + fn every_link_owner_resolves_and_scrubs() { + for (name, make) in FIXTURES { + let mut resource = make(); + assert_eq!( + links_of(&resource).len(), + 2, + "{name} should start with both links" + ); + + let owner = resource_links_mut(&mut resource) + .unwrap_or_else(|| panic!("{name} must resolve as a link owner")); + owner.links_mut().retain(|l| l.id != "cache"); + + let remaining: Vec<&str> = links_of(&resource).iter().map(|l| l.id.as_str()).collect(); + assert_eq!(remaining, vec!["store"], "{name} kept the wrong link"); + } + } + + /// Accepting must leave every link in place, or the scrub would be removing links it + /// was never asked to remove. + #[test] + fn no_declines_leaves_every_link_owner_untouched() { + for (name, make) in FIXTURES { + let mut resource = make(); + let before: Vec = links_of(&resource).to_vec(); + assert_eq!(before.len(), 2, "{name} should start with both links"); + // Resolved strictly: behind an `if let` a broken resolver would skip the + // mutation entirely and the equality below would still hold. + let owner = resource_links_mut(&mut resource) + .unwrap_or_else(|| panic!("{name} must resolve as a link owner")); + let declined: Vec = Vec::new(); + owner.links_mut().retain(|l| !declined.contains(&l.id)); + assert_eq!(links_of(&resource), before.as_slice(), "{name} changed"); + } + } + + /// A resource that owns no links resolves to `None` rather than an empty owner, so a + /// caller that requires one can fail loudly instead of silently seeing zero links. + #[test] + fn a_non_link_owner_does_not_resolve() { + let mut store = Resource::new(Kv::new("store".to_string()).build()); + + assert!(resource_links(&store).is_none()); + assert!(resource_links_mut(&mut store).is_none()); + assert!(links_of(&store).is_empty()); + } + + /// Every resource type the deserializer accepts, and whether it owns links. + /// + /// The `Deserialize for Resource` match is the registry; this only records classification. + const LINK_OWNERSHIP: &[(&str, bool)] = &[ + ("worker", true), + ("container", true), + ("daemon", true), + ("build", true), + ("vault", false), + ("compute-cluster", false), + ("kubernetes-cluster", false), + ("storage", false), + ("queue", false), + ("email", false), + ("kv", false), + ("postgres", false), + ("network", false), + ("service-account", false), + ("artifact-registry", false), + ("service_activation", false), + ("remote-stack-management", false), + ("azure_resource_group", false), + ("azure_storage_account", false), + ("azure_container_apps_environment", false), + ("azure_service_bus_namespace", false), + ("experimental/aws-opensearch", false), + ]; + + /// Drift guard. The registered types are read out of the deserializer's own + /// `unknown_variant` refusal rather than restated, so a new type that never declares + /// whether it owns links fails here instead of silently opting out of scrubbing. + #[test] + fn every_registered_resource_type_declares_link_ownership() { + let refusal = Resource::deserialize(serde_json::json!({ "type": "not-a-resource" })) + .expect_err("an unregistered type must be refused"); + let message = refusal.to_string(); + + let registered: Vec<&str> = message + .split('`') + .skip(1) + .step_by(2) + .filter(|tag| *tag != "not-a-resource") + .collect(); + + assert!( + registered.len() > 10, + "could not read the registered types out of: {message}" + ); + + for tag in ®istered { + assert!( + LINK_OWNERSHIP.iter().any(|(known, _)| known == tag), + "resource type '{tag}' is registered but does not declare whether it owns \ + links. Add it to LINK_OWNERSHIP in resource_links.rs" + ); + } + + for (tag, _) in LINK_OWNERSHIP { + assert!( + registered.contains(tag), + "'{tag}' is classified but no longer registered; drop it from LINK_OWNERSHIP" + ); + } + + // Presence alone would pass a type declared `true` that nobody wired into + // `impl_resource_links!`, which then never resolves and is never scrubbed. + let declared: Vec<&str> = LINK_OWNERSHIP + .iter() + .filter(|(_, owns)| *owns) + .map(|(tag, _)| *tag) + .collect(); + for (tag, make) in FIXTURES { + assert!(declared.contains(tag), "fixture '{tag}' is not declared a link owner"); + assert!( + resource_links(&make()).is_some(), + "'{tag}' is declared a link owner but does not resolve as one" + ); + } + assert_eq!( + declared.len(), + FIXTURES.len(), + "declared link owners {declared:?} have no fixture proving they resolve" + ); + } +} diff --git a/crates/alien-deployment/src/pending.rs b/crates/alien-deployment/src/pending.rs index 6696f5540..a2fdf9883 100644 --- a/crates/alien-deployment/src/pending.rs +++ b/crates/alien-deployment/src/pending.rs @@ -69,10 +69,9 @@ pub async fn handle_pending( info!("Deployment-time preflight checks completed successfully"); - // Step 3.5: Drop gated live resources whose input says no. Frozen - // declines were stripped before the mutations above; live declines apply - // here, after them, so a declined workload's provisioning baseline stays - // derived and acceptance can return later. + // Step 3.5: Drop gated live resources whose input says no. Applied after the + // mutations so a declined resource still derives the baseline that a later + // acceptance returns to. let mutated_stack = strip_declined_live_resources( mutated_stack, &config.input_values, @@ -199,9 +198,9 @@ fn strip_frozen_dominated_live_resources( /// Remove gated live resources whose input resolves false. Runs AFTER the /// mutations on both deployment paths, at the boundary where the executor's /// desired set is built: the mutations must keep seeing a declined live -/// resource so its provisioning baseline — service account, profile grants, -/// capacity contribution — stays stable and acceptance can return without a -/// frozen-compatibility violation. +/// resource so its service account and capacity contribution stay stable. Its +/// grant does not survive — `permission_profiles_unchanged` exempts a scope +/// keyed by a gated resource present on only one side, which absorbs that. /// /// The answer is the provided value, else the recorded answer while the input /// still gates a frozen resource, else the input's declared boolean default; @@ -467,6 +466,10 @@ pub fn audit_live_gate_transitions( } fn remove_declined(stack: &mut Stack, declined: &[String]) { + if declined.is_empty() { + return; + } + for resource_id in declined { info!( resource_id = %resource_id, @@ -474,6 +477,72 @@ fn remove_declined(stack: &mut Stack, declined: &[String]) { ); stack.resources.shift_remove(resource_id); } + + // Removing the resource without its inbound links would leave a survivor pointing at + // something that was never created, which the executor and binding resolution both + // reject. Scrubbing here is what lets an ungated resource link a gated one. + for (resource_id, entry) in stack.resources.iter_mut() { + let dropped = match alien_core::resource_links_mut(&mut entry.config) { + Some(owner) => { + let before = owner.links().len(); + owner + .links_mut() + .retain(|link| !declined.contains(&link.id)); + before - owner.links().len() + } + None => 0, + }; + if dropped > 0 { + info!( + resource_id = %resource_id, + dropped, + declined = ?declined, + "Dropped links to declined resources; this resource keeps its own lifecycle" + ); + } + + let ordering_before = entry.dependencies.len(); + entry + .dependencies + .retain(|dependency| !declined.contains(&dependency.id)); + // The release-time preflight refuses authored ordering edges onto gated resources, + // so one reaching here predates the rule; dropping it keeps the stack coherent. + if ordering_before > entry.dependencies.len() { + info!( + resource_id = %resource_id, + dropped = ordering_before - entry.dependencies.len(), + "Dropped ordering edges to declined resources" + ); + } + } + + scrub_declined_grants(stack, declined); +} + +/// Drop grants naming a declined resource from every permission profile. +/// +/// Not inert: GCP applies every non-`"*"` entry without consulting the desired resources. +/// Nothing is lost, because the mutations re-derive them whenever the gate is accepted. +fn scrub_declined_grants(stack: &mut Stack, declined: &[String]) { + let scrub = |profile: &mut alien_core::permissions::PermissionProfile| { + for resource_id in declined { + if profile.0.shift_remove(resource_id).is_some() { + info!( + resource_id = %resource_id, + "Dropped the grant for a declined resource" + ); + } + } + }; + + for profile in stack.permissions.profiles.values_mut() { + scrub(profile); + } + match &mut stack.permissions.management { + alien_core::permissions::ManagementPermissions::Extend(profile) + | alien_core::permissions::ManagementPermissions::Override(profile) => scrub(profile), + alien_core::permissions::ManagementPermissions::Auto => {} + } } /// A gate value from the wire: JSON booleans stay booleans, and the @@ -639,6 +708,244 @@ mod tests { .build() } + /// An ungated worker linking a gated live store: the shape the link scrub exists for. + fn live_gated_stack_with_linking_worker(default: Option) -> Stack { + let input = StackInputDefinition::deployer_boolean( + "cacheEnabled", + "Enable the cache", + "Whether to run the cache store.", + default, + ); + let cache = Kv::new("cache".to_string()).build(); + let worker = alien_core::Worker::new("api".to_string()) + .permissions("execution".to_string()) + .code(alien_core::WorkerCode::Image { + image: "example.com/api:latest".to_string(), + }) + .link(&cache) + .build(); + + Stack::new("gated-stack".to_string()) + .inputs(vec![input]) + .add_enabled_when(cache, ResourceLifecycle::Live, "cacheEnabled") + .add(worker, ResourceLifecycle::Live) + .build() + } + + fn strip_live(stack: Stack, accepted: bool) -> Stack { + let mut input_values = std::collections::HashMap::new(); + input_values.insert( + "cacheEnabled".to_string(), + serde_json::Value::Bool(accepted), + ); + strip_declined_live_resources( + stack, + &input_values, + &alien_core::GateAnswers::default(), + &std::collections::HashSet::new(), + ) + .expect("strip should resolve the gate") + } + + fn link_ids(stack: &Stack, resource_id: &str) -> Vec { + let config = &stack + .resources + .get(resource_id) + .expect("resource should be present") + .config; + alien_core::links_of(config) + .iter() + .map(|link| link.id.clone()) + .collect() + } + + /// A declined store leaves the stack with the linking worker's reference to it, while the + /// worker itself keeps its own lifecycle. + #[test] + fn a_declined_store_takes_the_linking_workers_link_with_it() { + let stack = strip_live(live_gated_stack_with_linking_worker(Some(true)), false); + + assert!( + !stack.resources.contains_key("cache"), + "the declined store must leave the desired stack" + ); + assert!( + stack.resources.contains_key("api"), + "the ungated worker must survive its declined link target" + ); + assert!( + link_ids(&stack, "api").is_empty(), + "the link must not outlive the resource: {:?}", + link_ids(&stack, "api") + ); + assert!( + stack + .resources + .get("api") + .expect("worker") + .config + .get_dependencies() + .is_empty(), + "no dependency may still name the declined store" + ); + } + + /// Accepting must leave the graph untouched, or the scrub would be dropping links it + /// was never asked to drop. + #[test] + fn an_accepted_store_keeps_the_link() { + let stack = strip_live(live_gated_stack_with_linking_worker(Some(false)), true); + + assert!(stack.resources.contains_key("cache")); + assert_eq!(link_ids(&stack, "api"), vec!["cache".to_string()]); + } + + /// Setup authorization and the frozen-unchanged gate both hash the frozen entries, so a + /// live decline that rewrote one would make those baselines disagree mid-update. + #[test] + fn a_live_decline_leaves_the_frozen_digest_untouched() { + let with_frozen = |default| { + let mut stack = live_gated_stack_with_linking_worker(default); + let store = Kv::new("ledger".to_string()).build(); + stack.resources.insert( + "ledger".to_string(), + alien_core::ResourceEntry { + config: alien_core::Resource::new(store), + lifecycle: ResourceLifecycle::Frozen, + dependencies: Vec::new(), + remote_access: false, + enabled_when: None, + }, + ); + stack + }; + + let digest_before = with_frozen(Some(true)).frozen_resources_digest(); + let stripped = strip_live(with_frozen(Some(true)), false); + + assert!(!stripped.resources.contains_key("cache")); + assert_eq!( + digest_before, + stripped.frozen_resources_digest(), + "a live decline must not rewrite any frozen entry" + ); + } + + /// The executor plans an update by diffing resource configs, and links live inside that + /// config. Without a config change the worker keeps the stale binding indefinitely. + #[test] + fn the_scrub_changes_the_config_the_executor_diffs() { + let before = live_gated_stack_with_linking_worker(Some(true)); + let before_config = before + .resources + .get("api") + .expect("worker") + .config + .clone(); + + let after = strip_live(live_gated_stack_with_linking_worker(Some(true)), false); + let after_config = &after.resources.get("api").expect("worker").config; + + assert_ne!( + &before_config, after_config, + "a scrubbed worker must not compare equal to the unscrubbed one" + ); + } + + /// A grant left in the desired stack is not inert: on GCP the walker applies every + /// named entry without consulting desired resources. Revoking an already-applied + /// binding is the reconciler's separate concern; this pins only the desired side. + #[test] + fn a_declined_resource_takes_its_grant_with_it() { + let input = StackInputDefinition::deployer_boolean( + "cacheEnabled", + "Enable the cache", + "Whether to run the cache store.", + Some(true), + ); + let cache = Kv::new("cache".to_string()).build(); + let store = Kv::new("store".to_string()).build(); + let worker = alien_core::Worker::new("api".to_string()) + .permissions("execution".to_string()) + .code(alien_core::WorkerCode::Image { + image: "example.com/api:latest".to_string(), + }) + .link(&cache) + .link(&store) + .build(); + + let mut profile = alien_core::permissions::PermissionProfile::new(); + profile.0.insert( + "cache".to_string(), + vec![alien_core::permissions::PermissionSetReference::from("kv/data-read")], + ); + profile.0.insert( + "store".to_string(), + vec![alien_core::permissions::PermissionSetReference::from("kv/data-read")], + ); + let mut profiles = indexmap::IndexMap::new(); + profiles.insert("execution".to_string(), profile); + + let stack = Stack::new("gated-stack".to_string()) + .inputs(vec![input]) + .add_enabled_when(cache, ResourceLifecycle::Live, "cacheEnabled") + .add(store, ResourceLifecycle::Live) + .add(worker, ResourceLifecycle::Live) + .permissions(alien_core::permissions::PermissionsConfig { + profiles, + ..Default::default() + }) + .build(); + + let stack = strip_live(stack, false); + + let execution = stack + .permissions + .profiles + .get("execution") + .expect("profile should survive"); + assert!( + !execution.0.contains_key("cache"), + "the declined resource's grant must not survive it: {:?}", + execution.0.keys().collect::>() + ); + assert!( + execution.0.contains_key("store"), + "an accepted resource must keep its grant" + ); + } + + /// Only the declined id is scrubbed; an unrelated link on the same resource stays. + #[test] + fn scrubbing_leaves_unrelated_links_alone() { + let input = StackInputDefinition::deployer_boolean( + "cacheEnabled", + "Enable the cache", + "Whether to run the cache store.", + Some(true), + ); + let cache = Kv::new("cache".to_string()).build(); + let store = Kv::new("store".to_string()).build(); + let worker = alien_core::Worker::new("api".to_string()) + .permissions("execution".to_string()) + .code(alien_core::WorkerCode::Image { + image: "example.com/api:latest".to_string(), + }) + .link(&cache) + .link(&store) + .build(); + let stack = Stack::new("gated-stack".to_string()) + .inputs(vec![input]) + .add_enabled_when(cache, ResourceLifecycle::Live, "cacheEnabled") + .add(store, ResourceLifecycle::Live) + .add(worker, ResourceLifecycle::Live) + .build(); + + let stack = strip_live(stack, false); + + assert_eq!(link_ids(&stack, "api"), vec!["store".to_string()]); + } + /// Resolve-then-strip is the whole contract, so the tests exercise the /// pair: what each path proves, and what the strip does with it. fn resolve_and_strip( diff --git a/crates/alien-deployment/src/updating.rs b/crates/alien-deployment/src/updating.rs index 29de8b971..21c7b855b 100644 --- a/crates/alien-deployment/src/updating.rs +++ b/crates/alien-deployment/src/updating.rs @@ -7,6 +7,7 @@ use alien_core::{ }; use alien_error::{AlienError, Context}; use alien_infra::{RunningResourcePolicy, StackExecutor}; +use std::collections::HashSet; use tracing::{debug, info}; fn machines_deployment_has_zero_machines(platform: Platform, stack_state: &StackState) -> bool { @@ -20,7 +21,12 @@ fn machines_deployment_has_zero_machines(platform: Platform, stack_state: &Stack }) } -fn compute_update_status(stack_state: &StackState, target_stack: &Stack) -> Result { +fn compute_update_status( + stack_state: &StackState, + target_stack: &Stack, + reconciled: &HashSet<&str>, + pending_deletions: &[&str], +) -> Result { let statuses = stack_state .resources .iter() @@ -35,11 +41,45 @@ fn compute_update_status(stack_state: &StackState, target_stack: &Stack) -> Resu return Ok(StackStatus::Running); } - StackState::compute_stack_status_from_resources(&statuses).context( + let status = StackState::compute_stack_status_from_resources(&statuses).context( ErrorData::StackExecutionFailed { message: "Failed to compute update status".to_string(), }, - ) + )?; + + // Health is not completion, in both directions. The executor defers a resource's update + // while its new dependency provisions, and defers a deletion while a consumer still + // records the dependency — either way every status reads Running with work outstanding. + if status == StackStatus::Running + && (!stack_has_converged(stack_state, target_stack, reconciled) + || !pending_deletions.is_empty()) + { + return Ok(StackStatus::InProgress); + } + + Ok(status) +} + +/// Whether every resource the executor reconciles matches the config the release asks for. +/// +/// Scoped to `reconciled` because a resource the executor's lifecycle filter excluded is never +/// planned: a setup-owned resource whose recorded config differs from the declared one would +/// otherwise hold the update open forever. A resource missing from state has not converged. +fn stack_has_converged( + stack_state: &StackState, + target_stack: &Stack, + reconciled: &HashSet<&str>, +) -> bool { + target_stack + .resources + .iter() + .filter(|(resource_id, _)| reconciled.contains(resource_id.as_str())) + .all(|(resource_id, desired)| { + stack_state + .resources + .get(resource_id) + .is_some_and(|deployed| deployed.config == desired.config) + }) } /// Handle UpdatePending → Updating transition @@ -89,10 +129,9 @@ pub async fn handle_update_pending( // previous prepared stack, which was stripped the same way, and an // unstripped new stack would read as "frozen resource added" and refuse // the update — or worse, resurrect the resource the deployer declined. - // Live declines apply AFTER the mutations instead, so a declined - // workload's derived baseline (service account, profile grants, capacity - // contribution) stays identical to the accepted render and the - // compatibility checks never see a difference. + // Live declines apply AFTER the mutations, so a declined workload's service + // account and capacity stay identical to the accepted render. Its grant is + // scrubbed, which `permission_profiles_unchanged`'s gated exemption absorbs. let target_stack = crate::pending::strip_frozen_declines( target_stack, &persisted_gate_answers, @@ -285,6 +324,10 @@ pub async fn handle_updating( message: "Failed to create stack executor for update".to_string(), })?; + // Captured before stepping: completion is judged over exactly what this executor + // reconciles, so the two cannot disagree about which resources are in scope. + let reconciled_ids = executor.tracked_resource_ids(); + // Execute one step let mut step_result = executor @@ -304,7 +347,17 @@ pub async fn handle_updating( ); // Compute the stack status from the resulting state - let stack_status = compute_update_status(&step_result.next_state, &target_stack)?; + let pending_deletions = executor + .pending_deletions(&step_result.next_state) + .context(ErrorData::StackExecutionFailed { + message: "Failed to determine outstanding deletions".to_string(), + })?; + let stack_status = compute_update_status( + &step_result.next_state, + &target_stack, + &reconciled_ids, + &pending_deletions, + )?; // Check if update is complete let waiting_for_machines = @@ -512,6 +565,169 @@ mod tests { entry } + /// An update is finished when the executor has nothing left to do, not when every + /// resource is healthy. A consumer deferred while its dependency provisioned is still + /// healthy on its old config, so status alone reports success too early. + #[test] + fn an_update_is_not_complete_while_a_resource_has_not_reached_its_desired_config() { + let cache = Kv::new("cache".to_string()).build(); + let deployed_agent = Worker::new("agent".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/agent:latest".to_string(), + }) + .build(); + // What the release asks for: the same worker, now linked to the cache. + let desired_agent = Worker::new("agent".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/agent:latest".to_string(), + }) + .link(&cache) + .build(); + + let target_stack = Stack::new("s".to_string()) + .add(desired_agent, ResourceLifecycle::Live) + .add(cache.clone(), ResourceLifecycle::Live) + .build(); + + // The cache finished provisioning, so every resource is healthy, but the worker is + // still running the config it had before the link was added. + let mut stack_state = StackState::new(Platform::Aws); + stack_state.resources.insert( + "agent".to_string(), + state_entry(Resource::new(deployed_agent), ResourceStatus::Running), + ); + stack_state.resources.insert( + "cache".to_string(), + state_entry(Resource::new(cache), ResourceStatus::Running), + ); + + let reconciled = HashSet::from(["agent", "cache"]); + let status = compute_update_status(&stack_state, &target_stack, &reconciled, &[]) + .expect("status should compute"); + + assert_eq!( + status, + StackStatus::InProgress, + "the worker has not reached its desired config, so the update is not complete" + ); + } + + /// Declining a gate that was accepted must take the resource away, and the executor defers + /// that deletion for a step while the consumer still records a dependency on it. Judging + /// completion by config alone misses the deletion entirely, so the resource keeps running. + #[test] + fn an_update_is_not_complete_while_a_declined_resource_is_still_deployed() { + let agent = Worker::new("agent".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/agent:latest".to_string(), + }) + .build(); + // The gate was declined, so the release's target no longer declares the worker. + let target_stack = Stack::new("s".to_string()) + .add(agent.clone(), ResourceLifecycle::Live) + .build(); + + let mut stack_state = StackState::new(Platform::Aws); + stack_state.resources.insert( + "agent".to_string(), + state_entry(Resource::new(agent), ResourceStatus::Running), + ); + stack_state.resources.insert( + "declined".to_string(), + state_entry( + Resource::new(Worker::new("declined".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/declined:latest".to_string(), + }) + .build()), + ResourceStatus::Running, + ), + ); + + // The declined worker is not in the target, so the executor does not track it; the + // executor reports it as a deletion it still owes. + let reconciled = HashSet::from(["agent"]); + let status = compute_update_status(&stack_state, &target_stack, &reconciled, &["declined"]) + .expect("status should compute"); + + assert_eq!( + status, + StackStatus::InProgress, + "a declined resource still deployed means the update has work left" + ); + } + + /// A resource the executor will never delete — outside its deletion scope, or held by a + /// dependent that scope excludes — is absent from `pending_deletions`, so waiting on it + /// cannot hold the update open. + #[test] + fn a_deletion_the_executor_will_not_perform_cannot_hold_the_update_open() { + let agent = Worker::new("agent".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/agent:latest".to_string(), + }) + .build(); + let target_stack = Stack::new("s".to_string()) + .add(agent.clone(), ResourceLifecycle::Live) + .build(); + + let mut stack_state = StackState::new(Platform::Aws); + stack_state.resources.insert( + "agent".to_string(), + state_entry(Resource::new(agent), ResourceStatus::Running), + ); + stack_state.resources.insert( + "out-of-scope".to_string(), + state_entry( + Resource::new(Kv::new("out-of-scope".to_string()).build()), + ResourceStatus::Running, + ), + ); + + let reconciled = HashSet::from(["agent"]); + let status = compute_update_status(&stack_state, &target_stack, &reconciled, &[]) + .expect("status should compute"); + + assert_eq!( + status, + StackStatus::Running, + "a deletion the executor does not own must not block completion" + ); + } + + /// A setup-owned resource is excluded by the executor's lifecycle filter, so nothing will + /// ever reconcile it. Holding the update open until its recorded config matches the + /// declared one would never finish. + #[test] + fn a_resource_the_executor_does_not_reconcile_cannot_hold_the_update_open() { + let declared = Kv::new("store".to_string()).build(); + let target_stack = Stack::new("s".to_string()) + .add(declared, ResourceLifecycle::Frozen) + .build(); + + let deployed = Kv::new("other-name".to_string()).build(); + let mut stack_state = StackState::new(Platform::Aws); + let mut entry = state_entry(Resource::new(deployed), ResourceStatus::Running); + entry.lifecycle = Some(ResourceLifecycle::Frozen); + stack_state.resources.insert("store".to_string(), entry); + + // The executor reconciles only Live resources, so the frozen store is not in scope. + let reconciled = HashSet::new(); + let status = compute_update_status(&stack_state, &target_stack, &reconciled, &[]) + .expect("status should compute"); + + assert_eq!( + status, + StackStatus::Running, + "a resource outside the executor's scope must not block completion" + ); + } + /// Build a release stack where `cache` is declared behind a gate. fn release_stack_with_gated_cache(agent: &alien_core::Worker) -> Stack { let mut stack = Stack::new("s".to_string()) diff --git a/crates/alien-deployment/tests/test_platform.rs b/crates/alien-deployment/tests/test_platform.rs index c9d41eec4..8f6509dba 100644 --- a/crates/alien-deployment/tests/test_platform.rs +++ b/crates/alien-deployment/tests/test_platform.rs @@ -1548,6 +1548,97 @@ async fn live_gate_flip_deprovisions_and_reprovisions_across_updates() { assert_eq!(store.status, alien_core::ResourceStatus::Running); } +/// The same live-gate flip, but an ungated worker links the gated store — the shape the +/// link scrub allows. The consumer's recorded dependency makes the executor defer the +/// store's deletion for a step, so the update must stay open until the store is actually +/// gone rather than finishing on the step that scrubs the link. +#[tokio::test] +async fn declining_a_linked_gate_deprovisions_the_store_before_the_update_completes() { + let _temp_dir = TempDir::new().expect("Failed to create temp dir"); + + let stack = create_live_gated_stack_with_linking_worker("linked-gate-stack", "cache", "api"); + let config = create_test_config("hash_v1", false); + let mut state = create_initial_state(stack.clone()); + + // The declared default is true, so both the store and its consumer come up. + state = run_to_completion(state, config.clone()).await; + assert_eq!(state.status, DeploymentStatus::Running); + assert!(state + .stack_state + .as_ref() + .unwrap() + .resources + .contains_key("cache")); + + // Declining takes the store away and scrubs the link. The worker survives. + start_update(&mut state, release_of("rel_v2", stack)); + state = run_to_completion(state, config_with_store_enabled(false)).await; + + assert_eq!(state.status, DeploymentStatus::Running); + let resources = &state.stack_state.as_ref().unwrap().resources; + assert!( + !resources.contains_key("cache"), + "the declined store must be deprovisioned, got {:?}", + resources + .iter() + .map(|(id, r)| format!("{id}={:?}", r.status)) + .collect::>() + ); + let worker = resources + .get("api") + .expect("the ungated worker must outlive the store it linked"); + assert_eq!(worker.status, alien_core::ResourceStatus::Running); + let links = alien_core::links_of(&worker.config); + assert!( + !links.iter().any(|link| link.id == "cache"), + "the link must leave with the resource, got {links:?}" + ); +} + +/// A live gated store with an ungated worker linking it. +fn create_live_gated_stack_with_linking_worker( + stack_id: &str, + store_id: &str, + function_id: &str, +) -> Stack { + let store = Storage::new(store_id.to_string()).build(); + let function = Worker::new(function_id.to_string()) + .code(WorkerCode::Image { + image: "test:latest".to_string(), + }) + .permissions("default".to_string()) + .link(&store) + .build(); + + let mut stack = create_test_stack(stack_id, function_id); + stack.resources.insert( + function_id.to_string(), + ResourceEntry { + config: alien_core::Resource::new(function), + lifecycle: ResourceLifecycle::Live, + dependencies: Vec::new(), + remote_access: false, + enabled_when: None, + }, + ); + stack.resources.insert( + store_id.to_string(), + ResourceEntry { + config: alien_core::Resource::new(store), + lifecycle: ResourceLifecycle::Live, + dependencies: Vec::new(), + remote_access: false, + enabled_when: Some("storeEnabled".to_string()), + }, + ); + stack.inputs = vec![boolean_gate_input( + "storeEnabled", + "Enable the store", + "Whether to run the store.", + )]; + stack +} + /// A frozen gate is answered once: the answer recorded at creation refuses /// every later conflicting input value. #[tokio::test] diff --git a/crates/alien-infra/src/core/executor.rs b/crates/alien-infra/src/core/executor.rs index 0c7542027..91bac0dc6 100644 --- a/crates/alien-infra/src/core/executor.rs +++ b/crates/alien-infra/src/core/executor.rs @@ -305,6 +305,15 @@ impl StackExecutor { StackExecutorConfig::builder(stack, client_config) } + /// The resources this executor reconciles, after the lifecycle filter. + /// + /// A caller deciding whether a stack has converged must ask about these and no others: a + /// resource the filter excluded is never planned, so holding it to the desired config + /// would wait on work that will never happen. + pub fn tracked_resource_ids(&self) -> HashSet<&str> { + self.resources.keys().map(String::as_str).collect() + } + /// Simple constructor for basic use cases (tests, examples). /// For production use, prefer the builder pattern. pub fn new( @@ -592,6 +601,105 @@ impl StackExecutor { Ok(true) } + /// Map of resource id to the ids still depending on it, for deletion safety checks. + fn deletion_dependents(&self, state: &StackState) -> HashMap> { + let mut has_dependents: HashMap> = HashMap::new(); + if self.lifecycle_filter.is_none() && !self.runtime_cleanup_filter { + return has_dependents; + } + + for (res_id, resource_state) in &state.resources { + if resource_state.status == ResourceStatus::Deleting + || resource_state.status == ResourceStatus::Deleted + { + continue; + } + // State dependencies, not config ones: they record what the deployed resource + // still holds, which is what makes deleting its target unsafe. + for dependency in &resource_state.dependencies { + has_dependents + .entry(dependency.id().to_string()) + .or_default() + .push(res_id.clone()); + } + } + has_dependents + } + + /// Whether this executor will delete a resource that is absent from the desired stack: + /// not already settled, inside the deletion scope, and not held by a dependent the scope + /// excludes. + fn is_deletion_planned( + &self, + resource_id: &str, + current_resource_state: &StackResourceState, + has_dependents: &HashMap>, + state: &StackState, + ) -> Result { + if current_resource_state.status == ResourceStatus::Deleting + || current_resource_state.status == ResourceStatus::Deleted + || current_resource_state.status == ResourceStatus::DeleteFailed + || (self.runtime_cleanup_filter + && current_resource_state.status == ResourceStatus::TeardownRequired) + { + return Ok(false); + } + + if self.lifecycle_filter.is_none() && !self.runtime_cleanup_filter { + return Ok(true); + } + + if !self.resource_matches_deletion_scope(resource_id, current_resource_state)? { + debug!( + "Resource '{}' is outside deletion scope, skipping deletion", + resource_id + ); + return Ok(false); + } + + if let Some(dependent_resources) = has_dependents.get(resource_id) { + for dep_id in dependent_resources { + let Some(dependent_state) = state.resources.get(dep_id) else { + continue; + }; + if dependent_state.status == ResourceStatus::Deleting + || dependent_state.status == ResourceStatus::Deleted + || dependent_state.status == ResourceStatus::TeardownRequired + { + continue; + } + if !self.resource_matches_deletion_scope(dep_id, dependent_state)? { + debug!( + "Resource '{}' has active dependents outside deletion scope, skipping deletion", + resource_id + ); + return Ok(false); + } + } + } + + Ok(true) + } + + /// The resources this executor will delete from `state` and has not deleted yet. + /// + /// Answered by the planner's own rule so a caller judging completion cannot wait on a + /// deletion the planner refuses to schedule — a resource out of scope, or held by a + /// dependent the scope excludes, is never pending here. + pub fn pending_deletions<'a>(&self, state: &'a StackState) -> Result> { + let has_dependents = self.deletion_dependents(state); + let mut pending = Vec::new(); + for (resource_id, resource_state) in &state.resources { + if self.resources.contains_key(resource_id) { + continue; + } + if self.is_deletion_planned(resource_id, resource_state, &has_dependents, state)? { + pending.push(resource_id.as_str()); + } + } + Ok(pending) + } + /// Computes the **diff** between the *desired* stack configuration and the /// *current* [`StackState`]. /// @@ -665,93 +773,25 @@ impl StackExecutor { } } - // Pre-check: When using lifecycle filters, build a dependency map to check if resources can be safely deleted - let mut has_dependents: HashMap> = HashMap::new(); - - if self.lifecycle_filter.is_some() || self.runtime_cleanup_filter { - // Build map of resource_id -> list of resources that depend on it - for (res_id, resource_state) in &state.resources { - // Skip resources that are already being deleted or deleted - if resource_state.status == ResourceStatus::Deleting - || resource_state.status == ResourceStatus::Deleted - { - continue; - } - - // Use the dependencies from the state instead of getting them from the config - for dependency in &resource_state.dependencies { - let dep_id = dependency.id().to_string(); - has_dependents - .entry(dep_id) - .or_default() - .push(res_id.clone()); - } - } - } + let has_dependents = self.deletion_dependents(state); // 2. Identify Deletes & Updates for (resource_id, current_resource_state) in &state.resources { match self.resources.get(resource_id) { // Resource NOT in desired state -> Delete (unless filtered by lifecycle) None => { - // Skip if the resource status is already Deleting, Deleted, or DeleteFailed - if current_resource_state.status == ResourceStatus::Deleting - || current_resource_state.status == ResourceStatus::Deleted - || current_resource_state.status == ResourceStatus::DeleteFailed - || (self.runtime_cleanup_filter - && current_resource_state.status == ResourceStatus::TeardownRequired) - { - continue; - } - - if self.lifecycle_filter.is_some() || self.runtime_cleanup_filter { - if !self - .resource_matches_deletion_scope(resource_id, current_resource_state)? - { - debug!( - "Resource '{}' is outside deletion scope, skipping deletion", - resource_id - ); - continue; - } - - // Now check if this resource has any active dependents - if let Some(dependent_resources) = has_dependents.get(resource_id) { - // Check if any active dependent is outside the same deletion scope. - let mut has_active_dependents_outside_filter = false; - for dep_id in dependent_resources { - let Some(dependent_state) = state.resources.get(dep_id) else { - continue; - }; - if dependent_state.status == ResourceStatus::Deleting - || dependent_state.status == ResourceStatus::Deleted - || dependent_state.status == ResourceStatus::TeardownRequired - { - continue; - } - - if !self.resource_matches_deletion_scope(dep_id, dependent_state)? { - has_active_dependents_outside_filter = true; - break; - } - } - - if has_active_dependents_outside_filter { - debug!( - "Resource '{}' has active dependents outside deletion scope, skipping deletion", - resource_id - ); - continue; - } - } + if self.is_deletion_planned( + resource_id, + current_resource_state, + &has_dependents, + state, + )? { + debug!( + "Planning DELETE for resource '{}' (status: {:?})", + resource_id, current_resource_state.status + ); + plan_result.deletes.push(resource_id.clone()); } - - // If we get here, the resource should be deleted - debug!( - "Planning DELETE for resource '{}' (status: {:?})", - resource_id, current_resource_state.status - ); - plan_result.deletes.push(resource_id.clone()); } // Resource EXISTS in desired state -> Check for Update Some(desired_config) => { @@ -1297,31 +1337,35 @@ impl StackExecutor { let update_state = current_state.clone(); + let desired_config = match self.resources.get(resource_id) { + Some(config) => config, + None => { + error!( + "Resource '{}' not found in desired config during update", + resource_id + ); + let failed_update_state = current_state.with_failure( + ResourceStatus::UpdateFailed, + AlienError::new(ErrorData::InfrastructureError { + message: format!( + "Resource '{}' not found in desired config during update", + resource_id + ), + operation: Some("update_dependencies".to_string()), + resource_id: Some(resource_id.clone()), + }) + .into_generic(), + ); + initial_transitions.insert(resource_id.clone(), failed_update_state); + continue; + } + }; + // Try to get the controller and transition to update match update_state.get_internal_controller() { Ok(Some(mut controller)) => { match controller.transition_to_update() { Ok(()) => { - // Calculate updated dependencies based on new config - let desired_config = match self.resources.get(resource_id) { - Some(config) => config, - None => { - error!( - "Resource '{}' not found in desired config during update", - resource_id - ); - let failed_update_state = current_state - .with_failure(ResourceStatus::UpdateFailed, AlienError::new(ErrorData::InfrastructureError { - message: format!("Resource '{}' not found in desired config during update", resource_id), - operation: Some("update_dependencies".to_string()), - resource_id: Some(resource_id.clone()), - }).into_generic()); - initial_transitions - .insert(resource_id.clone(), failed_update_state); - continue; - } - }; - let updated_state = update_state.with_updates(|state| { state.config = new_config.clone(); // Set to new desired config state.previous_config = Some(current_state.config.clone()); // Store old config @@ -1370,10 +1414,60 @@ impl StackExecutor { } } Ok(None) => { - warn!( - "Cannot start update transition from state {:?}, skipping planned update", - current_state.status - ); + // An external binding carries no controller and no cloud work: adopt + // the new config the way creation adopts the binding as Running, or + // the planner re-plans this same update forever. + if self.is_external_binding_resource(resource_id) { + let remote_access = self + .desired_stack + .resources + .get(resource_id) + .map(|entry| entry.remote_access) + .unwrap_or(false); + // Serialized only while remote access is on, and cleared when it + // is not, matching the create path and the controller step. + let remote_binding_params = match ( + remote_access, + self.deployment_config.external_bindings.get(resource_id), + ) { + (true, Some(binding)) => Some( + serde_json::to_value(binding).into_alien_error().context( + ErrorData::ResourceStateSerializationFailed { + resource_id: resource_id.clone(), + message: "Failed to serialize external binding parameters" + .to_string(), + }, + )?, + ), + _ => None, + }; + let updated_state = update_state.with_updates(|state| { + state.config = new_config.clone(); + state.previous_config = Some(current_state.config.clone()); + state.dependencies = desired_config.dependencies.clone(); + state.status = ResourceStatus::Running; + state.remote_binding_params = remote_binding_params; + state.retry_attempt = 0; + state.error = None; + state.last_failed_state = None; + }); + initial_transitions.insert(resource_id.clone(), updated_state); + } else { + error!( + "Cannot start update transition for '{}' from state {:?}: no controller state", + resource_id, current_state.status + ); + let failed_update_state = current_state.with_failure( + ResourceStatus::UpdateFailed, + AlienError::new(ErrorData::ResourceStateSerializationFailed { + resource_id: resource_id.clone(), + message: "Missing controller state for planned update" + .to_string(), + }) + .into_generic(), + ); + initial_transitions.insert(resource_id.clone(), failed_update_state); + } } Err(e) => { error!( diff --git a/crates/alien-infra/src/core/executor_tests/edge_cases_tests.rs b/crates/alien-infra/src/core/executor_tests/edge_cases_tests.rs index d75b4552c..71cff33ef 100644 --- a/crates/alien-infra/src/core/executor_tests/edge_cases_tests.rs +++ b/crates/alien-infra/src/core/executor_tests/edge_cases_tests.rs @@ -8,6 +8,7 @@ use alien_core::{ ExternalBindings, Resource, ResourceLifecycle, ResourceRef, ResourceStatus, Stack, StackSettings, Storage, StorageBinding, }; +use std::collections::HashSet; fn external_storage_config(resource_id: &str) -> DeploymentConfig { let mut external_bindings = ExternalBindings::new(); @@ -563,6 +564,135 @@ async fn test_external_binding_resource_syncs_without_controller() -> Result<()> Ok(()) } +/// A config change on an external binding has no cloud work behind it: the update +/// must adopt the new config directly, or the planner re-plans the same update on +/// every step and the stack never reaches synced. +#[tokio::test] +async fn test_external_binding_resource_adopts_config_changes() -> Result<()> { + let storage = test_storage_with_public_read("external-store", false); + let stack = Stack::new("external-binding-update-test".to_owned()) + .add(storage.clone(), ResourceLifecycle::Frozen) + .build(); + let deployment_config = external_storage_config("external-store"); + let executor = StackExecutor::builder(&stack, ClientConfig::Test) + .deployment_config(&deployment_config) + .build()?; + let state = run_to_synced(&executor, new_test_state()).await?; + + let updated_storage = test_storage_with_public_read("external-store", true); + let updated_stack = Stack::new("external-binding-update-test".to_owned()) + .add(updated_storage.clone(), ResourceLifecycle::Frozen) + .build(); + let update_executor = StackExecutor::builder(&updated_stack, ClientConfig::Test) + .deployment_config(&deployment_config) + .build()?; + + let state = run_steps(&update_executor, state, 3).await?; + let external_state = state.resources.get("external-store").unwrap(); + + assert_eq!(external_state.status, ResourceStatus::Running); + assert_eq!( + external_state.config, + Resource::new(updated_storage), + "the external binding must adopt the new declared config" + ); + assert!( + external_state.internal_state.is_none(), + "adopting a config change must not synthesize controller state" + ); + assert!( + update_executor.is_synced(&state), + "a config change on an external binding must reach synced" + ); + + Ok(()) +} + +/// A resource dropped from the desired stack is owed a deletion even while a consumer still +/// records a dependency on it — the executor defers that delete for a step, and a caller +/// judging completion has to see the debt or it declares success one step early. +#[tokio::test] +async fn test_pending_deletions_reports_a_deferred_delete() -> Result<()> { + let agent = test_function("agent"); + let stack = Stack::new("pending-deletions-test".to_owned()) + .add(agent, ResourceLifecycle::Live) + .build(); + + // State still holds the dropped worker, and the agent still depends on it. + let mut state = new_test_state(); + let mut agent_state = create_running_function_state("agent", "test-image-agent"); + agent_state.dependencies = vec![ResourceRef::new(alien_core::Worker::RESOURCE_TYPE, "dropped")]; + state.resources.insert("agent".to_string(), agent_state); + state.resources.insert( + "dropped".to_string(), + create_running_function_state("dropped", "test-image-dropped"), + ); + + let executor = new_executor_with_filter(&stack, vec![ResourceLifecycle::Live])?; + assert_eq!( + executor.pending_deletions(&state)?, + vec!["dropped"], + "a dropped resource still deployed is a deletion this executor owes" + ); + + Ok(()) +} + +/// The deletion scope is the whole answer: a resource this executor would never delete must +/// not be reported as owed, or a caller waiting on it waits forever. +#[tokio::test] +async fn test_pending_deletions_excludes_resources_outside_the_deletion_scope() -> Result<()> { + let agent = test_function("agent"); + let stack = Stack::new("pending-deletions-scope-test".to_owned()) + .add(agent, ResourceLifecycle::Live) + .build(); + + let mut state = new_test_state(); + state.resources.insert( + "agent".to_string(), + create_running_function_state("agent", "test-image-agent"), + ); + let mut frozen_state = create_running_function_state("frozen-leftover", "n/a"); + frozen_state.lifecycle = Some(ResourceLifecycle::Frozen); + state + .resources + .insert("frozen-leftover".to_string(), frozen_state); + + let executor = new_executor_with_filter(&stack, vec![ResourceLifecycle::Live])?; + assert!( + executor.pending_deletions(&state)?.is_empty(), + "a frozen leftover is outside a Live-filtered executor's deletion scope" + ); + + Ok(()) +} + +/// `tracked_resource_ids` is the executor's post-filter view: a caller judging +/// convergence must see exactly the resources this executor reconciles. +#[tokio::test] +async fn test_tracked_resource_ids_follow_the_lifecycle_filter() -> Result<()> { + let frozen_store = test_storage("frozen-store"); + let live_func = test_function("live-func"); + let stack = Stack::new("tracked-ids-test".to_owned()) + .add(frozen_store, ResourceLifecycle::Frozen) + .add(live_func, ResourceLifecycle::Live) + .build(); + + let filtered = new_executor_with_filter(&stack, vec![ResourceLifecycle::Live])?; + assert_eq!( + filtered.tracked_resource_ids(), + HashSet::from(["live-func"]) + ); + + let unfiltered = new_executor(&stack)?; + assert_eq!( + unfiltered.tracked_resource_ids(), + HashSet::from(["frozen-store", "live-func"]) + ); + + Ok(()) +} + /// Full-stack deletion should mark controllerless external bindings deleted /// instead of looping forever waiting for a controller that intentionally /// does not exist. diff --git a/crates/alien-preflights/src/compile_time/resource_enabled_valid.rs b/crates/alien-preflights/src/compile_time/resource_enabled_valid.rs index 64b5f53d7..338642808 100644 --- a/crates/alien-preflights/src/compile_time/resource_enabled_valid.rs +++ b/crates/alien-preflights/src/compile_time/resource_enabled_valid.rs @@ -149,60 +149,133 @@ impl CompileTimeCheck for ResourceEnabledValidCheck { } } - errors.extend(dependents_of_gated_resources(stack)); - - if errors.is_empty() { - Ok(CheckResult::success()) - } else { - Ok(CheckResult::failed(errors)) + let (dependent_errors, warnings) = dependents_of_gated_resources(stack); + errors.extend(dependent_errors); + + // Severity is per finding: a scrubbable link only warns, but every other rule here + // still fails, including the `"*"`-grant refusal above, which is a security rule. + match (errors.is_empty(), warnings.is_empty()) { + (true, true) => Ok(CheckResult::success()), + (true, false) => Ok(CheckResult::with_warnings(warnings)), + (false, _) => Ok(CheckResult::failed_with_warnings(errors, warnings)), } } } -/// Rejects resources that would outlive a gated resource they read outputs from. +/// Sorts dependents of gated resources into refusals and warnings. /// -/// `StackState::get_resource_outputs` errors when a resource is absent, so an -/// ungated dependent of a gated resource fails at deploy time in the customer's -/// account. Catching it here fails when the manifest is written instead. -fn dependents_of_gated_resources(stack: &Stack) -> Vec { +/// A pure link is dropped with the declined resource, so its owner keeps its own lifecycle +/// without that binding. Every other edge refuses, because nothing removes it. +fn dependents_of_gated_resources(stack: &Stack) -> (Vec, Vec) { let gates: HashMap<&str, &str> = stack .resources() .filter_map(|(id, entry)| Some((id.as_str(), entry.enabled_when.as_deref()?))) .collect(); + // A gate resolves either when setup renders the template or when the runtime strip runs, + // decided by where the gated resource is emitted. + let resolves_at_runtime: HashMap<&str, bool> = stack + .resources() + .map(|(id, entry)| { + let emitted_in_setup = alien_core::ownership_policy_for_resource_type( + entry.config.resource_type().as_ref(), + ) + .should_emit_in_setup(entry.lifecycle); + (id.as_str(), !emitted_in_setup) + }) + .collect(); + let mut errors = Vec::new(); + let mut warnings = Vec::new(); + for (dependent_id, entry) in stack.resources() { // `ResourceEntry` documents the total as `config.get_dependencies()` plus its own - // list, and each compute type folds its links and triggers into the former. That - // canonical aggregation is used here rather than the per-type downcast in - // `resource_link_permissions`, which only keeps the permission-bearing link types. + // list, and each compute type folds its links and triggers into the former. let config_dependencies = entry.config.get_dependencies(); + let links = alien_core::links_of(&entry.config); + let mut seen: Vec<&str> = Vec::new(); for dependency in config_dependencies.iter().chain(&entry.dependencies) { - let Some(dependency_gate) = gates.get(dependency.id()) else { + let dependency_id = dependency.id(); + let Some(dependency_gate) = gates.get(dependency_id) else { continue; }; - let dependency_id = dependency.id(); + // One finding per target id; the counts below re-read the full chain, so a + // duplicate edge still registers there. + if seen.contains(&dependency_id) { + continue; + } + seen.push(dependency_id); + + // Sharing the gate is already correct: the two rise and fall together. + if entry.enabled_when.as_deref() == Some(*dependency_gate) { + continue; + } + + // Counting rather than testing membership: a resource that is both linked and + // triggered appears twice, and the trigger is the half the scrub cannot remove. + let link_count = links.iter().filter(|l| l.id() == dependency_id).count(); + let total = config_dependencies + .iter() + .chain(&entry.dependencies) + .filter(|d| d.id() == dependency_id) + .count(); + + // The runtime cannot rewrite a setup-created resource, so a reference it holds to + // a runtime-resolved gate can be neither dropped nor restored. The `expect` is + // unreachable: both maps come from the same pass, and `gates` already resolved this id. + let owner_baked_in_setup = alien_core::ownership_policy_for_resource_type( + entry.config.resource_type().as_ref(), + ) + .should_emit_in_setup(entry.lifecycle); + let frozen_owner_of_runtime_gate = owner_baked_in_setup + && *resolves_at_runtime + .get(dependency_id) + .expect("a gated dependency is always a stack resource"); + + // A setup-created owner's link renders into the install template, so only an + // owner the runtime manages can survive its target's decline. + if link_count > 0 && total == link_count && !owner_baked_in_setup { + warnings.push(format!( + "Resource '{dependent_id}' links '{dependency_id}', which is enabled by input \ + '{dependency_gate}'. A deployer who says no drops the link too, so \ + '{dependent_id}' starts without {}. Make sure it runs without it", + alien_core::binding_env_var_name(dependency_id) + )); + continue; + } match entry.enabled_when.as_deref() { - Some(gate) if gate == *dependency_gate => {} Some(gate) => errors.push(format!( "Resource '{dependent_id}' depends on '{dependency_id}', but the two are \ gated on different inputs: '{gate}' and '{dependency_gate}'. Nothing makes a \ deployer answer both the same way, so '{dependent_id}' can be created while \ '{dependency_id}' is not. Gate both on '{dependency_gate}'" )), + None if frozen_owner_of_runtime_gate => errors.push(format!( + "Setup-created resource '{dependent_id}' depends on '{dependency_id}', which \ + input '{dependency_gate}' decides at runtime. Setup bakes that reference and \ + the runtime cannot rewrite a setup-created resource, so the answer could \ + never reach '{dependent_id}'. Gate '{dependent_id}' on '{dependency_gate}' \ + too, or make '{dependency_id}' setup-created so the answer is fixed at install" + )), + None if owner_baked_in_setup && total == link_count => errors.push(format!( + "Setup-created resource '{dependent_id}' links '{dependency_id}', which is \ + enabled by input '{dependency_gate}'. Setup renders that binding into the \ + install template, so declining '{dependency_id}' would break the template's \ + reference. Gate '{dependent_id}' on '{dependency_gate}' too" + )), None => errors.push(format!( "Resource '{dependent_id}' depends on '{dependency_id}', which is enabled by \ - input '{dependency_gate}'. A deployer who says no would leave \ - '{dependent_id}' looking up outputs of a resource that was never created. \ - Gate '{dependent_id}' on '{dependency_gate}' too" + input '{dependency_gate}'. The dependency is not a plain link, so declining \ + it would leave '{dependent_id}' pointing at a resource that was never \ + created. Gate '{dependent_id}' on '{dependency_gate}' too" )), } } } - errors + (errors, warnings) } #[cfg(test)] @@ -242,6 +315,158 @@ mod tests { .errors } + async fn result_for(stack: Stack) -> CheckResult { + ResourceEnabledValidCheck + .check(&stack, Platform::Aws) + .await + .expect("check should run") + } + + /// An ungated worker linking a gated store, which is the shape the scrub exists for. + fn stack_with_worker_linking_gated_store(worker_gate: Option<&str>) -> Stack { + let store = Kv::new("store".to_string()).build(); + let worker = Worker::new("api".to_string()) + .permissions("execution".to_string()) + .code(WorkerCode::Image { + image: "example.com/api:latest".to_string(), + }) + .link(&store) + .build(); + + let mut worker_input = boolean_input(); + worker_input.id = "workerEnabled".to_string(); + let builder = Stack::new("test-stack".to_string()) + .inputs(vec![boolean_input(), worker_input]) + .add_enabled_when(store, ResourceLifecycle::Live, "storeEnabled"); + + match worker_gate { + Some(gate) => builder.add_enabled_when(worker, ResourceLifecycle::Live, gate), + None => builder.add(worker, ResourceLifecycle::Live), + } + .build() + } + + /// The link is dropped with the resource, so the worker outliving it is legitimate — + /// but the author still has to handle the binding being absent. + #[tokio::test] + async fn warns_rather_than_rejects_an_ungated_worker_linking_a_gated_store() { + let result = result_for(stack_with_worker_linking_gated_store(None)).await; + + assert!(result.success, "should not block: {:?}", result.errors); + assert!(result.errors.is_empty(), "{:?}", result.errors); + assert_eq!(result.warnings.len(), 1, "{:?}", result.warnings); + assert!( + result.warnings[0].contains("Resource 'api' links 'store'"), + "{:?}", + result.warnings + ); + assert!( + result.warnings[0].contains("ALIEN_STORE_BINDING"), + "{:?}", + result.warnings + ); + } + + /// Two independent answers still produce a worker that can outlive its store, but the + /// link is scrubbed either way, so this is the same legitimate shape. + #[tokio::test] + async fn warns_for_a_linking_worker_gated_on_a_different_input() { + let result = result_for(stack_with_worker_linking_gated_store(Some("workerEnabled"))).await; + + assert!(result.success, "should not block: {:?}", result.errors); + assert_eq!(result.warnings.len(), 1, "{:?}", result.warnings); + } + + /// Sharing the gate means they rise and fall together, so there is nothing to say. + #[tokio::test] + async fn stays_silent_when_the_linking_worker_shares_the_gate() { + let result = result_for(stack_with_worker_linking_gated_store(Some("storeEnabled"))).await; + + assert!(result.success, "{:?}", result.errors); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + } + + /// Build is setup-created, so its link renders into the install template and a declined + /// target would break the template's reference; refused rather than warned. + #[tokio::test] + async fn rejects_a_setup_created_owner_linking_a_gated_store() { + let store = Kv::new("store".to_string()).build(); + let builder = alien_core::Build::new("packager".to_string()) + .permissions("build".to_string()) + .link(&store) + .build(); + + let stack = Stack::new("test-stack".to_string()) + .inputs(vec![boolean_input()]) + .add_enabled_when(store, ResourceLifecycle::Frozen, "storeEnabled") + .add(builder, ResourceLifecycle::Frozen) + .build(); + + let result = result_for(stack).await; + + assert!(!result.success, "a setup-created link owner must be refused"); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert_eq!(result.errors.len(), 1, "{:?}", result.errors); + assert!( + result.errors[0].contains("'packager' links 'store'"), + "{:?}", + result.errors + ); + } + + /// Setup bakes a frozen resource's links and the runtime cannot rewrite it, so a runtime + /// answer could never reach the link. The scrub would drop it from the desired stack while + /// the deployed resource kept it. + #[tokio::test] + async fn rejects_a_frozen_link_owner_against_a_runtime_resolved_gate() { + let store = Kv::new("store".to_string()).build(); + let builder = alien_core::Build::new("packager".to_string()) + .permissions("build".to_string()) + .link(&store) + .build(); + + let stack = Stack::new("test-stack".to_string()) + .inputs(vec![boolean_input()]) + .add_enabled_when(store, ResourceLifecycle::Live, "storeEnabled") + .add(builder, ResourceLifecycle::Frozen) + .build(); + + let result = result_for(stack).await; + assert_eq!(result.errors.len(), 1, "{:?}", result.errors); + assert!(result.errors[0].contains("decides at runtime"), "{:?}", result.errors); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + } + + /// The scrub only removes links. A queue reached by both a link and a trigger keeps the + /// trigger wiring on the source resource, so this must stay a refusal. + #[tokio::test] + async fn rejects_a_queue_that_is_both_linked_and_triggered() { + let queue = alien_core::Queue::new("jobs".to_string()).build(); + let queue_ref = alien_core::ResourceRef { + resource_type: alien_core::Queue::RESOURCE_TYPE.clone(), + id: "jobs".to_string(), + }; + let worker = Worker::new("consumer".to_string()) + .permissions("consumer".to_string()) + .code(WorkerCode::Image { + image: "example.com/consumer:latest".to_string(), + }) + .link(&queue) + .trigger(alien_core::WorkerTrigger::Queue { queue: queue_ref }) + .build(); + + let stack = Stack::new("test-stack".to_string()) + .inputs(vec![boolean_input()]) + .add_enabled_when(queue, ResourceLifecycle::Live, "storeEnabled") + .add(worker, ResourceLifecycle::Live) + .build(); + + let result = result_for(stack).await; + assert_eq!(result.errors.len(), 1, "{:?}", result.errors); + assert!(result.errors[0].contains("not a plain link"), "{:?}", result.errors); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + } + #[tokio::test] async fn rejects_a_framework_derived_resource() { let stack = Stack::new("test-stack".to_string()) diff --git a/crates/alien-test/src/distribution.rs b/crates/alien-test/src/distribution.rs index 568a19a54..b0cc6e16c 100644 --- a/crates/alien-test/src/distribution.rs +++ b/crates/alien-test/src/distribution.rs @@ -2350,6 +2350,49 @@ fn sanitize_kubernetes_dns_label(value: &str) -> String { } } +/// Re-import an already-installed stack. Unlike [`import_stack`] it skips fetching the +/// deployment record, which a flip already holds. +async fn reimport_stack( + manager_url: &str, + dg_token: &str, + request: StackImportRequest, +) -> anyhow::Result { + let url = format!("{manager_url}/v1/stack/import"); + // The manager refuses an import while a reconcile is settling and asks the caller to + // retry once stable; a background pass can open that window at any moment, so obey it. + let deadline = std::time::Instant::now() + REIMPORT_RETRY_TIMEOUT; + loop { + let response = reqwest::Client::new() + .post(&url) + .bearer_auth(dg_token) + .json(&request) + .send() + .await + .context("Failed to call /v1/stack/import for a gate flip")?; + + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + if status == reqwest::StatusCode::CONFLICT + && body.contains("IMPORTED_DEPLOYMENT_CONFLICT") + && std::time::Instant::now() < deadline + { + info!("Deployment is still reconciling; retrying the flip's import"); + tokio::time::sleep(std::time::Duration::from_secs(5)).await; + continue; + } + if !status.is_success() { + anyhow::bail!("stack re-import failed with {status}: {body}"); + } + + let response: StackImportResponse = + serde_json::from_str(&body).context("Failed to parse StackImportResponse")?; + return Ok(response.stack_state); + } +} + +/// How long a flip's import retries `IMPORTED_DEPLOYMENT_CONFLICT` before giving up. +const REIMPORT_RETRY_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); + async fn import_stack( prepared: &DistributionPrepared, request: StackImportRequest, diff --git a/crates/alien-test/tests/distribution.rs b/crates/alien-test/tests/distribution.rs index 97ad88656..b357dedb5 100644 --- a/crates/alien-test/tests/distribution.rs +++ b/crates/alien-test/tests/distribution.rs @@ -154,6 +154,15 @@ async fn check_enabled_demo(ctx: &mut alien_test::TestContext) -> anyhow::Result "optional-queue-off", ) .await?; + // A vault on AWS is an SSM name prefix, not a listable resource; its cloud footprint + // is the IAM policy carrying its id, which also proves the grant follows the gate. + assert_cloud_gate_pair( + &env, + &["iam", "get-account-authorization-details", "--output", "json"], + "optional-vault-on", + "optional-vault-off", + ) + .await?; // A compute gate rides the live strip: the declined worker's function is // never provisioned, the accepted one is. assert_cloud_gate_pair( @@ -181,9 +190,41 @@ async fn check_enabled_demo(ctx: &mut alien_test::TestContext) -> anyhow::Result ) .await?; + // The links half of the gate: an ungated worker linking gated resources keeps only the + // links whose target survived, so it holds the `-on` bindings and not the `-off` ones. + let links = agent_link_ids(&stack_state)?; + for id in ["optional-kv-on", "optional-worker-on"] { + anyhow::ensure!( + links.contains(&id.to_string()), + "agent should keep its link to accepted '{id}', got {links:?}" + ); + } + for id in ["optional-kv-off", "optional-worker-off"] { + anyhow::ensure!( + !links.contains(&id.to_string()), + "agent must not keep a link to declined '{id}', got {links:?}" + ); + } + + // Flipping a gate on a running deployment is an upgrade, which this harness does not + // cover for any app. The strip, scrub and deprovision it drives run against the test + // platform in `alien-deployment`'s `test_platform` suite instead. + Ok(()) } +/// The resource ids the ungated `agent` worker still links, read from the manager's view. +fn agent_link_ids(stack_state: &alien_core::StackState) -> anyhow::Result> { + let agent = stack_state + .resources + .get("agent") + .context("stack_state is missing the ungated 'agent' worker")?; + Ok(alien_core::links_of(&agent.config) + .iter() + .map(|link| link.id.clone()) + .collect()) +} + /// Runs one read-only `aws` list call and asserts the enabled sibling's id is /// present in the output while the declined sibling's id is absent. async fn assert_cloud_gate_pair( diff --git a/tests/e2e/test-apps/enabled-demo/alien.ts b/tests/e2e/test-apps/enabled-demo/alien.ts index 9a8c8e9e0..e285088b1 100644 --- a/tests/e2e/test-apps/enabled-demo/alien.ts +++ b/tests/e2e/test-apps/enabled-demo/alien.ts @@ -96,13 +96,6 @@ const queueOff = new alien.Queue("optional-queue-off").enabled(io.queueOff).buil const vaultOn = new alien.Vault("optional-vault-on").enabled(io.vaultOn).build() const vaultOff = new alien.Vault("optional-vault-off").enabled(io.vaultOff).build() -const agent = new alien.Worker("agent") - .code({ type: "source", src: "./", toolchain: { type: "typescript" } }) - .commandsEnabled(true) - .publicEndpoint("api") - .permissions("execution") - .build() - // A compute gate is a live gate: the declined worker's function must never be // provisioned while its profile-derived service account still exists — the // baseline that lets a later acceptance recreate the function. Each gated @@ -119,6 +112,18 @@ const workerOff = new alien.Worker("optional-worker-off") .enabled(io.workerOff) .build() +// Ungated, linking both halves of two gated pairs so one deployment proves both answers. +// The kv pair is frozen and fixed at install; the worker pair is live and grant-free, which +// is what makes it flippable without a runtime policy write on the setup-owned role. +const agent = new alien.Worker("agent") + .code({ type: "source", src: "./", toolchain: { type: "typescript" } }) + .permissions("execution") + .link(kvOn) + .link(kvOff) + .link(workerOn) + .link(workerOff) + .build() + export default new alien.Stack("enabled-demo") .inputs(io) .add(state, "frozen") @@ -135,11 +140,9 @@ export default new alien.Stack("enabled-demo") .add(workerOff, "live") .permissions({ profiles: { - // Each gated resource carries its own resource-scoped grant so the e2e can - // assert the grant follows the gate (present when on, gone when off). The - // worker binds only the ungated `state` store; it depends on no gated - // resource, so the ungated-dependent-of-a-gated-resource preflight stays - // satisfied. + // Each gated resource carries its own resource-scoped grant so the e2e can assert + // the grant follows the gate. A declined resource's grant leaves the desired stack + // with it, and acceptance re-derives it from the declared stack on the next deploy. execution: { state: ["kv/data-read"], "optional-kv-on": ["kv/data-read"],