Compare commits

..

1 Commits

Author SHA1 Message Date
2f9fb83316 feat(config): add named config instances API
Some checks failed
Run Check Script / check (pull_request) Failing after 29s
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.
2026-04-10 07:08:48 -04:00
8 changed files with 263 additions and 249 deletions

View File

@@ -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::<OPNSenseApiCredentials>("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::<OPNSenseApiCredentials>("fw-primary").await?;
// let backup_api = ConfigManager::get_or_prompt_named::<OPNSenseApiCredentials>("fw-backup").await?;
// See ROADMAP/11-named-config-instances.md for details.
let ssh_creds = SecretManager::get_or_prompt::<OPNSenseFirewallCredentials>()
.await
.expect("Failed to get SSH credentials");

View File

@@ -161,12 +161,8 @@ impl DnsServer for HAClusterTopology {
async fn register_hosts(&self, hosts: Vec<DnsRecord>) -> Result<(), ExecutorError> {
self.dns_server.register_hosts(hosts).await
}
async fn remove_record(
&self,
name: &str,
record_type: DnsRecordType,
) -> Result<(), ExecutorError> {
self.dns_server.remove_record(name, record_type).await
fn remove_record(&self, name: &str, record_type: DnsRecordType) -> Result<(), ExecutorError> {
self.dns_server.remove_record(name, record_type)
}
async fn list_records(&self) -> Vec<DnsRecord> {
self.dns_server.list_records().await
@@ -552,11 +548,7 @@ impl DnsServer for DummyInfra {
async fn register_hosts(&self, _hosts: Vec<DnsRecord>) -> Result<(), ExecutorError> {
unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
}
async fn remove_record(
&self,
_name: &str,
_record_type: DnsRecordType,
) -> Result<(), ExecutorError> {
fn remove_record(&self, _name: &str, _record_type: DnsRecordType) -> Result<(), ExecutorError> {
unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
}
async fn list_records(&self) -> Vec<DnsRecord> {

View File

@@ -90,11 +90,7 @@ pub trait DhcpServer: Send + Sync + Debug {
pub trait DnsServer: Send + Sync {
async fn register_dhcp_leases(&self, register: bool) -> Result<(), ExecutorError>;
async fn register_hosts(&self, hosts: Vec<DnsRecord>) -> Result<(), ExecutorError>;
async fn remove_record(
&self,
name: &str,
record_type: DnsRecordType,
) -> Result<(), ExecutorError>;
fn remove_record(&self, name: &str, record_type: DnsRecordType) -> Result<(), ExecutorError>;
async fn list_records(&self) -> Vec<DnsRecord>;
fn get_ip(&self) -> IpAddress;
fn get_host(&self) -> LogicalHost;
@@ -394,7 +390,7 @@ mod test {
Ok(())
}
async fn remove_record(
fn remove_record(
&self,
_name: &str,
_record_type: DnsRecordType,

View File

@@ -1,96 +1,29 @@
use crate::infra::opnsense::LogicalHost;
use crate::{
executors::ExecutorError,
topology::{DnsRecord, DnsRecordType, DnsServer},
topology::{DnsRecord, DnsServer},
};
use async_trait::async_trait;
use harmony_types::net::IpAddress;
use log::{info, warn};
use super::OPNSenseFirewall;
#[async_trait]
impl DnsServer for OPNSenseFirewall {
async fn register_hosts(&self, hosts: Vec<DnsRecord>) -> Result<(), ExecutorError> {
let dhcp = self.opnsense_config.dhcp();
for record in &hosts {
info!(
"Registering DNS host override: {}.{} -> {}",
record.host, record.domain, record.value
);
dhcp.add_dns_host(&record.host, &record.domain, &record.value.to_string())
.await
.map_err(|e| {
ExecutorError::UnexpectedError(format!(
"Failed to register DNS host {}.{}: {e}",
record.host, record.domain
))
})?;
}
Ok(())
async fn register_hosts(&self, _hosts: Vec<DnsRecord>) -> Result<(), ExecutorError> {
todo!("Refactor this to use dnsmasq API")
}
async fn remove_record(
fn remove_record(
&self,
name: &str,
_record_type: DnsRecordType,
_name: &str,
_record_type: crate::topology::DnsRecordType,
) -> Result<(), ExecutorError> {
let (hostname, domain) = name.split_once('.').ok_or_else(|| {
ExecutorError::UnexpectedError(format!(
"DNS record name '{name}' must be a fully qualified name (host.domain)"
))
})?;
info!("Removing DNS host override: {hostname}.{domain}");
self.opnsense_config
.dhcp()
.remove_dns_host(hostname, domain)
.await
.map_err(|e| {
ExecutorError::UnexpectedError(format!(
"Failed to remove DNS host {hostname}.{domain}: {e}"
))
})
todo!()
}
async fn list_records(&self) -> Vec<DnsRecord> {
match self.opnsense_config.dhcp().list_dns_hosts().await {
Ok(entries) => entries
.into_iter()
.filter_map(|entry| {
let ip: IpAddress = match entry.ip.parse() {
Ok(ip) => ip,
Err(e) => {
warn!(
"Skipping DNS host {}.{} with unparseable IP '{}': {e}",
entry.host, entry.domain, entry.ip
);
return None;
}
};
// Dnsmasq host overrides are A records (IPv4) or AAAA (IPv6)
let record_type = if ip.is_ipv4() {
DnsRecordType::A
} else {
DnsRecordType::AAAA
};
Some(DnsRecord {
host: entry.host,
domain: entry.domain,
record_type,
value: ip,
})
})
.collect(),
Err(e) => {
warn!("Failed to list DNS records: {e}");
vec![]
}
}
async fn list_records(&self) -> Vec<crate::topology::DnsRecord> {
todo!("Refactor this to use dnsmasq API")
}
fn get_ip(&self) -> IpAddress {
@@ -101,15 +34,8 @@ impl DnsServer for OPNSenseFirewall {
self.host.clone()
}
async fn register_dhcp_leases(&self, register: bool) -> Result<(), ExecutorError> {
info!("Setting register DHCP leases as DNS: {register}");
self.opnsense_config
.dhcp()
.set_register_dhcp_leases(register)
.await
.map_err(|e| {
ExecutorError::UnexpectedError(format!("Failed to set register DHCP leases: {e}"))
})
async fn register_dhcp_leases(&self, _register: bool) -> Result<(), ExecutorError> {
todo!("Refactor this to use dnsmasq API")
}
async fn commit_config(&self) -> Result<(), ExecutorError> {

View File

@@ -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<Arc<dyn ConfigSource>>,
}
@@ -82,24 +87,62 @@ impl ConfigManager {
}
pub async fn get<T: Config>(&self) -> Result<T, ConfigError> {
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<T: Config>(&self, name: &str) -> Result<T, ConfigError> {
let key = named_key(T::KEY, name);
self.get_by_key(&key).await
}
pub async fn get_or_prompt<T: Config>(&self) -> Result<T, ConfigError> {
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<T: Config>(&self, name: &str) -> Result<T, ConfigError> {
let key = named_key(T::KEY, name);
self.get_or_prompt_by_key(&key).await
}
pub async fn set<T: Config>(&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<T: Config>(&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<T: Config>(&self, key: &str) -> Result<T, ConfigError> {
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<T: Config>(&self) -> Result<T, ConfigError> {
match self.get::<T>().await {
async fn get_or_prompt_by_key<T: Config>(&self, key: &str) -> Result<T, ConfigError> {
match self.get_by_key::<T>(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<T: Config>(&self, config: &T) -> Result<(), ConfigError> {
async fn set_by_key<T: Config>(&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<T: Config>(config: &T) -> Result<(), ConfigError> {
.await
}
pub async fn get_named<T: Config>(name: &str) -> Result<T, ConfigError> {
let manager = CONFIG_MANAGER.lock().await;
manager
.as_ref()
.ok_or(ConfigError::NoSources)?
.get_named::<T>(name)
.await
}
pub async fn get_or_prompt_named<T: Config>(name: &str) -> Result<T, ConfigError> {
let manager = CONFIG_MANAGER.lock().await;
manager
.as_ref()
.ok_or(ConfigError::NoSources)?
.get_or_prompt_named::<T>(name)
.await
}
pub async fn set_named<T: Config>(name: &str, config: &T) -> Result<(), ConfigError> {
let manager = CONFIG_MANAGER.lock().await;
manager
.as_ref()
.ok_or(ConfigError::NoSources)?
.set_named::<T>(name, config)
.await
}
pub fn default_config_dir() -> Option<PathBuf> {
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<TestConfig, ConfigError> = 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<TestConfig, ConfigError> = manager.get_named("nonexistent").await;
assert!(matches!(result, Err(ConfigError::NotFound { ref key }) if key == "TestConfig/nonexistent"));
}
}

View File

@@ -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]

View File

@@ -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(),

View File

@@ -77,14 +77,6 @@ fn extract_selected_key(value: &serde_json::Value) -> Option<String> {
}
}
/// A DNS host override entry returned by [`DhcpConfigDnsMasq::list_dns_hosts`].
pub struct DnsHostEntry {
pub uuid: String,
pub host: String,
pub domain: String,
pub ip: String,
}
impl DhcpConfigDnsMasq {
pub fn new(client: OpnsenseClient, shell: Arc<dyn OPNsenseShell>) -> Self {
Self { client, shell }
@@ -451,128 +443,6 @@ dhcp-boot=tag:bios,tag:!ipxe,{bios_filename}{tftp_str}
Ok(result)
}
// ── DNS host override methods ────────────────────────────────────────
/// Lists all DNS host override entries (hostname, domain, IP).
///
/// Entries with missing hostname or IP are silently skipped.
pub async fn list_dns_hosts(&self) -> Result<Vec<DnsHostEntry>, Error> {
let settings = self.get_settings().await?;
let mut result = Vec::new();
for (uuid, entry) in &settings.dnsmasq.hosts {
let Some(host) = entry.host.as_deref().filter(|s| !s.is_empty()) else {
continue;
};
let Some(ip) = entry.ip.as_deref().filter(|s| !s.is_empty()) else {
continue;
};
let domain = entry.domain.as_deref().unwrap_or("").to_string();
result.push(DnsHostEntry {
uuid: uuid.clone(),
host: host.to_string(),
domain,
ip: ip.to_string(),
});
}
Ok(result)
}
/// Adds a DNS host override entry for the given hostname, domain, and IP.
///
/// If an entry with the same hostname and domain already exists, its IP is
/// updated instead of creating a duplicate.
pub async fn add_dns_host(&self, hostname: &str, domain: &str, ip: &str) -> Result<(), Error> {
let settings = self.get_settings().await?;
// Check for existing entry with same hostname + domain
let existing = settings.dnsmasq.hosts.iter().find(|(_, h)| {
h.host.as_deref() == Some(hostname) && h.domain.as_deref() == Some(domain)
});
let host = DnsmasqHost {
host: Some(hostname.to_string()),
domain: Some(domain.to_string()),
ip: Some(vec![ip.to_string()]),
local: Some(true),
..Default::default()
};
if let Some((uuid, _)) = existing {
info!("Updating DNS host override {uuid}: {hostname}.{domain} -> {ip}");
self.api().set_host(uuid, &host).await.map_err(Error::Api)?;
} else {
info!("Creating DNS host override: {hostname}.{domain} -> {ip}");
self.api().add_host(&host).await.map_err(Error::Api)?;
}
self.client
.reconfigure("dnsmasq")
.await
.map_err(Error::Api)?;
Ok(())
}
/// Removes a DNS host override by hostname and domain.
///
/// Returns `Ok(())` even if no matching entry exists (idempotent).
pub async fn remove_dns_host(&self, hostname: &str, domain: &str) -> Result<(), Error> {
let settings = self.get_settings().await?;
let matching: Vec<String> = settings
.dnsmasq
.hosts
.iter()
.filter(|(_, h)| {
h.host.as_deref() == Some(hostname) && h.domain.as_deref() == Some(domain)
})
.map(|(uuid, _)| uuid.clone())
.collect();
for uuid in &matching {
info!("Deleting DNS host override {uuid}: {hostname}.{domain}");
self.api().del_host(uuid).await.map_err(Error::Api)?;
}
if !matching.is_empty() {
self.client
.reconfigure("dnsmasq")
.await
.map_err(Error::Api)?;
}
Ok(())
}
/// Enable or disable registering DHCP leases as DNS entries.
///
/// Sets both `regdhcp` (dynamic leases) and `regdhcpstatic` (static mappings).
pub async fn set_register_dhcp_leases(&self, register: bool) -> Result<(), Error> {
let settings = opnsense_api::generated::dnsmasq::Dnsmasq {
regdhcp: Some(register),
regdhcpstatic: Some(register),
..Default::default()
};
// The OPNsense API expects the top-level settings wrapped in {"dnsmasq": {...}}
let envelope = serde_json::json!({ "dnsmasq": settings });
let _: serde_json::Value = self
.client
.post_typed("dnsmasq", "settings", "set", Some(&envelope))
.await
.map_err(Error::Api)?;
info!("Set register DHCP leases as DNS: regdhcp={register}, regdhcpstatic={register}");
self.client
.reconfigure("dnsmasq")
.await
.map_err(Error::Api)?;
Ok(())
}
fn is_valid_mac(mac: &str) -> bool {
let parts: Vec<&str> = mac.split(':').collect();
if parts.len() != 6 {