Skip to content

Commit

Permalink
fix(cli): Respect the --cfg flag (#506)
Browse files Browse the repository at this point in the history
Use the --cfg flag to load the configuration file for `iroh`.

This does not affect the config files for any of the managed
processes like iroh-p2p etc.

* Make argument a slice rather than a vec, the vec was not really used.
* Test the priority order and document it.
  • Loading branch information
flub committed Nov 17, 2022
1 parent d463a3e commit e7bcac6
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 21 deletions.
4 changes: 2 additions & 2 deletions iroh-api/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ impl Api {
overrides_map: HashMap<String, String>,
) -> Result<Self> {
let cfg_path = iroh_config_path(CONFIG_FILE_NAME)?;
let sources = vec![Some(cfg_path.as_path()), config_path];
let sources = [Some(cfg_path.as_path()), config_path];
let config = make_config(
// default
Config::default(),
// potential config files
sources,
&sources,
// env var prefix for this config
ENV_PREFIX,
// map of present command line arguments
Expand Down
4 changes: 2 additions & 2 deletions iroh-gateway/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ async fn main() -> Result<()> {
let args = Args::parse();

let cfg_path = iroh_config_path(CONFIG_FILE_NAME)?;
let sources = vec![Some(cfg_path.as_path()), args.cfg.as_deref()];
let sources = [Some(cfg_path.as_path()), args.cfg.as_deref()];
let mut config = make_config(
// default
Config::default(),
// potential config files
sources,
&sources,
// env var prefix for this config
ENV_PREFIX,
// map of present command line arguments
Expand Down
4 changes: 2 additions & 2 deletions iroh-one/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ async fn main() -> Result<()> {
let args = Args::parse();

let cfg_path = iroh_config_path(CONFIG_FILE_NAME)?;
let sources = vec![Some(cfg_path.as_path()), args.cfg.as_deref()];
let sources = [Some(cfg_path.as_path()), args.cfg.as_deref()];
let mut config = make_config(
// default
Config::default(),
// potential config files
sources,
&sources,
// env var prefix for this config
ENV_PREFIX,
// map of present command line arguments
Expand Down
4 changes: 2 additions & 2 deletions iroh-p2p/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ fn main() -> Result<()> {

// TODO: configurable network
let cfg_path = iroh_config_path(CONFIG_FILE_NAME)?;
let sources = vec![Some(cfg_path.as_path()), args.cfg.as_deref()];
let sources = [Some(cfg_path.as_path()), args.cfg.as_deref()];
let network_config = make_config(
// default
Config::default_grpc(),
// potential config files
sources,
&sources,
// env var prefix for this config
ENV_PREFIX,
// map of present command line arguments
Expand Down
2 changes: 1 addition & 1 deletion iroh-store/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ async fn main() -> anyhow::Result<()> {
println!("Starting iroh-store, version {version}");

let config_path = iroh_config_path(CONFIG_FILE_NAME)?;
let sources = vec![Some(config_path.as_path()), args.cfg.as_deref()];
let sources = &[Some(config_path.as_path()), args.cfg.as_deref()];
let config_data_path = config_data_path(args.path.clone())?;
let config = make_config(
// default
Expand Down
3 changes: 3 additions & 0 deletions iroh-util/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,8 @@ thiserror = "1.0"
toml = "0.5.9"
tracing = "0.1.34"

[dev-dependencies]
testdir = "0.6.0"

[target.'cfg(unix)'.dev-dependencies]
nix = "0.25"
58 changes: 50 additions & 8 deletions iroh-util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,20 @@ impl Source for MetricsSource {
}
}

/// make a config using a default, file sources, environment variables, and commandline flag
/// overrides
/// Make a config using a default, files, environment variables, and commandline flags.
///
/// environment variables are expected to start with the `env_prefix`. Nested fields can be
/// Later items in the *file_paths* slice will have a higher priority than earlier ones.
///
/// Environment variables are expected to start with the *env_prefix*. Nested fields can be
/// accessed using `.`, if your environment allows env vars with `.`
///
/// Note: For the metrics configuration env vars, it is recommended to use the metrics specific
/// prefix `IROH_METRICS` to set a field in the metrics config. You can use the above dot notation to set
/// a metrics field, eg, `IROH_CONFIG_METRICS.SERVICE_NAME`, but only if your environment allows it
/// Note: For the metrics configuration env vars, it is recommended to use the metrics
/// specific prefix `IROH_METRICS` to set a field in the metrics config. You can use the
/// above dot notation to set a metrics field, eg, `IROH_CONFIG_METRICS.SERVICE_NAME`, but
/// only if your environment allows it
pub fn make_config<T, S, V>(
default: T,
file_paths: Vec<Option<&Path>>,
file_paths: &[Option<&Path>],
env_prefix: &str,
flag_overrides: HashMap<S, V>,
) -> Result<T>
Expand All @@ -166,7 +168,7 @@ where
let mut builder = Config::builder().add_source(default);

// layer on config options from files
for path in file_paths.into_iter().flatten() {
for path in file_paths.iter().flatten() {
if path.exists() {
let p = path.to_str().ok_or_else(|| anyhow::anyhow!("empty path"))?;
builder = builder.add_source(File::with_name(p));
Expand Down Expand Up @@ -238,6 +240,9 @@ pub fn increase_fd_limit() -> std::io::Result<u64> {

#[cfg(test)]
mod tests {
use serde::Deserialize;
use testdir::testdir;

use super::*;

#[test]
Expand All @@ -247,4 +252,41 @@ mod tests {
let got = got.replace('\\', "/"); // handle windows paths
assert!(got.ends_with("/iroh/foo.bar"));
}

#[derive(Debug, Clone, Deserialize)]
struct Config {
item: String,
}

impl Source for Config {
fn clone_into_box(&self) -> Box<dyn Source + Send + Sync> {
Box::new(self.clone())
}

fn collect(&self) -> Result<Map<String, Value>, ConfigError> {
let mut map = Map::new();
insert_into_config_map(&mut map, "item", self.item.clone());
Ok(map)
}
}

#[test]
fn test_make_config_priority() {
// Asserting that later items have a higher priority
let cfgdir = testdir!();
let cfgfile0 = cfgdir.join("cfg0.toml");
std::fs::write(&cfgfile0, r#"item = "zero""#).unwrap();
let cfgfile1 = cfgdir.join("cfg1.toml");
std::fs::write(&cfgfile1, r#"item = "one""#).unwrap();
let cfg = make_config(
Config {
item: String::from("default"),
},
&[Some(cfgfile0.as_path()), Some(cfgfile1.as_path())],
"NO_PREFIX_PLEASE_",
HashMap::<String, String>::new(),
)
.unwrap();
assert_eq!(cfg.item, "one");
}
}
2 changes: 1 addition & 1 deletion iroh-util/tests/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ fn test_make_config() {
|| {
let got = make_config(
TestConfig::new(),
vec![
&[
Some(&PathBuf::from(CONFIG_A)),
Some(&PathBuf::from(CONFIG_B)),
None,
Expand Down
5 changes: 2 additions & 3 deletions iroh/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,12 @@ enum Commands {
impl Cli {
pub async fn run(&self) -> Result<()> {
let config_path = iroh_config_path(CONFIG_FILE_NAME)?;
// TODO(b5): allow suppliying some sort of config flag. maybe --config-cli?
let sources = vec![Some(config_path.as_path())];
let sources = [Some(config_path.as_path()), self.cfg.as_deref()];
let config = make_config(
// default
Config::new(),
// potential config files
sources,
&sources,
// env var prefix for this config
ENV_PREFIX,
// map of present command line arguments
Expand Down

0 comments on commit e7bcac6

Please sign in to comment.