From c0fcef05945864f9c0f47d3f9ab6b6a233e97458 Mon Sep 17 00:00:00 2001 From: Ross Younger Date: Mon, 23 Dec 2024 13:05:47 +1300 Subject: [PATCH] feat: move to ssh style config files, with options for the targetted Host - Update error message output to suit - --show-config outputs settings for the job where possible - This overhauled DisplayAdapter::fmt() - Tweak source display for values read from files - add cli_beats_config_file test - remove toml scaffolding - update config module docs for file format --- Cargo.lock | 28 ---- Cargo.toml | 3 +- src/cli/args.rs | 25 ++- src/cli/cli_main.rs | 28 ++-- src/client/options.rs | 30 +++- src/config/manager.rs | 346 +++++++++++++-------------------------- src/config/mod.rs | 56 +++++-- src/config/ssh/errors.rs | 19 +++ src/config/ssh/values.rs | 2 +- src/util/port_range.rs | 9 +- 10 files changed, 241 insertions(+), 305 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa364f2..5cda830 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -537,7 +537,6 @@ checksum = "8cb01cd46b0cf372153850f4c6c272d9cbea2da513e07538405148f95bd789f3" dependencies = [ "atomic", "serde", - "toml", "uncased", "version_check", ] @@ -1120,7 +1119,6 @@ dependencies = [ "tempfile", "tokio", "tokio-util", - "toml", "tracing", "tracing-subscriber", "wildmatch", @@ -1432,15 +1430,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_spanned" -version = "0.6.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87607cb1398ed59d48732e575a4c28a7a8ebf2454b964fe3f224f2afc07909e1" -dependencies = [ - "serde", -] - [[package]] name = "serde_test" version = "1.0.177" @@ -1778,26 +1767,11 @@ dependencies = [ "tokio", ] -[[package]] -name = "toml" -version = "0.8.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1ed1f98e3fdc28d6d910e6737ae6ab1a93bf1985935a1193e68f93eeb68d24e" -dependencies = [ - "serde", - "serde_spanned", - "toml_datetime", - "toml_edit", -] - [[package]] name = "toml_datetime" version = "0.6.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0dd7358ecb8fc2f8d014bf86f6f638ce72ba252a2c3a2572f2a795f1d23efb41" -dependencies = [ - "serde", -] [[package]] name = "toml_edit" @@ -1806,8 +1780,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4ae48d6208a266e853d946088ed816055e556cc6028c5e8e2b84d9fa5dd7c7f5" dependencies = [ "indexmap", - "serde", - "serde_spanned", "toml_datetime", "winnow", ] diff --git a/Cargo.toml b/Cargo.toml index 59b2fe1..0d0c5eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ derive-deftly = "0.14.2" dirs = "5.0.1" dns-lookup = "2.0.4" expanduser = "1.2.2" -figment = { version = "0.10.19", features = ["toml"] } +figment = { version = "0.10.19" } futures-util = { version = "0.3.31", default-features = false } gethostname = "0.5.0" glob = "0.3.1" @@ -74,7 +74,6 @@ rand = "0.8.5" serde_json = "1.0.133" serde_test = "1.0.177" tempfile = "3.14.0" -toml = "0.8.19" [lints.rust] dead_code = "warn" diff --git a/src/cli/args.rs b/src/cli/args.rs index 98ce82c..d6be003 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -6,7 +6,7 @@ use clap::{ArgAction::SetTrue, Args as _, FromArgMatches as _, Parser}; use crate::{config::Manager, util::AddressFamily}; /// Options that switch us into another mode i.e. which don't require source/destination arguments -pub(crate) const MODE_OPTIONS: &[&str] = &["server", "help_buffers", "show_config", "config_files"]; +pub(crate) const MODE_OPTIONS: &[&str] = &["server", "help_buffers", "config_files", "show_config"]; #[derive(Debug, Parser, Clone)] #[command( @@ -44,7 +44,14 @@ pub(crate) struct CliArgs { )] pub server: bool, - /// Outputs the configuration, then exits + /// Outputs the configuration, then exits. + /// + /// If a remote `SOURCE` or `DESTINATION` argument is given, outputs the configuration we would use + /// for operations to that host. + /// + /// If not, outputs only global settings from configuration, which may be overridden in + /// `Host` blocks in configuration files. + /// #[arg(long, help_heading("Configuration"))] pub show_config: bool, /// Outputs the paths to configuration file(s), then exits @@ -101,12 +108,14 @@ impl CliArgs { } } -impl From<&CliArgs> for Manager { - /// Merge options from the CLI into the structure. - /// Any new option packs (_Optional structs) need to be added here. - fn from(value: &CliArgs) -> Self { - let mut mgr = Manager::standard(); +impl TryFrom<&CliArgs> for Manager { + type Error = anyhow::Error; + + fn try_from(value: &CliArgs) -> Result { + let host = value.client_params.remote_host_lossy()?; + + let mut mgr = Manager::standard(host.as_deref()); mgr.merge_provider(&value.config); - mgr + Ok(mgr) } } diff --git a/src/cli/cli_main.rs b/src/cli/cli_main.rs index fac2cac..4b5bfa7 100644 --- a/src/cli/cli_main.rs +++ b/src/cli/cli_main.rs @@ -3,10 +3,7 @@ use std::process::ExitCode; -use super::{ - args::CliArgs, - styles::{ERROR_S, INFO_S}, -}; +use super::{args::CliArgs, styles::ERROR_S}; use crate::{ client::{client_main, Parameters as ClientParameters, MAX_UPDATE_FPS}, config::{Configuration, Manager}, @@ -58,19 +55,19 @@ pub async fn cli() -> anyhow::Result { } // Now fold the arguments in with the CLI config (which may fail) - let config_manager = Manager::from(&args); + let config_manager = match Manager::try_from(&args) { + Ok(m) => m, + Err(err) => { + eprintln!("{}: {err}", "ERROR".style(*ERROR_S)); + return Ok(ExitCode::FAILURE); + } + }; let config = match config_manager.get::() { Ok(c) => c, Err(err) => { - println!("{}: Failed to parse configuration", "ERROR".style(*ERROR_S)); - if err.count() == 1 { - println!("{err}"); - } else { - for (i, e) in err.into_iter().enumerate() { - println!("{}: {e}", (i + 1).style(*INFO_S)); - } - } + eprintln!("{}: Failed to parse configuration", "ERROR".style(*ERROR_S)); + err.into_iter().for_each(|e| eprintln!("{e}")); return Ok(ExitCode::FAILURE); } }; @@ -84,10 +81,7 @@ pub async fn cli() -> anyhow::Result { .inspect_err(|e| eprintln!("{e:?}"))?; if args.show_config { - println!( - "{}", - config_manager.to_display_adapter::(true) - ); + println!("{}", config_manager.to_display_adapter::()); Ok(ExitCode::SUCCESS) } else if args.server { let _span = error_span!("REMOTE").entered(); diff --git a/src/client/options.rs b/src/client/options.rs index 4ae3b16..c38b807 100644 --- a/src/client/options.rs +++ b/src/client/options.rs @@ -54,8 +54,7 @@ pub struct Parameters { /// /// Exactly one of source and destination must be remote. #[arg( - conflicts_with_all(crate::cli::MODE_OPTIONS), - required = true, + required_unless_present_any(crate::cli::MODE_OPTIONS), value_name = "SOURCE" )] pub source: Option, @@ -66,8 +65,7 @@ pub struct Parameters { /// /// Exactly one of source and destination must be remote. #[arg( - conflicts_with_all(crate::cli::MODE_OPTIONS), - required = true, + required_unless_present_any(crate::cli::MODE_OPTIONS), value_name = "DESTINATION" )] pub destination: Option, @@ -98,3 +96,27 @@ impl TryFrom<&Parameters> for CopyJobSpec { }) } } + +impl Parameters { + /// A best-effort attempt to extract a single remote host string from the parameters. + /// + /// # Output + /// If neither source nor dest are present, Ok("") + /// If at most one of source and dest contains a remote host, Ok() + /// + /// # Errors + /// If both source and dest contain a remote host, Err("Only one remote file argument is supported") + pub(crate) fn remote_host_lossy(&self) -> anyhow::Result> { + let src_host = self.source.as_ref().and_then(|fs| fs.host.as_ref()); + let dst_host = self.destination.as_ref().and_then(|fs| fs.host.as_ref()); + Ok(if let Some(src_host) = src_host { + if dst_host.is_some() { + anyhow::bail!("Only one remote file argument is supported"); + } + Some(src_host.to_string()) + } else { + // Destination without source would be an exotic situation, but do our best anyway: + dst_host.map(std::string::ToString::to_string) + }) + } +} diff --git a/src/config/manager.rs b/src/config/manager.rs index 8663791..2fee980 100644 --- a/src/config/manager.rs +++ b/src/config/manager.rs @@ -3,21 +3,20 @@ use crate::os::{AbstractPlatform as _, Platform}; -use super::Configuration; +use super::{ssh::SshConfigError, Configuration}; -use figment::{ - providers::{Format, Serialized, Toml}, - value::Value, - Figment, Metadata, Provider, -}; +use figment::{providers::Serialized, value::Value, Figment, Metadata, Provider}; use serde::Deserialize; use std::{ - collections::{BTreeMap, HashSet}, + collections::HashSet, fmt::{Debug, Display}, path::{Path, PathBuf}, }; use struct_field_names_as_array::FieldNamesAsSlice; -use tabled::{settings::style::Style, Table, Tabled}; +use tabled::{ + settings::{object::Rows, style::Style, Color}, + Table, Tabled, +}; use tracing::{debug, warn}; @@ -57,6 +56,8 @@ impl Provider for SystemDefault { pub struct Manager { /// Configuration data data: Figment, + /// The host argument this data was read for, if applicable + host: Option, } impl Default for Manager { @@ -64,6 +65,7 @@ impl Default for Manager { fn default() -> Self { Self { data: Figment::default(), + host: None, } } } @@ -72,18 +74,24 @@ impl Manager { /// Initialises this structure, reading the set of config files appropriate to the platform /// and the current user. #[must_use] - pub fn standard() -> Self { + pub fn standard(for_host: Option<&str>) -> Self { let mut new1 = Self { data: Figment::new(), - //..Self::default() + host: for_host.map(std::borrow::ToOwned::to_owned), }; new1.merge_provider(SystemDefault::default()); // N.B. This may leave data in a fused-error state, if a config file isn't parseable. - new1.add_config("system", Platform::system_config_path()); - new1.add_config("user", Platform::user_config_path()); + new1.add_config(false, "system", Platform::system_config_path(), for_host); + new1.add_config(true, "user", Platform::user_config_path(), for_host); new1 } - fn add_config(&mut self, what: &str, path: Option) { + fn add_config( + &mut self, + is_user: bool, + what: &str, + path: Option, + for_host: Option<&str>, + ) { let Some(path) = path else { warn!("could not determine {what} configuration file path"); return; @@ -92,7 +100,7 @@ impl Manager { debug!("{what} configuration file {path:?} not present"); return; } - self.merge_provider(Toml::file(path.as_path())); + self.merge_ssh_config(path, for_host, is_user); } /// Returns the list of configuration files we read. @@ -111,12 +119,10 @@ impl Manager { /// Testing/internal constructor, does not read files from system #[must_use] #[cfg(test)] - pub(crate) fn without_files() -> Self { + pub(crate) fn without_files(host: Option<&str>) -> Self { let data = Figment::new().merge(SystemDefault::default()); - Self { - data, - //..Self::default() - } + let host = host.map(std::string::ToString::to_string); + Self { data, host } } /// Merges in a data set, which is some sort of [figment::Provider](https://docs.rs/figment/latest/figment/trait.Provider.html). @@ -130,26 +136,14 @@ impl Manager { self.data = f.merge(provider); // in the error case, this leaves the provider in a fused state } - /// Merges in a data set from a TOML file - #[allow(unused)] - pub(crate) fn merge_toml_file(&mut self, toml: T) - where - T: AsRef, - { - let path = toml.as_ref(); - let provider = Toml::file_exact(path); - self.merge_provider(provider); - } - /// Merges in a data set from an ssh config file - pub fn merge_ssh_config(&mut self, file: F, host: &str) + pub fn merge_ssh_config(&mut self, file: F, host: Option<&str>, is_user: bool) where F: AsRef, { let path = file.as_ref(); - // TODO: differentiate between user and system configs (Include rules) - let p = super::ssh::Parser::for_path(file.as_ref(), true) - .and_then(|p| p.parse_file_for(Some(host))) + let p = super::ssh::Parser::for_path(file.as_ref(), is_user) + .and_then(|p| p.parse_file_for(host)) .map(|hc| self.merge_provider(hc.as_figment())); if let Err(e) = p { warn!("parsing {ff}: {e}", ff = path.to_string_lossy()); @@ -160,11 +154,21 @@ impl Manager { /// /// Within qcp, `T` is usually [Configuration], but it isn't intrinsically required to be. /// (This is useful for unit testing.) - pub fn get<'de, T>(&self) -> anyhow::Result + pub(crate) fn get<'de, T>(&self) -> anyhow::Result where T: Deserialize<'de>, { - self.data.extract_lossy::() + let profile = if let Some(host) = &self.host { + figment::Profile::new(host) + } else { + figment::Profile::Default + }; + + self.data + .clone() + .select(profile) + .extract_lossy::() + .map_err(SshConfigError::from) } } @@ -228,21 +232,11 @@ impl PrettyConfig { } } -static DEFAULT_EMPTY_MAP: BTreeMap = BTreeMap::new(); - -impl Display for Manager { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::fmt::Display::fmt(&self.display_everything_adapter(), f) - } -} - /// Pretty-printing type wrapper to Manager #[derive(Debug)] pub struct DisplayAdapter<'a> { /// Data source source: &'a Manager, - /// Whether to warn if unused fields are present - warn_on_unused: bool, /// The fields we want to output. (If empty, outputs everything.) fields: HashSet, } @@ -253,7 +247,7 @@ impl Manager { /// # Returns /// An ephemeral structure implementing `Display`. #[must_use] - pub fn to_display_adapter<'de, T>(&self, warn_on_unused: bool) -> DisplayAdapter<'_> + pub fn to_display_adapter<'de, T>(&self) -> DisplayAdapter<'_> where T: Deserialize<'de> + FieldNamesAsSlice, { @@ -261,23 +255,9 @@ impl Manager { fields.extend(T::FIELD_NAMES_AS_SLICE.iter().map(|s| String::from(*s))); DisplayAdapter { source: self, - warn_on_unused, fields, } } - - /// Creates a generic `DisplayAdapter` that outputs everything - /// - /// # Returns - /// An ephemeral structure implementing `Display`. - #[must_use] - pub fn display_everything_adapter(&self) -> DisplayAdapter<'_> { - DisplayAdapter { - source: self, - warn_on_unused: false, - fields: HashSet::::new(), - } - } } impl Display for DisplayAdapter<'_> { @@ -285,47 +265,39 @@ impl Display for DisplayAdapter<'_> { /// /// N.B. This function uses CLI styling. fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use crate::cli::styles::{ERROR_S, WARNING_S}; - use anstream::eprintln; - use owo_colors::OwoColorize as _; - - let data = match self.source.data.data() { - Ok(d) => d, - Err(e) => { - // This isn't terribly helpful as it doesn't have metadata attached; BUT attempting to get() a struct does. - eprintln!("{} {e}", "ERROR".style(*ERROR_S)); - return Ok(()); - } - }; - let profile = match data.first_key_value() { - None => &figment::Profile::Default, - Some((k, _)) => k, - }; - let data = data.get(profile).unwrap_or(&DEFAULT_EMPTY_MAP); + let mut data = self.source.data.clone(); let mut output = Vec::::new(); - - for field in data.keys() { - let meta = self.source.data.find_metadata(field); - if self.fields.is_empty() || self.fields.contains(field) { - let value = self.source.data.find_value(field); - let value = match value { - Ok(v) => v, - Err(e) => { - eprintln!("{}: error on {field}: {e}", "WARNING".style(*WARNING_S)); - continue; - } - }; + // First line of the table is special + let (host_string, host_colour) = if let Some(host) = &self.source.host { + let profile = figment::Profile::new(host); + data = data.select(profile); + (host.clone(), Color::FG_GREEN) + } else { + ("* (globals)".into(), Color::FG_CYAN) + }; + output.push(PrettyConfig { + field: "(Remote host)".into(), + value: host_string, + source: String::new(), + }); + + let mut keys = self.fields.iter().collect::>(); + keys.sort(); + + for field in keys { + if let Ok(value) = data.find_value(field) { + let meta = data.get_metadata(value.tag()); output.push(PrettyConfig::new(field, &value, meta)); - } else if self.warn_on_unused { - let source = PrettyConfig::render_source(meta); - eprintln!( - "{}: unrecognised field `{field}` in {source}", - "WARNING".style(*WARNING_S) - ); } } - write!(f, "{}", Table::new(output).with(Style::sharp())) + write!( + f, + "{}", + Table::new(output) + .modify(Rows::single(1), host_colour) + .with(Style::sharp()) + ) } } @@ -338,7 +310,7 @@ mod test { #[test] fn defaults() { - let mgr = Manager::without_files(); + let mgr = Manager::without_files(None); let result = mgr.get().unwrap(); let expected = Configuration::default(); assert_eq!(expected, result); @@ -356,53 +328,12 @@ mod test { ..Default::default() }; - let mut mgr = Manager::without_files(); + let mut mgr = Manager::without_files(None); mgr.merge_provider(entered); let result = mgr.get().unwrap(); assert_eq!(expected, result); } - #[test] - fn dump_config_cli_and_toml() { - // Not a unit test as such; this is a human test - let (path, _tempdir) = make_test_tempfile( - r#" - tx = 42 - congestion = "Bbr" - unused__ = 42 - "#, - "test.toml", - ); - let fake_cli = Configuration_Optional { - rtt: Some(999), - initial_congestion_window: Some(67890), - ..Default::default() - }; - let mut mgr = Manager::without_files(); - mgr.merge_toml_file(path); - mgr.merge_provider(fake_cli); - println!("{mgr}"); - } - - #[test] - fn unparseable_toml() { - // This is a semi unit test; there is one assert, but the secondary goal is that it outputs something sensible - let (path, _tempdir) = make_test_tempfile( - r" - a = 1 - rx 123 # this line is a syntax error - b = 2 - ", - "test.toml", - ); - let mut mgr = Manager::without_files(); - mgr.merge_toml_file(path); - let get = mgr.get::(); - assert!(get.is_err()); - println!("{}", get.unwrap_err()); - // println!("{mgr}"); - } - #[test] fn type_error() { // This is a semi unit test; this has a secondary goal of outputting something sensible @@ -414,86 +345,23 @@ mod test { let (path, _tempdir) = make_test_tempfile( r" - rx = true # invalid - rtt = 3.14159 # also invalid - magic_ = 42 + rx true # invalid + rtt 3.14159 # also invalid + magic_ 42 ", - "test.toml", + "test.conf", ); - let mut mgr = Manager::without_files(); - mgr.merge_toml_file(path); - // This TOML successfully merges into the config, but you can't extract the struct. + let mut mgr = Manager::without_files(None); + mgr.merge_ssh_config(path, None, false); + // This file successfully merges into the config, but you can't extract the struct. let err = mgr.get::().unwrap_err(); println!("Error: {err}"); - // TODO: Would really like a rich error message here pointing to the failing key and errant file. - // We get no metadata in the error :-( // But the config as a whole is not broken and other things can be extracted: let other_struct = mgr.get::().unwrap(); assert_eq!(other_struct.magic_, 42); } - #[test] - fn int_or_string() { - #[derive(Deserialize)] - struct Test { - t1: PortRange, - t2: PortRange, - t3: PortRange, - } - let (path, _tempdir) = make_test_tempfile( - r#" - t1 = 1234 - t2 = "2345" - t3 = "123-456" - "#, - "test.toml", - ); - let mut mgr = Manager::without_files(); - mgr.merge_toml_file(path); - let res = mgr.get::().unwrap(); - assert_eq!( - res.t1, - PortRange { - begin: 1234, - end: 1234 - } - ); - assert_eq!( - res.t2, - PortRange { - begin: 2345, - end: 2345 - } - ); - assert_eq!( - res.t3, - PortRange { - begin: 123, - end: 456 - } - ); - } - - #[test] - fn array_type() { - #[derive(Deserialize)] - struct Test { - ii: Vec, - } - - let (path, _tempdir) = make_test_tempfile( - r" - ii = [1,2,3,4,6] - ", - "test.toml", - ); - let mut mgr = Manager::without_files(); - mgr.merge_toml_file(path); - let result = mgr.get::().unwrap(); - assert_eq!(result.ii, vec![1, 2, 3, 4, 6]); - } - #[test] fn field_parse_failure() { #[derive(Debug, Deserialize)] @@ -502,13 +370,13 @@ mod test { } let (path, _tempdir) = make_test_tempfile( - r#" - _p = "234-123" - "#, - "test.toml", + r" + _p 234-123 + ", + "test.conf", ); - let mut mgr = Manager::without_files(); - mgr.merge_toml_file(path); + let mut mgr = Manager::without_files(None); + mgr.merge_ssh_config(path, None, true); let result = mgr.get::().unwrap_err(); println!("{result}"); assert!(result.to_string().contains("must be increasing")); @@ -520,7 +388,6 @@ mod test { struct Test { ssh_opt: Vec, } - // Bear in mind: in an ssh style config file, the first match for a particular keyword wins. let (path, _tempdir) = make_test_tempfile( r" @@ -531,15 +398,14 @@ mod test { ", "test.conf", ); - let mut mgr = Manager::default(); - mgr.merge_ssh_config(&path, "foo"); - //println!("{mgr}"); + let mut mgr = Manager::without_files(Some("foo")); + mgr.merge_ssh_config(&path, Some("foo"), false); + //println!("{}", mgr.to_display_adapter::(false)); let result = mgr.get::().unwrap(); assert_eq!(result.ssh_opt, vec!["a", "b", "c"]); - let mut mgr = Manager::without_files(); - mgr.merge_ssh_config(&path, "bar"); - //println!("{mgr}"); + let mut mgr = Manager::without_files(Some("bar")); + mgr.merge_ssh_config(&path, Some("bar"), false); let result = mgr.get::().unwrap(); assert_eq!(result.ssh_opt, vec!["d", "e", "f"]); } @@ -569,9 +435,9 @@ mod test { ", "test.conf", ); - let mut mgr = Manager::default(); - mgr.merge_ssh_config(&path, "foo"); - println!("{mgr}"); + let mut mgr = Manager::without_files(Some("foo")); + mgr.merge_ssh_config(&path, Some("foo"), false); + // println!("{mgr}"); let result = mgr.get::().unwrap(); assert_eq!( result, @@ -617,8 +483,8 @@ mod test { ) .expect("Unable to write tempfile"); // ... test it - let mut mgr = Manager::default(); - mgr.merge_ssh_config(&path, "foo"); + let mut mgr = Manager::without_files(Some("foo")); + mgr.merge_ssh_config(&path, Some("foo"), false); let result = mgr .get::() .inspect_err(|e| println!("ERROR: {e}")) @@ -649,9 +515,31 @@ mod test { "test.conf", ); let mut mgr = Manager::default(); - mgr.merge_ssh_config(&path, "foo"); + mgr.merge_ssh_config(&path, Some("foo"), false); //println!("{mgr:?}"); let err = mgr.get::().map_err(SshConfigError::from).unwrap_err(); println!("{err}"); } + + #[test] + fn cli_beats_config_file() { + // simulate a CLI + let entered = Configuration_Optional { + rx: Some(12345.into()), + ..Default::default() + }; + let (path, _tempdir) = make_test_tempfile( + r" + rx 66666 + ", + "test.conf", + ); + + let mut mgr = Manager::without_files(None); + mgr.merge_ssh_config(&path, Some("foo"), false); + // The order of merging mirrors what happens in Manager::try_from(&CliArgs) + mgr.merge_provider(entered); + let result = mgr.get::().unwrap(); + assert_eq!(12345, *result.rx); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index e10f425..584c765 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -3,8 +3,8 @@ //! //! qcp obtains run-time configuration from the following sources, in order: //! 1. Command-line options -//! 2. The user's configuration file (typically `~/.qcp.toml`) -//! 3. The system-wide configuration file (typically `/etc/qcp.toml`) +//! 2. The user's configuration file (typically `~/.qcp.conf`) +//! 3. The system-wide configuration file (typically `/etc/qcp.conf`) //! 4. Hard-wired defaults //! //! Each option may appear in multiple places, but only the first match is used. @@ -14,21 +14,55 @@ //! //! ## File format //! -//! Configuration files use the [TOML](https://toml.io/en/) format. -//! This is a textual `key=value` format that supports comments. +//! Configuration files use the same format as OpenSSH configuration files. +//! This is a textual `Key Value` format that supports comments. //! -//! **Note** Strings are quoted; booleans and integers are not. For example: +//! qcp supports `Host` directives with wildcard matching, and `Include` directives. +//! This allows you to tune your configuration for a range of network hosts. //! -//! ```toml -//! rx="5M" # we have 40Mbit download -//! tx=1000000 # we have 8Mbit upload; we could also have written this as "1M" -//! rtt=150 # servers we care about are an ocean away -//! congestion="bbr" # this works well for us +//! ### Example +//! +//! ```text +//! Host old-faithful +//! # This is an old server with a very limited CPU which we do not want to overstress +//! rx 125k +//! tx 0 +//! +//! Host *.internal.corp +//! # This is a nearby data centre which we have a dedicated 1Gbit connection to. +//! # We don't need to use qcp, but it's convenient to use one tool in our scripts. +//! rx 125M +//! tx 0 +//! rtt 10 +//! +//! # For all other hosts, try to maximise our VDSL +//! Host * +//! rx 5M # we have 40Mbit download +//! tx 1000000 # we have 8Mbit upload; we could also have written this as "1M" +//! rtt 150 # most servers we care about are an ocean away +//! congestion bbr # this works well for us //! ``` //! //! ## Configurable options //! //! The full list of supported fields is defined by [Configuration]. +//! +//! On the command line: +//! * `qcp --show-config` outputs a list of supported fields, their current values, and where each value came from. +//! * For an explanation of each field, refer to `qcp --help` . +//! * `qcp --config-files` outputs the list of configuration files for the current user and platform. +//! +//! ### Traps and tips +//! 1. Like OpenSSH, for each setting we use the value from the _first_ Host block we find that matches the remote hostname. +//! 1. Each setting is evaluated independently. +//! - In the example above, the `Host old-faithful` block sets an `rx` but does not set `rtt`. Any operations to `old-faithful` inherit `rtt 150` from the `Host *` block. +//! 1. The `tx` setting has a default value of 0, which means "use the active rx value". If you set `tx` in a `Host *` block, you probably want to set it explicitly everywhere you set `rx`. +//! +//! If you have a complicated config file we recommend you structure it as follows: +//! * Any global settings that are intended to apply to all hosts +//! * `Host` blocks; if you use wildcards, from most-specific to least-specific +//! * A `Host *` block to provide default settings to apply where no more specific value has been given +//! mod structure; pub use structure::Configuration; @@ -37,6 +71,6 @@ pub(crate) use structure::Configuration_Optional; mod manager; pub use manager::Manager; -pub(crate) const BASE_CONFIG_FILENAME: &str = "qcp.toml"; +pub(crate) const BASE_CONFIG_FILENAME: &str = "qcp.conf"; pub(crate) mod ssh; diff --git a/src/config/ssh/errors.rs b/src/config/ssh/errors.rs index c38995c..4e365fc 100644 --- a/src/config/ssh/errors.rs +++ b/src/config/ssh/errors.rs @@ -60,3 +60,22 @@ impl std::fmt::Display for SshConfigError { Ok(()) } } + +/// An iterator over all errors in an [`SshConfigError`] +pub(crate) struct IntoIter(::IntoIter); +impl Iterator for IntoIter { + type Item = SshConfigError; + + fn next(&mut self) -> Option { + self.0.next().map(std::convert::Into::into) + } +} + +impl IntoIterator for SshConfigError { + type Item = SshConfigError; + type IntoIter = IntoIter; + + fn into_iter(self) -> Self::IntoIter { + IntoIter(self.0.into_iter()) + } +} diff --git a/src/config/ssh/values.rs b/src/config/ssh/values.rs index dbe5039..3f581c1 100644 --- a/src/config/ssh/values.rs +++ b/src/config/ssh/values.rs @@ -44,7 +44,7 @@ impl figment::Provider for ValueProvider<'_> { Metadata::from( "configuration file", Source::Custom(format!( - "line {line} of {src}", + "{src} (line {line})", src = self.value.source, line = self.value.line_number )), diff --git a/src/util/port_range.rs b/src/util/port_range.rs index 8040cd6..7e9b356 100644 --- a/src/util/port_range.rs +++ b/src/util/port_range.rs @@ -12,11 +12,10 @@ use super::cli::IntOrString; /// /// Port 0 is allowed with the usual meaning ("any available port"), but 0 may not form part of a range. /// -/// In a configuration file, a range must be specified as a string. For example: -/// ```toml -/// remote_port=60000 # a single port can be an integer -/// remote_port="60000" # a single port can also be a string -/// remote_port="60000-60010" # a range must be specified as a string +/// In a configuration file, a range may specified as an integer or as a pair of ports. For example: +/// ```text +/// remote_port 60000 # a single port +/// remote_port 60000-60010 # a range /// ``` #[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize)] #[serde(from = "IntOrString", into = "String")]