diff --git a/Cargo.lock b/Cargo.lock index f64cda66..e89d11b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1131,7 +1131,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1810,7 +1810,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.5.10", + "socket2 0.6.4", "system-configuration", "tokio", "tower-service", @@ -2503,7 +2503,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -2919,6 +2919,7 @@ dependencies = [ "secrecy", "sentry", "serde", + "serde-vars", "serde_json", "stresstest", "tempfile", @@ -3545,7 +3546,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls", - "socket2 0.5.10", + "socket2 0.6.4", "thiserror", "tokio", "tracing", @@ -3583,7 +3584,7 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.5.10", + "socket2 0.6.4", "tracing", "windows-sys 0.60.2", ] @@ -4009,7 +4010,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4068,7 +4069,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4203,7 +4204,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b55fb86dfd3a2f5f76ea78310a88f96c4ea21a3031f8d212443d56123fd0521" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4404,6 +4405,15 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-vars" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c31ed6ad0418012bd1f7493bb01d9b107e81aec23042be9292d7d154909a5c45" +dependencies = [ + "serde", +] + [[package]] name = "serde_core" version = "1.0.228" @@ -4637,7 +4647,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52d1cfed4120b4d927bf7c0f86d2087a4a7d6027c906d9f9d525a80573b9be51" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -4768,7 +4778,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -5585,7 +5595,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 220b0473..07815bdd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ rdkafka = { version = "0.39.0", features = [ "tracing", ] } serde = { version = "1.0.228", features = ["derive"] } +serde-vars = "0.3.1" serde_json = "1.0.150" serde_yaml = "0.9.34-deprecated" sketches-ddsketch = "0.3.1" diff --git a/objectstore-server/Cargo.toml b/objectstore-server/Cargo.toml index 20d944ca..cd138466 100644 --- a/objectstore-server/Cargo.toml +++ b/objectstore-server/Cargo.toml @@ -42,6 +42,7 @@ rustls = { workspace = true } secrecy = { workspace = true, features = ["serde"] } sentry = { workspace = true, features = ["tower-axum-matched-path", "tracing", "logs"] } serde = { workspace = true } +serde-vars = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } thread_local = { workspace = true } diff --git a/objectstore-server/src/config.rs b/objectstore-server/src/config.rs index 56af859d..c08177ea 100644 --- a/objectstore-server/src/config.rs +++ b/objectstore-server/src/config.rs @@ -30,6 +30,39 @@ //! type: filesystem //! path: /data //! ``` +//! +//! # Variable References +//! +//! Any configuration value may be written as a reference, which is resolved after all +//! sources have been merged. `${file:PATH}` is replaced by that file's contents and +//! `${VAR_NAME}` by that environment variable: +//! +//! ```yaml +//! storage_cogs: +//! type: kafka +//! override_params: +//! sasl.password: ${file:/var/secrets/kafka-password} +//! ``` +//! +//! This allows for configuring secrets in YAML without actually revealing the secret. It +//! is also helpful for the specific Kafka case shown above: `override_params` is a dict +//! of `librdkafka` options like `sasl.password` which can't be expressed as environment +//! variables. +//! +//! A reference must be the entire value: `${A}` works, `prefix-${A}` does not. Referencing +//! a file that cannot be read, or an environment variable that is not set, is an error at +//! startup. +//! +//! ## Relationship to `OS__` environment variables +//! +//! These are different mechanisms and neither replaces the other. An `OS__` variable names +//! a *key*: `OS__SENTRY__DSN=abc` sets `sentry.dsn`, and can introduce a key the config +//! file never mentions. A `${...}` reference supplies a *value* for a key that is already +//! there. +//! +//! They can even be used together: `OS__SENTRY__DSN=${SENTRY_DSN}` will set the +//! `sentry.dsn` key in Objectstore's config to the value of `SENTRY_DSN` in the +//! environment. use std::borrow::Cow; use std::collections::{BTreeMap, HashSet}; @@ -685,21 +718,45 @@ impl Config { /// 2. YAML configuration file (if provided in `args`) /// 3. Environment variables (prefixed with `OS__`) /// + /// Any value in the merged configuration may then be written as `${file:PATH}` or + /// `${VAR_NAME}` to have it replaced by that file's contents or that environment + /// variable — see [variable references](self#variable-references). + /// /// # Errors /// /// Returns an error if: /// - The YAML configuration file cannot be read or parsed /// - Environment variables contain invalid values /// - Required fields are missing or invalid + /// - A `${file:PATH}` reference names a file that cannot be read, or a `${VAR_NAME}` + /// reference names an environment variable that is not set pub fn load(path: Option<&Path>) -> Result { let mut figment = figment::Figment::from(Serialized::defaults(Config::default())); if let Some(path) = path { figment = figment.merge(Yaml::file(path)); } - let config = figment + + // Merge first, then resolve variables against the merged value, so a reference is + // resolved wherever it came from and whichever layer won. + let merged: figment::value::Value = figment .merge(Env::prefixed(ENV_PREFIX).split("__")) .extract()?; + let base_path = path.and_then(Path::parent).unwrap_or(Path::new("")); + + // The file source must come first: `${file:x}` also matches the environment + // source's `${` prefix, which would otherwise look up a variable named `file:x`. + let mut source = ( + serde_vars::FileSource::new() + .with_variable_prefix("${file:") + .with_variable_suffix("}") + .with_base_path(base_path), + serde_vars::EnvSource::default() + .with_variable_prefix("${") + .with_variable_suffix("}"), + ); + let config = serde_vars::deserialize(&merged, &mut source)?; + Ok(config) } } @@ -956,6 +1013,172 @@ mod tests { }); } + #[test] + fn a_variable_reference_is_replaced_by_its_environment_variable() { + let mut tempfile = tempfile::NamedTempFile::new().unwrap(); + tempfile + .write_all( + br#" + storage_cogs: + type: kafka + topic: shared-resources-inventory + bootstrap_servers: [kafka:9092] + override_params: + sasl.mechanism: SCRAM-SHA-256 + sasl.password: ${KAFKA_SASL_PASSWORD} + "#, + ) + .unwrap(); + + figment::Jail::expect_with(|jail| { + jail.set_env("KAFKA_SASL_PASSWORD", "hunter2"); + + let config = Config::load(Some(tempfile.path())).unwrap(); + + let CostTrackerConfig::Kafka(sink) = + config.storage_cogs.as_ref().expect("kafka transport"); + assert_eq!(sink.override_params["sasl.password"], "hunter2"); + assert_eq!(sink.override_params["sasl.mechanism"], "SCRAM-SHA-256"); + + Ok(()) + }); + } + + #[test] + fn a_file_reference_is_replaced_by_the_file_contents() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("kafka-password"), "hunter2").unwrap(); + + let config_path = dir.path().join("config.yml"); + std::fs::write( + &config_path, + r#" + storage_cogs: + type: kafka + override_params: + sasl.password: ${file:kafka-password} + "#, + ) + .unwrap(); + + figment::Jail::expect_with(|_jail| { + let config = Config::load(Some(&config_path)).unwrap(); + + let CostTrackerConfig::Kafka(sink) = + config.storage_cogs.as_ref().expect("kafka transport"); + assert_eq!(sink.override_params["sasl.password"], "hunter2"); + + Ok(()) + }); + } + + #[test] + fn an_absolute_file_reference_ignores_the_config_directory() { + let secrets = tempfile::tempdir().unwrap(); + let secret_path = secrets.path().join("kafka-password"); + std::fs::write(&secret_path, "hunter2").unwrap(); + + let config_dir = tempfile::tempdir().unwrap(); + let config_path = config_dir.path().join("config.yml"); + std::fs::write( + &config_path, + format!( + r#" + storage_cogs: + type: kafka + override_params: + sasl.password: ${{file:{}}} + "#, + secret_path.display() + ), + ) + .unwrap(); + + figment::Jail::expect_with(|_jail| { + let config = Config::load(Some(&config_path)).unwrap(); + + let CostTrackerConfig::Kafka(sink) = + config.storage_cogs.as_ref().expect("kafka transport"); + assert_eq!(sink.override_params["sasl.password"], "hunter2"); + + Ok(()) + }); + } + + #[test] + fn a_missing_file_reference_fails_to_load() { + let dir = tempfile::tempdir().unwrap(); + let config_path = dir.path().join("config.yml"); + std::fs::write( + &config_path, + r#" + storage_cogs: + type: kafka + override_params: + sasl.password: ${file:nope} + "#, + ) + .unwrap(); + + figment::Jail::expect_with(|_jail| { + assert!(Config::load(Some(&config_path)).is_err()); + Ok(()) + }); + } + + #[test] + fn an_unset_variable_reference_fails_to_load() { + let mut tempfile = tempfile::NamedTempFile::new().unwrap(); + tempfile + .write_all( + br#" + storage_cogs: + type: kafka + override_params: + sasl.password: ${KAFKA_SASL_PASSWORD} + "#, + ) + .unwrap(); + + figment::Jail::expect_with(|_jail| { + assert!(Config::load(Some(tempfile.path())).is_err()); + Ok(()) + }); + } + + #[test] + fn a_variable_reference_resolves_in_an_env_override_too() { + figment::Jail::expect_with(|jail| { + jail.set_env("SENTRY_DSN", "https://public@example.invalid/1"); + jail.set_env("OS__SENTRY__DSN", "${SENTRY_DSN}"); + + let config = Config::load(None).unwrap(); + + assert_eq!( + config.sentry.dsn.unwrap().expose_secret().as_str(), + "https://public@example.invalid/1" + ); + + Ok(()) + }); + } + + #[test] + fn a_value_that_is_not_a_reference_is_left_alone() { + figment::Jail::expect_with(|jail| { + jail.set_env("OS__SENTRY__ENVIRONMENT", "prod-${NOT_A_VAR"); + + let config = Config::load(None).unwrap(); + + assert_eq!( + config.sentry.environment.as_deref(), + Some("prod-${NOT_A_VAR") + ); + + Ok(()) + }); + } + #[test] fn metrics_addr_via_env() { figment::Jail::expect_with(|jail| {