From 1e1aa53eaaffae1e86df496690d73e2a99571c91 Mon Sep 17 00:00:00 2001
From: jeangab <jeangabriel.gc@gmail.com>
Date: Mon, 23 Sep 2024 00:30:27 -0400
Subject: [PATCH 1/2] feat: Significant refactoring to introduce the
 HostBinding struct that has for sole purpose to bind a PhysicalHost and
 LogicalHost together. The PhysicalHost contains everything hardware up to the
 mac address, LogicalHost ip address, name and above

---
 harmony-rs/README.md                          |   7 ++
 .../harmony/src/domain/executors/mod.rs       |   7 +-
 harmony-rs/harmony/src/domain/hardware/mod.rs |  53 ++++++++-
 .../harmony/src/domain/interpret/mod.rs       |  21 +++-
 .../harmony/src/domain/inventory/mod.rs       |   6 +-
 harmony-rs/harmony/src/domain/maestro/mod.rs  |  24 ++--
 harmony-rs/harmony/src/domain/score.rs        |   1 -
 .../src/domain/topology/host_binding.rs       |  17 +++
 .../src/domain/topology/load_balancer.rs      |   5 +-
 harmony-rs/harmony/src/domain/topology/mod.rs |  41 +++----
 .../harmony/src/domain/topology/network.rs    |  31 ++++-
 .../harmony/src/domain/topology/router.rs     |  15 ++-
 .../ansible/opnsense_dhcp_configuration.rs    |   0
 .../harmony/src/infra/executors/boot/mod.rs   |   0
 harmony-rs/harmony/src/infra/executors/mod.rs |   1 -
 .../harmony/src/infra/executors/russh/mod.rs  |  25 +++-
 harmony-rs/harmony/src/infra/hp_ilo/mod.rs    |  11 +-
 harmony-rs/harmony/src/infra/intel_amt/mod.rs |  14 ++-
 harmony-rs/harmony/src/infra/mod.rs           |   4 +-
 .../harmony/src/infra/opnsense/management.rs  |  22 ++++
 harmony-rs/harmony/src/infra/opnsense/mod.rs  |  62 ++++++++--
 .../harmony/src/modules/opnsense_dhcp.rs      | 109 +++++++++++++-----
 22 files changed, 352 insertions(+), 124 deletions(-)
 create mode 100644 harmony-rs/README.md
 create mode 100644 harmony-rs/harmony/src/domain/topology/host_binding.rs
 delete mode 100644 harmony-rs/harmony/src/infra/executors/ansible/opnsense_dhcp_configuration.rs
 delete mode 100644 harmony-rs/harmony/src/infra/executors/boot/mod.rs
 create mode 100644 harmony-rs/harmony/src/infra/opnsense/management.rs

diff --git a/harmony-rs/README.md b/harmony-rs/README.md
new file mode 100644
index 0000000..83bab1a
--- /dev/null
+++ b/harmony-rs/README.md
@@ -0,0 +1,7 @@
+Due to the current setup being a mix of separate repositories with gitignore and rust workspace, a few options are required for cargo-watch to have the desired behavior :
+
+```sh
+RUST_LOG=info cargo watch --ignore-nothing -w harmony -w private_repos/ -x 'run --bin nationtech' 
+```
+
+This will run the nationtech bin (likely `private_repos/nationtech/src/main.rs`) on any change in the harmony or private_repos folders.
diff --git a/harmony-rs/harmony/src/domain/executors/mod.rs b/harmony-rs/harmony/src/domain/executors/mod.rs
index 9e991cb..e7ca378 100644
--- a/harmony-rs/harmony/src/domain/executors/mod.rs
+++ b/harmony-rs/harmony/src/domain/executors/mod.rs
@@ -31,5 +31,10 @@ impl std::error::Error for ExecutorError {}
 
 #[async_trait]
 pub trait SshClient {
-    async fn test_connection(&self, address: IpAddress, username: &str, password: &str) -> Result<(), ExecutorError>;
+    async fn test_connection(
+        &self,
+        address: IpAddress,
+        username: &str,
+        password: &str,
+    ) -> Result<(), ExecutorError>;
 }
diff --git a/harmony-rs/harmony/src/domain/hardware/mod.rs b/harmony-rs/harmony/src/domain/hardware/mod.rs
index 2a1fdb4..985e16e 100644
--- a/harmony-rs/harmony/src/domain/hardware/mod.rs
+++ b/harmony-rs/harmony/src/domain/hardware/mod.rs
@@ -1,25 +1,68 @@
+use std::sync::Arc;
+
 use derive_new::new;
 
-pub type HostGroup = Vec<Host>;
+use crate::topology::MacAddress;
+
+pub type HostGroup = Vec<PhysicalHost>;
 pub type SwitchGroup = Vec<Switch>;
-pub type FirewallGroup = Vec<Host>;
+pub type FirewallGroup = Vec<PhysicalHost>;
 #[derive(Debug, Clone)]
