From 10cb124d1c8aa300edd62b3df25167ee24b26c92 Mon Sep 17 00:00:00 2001 From: Alon Gubkin Date: Thu, 30 Jul 2026 23:12:22 +0300 Subject: [PATCH 1/2] fix: keep refresh and frozen IAM setup-owned --- crates/alien-deployment/src/running.rs | 9 +- crates/alien-deployment/src/updating.rs | 23 +---- crates/alien-infra/src/core/executor.rs | 73 +++++++++----- .../src/core/executor_tests/update_tests.rs | 35 +++++++ .../src/core/resource_permissions_helper.rs | 47 ++++----- .../src/deployment_prerequisites.rs | 95 ++++++++++++++++++- crates/alien-preflights/src/lib.rs | 3 + crates/alien-preflights/src/runner.rs | 6 +- 8 files changed, 205 insertions(+), 86 deletions(-) diff --git a/crates/alien-deployment/src/running.rs b/crates/alien-deployment/src/running.rs index 82176ec72..3a0de07d5 100644 --- a/crates/alien-deployment/src/running.rs +++ b/crates/alien-deployment/src/running.rs @@ -61,13 +61,6 @@ pub async fn handle_running( )?; } - // TODO: Add mechanism to limit executor to only perform read-only health checks - // and prevent any mutable operations on cloud resources during refresh. - // This could be done by: - // 1. Adding a "read-only" mode to StackExecutor - // 2. Having controllers check this mode and skip any mutating operations - // 3. Or creating a separate HealthCheckExecutor that only calls read methods - // Create executor with the target stack configuration // No lifecycle filter - check all resources during health checks let executor = StackExecutor::builder(&target_stack, client_config) @@ -82,7 +75,7 @@ pub async fn handle_running( // The Ready handlers perform heartbeat checks (e.g., verify function still exists, bucket is accessible) let step_result = executor - .step(stack_state) + .refresh(stack_state) .await .context(ErrorData::StackExecutionFailed { message: "Failed to execute health check step".to_string(), diff --git a/crates/alien-deployment/src/updating.rs b/crates/alien-deployment/src/updating.rs index 11e6c60f1..ad354c101 100644 --- a/crates/alien-deployment/src/updating.rs +++ b/crates/alien-deployment/src/updating.rs @@ -263,18 +263,6 @@ pub async fn handle_updating( message: "Pending prepared stack not found in runtime metadata".to_string(), }) })?; - let setup_update_authorized = runtime_metadata - .setup_update_authorization - .as_ref() - .is_some_and(|authorization| { - authorization.target_frozen_digest == target_stack.frozen_resources_digest() - && current - .target_release - .as_ref() - .and_then(|release| release.release_id.as_deref()) - == Some(authorization.release_id.as_str()) - }); - // Inject environment variables into the prepared stack crate::helpers::inject_environment_variables(&mut target_stack, &config, current.platform)?; @@ -305,19 +293,10 @@ pub async fn handle_updating( debug!("Secrets already synced, continuing with update"); } - // Create executor for resources - // By default, only deploy live resources (frozen resources don't change) - // If allow_frozen_changes is true, also deploy frozen resources - let mut lifecycle_filter_vec = vec![ResourceLifecycle::Live]; - if config.allow_frozen_changes || setup_update_authorized { - info!("Including frozen resources in authorized update"); - lifecycle_filter_vec.push(ResourceLifecycle::Frozen); - } - let executor = StackExecutor::builder(&target_stack, client_config) .deployment_config(&config) .running_resource_policy(RunningResourcePolicy::OptIn) - .lifecycle_filter(lifecycle_filter_vec) + .lifecycle_filter(vec![ResourceLifecycle::Live]) .service_provider(service_provider) .build() .context(ErrorData::StackExecutionFailed { diff --git a/crates/alien-infra/src/core/executor.rs b/crates/alien-infra/src/core/executor.rs index 3aa91dbb6..1006bb973 100644 --- a/crates/alien-infra/src/core/executor.rs +++ b/crates/alien-infra/src/core/executor.rs @@ -620,7 +620,10 @@ impl StackExecutor { fn external_binding_state( &self, resource_id: &str, - ) -> Result<(Option, Option)> { + ) -> Result<( + Option, + Option, + )> { let Some(binding) = self.deployment_config.external_bindings.get(resource_id) else { return Ok((None, None)); }; @@ -1139,36 +1142,55 @@ impl StackExecutor { /// The method is entirely *stateless* from the executor's perspective: all /// data required for the next iteration is carried within the returned /// `StackState`. + pub async fn step(&self, state: StackState) -> Result { + self.step_inner(state, true).await + } + + /// Steps controllers that are already active without planning stack changes. + /// + /// Refresh deliberately does not compare the persisted state with the desired + /// stack, resume failed updates, or initiate create/update/delete transitions. + /// It is suitable for periodic observation of a Running deployment. + pub async fn refresh(&self, state: StackState) -> Result { + self.step_inner(state, false).await + } + #[alien_event(AlienEvent::StackStep { next_state: state.clone(), suggested_delay_ms: None, })] - pub async fn step(&self, state: StackState) -> Result { + async fn step_inner(&self, state: StackState, apply_plan: bool) -> Result { validate_stack_controller_state_versions(&state)?; let mut state = state; - for (resource_id, resource_state) in &mut state.resources { - if resource_state.status != ResourceStatus::UpdateFailed { - continue; - } - let Some(desired) = self.resources.get(resource_id) else { - continue; - }; - if resource_state.config != desired.resource { - continue; - } - if resource_state.retry_failed()? { - debug!( - "Resumed unchanged failed update for '{}' before planning", - resource_id - ); + if apply_plan { + for (resource_id, resource_state) in &mut state.resources { + if resource_state.status != ResourceStatus::UpdateFailed { + continue; + } + let Some(desired) = self.resources.get(resource_id) else { + continue; + }; + if resource_state.config != desired.resource { + continue; + } + if resource_state.retry_failed()? { + debug!( + "Resumed unchanged failed update for '{}' before planning", + resource_id + ); + } } } let mut next_state = state.clone(); // Clone the input state to modify // --- Planning Phase --- - let plan_result = self.plan(&state)?; + let plan_result = if apply_plan { + self.plan(&state)? + } else { + PlanResult::default() + }; debug!( "Plan result: {} creates, {} updates, {} deletes", plan_result.creates.len(), @@ -1663,11 +1685,12 @@ impl StackExecutor { let context_resource: Resource; // Use current desired config from the stack if it exists, otherwise use stored config for deletion - context_resource = if let Some(resource_config) = self.resources.get(&resource_id) { - // Resource is still in desired stack (create/update case) - resource_config.resource.clone() + context_resource = if apply_plan { + self.resources + .get(&resource_id) + .map(|resource_config| resource_config.resource.clone()) + .unwrap_or_else(|| current_resource_state.config.clone()) } else { - // Resource is not in desired stack (deletion case) - use stored config from stack state current_resource_state.config.clone() }; @@ -1890,8 +1913,10 @@ impl StackExecutor { }; // Automatically update config to match desired state (except during deletion) - let next_config = if current_resource_state.status == ResourceStatus::Deleting { - // During deletion, preserve the current config + let next_config = if !apply_plan + || current_resource_state.status == ResourceStatus::Deleting + { + // Refresh and deletion preserve the deployed config. current_resource_state.config.clone() } else { // For create/update operations, ensure config matches desired state diff --git a/crates/alien-infra/src/core/executor_tests/update_tests.rs b/crates/alien-infra/src/core/executor_tests/update_tests.rs index c8129ad54..50f864d5e 100644 --- a/crates/alien-infra/src/core/executor_tests/update_tests.rs +++ b/crates/alien-infra/src/core/executor_tests/update_tests.rs @@ -38,6 +38,41 @@ async fn test_config_change_triggers_update() -> Result<()> { Ok(()) } +/// Refresh observes the persisted deployment; it must not turn desired release +/// drift into an update. +#[tokio::test] +async fn test_refresh_does_not_plan_config_changes() -> Result<()> { + let func_v1 = test_function_with_image("func1", "image-v1"); + let stack_v1 = Stack::new("refresh-test".to_owned()) + .add(func_v1.clone(), ResourceLifecycle::Live) + .build(); + let state = run_to_synced(&new_executor(&stack_v1)?, new_test_state()).await?; + + let stack_v2 = Stack::new("refresh-test".to_owned()) + .add( + test_function_with_image("func1", "image-v2"), + ResourceLifecycle::Live, + ) + .add(test_storage("new-storage"), ResourceLifecycle::Frozen) + .build(); + let refreshed = new_executor(&stack_v2)?.refresh(state).await?.next_state; + + assert_eq!( + refreshed.resources.len(), + 1, + "refresh must not create desired resources missing from persisted state" + ); + let resource = refreshed.resources.get("func1").unwrap(); + assert_eq!(resource.status, ResourceStatus::Running); + assert_eq!( + resource.config, + Resource::new(func_v1), + "refresh must retain the deployed config instead of applying desired drift" + ); + + Ok(()) +} + /// Tests that config changes while a resource is still provisioning do not /// interrupt the in-flight create. The update should happen after create reaches /// a stable state. diff --git a/crates/alien-infra/src/core/resource_permissions_helper.rs b/crates/alien-infra/src/core/resource_permissions_helper.rs index a1c13fccb..a9051fd0c 100644 --- a/crates/alien-infra/src/core/resource_permissions_helper.rs +++ b/crates/alien-infra/src/core/resource_permissions_helper.rs @@ -1069,6 +1069,15 @@ impl ResourcePermissionsHelper { resource_name: &str, resource_type: &str, ) -> Result<()> { + if !Self::resource_is_setup_owned(ctx, resource_id) { + debug!( + resource_id = %resource_id, + resource_name = %resource_name, + "Skipping AWS resource-scoped data policy attachment for live resource; these policies are setup-owned" + ); + return Ok(()); + } + let aws_config = ctx.get_aws_config()?; // Build permission context for this specific resource @@ -1132,34 +1141,16 @@ impl ResourcePermissionsHelper { } } - if Self::resource_is_setup_owned(ctx, resource_id) { - // Setup-owned resources run while setup credentials are still active. - // Live resource controllers must not edit the management role after - // the deployment has moved to provisioning credentials. - Self::apply_aws_management_resource_permissions( - ctx, - resource_id, - resource_name, - resource_type, - &generator, - &permission_context, - ) - .await?; - } else if ctx - .desired_stack - .management() - .profile() - .map(|profile| { - !Self::aws_management_resource_permission_refs(profile, resource_id).is_empty() - }) - .unwrap_or(false) - { - debug!( - resource_id = %resource_id, - resource_name = %resource_name, - "Skipping AWS management resource-scoped permissions for live resource; setup must grant them" - ); - } + // Setup-owned resources run while setup credentials are still active. + Self::apply_aws_management_resource_permissions( + ctx, + resource_id, + resource_name, + resource_type, + &generator, + &permission_context, + ) + .await?; Ok(()) } diff --git a/crates/alien-preflights/src/deployment_prerequisites.rs b/crates/alien-preflights/src/deployment_prerequisites.rs index 4be07126c..dadeb33b4 100644 --- a/crates/alien-preflights/src/deployment_prerequisites.rs +++ b/crates/alien-preflights/src/deployment_prerequisites.rs @@ -9,8 +9,8 @@ use crate::error::Result; use crate::{CheckResult, DeploymentPrerequisiteCheck}; use alien_core::{ validate_binding_type, ComputeBackend, ComputeCluster, Container, Daemon, DeploymentConfig, - EnvironmentVariable, ExposeProtocol, KubernetesCluster, PermissionSet, Platform, ResourceEntry, - ResourceLifecycle, Stack, StackState, Worker, + EnvironmentVariable, ExposeProtocol, KubernetesCluster, PermissionProfile, PermissionSet, + Platform, ResourceEntry, ResourceLifecycle, Stack, StackState, Worker, }; use alien_permissions::{ generators::AwsRuntimePermissionsGenerator, BindingTarget, PermissionContext, @@ -53,6 +53,73 @@ fn external_binding_required_types(platform: Platform) -> &'static [&'static str } } +/// AWS runtime resource controllers deliberately do not attach data-access +/// policies. An exact resource grant therefore has to target setup-owned +/// infrastructure, whose concrete policy is installed during setup. +pub struct AwsExactPermissionsSetupOwnedCheck; + +#[async_trait::async_trait] +impl DeploymentPrerequisiteCheck for AwsExactPermissionsSetupOwnedCheck { + fn code(&self) -> Option<&'static str> { + Some("AWS_EXACT_PERMISSION_REQUIRES_SETUP_OWNED_RESOURCE") + } + + fn description(&self) -> &'static str { + "AWS exact-resource permissions require setup-owned resources" + } + + fn should_run( + &self, + _stack: &Stack, + stack_state: &StackState, + _config: &DeploymentConfig, + ) -> bool { + stack_state.platform == Platform::Aws + } + + async fn check( + &self, + stack: &Stack, + _stack_state: &StackState, + _config: &DeploymentConfig, + ) -> Result { + let mut errors = Vec::new(); + + for (profile_name, profile) in &stack.permissions.profiles { + collect_live_exact_permission_errors(stack, profile_name, profile, &mut errors); + } + + if errors.is_empty() { + Ok(CheckResult::success()) + } else { + Ok(CheckResult::failed(errors)) + } + } +} + +fn collect_live_exact_permission_errors( + stack: &Stack, + profile_name: &str, + profile: &PermissionProfile, + errors: &mut Vec, +) { + for (resource_id, permissions) in profile + .0 + .iter() + .filter(|(resource_id, _)| *resource_id != "*") + { + if stack.resources.get(resource_id).is_some_and(|entry| { + entry.lifecycle == ResourceLifecycle::Live && !permissions.is_empty() + }) { + errors.push(format!( + "AWS permission profile '{profile_name}' targets Live resource '{resource_id}'. \ + Exact-resource data policies are setup-owned and runtime resource controllers cannot author them. \ + Make '{resource_id}' Frozen or use a stack-scoped ('*') permission." + )); + } + } +} + /// Validates that cloud container deployments have a managed container backend. pub struct ManagedContainerBackendRequiredCheck; @@ -651,6 +718,30 @@ mod tests { } } + #[tokio::test] + async fn aws_exact_permissions_reject_live_resources_but_allow_frozen_resources() { + let check = AwsExactPermissionsSetupOwnedCheck; + + for (lifecycle, expected_success) in [ + (ResourceLifecycle::Live, false), + (ResourceLifecycle::Frozen, true), + ] { + let mut stack = Stack::new("permissions-test".to_string()) + .add(Storage::new("objects".to_string()).build(), lifecycle) + .build(); + stack.permissions.profiles.insert( + "content".to_string(), + PermissionProfile::new().resource("objects", ["storage/data-write"]), + ); + + let result = check + .check(&stack, &stack_state(Platform::Aws), &deployment_config()) + .await + .unwrap(); + assert_eq!(result.success, expected_success, "{:?}", result.errors); + } + } + fn domain_metadata(resource_id: &str) -> DomainMetadata { DomainMetadata { base_domain: "example.com".to_string(), diff --git a/crates/alien-preflights/src/lib.rs b/crates/alien-preflights/src/lib.rs index d0c99ff57..eb1af66a4 100644 --- a/crates/alien-preflights/src/lib.rs +++ b/crates/alien-preflights/src/lib.rs @@ -356,6 +356,9 @@ impl PreflightRegistry { registry.add_deployment_prerequisite_check(Box::new( deployment_prerequisites::AwsLiveManagementPermissionsSetupCheck, )); + registry.add_deployment_prerequisite_check(Box::new( + deployment_prerequisites::AwsExactPermissionsSetupOwnedCheck, + )); registry.add_deployment_prerequisite_check(Box::new( deployment_prerequisites::TargetResourcesResolveCheck, )); diff --git a/crates/alien-preflights/src/runner.rs b/crates/alien-preflights/src/runner.rs index 52589b011..5c13f8d13 100644 --- a/crates/alien-preflights/src/runner.rs +++ b/crates/alien-preflights/src/runner.rs @@ -404,9 +404,11 @@ impl PreflightRunner { // Run compatibility checks on mutated stack if old stack is provided // This detects if mutations added frozen resources during updates - // Skip the check if allow_frozen_changes flag is set + // Frozen changes are setup-owned. A matching setup authorization proves + // that the setup workflow already applied and imported them; a runtime + // configuration flag must never authorize their mutation. if let Some(old_stack) = old_stack { - if !config.allow_frozen_changes && !setup_update_authorized { + if !setup_update_authorized { let compatibility_summary = self .run_compatibility_checks(old_stack, &mutated_stack) .await?; From 2a63ea7a4c9e013025ad3bd3db5f3294ea86f7a5 Mon Sep 17 00:00:00 2001 From: Alon Gubkin Date: Thu, 30 Jul 2026 23:36:49 +0300 Subject: [PATCH 2/2] fix: drop removed frozen state on reimport --- crates/alien-manager/src/routes/stack.rs | 64 +++++++++++++++++++ .../src/stores/sqlite/deployment.rs | 18 ++---- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/crates/alien-manager/src/routes/stack.rs b/crates/alien-manager/src/routes/stack.rs index f6b9c121c..6887fc167 100644 --- a/crates/alien-manager/src/routes/stack.rs +++ b/crates/alien-manager/src/routes/stack.rs @@ -571,9 +571,30 @@ fn merge_reimported_stack_state( })?; imported.resources.insert(resource.id.clone(), merged); } + + // The setup payload is authoritative for setup-owned resources: omission + // means setup removed one. Runtime-owned resources are not present in that + // payload, so carry their current controller state across explicitly. + preserve_live_runtime_state(stack, existing, &mut imported); + Ok(imported) } +fn preserve_live_runtime_state(stack: &Stack, existing: &StackState, imported: &mut StackState) { + for (resource_id, entry) in stack.resources() { + if entry.lifecycle != alien_core::ResourceLifecycle::Live + || imported.resources.contains_key(resource_id) + { + continue; + } + if let Some(existing_resource) = existing.resources.get(resource_id) { + imported + .resources + .insert(resource_id.clone(), existing_resource.clone()); + } + } +} + fn assert_supported_import_region( config: &crate::config::ManagerConfig, req: &StackImportRequest, @@ -1408,4 +1429,47 @@ mod setup_update_authorization_tests { ); assert!(metadata.registry_access_granted); } + + #[test] + fn reimport_preserves_live_state_without_resurrecting_removed_frozen_state() { + let target = stack("live", "frozen-new"); + let mut existing = StackState::new(Platform::Aws); + for resource_id in ["live", "frozen-old"] { + existing.resources.insert( + resource_id.to_string(), + StackResourceState::new_pending( + if resource_id == "live" { + Worker::RESOURCE_TYPE.to_string() + } else { + Storage::RESOURCE_TYPE.to_string() + }, + target + .resources + .get(if resource_id == "live" { + "live" + } else { + "frozen-new" + }) + .unwrap() + .config + .clone(), + Some(if resource_id == "live" { + ResourceLifecycle::Live + } else { + ResourceLifecycle::Frozen + }), + vec![], + ), + ); + } + let mut imported = StackState::new(Platform::Aws); + + preserve_live_runtime_state(&target, &existing, &mut imported); + + assert!(imported.resources.contains_key("live")); + assert!( + !imported.resources.contains_key("frozen-old"), + "an omitted setup-owned resource must remain removed" + ); + } } diff --git a/crates/alien-manager/src/stores/sqlite/deployment.rs b/crates/alien-manager/src/stores/sqlite/deployment.rs index dee8b9824..7e99b62bc 100644 --- a/crates/alien-manager/src/stores/sqlite/deployment.rs +++ b/crates/alien-manager/src/stores/sqlite/deployment.rs @@ -714,19 +714,11 @@ impl DeploymentStore for SqliteDeploymentStore { schedule_reconciliation, input_values, } = params; - let mut merged_stack_state = stack_state; - if let Some(existing) = self.get_deployment(caller, deployment_id).await? { - if let Some(mut existing_stack_state) = existing.stack_state { - for (resource_id, resource_state) in merged_stack_state.resources { - existing_stack_state - .resources - .insert(resource_id, resource_state); - } - merged_stack_state = existing_stack_state; - } - } - - let stack_state_json = serde_json::to_string(&merged_stack_state) + // The import route supplies a complete ownership-aware state: freshly + // imported setup resources plus preserved Live controller state. + // Re-merging here would resurrect setup-owned resources that the new + // setup payload intentionally omitted. + let stack_state_json = serde_json::to_string(&stack_state) .into_alien_error() .context(GenericError { message: "Failed to serialize imported stack_state".to_string(),