diff --git a/radicle-httpd/Cargo.lock b/radicle-httpd/Cargo.lock index 444c2ebc1..34f08c7de 100644 --- a/radicle-httpd/Cargo.lock +++ b/radicle-httpd/Cargo.lock @@ -2867,6 +2867,7 @@ dependencies = [ "libc", "mio 1.0.2", "pin-project-lite", + "signal-hook-registry", "slab", "socket2", "tokio-macros", diff --git a/radicle-httpd/Cargo.toml b/radicle-httpd/Cargo.toml index f1aa6cbc8..0337a8ae6 100644 --- a/radicle-httpd/Cargo.toml +++ b/radicle-httpd/Cargo.toml @@ -39,7 +39,7 @@ radicle-term = { version = "0.14.0", default-features = false } serde = { version = "1", features = ["derive"] } serde_json = { version = "1", features = ["preserve_order"] } thiserror = { version = "2" } -tokio = { version = "1.47.1", default-features = false, features = ["macros", "rt-multi-thread"] } +tokio = { version = "1.47.1", default-features = false, features = ["macros", "rt-multi-thread", "signal"] } tower-http = { version = "0.6.6", default-features = false, features = ["trace", "cors", "set-header"] } tracing = { version = "0.1.41", default-features = false, features = ["std", "log"] } tracing-logfmt = { version = "0.3.5", optional = true } diff --git a/radicle-httpd/src/api.rs b/radicle-httpd/src/api.rs index aba962416..c37101bc6 100644 --- a/radicle-httpd/src/api.rs +++ b/radicle-httpd/src/api.rs @@ -14,7 +14,8 @@ use radicle::node::NodeId; use radicle::patch::cache::Patches as _; use radicle::storage::git::Repository; use radicle::storage::{ReadRepository, ReadStorage}; -use radicle::Profile; +use radicle::{web, Profile}; +use tokio::sync::RwLock; mod error; mod json; @@ -29,17 +30,54 @@ pub const RADICLE_VERSION: &str = env!("RADICLE_VERSION"); // This version has to be updated on every breaking change to the radicle-httpd API. pub const API_VERSION: &str = "6.1.0"; +/// Thread-safe wrapper around radicle's web configuration. +/// +/// This struct provides concurrent read/write access to web configuration +/// that can be dynamically reloaded (e.g., via SIGHUP) without restarting the server. +/// All access is synchronized via an async [`RwLock`] to prevent race conditions. +#[derive(Clone)] +pub struct WebConfig { + inner: Arc>, +} + +impl WebConfig { + /// Creates a new WebConfig from a [`Profile`]'s web configuration. + pub fn from_profile(profile: &Profile) -> Self { + let config = profile.config.web.clone(); + Self { + inner: Arc::new(RwLock::new(config)), + } + } + + /// Return the underlying web configuration. + pub async fn read(&self) -> web::Config { + self.inner.read().await.clone() + } + + /// Atomically updates the config by applying a function while holding the write lock. + /// This prevents lost updates when multiple tasks attempt concurrent modifications. + pub async fn update(&self, f: F) + where + F: FnOnce(&mut web::Config), + { + let mut config = self.inner.write().await; + f(&mut config); + } +} + #[derive(Clone)] pub struct Context { profile: Arc, cache: Option, + web_config: WebConfig, } impl Context { - pub fn new(profile: Arc, options: &Options) -> Self { + pub fn new(profile: Arc, web_config: WebConfig, options: &Options) -> Self { Self { - profile, + profile: profile.clone(), cache: options.cache.map(Cache::new), + web_config, } } @@ -111,6 +149,14 @@ impl Context { Ok((repo, doc)) } + /// Returns a reference to the thread-safe web configuration. + /// + /// Use this instead of accessing [`radicle::web::Config`] from the [`Profile`] to ensure + /// you get the latest config after dynamic reloads. + pub fn web_config(&self) -> &WebConfig { + &self.web_config + } + #[cfg(test)] pub fn profile(&self) -> &Arc { &self.profile @@ -255,3 +301,142 @@ mod repo { pub seeding: usize, } } + +#[cfg(test)] +mod tests { + use crate::test; + + #[tokio::test] + async fn test_web_config_accessor() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test::seed(tmp.path()); + + let config = ctx.web_config.read().await; + assert_eq!(config.pinned.repositories.len(), 0); + } + + #[tokio::test] + async fn test_web_config_reload_simulation() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test::seed(tmp.path()); + + { + let config = ctx.web_config.read().await; + assert_eq!(config.pinned.repositories.len(), 0); + assert_eq!(config.description, None); + } + + { + ctx.web_config + .update(|config| { + config.description = Some("Updated description".to_string()); + config.avatar_url = Some("https://example.com/avatar.png".to_string()); + }) + .await; + } + + { + let config = ctx.web_config.read().await; + assert_eq!(config.description, Some("Updated description".to_string())); + assert_eq!( + config.avatar_url, + Some("https://example.com/avatar.png".to_string()) + ); + } + } + + #[tokio::test] + async fn test_web_config_concurrent_reads() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test::seed(tmp.path()); + + let mut handles = vec![]; + for _ in 0..10 { + let ctx_clone = ctx.clone(); + let handle = tokio::spawn(async move { + let config = ctx_clone.web_config.read().await; + config.pinned.repositories.len() + }); + handles.push(handle); + } + + for handle in handles { + handle.await.unwrap(); + } + } + + #[tokio::test] + async fn test_web_config_preserves_data_across_reads() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test::seed(tmp.path()); + + { + ctx.web_config + .update(|config| { + config.banner_url = Some("https://example.com/banner.png".to_string()); + }) + .await; + } + + for _ in 0..5 { + let config = ctx.web_config.read().await; + assert_eq!( + config.banner_url, + Some("https://example.com/banner.png".to_string()) + ); + } + } + + #[tokio::test] + async fn test_profile_immutable_after_reload() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test::seed(tmp.path()); + let original_key = ctx.profile.public_key; + let original_home = ctx.profile.home.path().to_path_buf(); + + { + ctx.web_config + .update(|config| { + config.description = Some("Updated".to_string()); + config.avatar_url = Some("https://example.com/new-avatar.png".to_string()); + }) + .await; + } + + assert_eq!(ctx.profile.public_key, original_key); + assert_eq!(ctx.profile.home.path(), original_home); + } + + #[tokio::test] + async fn test_empty_pinned_repos_transitions() { + use radicle::identity::RepoId; + use std::str::FromStr; + + let tmp = tempfile::tempdir().unwrap(); + let ctx = test::seed(tmp.path()); + + assert_eq!(ctx.web_config.read().await.pinned.repositories.len(), 0); + + let rid1 = RepoId::from_str("rad:z4FucBZHZMCsxTyQE1dfE2YR59Qbp").unwrap(); + let rid2 = RepoId::from_str("rad:z4GypKmh1gkEfmkXtarcYnkvtFUfE").unwrap(); + + { + ctx.web_config + .update(|config| { + config.pinned.repositories.insert(rid1); + config.pinned.repositories.insert(rid2); + }) + .await; + } + assert_eq!(ctx.web_config.read().await.pinned.repositories.len(), 2); + + { + ctx.web_config + .update(|config| { + config.pinned.repositories.clear(); + }) + .await; + } + assert_eq!(ctx.web_config.read().await.pinned.repositories.len(), 0); + } +} diff --git a/radicle-httpd/src/api/v1/delegates.rs b/radicle-httpd/src/api/v1/delegates.rs index 322d53192..0935ccd99 100644 --- a/radicle-httpd/src/api/v1/delegates.rs +++ b/radicle-httpd/src/api/v1/delegates.rs @@ -32,7 +32,8 @@ async fn delegates_repos_handler( let page = page.unwrap_or(0); let per_page = per_page.unwrap_or(10); let storage = &ctx.profile.storage; - let pinned = &ctx.profile.config.web.pinned; + let web_config = ctx.web_config().read().await; + let pinned = &web_config.pinned; let mut repos = match show { RepoQuery::All => storage .repositories()? diff --git a/radicle-httpd/src/api/v1/node.rs b/radicle-httpd/src/api/v1/node.rs index d1bed55a1..4a09ce85b 100644 --- a/radicle-httpd/src/api/v1/node.rs +++ b/radicle-httpd/src/api/v1/node.rs @@ -92,7 +92,7 @@ async fn node_handler(State(ctx): State) -> impl IntoResponse { agent, config, node_state.to_string(), - ctx.profile.config.web.clone(), + ctx.web_config().read().await, ); Ok::<_, Error>(cached_response(response, 600)) @@ -265,4 +265,37 @@ mod routes { ] ); } + + #[tokio::test] + async fn test_node_uses_reloadable_config() { + let tmp = tempfile::tempdir().unwrap(); + let seed = seed(tmp.path()); + + { + seed.web_config + .update(|config| { + config.description = Some("Test node description".to_string()); + config.avatar_url = Some("https://example.com/avatar.png".to_string()); + config.banner_url = Some("https://example.com/banner.png".to_string()); + }) + .await; + } + + let app = super::router(seed.clone()) + .layer(MockConnectInfo(SocketAddr::from(([127, 0, 0, 1], 8080)))); + let response = get(&app, "/node").await; + + assert_eq!(response.status(), StatusCode::OK); + let json_response = response.json().await; + + assert_eq!(json_response["description"], json!("Test node description")); + assert_eq!( + json_response["avatarUrl"], + json!("https://example.com/avatar.png") + ); + assert_eq!( + json_response["bannerUrl"], + json!("https://example.com/banner.png") + ); + } } diff --git a/radicle-httpd/src/api/v1/repos.rs b/radicle-httpd/src/api/v1/repos.rs index 6a0aceb59..778893f27 100644 --- a/radicle-httpd/src/api/v1/repos.rs +++ b/radicle-httpd/src/api/v1/repos.rs @@ -61,12 +61,13 @@ async fn repo_root_handler( per_page, } = qs; let page = page.unwrap_or(0); + let web_config = ctx.web_config().read().await; let per_page = per_page.unwrap_or_else(|| match show { - RepoQuery::Pinned => ctx.profile.config.web.pinned.repositories.len(), + RepoQuery::Pinned => web_config.pinned.repositories.len(), _ => 10, }); let storage = &ctx.profile.storage; - let pinned = &ctx.profile.config.web.pinned; + let pinned = &web_config.pinned; let policies = ctx.profile.policies()?; let mut repos = match show { @@ -1846,4 +1847,35 @@ mod routes { let response = get(&app, format!("/repos/{RID_PRIVATE}/remotes")).await; assert_eq!(response.status(), StatusCode::NOT_FOUND); } + + #[tokio::test] + async fn test_repos_uses_reloadable_pinned_config() { + use radicle::identity::RepoId; + use std::str::FromStr; + + let tmp = tempfile::tempdir().unwrap(); + let seed = seed(tmp.path()); + + let app = super::router(seed.clone()) + .layer(MockConnectInfo(SocketAddr::from(([127, 0, 0, 1], 8080)))); + let response = get(&app, "/repos?show=pinned").await; + assert_eq!(response.status(), StatusCode::OK); + let repos = response.json().await; + assert_eq!(repos.as_array().unwrap().len(), 0); + + { + let rid = RepoId::from_str(RID).unwrap(); + seed.web_config + .update(|config| { + config.pinned.repositories.insert(rid); + }) + .await; + } + + let response = get(&app, "/repos?show=pinned").await; + assert_eq!(response.status(), StatusCode::OK); + let repos = response.json().await; + assert_eq!(repos.as_array().unwrap().len(), 1); + assert_eq!(repos[0]["rid"], json!(RID)); + } } diff --git a/radicle-httpd/src/lib.rs b/radicle-httpd/src/lib.rs index 2f6af59ba..4c36a234f 100644 --- a/radicle-httpd/src/lib.rs +++ b/radicle-httpd/src/lib.rs @@ -10,6 +10,8 @@ use std::str; use std::sync::Arc; use std::time::Duration; +use tokio::signal::unix::{signal, SignalKind}; + use anyhow::Context as _; use axum::body::Body; use axum::http::Request; @@ -68,7 +70,35 @@ pub async fn run(options: Options) -> anyhow::Result<()> { tracing::info!("using radicle home at {}", profile.home().path().display()); - let app = router(options, profile)? + let web_config = api::WebConfig::from_profile(&profile); + let profile = Arc::new(profile); + let ctx = api::Context::new(profile.clone(), web_config.clone(), &options); + + tokio::spawn(async move { + let mut sighup = signal(SignalKind::hangup()).expect("Failed to register SIGHUP handler"); + + loop { + sighup.recv().await; + tracing::info!("Received SIGHUP, reloading web configuration"); + + match Profile::load() { + Ok(new_profile) => { + web_config + .update(|config| { + *config = new_profile.config.web.clone(); + }) + .await; + tracing::info!("Web configuration reloaded successfully"); + } + Err(e) => { + tracing::error!("Failed to reload configuration: {:#}", e); + tracing::warn!("Continuing with previous configuration"); + } + } + } + }); + + let app = router(options, profile, ctx)? .layer(middleware::from_fn(tracing_middleware)) .layer( TraceLayer::new_for_http() @@ -116,10 +146,7 @@ pub async fn run(options: Options) -> anyhow::Result<()> { } /// Create a router consisting of other sub-routers. -fn router(options: Options, profile: Profile) -> anyhow::Result { - let profile = Arc::new(profile); - let ctx = api::Context::new(profile.clone(), &options); - +fn router(options: Options, profile: Arc, ctx: api::Context) -> anyhow::Result { let api_router = api::router(ctx); let git_router = git::router(profile.clone(), options.aliases); let raw_router = raw::router(profile); @@ -217,19 +244,21 @@ mod routes { #[tokio::test] async fn test_invalid_route_returns_404() { let tmp = tempfile::tempdir().unwrap(); - let app = super::router( - super::Options { - aliases: HashMap::new(), - listen: DualAddr::Tcp(SocketAddr::from(([0, 0, 0, 0], 8080))), - cache: None, - }, - test::profile(tmp.path(), [0xff; 32]), - ) - .unwrap() - .layer(MockConnectInfo(DualAddr::Tcp(SocketAddr::from(( - [0, 0, 0, 0], - 8080, - ))))); + let options = super::Options { + aliases: HashMap::new(), + listen: DualAddr::Tcp(SocketAddr::from(([0, 0, 0, 0], 8080))), + cache: None, + }; + let profile = test::profile(tmp.path(), [0xff; 32]); + let web_config = crate::api::WebConfig::from_profile(&profile); + let profile = std::sync::Arc::new(profile); + let ctx = crate::api::Context::new(profile.clone(), web_config, &options); + let app = super::router(options, profile, ctx) + .unwrap() + .layer(MockConnectInfo(DualAddr::Tcp(SocketAddr::from(( + [0, 0, 0, 0], + 8080, + ))))); let response = test::get(&app, "/aa/a").await; diff --git a/radicle-httpd/src/test.rs b/radicle-httpd/src/test.rs index b8f401d10..5846559c9 100644 --- a/radicle-httpd/src/test.rs +++ b/radicle-httpd/src/test.rs @@ -330,7 +330,8 @@ fn seed_with_signer>( cache: Some(crate::DEFAULT_CACHE_SIZE), }; - Context::new(Arc::new(profile), &options) + let web_config = crate::api::WebConfig::from_profile(&profile); + Context::new(Arc::new(profile), web_config, &options) } pub async fn get(app: &Router, path: impl ToString) -> Response {