diff --git a/docs/easytier-web-managed-config-sync-plan.md b/docs/easytier-web-managed-config-sync-plan.md index c2ae168c..36d5d60e 100644 --- a/docs/easytier-web-managed-config-sync-plan.md +++ b/docs/easytier-web-managed-config-sync-plan.md @@ -7,9 +7,9 @@ - 上游依赖：后续由 Console 计算并发送 Patch - 兼容要求：保留现有 Full PUT -本文记录当前接收端方案。Session 在能够证明 Patch base 与已应用 revision 连续时 -只收敛 touched instances；重启、通知丢失、revision 断链或并发积压时沿用 Full -reconcile。 +本文记录当前接收端方案。Session 合并已持久化 Patch 的 touched instance IDs， +并在运行态收敛时读取这些实例的最新持久化状态。重启、通知丢失或无法安全判断 +实例 ownership 时沿用 Full reconcile。 ## 1. 背景与结论 @@ -30,8 +30,10 @@ Console 每次发布都会向该路径发送完整 Exact Set。实例很多时 3. PATCH 使用 `expected_config_revision` 做 compare-and-swap（CAS）。 4. Full/Patch 的配置变更与 revision 更新在一个 SQLite transaction 中提交。 5. Patch 只查询和写入 touched instances，不扫描完整 Target。 -6. 写入成功后通知 Session 本次 base、target 和 touched instance IDs。 -7. Session 仅在 applied revision 精确匹配 base 时增量收敛，否则安全回退 Full。 +6. 写入成功后通知 Session 本次 expected、target 和 transaction 实际 touched + instance IDs。 +7. Session 只合并 revision 连续的 touched IDs，并以 SQLite 当前状态为准增量 + 收敛；可信 runtime base、通知链或 persisted target 无法证明连续时回退 Full。 普通变更的接收端成本由： @@ -371,20 +373,33 @@ revision。只影响 user-owned rows 的操作不清除 managed revision。 - 只有带 target revision 的 `Applied` 才通知匹配的 live Session； `AlreadyApplied`、legacy unrevisioned Full、conflict 和失败不重复通知。 - Notification 必须发生在 commit 之后。 -- Full notification 清除任何 pending delta，触发完整收敛。 +- Full notification 将 pending reconcile hint 提升为 Full，触发完整收敛。 - Patch notification 携带 expected revision、target revision、upsert IDs 和本次 transaction 实际接受删除的 web-owned IDs。请求删除但数据库原本不存在的 ID - 仍是 no-op，不能借机删除 Core 中同 ID 的 user-owned 实例。只有 Session applied - revision 精确等于 expected revision，且没有更早的 Patch 等待处理时，才保留该 - delta。 -- 两次 Patch 在前一次完成前积压时不合并 delta；Session 清除 pending delta，并在 - 最新 heartbeat/revision 上执行一次 Full。这避免引入 Patch queue 或 delivery FSM。 -- 增量 round 只读取 upsert rows，只删除本次 delete IDs，只对 touched running - instances 执行 runtime Patch/Run。完成前再次校验 persisted target revision；只有 - 全部 touched instances 成功且 target 仍相同，才推进 applied revision。 + 仍是 no-op，不能借机删除 Core 中同 ID 的 user-owned 实例。 +- Session 将尚未应用的 Patch touched IDs 合并为一个 Dirty set，并始终以 SQLite + 最新 revision 下的 rows 为准。它不重放历史 Patch，也不维护 Patch queue 或 + delivery FSM。只有 incoming expected 等于 pending target 的通知才能合并；Dirty + hint 保留最早 expected 和最新 target。乱序、不连续或无法证明顺序的通知将 hint + 提升为 Full。多个连续 Patch 积压时，旧 round 由 runtime epoch 拦截，下一 round + 直接收敛到最新 target。 +- Session 分开记录对外报告的 applied revision 和内部可信的 runtime base。开始任何 + runtime side effect 前清除 applied；Patch round 的 side effects 完全包含在 Dirty + set 中，因此失败或被新通知拦截时仍保留最早 runtime base，以便按最新持久化状态 + 重试 Dirty set。Full round、direct mutation、授权失败或 Session ownership 中断会 + 清除 runtime base。 +- 只有可信 runtime base 等于 Dirty 最早 expected，并且 SQLite persisted revision + 等于 Dirty 最新 target 时，才允许增量 round。重连后 runtime base 未知、通知 + 丢失，或 SQLite 已经提交了更靠后的 revision 而通知尚未送达时都回退 Full，避免 + 不完整的 Dirty set 把完整 target revision 误标为已应用。 +- 增量 round 逐个读取 Dirty set 中的最新 row。仍然存在且启用的 web-owned row + 使用其最新 config；已经删除的 row 进入 delete set；遇到 disabled 或非 web-owned + row 时回退 Full，以保留 ownership 规则。完成前再次校验 persisted target + revision；只有全部 touched instances 成功且 target 仍相同，才推进 applied + revision。 - 任何通过 EasyTier Web mutation route 直接 Run、Save、Delete 或切换实例状态的 操作在执行前和结束后（包括部分 side effect 后返回错误）都清除 Session applied - revision 与 pending delta、增加运行配置 cache epoch，并唤醒一次 Full + revision、可信 runtime base 与 pending hint，增加运行配置 cache epoch，并唤醒一次 Full reconcile。旧 round 只有 epoch 仍匹配时才能推进 applied revision；新一轮不得 信任 mutation 前缓存的 runtime config。否则 runtime-only mutation 或 Core 成功、 SQLite 失败的复合 mutation 可能在 persisted revision 不变时破坏 Patch base 的 @@ -491,11 +506,13 @@ response 当作旧 receiver 并静默换一种 mutation contract；出现 404 - 超限 Full 稳定返回 413/422，而不是耗尽进程内存； - 并发请求无 deadlock，且 CAS 结果确定。 -Session 测试还必须验证：精确 base/target 使用 touched-instance reconcile；base -不匹配、目标 revision 已变化、Full notification 和 Patch backlog 都使用 Full； -touched runtime apply 失败不推进 applied revision；删除只作用于本次 delete IDs。 -运行态 Config Get/Patch/Run/Delete 数量应随 touched instances 增长。为确认运行实例 -身份而进行的一次 list/meta RPC 可以保留，它不发送或重写所有实例配置。 +Session 测试还必须验证：连续 Patch 的 Dirty IDs 会合并且保留最早 expected；未知 +或不匹配的 runtime base、乱序/不连续通知使用 Full；增量 round 读取最新 row；已经 +删除的 web-owned row 只删除对应 Dirty ID；Full notification 覆盖 Dirty hint；目标 +revision 已变化或 touched runtime apply 失败时不推进 applied revision；直接 runtime +mutation 使 revision 与运行配置 cache 同时失效。运行态 Config Get/Patch/Run/Delete +数量应随 touched instances 增长。为确认运行实例身份而进行的一次 list/meta RPC +可以保留，它不发送或重写所有实例配置。 ## 11. Observability @@ -523,9 +540,13 @@ Rollout acceptance： ### 12.1 Session runtime delta apply（已实现） -Patch commit outcome 已携带 touched IDs。Session 只在 applied revision 正好等于 -Patch base 时执行 touched-instance reconcile；重启、revision 断链、通知丢失或 -并发 Patch backlog 都退回 Full。接收端不保存 Patch queue，也不合并 delta。 +Patch commit outcome 已携带 expected、target 和 transaction 实际 touched IDs。 +Session 只合并 expected/target 连续的 Dirty IDs，并在每一轮从 SQLite 读取最新 +target revision 对应的当前 rows；因此正常积压只增加 Dirty set，不需要保留中间 +revision 的 Patch queue。可信 runtime base 必须等于 Dirty 最早 expected；Patch +side effect 失败可保留该 base 重试，通知丢失、乱序、进程重启或新 Session 尚无 +runtime base 时回退 Full。Full +notification、disabled row 或 ownership 无法证明时也回退 Full。 ### 12.2 Chunked Full diff --git a/easytier-core/src/instance/manager.rs b/easytier-core/src/instance/manager.rs index fa4b6ab9..3351d607 100644 --- a/easytier-core/src/instance/manager.rs +++ b/easytier-core/src/instance/manager.rs @@ -15,7 +15,7 @@ use dashmap::DashMap; use uuid::Uuid; use crate::config::toml::TomlConfig; -use crate::instance::{CoreInstance, CoreInstanceHost}; +use crate::instance::{CoreInstance, CoreInstanceHost, CoreInstanceState}; use crate::process_runtime::CoreProcessRuntime; #[cfg(feature = "web-client")] use crate::{ @@ -205,6 +205,39 @@ struct ActiveStopGuard { notifier: Arc, } +#[derive(Default)] +struct InstanceStateChanges { + generation: AtomicUsize, + notify: tokio::sync::Notify, +} + +impl InstanceStateChanges { + fn generation(&self) -> usize { + self.generation.load(Ordering::Acquire) + } + + fn mark_changed(&self) { + self.generation.fetch_add(1, Ordering::AcqRel); + self.notify.notify_waiters(); + } + + async fn wait_for_change(&self, generation: usize) -> usize { + loop { + let notified = self.notify.notified(); + tokio::pin!(notified); + // Register the waiter before reading the generation: notify_waiters + // does not retain permits, so a change landing between the read + // and the await would otherwise be missed until the next change. + notified.as_mut().enable(); + let current = self.generation(); + if current != generation { + return current; + } + notified.await; + } + } +} + impl Drop for ActiveStopGuard { fn drop(&mut self) { let previous = self.active_stops.fetch_sub(1, Ordering::AcqRel); @@ -224,6 +257,7 @@ pub struct InstanceManager { mutation_lock: Arc>, runtime_handle: Option, active_stops: Arc, + instance_state_changes: Arc, } impl InstanceManager { @@ -238,6 +272,7 @@ impl InstanceManager { mutation_lock: Arc::new(tokio::sync::Mutex::new(())), runtime_handle, active_stops: Arc::new(AtomicUsize::new(0)), + instance_state_changes: Arc::new(InstanceStateChanges::default()), } } @@ -311,6 +346,16 @@ impl InstanceManager { notifier: self.notifier.clone(), } } + + pub(crate) fn instance_state_generation(&self) -> usize { + self.instance_state_changes.generation() + } + + pub(crate) async fn wait_for_instance_state_change(&self, generation: usize) -> usize { + self.instance_state_changes + .wait_for_change(generation) + .await + } } impl InstanceManager { @@ -339,10 +384,12 @@ where let instance_id = instance.instance_id(); self.config_controls.insert(instance_id, control); let notifier = self.notifier.clone(); + let instance_state_changes = self.instance_state_changes.clone(); runtime.spawn(async move { if let Err(error) = instance.start().await { tracing::error!(%error, %instance_id, "instance failed to start"); } + instance_state_changes.mark_changed(); notifier.notify_one(); }); Ok(instance_id) @@ -373,6 +420,7 @@ where drop(active_stop); return Ok(self.instance_ids()); } + self.instance_state_changes.mark_changed(); runtime .spawn(async move { @@ -403,6 +451,19 @@ where .collect() } + pub fn failed_instance_ids(&self) -> Vec { + self.list() + .into_iter() + .filter(|instance| { + instance.state() == CoreInstanceState::Stopped + && instance + .latest_error() + .is_some_and(|error| !error.trim().is_empty()) + }) + .map(|instance| instance.instance_id()) + .collect() + } + pub fn instance(&self, instance_id: Uuid) -> Option>> { self.get(instance_id) } @@ -533,6 +594,39 @@ where } } +#[cfg(test)] +mod instance_state_change_tests { + use super::InstanceStateChanges; + use std::{sync::Arc, time::Duration}; + + #[tokio::test] + async fn state_change_wait_observes_existing_and_future_changes() { + let changes = Arc::new(InstanceStateChanges::default()); + + let observed = changes.generation(); + changes.mark_changed(); + assert_eq!( + tokio::time::timeout(Duration::from_secs(1), changes.wait_for_change(observed)) + .await + .expect("an existing change must not be lost"), + 1 + ); + + let observed = changes.generation(); + let waiter_changes = changes.clone(); + let waiter = tokio::spawn(async move { waiter_changes.wait_for_change(observed).await }); + tokio::task::yield_now().await; + changes.mark_changed(); + assert_eq!( + tokio::time::timeout(Duration::from_secs(1), waiter) + .await + .expect("a future change must wake the waiter") + .unwrap(), + 2 + ); + } +} + #[cfg(test)] mod tests { use std::{ diff --git a/easytier-core/src/instance/tests.rs b/easytier-core/src/instance/tests.rs index c2031a03..773993ad 100644 --- a/easytier-core/src/instance/tests.rs +++ b/easytier-core/src/instance/tests.rs @@ -1395,6 +1395,74 @@ virtual_ip = "10.82.0.2/24" ); } + #[tokio::test] + async fn manager_reports_only_stopped_instances_with_errors() { + use crate::instance::manager::{InstanceFactory, InstanceManager}; + + struct StateTestFactory; + + impl InstanceFactory for StateTestFactory { + type Instance = CoreInstance; + type CreateContext = (); + type Error = anyhow::Error; + + fn create( + &self, + config: TomlConfig, + (): Self::CreateContext, + ) -> Result, Self::Error> { + let (packet_sink, _packet_receiver) = tokio::sync::mpsc::channel(16); + CoreInstance::from_toml(config, adapters(None, Arc::new(packet_sink))) + } + } + + fn create_instance( + manager: &InstanceManager, + name: &str, + ) -> Arc> { + let config = TomlConfig::new_from_str(&format!("instance_name = \"{name}\"")).unwrap(); + manager.create(config, ()).unwrap() + } + + let manager = InstanceManager::new(StateTestFactory, None); + let running = create_instance(&manager, "running"); + running + .latest_error + .write() + .replace("old startup error".to_owned()); + running.set_state(CoreInstanceState::Running); + + let starting = create_instance(&manager, "starting"); + starting + .latest_error + .write() + .replace("old startup error".to_owned()); + starting.set_state(CoreInstanceState::Starting); + + let stopped_without_error = create_instance(&manager, "stopped-without-error"); + stopped_without_error.set_state(CoreInstanceState::Stopped); + + let stopped_with_blank_error = create_instance(&manager, "stopped-with-blank-error"); + stopped_with_blank_error + .latest_error + .write() + .replace(" \n".to_owned()); + stopped_with_blank_error.set_state(CoreInstanceState::Stopped); + + let failed = create_instance(&manager, "failed"); + failed + .latest_error + .write() + .replace("startup failed".to_owned()); + failed.set_state(CoreInstanceState::Stopped); + let failed_id = failed.instance_id(); + + assert_eq!(manager.failed_instance_ids(), vec![failed_id]); + + manager.delete_network_instances([failed_id]).await.unwrap(); + assert!(manager.failed_instance_ids().is_empty()); + } + #[tokio::test] async fn aborting_host_prepare_runs_unified_cleanup() { #[derive(Default)] @@ -1739,6 +1807,142 @@ virtual_ip = "10.82.0.2/24" assert!(instances.instances().is_empty()); } + #[cfg(feature = "web-client")] + #[tokio::test] + async fn process_management_rpc_collects_only_requested_instances() { + use std::{collections::VecDeque, sync::Mutex as StdMutex}; + + use crate::{ + config::toml::TomlConfig, + instance::manager::InstanceFactory, + management::{InstanceManager, ProcessManagementRpc, UnsupportedConfigFileStorage}, + }; + use easytier_proto::{ + api::manage::{CollectNetworkInfoRequest, WebClientService}, + rpc_types::controller::BaseController, + }; + + #[derive(Default)] + struct RecordingRuntimeHost { + collection_count: AtomicUsize, + } + + #[async_trait] + impl InstanceRuntimeHost for RecordingRuntimeHost { + async fn prepare( + &self, + _packet_plane: Arc, + ) -> anyhow::Result>> { + Ok(None) + } + + async fn shutdown(&self) {} + + fn management_events(&self) -> Vec { + self.collection_count.fetch_add(1, Ordering::Relaxed); + Vec::new() + } + } + + struct RecordingFactory { + process_runtime: Arc, + runtime_hosts: StdMutex>>, + } + + impl InstanceFactory for RecordingFactory { + type Instance = CoreInstance; + type CreateContext = (); + type Error = anyhow::Error; + + fn create( + &self, + config: TomlConfig, + (): Self::CreateContext, + ) -> Result, Self::Error> { + let runtime_host = self.runtime_hosts.lock().unwrap().pop_front().unwrap(); + let (packet_sink, _packet_receiver) = tokio::sync::mpsc::channel(16); + let mut adapters = adapters_with_process_runtime( + None, + Arc::new(packet_sink), + self.process_runtime.clone(), + ); + adapters.instance_runtime = runtime_host; + CoreInstance::from_toml(config, adapters) + } + } + + let requested_runtime = Arc::new(RecordingRuntimeHost::default()); + let unrequested_runtime = Arc::new(RecordingRuntimeHost::default()); + let instances = Arc::new(InstanceManager::new( + RecordingFactory { + process_runtime: CoreProcessRuntime::new(), + runtime_hosts: StdMutex::new(VecDeque::from([ + requested_runtime.clone(), + unrequested_runtime.clone(), + ])), + }, + Some(tokio::runtime::Handle::current()), + )); + let requested_id = uuid::Uuid::new_v4(); + let unrequested_id = uuid::Uuid::new_v4(); + for instance_id in [requested_id, unrequested_id] { + let config = TomlConfig::default(); + config.set_id(instance_id); + config.set_listeners(Vec::new()); + instances + .create(config, ()) + .unwrap() + .set_state(CoreInstanceState::Running); + } + let rpc = ProcessManagementRpc::::new( + instances, + Arc::new(()), + Arc::new(UnsupportedConfigFileStorage), + ); + + let response = rpc + .collect_network_info( + BaseController::default(), + CollectNetworkInfoRequest { + inst_ids: vec![ + requested_id.into(), + requested_id.into(), + uuid::Uuid::new_v4().into(), + ], + }, + ) + .await + .unwrap(); + let info = response.info.unwrap().map; + assert_eq!(info.len(), 1); + assert!(info.contains_key(&requested_id.to_string())); + assert_eq!( + requested_runtime.collection_count.load(Ordering::Relaxed), + 1 + ); + assert_eq!( + unrequested_runtime.collection_count.load(Ordering::Relaxed), + 0 + ); + + let response = rpc + .collect_network_info( + BaseController::default(), + CollectNetworkInfoRequest::default(), + ) + .await + .unwrap(); + assert_eq!(response.info.unwrap().map.len(), 2); + assert_eq!( + requested_runtime.collection_count.load(Ordering::Relaxed), + 2 + ); + assert_eq!( + unrequested_runtime.collection_count.load(Ordering::Relaxed), + 1 + ); + } + #[cfg(feature = "management")] #[tokio::test] async fn owned_selection_and_cleanup_share_the_canonical_transaction() { diff --git a/easytier-core/src/management/full/process_rpc.rs b/easytier-core/src/management/full/process_rpc.rs index 035ebd5f..b1d37d17 100644 --- a/easytier-core/src/management/full/process_rpc.rs +++ b/easytier-core/src/management/full/process_rpc.rs @@ -28,7 +28,7 @@ use crate::{ use super::{ ConfigFileControl, ConfigFilePermission, InstanceManager, config_source_from_rpc, - config_source_to_rpc, + config_source_to_rpc, network_instance_running_info, }; #[async_trait::async_trait] @@ -633,17 +633,29 @@ where let included = request .inst_ids .into_iter() - .map(|id| uuid::Uuid::from(id).to_string()) + .map(uuid::Uuid::from) .collect::>(); - let map = self - .management - .instances - .collect_network_infos() - .await? - .into_iter() - .map(|(id, info)| (id.to_string(), info)) - .filter(|(id, _)| included.is_empty() || included.contains(id)) - .collect(); + let map = if included.is_empty() { + self.management + .instances + .collect_network_infos() + .await? + .into_iter() + .map(|(id, info)| (id.to_string(), info)) + .collect() + } else { + let mut map = std::collections::BTreeMap::new(); + for instance_id in included { + let Some(instance) = self.management.instances.instance(instance_id) else { + continue; + }; + map.insert( + instance_id.to_string(), + network_instance_running_info(instance.as_ref()).await?, + ); + } + map + }; Ok(CollectNetworkInfoResponse { info: Some(NetworkInstanceRunningInfoMap { map }), }) diff --git a/easytier-core/src/management/full/web_client.rs b/easytier-core/src/management/full/web_client.rs index ef0e2375..d5a57cee 100644 --- a/easytier-core/src/management/full/web_client.rs +++ b/easytier-core/src/management/full/web_client.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::sync::{ Arc, Weak, atomic::{AtomicBool, Ordering}, @@ -7,7 +8,7 @@ use async_trait::async_trait; use easytier_proto::{ rpc_types::controller::BaseController, web::{ - DeviceOsInfo, GetFeatureRequest, GetFeatureResponse, HeartbeatRequest, + DeviceOsInfo, GetFeatureRequest, GetFeatureResponse, HeartbeatRequest, HeartbeatResponse, WebServerServiceClientFactory, }, }; @@ -33,6 +34,64 @@ const RETRY_INTERVAL: std::time::Duration = std::time::Duration::from_secs(1); // Keep retry ownership in this loop when transport or protocol handshakes stall. const CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); const FEATURE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(3); +const DEFAULT_HEARTBEAT_INTERVAL_MS: u32 = 3_500; +const DEFAULT_HEARTBEAT_TIMEOUT_MS: u32 = 15_000; +const MIN_HEARTBEAT_INTERVAL_MS: u32 = 1_000; +const MAX_HEARTBEAT_INTERVAL_MS: u32 = 60_000; +const MIN_HEARTBEAT_TIMEOUT_MS: u32 = 5_000; +const MAX_HEARTBEAT_TIMEOUT_MS: u32 = 120_000; +const MIN_HEARTBEAT_TIMEOUT_MARGIN_MS: u32 = 5_000; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct HeartbeatPolicy { + interval: std::time::Duration, + timeout_ms: i32, +} + +impl Default for HeartbeatPolicy { + fn default() -> Self { + Self { + interval: std::time::Duration::from_millis(DEFAULT_HEARTBEAT_INTERVAL_MS.into()), + timeout_ms: DEFAULT_HEARTBEAT_TIMEOUT_MS as i32, + } + } +} + +impl HeartbeatPolicy { + fn from_response(response: &HeartbeatResponse) -> (Self, bool) { + let requested_interval = response + .heartbeat_interval_ms + .unwrap_or(DEFAULT_HEARTBEAT_INTERVAL_MS); + let requested_timeout = response + .heartbeat_timeout_ms + .unwrap_or(DEFAULT_HEARTBEAT_TIMEOUT_MS); + let interval_ms = + requested_interval.clamp(MIN_HEARTBEAT_INTERVAL_MS, MAX_HEARTBEAT_INTERVAL_MS); + let timeout_ms = requested_timeout + .clamp(MIN_HEARTBEAT_TIMEOUT_MS, MAX_HEARTBEAT_TIMEOUT_MS) + .max(interval_ms.saturating_add(MIN_HEARTBEAT_TIMEOUT_MARGIN_MS)); + ( + Self { + interval: std::time::Duration::from_millis(interval_ms.into()), + timeout_ms: timeout_ms as i32, + }, + interval_ms != requested_interval || timeout_ms != requested_timeout, + ) + } + + fn controller(self) -> BaseController { + BaseController { + timeout_ms: self.timeout_ms, + ..Default::default() + } + } + + fn remaining_interval(self, elapsed: std::time::Duration) -> Option { + self.interval + .checked_sub(elapsed) + .filter(|delay| !delay.is_zero()) + } +} async fn connect_config_server( connector: &dyn TunnelDialer, @@ -100,6 +159,16 @@ pub(crate) trait WebClientBackend: Send + Sync + 'static { fn register(&self, registry: &ServiceRegistry); async fn instance_ids(&self) -> anyhow::Result>; + + fn failed_instance_ids(&self) -> Vec; + + fn instance_state_generation(&self) -> usize { + 0 + } + + async fn wait_for_instance_state_change(&self, _generation: usize) -> usize { + std::future::pending().await + } } struct NativeWebClientBackend @@ -141,11 +210,26 @@ where async fn instance_ids(&self) -> anyhow::Result> { Ok(self.instances.instance_ids()) } + + fn failed_instance_ids(&self) -> Vec { + self.instances.failed_instance_ids() + } + + fn instance_state_generation(&self) -> usize { + self.instances.instance_state_generation() + } + + async fn wait_for_instance_state_change(&self, generation: usize) -> usize { + self.instances + .wait_for_instance_state_change(generation) + .await + } } struct WebClientController { config: WebClientConfig, backend: Arc, + runtime_id: uuid::Uuid, } /// Portable config-server client. Hosts only supply identity and adapters. @@ -201,7 +285,11 @@ impl WebClient { backend: Arc, manager_guard: Option, ) -> Self { - let controller = Arc::new(WebClientController { config, backend }); + let controller = Arc::new(WebClientController { + config, + backend, + runtime_id: uuid::Uuid::new_v4(), + }); let connected = Arc::new(AtomicBool::new(false)); let tasks = AbortOnDropHandle::new(tokio::spawn(web_client_routine( controller.clone(), @@ -312,6 +400,59 @@ struct WebClientSession { tasks: Mutex>, } +fn running_instances_for_heartbeat( + instance_ids: Vec, + failed_instance_ids: &[uuid::Uuid], +) -> Vec { + let failed_instance_ids: HashSet<_> = failed_instance_ids.iter().copied().collect(); + instance_ids + .into_iter() + .filter(|instance_id| !failed_instance_ids.contains(instance_id)) + .collect() +} + +fn build_heartbeat_request( + config: &WebClientConfig, + runtime_id: uuid::Uuid, + running_network_instances: Vec, + failed_network_instances: Vec, +) -> HeartbeatRequest { + HeartbeatRequest { + machine_id: Some(config.machine_id.into()), + inst_id: Some(runtime_id.into()), + user_token: config.token.clone(), + easytier_version: config.easytier_version.clone(), + hostname: config.hostname.clone(), + report_time: chrono::Local::now().to_rfc3339(), + device_os: Some(config.device_os.clone()), + support_config_source: true, + running_network_instances: running_network_instances + .into_iter() + .map(Into::into) + .collect(), + failed_network_instances: failed_network_instances + .into_iter() + .map(Into::into) + .collect(), + support_heartbeat_policy: true, + } +} + +async fn wait_for_next_heartbeat( + backend: &dyn WebClientBackend, + observed_generation: usize, + policy: HeartbeatPolicy, + elapsed: std::time::Duration, +) { + let Some(delay) = policy.remaining_interval(elapsed) else { + return; + }; + tokio::select! { + _ = time::sleep(delay) => {} + _ = backend.wait_for_instance_state_change(observed_generation) => {} + } +} + impl WebClientSession { fn new(tunnel: Box, controller: Arc) -> Self { let rpc = BidirectRpcManager::new(); @@ -339,46 +480,60 @@ impl WebClientSession { tasks: &mut JoinSet<()>, ) { let controller = controller.upgrade().expect("web client controller"); - let machine_id = controller.config.machine_id; - let session_id = uuid::Uuid::new_v4(); - let token = controller.config.token.clone(); - let hostname = controller.config.hostname.clone(); - let device_os = controller.config.device_os.clone(); - let easytier_version = controller.config.easytier_version.clone(); let controller = Arc::downgrade(&controller); let client = rpc .rpc_client() .scoped_client::>(1, 1, String::new()); - let mut tick = time::interval(std::time::Duration::from_secs(1)); tasks.spawn(async move { + let mut heartbeat_policy = HeartbeatPolicy::default(); loop { - tick.tick().await; + let heartbeat_started_at = std::time::Instant::now(); let Some(controller) = controller.upgrade() else { break; }; + let observed_generation = controller.backend.instance_state_generation(); + let failed_network_instances = controller.backend.failed_instance_ids(); let running_network_instances = match controller.backend.instance_ids().await { - Ok(instance_ids) => instance_ids.into_iter().map(Into::into).collect(), + Ok(instance_ids) => { + running_instances_for_heartbeat(instance_ids, &failed_network_instances) + } Err(error) => { tracing::error!(%error, "failed to list config-server instances"); break; } }; - let request = HeartbeatRequest { - machine_id: Some(machine_id.into()), - inst_id: Some(session_id.into()), - user_token: token.clone(), - easytier_version: easytier_version.clone(), - hostname: hostname.clone(), - report_time: chrono::Local::now().to_rfc3339(), - device_os: Some(device_os.clone()), - support_config_source: true, + let request = build_heartbeat_request( + &controller.config, + controller.runtime_id, running_network_instances, - }; + failed_network_instances, + ); - match client.heartbeat(BaseController::default(), request).await { + match client + .heartbeat(heartbeat_policy.controller(), request) + .await + { Ok(response) => { tracing::debug!(?response, "config-server heartbeat response"); + let (next_policy, adjusted) = HeartbeatPolicy::from_response(&response); + if adjusted { + tracing::warn!( + requested_interval_ms = ?response.heartbeat_interval_ms, + requested_timeout_ms = ?response.heartbeat_timeout_ms, + applied_interval_ms = next_policy.interval.as_millis(), + applied_timeout_ms = next_policy.timeout_ms, + "config-server heartbeat policy was outside safe bounds" + ); + } + heartbeat_policy = next_policy; + wait_for_next_heartbeat( + controller.backend.as_ref(), + observed_generation, + heartbeat_policy, + heartbeat_started_at.elapsed(), + ) + .await; } Err(error) => { tracing::error!(?error, "config-server heartbeat failed"); @@ -430,6 +585,25 @@ mod tests { attempts: AtomicUsize, } + struct ImmediateStateChangeBackend; + + #[async_trait] + impl WebClientBackend for ImmediateStateChangeBackend { + fn register(&self, _registry: &ServiceRegistry) {} + + async fn instance_ids(&self) -> anyhow::Result> { + Ok(Vec::new()) + } + + fn failed_instance_ids(&self) -> Vec { + Vec::new() + } + + async fn wait_for_instance_state_change(&self, generation: usize) -> usize { + generation.wrapping_add(1) + } + } + #[async_trait] impl TunnelDialer for StalledThenReadyDialer { async fn connect(&self) -> anyhow::Result> { @@ -446,6 +620,20 @@ mod tests { } } + #[test] + fn heartbeat_hides_failed_instances_from_the_running_list() { + let running = uuid::Uuid::new_v4(); + let failed = uuid::Uuid::new_v4(); + let stopped_clean = uuid::Uuid::new_v4(); + let instance_ids = vec![running, failed, stopped_clean]; + let failed_instance_ids = vec![failed]; + + let reported = running_instances_for_heartbeat(instance_ids.clone(), &failed_instance_ids); + + assert_eq!(reported, vec![running, stopped_clean]); + assert!(running_instances_for_heartbeat(instance_ids, &[]).len() == 3); + } + #[tokio::test] async fn stalled_connection_attempt_times_out_and_allows_redial() { let connector = StalledThenReadyDialer { @@ -463,6 +651,26 @@ mod tests { assert_eq!(connector.attempts.load(Ordering::Relaxed), 2); } + #[tokio::test] + async fn instance_state_change_interrupts_a_long_heartbeat_interval() { + let policy = HeartbeatPolicy { + interval: std::time::Duration::from_secs(60), + timeout_ms: 65_000, + }; + + tokio::time::timeout( + std::time::Duration::from_secs(1), + wait_for_next_heartbeat( + &ImmediateStateChangeBackend, + 0, + policy, + std::time::Duration::ZERO, + ), + ) + .await + .expect("instance state change must wake heartbeat before its interval"); + } + #[test] fn endpoint_normalizes_non_websocket_paths() { let endpoint = @@ -502,4 +710,87 @@ mod tests { fn endpoint_rejects_an_empty_token() { assert!(ConfigServerEndpoint::parse("udp://example.com", |_| true).is_err()); } + + #[test] + fn heartbeat_request_carries_registered_and_failed_instance_ids() { + let runtime_id = uuid::Uuid::new_v4(); + let registered = uuid::Uuid::new_v4(); + let failed = uuid::Uuid::new_v4(); + let request = build_heartbeat_request( + &WebClientConfig { + token: "token".to_owned(), + machine_id: uuid::Uuid::new_v4(), + hostname: "host".to_owned(), + device_os: DeviceOsInfo::default(), + easytier_version: "test-version".to_owned(), + secure_mode: false, + }, + runtime_id, + vec![registered], + vec![failed], + ); + + assert_eq!(request.inst_id.map(uuid::Uuid::from), Some(runtime_id)); + assert_eq!( + request + .running_network_instances + .into_iter() + .map(uuid::Uuid::from) + .collect::>(), + vec![registered] + ); + assert_eq!( + request + .failed_network_instances + .into_iter() + .map(uuid::Uuid::from) + .collect::>(), + vec![failed] + ); + assert!(request.support_heartbeat_policy); + } + + #[test] + fn heartbeat_policy_uses_safe_defaults_for_legacy_servers() { + let (policy, adjusted) = HeartbeatPolicy::from_response(&HeartbeatResponse::default()); + + assert!(!adjusted); + assert_eq!( + policy.interval, + std::time::Duration::from_millis(DEFAULT_HEARTBEAT_INTERVAL_MS.into()) + ); + assert_eq!(policy.timeout_ms, DEFAULT_HEARTBEAT_TIMEOUT_MS as i32); + } + + #[test] + fn heartbeat_policy_clamps_server_values_and_preserves_timeout_margin() { + let (minimum, adjusted) = HeartbeatPolicy::from_response(&HeartbeatResponse { + heartbeat_interval_ms: Some(1), + heartbeat_timeout_ms: Some(1), + }); + assert!(adjusted); + assert_eq!( + minimum.interval, + std::time::Duration::from_millis(MIN_HEARTBEAT_INTERVAL_MS.into()) + ); + assert_eq!(minimum.timeout_ms, 6_000); + + let (maximum, adjusted) = HeartbeatPolicy::from_response(&HeartbeatResponse { + heartbeat_interval_ms: Some(u32::MAX), + heartbeat_timeout_ms: Some(u32::MAX), + }); + assert!(adjusted); + assert_eq!( + maximum.interval, + std::time::Duration::from_millis(MAX_HEARTBEAT_INTERVAL_MS.into()) + ); + assert_eq!(maximum.timeout_ms, MAX_HEARTBEAT_TIMEOUT_MS as i32); + + let (margin, adjusted) = HeartbeatPolicy::from_response(&HeartbeatResponse { + heartbeat_interval_ms: Some(60_000), + heartbeat_timeout_ms: Some(5_000), + }); + assert!(adjusted); + assert_eq!(margin.timeout_ms, 65_000); + } } diff --git a/easytier-core/src/wasi/web_client.rs b/easytier-core/src/wasi/web_client.rs index a4a5900e..1b814938 100644 --- a/easytier-core/src/wasi/web_client.rs +++ b/easytier-core/src/wasi/web_client.rs @@ -265,6 +265,10 @@ impl WebClientBackend for WasiWebClientBackend { .await?; Ok(response.inst_ids.into_iter().map(Into::into).collect()) } + + fn failed_instance_ids(&self) -> Vec { + Vec::new() + } } pub(super) struct WasiWebClientRuntime { diff --git a/easytier-go/proto/web/web.pb.go b/easytier-go/proto/web/web.pb.go index 365c3ea1..ece7eb55 100644 --- a/easytier-go/proto/web/web.pb.go +++ b/easytier-go/proto/web/web.pb.go @@ -93,6 +93,8 @@ type HeartbeatRequest struct { RunningNetworkInstances []*common.UUID `protobuf:"bytes,7,rep,name=running_network_instances,json=runningNetworkInstances,proto3" json:"running_network_instances,omitempty"` DeviceOs *DeviceOsInfo `protobuf:"bytes,8,opt,name=device_os,json=deviceOs,proto3" json:"device_os,omitempty"` SupportConfigSource bool `protobuf:"varint,9,opt,name=support_config_source,json=supportConfigSource,proto3" json:"support_config_source,omitempty"` + FailedNetworkInstances []*common.UUID `protobuf:"bytes,10,rep,name=failed_network_instances,json=failedNetworkInstances,proto3" json:"failed_network_instances,omitempty"` + SupportHeartbeatPolicy bool `protobuf:"varint,11,opt,name=support_heartbeat_policy,json=supportHeartbeatPolicy,proto3" json:"support_heartbeat_policy,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -190,10 +192,26 @@ func (x *HeartbeatRequest) GetSupportConfigSource() bool { return false } +func (x *HeartbeatRequest) GetFailedNetworkInstances() []*common.UUID { + if x != nil { + return x.FailedNetworkInstances + } + return nil +} + +func (x *HeartbeatRequest) GetSupportHeartbeatPolicy() bool { + if x != nil { + return x.SupportHeartbeatPolicy + } + return false +} + type HeartbeatResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + state protoimpl.MessageState `protogen:"open.v1"` + HeartbeatIntervalMs *uint32 `protobuf:"varint,1,opt,name=heartbeat_interval_ms,json=heartbeatIntervalMs,proto3,oneof" json:"heartbeat_interval_ms,omitempty"` + HeartbeatTimeoutMs *uint32 `protobuf:"varint,2,opt,name=heartbeat_timeout_ms,json=heartbeatTimeoutMs,proto3,oneof" json:"heartbeat_timeout_ms,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *HeartbeatResponse) Reset() { @@ -226,6 +244,20 @@ func (*HeartbeatResponse) Descriptor() ([]byte, []int) { return file_web_proto_rawDescGZIP(), []int{2} } +func (x *HeartbeatResponse) GetHeartbeatIntervalMs() uint32 { + if x != nil && x.HeartbeatIntervalMs != nil { + return *x.HeartbeatIntervalMs + } + return 0 +} + +func (x *HeartbeatResponse) GetHeartbeatTimeoutMs() uint32 { + if x != nil && x.HeartbeatTimeoutMs != nil { + return *x.HeartbeatTimeoutMs + } + return 0 +} + type GetFeatureRequest struct { state protoimpl.MessageState `protogen:"open.v1"` unknownFields protoimpl.UnknownFields @@ -314,7 +346,7 @@ const file_web_proto_rawDesc = "" + "\fDeviceOsInfo\x12\x17\n" + "\aos_type\x18\x01 \x01(\tR\x06osType\x12\x18\n" + "\aversion\x18\x02 \x01(\tR\aversion\x12\"\n" + - "\fdistribution\x18\x03 \x01(\tR\fdistribution\"\x9b\x03\n" + + "\fdistribution\x18\x03 \x01(\tR\fdistribution\"\x9d\x04\n" + "\x10HeartbeatRequest\x12+\n" + "\n" + "machine_id\x18\x01 \x01(\v2\f.common.UUIDR\tmachineId\x12%\n" + @@ -327,8 +359,15 @@ const file_web_proto_rawDesc = "" + "\bhostname\x18\x06 \x01(\tR\bhostname\x12H\n" + "\x19running_network_instances\x18\a \x03(\v2\f.common.UUIDR\x17runningNetworkInstances\x12.\n" + "\tdevice_os\x18\b \x01(\v2\x11.web.DeviceOsInfoR\bdeviceOs\x122\n" + - "\x15support_config_source\x18\t \x01(\bR\x13supportConfigSource\"\x13\n" + - "\x11HeartbeatResponse\"\x13\n" + + "\x15support_config_source\x18\t \x01(\bR\x13supportConfigSource\x12F\n" + + "\x18failed_network_instances\x18\n" + + " \x03(\v2\f.common.UUIDR\x16failedNetworkInstances\x128\n" + + "\x18support_heartbeat_policy\x18\v \x01(\bR\x16supportHeartbeatPolicy\"\xb6\x01\n" + + "\x11HeartbeatResponse\x127\n" + + "\x15heartbeat_interval_ms\x18\x01 \x01(\rH\x00R\x13heartbeatIntervalMs\x88\x01\x01\x125\n" + + "\x14heartbeat_timeout_ms\x18\x02 \x01(\rH\x01R\x12heartbeatTimeoutMs\x88\x01\x01B\x18\n" + + "\x16_heartbeat_interval_msB\x17\n" + + "\x15_heartbeat_timeout_ms\"\x13\n" + "\x11GetFeatureRequest\"C\n" + "\x12GetFeatureResponse\x12-\n" + "\x12support_encryption\x18\x01 \x01(\bR\x11supportEncryption2\x8d\x01\n" + @@ -363,15 +402,16 @@ var file_web_proto_depIdxs = []int32{ 5, // 1: web.HeartbeatRequest.inst_id:type_name -> common.UUID 5, // 2: web.HeartbeatRequest.running_network_instances:type_name -> common.UUID 0, // 3: web.HeartbeatRequest.device_os:type_name -> web.DeviceOsInfo - 1, // 4: web.WebServerService.Heartbeat:input_type -> web.HeartbeatRequest - 3, // 5: web.WebServerService.GetFeature:input_type -> web.GetFeatureRequest - 2, // 6: web.WebServerService.Heartbeat:output_type -> web.HeartbeatResponse - 4, // 7: web.WebServerService.GetFeature:output_type -> web.GetFeatureResponse - 6, // [6:8] is the sub-list for method output_type - 4, // [4:6] is the sub-list for method input_type - 4, // [4:4] is the sub-list for extension type_name - 4, // [4:4] is the sub-list for extension extendee - 0, // [0:4] is the sub-list for field type_name + 5, // 4: web.HeartbeatRequest.failed_network_instances:type_name -> common.UUID + 1, // 5: web.WebServerService.Heartbeat:input_type -> web.HeartbeatRequest + 3, // 6: web.WebServerService.GetFeature:input_type -> web.GetFeatureRequest + 2, // 7: web.WebServerService.Heartbeat:output_type -> web.HeartbeatResponse + 4, // 8: web.WebServerService.GetFeature:output_type -> web.GetFeatureResponse + 7, // [7:9] is the sub-list for method output_type + 5, // [5:7] is the sub-list for method input_type + 5, // [5:5] is the sub-list for extension type_name + 5, // [5:5] is the sub-list for extension extendee + 0, // [0:5] is the sub-list for field type_name } func init() { file_web_proto_init() } @@ -379,6 +419,7 @@ func file_web_proto_init() { if File_web_proto != nil { return } + file_web_proto_msgTypes[2].OneofWrappers = []any{} type x struct{} out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ diff --git a/easytier-proto/proto/web.proto b/easytier-proto/proto/web.proto index 4468e10f..1ce60766 100644 --- a/easytier-proto/proto/web.proto +++ b/easytier-proto/proto/web.proto @@ -22,9 +22,14 @@ message HeartbeatRequest { repeated common.UUID running_network_instances = 7; DeviceOsInfo device_os = 8; bool support_config_source = 9; + repeated common.UUID failed_network_instances = 10; + bool support_heartbeat_policy = 11; } -message HeartbeatResponse {} +message HeartbeatResponse { + optional uint32 heartbeat_interval_ms = 1; + optional uint32 heartbeat_timeout_ms = 2; +} message GetFeatureRequest {} diff --git a/easytier-web/locales/app.yml b/easytier-web/locales/app.yml index 8ebf5455..3043a448 100644 --- a/easytier-web/locales/app.yml +++ b/easytier-web/locales/app.yml @@ -41,8 +41,11 @@ cli: en: "The path to the GeoIP2 database file, used to lookup the location of the client, default is the embedded file (only country information) , recommend https://github.com/P3TERX/GeoLite.mmdb" zh-CN: "GeoIP2 数据库文件路径，用于查找客户端的位置，默认为嵌入文件（仅国家信息），推荐 https://github.com/P3TERX/GeoLite.mmdb" heartbeat_min_response_ms: - en: "Minimum response time for config-server heartbeat RPCs in milliseconds, default is 0" - zh-CN: "配置服务心跳 RPC 的最短响应时间，单位毫秒，默认为 0" + en: "Config-server heartbeat interval in milliseconds, default is 3500" + zh-CN: "配置服务心跳周期，单位毫秒，默认为 3500" + heartbeat_timeout_ms: + en: "Config-server heartbeat RPC timeout in milliseconds, default is 15000" + zh-CN: "配置服务心跳 RPC 超时时间，单位毫秒，默认为 15000" disable_registration: en: "Disable user registration" zh-CN: "禁用用户注册" diff --git a/easytier-web/src/client_manager/mod.rs b/easytier-web/src/client_manager/mod.rs index fccd1f40..43e4a1c5 100644 --- a/easytier-web/src/client_manager/mod.rs +++ b/easytier-web/src/client_manager/mod.rs @@ -3,15 +3,20 @@ mod runtime_reconcile; pub mod session; pub mod storage; -use std::sync::{ - Arc, - atomic::{AtomicU32, AtomicU64, Ordering}, -}; use std::time::Duration; +use std::{ + collections::HashSet, + sync::{ + Arc, + atomic::{AtomicU32, AtomicU64, Ordering}, + }, +}; use dashmap::DashMap; use easytier::proto::{ - api::manage::WebClientService, rpc_types::controller::BaseController, web::HeartbeatRequest, + api::manage::WebClientService, + rpc_types::controller::BaseController, + web::{HeartbeatRequest, HeartbeatResponse}, }; use easytier_core::{ management::remote_client::{self, RemoteClientManager}, @@ -19,7 +24,7 @@ use easytier_core::{ tunnel::{Tunnel, web_security}, }; use maxminddb::geoip2; -use session::{Location, ManagedConfigRevisionDelta, Session}; +use session::{Location, ManagedConfigPersistedChange, Session}; use storage::{Storage, StorageToken}; use crate::FeatureFlags; @@ -30,6 +35,67 @@ use crate::db::{Db, UserIdInDb, entity::user_running_network_configs}; pub(crate) use managed_config::ManagedConfigError; +const DEFAULT_HEARTBEAT_INTERVAL: Duration = Duration::from_millis(3_500); +const DEFAULT_HEARTBEAT_TIMEOUT: Duration = Duration::from_secs(15); +const MIN_HEARTBEAT_INTERVAL: Duration = Duration::from_secs(1); +const MAX_HEARTBEAT_INTERVAL: Duration = Duration::from_secs(60); +const MIN_HEARTBEAT_TIMEOUT: Duration = Duration::from_secs(5); +const MAX_HEARTBEAT_TIMEOUT: Duration = Duration::from_secs(120); +const HEARTBEAT_TIMEOUT_MARGIN: Duration = Duration::from_secs(5); + +#[derive(Debug, Clone, Copy)] +pub(crate) struct HeartbeatPolicy { + interval: Duration, + timeout: Duration, +} + +impl Default for HeartbeatPolicy { + fn default() -> Self { + Self { + interval: DEFAULT_HEARTBEAT_INTERVAL, + timeout: DEFAULT_HEARTBEAT_TIMEOUT, + } + } +} + +impl HeartbeatPolicy { + pub(crate) fn from_millis(interval_ms: u64, timeout_ms: u64) -> anyhow::Result { + let interval = if interval_ms == 0 { + DEFAULT_HEARTBEAT_INTERVAL + } else { + Duration::from_millis(interval_ms) + }; + let timeout = Duration::from_millis(timeout_ms); + if !(MIN_HEARTBEAT_INTERVAL..=MAX_HEARTBEAT_INTERVAL).contains(&interval) { + anyhow::bail!("heartbeat interval must be between 1000 and 60000 milliseconds"); + } + if !(MIN_HEARTBEAT_TIMEOUT..=MAX_HEARTBEAT_TIMEOUT).contains(&timeout) { + anyhow::bail!("heartbeat timeout must be between 5000 and 120000 milliseconds"); + } + if timeout < interval.saturating_add(HEARTBEAT_TIMEOUT_MARGIN) { + anyhow::bail!( + "heartbeat timeout must exceed the interval by at least 5000 milliseconds" + ); + } + Ok(Self { interval, timeout }) + } + + fn response(self) -> HeartbeatResponse { + HeartbeatResponse { + heartbeat_interval_ms: Some(self.interval.as_millis() as u32), + heartbeat_timeout_ms: Some(self.timeout.as_millis() as u32), + } + } + + fn session_rx_timeout(self) -> Duration { + Duration::from_secs(30).max(self.timeout.saturating_add(HEARTBEAT_TIMEOUT_MARGIN)) + } + + fn legacy_response_delay(self) -> Duration { + self.interval.min(DEFAULT_HEARTBEAT_INTERVAL) + } +} + #[derive(rust_embed::Embed)] #[folder = "resources/"] #[include = "geoip2-cn.mmdb"] @@ -69,14 +135,14 @@ pub struct ClientManager { webhook_config: SharedWebhookConfig, geoip_db: Arc>>>, - heartbeat_min_response_delay: Duration, + heartbeat_policy: HeartbeatPolicy, } impl ClientManager { pub fn new( db: Db, geoip_db: Option, - heartbeat_min_response_delay: Duration, + heartbeat_policy: HeartbeatPolicy, feature_flags: Arc, webhook_config: SharedWebhookConfig, ) -> Self { @@ -101,7 +167,7 @@ impl ClientManager { webhook_config, geoip_db: Arc::new(load_geoip_db(geoip_db)), - heartbeat_min_response_delay, + heartbeat_policy, } } @@ -117,7 +183,7 @@ impl ClientManager { let listeners_cnt = self.listeners_cnt.clone(); let next_session_epoch = self.next_session_epoch.clone(); let geoip_db = self.geoip_db.clone(); - let heartbeat_min_response_delay = self.heartbeat_min_response_delay; + let heartbeat_policy = self.heartbeat_policy; let feature_flags = self.feature_flags.clone(); let webhook_config = self.webhook_config.clone(); self.tasks.spawn(async move { @@ -146,7 +212,7 @@ impl ClientManager { storage.clone(), client_url.clone(), location, - heartbeat_min_response_delay, + heartbeat_policy, feature_flags.clone(), webhook_config.clone(), next_session_epoch.fetch_add(1, Ordering::Relaxed) + 1, @@ -188,7 +254,7 @@ impl ClientManager { .get_client_url_by_machine_id(user_id, machine_id)?; self.client_sessions .get(&c_url) - .map(|item| item.value().clone()) + .and_then(|item| item.is_running().then(|| item.value().clone())) } pub async fn disconnect_session_by_machine_id( @@ -238,11 +304,14 @@ impl ClientManager { if matches!( status, managed_config::ManagedConfigApplyStatus::Applied { .. } - ) && let Some(config_revision) = config_revision - && let Some(session) = self.get_session_by_machine_id(user_id, &machine_id) + ) && self.storage.record_full_managed_config_change( + user_id, + machine_id, + config_revision.as_deref(), + ) && let Some(session) = self.get_session_by_machine_id(user_id, &machine_id) { session - .notify_full_config_revision_changed(user_id, machine_id, config_revision) + .notify_managed_runtime_state_changed(user_id, machine_id) .await; } Ok(()) @@ -259,7 +328,7 @@ impl ClientManager { ) -> anyhow::Result<()> { let config_revision = config_revision.trim().to_string(); let expected_config_revision = expected_config_revision.trim().to_string(); - let upsert_instance_ids = upserts + let mut dirty_instance_ids: HashSet<_> = upserts .iter() .map(|config| config.instance_id.clone()) .collect(); @@ -276,23 +345,26 @@ impl ClientManager { if let managed_config::ManagedConfigApplyStatus::Applied { deleted_web_instance_ids, } = status - && let Some(session) = self.get_session_by_machine_id(user_id, &machine_id) { - session - .notify_patch_config_revision_changed( - user_id, - machine_id, - ManagedConfigRevisionDelta { - expected_revision: expected_config_revision, - target_revision: config_revision, - upsert_instance_ids, - delete_instance_ids: deleted_web_instance_ids - .into_iter() - .map(|instance_id| instance_id.to_string()) - .collect(), - }, - ) - .await; + dirty_instance_ids.extend( + deleted_web_instance_ids + .into_iter() + .map(|instance_id| instance_id.to_string()), + ); + let changed = self.storage.record_patch_managed_config_change( + user_id, + machine_id, + ManagedConfigPersistedChange { + expected_revision: expected_config_revision, + target_revision: config_revision, + dirty_instance_ids, + }, + ); + if changed && let Some(session) = self.get_session_by_machine_id(user_id, &machine_id) { + session + .notify_managed_runtime_state_changed(user_id, machine_id) + .await; + } } Ok(()) } @@ -302,9 +374,13 @@ impl ClientManager { user_id: UserIdInDb, machine_id: uuid::Uuid, ) { - if let Some(session) = self.get_session_by_machine_id(user_id, &machine_id) { + if self + .storage + .invalidate_managed_runtime_state(user_id, machine_id) + && let Some(session) = self.get_session_by_machine_id(user_id, &machine_id) + { session - .invalidate_applied_config_revision(user_id, machine_id) + .notify_managed_runtime_state_changed(user_id, machine_id) .await; } } @@ -474,11 +550,36 @@ mod tests { use sqlx::Executor; use crate::{ - FeatureFlags, client_manager::ClientManager, db::Db, webhook::ManagedNetworkConfig, + FeatureFlags, + client_manager::{ClientManager, HeartbeatPolicy, session::Session, storage::StorageToken}, + db::Db, + webhook::ManagedNetworkConfig, }; const MANAGED_CONFIG_TOKEN: &str = "managed-config-token"; + #[test] + fn heartbeat_policy_validates_server_configuration() { + let policy = HeartbeatPolicy::from_millis(3_500, 15_000).unwrap(); + let response = policy.response(); + assert_eq!(response.heartbeat_interval_ms, Some(3_500)); + assert_eq!(response.heartbeat_timeout_ms, Some(15_000)); + assert_eq!(policy.session_rx_timeout(), Duration::from_secs(30)); + + let legacy_default = HeartbeatPolicy::from_millis(0, 15_000).unwrap(); + assert_eq!(legacy_default.response().heartbeat_interval_ms, Some(3_500)); + + let slow = HeartbeatPolicy::from_millis(60_000, 65_000).unwrap(); + assert_eq!(slow.session_rx_timeout(), Duration::from_secs(70)); + assert_eq!(slow.legacy_response_delay(), Duration::from_millis(3_500)); + + assert!(HeartbeatPolicy::from_millis(999, 15_000).is_err()); + assert!(HeartbeatPolicy::from_millis(60_001, 120_000).is_err()); + assert!(HeartbeatPolicy::from_millis(3_500, 4_999).is_err()); + assert!(HeartbeatPolicy::from_millis(60_000, 64_999).is_err()); + assert!(HeartbeatPolicy::from_millis(3_500, 120_001).is_err()); + } + async fn wait_for_condition(mut condition: F, timeout: Duration) where F: FnMut() -> Fut, @@ -655,7 +756,7 @@ mod tests { let mut mgr = ClientManager::new( Db::memory_db().await, None, - Duration::ZERO, + HeartbeatPolicy::from_millis(0, 15_000).unwrap(), Arc::new(FeatureFlags::default()), webhook_config, ); @@ -683,6 +784,51 @@ mod tests { webhook_server.abort(); } + #[tokio::test] + async fn non_running_session_is_not_routable_by_machine_id() { + let db = Db::memory_db().await; + let mgr = ClientManager::new( + db.clone(), + None, + HeartbeatPolicy::default(), + Arc::new(FeatureFlags::default()), + Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ); + let user_id = db.auto_create_user("token").await.unwrap().id; + let machine_id = uuid::Uuid::new_v4(); + let client_url = url::Url::parse("udp://127.0.0.1:22020").unwrap(); + mgr.storage.update_client( + StorageToken { + token: "token".to_string(), + client_url: client_url.clone(), + machine_id, + user_id, + }, + 1, + true, + ); + let session = Arc::new(Session::new( + mgr.storage.weak_ref(), + client_url.clone(), + None, + HeartbeatPolicy::default(), + Arc::new(FeatureFlags::default()), + Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + 1, + )); + assert!(!session.is_running()); + mgr.client_sessions.insert(client_url, session); + + assert!( + mgr.get_session_by_machine_id(user_id, &machine_id) + .is_none() + ); + } + async fn wait_for_validated_user(mgr: &ClientManager, machine_id: uuid::Uuid) -> i32 { tokio::time::timeout(Duration::from_secs(12), async { loop { @@ -1008,7 +1154,7 @@ mod tests { let mut mgr = ClientManager::new( Db::memory_db().await, None, - Duration::ZERO, + HeartbeatPolicy::from_millis(0, 15_000).unwrap(), Arc::new(FeatureFlags::default()), Arc::new(crate::webhook::WebhookConfig::new( None, None, None, None, None, @@ -1075,7 +1221,7 @@ mod tests { let mut mgr = ClientManager::new( Db::memory_db().await, None, - Duration::ZERO, + HeartbeatPolicy::from_millis(0, 15_000).unwrap(), Arc::new(FeatureFlags::default()), webhook_config, ); @@ -1141,7 +1287,7 @@ mod tests { let mut mgr = ClientManager::new( Db::memory_db().await, None, - Duration::ZERO, + HeartbeatPolicy::from_millis(0, 15_000).unwrap(), Arc::new(FeatureFlags::default()), webhook_config, ); @@ -1333,7 +1479,7 @@ mod tests { let mut mgr = ClientManager::new( Db::memory_db().await, None, - Duration::ZERO, + HeartbeatPolicy::from_millis(0, 15_000).unwrap(), Arc::new(FeatureFlags::default()), webhook_config, ); diff --git a/easytier-web/src/client_manager/runtime_reconcile.rs b/easytier-web/src/client_manager/runtime_reconcile.rs index 71273ad5..c2e8d53d 100644 --- a/easytier-web/src/client_manager/runtime_reconcile.rs +++ b/easytier-web/src/client_manager/runtime_reconcile.rs @@ -28,7 +28,7 @@ use easytier::{ use super::session::{SessionConfigClient, SessionRpcClient}; pub(super) enum RuntimeReconcileAction { - None, + Unchanged(Box), Run { config: Box, overwrite: bool, @@ -269,27 +269,47 @@ fn normalized_managed_credentials( Ok(NetworkConfig::new_from_config(config.gen_config()?)?.managed_credentials) } +fn is_automatic_windows_dev_name(dev_name: &str) -> bool { + let Some((interface_count, suffix)) = dev_name + .strip_prefix("et_") + .and_then(|value| value.split_once('_')) + else { + return false; + }; + !interface_count.is_empty() + && interface_count.bytes().all(|byte| byte.is_ascii_digit()) + && suffix.len() == 4 + && suffix + .bytes() + .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit()) +} + fn web_source_runtime_patch( current: &NetworkConfig, desired: &NetworkConfig, ) -> anyhow::Result> { - if let Some(desired_hostname) = desired - .hostname - .as_deref() - .filter(|hostname| !hostname.is_empty()) - && current.hostname.as_deref() != Some(desired_hostname) - { - return Ok(None); - } let mut current_base = hot_patch_base(current)?; let mut desired_base = hot_patch_base(desired)?; - current_base.hostname = None; - desired_base.hostname = None; + if desired.dev_name.is_none() + || (desired.dev_name.as_deref() == Some("") + && current + .dev_name + .as_deref() + .is_some_and(is_automatic_windows_dev_name)) + { + current_base.dev_name = None; + desired_base.dev_name = None; + } + let current_hostname = current_base.hostname.take().unwrap_or_default(); + let desired_hostname = desired_base.hostname.take().unwrap_or_default(); if current_base != desired_base { return Ok(None); } let mut patch = InstanceConfigPatch::default(); + if desired.hostname.is_some() && current_hostname != desired_hostname { + patch.hostname = Some(desired_hostname); + } let current_acl = normalized_acl(¤t.acl); let desired_acl = normalized_acl(&desired.acl); if current_acl != desired_acl { @@ -358,14 +378,41 @@ fn web_source_runtime_patch( Ok(Some(patch)) } -fn ensure_runtime_config_converged( +// Release 2.6.4 omits a configured hostname that matches the device +// hostname from config readback. After a successful hostname mutation the +// desired value must be restored into the observed config, otherwise every +// later plan re-sends the same hostname patch. +pub(super) fn restore_omitted_hostname( + current: &mut NetworkConfig, + desired: &NetworkConfig, + hostname_applied: bool, +) { + if hostname_applied && current.hostname.is_none() && desired.hostname.is_some() { + current.hostname = desired.hostname.clone(); + } +} + +pub(super) fn ensure_runtime_config_converged( current: &NetworkConfig, desired: &NetworkConfig, + hostname_applied: bool, ) -> anyhow::Result<()> { let patch = web_source_runtime_patch(current, desired)?; match patch { - Some(patch) if patch == InstanceConfigPatch::default() => Ok(()), - Some(_) => anyhow::bail!("runtime config still needs patch after reconcile"), + Some(mut patch) => { + // Release 2.6.4 omits a configured hostname when it equals + // the device hostname. The successful mutation is therefore + // authoritative for hostname, while every other field remains + // verified from the runtime readback. + if hostname_applied && current.hostname.is_none() { + patch.hostname = None; + } + if patch == InstanceConfigPatch::default() { + Ok(()) + } else { + anyhow::bail!("runtime config still needs patch after reconcile") + } + } None => anyhow::bail!("runtime config still needs full overwrite after reconcile"), } } @@ -429,13 +476,19 @@ pub(super) fn prepare_web_source_runtime_reconcile_from_current( desired_config: NetworkConfig, ) -> anyhow::Result { let Some(patch) = web_source_runtime_patch(current_config, &desired_config)? else { + let mut run_config = desired_config; + if run_config.hostname.is_none() { + run_config.hostname = current_config.hostname.clone(); + } return Ok(RuntimeReconcileAction::Run { - config: Box::new(desired_config), + config: Box::new(run_config), overwrite: true, }); }; if patch == InstanceConfigPatch::default() { - return Ok(RuntimeReconcileAction::None); + return Ok(RuntimeReconcileAction::Unchanged(Box::new( + current_config.clone(), + ))); } Ok(RuntimeReconcileAction::Patch(Box::new(patch))) @@ -449,12 +502,17 @@ pub(super) async fn apply_web_source_runtime_reconcile( action: RuntimeReconcileAction, ) -> anyhow::Result { match action { - RuntimeReconcileAction::None => Ok(desired_config), + RuntimeReconcileAction::Unchanged(current_config) => Ok(*current_config), RuntimeReconcileAction::Run { config, overwrite } => { + let hostname_applied = config.hostname.is_some(); run_web_source_instance(rpc_client, inst_id, *config, overwrite).await?; - Ok(desired_config) + let mut current_config = get_runtime_config(rpc_client, inst_id).await?; + ensure_runtime_config_converged(¤t_config, &desired_config, hostname_applied)?; + restore_omitted_hostname(&mut current_config, &desired_config, hostname_applied); + Ok(current_config) } RuntimeReconcileAction::Patch(patch) => { + let hostname_applied = patch.hostname.is_some(); config_client .patch_config( BaseController::default(), @@ -464,8 +522,9 @@ pub(super) async fn apply_web_source_runtime_reconcile( }, ) .await?; - let current_config = get_runtime_config(rpc_client, inst_id).await?; - ensure_runtime_config_converged(¤t_config, &desired_config)?; + let mut current_config = get_runtime_config(rpc_client, inst_id).await?; + ensure_runtime_config_converged(¤t_config, &desired_config, hostname_applied)?; + restore_omitted_hostname(&mut current_config, &desired_config, hostname_applied); Ok(current_config) } } @@ -759,6 +818,63 @@ mod tests { ); } + #[test] + fn runtime_reconcile_ignores_automatic_device_name_when_unmanaged() { + let mut current = config_with_port_forwards(Vec::new()); + current.dev_name = Some("et_3_abcd".to_string()); + let desired = config_with_port_forwards(Vec::new()); + + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + + assert!(matches!(action, RuntimeReconcileAction::Unchanged(_))); + } + + #[test] + fn runtime_reconcile_ignores_automatic_device_name_for_empty_desired_name() { + let mut current = config_with_port_forwards(Vec::new()); + current.dev_name = Some("et_3_abcd".to_string()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.dev_name = Some(String::new()); + + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + + assert!(matches!(action, RuntimeReconcileAction::Unchanged(_))); + } + + #[test] + fn runtime_reconcile_clears_explicit_device_name() { + let mut current = config_with_port_forwards(Vec::new()); + current.dev_name = Some("managed-device".to_string()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.dev_name = Some(String::new()); + + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + let RuntimeReconcileAction::Run { overwrite, .. } = action else { + panic!("clearing an explicit device name should require a full overwrite"); + }; + + assert!(overwrite); + } + + #[test] + fn runtime_reconcile_applies_explicit_device_name() { + let mut current = config_with_port_forwards(Vec::new()); + current.dev_name = Some("et_3_abcd".to_string()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.dev_name = Some("managed-device".to_string()); + + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + let RuntimeReconcileAction::Run { overwrite, .. } = action else { + panic!("explicit device name should require a full overwrite"); + }; + + assert!(overwrite); + } + #[test] fn runtime_convergence_rejects_stale_extra_port_forward() { let current = config_with_port_forwards(vec![ @@ -769,7 +885,7 @@ mod tests { let desired = config_with_port_forwards(vec![port_forward(23000, 5174), port_forward(23007, 3389)]); - let err = ensure_runtime_config_converged(¤t, &desired) + let err = ensure_runtime_config_converged(¤t, &desired, false) .expect_err("extra runtime port forward should not converge"); assert!( @@ -791,13 +907,14 @@ mod tests { .expect("hot patch"); assert_eq!(patch, InstanceConfigPatch::default()); - ensure_runtime_config_converged(¤t, &desired).expect("runtime converged"); + ensure_runtime_config_converged(¤t, &desired, false).expect("runtime converged"); } #[test] fn runtime_patch_rejects_non_hot_config_change() { let current = config_with_port_forwards(Vec::new()); let mut desired = current.clone(); + desired.network_secret = Some("new-secret".to_string()); let patch = web_source_runtime_patch(¤t, &desired).expect("build patch"); @@ -805,6 +922,36 @@ mod tests { assert!(patch.is_none()); } + #[test] + fn full_overwrite_preserves_unmanaged_hostname_for_later_explicit_clear() { + let mut current = config_with_port_forwards(Vec::new()); + current.hostname = Some("runtime-host".to_string()); + let mut unmanaged_desired = current.clone(); + unmanaged_desired.hostname = None; + unmanaged_desired.network_secret = Some("new-secret".to_string()); + + let action = + prepare_web_source_runtime_reconcile_from_current(¤t, unmanaged_desired.clone()) + .expect("prepare full overwrite"); + let RuntimeReconcileAction::Run { config, overwrite } = action else { + panic!("non-hot change should require a full overwrite"); + }; + assert!(overwrite); + assert_eq!(config.hostname.as_deref(), Some("runtime-host")); + + let observed_after_run = *config; + let mut explicit_clear = unmanaged_desired; + explicit_clear.hostname = Some(String::new()); + let action = + prepare_web_source_runtime_reconcile_from_current(&observed_after_run, explicit_clear) + .expect("prepare explicit clear"); + let RuntimeReconcileAction::Patch(patch) = action else { + panic!("explicit clear should patch the preserved runtime hostname"); + }; + + assert_eq!(patch.hostname.as_deref(), Some("")); + } + #[test] fn runtime_patch_replaces_managed_credentials_without_full_run() { let current = config_with_port_forwards(Vec::new()); @@ -1086,14 +1233,136 @@ mod tests { } #[test] - fn runtime_patch_rejects_explicit_desired_hostname_change() { - let current = config_with_port_forwards(vec![port_forward(23000, 5174)]); - let mut desired = - config_with_port_forwards(vec![port_forward(23000, 5174), port_forward(23007, 3389)]); + fn runtime_reconcile_hot_patches_explicit_desired_hostname_change() { + let current = config_with_port_forwards(Vec::new()); + let mut desired = config_with_port_forwards(Vec::new()); desired.hostname = Some("desired-host".to_string()); - let patch = web_source_runtime_patch(¤t, &desired).expect("build patch"); + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + let RuntimeReconcileAction::Patch(patch) = action else { + panic!("hostname-only change should use a hot patch"); + }; - assert!(patch.is_none()); + assert_eq!(patch.hostname.as_deref(), Some("desired-host")); + } + + #[test] + fn runtime_convergence_accepts_hostname_only_readback_difference() { + let current = config_with_port_forwards(Vec::new()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("device-host".to_string()); + + ensure_runtime_config_converged(¤t, &desired, true) + .expect("hostname-only readback difference should be converged"); + } + + #[test] + fn runtime_convergence_rejects_omitted_hostname_before_apply() { + let current = config_with_port_forwards(Vec::new()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("device-host".to_string()); + + let err = ensure_runtime_config_converged(¤t, &desired, false) + .expect_err("omitted hostname before apply should not converge"); + + assert!( + err.to_string() + .contains("runtime config still needs patch after reconcile") + ); + } + + #[test] + fn runtime_convergence_rejects_explicit_wrong_hostname_after_apply() { + let mut current = config_with_port_forwards(Vec::new()); + current.hostname = Some("wrong-host".to_string()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("device-host".to_string()); + + let err = ensure_runtime_config_converged(¤t, &desired, true) + .expect_err("explicit wrong hostname should not converge"); + + assert!( + err.to_string() + .contains("runtime config still needs patch after reconcile") + ); + } + + #[test] + fn runtime_patch_clears_explicit_desired_hostname() { + let mut current = config_with_port_forwards(Vec::new()); + current.hostname = Some("runtime-host".to_string()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some(String::new()); + + let patch = web_source_runtime_patch(¤t, &desired) + .expect("build patch") + .expect("hot patch"); + + assert_eq!(patch.hostname.as_deref(), Some("")); + } + + #[test] + fn runtime_patch_normalizes_missing_runtime_hostname_for_explicit_clear() { + let current = config_with_port_forwards(Vec::new()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some(String::new()); + + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + + assert!(matches!(action, RuntimeReconcileAction::Unchanged(_))); + } + + #[test] + fn runtime_patch_skips_matching_explicit_hostname() { + let mut current = config_with_port_forwards(Vec::new()); + current.hostname = Some("desired-host".to_string()); + let desired = current.clone(); + + let action = prepare_web_source_runtime_reconcile_from_current(¤t, desired) + .expect("prepare reconcile"); + + assert!(matches!(action, RuntimeReconcileAction::Unchanged(_))); + } + + #[test] + fn runtime_patch_uses_core_normalized_hostname() { + let current = config_with_port_forwards(Vec::new()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("a".repeat(33)); + + let patch = web_source_runtime_patch(¤t, &desired) + .expect("build patch") + .expect("hot patch"); + + assert_eq!(patch.hostname.as_deref(), Some("a".repeat(32).as_str())); + } + + #[test] + fn runtime_patch_removes_hostname_control_characters() { + let current = config_with_port_forwards(Vec::new()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("node\u{7}-name".to_string()); + + let patch = web_source_runtime_patch(¤t, &desired) + .expect("build patch") + .expect("hot patch"); + + assert_eq!(patch.hostname.as_deref(), Some("node-name")); + } + + #[test] + fn runtime_patch_normalizes_control_only_hostname_to_clear() { + let mut current = config_with_port_forwards(Vec::new()); + current.hostname = Some("runtime-host".to_string()); + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("\u{7}\n".to_string()); + + let patch = web_source_runtime_patch(¤t, &desired) + .expect("build patch") + .expect("hot patch"); + + assert_eq!(patch.hostname.as_deref(), Some("")); } } diff --git a/easytier-web/src/client_manager/session.rs b/easytier-web/src/client_manager/session.rs index e616a6f9..70d1a5c3 100644 --- a/easytier-web/src/client_manager/session.rs +++ b/easytier-web/src/client_manager/session.rs @@ -2,7 +2,7 @@ use std::{ collections::HashSet, fmt::Debug, str::FromStr as _, - sync::Arc, + sync::{Arc, Mutex, MutexGuard}, time::{Duration, Instant}, }; @@ -20,7 +20,10 @@ use easytier_core::tunnel::Tunnel; use tokio::sync::{Notify, RwLock, broadcast}; use tokio_util::task::AbortOnDropHandle; -use super::storage::{Storage, StorageToken, WeakRefStorage}; +use super::{ + HeartbeatPolicy, + storage::{Storage, StorageToken, WeakRefStorage}, +}; use crate::FeatureFlags; use crate::webhook::SharedWebhookConfig; @@ -28,6 +31,8 @@ mod runtime_revision; mod webhook_validation; const WEBHOOK_VALIDATION_HEARTBEAT_INTERVAL: u32 = 10; +const CONNECTED_WEBHOOK_RETRY_DELAYS: [Duration; 2] = + [Duration::from_millis(100), Duration::from_millis(500)]; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct Location { @@ -44,13 +49,71 @@ enum SessionAuthState { } #[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct ManagedConfigRevisionDelta { +pub(super) struct ManagedConfigPersistedChange { pub expected_revision: String, pub target_revision: String, - pub upsert_instance_ids: HashSet, - pub delete_instance_ids: HashSet, + pub dirty_instance_ids: HashSet, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum ManagedConfigReconcileHint { + Full, + Dirty { + expected_revision: String, + target_revision: String, + instance_ids: HashSet, + }, +} + +pub(super) fn record_managed_config_reconcile_hint( + pending: &mut Option, + hint: ManagedConfigReconcileHint, +) { + match hint { + ManagedConfigReconcileHint::Full => { + *pending = Some(ManagedConfigReconcileHint::Full); + } + ManagedConfigReconcileHint::Dirty { + expected_revision, + target_revision, + instance_ids, + } => match pending { + Some(ManagedConfigReconcileHint::Full) => {} + Some(ManagedConfigReconcileHint::Dirty { + target_revision: pending_target, + instance_ids: pending_ids, + .. + }) => { + if *pending_target == expected_revision { + *pending_target = target_revision; + pending_ids.extend(instance_ids); + } else { + *pending = Some(ManagedConfigReconcileHint::Full); + } + } + None => { + *pending = Some(ManagedConfigReconcileHint::Dirty { + expected_revision, + target_revision, + instance_ids, + }); + } + }, + } +} + +#[derive(Debug, Default)] +pub(super) struct ManagedRuntimeState { + pub(super) applied_config_revision: Option, + pub(super) applied_config_revision_known: bool, + pub(super) known_runtime_base_revision: Option, + pub(super) pending_managed_config_reconcile: Option, + pub(super) runtime_config_epoch: u64, + pub(super) runtime_config_cache_epoch: u64, +} + +pub(super) type SharedManagedRuntimeState = Arc>; + impl SessionAuthState { fn is_authorized(self) -> bool { matches!(self, Self::Authorized) @@ -66,9 +129,8 @@ pub struct SessionData { storage_token: Option, binding_version: Option, - applied_config_revision: Option, - pending_managed_config_delta: Option, - runtime_config_epoch: u64, + managed_runtime: SharedManagedRuntimeState, + direct_run_failed_instance_ids: HashSet, notifier: broadcast::Sender, req: Option, location: Option, @@ -77,6 +139,7 @@ pub struct SessionData { auth_state: SessionAuthState, webhook_connected_binding_version: Option, webhook_validation_dirty: bool, + webhook_validation_change_epoch: u64, webhook_validation_notify: Arc, session_epoch: u64, } @@ -98,9 +161,8 @@ impl SessionData { client_url, storage_token: None, binding_version: None, - applied_config_revision: None, - pending_managed_config_delta: None, - runtime_config_epoch: 0, + managed_runtime: Arc::new(Mutex::new(ManagedRuntimeState::default())), + direct_run_failed_instance_ids: HashSet::new(), notifier: tx, req: None, location, @@ -109,6 +171,7 @@ impl SessionData { auth_state: SessionAuthState::Init, webhook_connected_binding_version: None, webhook_validation_dirty: false, + webhook_validation_change_epoch: 0, webhook_validation_notify: Arc::new(Notify::new()), session_epoch: 0, } @@ -125,6 +188,12 @@ impl SessionData { pub fn location(&self) -> Option<&Location> { self.location.as_ref() } + + fn managed_runtime(&self) -> MutexGuard<'_, ManagedRuntimeState> { + self.managed_runtime + .lock() + .expect("managed runtime state lock poisoned") + } } async fn send_webhook_node_disconnected( @@ -190,7 +259,18 @@ fn connection_state_matches( .is_some_and(|current| storage_tokens_match(current, storage_token)) } -async fn connection_state_is_current( +fn connected_delivery_state_matches( + data: &SessionData, + storage_token: &StorageToken, + binding_version: u64, +) -> bool { + connection_state_matches(data, storage_token, binding_version) + && data.storage.upgrade().is_some_and(|storage| { + storage.owns_authorized_session(storage_token, data.session_epoch) + }) +} + +async fn connected_delivery_is_current( session_data: &std::sync::Weak>, storage_token: &StorageToken, binding_version: u64, @@ -199,23 +279,38 @@ async fn connection_state_is_current( return false; }; let data = session_data.read().await; - connection_state_matches(&data, storage_token, binding_version) + connected_delivery_state_matches(&data, storage_token, binding_version) +} + +enum ConnectedBindingRecord { + Recorded, + /// The session identity moved on; the delivered connected webhook should + /// be compensated with a disconnect. + IdentityStale, + /// A newer session already owns the machine route; its bindings must be + /// left untouched so a stale disconnect cannot revoke them. + OwnershipLost, } async fn record_webhook_connected_binding_if_current( session_data: &std::sync::Weak>, storage_token: &StorageToken, binding_version: u64, -) -> bool { - let Some(session_data) = session_data.upgrade() else { - return false; - }; +) -> Option { + let session_data = session_data.upgrade()?; let mut data = session_data.write().await; if !connection_state_matches(&data, storage_token, binding_version) { - return false; + return Some(ConnectedBindingRecord::IdentityStale); + } + if !data + .storage + .upgrade() + .is_some_and(|storage| storage.owns_authorized_session(storage_token, data.session_epoch)) + { + return Some(ConnectedBindingRecord::OwnershipLost); } data.webhook_connected_binding_version = Some(binding_version); - true + Some(ConnectedBindingRecord::Recorded) } async fn send_webhook_connection_transition( @@ -235,7 +330,65 @@ async fn send_webhook_connection_transition( let Some(connect) = connect else { return; }; - if !connection_state_is_current( + let delivery_started_at = Instant::now(); + let mut attempt = 1; + loop { + if !connected_delivery_is_current( + &session_data, + &connect.storage_token, + connect.binding_version, + ) + .await + { + return; + } + match connect.webhook.notify_node_connected(&connect.req).await { + Ok(()) => { + let elapsed = delivery_started_at.elapsed(); + if attempt > 1 || elapsed >= Duration::from_secs(2) { + tracing::info!( + machine_id = %connect.storage_token.machine_id, + binding_version = connect.binding_version, + attempt, + elapsed_ms = elapsed.as_millis(), + "node-connected webhook delivery completed" + ); + } + break; + } + Err(error) => { + let retry_delay = if error.is_retryable() { + CONNECTED_WEBHOOK_RETRY_DELAYS.get(attempt - 1).copied() + } else { + None + }; + tracing::warn!( + machine_id = %connect.storage_token.machine_id, + binding_version = connect.binding_version, + attempt, + elapsed_ms = delivery_started_at.elapsed().as_millis(), + will_retry = retry_delay.is_some(), + %error, + "node-connected webhook delivery failed" + ); + let Some(retry_delay) = retry_delay else { + return; + }; + if !connected_delivery_is_current( + &session_data, + &connect.storage_token, + connect.binding_version, + ) + .await + { + return; + } + tokio::time::sleep(retry_delay).await; + attempt += 1; + } + } + } + if !connected_delivery_is_current( &session_data, &connect.storage_token, connect.binding_version, @@ -244,21 +397,29 @@ async fn send_webhook_connection_transition( { return; } - - connect.webhook.notify_node_connected(&connect.req).await; - if !record_webhook_connected_binding_if_current( + match record_webhook_connected_binding_if_current( &session_data, &connect.storage_token, connect.binding_version, ) .await { - send_webhook_node_disconnected( - connect.webhook, - connect.storage_token, - connect.binding_version, - ) - .await; + Some(ConnectedBindingRecord::Recorded) => {} + Some(ConnectedBindingRecord::OwnershipLost) => { + tracing::debug!( + machine_id = %connect.storage_token.machine_id, + binding_version = connect.binding_version, + "skip disconnect compensation because a newer session owns the route" + ); + } + Some(ConnectedBindingRecord::IdentityStale) | None => { + send_webhook_node_disconnected( + connect.webhook, + connect.storage_token, + connect.binding_version, + ) + .await; + } } } @@ -267,10 +428,20 @@ impl Drop for SessionData { if let Ok(storage) = Storage::try_from(self.storage.clone()) && let Some(token) = self.storage_token.as_ref() { - storage.remove_session_client(token, self.session_epoch); + let removed_current_session = storage.remove_session_client(token, self.session_epoch); + + if removed_current_session { + tracing::info!( + machine_id = %token.machine_id, + user_id = token.user_id, + session_epoch = self.session_epoch, + "session disconnected" + ); + } // Notify the webhook receiver when a node disconnects. - if self.webhook_config.is_enabled() + if removed_current_session + && self.webhook_config.is_enabled() && let Some(binding_version) = self.webhook_connected_binding_version { notify_webhook_node_disconnected( @@ -288,7 +459,13 @@ pub type SharedSessionData = Arc>; #[derive(Clone)] pub(super) struct SessionRpcService { data: SharedSessionData, - heartbeat_min_response_delay: Duration, + heartbeat_policy: HeartbeatPolicy, +} + +impl SessionRpcService { + fn heartbeat_response(&self) -> HeartbeatResponse { + self.heartbeat_policy.response() + } } fn heartbeat_response_delay(elapsed: Duration, min_response_delay: Duration) -> Option { @@ -297,12 +474,20 @@ fn heartbeat_response_delay(elapsed: Duration, min_response_delay: Duration) -> .filter(|delay| !delay.is_zero()) } -fn should_delay_heartbeat_response(is_paced_session: bool, is_first_heartbeat: bool) -> bool { - is_paced_session && !is_first_heartbeat +fn should_delay_heartbeat_response( + supports_heartbeat_policy: bool, + is_paced_session: bool, + is_first_heartbeat: bool, +) -> bool { + !supports_heartbeat_policy && is_paced_session && !is_first_heartbeat } -fn should_delay_session_heartbeat_response(data: &SessionData) -> bool { +fn should_delay_session_heartbeat_response( + data: &SessionData, + supports_heartbeat_policy: bool, +) -> bool { should_delay_heartbeat_response( + supports_heartbeat_policy, data.webhook_config.is_enabled() || data.auth_state.is_authorized(), data.req.is_none(), ) @@ -316,11 +501,16 @@ fn should_notify_webhook_validation(heartbeat_count: u32) -> bool { struct HeartbeatIdentity { token: String, machine_id: uuid::Uuid, + runtime_id: Option, } impl HeartbeatIdentity { - fn new(token: String, machine_id: uuid::Uuid) -> Self { - Self { token, machine_id } + fn new(token: String, machine_id: uuid::Uuid, runtime_id: Option) -> Self { + Self { + token, + machine_id, + runtime_id, + } } } @@ -375,6 +565,9 @@ impl SessionRpcService { Self::storage_token_matches_heartbeat(storage_token, current_req) }) && data.auth_state.is_authorized() + && data.storage.upgrade().is_some_and(|storage| { + storage.owns_authorized_session(storage_token, data.session_epoch) + }) }) } @@ -387,7 +580,15 @@ impl SessionRpcService { } fn heartbeat_identity(req: &HeartbeatRequest, machine_id: uuid::Uuid) -> HeartbeatIdentity { - HeartbeatIdentity::new(req.user_token.clone(), machine_id) + HeartbeatIdentity::new( + req.user_token.clone(), + machine_id, + Self::heartbeat_runtime_id(req), + ) + } + + fn heartbeat_runtime_id(req: &HeartbeatRequest) -> Option { + req.inst_id.map(uuid::Uuid::from).filter(|id| !id.is_nil()) } fn ensure_session_identity_locked( @@ -414,6 +615,98 @@ impl SessionRpcService { data.webhook_validation_notify.clone() } + fn mark_webhook_validation_state_changed_locked(data: &mut SessionData) -> Arc { + data.webhook_validation_change_epoch = data.webhook_validation_change_epoch.wrapping_add(1); + Self::mark_webhook_validation_dirty_locked(data) + } + + fn failed_instance_ids( + req: Option<&HeartbeatRequest>, + direct_run_failed_instance_ids: &HashSet, + ) -> HashSet { + let mut instance_ids = req + .into_iter() + .flat_map(|req| &req.failed_network_instances) + .map(ToString::to_string) + .collect::>(); + instance_ids.extend(direct_run_failed_instance_ids.iter().cloned()); + instance_ids + } + + fn failed_instance_ids_locked(data: &SessionData) -> HashSet { + Self::failed_instance_ids(data.req.as_ref(), &data.direct_run_failed_instance_ids) + } + + fn sorted_failed_instance_ids_locked(data: &SessionData) -> Vec { + let mut instance_ids = Self::failed_instance_ids_locked(data) + .into_iter() + .collect::>(); + instance_ids.sort_unstable(); + instance_ids + } + + fn update_heartbeat_failed_instance_ids_locked( + data: &mut SessionData, + req: &HeartbeatRequest, + ) -> Option> { + let previous_instance_ids = Self::failed_instance_ids_locked(data); + let next_instance_ids = + Self::failed_instance_ids(Some(req), &data.direct_run_failed_instance_ids); + if next_instance_ids == previous_instance_ids { + return None; + } + tracing::info!( + machine_id = ?req.machine_id, + failed_instance_ids = ?next_instance_ids, + "heartbeat failed instance set changed" + ); + Some(Self::mark_webhook_validation_state_changed_locked(data)) + } + + fn update_direct_run_failures_locked( + data: &mut SessionData, + update: impl FnOnce(&mut HashSet), + ) -> Option> { + let previous_failed_instance_ids = Self::failed_instance_ids_locked(data); + update(&mut data.direct_run_failed_instance_ids); + let failed_instance_ids = Self::failed_instance_ids_locked(data); + (failed_instance_ids != previous_failed_instance_ids) + .then(|| Self::mark_webhook_validation_state_changed_locked(data)) + } + + fn update_direct_run_failure_locked( + data: &mut SessionData, + instance_id: &str, + failed: bool, + ) -> Option> { + Self::update_direct_run_failures_locked(data, |direct_run_instance_ids| { + if failed { + direct_run_instance_ids.insert(instance_id.to_owned()); + } else { + direct_run_instance_ids.remove(instance_id); + } + }) + } + + fn retain_direct_run_failures_locked( + data: &mut SessionData, + desired_instance_ids: &HashSet, + ) -> Option> { + Self::update_direct_run_failures_locked(data, |direct_run_instance_ids| { + direct_run_instance_ids + .retain(|instance_id| desired_instance_ids.contains(instance_id)); + }) + } + + fn remove_direct_run_failures_locked( + data: &mut SessionData, + instance_ids: &HashSet, + ) -> Option> { + Self::update_direct_run_failures_locked(data, |direct_run_instance_ids| { + direct_run_instance_ids.retain(|instance_id| !instance_ids.contains(instance_id)); + }) + } + async fn handle_webhook_heartbeat( &self, storage: &Storage, @@ -431,13 +724,16 @@ impl SessionRpcService { ); return Err(anyhow::anyhow!("webhook session is invalid").into()); } + let failure_notify = Self::update_heartbeat_failed_instance_ids_locked(&mut data, &req); let runtime_req = Self::store_latest_heartbeat_req(&mut data, req); let heartbeat_count = data .heartbeat_count .fetch_add(1, std::sync::atomic::Ordering::Relaxed) + 1; - let notify = should_notify_webhook_validation(heartbeat_count) - .then(|| Self::mark_webhook_validation_dirty_locked(&mut data)); + let notify = failure_notify.or_else(|| { + should_notify_webhook_validation(heartbeat_count) + .then(|| Self::mark_webhook_validation_dirty_locked(&mut data)) + }); let authorized = data.auth_state.is_authorized(); if let Some(storage_token) = data.storage_token.clone() { let report_time = Self::heartbeat_report_timestamp(&runtime_req); @@ -459,7 +755,7 @@ impl SessionRpcService { if let Some(notify) = notify { notify.notify_one(); } - Ok(HeartbeatResponse {}) + Ok(self.heartbeat_response()) } async fn handle_heartbeat( @@ -470,7 +766,7 @@ impl SessionRpcService { let data = self.data.read().await; let Ok(storage) = Storage::try_from(data.storage.clone()) else { tracing::error!("Failed to get storage"); - return Ok(HeartbeatResponse {}); + return Ok(self.heartbeat_response()); }; ( storage, @@ -500,57 +796,70 @@ impl SessionRpcService { .db() .get_user_id_by_token(req.user_token.clone()) .await - .with_context(|| { - format!( - "Failed to get user id by token from db: {:?}", - req.user_token - ) - })? { + .with_context(|| "Failed to get user id by token from db".to_string())? + { Some(id) => id, None if feature_flags.allow_auto_create_user => storage .auto_create_user(&req.user_token) .await - .with_context(|| format!("Failed to auto-create user: {:?}", req.user_token))?, + .with_context(|| "Failed to auto-create user".to_string())?, None => { - return Err( - anyhow::anyhow!("User not found by token: {:?}", req.user_token).into(), - ); + return Err(anyhow::anyhow!("User not found by token").into()); } }; - let (storage_token, notifier, runtime_req, session_epoch) = { + let (storage_token, notifier, runtime_req, session_epoch, validation_notify) = { let mut data = self.data.write().await; let is_new_storage_token = data.storage_token.is_none(); + let validation_notify = + Self::update_heartbeat_failed_instance_ids_locked(&mut data, &req); let runtime_req = Self::store_latest_heartbeat_req(&mut data, req.clone()); data.heartbeat_count .fetch_add(1, std::sync::atomic::Ordering::Relaxed); if is_new_storage_token { assert!(data.storage_token.is_none()); + data.managed_runtime = storage.bind_managed_runtime_state( + user_id, + machine_id, + Self::heartbeat_runtime_id(&runtime_req), + data.session_epoch, + ); data.storage_token = Some(StorageToken { token: runtime_req.user_token.clone(), client_url: data.client_url.clone(), machine_id, user_id, }); + tracing::info!( + %machine_id, + user_id, + session_epoch = data.session_epoch, + client_url = %data.client_url, + "session identity established" + ); } data.auth_state = SessionAuthState::Authorized; let Some(storage_token) = data.storage_token.as_ref().cloned() else { tracing::error!("Heartbeat succeeded before session token was initialized"); - return Ok(HeartbeatResponse {}); + return Ok(self.heartbeat_response()); }; ( storage_token, data.notifier.clone(), runtime_req, data.session_epoch, + validation_notify, ) }; let report_time = Self::heartbeat_report_timestamp(&runtime_req); storage.update_session_client(storage_token, report_time, true, session_epoch); let _ = notifier.send(runtime_req); - Ok(HeartbeatResponse {}) + if let Some(notify) = validation_notify { + notify.notify_one(); + } + Ok(self.heartbeat_response()) } } @@ -564,9 +873,10 @@ impl WebServerService for SessionRpcService { req: HeartbeatRequest, ) -> rpc_types::error::Result { let started_at = Instant::now(); + let support_heartbeat_policy = req.support_heartbeat_policy; let should_delay_response = { let data = self.data.read().await; - should_delay_session_heartbeat_response(&data) + should_delay_session_heartbeat_response(&data, support_heartbeat_policy) }; let ret = self.handle_heartbeat(req).await; if ret.is_err() { @@ -574,8 +884,10 @@ impl WebServerService for SessionRpcService { // sleep for a while to avoid client busy loop tokio::time::sleep(std::time::Duration::from_secs(2)).await; } else if should_delay_response - && let Some(delay) = - heartbeat_response_delay(started_at.elapsed(), self.heartbeat_min_response_delay) + && let Some(delay) = heartbeat_response_delay( + started_at.elapsed(), + self.heartbeat_policy.legacy_response_delay(), + ) { tokio::time::sleep(delay).await; } @@ -617,7 +929,7 @@ impl Session { storage: WeakRefStorage, client_url: url::Url, location: Option, - heartbeat_min_response_delay: Duration, + heartbeat_policy: HeartbeatPolicy, feature_flags: Arc, webhook_config: SharedWebhookConfig, session_epoch: u64, @@ -628,12 +940,12 @@ impl Session { let data = Arc::new(RwLock::new(session_data)); let rpc_mgr = - BidirectRpcManager::new().set_rx_timeout(Some(std::time::Duration::from_secs(30))); + BidirectRpcManager::new().set_rx_timeout(Some(heartbeat_policy.session_rx_timeout())); rpc_mgr.rpc_server().registry().register( WebServerServiceServer::new(SessionRpcService { data: data.clone(), - heartbeat_min_response_delay, + heartbeat_policy, }), "", ); @@ -711,14 +1023,13 @@ impl Session { self.scoped_client::>() } - pub(super) async fn notify_full_config_revision_changed( + pub(super) async fn notify_managed_runtime_state_changed( &self, user_id: i32, machine_id: uuid::Uuid, - config_revision: String, ) { let notify = { - let mut data = self.data.write().await; + let data = self.data.read().await; if !data.auth_state.is_authorized() { return; } @@ -729,71 +1040,6 @@ impl Session { { return; } - if data.applied_config_revision.as_deref() == Some(config_revision.as_str()) { - return; - } - data.pending_managed_config_delta = None; - data.req.clone().map(|req| (data.notifier.clone(), req)) - }; - if let Some((notifier, req)) = notify { - let _ = notifier.send(req); - } - } - - pub(super) async fn notify_patch_config_revision_changed( - &self, - user_id: i32, - machine_id: uuid::Uuid, - delta: ManagedConfigRevisionDelta, - ) { - let notify = { - let mut data = self.data.write().await; - if !data.auth_state.is_authorized() { - return; - } - if !data - .storage_token - .as_ref() - .is_some_and(|token| token.user_id == user_id && token.machine_id == machine_id) - { - return; - } - if data.applied_config_revision.as_deref() == Some(delta.target_revision.as_str()) { - return; - } - - // A Patch may drive a targeted runtime reconcile only when the - // connected Session has applied its exact base and no earlier - // Patch is still pending. Otherwise the normal Full reconcile is - // the safe convergence path. - data.pending_managed_config_delta = (data.applied_config_revision.as_deref() - == Some(delta.expected_revision.as_str()) - && data.pending_managed_config_delta.is_none()) - .then_some(delta); - data.req.clone().map(|req| (data.notifier.clone(), req)) - }; - if let Some((notifier, req)) = notify { - let _ = notifier.send(req); - } - } - - pub(super) async fn invalidate_applied_config_revision( - &self, - user_id: i32, - machine_id: uuid::Uuid, - ) { - let notify = { - let mut data = self.data.write().await; - if !data - .storage_token - .as_ref() - .is_some_and(|token| token.user_id == user_id && token.machine_id == machine_id) - { - return; - } - data.applied_config_revision = None; - data.pending_managed_config_delta = None; - data.runtime_config_epoch = data.runtime_config_epoch.wrapping_add(1); data.req.clone().map(|req| (data.notifier.clone(), req)) }; if let Some((notifier, req)) = notify { @@ -803,13 +1049,18 @@ impl Session { pub(crate) async fn invalidate_runtime_config_for_direct_mutation(&self) { let notify = { - let mut data = self.data.write().await; + let data = self.data.write().await; if data.storage_token.is_none() { return; } - data.applied_config_revision = None; - data.pending_managed_config_delta = None; - data.runtime_config_epoch = data.runtime_config_epoch.wrapping_add(1); + let mut runtime = data.managed_runtime(); + runtime.applied_config_revision = None; + runtime.applied_config_revision_known = true; + runtime.known_runtime_base_revision = None; + runtime.pending_managed_config_reconcile = Some(ManagedConfigReconcileHint::Full); + runtime.runtime_config_epoch = runtime.runtime_config_epoch.wrapping_add(1); + runtime.runtime_config_cache_epoch = runtime.runtime_config_cache_epoch.wrapping_add(1); + drop(runtime); data.req.clone().map(|req| (data.notifier.clone(), req)) }; if let Some((notifier, req)) = notify { @@ -827,13 +1078,16 @@ impl Session { #[cfg(test)] pub(super) async fn applied_config_revision(&self) -> Option { - self.data.read().await.applied_config_revision.clone() + let data = self.data.read().await; + data.managed_runtime().applied_config_revision.clone() } } #[cfg(test)] mod tests { - use axum::{Json, Router, extract::State, routing::post}; + use std::sync::atomic::{AtomicUsize, Ordering}; + + use axum::{Json, Router, extract::State, http::StatusCode, routing::post}; use serde_json::json; use tokio::sync::{Mutex, Notify, oneshot}; @@ -857,10 +1111,12 @@ mod tests { #[test] fn heartbeat_response_delay_skips_unpaced_and_first_heartbeat() { - assert!(!should_delay_heartbeat_response(false, true)); - assert!(!should_delay_heartbeat_response(false, false)); - assert!(!should_delay_heartbeat_response(true, true)); - assert!(should_delay_heartbeat_response(true, false)); + assert!(!HeartbeatRequest::default().support_heartbeat_policy); + assert!(!should_delay_heartbeat_response(false, false, true)); + assert!(!should_delay_heartbeat_response(false, false, false)); + assert!(!should_delay_heartbeat_response(false, true, true)); + assert!(should_delay_heartbeat_response(false, true, false)); + assert!(!should_delay_heartbeat_response(true, true, false)); } #[tokio::test] @@ -881,13 +1137,14 @@ mod tests { )), ); - assert!(!should_delay_session_heartbeat_response(&data)); + assert!(!should_delay_session_heartbeat_response(&data, false)); data.req = Some(heartbeat_request("token", machine_id)); - assert!(should_delay_session_heartbeat_response(&data)); + assert!(should_delay_session_heartbeat_response(&data, false)); + assert!(!should_delay_session_heartbeat_response(&data, true)); data.auth_state = SessionAuthState::Invalid; - assert!(should_delay_session_heartbeat_response(&data)); + assert!(should_delay_session_heartbeat_response(&data, false)); } #[test] @@ -911,6 +1168,118 @@ mod tests { } } + async fn failure_state_test_data() -> SessionData { + let storage = Storage::new(crate::db::Db::memory_db().await); + SessionData::new( + storage.weak_ref(), + url::Url::parse("http://127.0.0.1").unwrap(), + None, + Arc::new(FeatureFlags::default()), + Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ) + } + + #[tokio::test] + async fn failed_instance_ids_merge_core_and_web_local_failures() { + let mut data = failure_state_test_data().await; + let core_failed = uuid::Uuid::new_v4(); + let local_failed = uuid::Uuid::new_v4().to_string(); + let core_failed_req = HeartbeatRequest { + failed_network_instances: vec![core_failed.into()], + ..Default::default() + }; + + assert!( + SessionRpcService::update_heartbeat_failed_instance_ids_locked( + &mut data, + &core_failed_req, + ) + .is_some() + ); + SessionRpcService::store_latest_heartbeat_req(&mut data, core_failed_req); + assert_eq!( + SessionRpcService::failed_instance_ids_locked(&data), + HashSet::from([core_failed.to_string()]) + ); + + assert!( + SessionRpcService::update_direct_run_failure_locked(&mut data, &local_failed, true) + .is_some() + ); + assert_eq!( + SessionRpcService::failed_instance_ids_locked(&data), + HashSet::from([core_failed.to_string(), local_failed.clone()]) + ); + + let recovered_req = HeartbeatRequest::default(); + SessionRpcService::update_heartbeat_failed_instance_ids_locked(&mut data, &recovered_req); + SessionRpcService::store_latest_heartbeat_req(&mut data, recovered_req); + assert_eq!( + SessionRpcService::failed_instance_ids_locked(&data), + HashSet::from([local_failed]) + ); + } + + #[tokio::test] + async fn stable_failed_instance_ids_do_not_repeat_validation_work() { + let mut data = failure_state_test_data().await; + let failed = uuid::Uuid::new_v4(); + let failed_req = HeartbeatRequest { + failed_network_instances: vec![failed.into()], + ..Default::default() + }; + assert!( + SessionRpcService::update_heartbeat_failed_instance_ids_locked(&mut data, &failed_req) + .is_some() + ); + SessionRpcService::store_latest_heartbeat_req(&mut data, failed_req); + data.webhook_validation_dirty = false; + let change_epoch = data.webhook_validation_change_epoch; + + assert!( + SessionRpcService::update_direct_run_failure_locked( + &mut data, + &failed.to_string(), + true, + ) + .is_none() + ); + let recovered_req = HeartbeatRequest::default(); + assert!( + SessionRpcService::update_heartbeat_failed_instance_ids_locked( + &mut data, + &recovered_req, + ) + .is_none() + ); + SessionRpcService::store_latest_heartbeat_req(&mut data, recovered_req); + assert!(!data.webhook_validation_dirty); + assert_eq!(data.webhook_validation_change_epoch, change_epoch); + } + + #[tokio::test] + async fn direct_run_failure_is_added_and_direct_success_clears_it() { + let mut data = failure_state_test_data().await; + let instance_id = uuid::Uuid::new_v4().to_string(); + + assert!( + SessionRpcService::update_direct_run_failure_locked(&mut data, &instance_id, true) + .is_some() + ); + assert_eq!( + SessionRpcService::failed_instance_ids_locked(&data), + HashSet::from([instance_id.clone()]) + ); + + assert!( + SessionRpcService::update_direct_run_failure_locked(&mut data, &instance_id, false) + .is_some() + ); + assert!(SessionRpcService::failed_instance_ids_locked(&data).is_empty()); + } + #[derive(Clone)] struct ValidateWebhookTestState { received: Arc>>>, @@ -954,6 +1323,12 @@ mod tests { .route("/validate-token", post(valid_validate_token_handler)) .route("/webhook/node-connected", post(node_connected_handler)) .with_state(state); + test_webhook_server(app).await + } + + async fn test_webhook_server( + app: Router, + ) -> (SharedWebhookConfig, tokio::task::JoinHandle<()>) { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); let server = tokio::spawn(async move { @@ -970,6 +1345,299 @@ mod tests { (webhook_config, server) } + #[derive(Clone)] + struct RetryingConnectedWebhookState { + attempts: Arc, + second_received: Arc, + second_release: Arc, + } + + async fn retrying_node_connected_handler( + State(state): State, + ) -> (StatusCode, Json) { + let attempt = state.attempts.fetch_add(1, Ordering::Relaxed) + 1; + if attempt == 1 { + return ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(json!({"status": "error"})), + ); + } + state.second_received.notify_one(); + state.second_release.notified().await; + (StatusCode::OK, Json(json!({"status": "ok"}))) + } + + #[tokio::test] + async fn connected_webhook_is_confirmed_only_after_successful_retry() { + let attempts = Arc::new(AtomicUsize::new(0)); + let second_received = Arc::new(Notify::new()); + let second_release = Arc::new(Notify::new()); + let app = Router::new() + .route( + "/webhook/node-connected", + post(retrying_node_connected_handler), + ) + .with_state(RetryingConnectedWebhookState { + attempts: attempts.clone(), + second_received: second_received.clone(), + second_release: second_release.clone(), + }); + let (webhook_config, server) = test_webhook_server(app).await; + let fixture = connected_delivery_fixture(webhook_config).await; + let session_data = fixture.session_data.clone(); + let delivery = tokio::spawn(send_webhook_connection_transition( + Arc::downgrade(&session_data), + None, + Some(fixture.notification), + )); + + tokio::time::timeout(Duration::from_secs(1), second_received.notified()) + .await + .expect("5xx connected webhook should be retried"); + assert_eq!( + session_data.read().await.webhook_connected_binding_version, + None + ); + + second_release.notify_one(); + delivery.await.unwrap(); + server.abort(); + + assert_eq!(attempts.load(Ordering::Relaxed), 2); + assert_eq!( + session_data.read().await.webhook_connected_binding_version, + Some(1) + ); + } + + #[derive(Clone)] + struct FailingConnectedWebhookState { + attempts: Arc, + first_received: Arc, + first_release: Option>, + status: StatusCode, + } + + async fn failing_node_connected_handler( + State(state): State, + ) -> (StatusCode, Json) { + let attempt = state.attempts.fetch_add(1, Ordering::Relaxed) + 1; + if attempt == 1 { + state.first_received.notify_one(); + if let Some(first_release) = state.first_release { + first_release.notified().await; + } + } + (state.status, Json(json!({"status": "error"}))) + } + + struct ConnectedDeliveryFixture { + storage: Storage, + session_data: Arc>, + notification: WebhookConnectNotification, + machine_id: uuid::Uuid, + user_id: i32, + } + + async fn connected_delivery_fixture( + webhook_config: SharedWebhookConfig, + ) -> ConnectedDeliveryFixture { + let machine_id = uuid::Uuid::new_v4(); + let storage = Storage::new(crate::db::Db::memory_db().await); + let user_id = storage.db().auto_create_user("token").await.unwrap().id; + let storage_token = StorageToken { + token: "token".to_string(), + client_url: url::Url::parse("http://127.0.0.1:1000").unwrap(), + machine_id, + user_id, + }; + storage.update_session_client(storage_token.clone(), 1, true, 1); + let mut session = SessionData::new( + storage.weak_ref(), + storage_token.client_url.clone(), + None, + Arc::new(FeatureFlags::default()), + webhook_config.clone(), + ); + session.storage_token = Some(storage_token.clone()); + session.auth_state = SessionAuthState::Authorized; + session.binding_version = Some(1); + session.session_epoch = 1; + + ConnectedDeliveryFixture { + storage, + session_data: Arc::new(RwLock::new(session)), + notification: WebhookConnectNotification { + webhook: webhook_config, + storage_token, + binding_version: 1, + req: crate::webhook::NodeConnectedRequest { + machine_id: machine_id.to_string(), + token: "token".to_string(), + user_id: Some(user_id), + hostname: String::new(), + version: String::new(), + os_type: None, + os_version: None, + os_distribution: None, + web_instance_id: None, + binding_version: Some(1), + }, + }, + machine_id, + user_id, + } + } + + #[tokio::test] + async fn connected_webhook_retry_stops_after_session_replacement() { + let attempts = Arc::new(AtomicUsize::new(0)); + let first_received = Arc::new(Notify::new()); + let first_release = Arc::new(Notify::new()); + let app = Router::new() + .route( + "/webhook/node-connected", + post(failing_node_connected_handler), + ) + .with_state(FailingConnectedWebhookState { + attempts: attempts.clone(), + first_received: first_received.clone(), + first_release: Some(first_release.clone()), + status: StatusCode::INTERNAL_SERVER_ERROR, + }); + let (webhook_config, server) = test_webhook_server(app).await; + + let fixture = connected_delivery_fixture(webhook_config).await; + let session_data = fixture.session_data.clone(); + let delivery = tokio::spawn(send_webhook_connection_transition( + Arc::downgrade(&session_data), + None, + Some(fixture.notification), + )); + + tokio::time::timeout(Duration::from_secs(1), first_received.notified()) + .await + .unwrap(); + fixture.storage.update_session_client( + StorageToken { + token: "token".to_string(), + client_url: url::Url::parse("http://127.0.0.1:2000").unwrap(), + machine_id: fixture.machine_id, + user_id: fixture.user_id, + }, + 2, + true, + 2, + ); + first_release.notify_one(); + delivery.await.unwrap(); + server.abort(); + + assert_eq!(attempts.load(Ordering::Relaxed), 1); + assert_eq!( + session_data.read().await.webhook_connected_binding_version, + None + ); + } + + #[tokio::test] + async fn connected_binding_record_respects_route_ownership() { + let webhook_config = Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )); + let fixture = connected_delivery_fixture(webhook_config).await; + let session_data = fixture.session_data.clone(); + let storage_token = fixture.notification.storage_token.clone(); + + let outcome = record_webhook_connected_binding_if_current( + &Arc::downgrade(&session_data), + &storage_token, + 1, + ) + .await; + assert!(matches!(outcome, Some(ConnectedBindingRecord::Recorded))); + assert_eq!( + session_data.read().await.webhook_connected_binding_version, + Some(1) + ); + + // A replacement session wins the machine route; the stale task must + // neither record its binding nor earn disconnect compensation. + fixture.storage.update_session_client( + StorageToken { + token: storage_token.token.clone(), + client_url: url::Url::parse("http://127.0.0.1:2000").unwrap(), + machine_id: fixture.machine_id, + user_id: fixture.user_id, + }, + 2, + true, + 2, + ); + session_data.write().await.webhook_connected_binding_version = None; + let outcome = record_webhook_connected_binding_if_current( + &Arc::downgrade(&session_data), + &storage_token, + 1, + ) + .await; + assert!(matches!( + outcome, + Some(ConnectedBindingRecord::OwnershipLost) + )); + assert_eq!( + session_data.read().await.webhook_connected_binding_version, + None + ); + } + + async fn run_failed_connected_delivery(status: StatusCode) -> (usize, Option) { + let attempts = Arc::new(AtomicUsize::new(0)); + let app = Router::new() + .route( + "/webhook/node-connected", + post(failing_node_connected_handler), + ) + .with_state(FailingConnectedWebhookState { + attempts: attempts.clone(), + first_received: Arc::new(Notify::new()), + first_release: None, + status, + }); + let (webhook_config, server) = test_webhook_server(app).await; + let fixture = connected_delivery_fixture(webhook_config).await; + let session_data = fixture.session_data.clone(); + + send_webhook_connection_transition( + Arc::downgrade(&session_data), + None, + Some(fixture.notification), + ) + .await; + server.abort(); + + let confirmed_binding_version = session_data.read().await.webhook_connected_binding_version; + (attempts.load(Ordering::Relaxed), confirmed_binding_version) + } + + #[tokio::test] + async fn connected_webhook_retry_is_bounded_when_receiver_keeps_failing() { + let (attempts, confirmed_binding_version) = + run_failed_connected_delivery(StatusCode::INTERNAL_SERVER_ERROR).await; + + assert_eq!(attempts, CONNECTED_WEBHOOK_RETRY_DELAYS.len() + 1); + assert_eq!(confirmed_binding_version, None); + } + + #[tokio::test] + async fn connected_webhook_does_not_retry_or_confirm_client_error() { + let (attempts, confirmed_binding_version) = + run_failed_connected_delivery(StatusCode::BAD_REQUEST).await; + + assert_eq!(attempts, 1); + assert_eq!(confirmed_binding_version, None); + } + #[test] fn heartbeat_identity_requires_matching_token_and_machine_id() { let machine_id = uuid::Uuid::new_v4(); @@ -989,6 +1657,21 @@ mod tests { )); } + #[test] + fn session_identity_includes_runtime_id() { + let machine_id = uuid::Uuid::new_v4(); + let mut request = heartbeat_request("token", machine_id); + let first_runtime_id = uuid::Uuid::new_v4(); + request.inst_id = Some(first_runtime_id.into()); + let first = SessionRpcService::heartbeat_identity(&request, machine_id); + + request.inst_id = Some(uuid::Uuid::new_v4().into()); + let restarted = SessionRpcService::heartbeat_identity(&request, machine_id); + + assert_eq!(first.runtime_id, Some(first_runtime_id)); + assert_ne!(first, restarted); + } + #[tokio::test] async fn webhook_heartbeat_saves_latest_and_marks_validation_dirty() { let machine_id = uuid::Uuid::new_v4(); @@ -1008,7 +1691,7 @@ mod tests { ))); let service = SessionRpcService { data: data.clone(), - heartbeat_min_response_delay: Duration::ZERO, + heartbeat_policy: HeartbeatPolicy::default(), }; service @@ -1018,6 +1701,7 @@ mod tests { let data = data.read().await; assert!(data.webhook_validation_dirty); + assert_eq!(data.webhook_validation_change_epoch, 0); assert_eq!(data.auth_state, SessionAuthState::Init); assert!(data.storage_token.is_none()); assert!(SessionRpcService::heartbeat_matches_identity( @@ -1071,9 +1755,12 @@ mod tests { webhook_config, client_url: url::Url::parse("http://127.0.0.1").unwrap(), applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), req, machine_id, }, + session_data.read().await.webhook_validation_change_epoch, )); received_rx.await.unwrap(); release.notify_waiters(); @@ -1147,7 +1834,7 @@ mod tests { let session_data = Arc::new(RwLock::new(data)); let service = SessionRpcService { data: session_data.clone(), - heartbeat_min_response_delay: Duration::ZERO, + heartbeat_policy: HeartbeatPolicy::default(), }; let err = service @@ -1216,7 +1903,7 @@ mod tests { let session_data = Arc::new(RwLock::new(data)); let service = SessionRpcService { data: session_data, - heartbeat_min_response_delay: Duration::ZERO, + heartbeat_policy: HeartbeatPolicy::default(), }; let err = service @@ -1271,6 +1958,7 @@ mod tests { data.webhook_connected_binding_version = Some(3); let session_data = Arc::new(RwLock::new(data)); + let validation_change_epoch = session_data.read().await.webhook_validation_change_epoch; webhook_validation::apply_rejected( &Arc::downgrade(&session_data), &webhook_validation::WebhookValidationInput { @@ -1280,9 +1968,12 @@ mod tests { )), client_url: url::Url::parse("http://127.0.0.1").unwrap(), applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), req, machine_id, }, + validation_change_epoch, ) .await; @@ -1344,10 +2035,13 @@ mod tests { )), client_url: client_url.clone(), applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), req: req.clone(), machine_id, }; - webhook_validation::apply_rejected(&weak_session, &input).await; + let validation_change_epoch = session_data.read().await.webhook_validation_change_epoch; + webhook_validation::apply_rejected(&weak_session, &input, validation_change_epoch).await; assert_eq!( session_data.read().await.webhook_connected_binding_version, None @@ -1361,6 +2055,7 @@ mod tests { Some(client_url.clone()) ); + let validation_change_epoch = session_data.read().await.webhook_validation_change_epoch; webhook_validation::apply_success( &weak_session, input, @@ -1369,6 +2064,7 @@ mod tests { binding_version: 7, }, user_id, + validation_change_epoch, ) .await; @@ -1424,7 +2120,7 @@ mod tests { let session_data = Arc::new(RwLock::new(data)); let service = SessionRpcService { data: session_data.clone(), - heartbeat_min_response_delay: Duration::ZERO, + heartbeat_policy: HeartbeatPolicy::default(), }; service @@ -1470,7 +2166,7 @@ mod tests { let session_data = Arc::new(RwLock::new(data)); let service = SessionRpcService { data: session_data, - heartbeat_min_response_delay: Duration::ZERO, + heartbeat_policy: HeartbeatPolicy::default(), }; service @@ -1518,6 +2214,7 @@ mod tests { data.webhook_connected_binding_version = Some(6); let session_data = Arc::new(RwLock::new(data)); + let validation_change_epoch = session_data.read().await.webhook_validation_change_epoch; webhook_validation::apply_success( &Arc::downgrade(&session_data), webhook_validation::WebhookValidationInput { @@ -1527,6 +2224,8 @@ mod tests { )), client_url, applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), req, machine_id, }, @@ -1535,6 +2234,7 @@ mod tests { binding_version: 7, }, user_id, + validation_change_epoch, ) .await; @@ -1545,7 +2245,7 @@ mod tests { } #[tokio::test] - async fn runtime_heartbeat_rechecks_webhook_state_before_reconcile() { + async fn rejected_session_stops_reconcile_without_clearing_runtime_state() { let machine_id = uuid::Uuid::new_v4(); let req = heartbeat_request("token", machine_id); let storage = Storage::new(crate::db::Db::memory_db().await); @@ -1569,15 +2269,23 @@ mod tests { None, )), ); + storage.update_session_client(storage_token.clone(), 1, true, 0); data.storage_token = Some(storage_token); data.session_identity = Some(SessionRpcService::heartbeat_identity(&req, machine_id)); data.req = Some(req.clone()); data.auth_state = SessionAuthState::Authorized; + { + let mut runtime = data.managed_runtime(); + runtime.applied_config_revision = Some("rev-1".to_string()); + runtime.applied_config_revision_known = true; + runtime.known_runtime_base_revision = Some("rev-1".to_string()); + } let session_data = Arc::new(RwLock::new(data)); let weak_session = Arc::downgrade(&session_data); assert!(SessionRpcService::runtime_heartbeat_is_current(&weak_session, &req).await); + let validation_change_epoch = session_data.read().await.webhook_validation_change_epoch; webhook_validation::apply_rejected( &weak_session, &webhook_validation::WebhookValidationInput { @@ -1587,13 +2295,42 @@ mod tests { )), client_url: url::Url::parse("http://127.0.0.1").unwrap(), applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), req: req.clone(), machine_id, }, + validation_change_epoch, ) .await; assert!(!SessionRpcService::runtime_heartbeat_is_current(&weak_session, &req).await); + let data = session_data.read().await; + let runtime = data.managed_runtime(); + assert_eq!(runtime.applied_config_revision.as_deref(), Some("rev-1")); + assert!(runtime.applied_config_revision_known); + assert_eq!( + runtime.known_runtime_base_revision.as_deref(), + Some("rev-1") + ); + } + + #[tokio::test] + async fn fresh_session_application_revision_is_unknown() { + let storage = Storage::new(crate::db::Db::memory_db().await); + let data = SessionData::new( + storage.weak_ref(), + url::Url::parse("http://127.0.0.1").unwrap(), + None, + Arc::new(FeatureFlags::default()), + Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ); + + let runtime = data.managed_runtime(); + assert_eq!(runtime.applied_config_revision, None); + assert!(!runtime.applied_config_revision_known); } #[test] @@ -1611,6 +2348,8 @@ mod tests { web_instance_api_base_url: Some("http://console".to_string()), persisted_config_revision: Some("rev-0".to_string()), applied_config_revision: Some("rev-1".to_string()), + applied_config_revision_known: true, + failed_instance_ids: vec!["failed-instance".to_string()], }; let value = serde_json::to_value(req).unwrap(); @@ -1626,5 +2365,95 @@ mod tests { .and_then(|v| v.as_str()), Some("rev-1") ); + assert_eq!( + value + .get("applied_config_revision_known") + .and_then(|v| v.as_bool()), + Some(true) + ); + assert_eq!( + value.get("failed_instance_ids"), + Some(&json!(["failed-instance"])) + ); + } + + #[test] + fn managed_patch_hints_merge_while_runtime_lags() { + let mut hint = None; + + record_managed_config_reconcile_hint( + &mut hint, + ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["instance-a".to_string()]), + }, + ); + record_managed_config_reconcile_hint( + &mut hint, + ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-b".to_string(), + target_revision: "rev-c".to_string(), + instance_ids: HashSet::from(["instance-b".to_string()]), + }, + ); + + assert_eq!( + hint, + Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-c".to_string(), + instance_ids: HashSet::from(["instance-a".to_string(), "instance-b".to_string(),]), + }) + ); + } + + #[test] + fn non_contiguous_managed_patch_hints_require_full_reconcile() { + let mut hint = Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["instance-a".to_string()]), + }); + + record_managed_config_reconcile_hint( + &mut hint, + ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-c".to_string(), + target_revision: "rev-d".to_string(), + instance_ids: HashSet::from(["instance-b".to_string()]), + }, + ); + + assert_eq!(hint, Some(ManagedConfigReconcileHint::Full)); + } + + #[test] + fn managed_patch_hint_does_not_narrow_pending_full_reconcile() { + let mut hint = Some(ManagedConfigReconcileHint::Full); + + record_managed_config_reconcile_hint( + &mut hint, + ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-0".to_string(), + target_revision: "rev-a".to_string(), + instance_ids: HashSet::from(["instance-a".to_string()]), + }, + ); + + assert_eq!(hint, Some(ManagedConfigReconcileHint::Full)); + } + + #[test] + fn full_reconcile_hint_replaces_pending_dirty_instances() { + let mut hint = Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-0".to_string(), + target_revision: "rev-a".to_string(), + instance_ids: HashSet::from(["instance-a".to_string()]), + }); + + record_managed_config_reconcile_hint(&mut hint, ManagedConfigReconcileHint::Full); + + assert_eq!(hint, Some(ManagedConfigReconcileHint::Full)); } } diff --git a/easytier-web/src/client_manager/session/runtime_revision.rs b/easytier-web/src/client_manager/session/runtime_revision.rs index c426f513..25dededa 100644 --- a/easytier-web/src/client_manager/session/runtime_revision.rs +++ b/easytier-web/src/client_manager/session/runtime_revision.rs @@ -13,7 +13,7 @@ use easytier_core::management::remote_client::{ListNetworkProps, Storage as _}; use tokio::sync::{RwLock, broadcast}; use super::{ - ManagedConfigRevisionDelta, SessionConfigClient, SessionData, SessionRpcClient, + ManagedConfigReconcileHint, SessionConfigClient, SessionData, SessionRpcClient, SessionRpcService, }; use crate::client_manager::{ @@ -79,23 +79,23 @@ pub(super) async fn reconcile_network_configs_on_heartbeat( RoundStatus::Skip => continue, RoundStatus::Stop => return, }; - if cache.runtime_config_epoch != round.runtime_config_epoch { - cache = ReconcileCache { - runtime_config_epoch: round.runtime_config_epoch, - ..Default::default() - }; - } + cache.reset_if_runtime_config_cache_epoch_changed(round.runtime_config_cache_epoch); let running_metas = match sync_running_sources_for_round(&mut rpc_client, &storage, &mut round).await { RoundStatus::Ready(running_metas) => running_metas, RoundStatus::Skip => continue, RoundStatus::Stop => return, }; - let mut mutation_fence = RuntimeMutationFence::default(); let context = ReconcileRoundContext { session_data: &session_data, round: &round, }; + match cleanup_direct_run_failures_for_round(&context).await { + RoundStatus::Ready(()) => {} + RoundStatus::Skip => continue, + RoundStatus::Stop => return, + } + let mut mutation_fence = RuntimeMutationFence::default(); let mut outcome = match &round.scope { ReconcileScope::Full => { @@ -118,16 +118,13 @@ pub(super) async fn reconcile_network_configs_on_heartbeat( RoundStatus::Stop => return, } } - ReconcileScope::Patch { - delete_instance_ids, - .. - } => { + ReconcileScope::Patch { .. } => { match cleanup_patch_deleted_instances( &session_data, &mut rpc_client, &round, running_metas.as_deref(), - delete_instance_ids, + &round.delete_instance_ids, &mut cache, &mut mutation_fence, ) @@ -158,13 +155,15 @@ pub(super) async fn reconcile_network_configs_on_heartbeat( managed_config::desired_web_source_instance_ids(&round.local_configs), ); } - ReconcileScope::Patch { - upsert_instance_ids, - delete_instance_ids, - } => { + ReconcileScope::Patch { dirty_instance_ids } => { if let Some(last) = &mut cache.last_desired_web_inst_ids { - last.retain(|id| !delete_instance_ids.contains(id)); - last.extend(upsert_instance_ids.iter().cloned()); + last.retain(|id| !dirty_instance_ids.contains(id)); + last.extend( + round + .local_configs + .iter() + .map(|config| config.network_instance_id.clone()), + ); } } } @@ -191,6 +190,7 @@ enum RoundStatus { Stop, } +#[derive(Clone, Copy, PartialEq, Eq)] enum ConfigActionResult { Success, Failed, @@ -199,12 +199,24 @@ enum ConfigActionResult { #[derive(Default)] struct ReconcileCache { - runtime_config_epoch: u64, + runtime_config_cache_epoch: u64, cleaned_web_source_instances: bool, last_desired_web_inst_ids: Option>, runtime_configs: SessionRuntimeConfigCache, } +impl ReconcileCache { + fn reset_if_runtime_config_cache_epoch_changed(&mut self, current_epoch: u64) { + if self.runtime_config_cache_epoch == current_epoch { + return; + } + *self = Self { + runtime_config_cache_epoch: current_epoch, + ..Default::default() + }; + } +} + #[derive(Default)] struct SessionRuntimeConfigCache { entries: HashMap, @@ -276,10 +288,12 @@ struct ReconcileRound { user_id: i32, running_inst_ids: HashSet, local_configs: Vec, + delete_instance_ids: HashSet, target_config_revision: Option, should_apply_runtime_revision: bool, scope: ReconcileScope, runtime_config_epoch: u64, + runtime_config_cache_epoch: u64, } struct ReconcileRoundContext<'a> { @@ -290,25 +304,24 @@ struct ReconcileRoundContext<'a> { #[derive(Debug, Clone, PartialEq, Eq)] enum ReconcileScope { Full, - Patch { - upsert_instance_ids: HashSet, - delete_instance_ids: HashSet, - }, + Patch { dirty_instance_ids: HashSet }, } fn select_reconcile_scope( - applied_revision: Option<&str>, + pending: Option<&ManagedConfigReconcileHint>, + known_runtime_base_revision: Option<&str>, target_revision: Option<&str>, - pending_delta: Option<&ManagedConfigRevisionDelta>, ) -> ReconcileScope { - match pending_delta { - Some(delta) - if applied_revision == Some(delta.expected_revision.as_str()) - && target_revision == Some(delta.target_revision.as_str()) => + match pending { + Some(ManagedConfigReconcileHint::Dirty { + expected_revision, + target_revision: dirty_target, + instance_ids, + }) if known_runtime_base_revision == Some(expected_revision.as_str()) + && target_revision == Some(dirty_target.as_str()) => { ReconcileScope::Patch { - upsert_instance_ids: delta.upsert_instance_ids.clone(), - delete_instance_ids: delta.delete_instance_ids.clone(), + dirty_instance_ids: instance_ids.clone(), } } _ => ReconcileScope::Full, @@ -337,44 +350,53 @@ async fn prepare_reconcile_round( { Ok(Some(user_id)) => user_id, Ok(None) => { - tracing::info!("User not found by token: {:?}", req.user_token); - return RoundStatus::Stop; + tracing::info!( + machine_id = ?req.machine_id, + "user not found by heartbeat token" + ); + return RoundStatus::Skip; } Err(e) => { tracing::error!("Failed to get user id by token, error: {:?}", e); - return RoundStatus::Stop; + return RoundStatus::Skip; } }; - let (applied_config_revision, pending_delta, runtime_config_epoch) = { + let ( + applied_config_revision, + known_runtime_base_revision, + pending_reconcile, + runtime_config_epoch, + runtime_config_cache_epoch, + failed_instance_ids, + ) = { let Some(data) = session_data.upgrade() else { return RoundStatus::Stop; }; let data = data.read().await; + let runtime = data.managed_runtime(); ( - data.applied_config_revision.clone(), - data.pending_managed_config_delta.clone(), - data.runtime_config_epoch, + runtime.applied_config_revision.clone(), + runtime.known_runtime_base_revision.clone(), + runtime.pending_managed_config_reconcile.clone(), + runtime.runtime_config_epoch, + runtime.runtime_config_cache_epoch, + SessionRpcService::failed_instance_ids_locked(&data), ) }; - let target_config_revision = match storage - .db - .get_managed_config_revision((user_id, machine_id)) - .await - { - Ok(revision) => revision, - Err(e) => { - tracing::error!("Failed to read managed config revision, error: {:?}", e); - return RoundStatus::Stop; - } - }; + let target_config_revision = + match read_managed_config_revision(storage, user_id, machine_id).await { + RoundStatus::Ready(revision) => revision, + RoundStatus::Skip => return RoundStatus::Skip, + RoundStatus::Stop => return RoundStatus::Stop, + }; let should_apply_runtime_revision = target_config_revision.is_some() && target_config_revision != applied_config_revision; let mut scope = if should_apply_runtime_revision { select_reconcile_scope( - applied_config_revision.as_deref(), + pending_reconcile.as_ref(), + known_runtime_base_revision.as_deref(), target_config_revision.as_deref(), - pending_delta.as_ref(), ) } else { ReconcileScope::Full @@ -385,6 +407,7 @@ async fn prepare_reconcile_round( user_id, machine_id, should_apply_runtime_revision, + &failed_instance_ids, ) .await { @@ -393,13 +416,17 @@ async fn prepare_reconcile_round( RoundStatus::Stop => return RoundStatus::Stop, }; - let local_configs = match load_round_configs(storage, user_id, machine_id, &scope).await { + let (local_configs, delete_instance_ids) = match load_round_configs( + storage, user_id, machine_id, &scope, + ) + .await + { Ok(Some(configs)) => configs, Ok(None) => { tracing::warn!( ?user_id, ?machine_id, - "Managed config Patch no longer matches persisted rows; using Full reconcile" + "Managed config dirty instance is no longer a web-owned row; using Full reconcile" ); scope = ReconcileScope::Full; match storage @@ -407,16 +434,16 @@ async fn prepare_reconcile_round( .list_network_configs((user_id, machine_id), ListNetworkProps::EnabledOnly) .await { - Ok(configs) => configs, + Ok(configs) => (configs, HashSet::new()), Err(e) => { tracing::error!("Failed to list network configs, error: {:?}", e); - return RoundStatus::Stop; + return RoundStatus::Skip; } } } Err(e) => { tracing::error!("Failed to load managed config Patch rows, error: {:?}", e); - return RoundStatus::Stop; + return RoundStatus::Skip; } }; @@ -426,50 +453,122 @@ async fn prepare_reconcile_round( user_id, running_inst_ids, local_configs, + delete_instance_ids, target_config_revision, should_apply_runtime_revision, scope, runtime_config_epoch, + runtime_config_cache_epoch, }) } +async fn update_direct_run_failures_if_current( + session_data: &std::sync::Weak>, + round: &ReconcileRound, + update: impl FnOnce(&mut SessionData) -> Option>, +) -> RoundStatus<()> { + let Some(data) = session_data.upgrade() else { + return RoundStatus::Stop; + }; + let notify = { + let mut data = data.write().await; + if !SessionRpcService::runtime_heartbeat_is_current_locked(&data, &round.req) { + return RoundStatus::Skip; + } + if data.managed_runtime().runtime_config_epoch != round.runtime_config_epoch { + return RoundStatus::Skip; + } + update(&mut data) + }; + if let Some(notify) = notify { + notify.notify_one(); + } + RoundStatus::Ready(()) +} + +async fn cleanup_direct_run_failures_for_round( + context: &ReconcileRoundContext<'_>, +) -> RoundStatus<()> { + let desired_instance_ids = + managed_config::desired_web_source_instance_ids(&context.round.local_configs); + update_direct_run_failures_if_current( + context.session_data, + context.round, + |data| match &context.round.scope { + ReconcileScope::Full => { + SessionRpcService::retain_direct_run_failures_locked(data, &desired_instance_ids) + } + ReconcileScope::Patch { .. } => SessionRpcService::remove_direct_run_failures_locked( + data, + &context.round.delete_instance_ids, + ), + }, + ) + .await +} + +async fn read_managed_config_revision( + storage: &StorageInner, + user_id: i32, + machine_id: uuid::Uuid, +) -> RoundStatus> { + match storage + .db + .get_managed_config_revision((user_id, machine_id)) + .await + { + Ok(revision) => RoundStatus::Ready(revision), + Err(e) => { + tracing::error!("Failed to read managed config revision, error: {:?}", e); + RoundStatus::Skip + } + } +} + async fn load_round_configs( storage: &StorageInner, user_id: i32, machine_id: uuid::Uuid, scope: &ReconcileScope, -) -> Result>, sea_orm::DbErr> { - let ReconcileScope::Patch { - upsert_instance_ids, - .. - } = scope - else { +) -> Result< + Option<( + Vec, + HashSet, + )>, + sea_orm::DbErr, +> { + let ReconcileScope::Patch { dirty_instance_ids } = scope else { return storage .db .list_network_configs((user_id, machine_id), ListNetworkProps::EnabledOnly) .await - .map(Some); + .map(|configs| Some((configs, HashSet::new()))); }; - let mut instance_ids = upsert_instance_ids.iter().collect::>(); + let mut instance_ids = dirty_instance_ids.iter().collect::>(); instance_ids.sort_unstable(); let mut configs = Vec::with_capacity(instance_ids.len()); + let mut delete_instance_ids = HashSet::new(); for instance_id in instance_ids { - let Some(config) = storage + let config = storage .db .get_network_config((user_id, machine_id), instance_id) - .await? - else { - return Ok(None); - }; - if config.disabled - || PersistedConfigSource::from_db(&config.source) != PersistedConfigSource::Web - { - return Ok(None); + .await?; + match config { + Some(config) + if !config.disabled + && PersistedConfigSource::from_db(&config.source) + == PersistedConfigSource::Web => + { + configs.push(config); + } + None => { + delete_instance_ids.insert(instance_id.clone()); + } + Some(_) => return Ok(None), } - configs.push(config); } - Ok(Some(configs)) + Ok(Some((configs, delete_instance_ids))) } async fn running_instance_ids_for_round( @@ -478,31 +577,38 @@ async fn running_instance_ids_for_round( user_id: i32, machine_id: uuid::Uuid, should_apply_runtime_revision: bool, + failed_instance_ids: &HashSet, ) -> RoundStatus> { - if !should_apply_runtime_revision { - return RoundStatus::Ready( - req.running_network_instances - .iter() - .map(|x| x.to_string()) - .collect(), - ); - } - - match rpc_client - .list_network_instance(BaseController::default(), ListNetworkInstanceRequest {}) - .await - { - Ok(resp) => RoundStatus::Ready(resp.inst_ids.iter().map(|x| x.to_string()).collect()), - Err(error) => { - tracing::warn!( - ?user_id, - ?machine_id, - ?error, - "Failed to refresh running instances for managed config revision" - ); - RoundStatus::Skip + // Both sources must agree on which instances are running: instances + // known to have failed are excluded so the reconciler restarts them + // instead of hot-patching a stopped instance forever. + let ids = if !should_apply_runtime_revision { + req.running_network_instances + .iter() + .map(|x| x.to_string()) + .collect::>() + } else { + match rpc_client + .list_network_instance(BaseController::default(), ListNetworkInstanceRequest {}) + .await + { + Ok(resp) => resp.inst_ids.iter().map(|x| x.to_string()).collect(), + Err(error) => { + tracing::warn!( + ?user_id, + ?machine_id, + ?error, + "Failed to refresh running instances for managed config revision" + ); + return RoundStatus::Skip; + } } - } + }; + RoundStatus::Ready( + ids.into_iter() + .filter(|id| !failed_instance_ids.contains(id)) + .collect(), + ) } async fn sync_running_sources_for_round( @@ -562,7 +668,7 @@ async fn sync_running_sources_for_round( "Failed to reload network configs after source sync, error: {:?}", e ); - return RoundStatus::Stop; + return RoundStatus::Skip; } }; } @@ -606,7 +712,7 @@ async fn cleanup_stale_web_source_instances( Ok(configs) => managed_config::desired_web_source_instance_ids(&configs), Err(e) => { tracing::error!("Failed to list all network configs, error: {:?}", e); - return RoundStatus::Stop; + return RoundStatus::Skip; } }; @@ -631,6 +737,7 @@ async fn cleanup_stale_web_source_instances( ); return RoundStatus::Skip; } + let operation_started_at = std::time::Instant::now(); let ret = rpc_client .delete_network_instance( BaseController::default(), @@ -641,9 +748,10 @@ async fn cleanup_stale_web_source_instances( .await; tracing::info!( user_id = ?round.user_id, - "Clean stale web-source network instances on heartbeat: {:?}, user_token: {:?}", - ret, - round.req.user_token + machine_id = ?round.machine_id, + elapsed_ms = operation_started_at.elapsed().as_millis(), + "Clean stale web-source network instances on heartbeat: {:?}", + ret ); match ret { Err(_) => outcome.record_failure(true), @@ -711,6 +819,7 @@ async fn cleanup_patch_deleted_instances( return RoundStatus::Skip; } + let operation_started_at = std::time::Instant::now(); let ret = rpc_client .delete_network_instance( BaseController::default(), @@ -723,6 +832,8 @@ async fn cleanup_patch_deleted_instances( .await; tracing::info!( user_id = ?round.user_id, + machine_id = ?round.machine_id, + elapsed_ms = operation_started_at.elapsed().as_millis(), deleted_instance_ids = ?running_web_instance_ids, "Apply managed config Patch deletions at runtime: {:?}", ret @@ -760,15 +871,23 @@ async fn begin_managed_runtime_mutation( let Some(data) = session_data.upgrade() else { return false; }; - let mut data = data.write().await; - if !SessionRpcService::runtime_heartbeat_is_current_locked(&data, &round.req) - || data.runtime_config_epoch != round.runtime_config_epoch - { + let data = data.write().await; + if !SessionRpcService::runtime_heartbeat_is_current_locked(&data, &round.req) { + return false; + } + let managed_runtime = data.managed_runtime.clone(); + let mut runtime = managed_runtime + .lock() + .expect("managed runtime state lock poisoned"); + if runtime.runtime_config_epoch != round.runtime_config_epoch { return false; } if !mutation_fence.started { - data.applied_config_revision = None; - data.pending_managed_config_delta = None; + runtime.applied_config_revision = None; + runtime.applied_config_revision_known = true; + if matches!(round.scope, ReconcileScope::Full) { + runtime.known_runtime_base_revision = None; + } mutation_fence.started = true; } true @@ -786,6 +905,19 @@ fn retained_requested_instance_ids( .collect() } +fn should_reconcile_running_web_config( + is_running: bool, + source: PersistedConfigSource, + round: &ReconcileRound, +) -> bool { + is_running + && source == PersistedConfigSource::Web + // Legacy consoles update web configs without a revision. With no + // revision to compare against, running web configs are checked + // every round so unrevisioned changes still converge. + && (round.should_apply_runtime_revision || round.target_config_revision.is_none()) +} + async fn reconcile_desired_runtime_configs( context: &ReconcileRoundContext<'_>, rpc_client: &mut SessionRpcClient, @@ -804,9 +936,8 @@ async fn reconcile_desired_runtime_configs( for config in &round.local_configs { let source = PersistedConfigSource::from_db(&config.source); let is_running = round.running_inst_ids.contains(&config.network_instance_id); - let should_reconcile_running_web_config = is_running - && round.should_apply_runtime_revision - && source == PersistedConfigSource::Web; + let should_reconcile_running_web_config = + should_reconcile_running_web_config(is_running, source, round); if is_running && !should_reconcile_running_web_config { continue; } @@ -921,6 +1052,7 @@ async fn reconcile_running_web_config( return ConfigActionResult::StopRound; } + let operation_started_at = std::time::Instant::now(); let ret = async { let action = match runtime_config_cache.plan(&config.network_instance_id, desired_config.clone())? { @@ -938,8 +1070,10 @@ async fn reconcile_running_web_config( if !SessionRpcService::runtime_heartbeat_is_current(session_data, &round.req).await { anyhow::bail!("webhook session is no longer current before runtime reconcile apply"); } - if !matches!(action, runtime_reconcile::RuntimeReconcileAction::None) - && !begin_managed_runtime_mutation(session_data, round, mutation_fence).await + if !matches!( + action, + runtime_reconcile::RuntimeReconcileAction::Unchanged(_) + ) && !begin_managed_runtime_mutation(session_data, round, mutation_fence).await { anyhow::bail!("managed runtime mutation fence is no longer current"); } @@ -957,10 +1091,11 @@ async fn reconcile_running_web_config( .await; tracing::info!( user_id = ?round.user_id, + machine_id = ?round.machine_id, instance_id = %config.network_instance_id, - "Reconcile running web-source network instance: {:?}, user_token: {:?}", - ret, - round.req.user_token + elapsed_ms = operation_started_at.elapsed().as_millis(), + "Reconcile running web-source network instance: {:?}", + ret ); if ret.is_ok() { @@ -1000,6 +1135,7 @@ async fn run_missing_network_config( return ConfigActionResult::StopRound; } + let operation_started_at = std::time::Instant::now(); let ret = rpc_client .run_network_instance( BaseController::default(), @@ -1013,16 +1149,40 @@ async fn run_missing_network_config( .await; tracing::info!( user_id = ?round.user_id, - "Run network instance: {:?}, user_token: {:?}", - ret, - round.req.user_token + machine_id = ?round.machine_id, + instance_id = %config.network_instance_id, + elapsed_ms = operation_started_at.elapsed().as_millis(), + "Run network instance: {:?}", + ret ); - if ret.is_ok() { + let action_result = if ret.is_ok() { ConfigActionResult::Success } else { ConfigActionResult::Failed + }; + if source == PersistedConfigSource::Web { + record_direct_run_result( + session_data, + round, + &config.network_instance_id, + matches!(action_result, ConfigActionResult::Failed), + ) + .await; } + action_result +} + +async fn record_direct_run_result( + session_data: &std::sync::Weak>, + round: &ReconcileRound, + instance_id: &str, + failed: bool, +) { + let _ = update_direct_run_failures_if_current(session_data, round, |data| { + SessionRpcService::update_direct_run_failure_locked(data, instance_id, failed) + }) + .await; } async fn remember_web_runtime_config_after_run( @@ -1050,7 +1210,10 @@ fn remember_if_runtime_matches_desired( &observed_config, desired_config.clone(), )?; - if !matches!(action, runtime_reconcile::RuntimeReconcileAction::None) { + if !matches!( + action, + runtime_reconcile::RuntimeReconcileAction::Unchanged(_) + ) { anyhow::bail!("runtime config still differs after managed run"); } runtime_config_cache.remember(inst_id, observed_config); @@ -1070,17 +1233,12 @@ async fn mark_config_revision_applied_if_current( return RoundStatus::Ready(()); } - let current_target_config_revision = match storage - .db - .get_managed_config_revision((round.user_id, round.machine_id)) - .await - { - Ok(revision) => revision, - Err(e) => { - tracing::error!("Failed to verify managed config revision, error: {:?}", e); - return RoundStatus::Stop; - } - }; + let current_target_config_revision = + match read_managed_config_revision(storage, round.user_id, round.machine_id).await { + RoundStatus::Ready(revision) => revision, + RoundStatus::Skip => return RoundStatus::Skip, + RoundStatus::Stop => return RoundStatus::Stop, + }; if current_target_config_revision != round.target_config_revision { return RoundStatus::Ready(()); } @@ -1092,10 +1250,11 @@ async fn mark_config_revision_applied_if_current( if !SessionRpcService::runtime_heartbeat_is_current_locked(&data, &round.req) { return RoundStatus::Ready(()); } - if data.runtime_config_epoch != round.runtime_config_epoch { - return RoundStatus::Ready(()); - } - record_applied_config_revision(&mut data, round.target_config_revision.clone()) + record_applied_config_revision( + &mut data, + Some(round.runtime_config_epoch), + round.target_config_revision.clone(), + ) }; if let Some(notify) = notify { notify.notify_one(); @@ -1106,12 +1265,34 @@ async fn mark_config_revision_applied_if_current( fn record_applied_config_revision( data: &mut SessionData, + expected_runtime_config_epoch: Option, revision: Option, ) -> Option> { - let changed = data.applied_config_revision != revision; - data.applied_config_revision = revision; - data.pending_managed_config_delta = None; - changed.then(|| SessionRpcService::mark_webhook_validation_dirty_locked(data)) + let managed_runtime = data.managed_runtime.clone(); + let mut runtime = managed_runtime + .lock() + .expect("managed runtime state lock poisoned"); + if expected_runtime_config_epoch + .is_some_and(|expected| runtime.runtime_config_epoch != expected) + { + return None; + } + let changed = + !runtime.applied_config_revision_known || runtime.applied_config_revision != revision; + if changed { + tracing::info!( + machine_id = ?data.req.as_ref().and_then(|req| req.machine_id), + previous_revision = ?runtime.applied_config_revision, + applied_revision = ?revision, + "managed config revision applied" + ); + } + runtime.known_runtime_base_revision = revision.clone(); + runtime.applied_config_revision = revision; + runtime.applied_config_revision_known = true; + runtime.pending_managed_config_reconcile = None; + drop(runtime); + changed.then(|| SessionRpcService::mark_webhook_validation_state_changed_locked(data)) } #[cfg(test)] @@ -1142,6 +1323,100 @@ mod tests { } } + #[tokio::test] + async fn full_and_patch_deletes_cleanup_direct_run_failures() { + let storage = + crate::client_manager::storage::Storage::new(crate::db::Db::memory_db().await); + let mut data = SessionData::new( + storage.weak_ref(), + url::Url::parse("http://127.0.0.1").unwrap(), + None, + std::sync::Arc::new(crate::FeatureFlags::default()), + std::sync::Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ); + let retained = uuid::Uuid::new_v4().to_string(); + let removed_by_full = uuid::Uuid::new_v4().to_string(); + let removed_by_patch = uuid::Uuid::new_v4().to_string(); + data.direct_run_failed_instance_ids = + HashSet::from([retained.clone(), removed_by_full, removed_by_patch.clone()]); + + SessionRpcService::retain_direct_run_failures_locked( + &mut data, + &HashSet::from([retained.clone(), removed_by_patch.clone()]), + ); + assert_eq!( + data.direct_run_failed_instance_ids, + HashSet::from([retained.clone(), removed_by_patch.clone()]) + ); + + SessionRpcService::remove_direct_run_failures_locked( + &mut data, + &HashSet::from([removed_by_patch]), + ); + assert_eq!( + data.direct_run_failed_instance_ids, + HashSet::from([retained]) + ); + } + + #[tokio::test] + async fn managed_revision_read_failure_retries_on_a_later_round() { + let storage = + crate::client_manager::storage::Storage::new(crate::db::Db::memory_db().await); + let user_id = storage.db().auto_create_user("token").await.unwrap().id; + let machine_id = uuid::Uuid::new_v4(); + let pool = storage.db().inner(); + sqlx::query("DROP TABLE managed_config_revisions") + .execute(&pool) + .await + .unwrap(); + let storage_inner = storage.weak_ref().upgrade().unwrap(); + + assert!(matches!( + read_managed_config_revision(&storage_inner, user_id, machine_id).await, + RoundStatus::Skip + )); + + sqlx::query( + r#" + CREATE TABLE managed_config_revisions ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + user_id INTEGER NOT NULL, + device_id TEXT NOT NULL, + config_revision TEXT NOT NULL, + create_time TEXT NOT NULL, + update_time TEXT NOT NULL, + CONSTRAINT fk_managed_config_revisions_user_id_to_users_id + FOREIGN KEY (user_id) REFERENCES users(id) + ON DELETE CASCADE + ON UPDATE CASCADE + ) + "#, + ) + .execute(&pool) + .await + .unwrap(); + sqlx::query( + "CREATE UNIQUE INDEX idx_managed_config_revisions_scope \ + ON managed_config_revisions(user_id, device_id)", + ) + .execute(&pool) + .await + .unwrap(); + storage + .db() + .set_managed_config_revision((user_id, machine_id), "rev-recovered") + .await + .unwrap(); + + assert!(matches!( + read_managed_config_revision(&storage_inner, user_id, machine_id).await, + RoundStatus::Ready(Some(revision)) if revision == "rev-recovered" + )); + } + #[tokio::test] async fn newly_applied_revision_wakes_webhook_validation() { let storage = @@ -1155,11 +1430,31 @@ mod tests { None, None, None, None, None, )), ); + data.managed_runtime().pending_managed_config_reconcile = + Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["managed".to_string()]), + }); - let notify = record_applied_config_revision(&mut data, Some("rev-applied".to_string())) - .expect("new applied revision should wake validation"); - assert_eq!(data.applied_config_revision.as_deref(), Some("rev-applied")); + let notify = + record_applied_config_revision(&mut data, None, Some("rev-applied".to_string())) + .expect("new applied revision should wake validation"); + { + let runtime = data.managed_runtime(); + assert_eq!( + runtime.applied_config_revision.as_deref(), + Some("rev-applied") + ); + assert!(runtime.applied_config_revision_known); + assert_eq!( + runtime.known_runtime_base_revision.as_deref(), + Some("rev-applied") + ); + assert_eq!(runtime.pending_managed_config_reconcile, None); + } assert!(data.webhook_validation_dirty); + assert_eq!(data.webhook_validation_change_epoch, 1); notify.notify_one(); tokio::time::timeout(std::time::Duration::from_millis(100), notify.notified()) @@ -1180,14 +1475,53 @@ mod tests { None, None, None, None, None, )), ); - data.applied_config_revision = Some("rev-applied".to_string()); + { + let mut runtime = data.managed_runtime(); + runtime.applied_config_revision = Some("rev-applied".to_string()); + runtime.applied_config_revision_known = true; + } assert!( - record_applied_config_revision(&mut data, Some("rev-applied".to_string())).is_none() + record_applied_config_revision(&mut data, None, Some("rev-applied".to_string())) + .is_none() + ); + assert_eq!( + data.managed_runtime() + .known_runtime_base_revision + .as_deref(), + Some("rev-applied") ); assert!(!data.webhook_validation_dirty); } + #[tokio::test] + async fn stale_round_cannot_record_applied_revision() { + let storage = + crate::client_manager::storage::Storage::new(crate::db::Db::memory_db().await); + let mut data = SessionData::new( + storage.weak_ref(), + url::Url::parse("http://127.0.0.1").unwrap(), + None, + std::sync::Arc::new(crate::FeatureFlags::default()), + std::sync::Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ); + { + let mut runtime = data.managed_runtime(); + runtime.applied_config_revision = Some("rev-a".to_string()); + runtime.applied_config_revision_known = true; + runtime.runtime_config_epoch = 2; + } + + assert!( + record_applied_config_revision(&mut data, Some(1), Some("rev-b".to_string())).is_none() + ); + let runtime = data.managed_runtime(); + assert_eq!(runtime.applied_config_revision.as_deref(), Some("rev-a")); + assert_eq!(runtime.runtime_config_epoch, 2); + } + #[test] fn patch_delete_requires_runtime_to_remove_every_requested_instance() { let deleted_id = uuid::Uuid::new_v4(); @@ -1214,7 +1548,7 @@ mod tests { } #[tokio::test] - async fn managed_runtime_mutation_clears_old_applied_revision_before_side_effects() { + async fn managed_runtime_mutation_preserves_base_only_for_patch_scope() { let machine_id = uuid::Uuid::new_v4(); let req = HeartbeatRequest { user_token: "token".to_string(), @@ -1233,28 +1567,42 @@ mod tests { None, None, None, None, None, )), ); - data.storage_token = Some(crate::client_manager::storage::StorageToken { + let storage_token = crate::client_manager::storage::StorageToken { token: req.user_token.clone(), client_url, machine_id, user_id: 7, - }); + }; + storage.update_session_client(storage_token.clone(), 1, true, 0); + data.storage_token = Some(storage_token); data.req = Some(req.clone()); data.auth_state = super::super::SessionAuthState::Authorized; - data.applied_config_revision = Some("rev-a".to_string()); - data.pending_managed_config_delta = Some(revision_delta("rev-a", "rev-b")); - data.runtime_config_epoch = 11; + { + let mut runtime = data.managed_runtime(); + runtime.applied_config_revision = Some("rev-a".to_string()); + runtime.known_runtime_base_revision = Some("rev-a".to_string()); + runtime.pending_managed_config_reconcile = Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["managed".to_string()]), + }); + runtime.runtime_config_epoch = 11; + } let session_data = std::sync::Arc::new(RwLock::new(data)); - let round = ReconcileRound { + let mut round = ReconcileRound { req, machine_id, user_id: 7, running_inst_ids: HashSet::new(), local_configs: Vec::new(), + delete_instance_ids: HashSet::new(), target_config_revision: Some("rev-b".to_string()), should_apply_runtime_revision: true, - scope: ReconcileScope::Full, + scope: ReconcileScope::Patch { + dirty_instance_ids: HashSet::from(["managed".to_string()]), + }, runtime_config_epoch: 11, + runtime_config_cache_epoch: 0, }; let mut mutation_fence = RuntimeMutationFence::default(); @@ -1267,53 +1615,226 @@ mod tests { .await ); - let data = session_data.read().await; - assert!(mutation_fence.started); - assert_eq!(data.applied_config_revision, None); - assert_eq!(data.pending_managed_config_delta, None); - assert_eq!(data.runtime_config_epoch, 11); - } + { + let data = session_data.read().await; + let runtime = data.managed_runtime(); + assert!(mutation_fence.started); + assert_eq!(runtime.applied_config_revision, None); + assert!(runtime.applied_config_revision_known); + assert_eq!( + runtime.known_runtime_base_revision.as_deref(), + Some("rev-a") + ); + assert_eq!( + runtime.pending_managed_config_reconcile, + Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["managed".to_string()]), + }) + ); + assert_eq!(runtime.runtime_config_epoch, 11); + assert_eq!( + select_reconcile_scope( + runtime.pending_managed_config_reconcile.as_ref(), + runtime.known_runtime_base_revision.as_deref(), + Some("rev-b"), + ), + ReconcileScope::Patch { + dirty_instance_ids: HashSet::from(["managed".to_string()]), + } + ); + } - fn revision_delta(base: &str, target: &str) -> ManagedConfigRevisionDelta { - ManagedConfigRevisionDelta { - expected_revision: base.to_string(), - target_revision: target.to_string(), - upsert_instance_ids: HashSet::from(["upsert".to_string()]), - delete_instance_ids: HashSet::from(["delete".to_string()]), + { + let data = session_data.write().await; + data.managed_runtime().applied_config_revision = Some("rev-a".to_string()); + } + round.scope = ReconcileScope::Full; + let mut mutation_fence = RuntimeMutationFence::default(); + assert!( + begin_managed_runtime_mutation( + &std::sync::Arc::downgrade(&session_data), + &round, + &mut mutation_fence, + ) + .await + ); + + { + let data = session_data.read().await; + let runtime = data.managed_runtime(); + assert_eq!(runtime.applied_config_revision, None); + assert!(runtime.applied_config_revision_known); + assert_eq!(runtime.known_runtime_base_revision, None); } } #[test] - fn exact_revision_delta_selects_targeted_reconcile() { - let delta = revision_delta("rev-1", "rev-2"); - + fn dirty_hint_without_known_runtime_base_uses_full_reconcile() { assert_eq!( - select_reconcile_scope(Some("rev-1"), Some("rev-2"), Some(&delta)), + select_reconcile_scope( + Some(&ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["upsert".to_string(), "delete".to_string(),]), + }), + None, + Some("rev-b"), + ), + ReconcileScope::Full + ); + } + + #[test] + fn matching_known_runtime_base_and_target_select_dirty_instances() { + assert_eq!( + select_reconcile_scope( + Some(&ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: HashSet::from(["upsert".to_string(), "delete".to_string(),]), + }), + Some("rev-a"), + Some("rev-b"), + ), ReconcileScope::Patch { - upsert_instance_ids: HashSet::from(["upsert".to_string()]), - delete_instance_ids: HashSet::from(["delete".to_string()]), + dirty_instance_ids: HashSet::from(["upsert".to_string(), "delete".to_string()]), } ); } #[test] - fn revision_gap_uses_full_reconcile() { - let delta = revision_delta("rev-1", "rev-2"); + fn mismatched_known_runtime_base_uses_full_reconcile() { + let hint = ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-b".to_string(), + target_revision: "rev-c".to_string(), + instance_ids: HashSet::from(["managed".to_string()]), + }; assert_eq!( - select_reconcile_scope(Some("older"), Some("rev-2"), Some(&delta)), + select_reconcile_scope(Some(&hint), Some("rev-a"), Some("rev-c")), + ReconcileScope::Full + ); + } + + #[test] + fn missing_or_full_hint_uses_full_reconcile() { + assert_eq!( + select_reconcile_scope( + Some(&ManagedConfigReconcileHint::Full), + Some("rev-a"), + Some("rev-b"), + ), ReconcileScope::Full ); assert_eq!( - select_reconcile_scope(Some("rev-1"), Some("newer"), Some(&delta)), + select_reconcile_scope(None, Some("rev-a"), Some("rev-b")), ReconcileScope::Full ); + } + + #[test] + fn dirty_hint_for_older_target_uses_full_reconcile() { + let hint = ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-0".to_string(), + target_revision: "rev-a".to_string(), + instance_ids: HashSet::from(["managed".to_string()]), + }; + assert_eq!( - select_reconcile_scope(Some("rev-1"), Some("rev-2"), None), + select_reconcile_scope(Some(&hint), Some("rev-0"), Some("rev-b")), ReconcileScope::Full ); } + #[test] + fn managed_revision_change_preserves_runtime_config_cache() { + let mut cache = ReconcileCache::default(); + cache.runtime_configs.remember( + "managed", + config_with_port_forwards(vec![port_forward(23000, 5174)]), + ); + + cache.reset_if_runtime_config_cache_epoch_changed(0); + + assert!(cache.runtime_configs.entries.contains_key("managed")); + } + + #[test] + fn direct_runtime_mutation_invalidates_runtime_config_cache() { + let mut cache = ReconcileCache::default(); + cache.runtime_configs.remember( + "managed", + config_with_port_forwards(vec![port_forward(23000, 5174)]), + ); + + cache.reset_if_runtime_config_cache_epoch_changed(1); + + assert!(!cache.runtime_configs.entries.contains_key("managed")); + assert_eq!(cache.runtime_config_cache_epoch, 1); + } + + #[tokio::test] + async fn patch_scope_reads_latest_persisted_state_for_dirty_instances() { + let storage = + crate::client_manager::storage::Storage::new(crate::db::Db::memory_db().await); + let user_id = storage.db().auto_create_user("token").await.unwrap().id; + let machine_id = uuid::Uuid::new_v4(); + let persisted_id = uuid::Uuid::new_v4(); + let missing_id = uuid::Uuid::new_v4(); + crate::client_manager::managed_config::reconcile_web_source_configs( + &storage, + user_id, + machine_id, + vec![crate::webhook::ManagedNetworkConfig { + instance_id: persisted_id.to_string(), + network_config: serde_json::to_value(config_with_port_forwards(Vec::new())) + .unwrap(), + }], + Some("rev-1"), + crate::client_manager::managed_config::ExpectedConfigRevision::Any, + ) + .await + .unwrap(); + let scope = ReconcileScope::Patch { + dirty_instance_ids: HashSet::from([persisted_id.to_string(), missing_id.to_string()]), + }; + + let storage_inner = storage.weak_ref().upgrade().unwrap(); + let (configs, deleted) = load_round_configs(&storage_inner, user_id, machine_id, &scope) + .await + .unwrap() + .unwrap(); + + assert_eq!(configs.len(), 1); + assert_eq!(configs[0].network_instance_id, persisted_id.to_string()); + assert_eq!(deleted, HashSet::from([missing_id.to_string()])); + + crate::client_manager::managed_config::patch_web_source_configs( + &storage, + user_id, + machine_id, + Vec::new(), + vec![persisted_id], + "rev-2", + "rev-1", + ) + .await + .unwrap(); + + let (configs, deleted) = load_round_configs(&storage_inner, user_id, machine_id, &scope) + .await + .unwrap() + .unwrap(); + + assert!(configs.is_empty()); + assert_eq!( + deleted, + HashSet::from([persisted_id.to_string(), missing_id.to_string()]) + ); + } + #[test] fn session_runtime_config_cache_misses_unknown_instance() { let cache = SessionRuntimeConfigCache::default(); @@ -1337,10 +1858,41 @@ mod tests { assert!(matches!( action, - runtime_reconcile::RuntimeReconcileAction::None + runtime_reconcile::RuntimeReconcileAction::Unchanged(_) )); } + #[test] + fn cache_preserves_ignored_runtime_hostname_for_later_explicit_clear() { + let mut cache = SessionRuntimeConfigCache::default(); + let mut observed = config_with_port_forwards(Vec::new()); + observed.hostname = Some("runtime-host".to_string()); + cache.remember("managed", observed); + + let unmanaged_desired = config_with_port_forwards(Vec::new()); + let action = cache + .plan("managed", unmanaged_desired) + .expect("prepare unmanaged hostname action") + .expect("cached action"); + let runtime_reconcile::RuntimeReconcileAction::Unchanged(observed) = action else { + panic!("unmanaged hostname should preserve the observed config"); + }; + assert_eq!(observed.hostname.as_deref(), Some("runtime-host")); + cache.remember("managed", *observed); + + let mut explicit_clear = config_with_port_forwards(Vec::new()); + explicit_clear.hostname = Some(String::new()); + let action = cache + .plan("managed", explicit_clear) + .expect("prepare explicit clear action") + .expect("cached action"); + let runtime_reconcile::RuntimeReconcileAction::Patch(patch) = action else { + panic!("explicit clear should patch the observed runtime hostname"); + }; + + assert_eq!(patch.hostname.as_deref(), Some("")); + } + #[test] fn session_runtime_config_cache_plans_patch_from_observed_config() { let mut cache = SessionRuntimeConfigCache::default(); @@ -1401,7 +1953,7 @@ mod tests { assert!(matches!( action, - runtime_reconcile::RuntimeReconcileAction::None + runtime_reconcile::RuntimeReconcileAction::Unchanged(_) )); } @@ -1424,4 +1976,94 @@ mod tests { .expect("prepare action after stale run result"); assert!(action.is_none()); } + + #[test] + fn missing_run_does_not_accept_omitted_hostname() { + let mut cache = SessionRuntimeConfigCache::default(); + let observed = config_with_port_forwards(Vec::new()); + let mut desired = observed.clone(); + desired.hostname = Some("device-host".to_string()); + + let err = remember_if_runtime_matches_desired("managed", &desired, observed, &mut cache) + .expect_err("missing run must not trust an omitted hostname"); + + assert!( + err.to_string() + .contains("runtime config still differs after managed run") + ); + assert!(!cache.entries.contains_key("managed")); + } + + #[test] + fn restored_omitted_hostname_prevents_repeated_hostname_patch() { + let mut desired = config_with_port_forwards(Vec::new()); + desired.hostname = Some("device-host".to_string()); + let mut observed = desired.clone(); + observed.hostname = None; + + runtime_reconcile::restore_omitted_hostname(&mut observed, &desired, true); + assert_eq!(observed.hostname.as_deref(), Some("device-host")); + + let mut cache = SessionRuntimeConfigCache::default(); + cache.remember("managed", observed); + let action = cache + .plan("managed", desired) + .expect("prepare action after restore") + .expect("cached action"); + + assert!(matches!( + action, + runtime_reconcile::RuntimeReconcileAction::Unchanged(_) + )); + } + + fn round_with_revision_state( + target_config_revision: Option<&str>, + should_apply_runtime_revision: bool, + ) -> ReconcileRound { + ReconcileRound { + req: HeartbeatRequest::default(), + machine_id: uuid::Uuid::new_v4(), + user_id: 1, + running_inst_ids: HashSet::new(), + local_configs: Vec::new(), + delete_instance_ids: HashSet::new(), + target_config_revision: target_config_revision.map(str::to_string), + should_apply_runtime_revision, + scope: ReconcileScope::Full, + runtime_config_epoch: 0, + runtime_config_cache_epoch: 0, + } + } + + #[test] + fn running_web_configs_reconcile_without_tracked_revision() { + use crate::client_manager::managed_config::PersistedConfigSource; + + assert!(should_reconcile_running_web_config( + true, + PersistedConfigSource::Web, + &round_with_revision_state(None, false), + )); + assert!(!should_reconcile_running_web_config( + true, + PersistedConfigSource::Web, + &round_with_revision_state(Some("rev-a"), false), + )); + assert!(should_reconcile_running_web_config( + true, + PersistedConfigSource::Web, + &round_with_revision_state(Some("rev-a"), true), + )); + assert!(!should_reconcile_running_web_config( + true, + PersistedConfigSource::User, + &round_with_revision_state(None, false), + )); + assert!(!should_reconcile_running_web_config( + false, + PersistedConfigSource::Web, + &round_with_revision_state(None, false), + )); + } } diff --git a/easytier-web/src/client_manager/session/webhook_validation.rs b/easytier-web/src/client_manager/session/webhook_validation.rs index 2121e901..407a44ea 100644 --- a/easytier-web/src/client_manager/session/webhook_validation.rs +++ b/easytier-web/src/client_manager/session/webhook_validation.rs @@ -25,6 +25,8 @@ pub(super) struct WebhookValidationInput { pub(super) webhook_config: SharedWebhookConfig, pub(super) client_url: url::Url, pub(super) applied_config_revision: Option, + pub(super) applied_config_revision_known: bool, + pub(super) failed_instance_ids: Vec, pub(super) req: HeartbeatRequest, pub(super) machine_id: uuid::Uuid, } @@ -40,28 +42,39 @@ pub(super) fn retry_delay(machine_id: uuid::Uuid) -> Duration { } async fn request_heartbeat_validation( - webhook_config: &crate::webhook::WebhookConfig, - client_url: &url::Url, + input: &WebhookValidationInput, persisted_config_revision: Option<&str>, - applied_config_revision: Option<&str>, - req: &HeartbeatRequest, - machine_id: uuid::Uuid, ) -> anyhow::Result> { let webhook_req = crate::webhook::ValidateTokenRequest { - token: req.user_token.clone(), - machine_id: machine_id.to_string(), - public_ip: client_url.host_str().map(str::to_string), - hostname: req.hostname.clone(), - version: req.easytier_version.clone(), - os_type: req.device_os.as_ref().map(|info| info.os_type.clone()), - os_version: req.device_os.as_ref().map(|info| info.version.clone()), - os_distribution: req.device_os.as_ref().map(|info| info.distribution.clone()), - web_instance_id: webhook_config.web_instance_id.clone(), - web_instance_api_base_url: webhook_config.web_instance_api_base_url.clone(), + token: input.req.user_token.clone(), + machine_id: input.machine_id.to_string(), + public_ip: input.client_url.host_str().map(str::to_string), + hostname: input.req.hostname.clone(), + version: input.req.easytier_version.clone(), + os_type: input + .req + .device_os + .as_ref() + .map(|info| info.os_type.clone()), + os_version: input + .req + .device_os + .as_ref() + .map(|info| info.version.clone()), + os_distribution: input + .req + .device_os + .as_ref() + .map(|info| info.distribution.clone()), + web_instance_id: input.webhook_config.web_instance_id.clone(), + web_instance_api_base_url: input.webhook_config.web_instance_api_base_url.clone(), persisted_config_revision: persisted_config_revision.map(str::to_string), - applied_config_revision: applied_config_revision.map(str::to_string), + applied_config_revision: input.applied_config_revision.as_deref().map(str::to_string), + applied_config_revision_known: input.applied_config_revision_known, + failed_instance_ids: input.failed_instance_ids.to_vec(), }; - let resp = webhook_config + let resp = input + .webhook_config .validate_token(&webhook_req) .await .map_err(|e| anyhow::anyhow!("Webhook token validation failed: {:?}", e))?; @@ -87,7 +100,7 @@ async fn resolve_user_id(storage: &Storage, token: &str) -> anyhow::Result None => storage .auto_create_user(token) .await - .with_context(|| format!("Failed to auto-create webhook user: {:?}", token))?, + .with_context(|| "Failed to auto-create webhook user".to_string())?, }; Ok(user_id) @@ -115,7 +128,7 @@ async fn persisted_config_revision_for_token( async fn wait_for_input( session_data: std::sync::Weak>, -) -> Option { +) -> Option<(WebhookValidationInput, u64)> { loop { let notify = { let session_data = session_data.upgrade()?; @@ -133,14 +146,28 @@ async fn wait_for_input( let req = data.req.clone()?; let machine_id = req.machine_id.map(Into::into)?; let storage = Storage::try_from(data.storage.clone()).ok()?; - return Some(WebhookValidationInput { - storage, - webhook_config: data.webhook_config.clone(), - client_url: data.client_url.clone(), - applied_config_revision: data.applied_config_revision.clone(), - req, - machine_id, - }); + let (applied_config_revision, applied_config_revision_known) = { + let runtime = data.managed_runtime(); + ( + runtime.applied_config_revision.clone(), + runtime.applied_config_revision_known, + ) + }; + return Some(( + WebhookValidationInput { + storage, + webhook_config: data.webhook_config.clone(), + client_url: data.client_url.clone(), + applied_config_revision, + applied_config_revision_known, + failed_instance_ids: SessionRpcService::sorted_failed_instance_ids_locked( + &data, + ), + req, + machine_id, + }, + data.webhook_validation_change_epoch, + )); } data.webhook_validation_notify.clone() }; @@ -148,17 +175,64 @@ async fn wait_for_input( } } +async fn wait_for_retry_or_state_change( + session_data: &std::sync::Weak>, + machine_id: uuid::Uuid, + validation_change_epoch: u64, + delay: Duration, +) { + let retry_deadline = tokio::time::sleep(delay); + tokio::pin!(retry_deadline); + + loop { + let notify = { + let Some(session_data) = session_data.upgrade() else { + return; + }; + let data = session_data.read().await; + let Some(req) = data.req.as_ref() else { + return; + }; + if req.machine_id.map(uuid::Uuid::from) != Some(machine_id) + || matches!(data.auth_state, SessionAuthState::Invalid) + { + return; + } + if data.webhook_validation_change_epoch != validation_change_epoch { + return; + } + data.webhook_validation_notify.clone() + }; + + // Notify is only a wake-up hint. Periodic validation can set dirty, + // but only a meaningful validation-state change may bypass backoff. + // Recheck the epoch after every wake without resetting the deadline. + tokio::select! { + _ = &mut retry_deadline => { + mark_dirty_if_current(session_data, machine_id).await; + return; + } + _ = notify.notified() => {} + } + } +} + pub(super) async fn run_worker(session_data: std::sync::Weak>) { - while let Some(input) = wait_for_input(session_data.clone()).await { + while let Some((input, validation_change_epoch)) = wait_for_input(session_data.clone()).await { let machine_id = input.machine_id; - if let Err(error) = run_round(session_data.clone(), input).await { + if let Err(error) = run_round(session_data.clone(), input, validation_change_epoch).await { tracing::warn!( ?machine_id, %error, "webhook validation failed, will retry later" ); - tokio::time::sleep(retry_delay(machine_id)).await; - mark_dirty_if_current(&session_data, machine_id).await; + wait_for_retry_or_state_change( + &session_data, + machine_id, + validation_change_epoch, + retry_delay(machine_id), + ) + .await; } } } @@ -166,6 +240,7 @@ pub(super) async fn run_worker(session_data: std::sync::Weak pub(super) async fn run_round( session_data: std::sync::Weak>, input: WebhookValidationInput, + validation_change_epoch: u64, ) -> anyhow::Result<()> { let persisted_config_revision = persisted_config_revision_for_token( &input.storage, @@ -173,26 +248,53 @@ pub(super) async fn run_round( input.machine_id, ) .await?; - let validation = request_heartbeat_validation( - &input.webhook_config, - &input.client_url, - persisted_config_revision.as_deref(), - input.applied_config_revision.as_deref(), - &input.req, - input.machine_id, - ) - .await?; + let validation = + request_heartbeat_validation(&input, persisted_config_revision.as_deref()).await?; + + // The HTTP round trip can span heartbeats, revision updates, and + // failed-instance changes. Results older than the current epoch are + // discarded so a stale rejection cannot invalidate the session and a + // stale success cannot emit outdated connection transitions. + if !validation_results_are_current(&session_data, &input, validation_change_epoch).await { + return Ok(()); + } let Some(validation) = validation else { - apply_rejected(&session_data, &input).await; + apply_rejected(&session_data, &input, validation_change_epoch).await; return Ok(()); }; let user_id = resolve_user_id(&input.storage, &input.req.user_token).await?; - apply_success(&session_data, input, validation, user_id).await; + apply_success( + &session_data, + input, + validation, + user_id, + validation_change_epoch, + ) + .await; Ok(()) } +async fn validation_results_are_current( + session_data: &std::sync::Weak>, + input: &WebhookValidationInput, + validation_change_epoch: u64, +) -> bool { + let Some(session_data) = session_data.upgrade() else { + return false; + }; + let data = session_data.read().await; + if data.webhook_validation_change_epoch != validation_change_epoch { + tracing::debug!( + machine_id = %input.machine_id, + "discard stale webhook validation result" + ); + return false; + } + true +} + async fn mark_dirty_if_current( session_data: &std::sync::Weak>, machine_id: uuid::Uuid, @@ -224,6 +326,7 @@ async fn mark_dirty_if_current( pub(super) async fn apply_rejected( session_data: &std::sync::Weak>, input: &WebhookValidationInput, + validation_change_epoch: u64, ) { let Some(session_data) = session_data.upgrade() else { return; @@ -239,6 +342,13 @@ pub(super) async fn apply_rejected( }) { return; } + if data.webhook_validation_change_epoch != validation_change_epoch { + tracing::debug!( + machine_id = %input.machine_id, + "discard stale webhook validation rejection" + ); + return; + } tracing::info!( machine_id = %input.machine_id, client_url = %data.client_url, @@ -247,8 +357,6 @@ pub(super) async fn apply_rejected( data.auth_state = SessionAuthState::Invalid; data.webhook_validation_dirty = false; data.binding_version = None; - data.applied_config_revision = None; - data.pending_managed_config_delta = None; let storage_token = data.storage_token.clone(); let disconnect_notification = storage_token.as_ref().and_then(|storage_token| { data.webhook_connected_binding_version @@ -282,6 +390,7 @@ pub(super) async fn apply_success( input: WebhookValidationInput, validation: WebhookHeartbeatValidation, user_id: i32, + validation_change_epoch: u64, ) { let WebhookHeartbeatValidation { config_revision: _, @@ -296,6 +405,7 @@ pub(super) async fn apply_success( notifier, disconnect_notification, connect_notification, + validation_notify, runtime_req, session_epoch, ) = { @@ -310,6 +420,13 @@ pub(super) async fn apply_success( ) { return; } + if data.webhook_validation_change_epoch != validation_change_epoch { + tracing::debug!( + machine_id = %input.machine_id, + "discard stale webhook validation success" + ); + return; + } if matches!(data.auth_state, SessionAuthState::Invalid) { tracing::info!( machine_id = %input.machine_id, @@ -321,6 +438,20 @@ pub(super) async fn apply_success( let previous_connected_binding_version = data.webhook_connected_binding_version; let client_url = data.client_url.clone(); + let is_new_storage_token = data.storage_token.is_none(); + let mut restored_runtime_revision = false; + if is_new_storage_token { + data.managed_runtime = input.storage.bind_managed_runtime_state( + user_id, + input.machine_id, + SessionRpcService::heartbeat_runtime_id(&runtime_req), + data.session_epoch, + ); + let runtime = data.managed_runtime(); + restored_runtime_revision = runtime.applied_config_revision_known + && (!input.applied_config_revision_known + || input.applied_config_revision != runtime.applied_config_revision); + } let storage_token = data.storage_token.get_or_insert_with(|| StorageToken { token: runtime_req.user_token.clone(), client_url, @@ -330,6 +461,16 @@ pub(super) async fn apply_success( let storage_token = storage_token.clone(); data.auth_state = SessionAuthState::Authorized; data.binding_version = Some(binding_version); + if is_new_storage_token { + tracing::info!( + machine_id = %input.machine_id, + user_id, + session_epoch = data.session_epoch, + binding_version, + client_url = %data.client_url, + "session identity established" + ); + } let should_notify_connected = previous_connected_binding_version != Some(binding_version); let disconnect_notification = previous_connected_binding_version .filter(|previous_binding_version| *previous_binding_version != binding_version) @@ -368,12 +509,15 @@ pub(super) async fn apply_success( binding_version: Some(binding_version), }, }); + let validation_notify = restored_runtime_revision + .then(|| SessionRpcService::mark_webhook_validation_state_changed_locked(&mut data)); ( storage_token, data.notifier.clone(), disconnect_notification, connect_notification, + validation_notify, runtime_req, data.session_epoch, ) @@ -384,6 +528,10 @@ pub(super) async fn apply_success( .storage .update_session_client(storage_token, report_time, true, session_epoch); + if let Some(validation_notify) = validation_notify { + validation_notify.notify_one(); + } + if disconnect_notification.is_some() || connect_notification.is_some() { wait_webhook_connection_transition( Arc::downgrade(&session_data), @@ -410,3 +558,301 @@ async fn wait_webhook_connection_transition( tracing::warn!(%error, "webhook connection transition task failed"); } } + +#[cfg(test)] +mod tests { + use super::*; + + async fn validation_session(machine_id: uuid::Uuid) -> Arc> { + let storage = Storage::new(crate::db::Db::memory_db().await); + let mut data = SessionData::new( + storage.weak_ref(), + url::Url::parse("http://127.0.0.1").unwrap(), + None, + Arc::new(crate::FeatureFlags::default()), + Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ); + data.req = Some(HeartbeatRequest { + user_token: "token".to_string(), + machine_id: Some(machine_id.into()), + ..Default::default() + }); + data.auth_state = SessionAuthState::Authorized; + Arc::new(RwLock::new(data)) + } + + #[tokio::test] + async fn reconnect_immediately_reports_restored_runtime_revision() { + let storage = Storage::new(crate::db::Db::memory_db().await); + let user_id = storage.db().auto_create_user("token").await.unwrap().id; + let machine_id = uuid::Uuid::new_v4(); + let runtime_id = uuid::Uuid::new_v4(); + let shared = storage.bind_managed_runtime_state(user_id, machine_id, Some(runtime_id), 1); + { + let mut runtime = shared.lock().unwrap(); + runtime.applied_config_revision = Some("rev-applied".to_string()); + runtime.applied_config_revision_known = true; + } + let request = HeartbeatRequest { + user_token: "token".to_string(), + machine_id: Some(machine_id.into()), + inst_id: Some(runtime_id.into()), + ..Default::default() + }; + let mut data = SessionData::new( + storage.weak_ref(), + url::Url::parse("http://127.0.0.1").unwrap(), + None, + Arc::new(crate::FeatureFlags::default()), + Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + ); + data.req = Some(request.clone()); + data.session_identity = Some(SessionRpcService::heartbeat_identity(&request, machine_id)); + data.session_epoch = 2; + let session_data = Arc::new(RwLock::new(data)); + + let validation_change_epoch = session_data.read().await.webhook_validation_change_epoch; + apply_success( + &Arc::downgrade(&session_data), + WebhookValidationInput { + storage, + webhook_config: Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + client_url: url::Url::parse("http://127.0.0.1").unwrap(), + applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), + req: request, + machine_id, + }, + WebhookHeartbeatValidation { + config_revision: "rev-applied".to_string(), + binding_version: 1, + }, + user_id, + validation_change_epoch, + ) + .await; + + let data = session_data.read().await; + assert!(Arc::ptr_eq(&data.managed_runtime, &shared)); + assert!(data.webhook_validation_dirty); + assert_eq!(data.webhook_validation_change_epoch, 1); + } + + #[tokio::test] + async fn validation_input_carries_merged_failed_instance_ids() { + let machine_id = uuid::Uuid::new_v4(); + let core_failed = uuid::Uuid::new_v4(); + let local_failed = uuid::Uuid::new_v4().to_string(); + let session_data = validation_session(machine_id).await; + let storage = Storage::new(crate::db::Db::memory_db().await); + { + let mut data = session_data.write().await; + data.storage = storage.weak_ref(); + data.req + .as_mut() + .unwrap() + .failed_network_instances + .push(core_failed.into()); + data.direct_run_failed_instance_ids + .insert(local_failed.clone()); + data.webhook_validation_dirty = true; + } + + let (input, _) = wait_for_input(Arc::downgrade(&session_data)) + .await + .expect("validation input"); + let mut expected = vec![core_failed.to_string(), local_failed]; + expected.sort_unstable(); + + assert_eq!(input.failed_instance_ids, expected); + } + + #[tokio::test] + async fn stale_notification_does_not_bypass_validation_retry_delay() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + let notify = session_data.read().await.webhook_validation_notify.clone(); + notify.notify_one(); + let weak_session = Arc::downgrade(&session_data); + + let wait = + wait_for_retry_or_state_change(&weak_session, machine_id, 0, Duration::from_secs(10)); + tokio::pin!(wait); + + assert!( + tokio::time::timeout(Duration::from_millis(20), &mut wait) + .await + .is_err() + ); + assert!(!session_data.read().await.webhook_validation_dirty); + } + + #[tokio::test] + async fn validation_retry_deadline_rearms_dirty_state() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + let weak_session = Arc::downgrade(&session_data); + + tokio::time::timeout( + Duration::from_secs(1), + wait_for_retry_or_state_change(&weak_session, machine_id, 0, Duration::from_millis(20)), + ) + .await + .expect("retry deadline should eventually expire"); + assert!(session_data.read().await.webhook_validation_dirty); + } + + #[tokio::test] + async fn periodic_dirty_state_does_not_interrupt_validation_retry_delay() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + let weak_session = Arc::downgrade(&session_data); + let wait = + wait_for_retry_or_state_change(&weak_session, machine_id, 0, Duration::from_secs(10)); + tokio::pin!(wait); + + assert!( + tokio::time::timeout(Duration::from_millis(20), &mut wait) + .await + .is_err() + ); + mark_dirty_if_current(&weak_session, machine_id).await; + assert!( + tokio::time::timeout(Duration::from_millis(20), &mut wait) + .await + .is_err() + ); + assert!(session_data.read().await.webhook_validation_dirty); + } + + #[tokio::test] + async fn validation_state_change_interrupts_retry_delay() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + let weak_session = Arc::downgrade(&session_data); + let wait = + wait_for_retry_or_state_change(&weak_session, machine_id, 0, Duration::from_secs(10)); + tokio::pin!(wait); + + assert!( + tokio::time::timeout(Duration::from_millis(20), &mut wait) + .await + .is_err() + ); + let notify = { + let mut data = session_data.write().await; + SessionRpcService::mark_webhook_validation_state_changed_locked(&mut data) + }; + notify.notify_one(); + tokio::time::timeout(Duration::from_millis(500), &mut wait) + .await + .expect("validation state change should interrupt retry delay"); + assert!(session_data.read().await.webhook_validation_dirty); + } + + #[tokio::test] + async fn invalid_session_does_not_rearm_validation_retry() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + session_data.write().await.auth_state = SessionAuthState::Invalid; + let weak_session = Arc::downgrade(&session_data); + + tokio::time::timeout( + Duration::from_millis(100), + wait_for_retry_or_state_change(&weak_session, machine_id, 0, Duration::from_secs(10)), + ) + .await + .expect("invalid session should stop waiting"); + assert!(!session_data.read().await.webhook_validation_dirty); + } + + #[tokio::test] + async fn validation_results_require_current_epoch_or_live_session() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + let input = WebhookValidationInput { + storage: Storage::new(crate::db::Db::memory_db().await), + webhook_config: Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + client_url: url::Url::parse("http://127.0.0.1").unwrap(), + applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), + req: HeartbeatRequest::default(), + machine_id, + }; + let weak_session = Arc::downgrade(&session_data); + + assert!( + validation_results_are_current(&weak_session, &input, 0).await, + "matching epoch is current" + ); + + session_data.write().await.webhook_validation_change_epoch = 7; + assert!( + !validation_results_are_current(&weak_session, &input, 0).await, + "epoch bump discards stale results" + ); + + drop(session_data); + assert!( + !validation_results_are_current(&weak_session, &input, 7).await, + "dropped session discards results" + ); + } + + #[tokio::test] + async fn apply_paths_discard_results_from_stale_epochs() { + let machine_id = uuid::Uuid::new_v4(); + let session_data = validation_session(machine_id).await; + session_data.write().await.webhook_connected_binding_version = Some(3); + let input = WebhookValidationInput { + storage: Storage::new(crate::db::Db::memory_db().await), + webhook_config: Arc::new(crate::webhook::WebhookConfig::new( + None, None, None, None, None, + )), + client_url: url::Url::parse("http://127.0.0.1").unwrap(), + applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), + req: HeartbeatRequest { + user_token: "token".to_string(), + machine_id: Some(machine_id.into()), + ..Default::default() + }, + machine_id, + }; + + let stale_epoch = session_data.read().await.webhook_validation_change_epoch; + session_data.write().await.webhook_validation_change_epoch = stale_epoch + 1; + + apply_rejected(&Arc::downgrade(&session_data), &input, stale_epoch).await; + let data = session_data.read().await; + assert_eq!(data.auth_state, SessionAuthState::Authorized); + assert_eq!(data.webhook_connected_binding_version, Some(3)); + drop(data); + + apply_success( + &Arc::downgrade(&session_data), + input, + WebhookHeartbeatValidation { + config_revision: "rev-1".to_string(), + binding_version: 9, + }, + 1, + stale_epoch, + ) + .await; + let data = session_data.read().await; + assert_eq!(data.binding_version, None); + assert_eq!(data.webhook_connected_binding_version, Some(3)); + } +} diff --git a/easytier-web/src/client_manager/storage.rs b/easytier-web/src/client_manager/storage.rs index 063fe563..9ff7ee95 100644 --- a/easytier-web/src/client_manager/storage.rs +++ b/easytier-web/src/client_manager/storage.rs @@ -1,9 +1,14 @@ use std::sync::{Arc, Weak}; -use dashmap::DashMap; +use dashmap::{DashMap, mapref::entry::Entry}; use crate::db::{Db, UserIdInDb}; +use super::session::{ + ManagedConfigPersistedChange, ManagedConfigReconcileHint, ManagedRuntimeState, + SharedManagedRuntimeState, record_managed_config_reconcile_hint, +}; + // use this to maintain Storage #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct StorageToken { @@ -21,12 +26,46 @@ struct ClientInfo { session_epoch: u64, } +#[derive(Debug, Clone)] +struct ManagedRuntimeContinuity { + // Accepted trade-off: continuity assumes managed configuration is only + // mutated through easytier-web. A local management RPC can make Core + // drift without invalidating this state; detecting that would require a + // Core-wide mutation generation outside this compatibility path. + runtime_id: Option, + session_epoch: u64, + state: SharedManagedRuntimeState, +} + #[derive(Debug)] pub struct StorageInner { user_clients_map: DashMap>, + managed_runtime_states: DashMap<(UserIdInDb, uuid::Uuid), ManagedRuntimeContinuity>, pub db: Db, } +impl StorageInner { + pub(super) fn owns_authorized_session( + &self, + stoken: &StorageToken, + session_epoch: u64, + ) -> bool { + self.user_clients_map + .get(&stoken.user_id) + .and_then(|clients| { + clients.get(&stoken.machine_id).map(|client| { + client.authorized + && client.session_epoch == session_epoch + && client.storage_token.token == stoken.token + && client.storage_token.client_url == stoken.client_url + && client.storage_token.user_id == stoken.user_id + && client.storage_token.machine_id == stoken.machine_id + }) + }) + .unwrap_or(false) + } +} + #[derive(Debug, Clone)] pub struct Storage(Arc); pub type WeakRefStorage = Weak; @@ -43,20 +82,149 @@ impl Storage { pub fn new(db: Db) -> Self { Storage(Arc::new(StorageInner { user_clients_map: DashMap::new(), + managed_runtime_states: DashMap::new(), db, })) } + pub(super) fn bind_managed_runtime_state( + &self, + user_id: UserIdInDb, + machine_id: uuid::Uuid, + runtime_id: Option, + session_epoch: u64, + ) -> SharedManagedRuntimeState { + let new_state = || Arc::new(std::sync::Mutex::new(ManagedRuntimeState::default())); + match self.0.managed_runtime_states.entry((user_id, machine_id)) { + Entry::Occupied(mut entry) => { + let current = entry.get(); + if runtime_id.is_some() && current.runtime_id == runtime_id { + let state = current.state.clone(); + if session_epoch > current.session_epoch { + entry.get_mut().session_epoch = session_epoch; + } + return state; + } + if session_epoch < current.session_epoch { + return new_state(); + } + let state = new_state(); + entry.insert(ManagedRuntimeContinuity { + runtime_id, + session_epoch, + state: state.clone(), + }); + state + } + Entry::Vacant(entry) => { + let state = new_state(); + entry.insert(ManagedRuntimeContinuity { + runtime_id, + session_epoch, + state: state.clone(), + }); + state + } + } + } + + fn current_managed_runtime_state( + &self, + user_id: UserIdInDb, + machine_id: uuid::Uuid, + ) -> Option { + self.0 + .managed_runtime_states + .get(&(user_id, machine_id)) + .map(|entry| entry.state.clone()) + } + + pub(super) fn record_full_managed_config_change( + &self, + user_id: UserIdInDb, + machine_id: uuid::Uuid, + config_revision: Option<&str>, + ) -> bool { + let Some(state) = self.current_managed_runtime_state(user_id, machine_id) else { + return false; + }; + let mut state = state.lock().expect("managed runtime state lock poisoned"); + // Unrevisioned legacy updates carry no revision to compare against, + // so they always invalidate: record the hint and bump the epoch that + // fences in-flight reconcile rounds. + let target_already_applied = config_revision + .is_some_and(|revision| state.applied_config_revision.as_deref() == Some(revision)); + if target_already_applied && state.pending_managed_config_reconcile.is_none() { + return false; + } + if !target_already_applied { + record_managed_config_reconcile_hint( + &mut state.pending_managed_config_reconcile, + ManagedConfigReconcileHint::Full, + ); + } + state.runtime_config_epoch = state.runtime_config_epoch.wrapping_add(1); + true + } + + pub(super) fn record_patch_managed_config_change( + &self, + user_id: UserIdInDb, + machine_id: uuid::Uuid, + change: ManagedConfigPersistedChange, + ) -> bool { + let Some(state) = self.current_managed_runtime_state(user_id, machine_id) else { + return false; + }; + let mut state = state.lock().expect("managed runtime state lock poisoned"); + let target_already_applied = + state.applied_config_revision.as_deref() == Some(change.target_revision.as_str()); + if target_already_applied && state.pending_managed_config_reconcile.is_none() { + return false; + } + if !target_already_applied { + record_managed_config_reconcile_hint( + &mut state.pending_managed_config_reconcile, + ManagedConfigReconcileHint::Dirty { + expected_revision: change.expected_revision, + target_revision: change.target_revision, + instance_ids: change.dirty_instance_ids, + }, + ); + } + state.runtime_config_epoch = state.runtime_config_epoch.wrapping_add(1); + true + } + + pub(super) fn invalidate_managed_runtime_state( + &self, + user_id: UserIdInDb, + machine_id: uuid::Uuid, + ) -> bool { + let Some(state) = self.current_managed_runtime_state(user_id, machine_id) else { + return false; + }; + let mut state = state.lock().expect("managed runtime state lock poisoned"); + state.applied_config_revision = None; + state.applied_config_revision_known = true; + state.known_runtime_base_revision = None; + state.pending_managed_config_reconcile = Some(ManagedConfigReconcileHint::Full); + state.runtime_config_epoch = state.runtime_config_epoch.wrapping_add(1); + state.runtime_config_cache_epoch = state.runtime_config_cache_epoch.wrapping_add(1); + true + } + fn remove_client_info_map( map: &DashMap, stoken: &StorageToken, session_epoch: u64, - ) { + ) -> bool { map.remove_if(&stoken.machine_id, |_, v| { v.storage_token.client_url == stoken.client_url && v.storage_token.user_id == stoken.user_id && v.session_epoch == session_epoch - }); + }) + .is_some() } fn update_client_info_map(map: &DashMap, client_info: &ClientInfo) { @@ -99,6 +267,20 @@ impl Storage { authorized: bool, session_epoch: u64, ) { + let mut continuity = self + .0 + .managed_runtime_states + .entry((stoken.user_id, stoken.machine_id)) + .or_insert_with(|| ManagedRuntimeContinuity { + runtime_id: None, + session_epoch, + state: Arc::new(std::sync::Mutex::new(ManagedRuntimeState::default())), + }); + if session_epoch < continuity.session_epoch { + return; + } + continuity.session_epoch = session_epoch; + let inner = self.0.user_clients_map.entry(stoken.user_id).or_default(); let client_info = ClientInfo { @@ -111,16 +293,30 @@ impl Storage { } pub fn remove_client(&self, stoken: &StorageToken) { - self.remove_session_client(stoken, 0); + let _ = self.remove_session_client(stoken, 0); } - pub(super) fn remove_session_client(&self, stoken: &StorageToken, session_epoch: u64) { + pub(super) fn remove_session_client(&self, stoken: &StorageToken, session_epoch: u64) -> bool { + let Some(mut continuity) = self + .0 + .managed_runtime_states + .get_mut(&(stoken.user_id, stoken.machine_id)) + else { + return false; + }; + if session_epoch < continuity.session_epoch { + return false; + } + continuity.session_epoch = session_epoch; + + let mut removed = false; self.0 .user_clients_map .remove_if(&stoken.user_id, |_, set| { - Self::remove_client_info_map(set, stoken, session_epoch); + removed = Self::remove_client_info_map(set, stoken, session_epoch); set.is_empty() }); + removed } pub fn weak_ref(&self) -> WeakRefStorage { @@ -262,6 +458,36 @@ mod tests { assert_eq!(storage.get_client_url_by_machine_id(2, &machine_id), None); } + #[tokio::test] + async fn unrevisioned_full_change_always_fences_runtime_epochs() { + let storage = Storage::new(Db::memory_db().await); + let machine_id = uuid::Uuid::new_v4(); + let state = storage.bind_managed_runtime_state(7, machine_id, None, 1); + { + let mut runtime = state.lock().unwrap(); + runtime.applied_config_revision = Some("rev-1".to_string()); + runtime.applied_config_revision_known = true; + } + + assert!(storage.record_full_managed_config_change(7, machine_id, None)); + { + let runtime = state.lock().unwrap(); + assert_eq!(runtime.runtime_config_epoch, 1); + assert!(matches!( + runtime.pending_managed_config_reconcile, + Some(ManagedConfigReconcileHint::Full) + )); + } + + // Repeated unrevisioned updates keep fencing, while a revision that + // is already applied with no pending hint stays a no-op. + assert!(storage.record_full_managed_config_change(7, machine_id, None)); + assert_eq!(state.lock().unwrap().runtime_config_epoch, 2); + state.lock().unwrap().pending_managed_config_reconcile = None; + assert!(!storage.record_full_managed_config_change(7, machine_id, Some("rev-1"))); + assert_eq!(state.lock().unwrap().runtime_config_epoch, 2); + } + #[tokio::test] async fn newer_session_epoch_owns_route_until_it_is_removed() { let storage = Storage::new(Db::memory_db().await); @@ -273,18 +499,127 @@ mod tests { storage.update_session_client(current.clone(), 20, true, 2); storage.update_session_client(old.clone(), 30, true, 1); + assert!(!storage.0.owns_authorized_session(&old, 1)); + assert!(storage.0.owns_authorized_session(¤t, 2)); assert_eq!( storage.get_client_url_by_machine_id(1, &machine_id), Some(current.client_url.clone()) ); - storage.remove_session_client(&old, 1); + assert!(!storage.remove_session_client(&old, 1)); assert_eq!( storage.get_client_url_by_machine_id(1, &machine_id), Some(current.client_url.clone()) ); - storage.remove_session_client(¤t, 2); + assert!(storage.remove_session_client(¤t, 2)); assert_eq!(storage.get_client_url_by_machine_id(1, &machine_id), None); + + storage.update_session_client(old.clone(), 40, true, 1); + assert!(!storage.0.owns_authorized_session(&old, 1)); + assert_eq!(storage.get_client_url_by_machine_id(1, &machine_id), None); + } + + #[tokio::test] + async fn same_runtime_reuses_state_across_sessions() { + let storage = Storage::new(Db::memory_db().await); + let machine_id = uuid::Uuid::new_v4(); + let runtime_id = uuid::Uuid::new_v4(); + + let first = storage.bind_managed_runtime_state(1, machine_id, Some(runtime_id), 1); + { + let mut state = first.lock().unwrap(); + state.applied_config_revision = Some("rev-a".to_string()); + state.applied_config_revision_known = true; + } + + let reconnected = storage.bind_managed_runtime_state(1, machine_id, Some(runtime_id), 2); + + assert!(Arc::ptr_eq(&first, &reconnected)); + let state = reconnected.lock().unwrap(); + assert_eq!(state.applied_config_revision.as_deref(), Some("rev-a")); + assert!(state.applied_config_revision_known); + } + + #[tokio::test] + async fn changed_or_missing_runtime_id_starts_with_unknown_state() { + let storage = Storage::new(Db::memory_db().await); + let machine_id = uuid::Uuid::new_v4(); + let first = + storage.bind_managed_runtime_state(1, machine_id, Some(uuid::Uuid::new_v4()), 1); + { + let mut state = first.lock().unwrap(); + state.applied_config_revision = Some("rev-a".to_string()); + state.applied_config_revision_known = true; + } + + let restarted = + storage.bind_managed_runtime_state(1, machine_id, Some(uuid::Uuid::new_v4()), 2); + assert!(!Arc::ptr_eq(&first, &restarted)); + assert!(!restarted.lock().unwrap().applied_config_revision_known); + + let legacy = storage.bind_managed_runtime_state(1, machine_id, None, 3); + assert!(!Arc::ptr_eq(&restarted, &legacy)); + let legacy_reconnected = storage.bind_managed_runtime_state(1, machine_id, None, 4); + assert!(!Arc::ptr_eq(&legacy, &legacy_reconnected)); + assert!( + !legacy_reconnected + .lock() + .unwrap() + .applied_config_revision_known + ); + } + + #[tokio::test] + async fn stale_session_cannot_replace_current_runtime_state() { + let storage = Storage::new(Db::memory_db().await); + let machine_id = uuid::Uuid::new_v4(); + let current_runtime_id = uuid::Uuid::new_v4(); + let current = + storage.bind_managed_runtime_state(1, machine_id, Some(current_runtime_id), 2); + + let stale = + storage.bind_managed_runtime_state(1, machine_id, Some(uuid::Uuid::new_v4()), 1); + assert!(!Arc::ptr_eq(¤t, &stale)); + + let reconnected = + storage.bind_managed_runtime_state(1, machine_id, Some(current_runtime_id), 3); + assert!(Arc::ptr_eq(¤t, &reconnected)); + } + + #[tokio::test] + async fn patch_hint_survives_disconnect_until_same_runtime_reconnects() { + let storage = Storage::new(Db::memory_db().await); + let machine_id = uuid::Uuid::new_v4(); + let runtime_id = uuid::Uuid::new_v4(); + let state = storage.bind_managed_runtime_state(1, machine_id, Some(runtime_id), 1); + { + let mut state = state.lock().unwrap(); + state.applied_config_revision = Some("rev-a".to_string()); + state.applied_config_revision_known = true; + state.known_runtime_base_revision = Some("rev-a".to_string()); + } + + assert!(storage.record_patch_managed_config_change( + 1, + machine_id, + ManagedConfigPersistedChange { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + dirty_instance_ids: std::collections::HashSet::from(["instance-a".to_string(),]), + }, + )); + + let reconnected = storage.bind_managed_runtime_state(1, machine_id, Some(runtime_id), 2); + let state = reconnected.lock().unwrap(); + assert_eq!( + state.pending_managed_config_reconcile, + Some(ManagedConfigReconcileHint::Dirty { + expected_revision: "rev-a".to_string(), + target_revision: "rev-b".to_string(), + instance_ids: std::collections::HashSet::from(["instance-a".to_string(),]), + }) + ); + assert_eq!(state.runtime_config_epoch, 1); } #[tokio::test] diff --git a/easytier-web/src/main.rs b/easytier-web/src/main.rs index 697c4d9a..acc62c2d 100644 --- a/easytier-web/src/main.rs +++ b/easytier-web/src/main.rs @@ -3,8 +3,8 @@ #[macro_use] extern crate rust_i18n; +use std::net::IpAddr; use std::sync::Arc; -use std::{net::IpAddr, time::Duration}; use clap::Parser; use easytier::tunnel::websocket::WsTunnelListener; @@ -116,11 +116,19 @@ struct Cli { #[arg( long, env = "ET_HEARTBEAT_MIN_RESPONSE_MS", - default_value = "0", + default_value = "3500", help = t!("cli.heartbeat_min_response_ms").to_string(), )] heartbeat_min_response_ms: u64, + #[arg( + long, + env = "ET_HEARTBEAT_TIMEOUT_MS", + default_value = "15000", + help = t!("cli.heartbeat_timeout_ms").to_string(), + )] + heartbeat_timeout_ms: u64, + #[cfg(feature = "embed")] #[arg( long, @@ -288,7 +296,21 @@ async fn main() { setup_panic_handler(); let cli = Cli::parse(); - log::init(&cli, false).unwrap(); + log::init_with_default_console_targets(&cli, false, &["CORE", "easytier_web"]).unwrap(); + tracing::info!( + version = EASYTIER_VERSION, + web_instance_id = ?cli.webhook.web_instance_id, + api_address = %cli.api_server_addr, + api_port = cli.api_server_port, + config_protocol = %cli.config_server_protocol, + config_port = cli.config_server_port, + heartbeat_min_response_ms = cli.heartbeat_min_response_ms, + heartbeat_timeout_ms = cli.heartbeat_timeout_ms, + webhook_enabled = cli.webhook.webhook_url.as_deref().is_some_and(|url| !url.trim().is_empty()), + rust_log_override = std::env::var_os("RUST_LOG").is_some(), + console_log_override = cli.console_log_level.is_some(), + "easytier-web starting" + ); // Validate OIDC configuration: check split-deploy specific requirements // Basic OIDC parameter validation is handled in OidcConfig::from_params @@ -326,10 +348,18 @@ async fn main() { cli.webhook.web_instance_id, cli.webhook.web_instance_api_base_url, )); + let heartbeat_policy = client_manager::HeartbeatPolicy::from_millis( + cli.heartbeat_min_response_ms, + cli.heartbeat_timeout_ms, + ) + .unwrap_or_else(|error| { + eprintln!("Invalid heartbeat configuration: {error}"); + std::process::exit(2); + }); let mut mgr = client_manager::ClientManager::new( db.clone(), cli.geoip_db, - Duration::from_millis(cli.heartbeat_min_response_ms), + heartbeat_policy, feature_flags.clone(), webhook_config.clone(), ); diff --git a/easytier-web/src/webhook.rs b/easytier-web/src/webhook.rs index 59e41b38..5cf57de8 100644 --- a/easytier-web/src/webhook.rs +++ b/easytier-web/src/webhook.rs @@ -308,6 +308,8 @@ pub struct ValidateTokenRequest { pub persisted_config_revision: Option, #[serde(skip_serializing_if = "Option::is_none")] pub applied_config_revision: Option, + pub applied_config_revision_known: bool, + pub failed_instance_ids: Vec, } #[derive(Debug, Deserialize)] @@ -350,6 +352,26 @@ pub struct NodeDisconnectedRequest { pub binding_version: Option, } +#[derive(Debug, thiserror::Error)] +pub(crate) enum WebhookDeliveryError { + #[error("webhook endpoint is invalid: {0}")] + Configuration(#[source] anyhow::Error), + #[error("webhook request failed: {0}")] + Transport(#[source] reqwest::Error), + #[error("webhook returned status {0}")] + ResponseStatus(reqwest::StatusCode), +} + +impl WebhookDeliveryError { + pub(crate) fn is_retryable(&self) -> bool { + match self { + Self::Transport(_) => true, + Self::ResponseStatus(status) => status.is_server_error(), + Self::Configuration(_) => false, + } + } +} + // --- Webhook client --- impl WebhookConfig { @@ -384,7 +406,10 @@ impl WebhookConfig { http_timeout: Duration, ) -> anyhow::Result { let url = self.webhook_endpoint("validate-token")?; + let started_at = Instant::now(); let permit = self.validate_limiter.acquire().await; + let queue_elapsed = started_at.elapsed(); + let http_started_at = Instant::now(); let ret = match tokio::time::timeout(http_timeout, async { let resp = self .client @@ -406,25 +431,45 @@ impl WebhookConfig { Err(_) => Err(anyhow::anyhow!("webhook validate-token timed out")), }; permit.complete(ret.is_ok()); + let http_elapsed = http_started_at.elapsed(); + let elapsed = started_at.elapsed(); + if queue_elapsed >= Duration::from_secs(1) || http_elapsed >= VALIDATE_TOKEN_SLOW_THRESHOLD + { + tracing::warn!( + machine_id = %req.machine_id, + queue_ms = queue_elapsed.as_millis(), + http_ms = http_elapsed.as_millis(), + elapsed_ms = elapsed.as_millis(), + success = ret.is_ok(), + "validate-token completed slowly" + ); + } ret } /// Notify the webhook receiver that a node has connected. - pub async fn notify_node_connected(&self, req: &NodeConnectedRequest) { + pub(crate) async fn notify_node_connected( + &self, + req: &NodeConnectedRequest, + ) -> Result<(), WebhookDeliveryError> { if !self.is_enabled() { - return; + return Ok(()); } - let Ok(url) = self.webhook_endpoint("webhook/node-connected") else { - tracing::warn!("skip node-connected webhook because webhook_url is not configured"); - return; - }; - let _ = self + let url = self + .webhook_endpoint("webhook/node-connected") + .map_err(WebhookDeliveryError::Configuration)?; + let response = self .client .post(&url) .header("X-Internal-Auth", self.webhook_auth_secret()) .json(req) .send() - .await; + .await + .map_err(WebhookDeliveryError::Transport)?; + if !response.status().is_success() { + return Err(WebhookDeliveryError::ResponseStatus(response.status())); + } + Ok(()) } /// Notify the webhook receiver that a node has disconnected. @@ -436,13 +481,35 @@ impl WebhookConfig { tracing::warn!("skip node-disconnected webhook because webhook_url is not configured"); return; }; - let _ = self + let started_at = Instant::now(); + let result = self .client .post(&url) .header("X-Internal-Auth", self.webhook_auth_secret()) .json(req) .send() .await; + let elapsed = started_at.elapsed(); + match result { + Err(error) => tracing::warn!( + machine_id = %req.machine_id, + elapsed_ms = elapsed.as_millis(), + %error, + "node-disconnected webhook delivery failed" + ), + Ok(response) if !response.status().is_success() => tracing::warn!( + machine_id = %req.machine_id, + status = %response.status(), + elapsed_ms = elapsed.as_millis(), + "node-disconnected webhook returned failure status" + ), + Ok(_) if elapsed >= VALIDATE_TOKEN_SLOW_THRESHOLD => tracing::warn!( + machine_id = %req.machine_id, + elapsed_ms = elapsed.as_millis(), + "node-disconnected webhook completed slowly" + ), + Ok(_) => {} + } } fn webhook_auth_secret(&self) -> &str { @@ -461,6 +528,21 @@ mod tests { use axum::{Json, Router, routing::post}; use serde_json::json; + fn node_connected_request() -> NodeConnectedRequest { + NodeConnectedRequest { + machine_id: uuid::Uuid::new_v4().to_string(), + token: "token".to_string(), + user_id: Some(1), + hostname: String::new(), + version: String::new(), + os_type: None, + os_version: None, + os_distribution: None, + web_instance_id: None, + binding_version: Some(1), + } + } + #[test] fn adaptive_validate_limiter_increases_under_queue_pressure() { let now = Instant::now(); @@ -736,6 +818,8 @@ mod tests { web_instance_api_base_url: None, persisted_config_revision: None, applied_config_revision: None, + applied_config_revision_known: false, + failed_instance_ids: Vec::new(), }; validate_webhook .validate_token_with_http_timeout(&req, Duration::from_millis(20)) @@ -773,4 +857,23 @@ mod tests { assert!(resp.valid); assert!(resp.config_revision.is_empty()); } + + #[tokio::test] + async fn node_connected_transport_error_is_retryable() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + drop(listener); + let webhook = WebhookConfig::new(Some(format!("http://{addr}")), None, None, None, None); + + let error = tokio::time::timeout( + Duration::from_secs(1), + webhook.notify_node_connected(&node_connected_request()), + ) + .await + .unwrap() + .unwrap_err(); + + assert!(matches!(error, WebhookDeliveryError::Transport(_))); + assert!(error.is_retryable()); + } } diff --git a/easytier/src/common/log/management.rs b/easytier/src/common/log/management.rs index 3ef39c9b..2c22007e 100644 --- a/easytier/src/common/log/management.rs +++ b/easytier/src/common/log/management.rs @@ -5,6 +5,14 @@ use crate::common::config::LoggingConfigLoader; use super::{FileSink, Logger, TargetFilter, install, parse_level}; pub fn init(config: impl LoggingConfigLoader, reload: bool) -> anyhow::Result<()> { + init_with_default_console_targets(config, reload, &[super::LOG_TARGET]) +} + +pub fn init_with_default_console_targets( + config: impl LoggingConfigLoader, + reload: bool, + default_targets: &[&str], +) -> anyhow::Result<()> { let console_config = config.get_console_logger_config(); let console_level = console_config .level @@ -12,7 +20,7 @@ pub fn init(config: impl LoggingConfigLoader, reload: bool) -> anyhow::Result<() .map(parse_level) .transpose() .context("invalid console log level")?; - let console = TargetFilter::console(console_level)?; + let console = TargetFilter::console_with_default_targets(console_level, default_targets)?; let file = FileSink::from_config(config.get_file_logger_config(), reload)?; install(Logger::new(console, file)) diff --git a/easytier/src/common/log/mod.rs b/easytier/src/common/log/mod.rs index e918f055..4f1efa16 100644 --- a/easytier/src/common/log/mod.rs +++ b/easytier/src/common/log/mod.rs @@ -20,7 +20,7 @@ mod file; #[cfg(feature = "management")] mod management; #[cfg(feature = "management")] -pub use management::init; +pub use management::{init, init_with_default_console_targets}; mod tracing_backend; use file::FileSink; @@ -107,6 +107,13 @@ struct TargetFilter { impl TargetFilter { fn console(level: Option) -> anyhow::Result { + Self::console_with_default_targets(level, &[LOG_TARGET]) + } + + fn console_with_default_targets( + level: Option, + default_targets: &[&str], + ) -> anyhow::Result { if level == Some(LevelFilter::Off) { return Ok(Self::off()); } @@ -115,7 +122,10 @@ impl TargetFilter { Some(level) => Self::with_default(level), None => Self { default: LevelFilter::Off, - targets: vec![(LOG_TARGET.into(), LevelFilter::Info)], + targets: default_targets + .iter() + .map(|target| ((*target).into(), LevelFilter::Info)) + .collect(), }, }; Self::from_environment(fallback) @@ -452,6 +462,20 @@ mod tests { assert!(!filter.enabled("other", Level::Error)); } + #[test] + #[serial_test::serial] + fn additional_default_console_targets_are_scoped() { + let _env = EnvVarGuard::set(None); + let filter = + TargetFilter::console_with_default_targets(None, &[LOG_TARGET, "easytier_web"]) + .unwrap(); + + assert!(filter.enabled("CORE::peer", Level::Info)); + assert!(filter.enabled("easytier_web::client_manager", Level::Info)); + assert!(!filter.enabled("easytier_web", Level::Debug)); + assert!(!filter.enabled("other", Level::Error)); + } + #[test] fn rust_log_supports_global_and_target_levels() { let filter = TargetFilter::parse("warn,easytier_core=debug,hyper=off") diff --git a/easytier/src/tests/credential_tests.rs b/easytier/src/tests/credential_tests.rs index ad6c8d44..867d0820 100644 --- a/easytier/src/tests/credential_tests.rs +++ b/easytier/src/tests/credential_tests.rs @@ -13,6 +13,12 @@ use easytier_core::{ process_runtime::CoreProcessRuntime, }; +#[cfg(feature = "wireguard")] +use crate::{ + common::config::{VpnPortalClientConfig, VpnPortalConfig}, + tests::three_node::{run_wireguard_client, wireguard_ifname}, + vpn_portal::wireguard::test_wireguard_keys, +}; use crate::{ common::{ config::{ConfigLoader, NetworkIdentity, PeerConfig, TomlConfigLoader}, @@ -23,6 +29,12 @@ use crate::{ tests::three_node::{generate_secure_mode_config, generate_secure_mode_config_with_key}, tunnel::common::tests::wait_for_condition, }; +#[cfg(feature = "wireguard")] +use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64_STANDARD}; +#[cfg(feature = "wireguard")] +use defguard_wireguard_rs::key::Key; +#[cfg(feature = "wireguard")] +use easytier_core::gateway::vpn_portal::PortalClientState; use super::{ InstanceTestExt as _, add_ns_to_bridge, create_netns, del_netns, drop_insts, ping_test, @@ -96,13 +108,8 @@ async fn set_prefer_peer_relay(inst: &Instance, prefer_peer_relay: bool) { ); } -fn forwarded_data_packets(inst: &Instance) -> u64 { - let labels = LabelSet::new().with_label_type(LabelType::NetworkName( - inst.get_global_ctx() - .get_network_identity() - .network_name - .clone(), - )); +fn forwarded_data_packets_for_network(inst: &Instance, network_name: &str) -> u64 { + let labels = LabelSet::new().with_label_type(LabelType::NetworkName(network_name.to_owned())); inst.get_core_instance() .metric_snapshots() .into_iter() @@ -112,6 +119,13 @@ fn forwarded_data_packets(inst: &Instance) -> u64 { .map_or(0, |metric| metric.value) } +fn forwarded_data_packets(inst: &Instance) -> u64 { + forwarded_data_packets_for_network( + inst, + &inst.get_global_ctx().get_network_identity().network_name, + ) +} + async fn assert_ping_forwarded_by( src_ns: &str, dst_ip: &str, @@ -714,6 +728,191 @@ async fn credential_peers_p2p_to_need_p2p_admin_through_public_server( .await; } +#[cfg(feature = "wireguard")] +#[tokio::test] +#[serial_test::serial] +async fn credential_peer_reconnects_to_admin_with_portal_client_online() { + prepare_credential_network(); + let process_runtime = CoreProcessRuntime::new(); + + let public_server_config = create_public_server_config(); + let mut public_server_flags = public_server_config.get_flags(); + public_server_flags.disable_relay_data = true; + public_server_config.set_flags(public_server_flags); + let mut public_server_inst = + Instance::new_with_process_runtime(public_server_config, process_runtime.clone()); + public_server_inst.run().await.unwrap(); + + let admin_config = create_need_p2p_admin_config("udp"); + admin_config.set_vpn_portal_config(VpnPortalConfig { + wireguard_listen: "0.0.0.0:22121".parse().unwrap(), + wireguard_private_key: Some(BASE64_STANDARD.encode([42u8; 32])), + clients: vec![VpnPortalClientConfig { + name: "portal-client".to_owned(), + virtual_ip: "10.154.0.10/24".parse().unwrap(), + groups: Vec::new(), + }], + }); + let mut admin_inst = Instance::new_with_process_runtime(admin_config, process_runtime.clone()); + admin_inst.run().await.unwrap(); + admin_inst.add_connector_url("udp://10.1.1.1:11010".parse().unwrap()); + wait_foreign_network_count(&public_server_inst, 1, Duration::from_secs(10)).await; + + let (_credential_id, credential_secret) = generate_credential_with_options( + &admin_inst, + Vec::new(), + false, + Vec::new(), + Duration::from_secs(3600), + Some("portal-p2p-credential".to_owned()), + false, + ) + .await; + admin_inst + .get_global_ctx() + .issue_event(GlobalCtxEvent::CredentialChanged); + + let credential_config = create_public_server_credential_config( + &credential_secret, + "portal-credential-peer", + "portal-credential-peer", + "ns_c1", + "10.154.0.1", + "fd00::1/64", + 11030, + 11031, + &[], + ); + let mut credential_inst = + Instance::new_with_process_runtime(credential_config, process_runtime); + credential_inst.run().await.unwrap(); + credential_inst.add_connector_url("udp://10.1.1.1:11010".parse().unwrap()); + + let admin_peer_id = admin_inst.peer_id(); + let credential_peer_id = credential_inst.peer_id(); + wait_direct_peer( + &credential_inst, + admin_peer_id, + Duration::from_secs(30), + "credential -> admin before portal client", + ) + .await; + wait_direct_peer( + &admin_inst, + credential_peer_id, + Duration::from_secs(10), + "admin -> credential before portal client", + ) + .await; + wait_route_cost( + &credential_inst, + admin_peer_id, + 1, + Duration::from_secs(10), + "credential route to admin before portal client", + ) + .await; + + let portal_config = admin_inst + .get_global_ctx() + .config + .get_vpn_portal_config() + .unwrap(); + let (server_public, client_private) = + test_wireguard_keys(&portal_config, "portal-client").unwrap(); + { + let net_ns = crate::common::netns::NetNS::new(Some("ns_c4".to_owned())); + let _guard = net_ns.guard(); + run_wireguard_client( + &wireguard_ifname("wg0"), + "10.1.1.4:22121".parse().unwrap(), + Key::try_from(server_public.as_slice()).unwrap(), + Key::try_from(client_private.as_slice()).unwrap(), + vec!["10.154.0.0/24".to_owned()], + "10.154.0.10".to_owned(), + ) + .unwrap(); + } + + let public_server_forwarded_before = + forwarded_data_packets_for_network(&public_server_inst, NEED_P2P_ADMIN_NETWORK_NAME); + wait_for_condition( + || async { + ping_test("ns_c4", "10.154.0.1", None).await; + admin_inst + .get_core_instance() + .vpn_portal_info() + .await + .clients + .iter() + .any(|client| client.state == PortalClientState::Online) + }, + Duration::from_secs(10), + ) + .await; + + let old_conn_ids = credential_inst + .get_core_instance() + .peer_snapshots() + .await + .into_iter() + .find(|peer| peer.peer_id == admin_peer_id) + .map(|peer| peer.directly_connected_conns) + .unwrap_or_default(); + assert!( + !old_conn_ids.is_empty(), + "credential peer must have a direct admin connection to replace" + ); + for conn_id in &old_conn_ids { + credential_inst + .get_core_instance() + .close_peer_conn(admin_peer_id, conn_id) + .await + .unwrap(); + } + + wait_for_condition( + || async { + let has_new_connection = credential_inst + .get_core_instance() + .peer_snapshots() + .await + .into_iter() + .find(|peer| peer.peer_id == admin_peer_id) + .is_some_and(|peer| { + peer.directly_connected_conns + .iter() + .any(|conn_id| !old_conn_ids.contains(conn_id)) + }); + let has_direct_route = credential_inst + .get_core_instance() + .route_snapshots() + .await + .iter() + .any(|route| { + route.peer_id == admin_peer_id + && route.next_hop_peer_id == admin_peer_id + && route.cost == 1 + }); + has_new_connection && has_direct_route + }, + Duration::from_secs(30), + ) + .await; + + wait_ping_reachability("ns_c1", "10.154.0.10", true, Duration::from_secs(10)).await; + for _ in 0..3 { + assert!(ping_test("ns_c1", "10.154.0.10", None).await); + } + assert_eq!( + forwarded_data_packets_for_network(&public_server_inst, NEED_P2P_ADMIN_NETWORK_NAME), + public_server_forwarded_before, + "public server forwarded data despite disable_relay_data" + ); + + drop_insts(vec![public_server_inst, admin_inst, credential_inst]).await; +} + async fn create_generated_credential_config( admin_inst: &Instance, inst_name: &str, @@ -872,7 +1071,76 @@ async fn wait_stable_single_visible_peer_on_admins( assert!( start.elapsed() < timeout, - "timed out waiting for a stable single visible peer on both admins: a={:?} c={:?}", + "timed out waiting for a stable single visible peer on both admins after {:?}: a_has_a={} a_has_b={} c_has_a={} c_has_b={} a={:?} c={:?}", + start.elapsed(), + admin_a_has_a, + admin_a_has_b, + admin_c_has_a, + admin_c_has_b, + admin_a_routes.iter().map(|r| r.peer_id).collect::>(), + admin_c_routes.iter().map(|r| r.peer_id).collect::>() + ); + + tokio::time::sleep(Duration::from_secs(1)).await; + } +} + +async fn wait_stable_failover_visibility_on_admins( + admin_a_inst: &Instance, + admin_c_inst: &Instance, + present_peer_id: u32, + absent_peer_id: u32, + timeout: Duration, +) { + let start = std::time::Instant::now(); + let mut stable_samples = 0; + + loop { + let admin_a_routes = admin_a_inst.get_core_instance().route_snapshots().await; + let admin_c_routes = admin_c_inst.get_core_instance().route_snapshots().await; + + let admin_a_has_present = admin_a_routes.iter().any(|r| r.peer_id == present_peer_id); + let admin_c_has_present = admin_c_routes.iter().any(|r| r.peer_id == present_peer_id); + let admin_a_has_absent = admin_a_routes.iter().any(|r| r.peer_id == absent_peer_id); + let admin_c_has_absent = admin_c_routes.iter().any(|r| r.peer_id == absent_peer_id); + + let failover_stable = admin_a_has_present + && admin_c_has_present + && !admin_a_has_absent + && !admin_c_has_absent; + + println!( + "failover visibility: present={} a_has_present={} c_has_present={} absent={} a_has_absent={} c_has_absent={} stable={} samples={}", + present_peer_id, + admin_a_has_present, + admin_c_has_present, + absent_peer_id, + admin_a_has_absent, + admin_c_has_absent, + failover_stable, + stable_samples + ); + + if failover_stable { + stable_samples += 1; + } else { + stable_samples = 0; + } + + if stable_samples >= 3 { + return; + } + + assert!( + start.elapsed() < timeout, + "timed out waiting for stable failover visibility on both admins after {:?}: present={} a_has_present={} c_has_present={} absent={} a_has_absent={} c_has_absent={} a={:?} c={:?}", + start.elapsed(), + present_peer_id, + admin_a_has_present, + admin_c_has_present, + absent_peer_id, + admin_a_has_absent, + admin_c_has_absent, admin_a_routes.iter().map(|r| r.peer_id).collect::>(), admin_c_routes.iter().map(|r| r.peer_id).collect::>() ); @@ -2399,9 +2667,9 @@ async fn credential_non_reusable_across_two_admins_allows_only_one_peer() { let a_routes = admin_a_inst.get_core_instance().route_snapshots().await; let c_routes = admin_c_inst.get_core_instance().route_snapshots().await; a_routes.iter().any(|r| r.peer_id == admin_c_peer_id) - || c_routes.iter().any(|r| r.peer_id == admin_a_inst.peer_id()) + && c_routes.iter().any(|r| r.peer_id == admin_a_inst.peer_id()) }, - Duration::from_secs(10), + Duration::from_secs(20), ) .await; @@ -2519,15 +2787,11 @@ async fn credential_non_reusable_across_two_admins_allows_only_one_peer() { drop_insts(vec![cred_right_inst.take().unwrap()]).await; } - wait_for_condition( - || async { - let admin_a_routes = admin_a_inst.get_core_instance().route_snapshots().await; - let admin_c_routes = admin_c_inst.get_core_instance().route_snapshots().await; - admin_a_routes.iter().any(|r| r.peer_id == loser_peer_id) - && admin_c_routes.iter().any(|r| r.peer_id == loser_peer_id) - && !admin_a_routes.iter().any(|r| r.peer_id == winner_peer_id) - && !admin_c_routes.iter().any(|r| r.peer_id == winner_peer_id) - }, + wait_stable_failover_visibility_on_admins( + &admin_a_inst, + &admin_c_inst, + loser_peer_id, + winner_peer_id, Duration::from_secs(60), ) .await; diff --git a/easytier/src/tests/three_node.rs b/easytier/src/tests/three_node.rs index 764767ea..5ec7de7b 100644 --- a/easytier/src/tests/three_node.rs +++ b/easytier/src/tests/three_node.rs @@ -1692,7 +1692,7 @@ use defguard_wireguard_rs::{ InterfaceConfiguration, WGApi, WireguardInterfaceApi, host::Peer, key::Key, net::IpAddrMask, }; -fn wireguard_ifname(base: &str) -> String { +pub(super) fn wireguard_ifname(base: &str) -> String { if cfg!(target_os = "linux") || cfg!(target_os = "freebsd") { base.to_owned() } else { @@ -1701,7 +1701,7 @@ fn wireguard_ifname(base: &str) -> String { } #[allow(clippy::too_many_arguments)] -fn run_wireguard_client( +pub(super) fn run_wireguard_client( ifname: &str, endpoint: SocketAddr, peer_public_key: Key,