Skip to content

Commit

Permalink
refactor: Split configs in server and service (#671)
Browse files Browse the repository at this point in the history
We used to use the same config for the service and server (aka the
binary).  This is confusing when creating configs to use with
e.g. iroh-one, iroh-embed or iroh-share because some fields are not
used.

This splits off the config structs to avoid this problem, services now
have a Config and binaries a ServerConfig.  This allows creating the
services standalone without all the baggage a server needs.

While this isn't many fields yet, this will get worse as we add more
features (this is split off from another PR where this seemed useful).
  • Loading branch information
flub authored Jan 4, 2023
1 parent 1d30916 commit a0db860
Show file tree
Hide file tree
Showing 16 changed files with 243 additions and 169 deletions.
10 changes: 2 additions & 8 deletions iroh-api/benches/add.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion};
use futures::TryStreamExt;
use iroh_metrics::config::Config as MetricsConfig;
use iroh_resolver::resolver::Resolver;
use iroh_rpc_client::{Client, Config as RpcClientConfig};
use iroh_rpc_types::Addr;
Expand Down Expand Up @@ -37,15 +36,10 @@ fn add_benchmark(c: &mut Criterion) {
let server_addr = Addr::new_mem();
let client_addr = server_addr.clone();
let rpc_client = RpcClientConfig {
store_addr: Some(client_addr),
store_addr: Some(client_addr.clone()),
..Default::default()
};

let config = StoreConfig {
path: dir.path().join("db"),
rpc_client: rpc_client.clone(),
metrics: MetricsConfig::default(),
};
let config = StoreConfig::with_rpc_addr(dir.path().join("db"), client_addr);
let (_task, client, resolver) = executor.block_on(async {
let store = Store::create(config).await.unwrap();
let task = executor.spawn(async move {
Expand Down
12 changes: 1 addition & 11 deletions iroh-embed/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::path::PathBuf;

use anyhow::Result;
use iroh_one::mem_store;
use iroh_rpc_client::Config as RpcClientConfig;
use iroh_rpc_types::store::StoreAddr;
use iroh_rpc_types::Addr;
use iroh_store::Config as StoreConfig;
Expand All @@ -26,16 +25,7 @@ impl RocksStoreService {
/// This implicitly starts a task on the tokio runtime to manage the storage node.
pub async fn new(path: PathBuf) -> Result<Self> {
let addr = Addr::new_mem();
let config = StoreConfig {
path,
rpc_client: RpcClientConfig {
gateway_addr: None,
p2p_addr: None,
store_addr: Some(addr.clone()),
channels: Some(1),
},
metrics: Default::default(),
};
let config = StoreConfig::with_rpc_addr(path, addr.clone());
let task = mem_store::start(addr.clone(), config).await?;
Ok(Self { task, addr })
}
Expand Down
1 change: 1 addition & 0 deletions iroh-gateway/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ tracing-subscriber = { workspace = true, features = ["env-filter"] }
tracing.workspace = true
url.workspace = true
urlencoding.workspace = true
testdir.workspace = true

[dev-dependencies]
iroh-store.workspace = true
Expand Down
105 changes: 83 additions & 22 deletions iroh-gateway/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,43 @@ pub const CONFIG_FILE_NAME: &str = "gateway.config.toml";
pub const ENV_PREFIX: &str = "IROH_GATEWAY";
pub const DEFAULT_PORT: u16 = 9050;

/// The configuration for the gateway server.
///
/// This is the configuration which the gateway server binary needs to run. This is a
/// superset from the configuration needed by the gateway service, which can also run
/// integrated into another binary like iroh-one, iroh-share or iroh-embed.
// NOTE: If this was moved into main.rs it would not need to be public. Our binary is build
// as being outside of the lib crate, hence here it needs to be public.
#[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize)]
pub struct ServerConfig {
/// Configuration of the gateway service.
pub gateway: Config,
/// Metrics configuration.
pub metrics: MetricsConfig,
}

impl ServerConfig {
pub fn new(port: u16, rpc_client: RpcClientConfig) -> Self {
Self {
gateway: Config::new(port, rpc_client),
metrics: MetricsConfig::default(),
}
}
}

impl Source for ServerConfig {
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<String, Value> = Map::new();
insert_into_config_map(&mut map, "gateway", self.gateway.collect()?);
insert_into_config_map(&mut map, "metrics", self.metrics.collect()?);
Ok(map)
}
}

/// Configuration for [`iroh-gateway`].
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
pub struct Config {
Expand All @@ -43,8 +80,6 @@ pub struct Config {
pub indexer_endpoint: Option<String>,
/// rpc addresses for the gateway & addresses for the rpc client to dial
pub rpc_client: RpcClientConfig,
/// metrics configuration
pub metrics: MetricsConfig,
// NOTE: for toml to serialize properly, the "table" values must be serialized at the end, and
// so much come at the end of the `Config` struct
/// set of user provided headers to attach to all responses
Expand All @@ -65,7 +100,6 @@ impl Config {
http_resolvers: None,
dns_resolver: DnsResolverConfig::default(),
indexer_endpoint: None,
metrics: MetricsConfig::default(),
use_denylist: false,
redirect_to_subdomain: false,
}
Expand All @@ -80,6 +114,12 @@ impl Config {
}
}

impl From<ServerConfig> for Config {
fn from(source: ServerConfig) -> Self {
source.gateway
}
}

fn default_headers() -> HeaderMap {
let mut headers = HeaderMap::new();
headers.typed_insert(AccessControlAllowOrigin::ANY);
Expand Down Expand Up @@ -136,7 +176,6 @@ impl Default for Config {
http_resolvers: None,
dns_resolver: DnsResolverConfig::default(),
indexer_endpoint: None,
metrics: MetricsConfig::default(),
use_denylist: false,
redirect_to_subdomain: false,
};
Expand All @@ -160,8 +199,6 @@ impl Source for Config {
insert_into_config_map(&mut map, "port", self.port as i32);
insert_into_config_map(&mut map, "headers", collect_headers(&self.headers)?);
insert_into_config_map(&mut map, "rpc_client", rpc_client);
let metrics = self.metrics.collect()?;
insert_into_config_map(&mut map, "metrics", metrics);

if let Some(http_resolvers) = &self.http_resolvers {
insert_into_config_map(&mut map, "http_resolvers", http_resolvers.clone());
Expand Down Expand Up @@ -209,6 +246,10 @@ fn collect_headers(headers: &HeaderMap) -> Result<Map<String, Value>, ConfigErro

#[cfg(test)]
mod tests {
use std::collections::HashMap;

use testdir::testdir;

use super::*;
use config::Config as ConfigBuilder;

Expand All @@ -228,24 +269,11 @@ mod tests {

#[test]
fn test_collect() {
let default = Config::default();
let default = ServerConfig::default();
let mut expect: Map<String, Value> = Map::new();
expect.insert(
"public_url_base".to_string(),
Value::new(None, default.public_url_base.clone()),
);
expect.insert("port".to_string(), Value::new(None, default.port as i64));
expect.insert(
"use_denylist".to_string(),
Value::new(None, default.use_denylist),
);
expect.insert(
"headers".to_string(),
Value::new(None, collect_headers(&default.headers).unwrap()),
);
expect.insert(
"rpc_client".to_string(),
Value::new(None, default.rpc_client.collect().unwrap()),
"gateway".to_string(),
Value::new(None, default.gateway.collect().unwrap()),
);
expect.insert(
"metrics".to_string(),
Expand Down Expand Up @@ -301,4 +329,37 @@ mod tests {

assert_eq!(expect, got);
}

#[test]
fn test_toml_file() {
let dir = testdir!();
let cfg_file = dir.join("config.toml");
std::fs::write(
&cfg_file,
r#"
[gateway]
public_url_base = "http://example.com/base/"
port = 2001
[gateway.rpc_client]
gateway_addr = "irpc://127.0.0.42:1234"
"#,
)
.unwrap();
let sources = [Some(cfg_file.as_path())];
let cfg = iroh_util::make_config(
ServerConfig::default(),
&sources,
ENV_PREFIX,
HashMap::<&str, String>::new(),
)
.unwrap();
dbg!(&cfg);

assert_eq!(cfg.gateway.public_url_base, "http://example.com/base/");
assert_eq!(cfg.gateway.port, 2001);
assert_eq!(
cfg.gateway.rpc_client.gateway_addr.unwrap(),
"irpc://127.0.0.42:1234".parse().unwrap(),
);
}
}
6 changes: 1 addition & 5 deletions iroh-gateway/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,7 @@ mod tests {
let server_addr = Addr::new_mem();
let client_addr = server_addr.clone();
let store_dir = tempfile::tempdir().unwrap();
let config = iroh_store::Config {
path: store_dir.path().join("db"),
rpc_client: RpcClientConfig::default(),
metrics: iroh_metrics::config::Config::default(),
};
let config = iroh_store::Config::new(store_dir.path().join("db"));
let store = iroh_store::Store::create(config).await.unwrap();
let task =
tokio::spawn(async move { iroh_store::rpc::new(server_addr, store).await.unwrap() });
Expand Down
9 changes: 5 additions & 4 deletions iroh-gateway/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use clap::Parser;
use iroh_gateway::{
bad_bits::{self, BadBits},
cli::Args,
config::{Config, CONFIG_FILE_NAME, ENV_PREFIX},
config::{Config, ServerConfig, CONFIG_FILE_NAME, ENV_PREFIX},
core::Core,
metrics,
};
Expand All @@ -26,7 +26,7 @@ async fn main() -> Result<()> {
let sources = [Some(cfg_path.as_path()), args.cfg.as_deref()];
let mut config = make_config(
// default
Config::default(),
ServerConfig::default(),
// potential config files
&sources,
// env var prefix for this config
Expand All @@ -39,11 +39,12 @@ async fn main() -> Result<()> {
println!("{config:#?}");

let metrics_config = config.metrics.clone();
let dns_resolver_config = config.dns_resolver.clone();
let bad_bits = match config.use_denylist {
let dns_resolver_config = config.gateway.dns_resolver.clone();
let bad_bits = match config.gateway.use_denylist {
true => Arc::new(Some(RwLock::new(BadBits::new()))),
false => Arc::new(None),
};
let config = Config::from(config);
let rpc_addr = config
.rpc_addr()
.ok_or_else(|| anyhow!("missing gateway rpc addr"))?;
Expand Down
21 changes: 5 additions & 16 deletions iroh-one/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ impl Config {
self.gateway.rpc_client = self.rpc_client.clone();
self.p2p.rpc_client = self.rpc_client.clone();
self.store.rpc_client = self.rpc_client.clone();
self.gateway.metrics = self.metrics.clone();
self.p2p.metrics = self.metrics.clone();
self.store.metrics = self.metrics.clone();
}
}

Expand All @@ -93,15 +90,14 @@ impl Default for Config {
.join("ipfsd.http");
let rpc_client = Self::default_rpc_config();
let metrics_config = MetricsConfig::default();
let store_config =
default_store_config(None, rpc_client.clone(), metrics_config.clone()).unwrap();
let store_config = default_store_config(None, rpc_client.clone()).unwrap();
let key_store_path = iroh_util::iroh_data_root().unwrap();
Self {
rpc_client: rpc_client.clone(),
metrics: metrics_config.clone(),
metrics: metrics_config,
gateway: iroh_gateway::config::Config::default(),
store: store_config,
p2p: default_p2p_config(rpc_client, metrics_config, key_store_path),
p2p: default_p2p_config(rpc_client, key_store_path),
#[cfg(all(feature = "http-uds-gateway", unix))]
gateway_uds_path: Some(gateway_uds_path),
}
Expand All @@ -111,26 +107,19 @@ impl Default for Config {
fn default_store_config(
store_path: Option<PathBuf>,
ipfsd: RpcClientConfig,
metrics: iroh_metrics::config::Config,
) -> Result<iroh_store::config::Config> {
let path = config_data_path(store_path)?;
Ok(iroh_store::config::Config {
path,
rpc_client: ipfsd,
metrics,
})
}

fn default_p2p_config(
ipfsd: RpcClientConfig,
metrics: iroh_metrics::config::Config,
key_store_path: PathBuf,
) -> iroh_p2p::config::Config {
fn default_p2p_config(ipfsd: RpcClientConfig, key_store_path: PathBuf) -> iroh_p2p::config::Config {
iroh_p2p::config::Config {
key_store_path,
libp2p: Libp2pConfig::default(),
rpc_client: ipfsd,
metrics,
key_store_path,
}
}

Expand Down
Loading

0 comments on commit a0db860

Please sign in to comment.