From f2106c27336d7d03671dbbbcd1401232c2beb61f Mon Sep 17 00:00:00 2001 From: Felix Scheinost Date: Wed, 4 Jan 2023 18:28:56 +0100 Subject: Implement `rpc_secret_file` --- src/garage/main.rs | 2 +- src/model/garage.rs | 2 +- src/util/config.rs | 26 ++++++++++++++++++++++++-- 3 files changed, 26 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/garage/main.rs b/src/garage/main.rs index 107b1389..736e11ec 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -173,7 +173,7 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { let net_key_hex_str = opt .rpc_secret .as_ref() - .or_else(|| config.as_ref().map(|c| &c.rpc_secret)) + .or_else(|| config.as_ref().and_then(|c| c.rpc_secret.as_ref())) .ok_or("No RPC secret provided")?; let network_key = NetworkKey::from_slice( &hex::decode(net_key_hex_str).err_context("Invalid RPC secret key (bad hex)")?[..], diff --git a/src/model/garage.rs b/src/model/garage.rs index ac1846ce..ffa54dc5 100644 --- a/src/model/garage.rs +++ b/src/model/garage.rs @@ -159,7 +159,7 @@ impl Garage { }; let network_key = NetworkKey::from_slice( - &hex::decode(&config.rpc_secret).expect("Invalid RPC secret key")[..], + &hex::decode(&config.rpc_secret.as_ref().unwrap()).expect("Invalid RPC secret key")[..], ) .expect("Invalid RPC secret key"); diff --git a/src/util/config.rs b/src/util/config.rs index 04f8375a..e1120822 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -34,7 +34,10 @@ pub struct Config { pub compression_level: Option, /// RPC secret key: 32 bytes hex encoded - pub rpc_secret: String, + pub rpc_secret: Option, + + /// Optional file where RPC secret key is read from + pub rpc_secret_file: Option, /// Address to bind for RPC pub rpc_bind_addr: SocketAddr, @@ -177,7 +180,26 @@ pub fn read_config(config_file: PathBuf) -> Result { let mut config = String::new(); file.read_to_string(&mut config)?; - Ok(toml::from_str(&config)?) + let mut parsed_config: Config = toml::from_str(&config)?; + + match (&parsed_config.rpc_secret, &parsed_config.rpc_secret_file) { + (Some(_), _) => {} + (None, Some(rpc_secret_file_path_string)) => { + let mut rpc_secret_file = std::fs::OpenOptions::new() + .read(true) + .open(rpc_secret_file_path_string)?; + let mut rpc_secret_from_file = String::new(); + rpc_secret_file.read_to_string(&mut rpc_secret_from_file)?; + // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. + // also editors sometimes add a trailing newline + parsed_config.rpc_secret = Some(String::from(rpc_secret_from_file.trim_end())); + } + (None, None) => { + return Err("either `rpc_secret` or `rpc_secret_file` needs to be set".into()) + } + }; + + Ok(parsed_config) } fn default_compression() -> Option { -- cgit v1.2.3 From 7b62fe3f0b1dcb336f153e54e72d8587292aa01b Mon Sep 17 00:00:00 2001 From: Felix Scheinost Date: Sat, 7 Jan 2023 13:49:03 +0100 Subject: Error on both `rpc_secret` and `rpc_secret_file` --- src/util/config.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/util/config.rs b/src/util/config.rs index e1120822..5471fc41 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -34,6 +34,7 @@ pub struct Config { pub compression_level: Option, /// RPC secret key: 32 bytes hex encoded + /// Note: When using `read_config` this should never be `None` pub rpc_secret: Option, /// Optional file where RPC secret key is read from @@ -183,7 +184,12 @@ pub fn read_config(config_file: PathBuf) -> Result { let mut parsed_config: Config = toml::from_str(&config)?; match (&parsed_config.rpc_secret, &parsed_config.rpc_secret_file) { - (Some(_), _) => {} + (Some(_), None) => { + // no-op + } + (Some(_), Some(_)) => { + return Err("only one of `rpc_secret` and `rpc_secret_file` can be set".into()) + } (None, Some(rpc_secret_file_path_string)) => { let mut rpc_secret_file = std::fs::OpenOptions::new() .read(true) -- cgit v1.2.3 From d6ea0cbefa6dcf89ea30e1cd2d36854d8160d6b1 Mon Sep 17 00:00:00 2001 From: Felix Scheinost Date: Sat, 7 Jan 2023 13:49:15 +0100 Subject: Add tests for `rpc_secret_file` --- src/util/Cargo.toml | 2 + src/util/config.rs | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) (limited to 'src') diff --git a/src/util/Cargo.toml b/src/util/Cargo.toml index 32e9c851..1017b1ce 100644 --- a/src/util/Cargo.toml +++ b/src/util/Cargo.toml @@ -47,6 +47,8 @@ hyper = "0.14" opentelemetry = { version = "0.17", features = [ "rt-tokio", "metrics", "trace" ] } +[dev-dependencies] +mktemp = "0.4" [features] k2v = [] diff --git a/src/util/config.rs b/src/util/config.rs index 5471fc41..f0a881aa 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -261,3 +261,123 @@ where deserializer.deserialize_any(OptionVisitor) } + +#[cfg(test)] +mod tests { + use crate::error::Error; + use std::fs::File; + use std::io::Write; + + #[test] + fn test_rpc_secret_is_required() -> Result<(), Error> { + let path1 = mktemp::Temp::new_file()?; + let mut file1 = File::create(path1.as_path())?; + writeln!( + file1, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "# + )?; + assert_eq!( + "either `rpc_secret` or `rpc_secret_file` needs to be set", + super::read_config(path1.to_path_buf()) + .unwrap_err() + .to_string() + ); + drop(path1); + drop(file1); + + let path2 = mktemp::Temp::new_file()?; + let mut file2 = File::create(path2.as_path())?; + writeln!( + file2, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret = "foo" + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "# + )?; + + let config = super::read_config(path2.to_path_buf())?; + assert_eq!("foo", config.rpc_secret.unwrap()); + drop(path2); + drop(file2); + + Ok(()) + } + + #[test] + fn test_rpc_secret_file_works() -> Result<(), Error> { + let path_secret = mktemp::Temp::new_file()?; + let mut file_secret = File::create(path_secret.as_path())?; + writeln!(file_secret, "foo")?; + drop(file_secret); + + let path_config = mktemp::Temp::new_file()?; + let mut file_config = File::create(path_config.as_path())?; + let path_secret_path = path_secret.as_path().display(); + writeln!( + file_config, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret_file = "{path_secret_path}" + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "# + )?; + let config = super::read_config(path_config.to_path_buf())?; + assert_eq!("foo", config.rpc_secret.unwrap()); + drop(path_config); + drop(path_secret); + drop(file_config); + Ok(()) + } + + #[test] + fn test_rcp_secret_and_rpc_secret_file_cannot_be_set_both() -> Result<(), Error> { + let path_config = mktemp::Temp::new_file()?; + let mut file_config = File::create(path_config.as_path())?; + writeln!( + file_config, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret= "dummy" + rpc_secret_file = "dummy" + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "# + )?; + assert_eq!( + "only one of `rpc_secret` and `rpc_secret_file` can be set", + super::read_config(path_config.to_path_buf()) + .unwrap_err() + .to_string() + ); + drop(path_config); + drop(file_config); + Ok(()) + } +} -- cgit v1.2.3 From aac348fe937442bcf750316d6ebd47936d86ed90 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Mon, 9 Jan 2023 17:13:41 +0000 Subject: Added system_metrics.rs file. --- src/rpc/lib.rs | 2 ++ src/rpc/system_metrics.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/rpc/system_metrics.rs (limited to 'src') diff --git a/src/rpc/lib.rs b/src/rpc/lib.rs index 86f63568..a8cc0030 100644 --- a/src/rpc/lib.rs +++ b/src/rpc/lib.rs @@ -17,3 +17,5 @@ mod metrics; pub mod rpc_helper; pub use rpc_helper::*; + +pub mod system_metrics; diff --git a/src/rpc/system_metrics.rs b/src/rpc/system_metrics.rs new file mode 100644 index 00000000..d96b67e4 --- /dev/null +++ b/src/rpc/system_metrics.rs @@ -0,0 +1,33 @@ +use opentelemetry::{global, metrics::*, KeyValue}; + +/// TableMetrics reference all counter used for metrics +pub struct SystemMetrics { + pub(crate) _garage_build_info: ValueObserver, + pub(crate) _replication_factor: ValueObserver, +} + +impl SystemMetrics { + pub fn new(replication_factor: usize) -> Self { + let meter = global::meter("garage_system"); + Self { + _garage_build_info: meter + .u64_value_observer("garage_build_info", move |observer| { + observer.observe( + 1, + &[KeyValue::new( + "version", + garage_util::version::garage_version(), + )], + ) + }) + .with_description("Garage build info") + .init(), + _replication_factor: meter + .u64_value_observer("garage_replication_factor", move |observer| { + observer.observe(replication_factor as u64, &[]) + }) + .with_description("Garage replication factor setting") + .init(), + } + } +} -- cgit v1.2.3 From df1d9a98737e8a38656de8ac71bb23e8d3ad2ce9 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Mon, 9 Jan 2023 17:15:55 +0000 Subject: system.rs: Integrated SystemMetrics into System implementation. --- src/rpc/system.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src') diff --git a/src/rpc/system.rs b/src/rpc/system.rs index ed10aa4b..90f6a4c2 100644 --- a/src/rpc/system.rs +++ b/src/rpc/system.rs @@ -38,6 +38,9 @@ use crate::replication_mode::*; use crate::ring::*; use crate::rpc_helper::*; +#[cfg(feature = "metrics")] +use crate::system_metrics::*; + const DISCOVERY_INTERVAL: Duration = Duration::from_secs(60); const STATUS_EXCHANGE_INTERVAL: Duration = Duration::from_secs(10); @@ -103,6 +106,8 @@ pub struct System { consul_discovery: Option, #[cfg(feature = "kubernetes-discovery")] kubernetes_discovery: Option, + #[cfg(feature = "metrics")] + metrics: SystemMetrics, replication_mode: ReplicationMode, replication_factor: usize, @@ -275,6 +280,9 @@ impl System { cluster_layout_staging_hash: cluster_layout.staging_hash, }; + #[cfg(feature = "metrics")] + let metrics = SystemMetrics::new(replication_factor); + let ring = Ring::new(cluster_layout, replication_factor); let (update_ring, ring) = watch::channel(Arc::new(ring)); @@ -365,6 +373,8 @@ impl System { consul_discovery, #[cfg(feature = "kubernetes-discovery")] kubernetes_discovery: config.kubernetes_discovery.clone(), + #[cfg(feature = "metrics")] + metrics, ring, update_ring: Mutex::new(update_ring), -- cgit v1.2.3 From 4cfb469d2bcce5bd41fe0af0022b34716ac7279a Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Mon, 9 Jan 2023 18:49:33 +0000 Subject: block/metrics.rs: Added compression_level metric. --- src/block/manager.rs | 8 ++++++-- src/block/metrics.rs | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/block/manager.rs b/src/block/manager.rs index 1655be06..051a9f93 100644 --- a/src/block/manager.rs +++ b/src/block/manager.rs @@ -129,8 +129,12 @@ impl BlockManager { .netapp .endpoint("garage_block/manager.rs/Rpc".to_string()); - let metrics = - BlockManagerMetrics::new(rc.rc.clone(), resync.queue.clone(), resync.errors.clone()); + let metrics = BlockManagerMetrics::new( + compression_level, + rc.rc.clone(), + resync.queue.clone(), + resync.errors.clone(), + ); let scrub_persister = PersisterShared::new(&system.metadata_dir, "scrub_info"); diff --git a/src/block/metrics.rs b/src/block/metrics.rs index fbef95af..500022fc 100644 --- a/src/block/metrics.rs +++ b/src/block/metrics.rs @@ -5,6 +5,7 @@ use garage_db::counted_tree_hack::CountedTree; /// TableMetrics reference all counter used for metrics pub struct BlockManagerMetrics { + pub(crate) _compression_level: ValueObserver, pub(crate) _rc_size: ValueObserver, pub(crate) _resync_queue_len: ValueObserver, pub(crate) _resync_errored_blocks: ValueObserver, @@ -25,9 +26,23 @@ pub struct BlockManagerMetrics { } impl BlockManagerMetrics { - pub fn new(rc_tree: db::Tree, resync_queue: CountedTree, resync_errors: CountedTree) -> Self { + pub fn new( + compression_level: Option, + rc_tree: db::Tree, + resync_queue: CountedTree, + resync_errors: CountedTree, + ) -> Self { let meter = global::meter("garage_model/block"); Self { + _compression_level: meter + .u64_value_observer("block.compression_level", move |observer| { + match compression_level { + Some(v) => observer.observe(v as u64, &[]), + None => observer.observe(0 as u64, &[]), + } + }) + .with_description("Garage compression level for node") + .init(), _rc_size: meter .u64_value_observer("block.rc_size", move |observer| { if let Ok(Some(v)) = rc_tree.fast_len() { -- cgit v1.2.3