From 2f9fb83316f6b592c823aedf0edac5003f4ac6d4 Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Fri, 10 Apr 2026 07:08:48 -0400 Subject: [PATCH] feat(config): add named config instances API Adds get_named/set_named/get_or_prompt_named to ConfigManager so multiple instances of the same config type can coexist (e.g., separate API credentials for primary and backup of a firewall pair). Key format: {T::KEY}/{instance_name}. EnvSource maps slashes and hyphens to underscores so the env var name remains valid (e.g., HARMONY_CONFIG_TestConfig_fw_primary). LocalFileSource creates parent directories for nested keys. The firewall_pair.rs TODO comment is updated to reference the new API; the actual migration is deferred until Config is implemented on the credential types. --- harmony/src/domain/topology/firewall_pair.rs | 11 +- harmony_config/src/lib.rs | 243 ++++++++++++++++++- harmony_config/src/source/env.rs | 5 +- harmony_config/src/source/local_file.rs | 5 +- 4 files changed, 247 insertions(+), 17 deletions(-) diff --git a/harmony/src/domain/topology/firewall_pair.rs b/harmony/src/domain/topology/firewall_pair.rs index 3954816..2bd7ace 100644 --- a/harmony/src/domain/topology/firewall_pair.rs +++ b/harmony/src/domain/topology/firewall_pair.rs @@ -60,9 +60,14 @@ impl FirewallPairTopology { /// /// Credentials are loaded via `SecretManager::get_or_prompt`. pub async fn opnsense_from_config() -> Self { - // TODO: both firewalls share the same credentials. Once named config - // instances are available (ROADMAP/11), use per-device credentials: - // ConfigManager::get_named::("fw-primary") + // TODO: both firewalls share the same credentials. Named config instances + // are now available in harmony_config (ConfigManager::get_named / + // get_or_prompt_named). To use per-device credentials here, add + // harmony_config as a dependency and impl Config for OPNSenseApiCredentials + // and OPNSenseFirewallCredentials, then replace the calls below with: + // let api_creds = ConfigManager::get_or_prompt_named::("fw-primary").await?; + // let backup_api = ConfigManager::get_or_prompt_named::("fw-backup").await?; + // See ROADMAP/11-named-config-instances.md for details. let ssh_creds = SecretManager::get_or_prompt::() .await .expect("Failed to get SSH credentials"); diff --git a/harmony_config/src/lib.rs b/harmony_config/src/lib.rs index 9ac2480..4aeb302 100644 --- a/harmony_config/src/lib.rs +++ b/harmony_config/src/lib.rs @@ -72,6 +72,11 @@ pub trait ConfigSource: Send + Sync { } } +/// Build a composite key for a named config instance: `{base_key}/{name}`. +fn named_key(base_key: &str, name: &str) -> String { + format!("{}/{}", base_key, name) +} + pub struct ConfigManager { sources: Vec>, } @@ -82,24 +87,62 @@ impl ConfigManager { } pub async fn get(&self) -> Result { + self.get_by_key(T::KEY).await + } + + /// Retrieve a named instance of a config type. + /// + /// The storage key becomes `{T::KEY}/{name}`, allowing multiple instances + /// of the same config type (e.g., separate credentials for primary and + /// backup firewalls). + pub async fn get_named(&self, name: &str) -> Result { + let key = named_key(T::KEY, name); + self.get_by_key(&key).await + } + + pub async fn get_or_prompt(&self) -> Result { + self.get_or_prompt_by_key(T::KEY).await + } + + /// Retrieve a named instance, falling back to interactive prompt if not + /// found in any source. The prompt will display the instance name for + /// clarity. + pub async fn get_or_prompt_named(&self, name: &str) -> Result { + let key = named_key(T::KEY, name); + self.get_or_prompt_by_key(&key).await + } + + pub async fn set(&self, config: &T) -> Result<(), ConfigError> { + self.set_by_key(T::KEY, config).await + } + + /// Store a named instance of a config type. + pub async fn set_named(&self, name: &str, config: &T) -> Result<(), ConfigError> { + let key = named_key(T::KEY, name); + self.set_by_key(&key, config).await + } + + // ── Internal helpers ────────────────────────────────────────────── + + async fn get_by_key(&self, key: &str) -> Result { for source in &self.sources { - if let Some(value) = source.get(T::KEY).await? { + if let Some(value) = source.get(key).await? { let config: T = serde_json::from_value(value).map_err(|e| ConfigError::Deserialization { - key: T::KEY.to_string(), + key: key.to_string(), source: e, })?; - debug!("Retrieved config for key {} from source", T::KEY); + debug!("Retrieved config for key {} from source", key); return Ok(config); } } Err(ConfigError::NotFound { - key: T::KEY.to_string(), + key: key.to_string(), }) } - pub async fn get_or_prompt(&self) -> Result { - match self.get::().await { + async fn get_or_prompt_by_key(&self, key: &str) -> Result { + match self.get_by_key::(key).await { Ok(config) => Ok(config), Err(ConfigError::NotFound { .. }) => { let config = @@ -107,7 +150,7 @@ impl ConfigManager { let value = serde_json::to_value(&config).map_err(|e| ConfigError::Serialization { - key: T::KEY.to_string(), + key: key.to_string(), source: e, })?; @@ -115,7 +158,7 @@ impl ConfigManager { if !source.should_persist() { continue; } - if source.set(T::KEY, &value).await.is_ok() { + if source.set(key, &value).await.is_ok() { break; } } @@ -126,14 +169,14 @@ impl ConfigManager { } } - pub async fn set(&self, config: &T) -> Result<(), ConfigError> { + async fn set_by_key(&self, key: &str, config: &T) -> Result<(), ConfigError> { let value = serde_json::to_value(config).map_err(|e| ConfigError::Serialization { - key: T::KEY.to_string(), + key: key.to_string(), source: e, })?; for source in &self.sources { - source.set(T::KEY, &value).await?; + source.set(key, &value).await?; } Ok(()) @@ -174,6 +217,33 @@ pub async fn set(config: &T) -> Result<(), ConfigError> { .await } +pub async fn get_named(name: &str) -> Result { + let manager = CONFIG_MANAGER.lock().await; + manager + .as_ref() + .ok_or(ConfigError::NoSources)? + .get_named::(name) + .await +} + +pub async fn get_or_prompt_named(name: &str) -> Result { + let manager = CONFIG_MANAGER.lock().await; + manager + .as_ref() + .ok_or(ConfigError::NoSources)? + .get_or_prompt_named::(name) + .await +} + +pub async fn set_named(name: &str, config: &T) -> Result<(), ConfigError> { + let manager = CONFIG_MANAGER.lock().await; + manager + .as_ref() + .ok_or(ConfigError::NoSources)? + .set_named::(name, config) + .await +} + pub fn default_config_dir() -> Option { ProjectDirs::from("io", "NationTech", "Harmony").map(|dirs| dirs.data_dir().join("config")) } @@ -817,4 +887,155 @@ mod tests { assert_eq!(result.name, "from_sqlite"); assert_eq!(result.count, 99); } + + // ── Named config instance tests ─────────────────────────────────── + + #[tokio::test] + async fn test_named_get_returns_value_for_named_key() { + let primary = TestConfig { + name: "primary".to_string(), + count: 1, + }; + let mut data = std::collections::HashMap::new(); + data.insert( + "TestConfig/primary".to_string(), + serde_json::to_value(&primary).unwrap(), + ); + + let source = Arc::new(MockSource::with_data(data)); + let manager = ConfigManager::new(vec![source]); + + let result: TestConfig = manager.get_named("primary").await.unwrap(); + assert_eq!(result, primary); + } + + #[tokio::test] + async fn test_named_and_unnamed_keys_do_not_collide() { + let unnamed = TestConfig { + name: "unnamed".to_string(), + count: 0, + }; + let named_primary = TestConfig { + name: "primary".to_string(), + count: 1, + }; + let named_backup = TestConfig { + name: "backup".to_string(), + count: 2, + }; + + let mut data = std::collections::HashMap::new(); + data.insert( + "TestConfig".to_string(), + serde_json::to_value(&unnamed).unwrap(), + ); + data.insert( + "TestConfig/primary".to_string(), + serde_json::to_value(&named_primary).unwrap(), + ); + data.insert( + "TestConfig/backup".to_string(), + serde_json::to_value(&named_backup).unwrap(), + ); + + let source = Arc::new(MockSource::with_data(data)); + let manager = ConfigManager::new(vec![source]); + + let r_unnamed: TestConfig = manager.get().await.unwrap(); + let r_primary: TestConfig = manager.get_named("primary").await.unwrap(); + let r_backup: TestConfig = manager.get_named("backup").await.unwrap(); + + assert_eq!(r_unnamed, unnamed); + assert_eq!(r_primary, named_primary); + assert_eq!(r_backup, named_backup); + } + + #[tokio::test] + async fn test_named_set_and_get_roundtrip() { + let source = Arc::new(MockSource::new()); + let manager = ConfigManager::new(vec![source.clone()]); + + let config = TestConfig { + name: "instance_a".to_string(), + count: 42, + }; + + manager.set_named("instance_a", &config).await.unwrap(); + + let result: TestConfig = manager.get_named("instance_a").await.unwrap(); + assert_eq!(result, config); + + // Unnamed get should NOT find the named value + let unnamed: Result = manager.get().await; + assert!(matches!(unnamed, Err(ConfigError::NotFound { .. }))); + } + + #[tokio::test] + async fn test_named_resolution_through_source_chain() { + use tempfile::NamedTempFile; + + let temp_file = NamedTempFile::new().unwrap(); + let sqlite = SqliteSource::open(temp_file.path().to_path_buf()) + .await + .unwrap(); + let sqlite = Arc::new(sqlite); + + // Empty first source, config in sqlite + let source1 = Arc::new(MockSource::new()); + let manager = ConfigManager::new(vec![source1.clone(), sqlite.clone()]); + + let config = TestConfig { + name: "from_sqlite_named".to_string(), + count: 77, + }; + sqlite + .set( + "TestConfig/my-instance", + &serde_json::to_value(&config).unwrap(), + ) + .await + .unwrap(); + + let result: TestConfig = manager.get_named("my-instance").await.unwrap(); + assert_eq!(result, config); + + // First source was checked but had nothing + assert_eq!(source1.get_call_count(), 1); + } + + #[tokio::test] + async fn test_named_env_var_format() { + let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + + let config = TestConfig { + name: "from_env_named".to_string(), + count: 55, + }; + + // Named key "TestConfig/fw-primary" should map to env var + // HARMONY_CONFIG_TestConfig_fw_primary + let env_key = "HARMONY_CONFIG_TestConfig_fw_primary"; + unsafe { + std::env::set_var(env_key, serde_json::to_string(&config).unwrap()); + } + + let env_source = Arc::new(EnvSource); + let manager = ConfigManager::new(vec![env_source]); + + let result: TestConfig = manager.get_named("fw-primary").await.unwrap(); + assert_eq!(result, config); + + unsafe { + std::env::remove_var(env_key); + } + } + + #[tokio::test] + async fn test_named_not_found() { + let source = Arc::new(MockSource::new()); + let manager = ConfigManager::new(vec![source]); + + let result: Result = manager.get_named("nonexistent").await; + assert!(matches!(result, Err(ConfigError::NotFound { ref key }) if key == "TestConfig/nonexistent")); + } } diff --git a/harmony_config/src/source/env.rs b/harmony_config/src/source/env.rs index 5e20159..ef1587b 100644 --- a/harmony_config/src/source/env.rs +++ b/harmony_config/src/source/env.rs @@ -4,7 +4,10 @@ use async_trait::async_trait; pub struct EnvSource; fn env_key_for(config_key: &str) -> String { - format!("HARMONY_CONFIG_{}", config_key) + // Replace `/` and `-` with `_` so named keys like "MyConfig/fw-primary" + // become valid env var names: HARMONY_CONFIG_MyConfig_fw_primary + let sanitized = config_key.replace(['/', '-'], "_"); + format!("HARMONY_CONFIG_{}", sanitized) } #[async_trait] diff --git a/harmony_config/src/source/local_file.rs b/harmony_config/src/source/local_file.rs index f803b2c..46d8940 100644 --- a/harmony_config/src/source/local_file.rs +++ b/harmony_config/src/source/local_file.rs @@ -46,9 +46,10 @@ impl ConfigSource for LocalFileSource { } async fn set(&self, key: &str, value: &serde_json::Value) -> Result<(), ConfigError> { - fs::create_dir_all(&self.base_path).await?; - let path = self.file_path_for(key); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).await?; + } let contents = serde_json::to_string_pretty(value).map_err(|e| ConfigError::Serialization { key: key.to_string(), -- 2.39.5