chore(homeserver): unify source of default config (#116)

* chore(homeserver): unify source of default configs

* add docstring

* introduce serde toml merge

* Fix defaults to be None

* simplify remove once cell

* add sample config

* fix and validate sample config

* Update pubky-homeserver/src/data_directory/config_toml.rs

Co-authored-by: Severin Alexander Bühler <8782386+SeverinAlexB@users.noreply.github.com>

* Update pubky-homeserver/src/data_directory/config_toml.rs

Co-authored-by: Severin Alexander Bühler <8782386+SeverinAlexB@users.noreply.github.com>

* Update pubky-homeserver/src/data_directory/persistent_data_dir.rs

Co-authored-by: Severin Alexander Bühler <8782386+SeverinAlexB@users.noreply.github.com>

* fix

---------

Co-authored-by: Severin Alexander Bühler <8782386+SeverinAlexB@users.noreply.github.com>

Cargo.lock

@@ -1899,9 +1899,9 @@ dependencies = [
 
 [[package]]
 name = "once_cell"
-version = "1.20.3"
+version = "1.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "945462a4b81e43c4e3ba96bd7b49d834c6f61198356aa858733bc4acf3cbe62e"
+checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d"
 
 [[package]]
 name = "opaque-debug"
@@ -2344,6 +2344,7 @@ dependencies = [
  "pubky-common",
  "reqwest",
  "serde",
+ "serde-toml-merge",
  "tempfile",
  "thiserror 2.0.12",
  "tokio",
@@ -2898,6 +2899,15 @@ dependencies = [
  "serde_derive",
 ]
 
+[[package]]
+name = "serde-toml-merge"
+version = "0.3.9"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5817202d670278fb4dded71a70bae5181e00112543b1313463b02d43fc2d9243"
+dependencies = [
+ "toml",
+]
+
 [[package]]
 name = "serde_bencode"
 version = "0.2.4"

pubky-homeserver/Cargo.toml

@@ -36,6 +36,7 @@ pubky-common = { version = "0.4.0-rc.0", path = "../pubky-common" }
 serde = { version = "1.0.217", features = ["derive"] }
 tokio = { version = "1.43.0", features = ["full"] }
 toml = "0.8.20"
+serde-toml-merge = "0.3.9"
 tower-cookies = "0.11.0"
 tower-http = { version = "0.6.2", features = ["cors", "trace"] }
 tracing = "0.1.41"

pubky-homeserver/config.sample.toml

@@ -39,11 +39,11 @@ admin_password = "admin"
 public_ip = "127.0.0.1"
 
 # The pubky tls port in case it differs from the pubky_listen_socket port.
-# Defaults to the pubky_listen_socket port.
+# If not set defaults to the pubky_listen_socket port.
 public_pubky_tls_port = 6287
 
 # The icann http port in case it differs from the icann_listen_socket port.
-# Defaults to the icann_listen_socket port.
+# If not set defaults to the icann_listen_socket port.
 public_icann_http_port = 80
 
 # An ICANN domain name is necessary to support legacy browsers
@@ -62,6 +62,7 @@ icann_domain = "localhost"
 user_keys_republisher_interval = 14400 # 4 hours in seconds
 
 # List of bootstrap nodes for the DHT.
+# If not set, the default pkarr bootstrap nodes will be used.
 # domain:port format.
 dht_bootstrap_nodes = [
     "router.bittorrent.com:6881",

pubky-homeserver/src/data_directory/config.default.toml

@@ -0,0 +1,23 @@
+[general]
+signup_mode = "token_required"
+lmdb_backup_interval_s = 0
+user_storage_quota_mb = 0
+
+[drive]
+pubky_listen_socket = "127.0.0.1:6287"
+icann_listen_socket = "127.0.0.1:6286"
+
+[admin]
+listen_socket = "127.0.0.1:6288"
+admin_password = "admin"
+
+[pkdns]
+public_ip = "127.0.0.1"
+icann_domain = "localhost"
+user_keys_republisher_interval = 14400 # 4 hours in seconds
+# The following params exist, but the default value is None. 
+# See ./sample.config.toml for usage
+# public_icann_http_port =
+# dht_bootstrap_nodes =
+# dht_relay_nodes =
+# dht_request_timeout_ms =

pubky-homeserver/src/data_directory/config_toml.rs

@@ -1,225 +1,152 @@
-//!
 //! Configuration file for the homeserver.
 //!
+//! All default values live exclusively in `config.default.toml`.
+//! This module embeds that file at compile-time, parses it once,
+//! and lets callers optionally layer their own TOML on top.
+
 use super::{domain_port::DomainPort, Domain, SignupMode};
 use serde::{Deserialize, Serialize};
 use std::{
     fmt::Debug,
-    net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4},
+    fs,
+    net::{IpAddr, SocketAddr},
     num::NonZeroU64,
+    path::Path,
     str::FromStr,
 };
 use url::Url;
 
-/// Default TOML configuration for the homeserver.
-/// This is used to create a default config file if it doesn't exist.
-/// Why not use the Default trait? The `toml` crate doesn't support adding comments.
-/// So we maintain this default manually.
-pub const DEFAULT_CONFIG: &str = include_str!("../../config.default.toml");
+/// Embedded copy of the default configuration (single source of truth for defaults)
+pub const DEFAULT_CONFIG: &str = include_str!("config.default.toml");
+
+/// Example configuration file
+pub const SAMPLE_CONFIG: &str = include_str!("../../config.sample.toml");
+
+/// Error that can occur when reading a configuration file.
+#[derive(Debug, thiserror::Error)]
+pub enum ConfigReadError {
+    /// The file did not exist or could not be read.
+    #[error("config file not found: {0}")]
+    ConfigFileNotFound(#[from] std::io::Error),
+    /// The TOML was syntactically invalid.
+    #[error("config file is not valid TOML: {0}")]
+    ConfigFileNotValid(#[from] toml::de::Error),
+    /// Failed to merge defaults with overrides.
+    #[error("failed to merge embedded and user TOML: {0}")]
+    ConfigMergeError(String),
+}
+
+/// Config structs
 
-/// All configuration related to the DHT
-/// and /pkarr.
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
 pub struct PkdnsToml {
-    /// The public IP address and port of the server to be advertised in the DHT.
-    #[serde(default = "default_public_ip")]
     pub public_ip: IpAddr,
-
-    /// The public port of the Pubky TLS Drive API in case it's different from the listening port.
-    #[serde(default)]
     pub public_pubky_tls_port: Option<u16>,
-
-    /// The public port of the regular http API in case it's different from the listening port.
-    #[serde(default)]
     pub public_icann_http_port: Option<u16>,
-
-    /// Optional domain name of the regular http API.
-    #[serde(default = "default_icann_domain")]
     pub icann_domain: Option<Domain>,
-
-    /// The interval at which the user keys are republished in the DHT.
-    /// 0 means disabled.
-    #[serde(default = "default_user_keys_republisher_interval")]
     pub user_keys_republisher_interval: u64,
-
-    /// The list of bootstrap nodes for the DHT. If None, the default pkarr bootstrap nodes will be used.
-    #[serde(default = "default_dht_bootstrap_nodes")]
     pub dht_bootstrap_nodes: Option<Vec<DomainPort>>,
-
-    /// The list of relay nodes for the DHT.
-    /// If not set and no bootstrap nodes are set, the default pkarr relay nodes will be used.
-    #[serde(default = "default_dht_relay_nodes")]
     pub dht_relay_nodes: Option<Vec<Url>>,
-
-    /// The request timeout for the DHT. If None, the default pkarr request timeout will be used.
-    #[serde(default = "default_dht_request_timeout")]
     pub dht_request_timeout_ms: Option<NonZeroU64>,
 }
 
-impl Default for PkdnsToml {
-    fn default() -> Self {
-        Self {
-            public_ip: default_public_ip(),
-            public_pubky_tls_port: Option::default(),
-            public_icann_http_port: Option::default(),
-            icann_domain: default_icann_domain(),
-            user_keys_republisher_interval: default_user_keys_republisher_interval(),
-            dht_bootstrap_nodes: default_dht_bootstrap_nodes(),
-            dht_relay_nodes: default_dht_relay_nodes(),
-            dht_request_timeout_ms: default_dht_request_timeout(),
-        }
-    }
-}
-
-fn default_public_ip() -> IpAddr {
-    IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))
-}
-
-fn default_dht_bootstrap_nodes() -> Option<Vec<DomainPort>> {
-    None
-}
-
-fn default_dht_relay_nodes() -> Option<Vec<Url>> {
-    None
-}
-
-fn default_dht_request_timeout() -> Option<NonZeroU64> {
-    None
-}
-
-fn default_user_keys_republisher_interval() -> u64 {
-    // 4 hours
-    14400
-}
-
-fn default_icann_domain() -> Option<Domain> {
-    Some(Domain::from_str("localhost").expect("localhost is a valid domain"))
-}
-
-/// All configuration related to file drive
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
 pub struct DriveToml {
-    /// The port on which the Pubky TLS Drive API will listen.
-    #[serde(default = "default_pubky_drive_listen_socket")]
     pub pubky_listen_socket: SocketAddr,
-    /// The port on which the regular http API will listen.
-    #[serde(default = "default_icann_drive_listen_socket")]
     pub icann_listen_socket: SocketAddr,
 }
 
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
+pub struct AdminToml {
+    pub listen_socket: SocketAddr,
+    pub admin_password: String,
+}
+
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Default)]
+pub struct GeneralToml {
+    pub signup_mode: SignupMode,
+    pub lmdb_backup_interval_s: u64,
+    pub user_storage_quota_mb: u64,
+}
+
+/// The overall application configuration, composed of several subsections.
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
+pub struct ConfigToml {
+    /// General application settings (signup mode, quotas, backups).
+    pub general: GeneralToml,
+    /// File‐drive API settings (listen sockets for Pubky TLS and HTTP).
+    pub drive: DriveToml,
+    /// Administrative API settings (listen socket and password).
+    pub admin: AdminToml,
+    /// Peer‐to‐peer DHT / PKDNS settings (public endpoints, bootstrap, relays).
+    pub pkdns: PkdnsToml,
+}
+
+impl Default for ConfigToml {
+    fn default() -> Self {
+        ConfigToml::from_str(DEFAULT_CONFIG).expect("Embedded config.default.toml must be valid")
+    }
+}
+
 impl Default for DriveToml {
     fn default() -> Self {
-        Self {
-            pubky_listen_socket: default_pubky_drive_listen_socket(),
-            icann_listen_socket: default_icann_drive_listen_socket(),
-        }
+        ConfigToml::default().drive
     }
 }
 
-fn default_pubky_drive_listen_socket() -> SocketAddr {
-    SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 6287))
-}
-
-fn default_icann_drive_listen_socket() -> SocketAddr {
-    SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 6286))
-}
-
-/// All configuration related to the admin API
-#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
-pub struct AdminToml {
-    /// The socket on which the admin API will listen.
-    #[serde(default = "default_admin_listen_socket")]
-    pub listen_socket: SocketAddr,
-    /// The password for the admin API.
-    #[serde(default = "default_admin_password")]
-    pub admin_password: String,
-}
-
 impl Default for AdminToml {
     fn default() -> Self {
-        Self {
-            listen_socket: default_admin_listen_socket(),
-            admin_password: default_admin_password(),
-        }
+        ConfigToml::default().admin
     }
 }
 
