Compare commits

..

2 Commits

Author SHA1 Message Date
904d316605 feat(dns): make DnsServer::remove_record async and implement for OPNsense
The trait method was sync, which prevented the OPNsense adapter from
calling the async dnsmasq API. Make the method async across the trait
and all implementations, and wire the OPNsense impl to
dhcp().remove_dns_host().
2026-04-10 07:11:26 -04:00
3b59cb605d feat(opnsense): implement DnsServer trait via dnsmasq API
Replaces the four todo!() stubs in infra/opnsense/dns.rs with
implementations backed by the typed dnsmasq API in opnsense-config.

- register_hosts: iterates DnsRecord list, creates host overrides
- list_records: returns A/AAAA records from dnsmasq host overrides
- register_dhcp_leases: toggles regdhcp/regdhcpstatic flags
- remove_record: returns an error (trait method is sync, dnsmasq API
  is async — follow-up needed to make the trait method async)

Adds DnsHostEntry, list_dns_hosts, add_dns_host (idempotent —
updates IP if hostname+domain exists), remove_dns_host, and
set_register_dhcp_leases to opnsense-config dnsmasq module.
2026-04-10 07:08:53 -04:00
8 changed files with 249 additions and 263 deletions

View File

@@ -60,14 +60,9 @@ impl FirewallPairTopology {
///
/// Credentials are loaded via `SecretManager::get_or_prompt`.
pub async fn opnsense_from_config() -> Self {
// 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.
// 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")
let ssh_creds = SecretManager::get_or_prompt::<OPNSenseFirewallCredentials>()
.await
.expect("Failed to get SSH credentials");

View File

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

View File

@@ -1,29 +1,96 @@
use crate::infra::opnsense::LogicalHost;
use crate::{
executors::ExecutorError,
topology::{DnsRecord, DnsServer},
topology::{DnsRecord, DnsRecordType, 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> {
todo!("Refactor this to use dnsmasq API")
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(())
}
fn remove_record(
async fn remove_record(
&self,
_name: &str,
_record_type: crate::topology::DnsRecordType,
name: &str,
_record_type: DnsRecordType,
) -> Result<(), ExecutorError> {
todo!()
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}"
))
})
}
async fn list_records(&self) -> Vec<crate::topology::DnsRecord> {
todo!("Refactor this to use dnsmasq API")
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![]
}
}
}
fn get_ip(&self) -> IpAddress {
@@ -34,8 +101,15 @@ impl DnsServer for OPNSenseFirewall {
self.host.clone()
}
async fn register_dhcp_leases(&self, _register: bool) -> Result<(), ExecutorError> {
todo!("Refactor this to use dnsmasq API")
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 commit_config(&self) -> Result<(), ExecutorError> {

View File

@@ -72,11 +72,6 @@ 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>>,
}
@@ -87,62 +82,24 @@ 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(key).await? {
if let Some(value) = source.get(T::KEY).await? {
let config: T =
serde_json::from_value(value).map_err(|e| ConfigError::Deserialization {
key: key.to_string(),
key: T::KEY.to_string(),
source: e,
})?;
debug!("Retrieved config for key {} from source", key);
debug!("Retrieved config for key {} from source", T::KEY);
return Ok(config);
}
}
Err(ConfigError::NotFound {
key: key.to_string(),
key: T::KEY.to_string(),
})
}
async fn get_or_prompt_by_key<T: Config>(&self, key: &str) -> Result<T, ConfigError> {
match self.get_by_key::<T>(key).await {
pub async fn get_or_prompt<T: Config>(&self) -> Result<T, ConfigError> {
match self.get::<T>().await {
Ok(config) => Ok(config),
Err(ConfigError::NotFound { .. }) => {
let config =
@@ -150,7 +107,7 @@ impl ConfigManager {
let value =
serde_json::to_value(&config).map_err(|e| ConfigError::Serialization {
key: key.to_string(),
key: T::KEY.to_string(),
source: e,
})?;
@@ -158,7 +115,7 @@ impl ConfigManager {
if !source.should_persist() {
continue;
}
if source.set(key, &value).await.is_ok() {
if source.set(T::KEY, &value).await.is_ok() {
break;
}
}
@@ -169,14 +126,14 @@ impl ConfigManager {
}
}
async fn set_by_key<T: Config>(&self, key: &str, config: &T) -> Result<(), ConfigError> {
pub async fn set<T: Config>(&self, config: &T) -> Result<(), ConfigError> {
let value = serde_json::to_value(config).map_err(|e| ConfigError::Serialization {
key: key.to_string(),
key: T::KEY.to_string(),
source: e,
})?;
for source in &self.sources {
source.set(key, &value).await?;
source.set(T::KEY, &value).await?;
}
Ok(())
@@ -217,33 +174,6 @@ 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"))
}
@@ -887,155 +817,4 @@ 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,10 +4,7 @@ use async_trait::async_trait;
pub struct EnvSource;
fn env_key_for(config_key: &str) -> String {
// 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)
format!("HARMONY_CONFIG_{}", config_key)
}
#[async_trait]

View File

@@ -46,10 +46,9 @@ 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,6 +77,14 @@ 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 }
@@ -443,6 +451,128 @@ 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 {