fix(iroh-cli)!: Improve cli and configuration file

iroh-cli/Cargo.toml

@@ -72,6 +72,7 @@ nix = { version = "0.27", features = ["signal", "process"] }
 rand_xorshift = "0.3.0"
 regex = "1.10.3"
 testdir = "0.9.1"
+url = "2.5.0"
 walkdir = "2"
 
 [features]

iroh-cli/src/commands.rs

@@ -23,15 +23,15 @@ pub(crate) mod rpc;
 pub(crate) mod start;
 pub(crate) mod tags;
 
-/// iroh is a tool for syncing bytes
+/// iroh is a tool for building distributed apps
 /// https://iroh.computer/docs
 #[derive(Parser, Debug, Clone)]
 #[clap(version, verbatim_doc_comment)]
 pub(crate) struct Cli {
     #[clap(subcommand)]
     pub(crate) command: Commands,
 
-    /// Path to the configuration file.
+    /// Path to the configuration file, see https://iroh.computer/docs/reference/config.
     #[clap(long)]
     pub(crate) config: Option<PathBuf>,
 
@@ -47,7 +47,7 @@ pub(crate) struct Cli {
     #[clap(long)]
     pub(crate) rpc_addr: Option<SocketAddr>,
 
-    /// If set, metrics will be dumped in CSV format to the specified path at regular intervals (100ms).
+    /// Write metrics in CSV format at 100ms intervals. Disabled by default.
     #[clap(long)]
     pub(crate) metrics_dump_path: Option<PathBuf>,
 }

iroh-cli/src/commands/start.rs

@@ -10,7 +10,7 @@ use iroh::{
     net::relay::{RelayMap, RelayMode},
     node::RpcStatus,
 };
-use tracing::{info_span, Instrument};
+use tracing::{info_span, trace, Instrument};
 
 /// Whether to stop the node after running a command or run forever until stopped.
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
@@ -79,6 +79,7 @@ where
     F: FnOnce(iroh::client::Iroh) -> T + Send + 'static,
     T: Future<Output = Result<()>> + 'static,
 {
+    trace!(?config, "using config");
     let relay_map = config.relay_map()?;
 
     let spinner = create_spinner("Iroh booting...");

iroh-cli/src/config.rs

@@ -6,6 +6,7 @@ use std::{
     path::{Path, PathBuf},
     str::FromStr,
     sync::Arc,
+    time::Duration,
 };
 
 use anyhow::{anyhow, bail, Context, Result};
@@ -48,15 +49,18 @@ impl ConsolePaths {
 }
 
 /// The configuration for an iroh node.
-#[derive(PartialEq, Eq, Debug, Deserialize, Serialize, Clone)]
-#[serde(default)]
+// Please note that this is documented in the `iroh.computer` repository under
+// `src/app/docs/reference/config/page.mdx`.  Any changes to this need to be updated there.
+#[derive(PartialEq, Eq, Debug, Deserialize, Clone)]
+#[serde(default, deny_unknown_fields)]
 pub(crate) struct NodeConfig {
     /// The nodes for relay to use.
     pub(crate) relay_nodes: Vec<RelayNode>,
     /// How often to run garbage collection.
-    pub(crate) gc_policy: GcPolicy,
+    pub(crate) gc_policy: GcPolicyConfig,
     /// Bind address on which to serve Prometheus metrics
     pub(crate) metrics_addr: Option<SocketAddr>,
+    /// Configuration for the logfile.
     pub(crate) file_logs: super::logging::FileLogging,
     /// Path to dump metrics to in CSV format.
     pub(crate) metrics_dump_path: Option<PathBuf>,
@@ -82,7 +86,7 @@ impl Default for NodeConfig {
         };
         Self {
             relay_nodes: relay_nodes.into(),
-            gc_policy: GcPolicy::Disabled,
+            gc_policy: GcPolicyConfig::default(),
             metrics_addr: Some(([127, 0, 0, 1], 9090).into()),
             file_logs: Default::default(),
             metrics_dump_path: None,
@@ -91,8 +95,12 @@ impl Default for NodeConfig {
 }
 
 impl NodeConfig {
-    /// Create a config using defaults, and the passed in config file.
-    pub async fn load(file: Option<&Path>) -> Result<NodeConfig> {
+    /// Creates a config from default config file.
+    ///
+    /// If the *file* is `Some` the configuration will be read from it.  Otherwise the
+    /// default config file will be loaded.  If that is not present the default config will
+    /// be used.
+    pub(crate) async fn load(file: Option<&Path>) -> Result<NodeConfig> {
         let default_config = iroh_config_path(CONFIG_FILE_NAME)?;
 
         let config_file = match file {
@@ -107,7 +115,7 @@ impl NodeConfig {
         };
         let mut config = if let Some(file) = config_file {
             let config = tokio::fs::read_to_string(file).await?;
-            toml::from_str(&config)?
+            Self::load_toml(&config)?
         } else {
             Self::default()
         };
@@ -119,6 +127,11 @@ impl NodeConfig {
         Ok(config)
     }
 
+    fn load_toml(s: &str) -> Result<NodeConfig> {
+        let config = toml::from_str(s)?;
+        Ok(config)
+    }
+
     /// Constructs a `RelayMap` based on the current configuration.
     pub(crate) fn relay_map(&self) -> Result<Option<RelayMap>> {
         if self.relay_nodes.is_empty() {
@@ -128,6 +141,29 @@ impl NodeConfig {
     }
 }
 
+/// Serde-compatible configuration for [`GcPolicy`].
+///
+/// The [`GcPolicy`] struct is not amenable to TOML serialisation, this covers this gap.
+#[derive(PartialEq, Eq, Debug, Default, Deserialize, Clone)]
+#[serde(default, deny_unknown_fields, rename = "gc_policy")]
+pub(crate) struct GcPolicyConfig {
+    enabled: bool,
+    interval: Option<u64>,
+}
+
+impl From<GcPolicyConfig> for GcPolicy {
+    fn from(source: GcPolicyConfig) -> Self {
+        if source.enabled {
+            match source.interval {
+                Some(interval) => Self::Interval(Duration::from_secs(interval)),
+                None => Self::default(),
+            }
+        } else {
+            Self::Disabled
+        }
+    }
+}
+
 /// Environment for CLI and REPL
 ///
 /// This is cheaply cloneable and has interior mutability. If not running in the console
@@ -415,12 +451,132 @@ pub(crate) fn iroh_cache_path(file_name: &Path) -> Result<PathBuf> {
 
 #[cfg(test)]
 mod tests {
+    use std::net::{Ipv4Addr, Ipv6Addr};
+
+    use url::Url;
+
+    use crate::logging::{EnvFilter, Rotation};
+
     use super::*;
 
-    #[tokio::test]
-    async fn test_default_settings() {
-        let config = NodeConfig::load(None).await.unwrap();
+    #[test]
+    fn test_toml_invalid_field() {
+        let source = r#"
+          not_a_field = true
+        "#;
+        let res = NodeConfig::load_toml(source);
+        assert!(res.is_err());
+    }
+
+    #[test]
+    fn test_toml_relay_nodes() {
+        let source = r#"
+          [[relay_nodes]]
+          url = "https://example.org."
+          stun_only = false
+          stun_port = 123
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+
+        let expected = RelayNode {
+            url: Url::parse("https://example.org./").unwrap().into(),
+            stun_only: false,
+            stun_port: 123,
+        };
+        assert_eq!(config.relay_nodes, vec![expected]);
+    }
+
+    #[test]
+    fn test_toml_gc_policy() {
+        let source = r#"
+          [gc_policy]
+          enabled = false
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        assert_eq!(GcPolicy::from(config.gc_policy), GcPolicy::Disabled);
+
+        // Default interval should be used.
+        let source = r#"
+          [gc_policy]
+          enabled = true
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        let gc_policy = GcPolicy::from(config.gc_policy);
+        assert!(matches!(gc_policy, GcPolicy::Interval(_)));
+        assert_eq!(gc_policy, GcPolicy::default());
+
+        let source = r#"
+          [gc_policy]
+          enabled = true
+          interval = 1234
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        assert_eq!(
+            GcPolicy::from(config.gc_policy),
+            GcPolicy::Interval(Duration::from_secs(1234))
+        );
+
+        let source = r#"
+            [gc_policy]
+            not_a_field = true
+        "#;
+        let res = NodeConfig::load_toml(source);
+        assert!(res.is_err());
+    }
+
+    #[test]
+    fn test_toml_metrics_addr() {
+        let source = r#"
+            metrics_addr = "1.2.3.4:1234"
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        assert_eq!(
+            config.metrics_addr,
+            Some(SocketAddr::new(Ipv4Addr::new(1, 2, 3, 4).into(), 1234)),
+        );
+
+        let source = r#"
+            metrics_addr = "[123:456::789:abc]:1234"
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        assert_eq!(
+            config.metrics_addr,
+            Some(SocketAddr::new(
+                Ipv6Addr::new(0x123, 0x456, 0, 0, 0, 0, 0x789, 0xabc).into(),
+                1234
+            )),
+        );
+    }
 
-        assert_eq!(config.relay_nodes.len(), 2);
+    #[test]
+    fn test_toml_file_logs() {
+        let source = r#"
+            [file_logs]
+            rust_log = "iroh_net=trace"
+            max_files = 123
+            rotation = "daily"
+            dir = "/var/log/iroh"
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        assert_eq!(
+            config.file_logs.rust_log,
+            EnvFilter::from_str("iroh_net=trace").unwrap()
+        );
+        assert_eq!(config.file_logs.max_files, 123);
+        assert_eq!(config.file_logs.rotation, Rotation::Daily);
+        assert_eq!(config.file_logs.dir, Some(PathBuf::from("/var/log/iroh")));
+
+        let source = r#"
+            [file_logs]
+            rust_log = "info"
+        "#;
+        let config = NodeConfig::load_toml(source).unwrap();
+        assert_eq!(
+            config.file_logs.rust_log,
+            EnvFilter::from_str("info").unwrap()
+        );
+        assert_eq!(config.file_logs.max_files, 4);
+        assert_eq!(config.file_logs.rotation, Rotation::Hourly);
+        assert_eq!(config.file_logs.dir, None);
     }
 }

iroh-cli/src/logging.rs

@@ -97,8 +97,11 @@ pub(crate) fn init_terminal_logging() -> anyhow::Result<()> {
     Ok(())
 }
 
+/// Configuration for the logfiles.
+// Please note that this is documented in the `iroh.computer` repository under
+// `src/app/docs/reference/config/page.mdx`.  Any changes to this need to be updated there.
 #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)]
-#[serde(default)]
+#[serde(default, deny_unknown_fields)]
 pub(crate) struct FileLogging {
     /// RUST_LOG directive to filter file logs.
     pub(crate) rust_log: EnvFilter,

iroh-net/src/relay/map.rs

@@ -116,6 +116,8 @@ impl fmt::Display for RelayMap {
 /// Information on a specific relay server.
 ///
 /// Includes the Url where it can be dialed.
+// Please note that this is documented in the `iroh.computer` repository under
+// `src/app/docs/reference/config/page.mdx`.  Any changes to this need to be updated there.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, PartialOrd, Ord)]
 pub struct RelayNode {
     /// The [`RelayUrl`] where this relay server can be dialed.

iroh/src/node/builder.rs

@@ -802,6 +802,8 @@ impl<D: iroh_blobs::store::Store> ProtocolBuilder<D> {
 }
 
 /// Policy for garbage collection.
+// Please note that this is documented in the `iroh.computer` repository under
+// `src/app/docs/reference/config/page.mdx`.  Any changes to this need to be updated there.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub enum GcPolicy {
     /// Garbage collection is disabled.