From f241bf793e6a228ce85fabb59aecc71545e35433 Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Sun, 12 Jan 2025 15:32:14 -0500 Subject: [PATCH] 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. --- harmony-rs/harmony/src/domain/data/version.rs | 4 +-- .../harmony/src/domain/executors/mod.rs | 4 --- harmony-rs/harmony/src/domain/filter.rs | 4 +-- harmony-rs/harmony/src/domain/hardware/mod.rs | 8 +++--- .../harmony/src/domain/inventory/mod.rs | 5 ++-- harmony-rs/harmony/src/domain/maestro/mod.rs | 1 - harmony-rs/harmony/src/domain/score.rs | 2 +- .../harmony/src/domain/topology/network.rs | 2 +- harmony-rs/harmony/src/infra/hp_ilo/mod.rs | 6 +++++ harmony-rs/harmony/src/infra/intel_amt/mod.rs | 2 ++ .../src/infra/opnsense/load_balancer.rs | 4 +-- harmony-rs/harmony/src/infra/opnsense/mod.rs | 3 --- harmony-rs/harmony/src/infra/opnsense/tftp.rs | 4 +-- harmony-rs/harmony/src/lib.rs | 2 +- harmony-rs/harmony/src/modules/dhcp.rs | 11 +------- harmony-rs/harmony/src/modules/dns.rs | 8 +----- harmony-rs/harmony/src/modules/http.rs | 2 +- .../harmony/src/modules/load_balancer.rs | 2 +- .../harmony/src/modules/okd/bootstrap_dhcp.rs | 3 +-- harmony-rs/harmony/src/modules/tftp.rs | 2 +- harmony-rs/harmony_types/src/lib.rs | 2 +- harmony-rs/opnsense-config/Cargo.toml | 2 +- .../opnsense-config/src/config/config.rs | 2 +- .../opnsense-config/src/modules/dhcp.rs | 27 +------------------ harmony-rs/opnsense-config/src/modules/dns.rs | 10 ++----- 25 files changed, 38 insertions(+), 84 deletions(-) diff --git a/harmony-rs/harmony/src/domain/data/version.rs b/harmony-rs/harmony/src/domain/data/version.rs index c76bd14..b6b4193 100644 --- a/harmony-rs/harmony/src/domain/data/version.rs +++ b/harmony-rs/harmony/src/domain/data/version.rs @@ -5,13 +5,13 @@ pub struct Version { #[derive(Debug, Clone)] pub struct VersionError { - msg: String, + _msg: String, } impl From for VersionError { fn from(value: semver::Error) -> Self { Self { - msg: value.to_string(), + _msg: value.to_string(), } } } diff --git a/harmony-rs/harmony/src/domain/executors/mod.rs b/harmony-rs/harmony/src/domain/executors/mod.rs index e7ca378..fb95701 100644 --- a/harmony-rs/harmony/src/domain/executors/mod.rs +++ b/harmony-rs/harmony/src/domain/executors/mod.rs @@ -4,10 +4,6 @@ use async_trait::async_trait; use super::topology::IpAddress; -pub struct ExecutorResult { - message: String, -} - #[derive(Debug)] pub enum ExecutorError { NetworkError(String), diff --git a/harmony-rs/harmony/src/domain/filter.rs b/harmony-rs/harmony/src/domain/filter.rs index 39cd734..61e4507 100644 --- a/harmony-rs/harmony/src/domain/filter.rs +++ b/harmony-rs/harmony/src/domain/filter.rs @@ -10,6 +10,6 @@ pub type FilterValue = String; #[derive(Debug, new, Clone)] pub struct Filter { - kind: FilterKind, - value: FilterValue, + _kind: FilterKind, + _value: FilterValue, } diff --git a/harmony-rs/harmony/src/domain/hardware/mod.rs b/harmony-rs/harmony/src/domain/hardware/mod.rs index a1d5614..73233db 100644 --- a/harmony-rs/harmony/src/domain/hardware/mod.rs +++ b/harmony-rs/harmony/src/domain/hardware/mod.rs @@ -98,14 +98,14 @@ pub struct Storage { #[derive(Debug, Clone)] pub struct Switch { - interface: Vec, - management_interface: NetworkInterface, + _interface: Vec, + _management_interface: NetworkInterface, } #[derive(Debug, new, Clone)] pub struct Label { - name: String, - value: String, + _name: String, + _value: String, } pub type Address = String; diff --git a/harmony-rs/harmony/src/domain/inventory/mod.rs b/harmony-rs/harmony/src/domain/inventory/mod.rs index 930ee02..c33c9f8 100644 --- a/harmony-rs/harmony/src/domain/inventory/mod.rs +++ b/harmony-rs/harmony/src/domain/inventory/mod.rs @@ -7,12 +7,13 @@ pub struct InventorySlice; impl InventoryFilter { pub fn apply(&self, _inventory: &Inventory) -> InventorySlice { - // TODO apply inventory filter, refactor as a slice - todo!() + info!("Applying inventory filter {:?}", self.target); + todo!("TODO apply inventory filter, refactor as a slice") } } use derive_new::new; +use log::info; use super::{ filter::Filter, diff --git a/harmony-rs/harmony/src/domain/maestro/mod.rs b/harmony-rs/harmony/src/domain/maestro/mod.rs index 05a6f2a..713b9da 100644 --- a/harmony-rs/harmony/src/domain/maestro/mod.rs +++ b/harmony-rs/harmony/src/domain/maestro/mod.rs @@ -1,7 +1,6 @@ use derive_new::new; use log::info; -use crate::topology::HostBinding; use super::{ interpret::{Interpret, InterpretError, Outcome}, diff --git a/harmony-rs/harmony/src/domain/score.rs b/harmony-rs/harmony/src/domain/score.rs index cc8b05f..fedc848 100644 --- a/harmony-rs/harmony/src/domain/score.rs +++ b/harmony-rs/harmony/src/domain/score.rs @@ -1,4 +1,4 @@ -use super::{interpret::Interpret, inventory::InventorySlice}; +use super::interpret::Interpret; pub trait Score: std::fmt::Debug { type InterpretType: Interpret + std::fmt::Debug; diff --git a/harmony-rs/harmony/src/domain/topology/network.rs b/harmony-rs/harmony/src/domain/topology/network.rs index c05142a..f45c87d 100644 --- a/harmony-rs/harmony/src/domain/topology/network.rs +++ b/harmony-rs/harmony/src/domain/topology/network.rs @@ -1,4 +1,4 @@ -use std::{error::Error, net::Ipv4Addr, str::FromStr}; +use std::{net::Ipv4Addr, str::FromStr}; use async_trait::async_trait; use harmony_types::net::MacAddress; diff --git a/harmony-rs/harmony/src/infra/hp_ilo/mod.rs b/harmony-rs/harmony/src/infra/hp_ilo/mod.rs index 798b2cd..051f175 100644 --- a/harmony-rs/harmony/src/infra/hp_ilo/mod.rs +++ b/harmony-rs/harmony/src/infra/hp_ilo/mod.rs @@ -2,6 +2,7 @@ use crate::hardware::ManagementInterface; use crate::topology::IpAddress; use derive_new::new; use harmony_types::net::MacAddress; +use log::info; #[derive(new)] pub struct HPIlo { @@ -11,6 +12,11 @@ pub struct HPIlo { impl ManagementInterface for HPIlo { 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!() } diff --git a/harmony-rs/harmony/src/infra/intel_amt/mod.rs b/harmony-rs/harmony/src/infra/intel_amt/mod.rs index 62815c5..088afd5 100644 --- a/harmony-rs/harmony/src/infra/intel_amt/mod.rs +++ b/harmony-rs/harmony/src/infra/intel_amt/mod.rs @@ -1,6 +1,7 @@ use crate::hardware::ManagementInterface; use derive_new::new; use harmony_types::net::MacAddress; +use log::info; #[derive(new)] pub struct IntelAmtManagement { @@ -9,6 +10,7 @@ pub struct IntelAmtManagement { impl ManagementInterface for IntelAmtManagement { fn boot_to_pxe(&self) { + info!("Launching boot to pxe for mac address {}", self.mac_address); todo!() } diff --git a/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs b/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs index c92698d..ae5ecef 100644 --- a/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs +++ b/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs @@ -39,7 +39,7 @@ impl LoadBalancer for OPNSenseFirewall { } async fn remove_service(&self, service: &LoadBalancerService) -> Result<(), ExecutorError> { - todo!() + todo!("Remove service not implemented yet {service:?}") } 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 let healthcheck = if let Some(health_check) = &service.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 { name: format!("HTTP_{http_method}_{path}"), uuid: Uuid::new_v4().to_string(), diff --git a/harmony-rs/harmony/src/infra/opnsense/mod.rs b/harmony-rs/harmony/src/infra/opnsense/mod.rs index 502690c..1bbea69 100644 --- a/harmony-rs/harmony/src/infra/opnsense/mod.rs +++ b/harmony-rs/harmony/src/infra/opnsense/mod.rs @@ -20,7 +20,6 @@ use crate::{ pub struct OPNSenseFirewall { opnsense_config: Arc>, host: LogicalHost, - cluster_nic_name: String, } impl OPNSenseFirewall { @@ -31,7 +30,6 @@ impl OPNSenseFirewall { pub async fn new( host: LogicalHost, port: Option, - cluster_nic_name: &str, username: &str, password: &str, ) -> Self { @@ -40,7 +38,6 @@ impl OPNSenseFirewall { opnsense_config::Config::from_credentials(host.ip, port, username, password).await, )), host, - cluster_nic_name: cluster_nic_name.into(), } } diff --git a/harmony-rs/harmony/src/infra/opnsense/tftp.rs b/harmony-rs/harmony/src/infra/opnsense/tftp.rs index 0c8b293..3f1156e 100644 --- a/harmony-rs/harmony/src/infra/opnsense/tftp.rs +++ b/harmony-rs/harmony/src/infra/opnsense/tftp.rs @@ -1,5 +1,5 @@ use async_trait::async_trait; -use log::{debug, info}; +use log::info; use crate::{ executors::ExecutorError, @@ -22,7 +22,7 @@ impl TftpServer for OPNSenseFirewall { .await .map_err(|e| ExecutorError::UnexpectedError(e.to_string()))?; } - Url::Remote(url) => todo!(), + Url::Remote(url) => todo!("This url is not supported yet {url}"), } Ok(()) } diff --git a/harmony-rs/harmony/src/lib.rs b/harmony-rs/harmony/src/lib.rs index 3c2dff0..9e54190 100644 --- a/harmony-rs/harmony/src/lib.rs +++ b/harmony-rs/harmony/src/lib.rs @@ -5,5 +5,5 @@ pub mod modules; #[cfg(test)] mod test { - use crate::infra::opnsense::OPNSenseFirewall; + } diff --git a/harmony-rs/harmony/src/modules/dhcp.rs b/harmony-rs/harmony/src/modules/dhcp.rs index 18798b4..017c60f 100644 --- a/harmony-rs/harmony/src/modules/dhcp.rs +++ b/harmony-rs/harmony/src/modules/dhcp.rs @@ -5,10 +5,7 @@ use derive_new::new; use log::info; use crate::{ - domain::{ - data::{Id, Version}, - interpret::InterpretStatus, - }, + domain::{data::Version, interpret::InterpretStatus}, interpret::{Interpret, InterpretError, InterpretName, Outcome}, inventory::Inventory, topology::{DHCPStaticEntry, HAClusterTopology, HostBinding, IpAddress}, @@ -36,21 +33,15 @@ impl Score for DhcpScore { pub struct DhcpInterpret { score: DhcpScore, version: Version, - id: Id, - name: String, status: InterpretStatus, } impl DhcpInterpret { pub fn new(score: DhcpScore) -> Self { 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 { version, - id, - name, score, status: InterpretStatus::QUEUED, } diff --git a/harmony-rs/harmony/src/modules/dns.rs b/harmony-rs/harmony/src/modules/dns.rs index 76c5be1..3bb005e 100644 --- a/harmony-rs/harmony/src/modules/dns.rs +++ b/harmony-rs/harmony/src/modules/dns.rs @@ -3,7 +3,7 @@ use derive_new::new; use log::info; use crate::{ - data::{Id, Version}, + data::Version, interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome}, inventory::Inventory, score::Score, @@ -29,21 +29,15 @@ impl Score for DnsScore { pub struct DnsInterpret { score: DnsScore, version: Version, - id: Id, - name: String, status: InterpretStatus, } impl DnsInterpret { pub fn new(score: DnsScore) -> Self { 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 { version, - id, - name, score, status: InterpretStatus::QUEUED, } diff --git a/harmony-rs/harmony/src/modules/http.rs b/harmony-rs/harmony/src/modules/http.rs index d6dd374..b5d0220 100644 --- a/harmony-rs/harmony/src/modules/http.rs +++ b/harmony-rs/harmony/src/modules/http.rs @@ -31,7 +31,7 @@ pub struct HttpInterpret { impl Interpret for HttpInterpret { async fn execute( &self, - inventory: &Inventory, + _inventory: &Inventory, topology: &HAClusterTopology, ) -> Result { let http_server = &topology.http_server; diff --git a/harmony-rs/harmony/src/modules/load_balancer.rs b/harmony-rs/harmony/src/modules/load_balancer.rs index 0836930..a140fe1 100644 --- a/harmony-rs/harmony/src/modules/load_balancer.rs +++ b/harmony-rs/harmony/src/modules/load_balancer.rs @@ -48,7 +48,7 @@ impl LoadBalancerInterpret { impl Interpret for LoadBalancerInterpret { async fn execute( &self, - inventory: &Inventory, + _inventory: &Inventory, topology: &HAClusterTopology, ) -> Result { topology.load_balancer.ensure_initialized().await?; diff --git a/harmony-rs/harmony/src/modules/okd/bootstrap_dhcp.rs b/harmony-rs/harmony/src/modules/okd/bootstrap_dhcp.rs index abd96fe..9b34a71 100644 --- a/harmony-rs/harmony/src/modules/okd/bootstrap_dhcp.rs +++ b/harmony-rs/harmony/src/modules/okd/bootstrap_dhcp.rs @@ -2,10 +2,9 @@ use crate::{ inventory::Inventory, modules::dhcp::DhcpScore, score::Score, - topology::{HAClusterTopology, HostBinding, LogicalHost}, + topology::{HAClusterTopology, HostBinding}, }; -use harmony_macros::ip; #[derive(Debug)] pub struct OKDBootstrapDhcpScore { dhcp_score: DhcpScore, diff --git a/harmony-rs/harmony/src/modules/tftp.rs b/harmony-rs/harmony/src/modules/tftp.rs index f41e5bf..df68974 100644 --- a/harmony-rs/harmony/src/modules/tftp.rs +++ b/harmony-rs/harmony/src/modules/tftp.rs @@ -31,7 +31,7 @@ pub struct TftpInterpret { impl Interpret for TftpInterpret { async fn execute( &self, - inventory: &Inventory, + _inventory: &Inventory, topology: &HAClusterTopology, ) -> Result { let tftp_server = &topology.tftp_server; diff --git a/harmony-rs/harmony_types/src/lib.rs b/harmony-rs/harmony_types/src/lib.rs index 05048c7..71dbbaf 100644 --- a/harmony-rs/harmony_types/src/lib.rs +++ b/harmony-rs/harmony_types/src/lib.rs @@ -1,5 +1,5 @@ pub mod net { - #[derive(Clone, Debug, PartialEq, Eq, Hash)] + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub struct MacAddress(pub [u8; 6]); impl MacAddress { diff --git a/harmony-rs/opnsense-config/Cargo.toml b/harmony-rs/opnsense-config/Cargo.toml index 16954eb..bb9a425 100644 --- a/harmony-rs/opnsense-config/Cargo.toml +++ b/harmony-rs/opnsense-config/Cargo.toml @@ -18,7 +18,7 @@ opnsense-config-xml = { path = "../opnsense-config-xml" } chrono = "0.4.38" russh-sftp = "2.0.6" serde_json = "1.0.133" -tokio-util = "0.7.13" +tokio-util = { version = "0.7.13", features = [ "codec" ] } tokio-stream = "0.1.17" [dev-dependencies] diff --git a/harmony-rs/opnsense-config/src/config/config.rs b/harmony-rs/opnsense-config/src/config/config.rs index 6681ad9..5d5ca6f 100644 --- a/harmony-rs/opnsense-config/src/config/config.rs +++ b/harmony-rs/opnsense-config/src/config/config.rs @@ -35,7 +35,7 @@ impl Config { } 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 { diff --git a/harmony-rs/opnsense-config/src/modules/dhcp.rs b/harmony-rs/opnsense-config/src/modules/dhcp.rs index 83d8487..ed1e0bf 100644 --- a/harmony-rs/opnsense-config/src/modules/dhcp.rs +++ b/harmony-rs/opnsense-config/src/modules/dhcp.rs @@ -86,13 +86,7 @@ impl<'a> DhcpConfig<'a> { return Err(DhcpError::InvalidMacAddress(mac)); } - // TODO verify if 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())); - // } + // TODO validate that address is in subnet range if existing_mappings.iter().any(|m| { m.ipaddr @@ -147,25 +141,6 @@ impl<'a> DhcpConfig<'a> { .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::() - .expect("Invalid DHCP range start"); - let range_end = range.to.parse::().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, Error> { let list_static_output = self .opnsense_shell diff --git a/harmony-rs/opnsense-config/src/modules/dns.rs b/harmony-rs/opnsense-config/src/modules/dns.rs index 4f83502..cdb1952 100644 --- a/harmony-rs/opnsense-config/src/modules/dns.rs +++ b/harmony-rs/opnsense-config/src/modules/dns.rs @@ -1,20 +1,14 @@ -use std::sync::Arc; - use opnsense_config_xml::{Host, OPNsense}; use crate::config::OPNsenseShell; pub struct DnsConfig<'a> { opnsense: &'a mut OPNsense, - opnsense_shell: Arc, } impl<'a> DnsConfig<'a> { - pub fn new(opnsense: &'a mut OPNsense, opnsense_shell: Arc) -> Self { - Self { - opnsense, - opnsense_shell, - } + pub fn new(opnsense: &'a mut OPNsense) -> Self { + Self { opnsense } } pub fn register_hosts(&mut self, mut hosts: Vec) {