fix(dhcp): remove unused IP range check and simplify DnsConfig

Remove the commented-out IP range validation in `DhcpConfig` and simplify the `DnsConfig` constructor by removing an unnecessary parameter, addressing several compiler warnings.
This commit is contained in:
Jean-Gabriel Gill-Couture 2025-01-12 15:32:14 -05:00
parent cad63ecf20
commit f241bf793e
25 changed files with 38 additions and 84 deletions

View File

@ -5,13 +5,13 @@ pub struct Version {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct VersionError { pub struct VersionError {
msg: String, _msg: String,
} }
impl From<semver::Error> for VersionError { impl From<semver::Error> for VersionError {
fn from(value: semver::Error) -> Self { fn from(value: semver::Error) -> Self {
Self { Self {
msg: value.to_string(), _msg: value.to_string(),
} }
} }
} }

View File

@ -4,10 +4,6 @@ use async_trait::async_trait;
use super::topology::IpAddress; use super::topology::IpAddress;
pub struct ExecutorResult {
message: String,
}
#[derive(Debug)] #[derive(Debug)]
pub enum ExecutorError { pub enum ExecutorError {
NetworkError(String), NetworkError(String),

View File

@ -10,6 +10,6 @@ pub type FilterValue = String;
#[derive(Debug, new, Clone)] #[derive(Debug, new, Clone)]
pub struct Filter { pub struct Filter {
kind: FilterKind, _kind: FilterKind,
value: FilterValue, _value: FilterValue,
} }

View File

@ -98,14 +98,14 @@ pub struct Storage {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct Switch { pub struct Switch {
interface: Vec<NetworkInterface>, _interface: Vec<NetworkInterface>,
management_interface: NetworkInterface, _management_interface: NetworkInterface,
} }
#[derive(Debug, new, Clone)] #[derive(Debug, new, Clone)]
pub struct Label { pub struct Label {
name: String, _name: String,
value: String, _value: String,
} }
pub type Address = String; pub type Address = String;

View File

@ -7,12 +7,13 @@ pub struct InventorySlice;
impl InventoryFilter { impl InventoryFilter {
pub fn apply(&self, _inventory: &Inventory) -> InventorySlice { pub fn apply(&self, _inventory: &Inventory) -> InventorySlice {
// TODO apply inventory filter, refactor as a slice info!("Applying inventory filter {:?}", self.target);
todo!() todo!("TODO apply inventory filter, refactor as a slice")
} }
} }
use derive_new::new; use derive_new::new;
use log::info;
use super::{ use super::{
filter::Filter, filter::Filter,

View File

@ -1,7 +1,6 @@
use derive_new::new; use derive_new::new;
use log::info; use log::info;
use crate::topology::HostBinding;
use super::{ use super::{
interpret::{Interpret, InterpretError, Outcome}, interpret::{Interpret, InterpretError, Outcome},

View File

@ -1,4 +1,4 @@
use super::{interpret::Interpret, inventory::InventorySlice}; use super::interpret::Interpret;
pub trait Score: std::fmt::Debug { pub trait Score: std::fmt::Debug {
type InterpretType: Interpret + std::fmt::Debug; type InterpretType: Interpret + std::fmt::Debug;

View File

@ -1,4 +1,4 @@
use std::{error::Error, net::Ipv4Addr, str::FromStr}; use std::{net::Ipv4Addr, str::FromStr};
use async_trait::async_trait; use async_trait::async_trait;
use harmony_types::net::MacAddress; use harmony_types::net::MacAddress;

View File

@ -2,6 +2,7 @@ use crate::hardware::ManagementInterface;
use crate::topology::IpAddress; use crate::topology::IpAddress;
use derive_new::new; use derive_new::new;
use harmony_types::net::MacAddress; use harmony_types::net::MacAddress;
use log::info;
#[derive(new)] #[derive(new)]
pub struct HPIlo { pub struct HPIlo {
@ -11,6 +12,11 @@ pub struct HPIlo {
impl ManagementInterface for HPIlo { impl ManagementInterface for HPIlo {
fn boot_to_pxe(&self) { fn boot_to_pxe(&self) {
info!(
"Launching boot to pxe for ip {} mac address {}",
&self.ip_address.map_or(String::new(), |i| i.to_string()),
&self.mac_address.map_or(String::new(), |m| m.to_string()),
);
todo!() todo!()
} }

View File

@ -1,6 +1,7 @@
use crate::hardware::ManagementInterface; use crate::hardware::ManagementInterface;
use derive_new::new; use derive_new::new;
use harmony_types::net::MacAddress; use harmony_types::net::MacAddress;
use log::info;
#[derive(new)] #[derive(new)]
pub struct IntelAmtManagement { pub struct IntelAmtManagement {
@ -9,6 +10,7 @@ pub struct IntelAmtManagement {
impl ManagementInterface for IntelAmtManagement { impl ManagementInterface for IntelAmtManagement {
fn boot_to_pxe(&self) { fn boot_to_pxe(&self) {
info!("Launching boot to pxe for mac address {}", self.mac_address);
todo!() todo!()
} }

View File

@ -39,7 +39,7 @@ impl LoadBalancer for OPNSenseFirewall {
} }
async fn remove_service(&self, service: &LoadBalancerService) -> Result<(), ExecutorError> { async fn remove_service(&self, service: &LoadBalancerService) -> Result<(), ExecutorError> {
todo!() todo!("Remove service not implemented yet {service:?}")
} }
async fn commit_config(&self) -> Result<(), ExecutorError> { async fn commit_config(&self) -> Result<(), ExecutorError> {
@ -234,7 +234,7 @@ pub(crate) fn harmony_load_balancer_service_to_haproxy_xml(
// frontend points to backend // frontend points to backend
let healthcheck = if let Some(health_check) = &service.health_check { let healthcheck = if let Some(health_check) = &service.health_check {
match health_check { match health_check {
HealthCheck::HTTP(path, http_method, http_status_code) => { HealthCheck::HTTP(path, http_method, _http_status_code) => {
let haproxy_check = HAProxyHealthCheck { let haproxy_check = HAProxyHealthCheck {
name: format!("HTTP_{http_method}_{path}"), name: format!("HTTP_{http_method}_{path}"),
uuid: Uuid::new_v4().to_string(), uuid: Uuid::new_v4().to_string(),

View File

@ -20,7 +20,6 @@ use crate::{
pub struct OPNSenseFirewall { pub struct OPNSenseFirewall {
opnsense_config: Arc<RwLock<opnsense_config::Config>>, opnsense_config: Arc<RwLock<opnsense_config::Config>>,
host: LogicalHost, host: LogicalHost,
cluster_nic_name: String,
} }
impl OPNSenseFirewall { impl OPNSenseFirewall {
@ -31,7 +30,6 @@ impl OPNSenseFirewall {
pub async fn new( pub async fn new(
host: LogicalHost, host: LogicalHost,
port: Option<u16>, port: Option<u16>,
cluster_nic_name: &str,
username: &str, username: &str,
password: &str, password: &str,
) -> Self { ) -> Self {
@ -40,7 +38,6 @@ impl OPNSenseFirewall {
opnsense_config::Config::from_credentials(host.ip, port, username, password).await, opnsense_config::Config::from_credentials(host.ip, port, username, password).await,
)), )),
host, host,
cluster_nic_name: cluster_nic_name.into(),
} }
} }

View File

@ -1,5 +1,5 @@
use async_trait::async_trait; use async_trait::async_trait;
use log::{debug, info}; use log::info;
use crate::{ use crate::{
executors::ExecutorError, executors::ExecutorError,
@ -22,7 +22,7 @@ impl TftpServer for OPNSenseFirewall {
.await .await
.map_err(|e| ExecutorError::UnexpectedError(e.to_string()))?; .map_err(|e| ExecutorError::UnexpectedError(e.to_string()))?;
} }
Url::Remote(url) => todo!(), Url::Remote(url) => todo!("This url is not supported yet {url}"),
} }
Ok(()) Ok(())
} }

View File

@ -5,5 +5,5 @@ pub mod modules;
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use crate::infra::opnsense::OPNSenseFirewall;
} }

View File

@ -5,10 +5,7 @@ use derive_new::new;
use log::info; use log::info;
use crate::{ use crate::{
domain::{ domain::{data::Version, interpret::InterpretStatus},
data::{Id, Version},
interpret::InterpretStatus,
},
interpret::{Interpret, InterpretError, InterpretName, Outcome}, interpret::{Interpret, InterpretError, InterpretName, Outcome},
inventory::Inventory, inventory::Inventory,
topology::{DHCPStaticEntry, HAClusterTopology, HostBinding, IpAddress}, topology::{DHCPStaticEntry, HAClusterTopology, HostBinding, IpAddress},
@ -36,21 +33,15 @@ impl Score for DhcpScore {
pub struct DhcpInterpret { pub struct DhcpInterpret {
score: DhcpScore, score: DhcpScore,
version: Version, version: Version,
id: Id,
name: String,
status: InterpretStatus, status: InterpretStatus,
} }
impl DhcpInterpret { impl DhcpInterpret {
pub fn new(score: DhcpScore) -> Self { pub fn new(score: DhcpScore) -> Self {
let version = Version::from("1.0.0").expect("Version should be valid"); let version = Version::from("1.0.0").expect("Version should be valid");
let name = "DhcpInterpret".to_string();
let id = Id::from_string(format!("{name}_{version}"));
Self { Self {
version, version,
id,
name,
score, score,
status: InterpretStatus::QUEUED, status: InterpretStatus::QUEUED,
} }

View File

@ -3,7 +3,7 @@ use derive_new::new;
use log::info; use log::info;
use crate::{ use crate::{
data::{Id, Version}, data::Version,
interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome}, interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
inventory::Inventory, inventory::Inventory,
score::Score, score::Score,
@ -29,21 +29,15 @@ impl Score for DnsScore {
pub struct DnsInterpret { pub struct DnsInterpret {
score: DnsScore, score: DnsScore,
version: Version, version: Version,
id: Id,
name: String,
status: InterpretStatus, status: InterpretStatus,
} }
impl DnsInterpret { impl DnsInterpret {
pub fn new(score: DnsScore) -> Self { pub fn new(score: DnsScore) -> Self {
let version = Version::from("1.0.0").expect("Version should be valid"); let version = Version::from("1.0.0").expect("Version should be valid");
let name = "DnsInterpret".to_string();
let id = Id::from_string(format!("{name}_{version}"));
Self { Self {
version, version,
id,
name,
score, score,
status: InterpretStatus::QUEUED, status: InterpretStatus::QUEUED,
} }

View File

@ -31,7 +31,7 @@ pub struct HttpInterpret {
impl Interpret for HttpInterpret { impl Interpret for HttpInterpret {
async fn execute( async fn execute(
&self, &self,
inventory: &Inventory, _inventory: &Inventory,
topology: &HAClusterTopology, topology: &HAClusterTopology,
) -> Result<Outcome, InterpretError> { ) -> Result<Outcome, InterpretError> {
let http_server = &topology.http_server; let http_server = &topology.http_server;

View File

@ -48,7 +48,7 @@ impl LoadBalancerInterpret {
impl Interpret for LoadBalancerInterpret { impl Interpret for LoadBalancerInterpret {
async fn execute( async fn execute(
&self, &self,
inventory: &Inventory, _inventory: &Inventory,
topology: &HAClusterTopology, topology: &HAClusterTopology,
) -> Result<Outcome, InterpretError> { ) -> Result<Outcome, InterpretError> {
topology.load_balancer.ensure_initialized().await?; topology.load_balancer.ensure_initialized().await?;

View File

@ -2,10 +2,9 @@ use crate::{
inventory::Inventory, inventory::Inventory,
modules::dhcp::DhcpScore, modules::dhcp::DhcpScore,
score::Score, score::Score,
topology::{HAClusterTopology, HostBinding, LogicalHost}, topology::{HAClusterTopology, HostBinding},
}; };
use harmony_macros::ip;
#[derive(Debug)] #[derive(Debug)]
pub struct OKDBootstrapDhcpScore { pub struct OKDBootstrapDhcpScore {
dhcp_score: DhcpScore, dhcp_score: DhcpScore,

View File

@ -31,7 +31,7 @@ pub struct TftpInterpret {
impl Interpret for TftpInterpret { impl Interpret for TftpInterpret {
async fn execute( async fn execute(
&self, &self,
inventory: &Inventory, _inventory: &Inventory,
topology: &HAClusterTopology, topology: &HAClusterTopology,
) -> Result<Outcome, InterpretError> { ) -> Result<Outcome, InterpretError> {
let tftp_server = &topology.tftp_server; let tftp_server = &topology.tftp_server;

View File

@ -1,5 +1,5 @@
pub mod net { pub mod net {
#[derive(Clone, Debug, PartialEq, Eq, Hash)] #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct MacAddress(pub [u8; 6]); pub struct MacAddress(pub [u8; 6]);
impl MacAddress { impl MacAddress {

View File

@ -18,7 +18,7 @@ opnsense-config-xml = { path = "../opnsense-config-xml" }
chrono = "0.4.38" chrono = "0.4.38"
russh-sftp = "2.0.6" russh-sftp = "2.0.6"
serde_json = "1.0.133" serde_json = "1.0.133"
tokio-util = "0.7.13" tokio-util = { version = "0.7.13", features = [ "codec" ] }
tokio-stream = "0.1.17" tokio-stream = "0.1.17"
[dev-dependencies] [dev-dependencies]

View File

@ -35,7 +35,7 @@ impl Config {
} }
pub fn dns(&mut self) -> DnsConfig { pub fn dns(&mut self) -> DnsConfig {
DnsConfig::new(&mut self.opnsense, self.shell.clone()) DnsConfig::new(&mut self.opnsense)
} }
pub fn tftp(&mut self) -> TftpConfig { pub fn tftp(&mut self) -> TftpConfig {

View File

@ -86,13 +86,7 @@ impl<'a> DhcpConfig<'a> {
return Err(DhcpError::InvalidMacAddress(mac)); return Err(DhcpError::InvalidMacAddress(mac));
} }
// TODO verify if address is in subnet range // TODO validate that address is in subnet range
// This check here does not do what we want to do, as we want to assign static leases
// outside of the dynamic DHCP pool
// let range = &lan_dhcpd.range;
// if !Self::is_ip_in_range(&ipaddr, range) {
// return Err(DhcpError::IpAddressOutOfRange(ipaddr.to_string()));
// }
if existing_mappings.iter().any(|m| { if existing_mappings.iter().any(|m| {
m.ipaddr m.ipaddr
@ -147,25 +141,6 @@ impl<'a> DhcpConfig<'a> {
.all(|part| part.len() <= 2 && part.chars().all(|c| c.is_ascii_hexdigit())) .all(|part| part.len() <= 2 && part.chars().all(|c| c.is_ascii_hexdigit()))
} }
fn is_ip_in_range(ip: &Ipv4Addr, range: &Range) -> bool {
let range_start = range
.from
.parse::<Ipv4Addr>()
.expect("Invalid DHCP range start");
let range_end = range.to.parse::<Ipv4Addr>().expect("Invalid DHCP range to");
let start_compare = range_start.cmp(ip);
let end_compare = range_end.cmp(ip);
if (Ordering::Less == start_compare || Ordering::Equal == start_compare)
&& (Ordering::Greater == end_compare || Ordering::Equal == end_compare)
{
return true;
} else {
return false;
}
}
pub async fn get_static_mappings(&self) -> Result<Vec<StaticMap>, Error> { pub async fn get_static_mappings(&self) -> Result<Vec<StaticMap>, Error> {
let list_static_output = self let list_static_output = self
.opnsense_shell .opnsense_shell

View File

@ -1,20 +1,14 @@
use std::sync::Arc;
use opnsense_config_xml::{Host, OPNsense}; use opnsense_config_xml::{Host, OPNsense};
use crate::config::OPNsenseShell; use crate::config::OPNsenseShell;
pub struct DnsConfig<'a> { pub struct DnsConfig<'a> {
opnsense: &'a mut OPNsense, opnsense: &'a mut OPNsense,
opnsense_shell: Arc<dyn OPNsenseShell>,
} }
impl<'a> DnsConfig<'a> { impl<'a> DnsConfig<'a> {
pub fn new(opnsense: &'a mut OPNsense, opnsense_shell: Arc<dyn OPNsenseShell>) -> Self { pub fn new(opnsense: &'a mut OPNsense) -> Self {
Self { Self { opnsense }
opnsense,
opnsense_shell,
}
} }
pub fn register_hosts(&mut self, mut hosts: Vec<Host>) { pub fn register_hosts(&mut self, mut hosts: Vec<Host>) {