feat(config): resolve ${file:...} and ${VAR} references in config

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"

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 }

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<Self> {
         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| {