-fn default_admin_password() -> String {
-    "admin".to_string()
-}
-
-fn default_admin_listen_socket() -> SocketAddr {
-    SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 6288))
-}
-
-/// All configuration related to the admin API
-#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Default)]
-pub struct GeneralToml {
-    /// The mode of the signup.
-    #[serde(default)]
-    pub signup_mode: SignupMode,
-    /// LMDB backup interval in seconds. 0 means disabled.
-    #[serde(default)]
-    pub lmdb_backup_interval_s: u64,
-    /// Per‑user storage quota in MB (0 = unlimited, defaults 0).
-    #[serde(default)]
-    pub user_storage_quota_mb: u64,
-}
-
-/// The error that can occur when reading the config file
-#[derive(Debug, thiserror::Error)]
-pub enum ConfigReadError {
-    /// The config file not found
-    #[error("Config file not found. {0}")]
-    ConfigFileNotFound(#[from] std::io::Error),
-    /// The config file is not valid    
-    #[error("Config file is not valid. {0}")]
-    ConfigFileNotValid(#[from] toml::de::Error),
-}
-
-/// The main server configuration
-#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
-pub struct ConfigToml {
-    /// The configuration for the general settings.
-    #[serde(default)]
-    pub general: GeneralToml,
-    /// The configuration for the drive files.
-    #[serde(default)]
-    pub drive: DriveToml,
-    /// The configuration for the admin API.
-    #[serde(default)]
-    pub admin: AdminToml,
-    /// The configuration for the pkdns.
-    #[serde(default)]
-    pub pkdns: PkdnsToml,
+impl Default for PkdnsToml {
+    fn default() -> Self {
+        ConfigToml::default().pkdns
+    }
 }
 
 impl ConfigToml {
-    /// Reads the configuration from a TOML file at the specified path.
+    /// Read and parse a configuration file, overlaying it on top of the embedded defaults.
     ///
     /// # Arguments
     /// * `path` - The path to the TOML configuration file
     ///
     /// # Returns
     /// * `Result<ConfigToml>` - The parsed configuration or an error if reading/parsing fails
-    pub fn from_file(path: impl AsRef<std::path::Path>) -> Result<Self, ConfigReadError> {
-        let contents = std::fs::read_to_string(path)?;
-        let config: ConfigToml = contents.parse()?;
-        Ok(config)
+    pub fn from_file(path: impl AsRef<Path>) -> Result<Self, ConfigReadError> {
+        let raw = fs::read_to_string(path)?;
+        Self::from_str_with_defaults(&raw)
     }
 
-    /// Returns the default config with all variables commented out.
-    pub fn default_string() -> String {
-        // Comment out all variables so they are not fixed by default.
-        DEFAULT_CONFIG
-            .split("\n")
-            .map(|line| {
-                let is_title = line.starts_with("[");
-                let is_comment = line.starts_with("#");
-                let is_empty = line.is_empty();
+    /// Parse a raw TOML string, overlaying it on top of the embedded defaults.
+    pub fn from_str_with_defaults(raw: &str) -> Result<Self, ConfigReadError> {
+        // 1. Parse the embedded defaults
+        let default_val: toml::Value = DEFAULT_CONFIG
+            .parse()
+            .expect("embedded defaults invalid TOML");
 
-                let is_other = !is_title && !is_comment && !is_empty;
-                if is_other {
+        // 2. Parse the user's overrides
+        let user_val: toml::Value = raw.parse()?;
+
+        // 3. Deep‐merge
+        let merged_val = serde_toml_merge::merge(default_val, user_val)
+            .map_err(|e| ConfigReadError::ConfigMergeError(e.to_string()))?;
+
+        // 4. Deserialize into our strongly typed struct (can fail with toml::de::Error)
+        Ok(merged_val.try_into()?)
+    }
+
+    /// Render the embedded sample config but comment out every value,
+    /// producing a handy template for end-users.
+    pub fn sample_string() -> String {
+        SAMPLE_CONFIG
+            .lines()
+            .map(|line| {
+                let trimmed = line.trim_start();
+                let is_title = trimmed.starts_with('[');
+                let is_comment = trimmed.starts_with('#');
+                if !is_title && !is_comment && !trimmed.is_empty() {
                     format!("# {}", line)
                 } else {
                     line.to_string()
@@ -229,12 +156,11 @@ impl ConfigToml {
             .join("\n")
     }
 
-    /// Returns a default config appropriate for testing.
+    /// Returns a default config tuned for unit tests.
     pub fn test() -> Self {
         let mut config = Self::default();
-        // For easy testing, we set the signup mode to open.
         config.general.signup_mode = SignupMode::Open;
-        // Set the listen ports to randomly available ports so they don't conflict.
+        // Use ephemeral ports (0) so parallel tests don’t collide.
         config.drive.icann_listen_socket = SocketAddr::from(([127, 0, 0, 1], 0));
         config.drive.pubky_listen_socket = SocketAddr::from(([127, 0, 0, 1], 0));
         config.admin.listen_socket = SocketAddr::from(([127, 0, 0, 1], 0));
@@ -244,87 +170,78 @@ impl ConfigToml {
     }
 }
 
-impl Default for ConfigToml {
-    fn default() -> Self {
-        ConfigToml::default_string()
-            .parse()
-            .expect("Default config is always valid")
-    }
-}
-
 impl FromStr for ConfigToml {
     type Err = toml::de::Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let config: ConfigToml = toml::from_str(s)?;
-        Ok(config)
+        toml::from_str(s)
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::{
+        net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4},
+        str::FromStr,
+    };
 
     #[test]
     fn test_default_config() {
-        let c: ConfigToml = ConfigToml::default();
-
+        let c = ConfigToml::default();
         assert_eq!(c.general.signup_mode, SignupMode::TokenRequired);
         assert_eq!(c.general.user_storage_quota_mb, 0);
         assert_eq!(c.general.lmdb_backup_interval_s, 0);
         assert_eq!(
             c.drive.icann_listen_socket,
-            default_icann_drive_listen_socket()
+            SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 6286))
+        );
+        assert_eq!(
+            c.pkdns.icann_domain,
+            Some(Domain::from_str("localhost").unwrap())
         );
-        assert_eq!(c.pkdns.icann_domain, default_icann_domain());
-
         assert_eq!(
             c.drive.pubky_listen_socket,
-            default_pubky_drive_listen_socket()
+            SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 6287))
         );
-
-        assert_eq!(c.admin.listen_socket, default_admin_listen_socket());
-        assert_eq!(c.admin.admin_password, default_admin_password());
-
-        // Verify pkdns config
-        assert_eq!(c.pkdns.public_ip, default_public_ip());
+        assert_eq!(
+            c.admin.listen_socket,
+            SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 6288))
+        );
+        assert_eq!(c.admin.admin_password, "admin");
+        assert_eq!(c.pkdns.public_ip, IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)));
         assert_eq!(c.pkdns.public_pubky_tls_port, None);
         assert_eq!(c.pkdns.public_icann_http_port, None);
-        assert_eq!(
-            c.pkdns.user_keys_republisher_interval,
-            default_user_keys_republisher_interval()
-        );
+        assert_eq!(c.pkdns.user_keys_republisher_interval, 14400);
         assert_eq!(c.pkdns.dht_bootstrap_nodes, None);
         assert_eq!(c.pkdns.dht_relay_nodes, None);
-
         assert_eq!(c.pkdns.dht_request_timeout_ms, None);
     }
 
     #[test]
-    fn test_default_config_commented_out() {
-        // Sanity check that the default config is valid
-        // even when the variables are commented out.
-        let s = ConfigToml::default_string();
-        let parsed: ConfigToml = s.parse().expect("Failed to parse config");
-        assert_eq!(
-            parsed.pkdns.dht_bootstrap_nodes, None,
-            "dht_bootstrap_nodes not commented out"
-        );
+    fn test_sample_config() {
+        // Validate that the sample config can be parsed
+        ConfigToml::from_str(SAMPLE_CONFIG).expect("Embedded config.default.toml must be valid");
+    }
+
+    #[test]
+    fn test_sample_config_commented_out() {
+        // Sanity check that the sample config is valid even when the variables are commented out.
+        // An empty or fully commented out .toml should still be equal to the default ConfigToml
+        let s = ConfigToml::sample_string();
+        let parsed: ConfigToml =
+            ConfigToml::from_str_with_defaults(&s).expect("Should be valid config file");
+        assert_eq!(parsed, ConfigToml::default());
     }
 
     #[test]
     fn test_empty_config() {
         // Test that a minimal config with only the general section works
-        let s = "[general]
-        signup_mode = \"open\"
-        ";
-        let parsed: ConfigToml = s.parse().unwrap();
-
+        let s = "[general]\nsignup_mode = \"open\"\n";
+        let parsed: ConfigToml = ConfigToml::from_str_with_defaults(s).unwrap();
         // Check that explicitly set values are preserved
-        assert_eq!(
-            parsed.general.signup_mode,
-            SignupMode::Open,
-            "signup_mode not set correctly"
-        );
+        assert_eq!(parsed.general.signup_mode, SignupMode::Open);
+        // Other fields that were not set (left empty) should still match the default.
+        assert_eq!(parsed.admin, ConfigToml::default().admin);
     }
 }

pubky-homeserver/src/data_directory/persistent_data_dir.rs

@@ -49,8 +49,8 @@ impl PersistentDataDir {
         self.expanded_path.join("config.toml")
     }
 
-    fn write_default_config_file(&self) -> anyhow::Result<()> {
-        let config_string = ConfigToml::default_string();
+    fn write_sample_config_file(&self) -> anyhow::Result<()> {
+        let config_string = ConfigToml::sample_string();
         let config_file_path = self.get_config_file_path();
         let mut config_file = std::fs::File::create(config_file_path)?;
         config_file.write_all(config_string.as_bytes())?;
@@ -108,7 +108,7 @@ impl DataDir for PersistentDataDir {
     fn read_or_create_config_file(&self) -> anyhow::Result<ConfigToml> {
         let config_file_path = self.get_config_file_path();
         if !config_file_path.exists() {
-            self.write_default_config_file()?;
+            self.write_sample_config_file()?;
         }
         let config = ConfigToml::from_file(config_file_path)?;
         Ok(config)