-pub struct Host {
+pub struct PhysicalHost {
     pub category: HostCategory,
     pub network: Vec<NetworkInterface>,
+    pub management: Arc<dyn ManagementInterface>,
     pub storage: Vec<Storage>,
     pub labels: Vec<Label>,
 }
 
-impl Host {
+impl PhysicalHost {
     pub fn new_empty(category: HostCategory) -> Self {
         Self {
             category,
             network: vec![],
             storage: vec![],
             labels: vec![],
+            management: Arc::new(ManualManagementInterface {}),
         }
     }
+
+    pub fn cluster_mac(&self) -> MacAddress {
+        self.network.get(0).expect("Cluster physical host should have a network interface").mac_address.clone()
+    }
+}
+
+#[derive(new)]
+pub struct ManualManagementInterface;
+
+impl ManagementInterface for ManualManagementInterface {
+    fn boot_to_pxe(&self) {
+        todo!()
+    }
+
+    fn get_mac_address(&self) -> MacAddress {
+        todo!()
+    }
+
+    fn get_supported_protocol_names(&self) -> String {
+        todo!()
+    }
+}
+
+pub trait ManagementInterface: Send + Sync {
+    fn boot_to_pxe(&self);
+    fn get_mac_address(&self) -> MacAddress;
+    fn get_supported_protocol_names(&self) -> String;
+}
+
+impl std::fmt::Debug for dyn ManagementInterface {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_fmt(format_args!(
+            "ManagementInterface mac : {}, protocols : {}",
+            self.get_mac_address(),
+            self.get_supported_protocol_names(),
+        ))
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -34,9 +77,7 @@ pub struct NetworkInterface {
     pub name: String,
     pub mac_address: MacAddress,
     pub speed: u64,
-    pub plugged_in: bool,
 }
-type MacAddress = String;
 
 #[derive(Debug, new, Clone)]
 pub enum StorageConnectionType {
diff --git a/harmony-rs/harmony/src/domain/interpret/mod.rs b/harmony-rs/harmony/src/domain/interpret/mod.rs
index 00fa975..ecf953e 100644
--- a/harmony-rs/harmony/src/domain/interpret/mod.rs
+++ b/harmony-rs/harmony/src/domain/interpret/mod.rs
@@ -4,7 +4,11 @@ use async_trait::async_trait;
 use derive_new::new;
 
 use super::{
-    data::{Id, Version}, executors::ExecutorError, inventory::Inventory, score::Score, topology::HAClusterTopology
+    data::{Id, Version},
+    executors::ExecutorError,
+    inventory::Inventory,
+    score::Score,
+    topology::HAClusterTopology,
 };
 
 pub enum InterpretName {
@@ -21,7 +25,11 @@ impl std::fmt::Display for InterpretName {
 
 #[async_trait]
 pub trait Interpret {
-    async fn execute(&self, inventory: &Inventory, topology: &HAClusterTopology) -> Result<Outcome, InterpretError>;
+    async fn execute(
+        &self,
+        inventory: &Inventory,
+        topology: &HAClusterTopology,
+    ) -> Result<Outcome, InterpretError>;
     fn get_name(&self) -> InterpretName;
     fn get_version(&self) -> Version;
     fn get_status(&self) -> InterpretStatus;
@@ -54,14 +62,13 @@ impl std::fmt::Display for InterpretStatus {
             InterpretStatus::SUCCESS => "SUCCESS",
             InterpretStatus::FAILURE => "FAILURE",
             InterpretStatus::RUNNING => "RUNNING",
-            InterpretStatus::QUEUED  => "QUEUED",
+            InterpretStatus::QUEUED => "QUEUED",
             InterpretStatus::BLOCKED => "BLOCKED",
         };
         f.write_str(msg)
     }
 }
 
-
 #[derive(Debug)]
 pub struct InterpretError {
     msg: String,
@@ -74,8 +81,10 @@ impl std::fmt::Display for InterpretError {
 }
 impl Error for InterpretError {}
 
-impl From<ExecutorError> for InterpretError{
+impl From<ExecutorError> for InterpretError {
     fn from(value: ExecutorError) -> Self {
-        Self { msg: format!("InterpretError : {value}") }
+        Self {
+            msg: format!("InterpretError : {value}"),
+        }
     }
 }
diff --git a/harmony-rs/harmony/src/domain/inventory/mod.rs b/harmony-rs/harmony/src/domain/inventory/mod.rs
index 90e9edb..af4a00c 100644
--- a/harmony-rs/harmony/src/domain/inventory/mod.rs
+++ b/harmony-rs/harmony/src/domain/inventory/mod.rs
@@ -16,15 +16,17 @@ use derive_new::new;
 
 use super::{
     filter::Filter,
-    hardware::{Location, FirewallGroup, HostGroup, SwitchGroup},
+    hardware::{FirewallGroup, HostGroup, Location, SwitchGroup},
 };
 
 #[derive(Debug)]
 pub struct Inventory {
     pub location: Location,
-    pub host: HostGroup,
     pub switch: SwitchGroup,
     pub firewall: FirewallGroup,
+    pub worker_host: HostGroup,
+    pub storage_host: HostGroup,
+    pub control_plane_host: HostGroup,
 }
 
 impl Inventory {
diff --git a/harmony-rs/harmony/src/domain/maestro/mod.rs b/harmony-rs/harmony/src/domain/maestro/mod.rs
index a163cc4..05a6f2a 100644
--- a/harmony-rs/harmony/src/domain/maestro/mod.rs
+++ b/harmony-rs/harmony/src/domain/maestro/mod.rs
@@ -1,7 +1,14 @@
 use derive_new::new;
 use log::info;
 
-use super::{interpret::{Interpret, InterpretError, Outcome}, inventory::Inventory, score::Score, topology::HAClusterTopology};
+use crate::topology::HostBinding;
+
+use super::{
+    interpret::{Interpret, InterpretError, Outcome},
+    inventory::Inventory,
+    score::Score,
+    topology::HAClusterTopology,
+};
 
 #[derive(new)]
 pub struct Maestro {
@@ -12,21 +19,6 @@ pub struct Maestro {
 impl Maestro {
     pub fn start(&mut self) {
         info!("Starting Maestro");
-        self.load_score();
-        self.load_inventory();
-        self.launch_interprets();
-    }
-
-    fn load_score(&mut self) {
-        todo!()
-    }
-
-    fn load_inventory(&mut self) {
-        todo!()
-    }
-
-    fn launch_interprets(&mut self) {
-        todo!()
     }
 
     pub async fn interpret<S: Score>(&self, score: S) -> Result<Outcome, InterpretError> {
diff --git a/harmony-rs/harmony/src/domain/score.rs b/harmony-rs/harmony/src/domain/score.rs
index 32647d6..cc8b05f 100644
--- a/harmony-rs/harmony/src/domain/score.rs
+++ b/harmony-rs/harmony/src/domain/score.rs
@@ -2,6 +2,5 @@ use super::{interpret::Interpret, inventory::InventorySlice};
 
 pub trait Score: std::fmt::Debug {
     type InterpretType: Interpret + std::fmt::Debug;
-    fn get_inventory_filter(&self) -> InventorySlice;
     fn create_interpret(self) -> Self::InterpretType;
 }
diff --git a/harmony-rs/harmony/src/domain/topology/host_binding.rs b/harmony-rs/harmony/src/domain/topology/host_binding.rs
new file mode 100644
index 0000000..c01869a
--- /dev/null
+++ b/harmony-rs/harmony/src/domain/topology/host_binding.rs
@@ -0,0 +1,17 @@
+use derive_new::new;
+
+use crate::hardware::PhysicalHost;
+
+use super::LogicalHost;
+
+/// Represents the binding between a LogicalHost and a PhysicalHost.
+///
+/// This is the only construct that directly maps a logical host to a physical host.
+/// It serves as a bridge between the logical cluster structure and the physical infrastructure.
+#[derive(Debug, new, Clone)]
+pub struct HostBinding {
+    /// Reference to the LogicalHost
+    pub logical_host: LogicalHost,
+    /// Reference to the PhysicalHost
+    pub physical_host: PhysicalHost,
+}
diff --git a/harmony-rs/harmony/src/domain/topology/load_balancer.rs b/harmony-rs/harmony/src/domain/topology/load_balancer.rs
index ce26f55..cf79ac3 100644
--- a/harmony-rs/harmony/src/domain/topology/load_balancer.rs
+++ b/harmony-rs/harmony/src/domain/topology/load_balancer.rs
@@ -1,6 +1,6 @@
-use super::IpAddress;
+use super::{IpAddress, LogicalHost};
 
-pub trait LoadBalancer : Send + Sync{
+pub trait LoadBalancer: Send + Sync {
     fn add_backend(&mut self, backend: Backend) -> Result<(), LoadBalancerError>;
     fn remove_backend(&mut self, backend_id: &str) -> Result<(), LoadBalancerError>;
     fn add_frontend(&mut self, frontend: Frontend) -> Result<(), LoadBalancerError>;
@@ -8,6 +8,7 @@ pub trait LoadBalancer : Send + Sync{
     fn list_backends(&self) -> Vec<Backend>;
     fn list_frontends(&self) -> Vec<Frontend>;
     fn get_ip(&self) -> IpAddress;
+    fn get_host(&self) -> LogicalHost;
 }
 
 impl std::fmt::Debug for dyn LoadBalancer {
diff --git a/harmony-rs/harmony/src/domain/topology/mod.rs b/harmony-rs/harmony/src/domain/topology/mod.rs
index 6612d3d..bd4a172 100644
--- a/harmony-rs/harmony/src/domain/topology/mod.rs
+++ b/harmony-rs/harmony/src/domain/topology/mod.rs
@@ -1,14 +1,14 @@
+mod host_binding;
 mod load_balancer;
 mod router;
 pub use load_balancer::*;
 pub use router::*;
 mod network;
+pub use host_binding::*;
 pub use network::*;
 
 use std::{net::IpAddr, sync::Arc};
 
-use super::hardware::Host;
-
 #[derive(Debug, Clone)]
 pub struct HAClusterTopology {
     pub router: Arc<dyn Router>,
@@ -16,29 +16,24 @@ pub struct HAClusterTopology {
     pub firewall: Arc<dyn Firewall>,
     pub dhcp_server: Arc<dyn DhcpServer>,
     pub dns_server: Arc<dyn DnsServer>,
-    pub control_plane: Vec<ClusterMember>,
-    pub workers: Vec<ClusterMember>,
-    pub switch: Vec<ClusterMember>,
+    pub control_plane: Vec<LogicalHost>,
+    pub workers: Vec<LogicalHost>,
+    pub switch: Vec<LogicalHost>,
 }
 
 pub type IpAddress = IpAddr;
-
+/// Represents a logical member of a cluster that provides one or more services.
+///
+/// A LogicalHost can represent various roles within the infrastructure, such as:
+/// - A firewall appliance hosting DHCP, DNS, PXE, and load balancer services
+/// - A Kubernetes worker node
+/// - A combined Kubernetes worker and Ceph storage node
+/// - A control plane node
+///
+/// This abstraction focuses on the logical role and services, independent of the physical hardware.
 #[derive(Debug, Clone)]
-pub struct ClusterMember {
-    pub management: Arc<dyn ManagementInterface>,
-    pub host: Host,
-}
-
-pub trait ManagementInterface: Send + Sync {
-    fn boot_to_pxe(&self);
-    fn get_ip(&self) -> IpAddress;
-}
-
-impl std::fmt::Debug for dyn ManagementInterface {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_fmt(format_args!(
-            "ManagementInterface with ip {}",
-            self.get_ip()
-        ))
-    }
+pub struct LogicalHost {
+    /// The set of services this logical host provides
+    pub ip: IpAddress,
+    pub name: String,
 }
diff --git a/harmony-rs/harmony/src/domain/topology/network.rs b/harmony-rs/harmony/src/domain/topology/network.rs
index 9d17dca..f88731d 100644
--- a/harmony-rs/harmony/src/domain/topology/network.rs
+++ b/harmony-rs/harmony/src/domain/topology/network.rs
@@ -1,10 +1,11 @@
-use super::IpAddress;
+use super::{IpAddress, LogicalHost};
 
 pub trait Firewall: Send + Sync {
     fn add_rule(&mut self, rule: FirewallRule) -> Result<(), FirewallError>;
     fn remove_rule(&mut self, rule_id: &str) -> Result<(), FirewallError>;
     fn list_rules(&self) -> Vec<FirewallRule>;
     fn get_ip(&self) -> IpAddress;
+    fn get_host(&self) -> LogicalHost;
 }
 
 impl std::fmt::Debug for dyn Firewall {
@@ -14,7 +15,7 @@ impl std::fmt::Debug for dyn Firewall {
 }
 
 pub struct NetworkDomain {
-    pub name: String
+    pub name: String,
 }
 
 pub trait DhcpServer: Send + Sync {
@@ -22,6 +23,7 @@ pub trait DhcpServer: Send + Sync {
     fn remove_static_mapping(&mut self, mac: &MacAddress) -> Result<(), DhcpError>;
     fn list_static_mappings(&self) -> Vec<(MacAddress, IpAddress)>;
     fn get_ip(&self) -> IpAddress;
+    fn get_host(&self) -> LogicalHost;
 }
 
 impl std::fmt::Debug for dyn DhcpServer {
@@ -31,10 +33,16 @@ impl std::fmt::Debug for dyn DhcpServer {
 }
 
 pub trait DnsServer: Send + Sync {
-    fn add_record(&mut self, name: &str, record_type: DnsRecordType, value: &str) -> Result<(), DnsError>;
+    fn add_record(
+        &mut self,
+        name: &str,
+        record_type: DnsRecordType,
+        value: &str,
+    ) -> Result<(), DnsError>;
     fn remove_record(&mut self, name: &str, record_type: DnsRecordType) -> Result<(), DnsError>;
     fn list_records(&self) -> Vec<DnsRecord>;
     fn get_ip(&self) -> IpAddress;
+    fn get_host(&self) -> LogicalHost;
 }
 
 impl std::fmt::Debug for dyn DnsServer {
@@ -66,7 +74,22 @@ pub enum Action {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
-pub struct MacAddress([u8; 6]);
+pub struct MacAddress(pub [u8; 6]);
+
+impl MacAddress {
+    pub fn dummy() -> Self {
+        Self([0,0,0,0,0,0])
+    }
+}
+
+impl std::fmt::Display for MacAddress {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_fmt(format_args!(
+            "MacAddress {}:{}:{}:{}:{}:{}",
+            self.0[0], self.0[1], self.0[2], self.0[3], self.0[4], self.0[5]
+        ))
+    }
+}
 
 #[derive(Clone, Debug)]
 pub enum DnsRecordType {
diff --git a/harmony-rs/harmony/src/domain/topology/router.rs b/harmony-rs/harmony/src/domain/topology/router.rs
index 58e67fb..de4d313 100644
--- a/harmony-rs/harmony/src/domain/topology/router.rs
+++ b/harmony-rs/harmony/src/domain/topology/router.rs
@@ -1,18 +1,21 @@
-use std::sync::Arc;
-
 use cidr::Ipv4Cidr;
 use derive_new::new;
 
-use super::IpAddress;
+use super::{IpAddress, LogicalHost};
 
 pub trait Router: Send + Sync {
     fn get_gateway(&self) -> IpAddress;
     fn get_cidr(&self) -> Ipv4Cidr;
+    fn get_host(&self) -> LogicalHost;
 }
 
 impl std::fmt::Debug for dyn Router {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_fmt(format_args!("Router Gateway : {}, CIDR : {}", self.get_gateway(), self.get_cidr()))
+        f.write_fmt(format_args!(
+            "Router Gateway : {}, CIDR : {}",
+            self.get_gateway(),
+            self.get_cidr()
+        ))
     }
 }
 
@@ -30,4 +33,8 @@ impl Router for UnmanagedRouter {
     fn get_cidr(&self) -> Ipv4Cidr {
         self.cidr.clone()
     }
+
+    fn get_host(&self) -> LogicalHost {
+        todo!()
+    }
 }
diff --git a/harmony-rs/harmony/src/infra/executors/ansible/opnsense_dhcp_configuration.rs b/harmony-rs/harmony/src/infra/executors/ansible/opnsense_dhcp_configuration.rs
deleted file mode 100644
index e69de29..0000000
diff --git a/harmony-rs/harmony/src/infra/executors/boot/mod.rs b/harmony-rs/harmony/src/infra/executors/boot/mod.rs
deleted file mode 100644
index e69de29..0000000
diff --git a/harmony-rs/harmony/src/infra/executors/mod.rs b/harmony-rs/harmony/src/infra/executors/mod.rs
index bcad313..06ffe5e 100644
--- a/harmony-rs/harmony/src/infra/executors/mod.rs
+++ b/harmony-rs/harmony/src/infra/executors/mod.rs
@@ -1,2 +1 @@
 pub mod russh;
-
diff --git a/harmony-rs/harmony/src/infra/executors/russh/mod.rs b/harmony-rs/harmony/src/infra/executors/russh/mod.rs
index ea8acaf..4bc6e73 100644
--- a/harmony-rs/harmony/src/infra/executors/russh/mod.rs
+++ b/harmony-rs/harmony/src/infra/executors/russh/mod.rs
@@ -1,21 +1,34 @@
-use std::sync::Arc;
 use async_trait::async_trait;
+use std::sync::Arc;
 
 use russh::{client, keys::key};
 
-use crate::{domain::executors::{ExecutorError, SshClient}, topology::IpAddress};
+use crate::{
+    domain::executors::{ExecutorError, SshClient},
+    topology::IpAddress,
+};
 
 pub struct RusshClient;
 
 #[async_trait]
 impl SshClient for RusshClient {
-    async fn test_connection(&self, address: IpAddress,_username: &str, _password: &str) -> Result<(), crate::domain::executors::ExecutorError> {
+    async fn test_connection(
+        &self,
+        address: IpAddress,
+        _username: &str,
+        _password: &str,
+    ) -> Result<(), crate::domain::executors::ExecutorError> {
         let config = client::Config::default();
-        let c = Client{};
+        let c = Client {};
         let mut client = client::connect(Arc::new(config), (address, 22), c).await?;
-        match client.authenticate_password("nationtech", "opnsense").await? {
+        match client
+            .authenticate_password("nationtech", "opnsense")
+            .await?
+        {
             true => Ok(()),
-            false => Err(ExecutorError::AuthenticationError("ssh authentication failed".to_string())),
+            false => Err(ExecutorError::AuthenticationError(
+                "ssh authentication failed".to_string(),
+            )),
         }
     }
 }
diff --git a/harmony-rs/harmony/src/infra/hp_ilo/mod.rs b/harmony-rs/harmony/src/infra/hp_ilo/mod.rs
index dc47a8c..eb6fbd7 100644
--- a/harmony-rs/harmony/src/infra/hp_ilo/mod.rs
+++ b/harmony-rs/harmony/src/infra/hp_ilo/mod.rs
@@ -1,10 +1,9 @@
+use crate::{hardware::ManagementInterface, topology::{IpAddress, MacAddress}};
 use derive_new::new;
-use crate::topology::{IpAddress, MacAddress, ManagementInterface};
 
 #[derive(new)]
 pub struct HPIlo {
     ip_address: IpAddress,
-    mac_address: MacAddress,
 }
 
 impl ManagementInterface for HPIlo {
@@ -12,7 +11,11 @@ impl ManagementInterface for HPIlo {
         todo!()
     }
 
-    fn get_ip(&self) -> IpAddress {
-        self.ip_address
+    fn get_mac_address(&self) -> MacAddress {
+        todo!()
+    }
+
+    fn get_supported_protocol_names(&self) -> 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 d51310b..cd80600 100644
--- a/harmony-rs/harmony/src/infra/intel_amt/mod.rs
+++ b/harmony-rs/harmony/src/infra/intel_amt/mod.rs
@@ -1,9 +1,11 @@
+use crate::{
+    hardware::ManagementInterface,
+    topology::{IpAddress, MacAddress},
+};
 use derive_new::new;
-use crate::topology::{IpAddress, MacAddress, ManagementInterface};
 
 #[derive(new)]
 pub struct IntelAmtManagement {
-    ip_address: IpAddress,
     mac_address: MacAddress,
 }
 
@@ -12,7 +14,11 @@ impl ManagementInterface for IntelAmtManagement {
         todo!()
     }
 
-    fn get_ip(&self) -> IpAddress {
-        self.ip_address
+    fn get_mac_address(&self) -> MacAddress {
+        self.mac_address.clone()
+    }
+
+    fn get_supported_protocol_names(&self) -> String {
+        "IntelAMT".to_string()
     }
 }
diff --git a/harmony-rs/harmony/src/infra/mod.rs b/harmony-rs/harmony/src/infra/mod.rs
index 6240d17..b3a94ea 100644
--- a/harmony-rs/harmony/src/infra/mod.rs
+++ b/harmony-rs/harmony/src/infra/mod.rs
@@ -1,4 +1,4 @@
 pub mod executors;
-pub mod opnsense;
-pub mod intel_amt;
 pub mod hp_ilo;
+pub mod intel_amt;
+pub mod opnsense;
diff --git a/harmony-rs/harmony/src/infra/opnsense/management.rs b/harmony-rs/harmony/src/infra/opnsense/management.rs
new file mode 100644
index 0000000..ce0c6b9
--- /dev/null
+++ b/harmony-rs/harmony/src/infra/opnsense/management.rs
@@ -0,0 +1,22 @@
+use derive_new::new;
+
+use crate::{hardware::ManagementInterface, topology::MacAddress};
+
+#[derive(new)]
+pub struct OPNSenseManagementInterface {
+    mac: MacAddress,
+}
+
+impl ManagementInterface for OPNSenseManagementInterface {
+    fn boot_to_pxe(&self) {
+        todo!()
+    }
+
+    fn get_mac_address(&self) -> MacAddress {
+        self.mac.clone()
+    }
+
+    fn get_supported_protocol_names(&self) -> String {
+        "OPNSenseSSH".to_string()
+    }
+}
diff --git a/harmony-rs/harmony/src/infra/opnsense/mod.rs b/harmony-rs/harmony/src/infra/opnsense/mod.rs
index b25a4f1..35e2a2e 100644
--- a/harmony-rs/harmony/src/infra/opnsense/mod.rs
+++ b/harmony-rs/harmony/src/infra/opnsense/mod.rs
@@ -1,10 +1,21 @@
+mod management;
+pub use management::*;
+
+use crate::topology::{
+    Backend, DhcpServer, DnsServer, Firewall, FirewallError, FirewallRule, Frontend, IpAddress,
+    LoadBalancer, LoadBalancerError, LogicalHost,
+};
 use derive_new::new;
-use crate::{hardware::NetworkInterface, topology::{Backend, DhcpServer, DnsServer, Firewall, FirewallError, FirewallRule, Frontend, IpAddress, LoadBalancer, LoadBalancerError}};
 
 #[derive(new, Clone)]
 pub struct OPNSenseFirewall {
-    ip_address: IpAddress,
-    interfaces: Vec<NetworkInterface>,
+    host: LogicalHost,
+}
+
+impl OPNSenseFirewall {
+    pub fn get_ip(&self) -> IpAddress {
+        self.host.ip
+    }
 }
 
 impl Firewall for OPNSenseFirewall {
@@ -21,7 +32,10 @@ impl Firewall for OPNSenseFirewall {
     }
 
     fn get_ip(&self) -> IpAddress {
-        self.ip_address.clone()
+        OPNSenseFirewall::get_ip(self)
+    }
+    fn get_host(&self) -> LogicalHost{
+        self.host.clone()
     }
 }
 
@@ -51,16 +65,26 @@ impl LoadBalancer for OPNSenseFirewall {
     }
 
     fn get_ip(&self) -> IpAddress {
-        self.ip_address.clone()
+        OPNSenseFirewall::get_ip(self)
+    }
+    fn get_host(&self) -> LogicalHost{
+        self.host.clone()
     }
 }
 
 impl DhcpServer for OPNSenseFirewall {
-    fn add_static_mapping(&mut self, _mac: crate::topology::MacAddress, _ip: IpAddress) -> Result<(), crate::topology::DhcpError> {
+    fn add_static_mapping(
+        &mut self,
+        _mac: crate::topology::MacAddress,
+        _ip: IpAddress,
+    ) -> Result<(), crate::topology::DhcpError> {
         todo!()
     }
 
-    fn remove_static_mapping(&mut self, _mac: &crate::topology::MacAddress) -> Result<(), crate::topology::DhcpError> {
+    fn remove_static_mapping(
+        &mut self,
+        _mac: &crate::topology::MacAddress,
+    ) -> Result<(), crate::topology::DhcpError> {
         todo!()
     }
 
@@ -69,15 +93,27 @@ impl DhcpServer for OPNSenseFirewall {
     }
 
     fn get_ip(&self) -> IpAddress {
-        self.ip_address.clone()
+        OPNSenseFirewall::get_ip(self)
+    }
+    fn get_host(&self) -> LogicalHost{
+        self.host.clone()
     }
 }
 impl DnsServer for OPNSenseFirewall {
-    fn add_record(&mut self, _name: &str, _record_type: crate::topology::DnsRecordType, _value: &str) -> Result<(), crate::topology::DnsError> {
+    fn add_record(
+        &mut self,
+        _name: &str,
+        _record_type: crate::topology::DnsRecordType,
+        _value: &str,
+    ) -> Result<(), crate::topology::DnsError> {
         todo!()
     }
 
-    fn remove_record(&mut self, _name: &str, _record_type: crate::topology::DnsRecordType) -> Result<(), crate::topology::DnsError> {
+    fn remove_record(
+        &mut self,
+        _name: &str,
+        _record_type: crate::topology::DnsRecordType,
+    ) -> Result<(), crate::topology::DnsError> {
         todo!()
     }
 
@@ -86,6 +122,10 @@ impl DnsServer for OPNSenseFirewall {
     }
 
     fn get_ip(&self) -> IpAddress {
-        self.ip_address.clone()
+        OPNSenseFirewall::get_ip(&self)
+    }
+
+    fn get_host(&self) -> LogicalHost{
+        self.host.clone()
     }
 }
diff --git a/harmony-rs/harmony/src/modules/opnsense_dhcp.rs b/harmony-rs/harmony/src/modules/opnsense_dhcp.rs
index c9ab56f..68f9e8b 100644
--- a/harmony-rs/harmony/src/modules/opnsense_dhcp.rs
+++ b/harmony-rs/harmony/src/modules/opnsense_dhcp.rs
@@ -2,32 +2,76 @@ use async_trait::async_trait;
 use derive_new::new;
 use log::info;
 
-use crate::{domain::{
-    data::{Id, Version}, hardware::NetworkInterface, interpret::{InterpretError, InterpretStatus, Outcome}, topology::IpAddress
-}, executors::SshClient, infra::executors::russh::RusshClient, inventory::Inventory, topology::HAClusterTopology};
-
-use crate::domain::{
-    interpret::Interpret, interpret::InterpretName, inventory::InventorySlice, score::Score,
+use crate::{
+    domain::{
+        data::{Id, Version},
+        interpret::InterpretStatus,
+    },
+    infra::executors::russh::RusshClient,
+    interpret::{Interpret, InterpretError, InterpretName, Outcome},
+    inventory::Inventory,
+    topology::{HAClusterTopology, HostBinding, IpAddress, MacAddress},
 };
 
-use crate::domain::executors::{ExecutorError, ExecutorResult};
+use crate::domain::score::Score;
 
+#[derive(Debug)]
+pub struct DHCPStaticEntry {
+    name: String,
+    mac: MacAddress,
+    ip: IpAddress,
+}
+
+/// OPNSenseDhcpScore will set static DHCP entries using index based hostname
+/// and ip addresses.
+///
+/// For example :
+/// ```rust
+///
+/// let node1 = todo!(); // Node pointing to clustermember controlplane0 with ip 10.10.0.20 and host with mac 01
+/// let node2 = todo!(); // Node pointing to clustermember controlplane1 with ip 10.10.0.21 and host with mac 02
+/// let node3 = todo!(); // Node pointing to clustermember controlplane2 with ip 10.10.0.22 and host with mac 03
+///
+/// let score = OPNSenseDhcpScore {
+///     nodes: vec![node1, node2, node3],
+/// }
+/// ```
+///
+/// Running such a score would create these static entries :
+///
+/// ```rust
+/// let entries = vec![
+///     DHCPEntry {
+///       mac: 01,
+///       ip: 10.10.0.20,
+///       hostname: "controlplane0"
+///     }
+///     DHCPEntry {
+///       mac: 02,
+///       ip: 10.10.0.21,
+///       hostname: "controlplane0"
+///     }
+///     DHCPEntry {
+///       mac: 03,
+///       ip: 10.10.0.22,
+///       hostname: "controlplane2"
+///     }
+/// ]
+/// ```
 #[derive(Debug, new, Clone)]
-pub struct OPNSenseDhcpScore {}
+pub struct OPNSenseDhcpScore {
+    host_binding: Vec<HostBinding>,
+}
 
 impl Score for OPNSenseDhcpScore {
     type InterpretType = OPNSenseDhcpInterpret;
 
-    fn get_inventory_filter(&self) -> InventorySlice {
-        todo!()
-    }
-
     fn create_interpret(self) -> OPNSenseDhcpInterpret {
         OPNSenseDhcpInterpret::new(self)
     }
 }
 
-/// https://docs.opnsense.org/manual/dhcp.html#advanced-settings
+// https://docs.opnsense.org/manual/dhcp.html#advanced-settings
 #[derive(Debug, Clone)]
 pub struct OPNSenseDhcpInterpret {
     score: OPNSenseDhcpScore,
@@ -71,31 +115,34 @@ impl Interpret for OPNSenseDhcpInterpret {
         todo!()
     }
 
-    async fn execute(&self, inventory: &Inventory, topology: &HAClusterTopology) -> Result<Outcome, InterpretError> {
+    async fn execute(
+        &self,
+        inventory: &Inventory,
+        topology: &HAClusterTopology,
+    ) -> Result<Outcome, InterpretError> {
         info!("Executing {} on inventory {inventory:?}", self.get_name());
-        let ssh_client = RusshClient{};
+        let ssh_client = RusshClient {};
 
+        let entries: Vec<DHCPStaticEntry> = self
+            .score
+            .host_binding
+            .iter()
+            .map(|binding| DHCPStaticEntry {
+                name: binding.logical_host.name.clone(),
+                mac: binding.physical_host.cluster_mac(),
+                ip: binding.logical_host.ip,
+            })
+            .collect();
+        info!("DHCPStaticEntry : {:?}", entries);
         todo!("Filter proper network interfaces and prepare the DHCP configuration");
 
-        Ok(Outcome::new(InterpretStatus::SUCCESS, "Connection test successful".to_string()))
+        Ok(Outcome::new(
+            InterpretStatus::SUCCESS,
+            "Connection test successful".to_string(),
+        ))
     }
 }
 
-pub trait OPNSenseDhcpConfigEditor {
-    fn add_static_host(
-        &self,
-        opnsense_host: IpAddress,
-        credentials: OPNSenseCredentials,
-        interface: NetworkInterface,
-        address: IpAddress,
-    ) -> Result<ExecutorResult, ExecutorError>;
-}
-
-pub struct OPNSenseCredentials {
-    pub user: String,
-    pub password: String,
-}
-
 #[cfg(test)]
 mod test {
 

From 13ba3964c07bf9aac3b8746e1a0e9079c291921c Mon Sep 17 00:00:00 2001
From: jeangab <jeangabriel.gc@gmail.com>
Date: Thu, 26 Sep 2024 22:22:56 -0400
Subject: [PATCH 2/2] feat: Improve DHCP architecture, much better,
 architecture feels good right now

---
 .../harmony/src/domain/topology/network.rs    | 19 +++++++--
 harmony-rs/harmony/src/infra/opnsense/mod.rs  | 21 +++++-----
 .../src/modules/{opnsense_dhcp.rs => dhcp.rs} | 40 ++++++++++---------
 harmony-rs/harmony/src/modules/mod.rs         |  2 +-
 4 files changed, 48 insertions(+), 34 deletions(-)
 rename harmony-rs/harmony/src/modules/{opnsense_dhcp.rs => dhcp.rs} (82%)

diff --git a/harmony-rs/harmony/src/domain/topology/network.rs b/harmony-rs/harmony/src/domain/topology/network.rs
index f88731d..00189e4 100644
--- a/harmony-rs/harmony/src/domain/topology/network.rs
+++ b/harmony-rs/harmony/src/domain/topology/network.rs
@@ -1,5 +1,18 @@
 use super::{IpAddress, LogicalHost};
 
+#[derive(Debug)]
+pub struct DHCPStaticEntry {
+    pub name: String,
+    pub mac: MacAddress,
+    pub ip: IpAddress,
+}
+
+impl std::fmt::Display for DHCPStaticEntry {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_fmt(format_args!("DHCPStaticEntry : name {}, mac {}, ip {}", self.name, self.mac, self.ip))
+    }
+}
+
 pub trait Firewall: Send + Sync {
     fn add_rule(&mut self, rule: FirewallRule) -> Result<(), FirewallError>;
     fn remove_rule(&mut self, rule_id: &str) -> Result<(), FirewallError>;
@@ -19,8 +32,8 @@ pub struct NetworkDomain {
 }
 
 pub trait DhcpServer: Send + Sync {
-    fn add_static_mapping(&mut self, mac: MacAddress, ip: IpAddress) -> Result<(), DhcpError>;
-    fn remove_static_mapping(&mut self, mac: &MacAddress) -> Result<(), DhcpError>;
+    fn add_static_mapping(&self, entry: &DHCPStaticEntry) -> Result<(), DhcpError>;
+    fn remove_static_mapping(&self, mac: &MacAddress) -> Result<(), DhcpError>;
     fn list_static_mappings(&self) -> Vec<(MacAddress, IpAddress)>;
     fn get_ip(&self) -> IpAddress;
     fn get_host(&self) -> LogicalHost;
@@ -78,7 +91,7 @@ pub struct MacAddress(pub [u8; 6]);
 
 impl MacAddress {
     pub fn dummy() -> Self {
-        Self([0,0,0,0,0,0])
+        Self([0, 0, 0, 0, 0, 0])
     }
 }
 
diff --git a/harmony-rs/harmony/src/infra/opnsense/mod.rs b/harmony-rs/harmony/src/infra/opnsense/mod.rs
index 35e2a2e..92f1826 100644
--- a/harmony-rs/harmony/src/infra/opnsense/mod.rs
+++ b/harmony-rs/harmony/src/infra/opnsense/mod.rs
@@ -2,8 +2,8 @@ mod management;
 pub use management::*;
 
 use crate::topology::{
-    Backend, DhcpServer, DnsServer, Firewall, FirewallError, FirewallRule, Frontend, IpAddress,
-    LoadBalancer, LoadBalancerError, LogicalHost,
+    Backend, DHCPStaticEntry, DhcpServer, DnsServer, Firewall, FirewallError, FirewallRule,
+    Frontend, IpAddress, LoadBalancer, LoadBalancerError, LogicalHost,
 };
 use derive_new::new;
 
@@ -34,7 +34,7 @@ impl Firewall for OPNSenseFirewall {
     fn get_ip(&self) -> IpAddress {
         OPNSenseFirewall::get_ip(self)
     }
-    fn get_host(&self) -> LogicalHost{
+    fn get_host(&self) -> LogicalHost {
         self.host.clone()
     }
 }
@@ -67,22 +67,21 @@ impl LoadBalancer for OPNSenseFirewall {
     fn get_ip(&self) -> IpAddress {
         OPNSenseFirewall::get_ip(self)
     }
-    fn get_host(&self) -> LogicalHost{
+    fn get_host(&self) -> LogicalHost {
         self.host.clone()
     }
 }
 
 impl DhcpServer for OPNSenseFirewall {
     fn add_static_mapping(
-        &mut self,
-        _mac: crate::topology::MacAddress,
-        _ip: IpAddress,
+        &self,
+        entry: &DHCPStaticEntry,
     ) -> Result<(), crate::topology::DhcpError> {
-        todo!()
+        todo!("Register {:?}", entry)
     }
 
     fn remove_static_mapping(
-        &mut self,
+        &self,
         _mac: &crate::topology::MacAddress,
     ) -> Result<(), crate::topology::DhcpError> {
         todo!()
@@ -95,7 +94,7 @@ impl DhcpServer for OPNSenseFirewall {
     fn get_ip(&self) -> IpAddress {
         OPNSenseFirewall::get_ip(self)
     }
-    fn get_host(&self) -> LogicalHost{
+    fn get_host(&self) -> LogicalHost {
         self.host.clone()
     }
 }
@@ -125,7 +124,7 @@ impl DnsServer for OPNSenseFirewall {
         OPNSenseFirewall::get_ip(&self)
     }
 
-    fn get_host(&self) -> LogicalHost{
+    fn get_host(&self) -> LogicalHost {
         self.host.clone()
     }
 }
diff --git a/harmony-rs/harmony/src/modules/opnsense_dhcp.rs b/harmony-rs/harmony/src/modules/dhcp.rs
similarity index 82%
rename from harmony-rs/harmony/src/modules/opnsense_dhcp.rs
rename to harmony-rs/harmony/src/modules/dhcp.rs
index 68f9e8b..ec100ea 100644
--- a/harmony-rs/harmony/src/modules/opnsense_dhcp.rs
+++ b/harmony-rs/harmony/src/modules/dhcp.rs
@@ -10,18 +10,11 @@ use crate::{
     infra::executors::russh::RusshClient,
     interpret::{Interpret, InterpretError, InterpretName, Outcome},
     inventory::Inventory,
-    topology::{HAClusterTopology, HostBinding, IpAddress, MacAddress},
+    topology::{DHCPStaticEntry, HAClusterTopology, HostBinding},
 };
 
 use crate::domain::score::Score;
 
-#[derive(Debug)]
-pub struct DHCPStaticEntry {
-    name: String,
-    mac: MacAddress,
-    ip: IpAddress,
-}
-
 /// OPNSenseDhcpScore will set static DHCP entries using index based hostname
 /// and ip addresses.
 ///
@@ -59,30 +52,30 @@ pub struct DHCPStaticEntry {
 /// ]
 /// ```
 #[derive(Debug, new, Clone)]
-pub struct OPNSenseDhcpScore {
+pub struct DhcpScore {
     host_binding: Vec<HostBinding>,
 }
 
-impl Score for OPNSenseDhcpScore {
-    type InterpretType = OPNSenseDhcpInterpret;
+impl Score for DhcpScore {
+    type InterpretType = DhcpInterpret;
 
-    fn create_interpret(self) -> OPNSenseDhcpInterpret {
-        OPNSenseDhcpInterpret::new(self)
+    fn create_interpret(self) -> DhcpInterpret {
+        DhcpInterpret::new(self)
     }
 }
 
 // https://docs.opnsense.org/manual/dhcp.html#advanced-settings
 #[derive(Debug, Clone)]
-pub struct OPNSenseDhcpInterpret {
-    score: OPNSenseDhcpScore,
+pub struct DhcpInterpret {
+    score: DhcpScore,
     version: Version,
     id: Id,
     name: String,
     status: InterpretStatus,
 }
 
-impl OPNSenseDhcpInterpret {
-    pub fn new(score: OPNSenseDhcpScore) -> Self {
+impl DhcpInterpret {
+    pub fn new(score: DhcpScore) -> Self {
         let version = Version::from("1.0.0").expect("Version should be valid");
         let name = "OPNSenseDhcpScore".to_string();
         let id = Id::from_string(format!("{name}_{version}"));
@@ -98,7 +91,7 @@ impl OPNSenseDhcpInterpret {
 }
 
 #[async_trait]
-impl Interpret for OPNSenseDhcpInterpret {
+impl Interpret for DhcpInterpret {
     fn get_name(&self) -> InterpretName {
         InterpretName::OPNSenseDHCP
     }
@@ -134,7 +127,16 @@ impl Interpret for OPNSenseDhcpInterpret {
             })
             .collect();
         info!("DHCPStaticEntry : {:?}", entries);
-        todo!("Filter proper network interfaces and prepare the DHCP configuration");
+
+        let dhcp = topology.dhcp_server.clone();
+        info!("DHCP server : {:?}", dhcp);
+        entries.iter().for_each(|entry| {
+            match dhcp.add_static_mapping(&entry) {
+                Ok(_) => info!("Successfully registered DHCPStaticEntry {}", entry),
+                Err(_) => todo!(),
+            }
+        });
+        todo!("Configure DHCPServer");
 
         Ok(Outcome::new(
             InterpretStatus::SUCCESS,
diff --git a/harmony-rs/harmony/src/modules/mod.rs b/harmony-rs/harmony/src/modules/mod.rs
index f47d64a..5833493 100644
--- a/harmony-rs/harmony/src/modules/mod.rs
+++ b/harmony-rs/harmony/src/modules/mod.rs
@@ -1 +1 @@
-pub mod opnsense_dhcp;
+pub mod dhcp;