From f2106c27336d7d03671dbbbcd1401232c2beb61f Mon Sep 17 00:00:00 2001 From: Felix Scheinost Date: Wed, 4 Jan 2023 18:28:56 +0100 Subject: [PATCH 1/3] Implement `rpc_secret_file` --- .envrc | 1 + .gitignore | 1 + doc/book/development/devenv.md | 2 +- doc/book/reference-manual/configuration.md | 6 +++- flake.lock | 16 +++++++++ flake.nix | 42 +++++++++++++--------- src/garage/main.rs | 2 +- src/model/garage.rs | 2 +- src/util/config.rs | 26 ++++++++++++-- 9 files changed, 75 insertions(+), 23 deletions(-) create mode 100644 .envrc diff --git a/.envrc b/.envrc new file mode 100644 index 00000000..3550a30f --- /dev/null +++ b/.envrc @@ -0,0 +1 @@ +use flake diff --git a/.gitignore b/.gitignore index 95f8a160..ef7a56eb 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /pki **/*.rs.bk *.swp +/.direnv \ No newline at end of file diff --git a/doc/book/development/devenv.md b/doc/book/development/devenv.md index c2ef4e7d..8d7d2e95 100644 --- a/doc/book/development/devenv.md +++ b/doc/book/development/devenv.md @@ -39,7 +39,7 @@ Now you can enter our nix-shell, all the required packages will be downloaded bu nix-shell ``` -You can use the traditionnal Rust development workflow: +You can use the traditional Rust development workflow: ```bash cargo build # compile the project diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index 2d9c3f0c..353963ef 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -96,7 +96,7 @@ Performance characteristics of the different DB engines are as follows: - Sled: the default database engine, which tends to produce large data files and also has performance issues, especially when the metadata folder - is on a traditionnal HDD and not on SSD. + is on a traditional HDD and not on SSD. - LMDB: the recommended alternative on 64-bit systems, much more space-efficiant and slightly faster. Note that the data format of LMDB is not portable between architectures, so for instance the Garage database of an x86-64 @@ -267,6 +267,10 @@ This key should be specified here in the form of a 32-byte hex-encoded random string. Such a string can be generated with a command such as `openssl rand -hex 32`. +### `rpc_secret_file` + +Like `rpc_secret` above, just that this is the path to a file that Garage will try to read the secret from. + ### `rpc_bind_addr` The address and port on which to bind for inter-cluster communcations diff --git a/flake.lock b/flake.lock index 4b5713dc..beba6a7b 100644 --- a/flake.lock +++ b/flake.lock @@ -55,6 +55,21 @@ "type": "github" } }, + "flake-utils_2": { + "locked": { + "lastModified": 1667395993, + "narHash": "sha256-nuEHfE/LcWyuSWnS8t12N1wc105Qtau+/OdUAjtQ0rA=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "5aed5285a952e0b949eb3ba02c12fa4fcfef535f", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, "nixpkgs": { "locked": { "lastModified": 1665657542, @@ -74,6 +89,7 @@ "root": { "inputs": { "cargo2nix": "cargo2nix", + "flake-utils": "flake-utils_2", "nixpkgs": "nixpkgs" } }, diff --git a/flake.nix b/flake.nix index 7d152195..85a500ce 100644 --- a/flake.nix +++ b/flake.nix @@ -7,22 +7,30 @@ url = "github:Alexis211/cargo2nix/a7a61179b66054904ef6a195d8da736eaaa06c36"; inputs.nixpkgs.follows = "nixpkgs"; }; + inputs.flake-utils.url = "github:numtide/flake-utils"; - outputs = { self, nixpkgs, cargo2nix }: let - git_version = self.lastModifiedDate; - compile = import ./nix/compile.nix; - forAllSystems = nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed; - in - { - packages = forAllSystems (system: { - default = (compile { - inherit system git_version; - pkgsSrc = nixpkgs; - cargo2nixOverlay = cargo2nix.overlays.default; - release = true; - }).workspace.garage { - compileMode = "build"; - }; - }); - }; + outputs = { self, nixpkgs, cargo2nix, flake-utils }: + let + git_version = self.lastModifiedDate; + compile = import ./nix/compile.nix; + in flake-utils.lib.eachDefaultSystem (system: + let pkgs = nixpkgs.legacyPackages.${system}; + in { + packages = { + default = (compile { + inherit system git_version; + pkgsSrc = nixpkgs; + cargo2nixOverlay = cargo2nix.overlays.default; + release = true; + }).workspace.garage { compileMode = "build"; }; + }; + devShell = ((compile { + inherit system git_version; + pkgsSrc = nixpkgs; + cargo2nixOverlay = cargo2nix.overlays.default; + release = false; + }).workspaceShell { + packages = [ pkgs.rustfmt ]; + }); + }); } 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 { From 7b62fe3f0b1dcb336f153e54e72d8587292aa01b Mon Sep 17 00:00:00 2001 From: Felix Scheinost Date: Sat, 7 Jan 2023 13:49:03 +0100 Subject: [PATCH 2/3] Error on both `rpc_secret` and `rpc_secret_file` --- src/util/config.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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) From d6ea0cbefa6dcf89ea30e1cd2d36854d8160d6b1 Mon Sep 17 00:00:00 2001 From: Felix Scheinost Date: Sat, 7 Jan 2023 13:49:15 +0100 Subject: [PATCH 3/3] Add tests for `rpc_secret_file` --- Cargo.lock | 1 + Cargo.nix | 5 +- flake.nix | 2 +- src/util/Cargo.toml | 2 + src/util/config.rs | 120 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fb8b4f5c..5d38b92a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1276,6 +1276,7 @@ dependencies = [ "http", "hyper", "lazy_static", + "mktemp", "netapp", "opentelemetry", "rand 0.8.5", diff --git a/Cargo.nix b/Cargo.nix index 79601cdd..9ee5b9fb 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -32,7 +32,7 @@ args@{ ignoreLockHash, }: let - nixifiedLockHash = "b6aeefc112eb232904b24398f4e5da776c8ee2c13d427a26dbdf1732205d4fc9"; + nixifiedLockHash = "8461dcfb984a8d042fecb5745d5da17912135dbf2a8ef7e6c3ae8e64c03d9744"; workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc; currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock); lockHashIgnored = if ignoreLockHash @@ -1820,6 +1820,9 @@ in tracing = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".tracing."0.1.32" { inherit profileName; }).out; xxhash_rust = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".xxhash-rust."0.8.4" { inherit profileName; }).out; }; + devDependencies = { + mktemp = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".mktemp."0.4.1" { inherit profileName; }).out; + }; }); "unknown".garage_web."0.8.1" = overridableMkRustCrate (profileName: rec { diff --git a/flake.nix b/flake.nix index 85a500ce..c1d772bb 100644 --- a/flake.nix +++ b/flake.nix @@ -30,7 +30,7 @@ cargo2nixOverlay = cargo2nix.overlays.default; release = false; }).workspaceShell { - packages = [ pkgs.rustfmt ]; + packages = [ pkgs.rustfmt cargo2nix.packages.${system}.default ]; }); }); } 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(()) + } +}