Merge pull request 'fix(opnsense-config): mark Interface::enable as optional' (#181) from fix-opnsense-config into master

Reviewed-on: #181

examples/nanodc/src/main.rs

@@ -106,7 +106,6 @@ async fn main() {
                 name: "wk2".to_string(),
             },
         ],
-        node_exporter: opnsense.clone(),
         switch_client: switch_client.clone(),
     };
 

examples/okd_installation/src/topology.rs

@@ -80,7 +80,6 @@ pub async fn get_topology() -> HAClusterTopology {
             name: "bootstrap".to_string(),
         },
         workers: vec![],
-        node_exporter: opnsense.clone(),
         switch_client: switch_client.clone(),
     }
 }

examples/okd_pxe/src/topology.rs

@@ -75,7 +75,6 @@ pub async fn get_topology() -> HAClusterTopology {
             name: "cp0".to_string(),
         },
         workers: vec![],
-        node_exporter: opnsense.clone(),
         switch_client: switch_client.clone(),
     }
 }

examples/opnsense/src/main.rs

@@ -78,7 +78,6 @@ async fn main() {
             name: "cp0".to_string(),
         },
         workers: vec![],
-        node_exporter: opnsense.clone(),
         switch_client: switch_client.clone(),
     };
 

examples/opnsense_node_exporter/Cargo.toml

@@ -1,21 +0,0 @@
-[package]
-name = "example-opnsense-node-exporter"
-edition = "2024"
-version.workspace = true
-readme.workspace = true
-license.workspace = true
-
-[dependencies]
-harmony = { path = "../../harmony" }
-harmony_cli = { path = "../../harmony_cli" }
-harmony_types = { path = "../../harmony_types" }
-harmony_secret = { path = "../../harmony_secret" }
-harmony_secret_derive = { path = "../../harmony_secret_derive" }
-cidr = { workspace = true }
-tokio = { workspace = true }
-harmony_macros = { path = "../../harmony_macros" }
-log = { workspace = true }
-env_logger = { workspace = true }
-url = { workspace = true }
-serde.workspace = true
-async-trait.workspace = true

examples/opnsense_node_exporter/src/main.rs

@@ -1,79 +0,0 @@
-use std::{
-    net::{IpAddr, Ipv4Addr},
-    sync::Arc,
-};
-
-use async_trait::async_trait;
-use cidr::Ipv4Cidr;
-use harmony::{
-    executors::ExecutorError,
-    hardware::{HostCategory, Location, PhysicalHost, SwitchGroup},
-    infra::opnsense::OPNSenseManagementInterface,
-    inventory::Inventory,
-    modules::opnsense::node_exporter::NodeExporterScore,
-    topology::{
-        HAClusterTopology, LogicalHost, PreparationError, PreparationOutcome, Topology,
-        UnmanagedRouter, node_exporter::NodeExporter,
-    },
-};
-use harmony_macros::{ip, ipv4, mac_address};
-
-struct OpnSenseTopology {
-    node_exporter: Arc<dyn NodeExporter>,
-}
-
-#[async_trait]
-impl Topology for OpnSenseTopology {
-    async fn ensure_ready(&self) -> Result<PreparationOutcome, PreparationError> {
-        Ok(PreparationOutcome::Success {
-            details: "Success".to_string(),
-        })
-    }
-    fn name(&self) -> &str {
-        "OpnsenseTopology"
-    }
-}
-
-#[async_trait]
-impl NodeExporter for OpnSenseTopology {
-    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
-        self.node_exporter.ensure_initialized().await
-    }
-
-    async fn commit_config(&self) -> Result<(), ExecutorError> {
-        self.node_exporter.commit_config().await
-    }
-
-    async fn reload_restart(&self) -> Result<(), ExecutorError> {
-        self.node_exporter.reload_restart().await
-    }
-}
-
-#[tokio::main]
-async fn main() {
-    let firewall = harmony::topology::LogicalHost {
-        ip: ip!("192.168.1.1"),
-        name: String::from("fw0"),
-    };
-
-    let opnsense = Arc::new(
-        harmony::infra::opnsense::OPNSenseFirewall::new(firewall, None, "root", "opnsense").await,
-    );
-
-    let topology = OpnSenseTopology {
-        node_exporter: opnsense.clone(),
-    };
-
-    let inventory = Inventory::empty();
-
-    let node_exporter_score = NodeExporterScore {};
-
-    harmony_cli::run(
-        inventory,
-        topology,
-        vec![Box::new(node_exporter_score)],
-        None,
-    )
-    .await
-    .unwrap();
-}

harmony/src/domain/topology/ha_cluster.rs

@@ -8,15 +8,12 @@ use kube::api::ObjectMeta;
 use log::debug;
 use log::info;
 
+use crate::modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy};
 use crate::topology::PxeOptions;
 use crate::{data::FileContent, modules::okd::crd::nmstate::NMState};
 use crate::{
     executors::ExecutorError, modules::okd::crd::nmstate::NodeNetworkConfigurationPolicySpec,
 };
-use crate::{
-    modules::okd::crd::nmstate::{self, NodeNetworkConfigurationPolicy},
-    topology::node_exporter::NodeExporter,
-};
 
 use super::{
     DHCPStaticEntry, DhcpServer, DnsRecord, DnsRecordType, DnsServer, Firewall, HostNetworkConfig,
@@ -38,7 +35,6 @@ pub struct HAClusterTopology {
     pub tftp_server: Arc<dyn TftpServer>,
     pub http_server: Arc<dyn HttpServer>,
     pub dns_server: Arc<dyn DnsServer>,
-    pub node_exporter: Arc<dyn NodeExporter>,
     pub switch_client: Arc<dyn SwitchClient>,
     pub bootstrap_host: LogicalHost,
     pub control_plane: Vec<LogicalHost>,
@@ -301,7 +297,6 @@ impl HAClusterTopology {
             tftp_server: dummy_infra.clone(),
             http_server: dummy_infra.clone(),
             dns_server: dummy_infra.clone(),
-            node_exporter: dummy_infra.clone(),
             switch_client: dummy_infra.clone(),
             bootstrap_host: dummy_host,
             control_plane: vec![],
@@ -479,23 +474,6 @@ impl Switch for HAClusterTopology {
         self.configure_bond(config).await?;
         self.configure_port_channel(config).await
     }
-
-    //TODO add snmp here
-}
-
-#[async_trait]
-impl NodeExporter for HAClusterTopology {
-    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
-        self.node_exporter.ensure_initialized().await
-    }
-
-    async fn commit_config(&self) -> Result<(), ExecutorError> {
-        self.node_exporter.commit_config().await
-    }
-
-    async fn reload_restart(&self) -> Result<(), ExecutorError> {
-        self.node_exporter.reload_restart().await
-    }
 }
 
 #[derive(Debug)]
@@ -685,21 +663,6 @@ impl DnsServer for DummyInfra {
     }
 }
 
-#[async_trait]
-impl NodeExporter for DummyInfra {
-    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
-        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
-    }
-
-    async fn commit_config(&self) -> Result<(), ExecutorError> {
-        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
-    }
-
-    async fn reload_restart(&self) -> Result<(), ExecutorError> {
-        unimplemented!("{}", UNIMPLEMENTED_DUMMY_INFRA)
-    }
-}
-
 #[async_trait]
 impl SwitchClient for DummyInfra {
     async fn setup(&self) -> Result<(), SwitchError> {

harmony/src/domain/topology/mod.rs

@@ -1,6 +1,5 @@
 mod ha_cluster;
 pub mod ingress;
-pub mod node_exporter;
 use harmony_types::net::IpAddress;
 mod host_binding;
 mod http;

harmony/src/domain/topology/node_exporter.rs

@@ -1,17 +0,0 @@
-use async_trait::async_trait;
-
-use crate::executors::ExecutorError;
-
-#[async_trait]
-pub trait NodeExporter: Send + Sync {
-    async fn ensure_initialized(&self) -> Result<(), ExecutorError>;
-    async fn commit_config(&self) -> Result<(), ExecutorError>;
-    async fn reload_restart(&self) -> Result<(), ExecutorError>;
-}
-
-//TODO complete this impl
-impl std::fmt::Debug for dyn NodeExporter {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_fmt(format_args!("NodeExporter ",))
-    }
-}

harmony/src/infra/opnsense/mod.rs

@@ -4,7 +4,6 @@ mod firewall;
 mod http;
 mod load_balancer;
 mod management;
-pub mod node_exporter;
 mod tftp;
 use std::sync::Arc;
 

harmony/src/infra/opnsense/node_exporter.rs

@@ -1,47 +0,0 @@
-use async_trait::async_trait;
-use log::debug;
-
-use crate::{
-    executors::ExecutorError, infra::opnsense::OPNSenseFirewall,
-    topology::node_exporter::NodeExporter,
-};
-
-#[async_trait]
-impl NodeExporter for OPNSenseFirewall {
-    async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
-        let mut config = self.opnsense_config.write().await;
-        let node_exporter = config.node_exporter();
-        if let Some(config) = node_exporter.get_full_config() {
-            debug!(
-                "Node exporter available in opnsense config, assuming it is already installed. {config:?}"
-            );
-        } else {
-            config
-                .install_package("os-node_exporter")
-                .await
-                .map_err(|e| {
-                        ExecutorError::UnexpectedError(format!("Executor failed when trying to install os-node_exporter package with error {e:?}"
-                                ))
-                    })?;
-        }
-
-        config
-            .node_exporter()
-            .enable(true)
-            .map_err(|e| ExecutorError::UnexpectedError(e.to_string()))?;
-        Ok(())
-    }
-    async fn commit_config(&self) -> Result<(), ExecutorError> {
-        OPNSenseFirewall::commit_config(self).await
-    }
-
-    async fn reload_restart(&self) -> Result<(), ExecutorError> {
-        self.opnsense_config
-            .write()
-            .await
-            .node_exporter()
-            .reload_restart()
-            .await
-            .map_err(|e| ExecutorError::UnexpectedError(e.to_string()))
-    }
-}

harmony/src/modules/opnsense/mod.rs

@@ -1,4 +1,3 @@
-pub mod node_exporter;
 mod shell;
 mod upgrade;
 pub use shell::*;

harmony/src/modules/opnsense/node_exporter.rs

@@ -1,70 +0,0 @@
-use async_trait::async_trait;
-use harmony_types::id::Id;
-use log::info;
-use serde::Serialize;
-
-use crate::{
-    data::Version,
-    interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
-    inventory::Inventory,
-    score::Score,
-    topology::{Topology, node_exporter::NodeExporter},
-};
-
-#[derive(Debug, Clone, Serialize)]
-pub struct NodeExporterScore {}
-
-impl<T: Topology + NodeExporter> Score<T> for NodeExporterScore {
-    fn name(&self) -> String {
-        "NodeExporterScore".to_string()
-    }
-
-    fn create_interpret(&self) -> Box<dyn Interpret<T>> {
-        Box::new(NodeExporterInterpret {})
-    }
-}
-
-#[derive(Debug)]
-pub struct NodeExporterInterpret {}
-
-#[async_trait]
-impl<T: Topology + NodeExporter> Interpret<T> for NodeExporterInterpret {
-    async fn execute(
-        &self,
-        _inventory: &Inventory,
-        node_exporter: &T,
-    ) -> Result<Outcome, InterpretError> {
-        info!(
-            "Making sure node exporter is initiailized: {:?}",
-            node_exporter.ensure_initialized().await?
-        );
-
-        info!("Applying Node Exporter configuration");
-
-        node_exporter.commit_config().await?;
-
-        info!("Reloading and restarting Node Exporter");
-
-        node_exporter.reload_restart().await?;
-
-        Ok(Outcome::success(format!(
-            "NodeExporter successfully configured"
-        )))
-    }
-
-    fn get_name(&self) -> InterpretName {
-        InterpretName::Custom("NodeExporter")
-    }
-
-    fn get_version(&self) -> Version {
-        todo!()
-    }
-
-    fn get_status(&self) -> InterpretStatus {
-        todo!()
-    }
-
-    fn get_children(&self) -> Vec<Id> {
-        todo!()
-    }
-}

opnsense-config-xml/src/data/interfaces.rs

@@ -9,7 +9,7 @@ pub struct Interface {
     pub physical_interface_name: String,
     pub descr: Option<MaybeString>,
     pub mtu: Option<MaybeString>,
-    pub enable: MaybeString,
+    pub enable: Option<MaybeString>,
     pub lock: Option<MaybeString>,
     #[yaserde(rename = "spoofmac")]
     pub spoof_mac: Option<MaybeString>,
@@ -134,19 +134,15 @@ mod test {
   <interfaces>
     <paul>
       <if></if>
-      <enable/>
     </paul>
     <anotherpaul>
       <if></if>
-      <enable/>
     </anotherpaul>
     <thirdone>
       <if></if>
-      <enable/>
     </thirdone>
     <andgofor4>
       <if></if>
-      <enable/>
     </andgofor4>
   </interfaces>
   <bar>foo</bar>

opnsense-config-xml/src/data/opnsense.rs

@@ -17,7 +17,7 @@ pub struct OPNsense {
     pub interfaces: NamedList<Interface>,
     pub dhcpd: NamedList<DhcpInterface>,
     pub snmpd: Snmpd,
-    pub syslog: Option<Syslog>,
+    pub syslog: Syslog,
     pub nat: Nat,
     pub filter: Filters,
     pub load_balancer: Option<LoadBalancer>,
@@ -190,7 +190,7 @@ pub struct System {
     pub webgui: WebGui,
     pub usevirtualterminal: u8,
     pub disablenatreflection: Option<String>,
-    pub disableconsolemenu: Option<u8>,
+    pub disableconsolemenu: u8,
     pub disablevlanhwfilter: u8,
     pub disablechecksumoffloading: u8,
     pub disablesegmentationoffloading: u8,
@@ -216,7 +216,7 @@ pub struct System {
     pub maximumfrags: Option<MaybeString>,
     pub aliasesresolveinterval: Option<MaybeString>,
     pub maximumtableentries: Option<MaybeString>,
-    pub language: Option<String>,
+    pub language: String,
     pub dnsserver: Option<MaybeString>,
     pub dns1gw: Option<String>,
     pub dns2gw: Option<String>,
@@ -233,16 +233,16 @@ pub struct System {
 #[derive(Default, PartialEq, Debug, YaSerialize, YaDeserialize)]
 pub struct Ssh {
     pub group: String,
-    pub noauto: Option<u8>,
+    pub noauto: u8,
-    pub interfaces: Option<MaybeString>,
+    pub interfaces: MaybeString,
-    pub kex: Option<MaybeString>,
+    pub kex: MaybeString,
-    pub ciphers: Option<MaybeString>,
+    pub ciphers: MaybeString,
-    pub macs: Option<MaybeString>,
+    pub macs: MaybeString,
-    pub keys: Option<MaybeString>,
+    pub keys: MaybeString,
-    pub enabled: Option<String>,
+    pub enabled: String,
-    pub passwordauth: Option<u8>,
+    pub passwordauth: u8,
-    pub keysig: Option<MaybeString>,
+    pub keysig: MaybeString,
-    pub permitrootlogin: Option<u8>,
+    pub permitrootlogin: u8,
     pub rekeylimit: Option<MaybeString>,
 }
 
@@ -306,11 +306,11 @@ pub struct WebGui {
     pub protocol: String,
     #[yaserde(rename = "ssl-certref")]
     pub ssl_certref: String,
-    pub port: Option<MaybeString>,
+    pub port: MaybeString,
     #[yaserde(rename = "ssl-ciphers")]
-    pub ssl_ciphers: Option<MaybeString>,
+    pub ssl_ciphers: MaybeString,
-    pub interfaces: Option<MaybeString>,
+    pub interfaces: MaybeString,
-    pub compression: Option<MaybeString>,
+    pub compression: MaybeString,
     pub nohttpreferercheck: Option<u8>,
 }
 
@@ -433,7 +433,7 @@ pub struct OPNsenseXmlSection {
     #[yaserde(rename = "Interfaces")]
     pub interfaces: Option<ConfigInterfaces>,
     #[yaserde(rename = "NodeExporter")]
-    pub node_exporter: Option<NodeExporter>,
+    pub node_exporter: Option<RawXml>,
     #[yaserde(rename = "Kea")]
     pub kea: Option<RawXml>,
     pub monit: Option<Monit>,
@@ -1595,21 +1595,3 @@ pub struct Ifgroups {
     #[yaserde(attribute = true)]
     pub version: String,
 }
-
-#[derive(Default, PartialEq, Debug, YaSerialize, YaDeserialize)]
-pub struct NodeExporter {
-    pub enabled: u8,
-    pub listenaddress: Option<MaybeString>,
-    pub listenport: u16,
-    pub cpu: u8,
-    pub exec: u8,
-    pub filesystem: u8,
-    pub loadavg: u8,
-    pub meminfo: u8,
-    pub netdev: u8,
-    pub time: u8,
-    pub devstat: u8,
-    pub interrupts: u8,
-    pub ntp: u8,
-    pub zfs: u8,
-}

opnsense-config/src/config/config.rs

@@ -5,8 +5,7 @@ use crate::{
     error::Error,
     modules::{
         caddy::CaddyConfig, dhcp_legacy::DhcpConfigLegacyISC, dns::UnboundDnsConfig,
-        dnsmasq::DhcpConfigDnsMasq, load_balancer::LoadBalancerConfig,
+        dnsmasq::DhcpConfigDnsMasq, load_balancer::LoadBalancerConfig, tftp::TftpConfig,
-        node_exporter::NodeExporterConfig, tftp::TftpConfig,
     },
 };
 use log::{debug, info, trace, warn};
@@ -14,7 +13,6 @@ use opnsense_config_xml::OPNsense;
 use russh::client;
 use serde::Serialize;
 use sha2::Digest;
-use tokio::time::{sleep, Duration};
 
 use super::{ConfigManager, OPNsenseShell};
 
@@ -73,10 +71,6 @@ impl Config {
         LoadBalancerConfig::new(&mut self.opnsense, self.shell.clone())
     }
 
-    pub fn node_exporter(&mut self) -> NodeExporterConfig<'_> {
-        NodeExporterConfig::new(&mut self.opnsense, self.shell.clone())
-    }
-
     pub async fn upload_files(&self, source: &str, destination: &str) -> Result<String, Error> {
         self.shell.upload_folder(source, destination).await
     }
@@ -156,8 +150,7 @@ impl Config {
 
     async fn reload_config(&mut self) -> Result<(), Error> {
         info!("Reloading opnsense live config");
-        let (opnsense, _sha2) = Self::get_opnsense_instance(self.repository.clone()).await?;
+        let (opnsense, sha2) = Self::get_opnsense_instance(self.repository.clone()).await?;
-        self.opnsense = opnsense;
         Ok(())
     }
 

opnsense-config/src/modules/mod.rs

@@ -4,5 +4,4 @@ pub mod dhcp_legacy;
 pub mod dns;
 pub mod dnsmasq;
 pub mod load_balancer;
-pub mod node_exporter;
 pub mod tftp;

opnsense-config/src/modules/node_exporter.rs

@@ -1,54 +0,0 @@
-use std::sync::Arc;
-
-use opnsense_config_xml::{NodeExporter, OPNsense};
-
-use crate::{config::OPNsenseShell, Error};
-
-pub struct NodeExporterConfig<'a> {
-    opnsense: &'a mut OPNsense,
-    opnsense_shell: Arc<dyn OPNsenseShell>,
-}
-
-impl<'a> NodeExporterConfig<'a> {
-    pub fn new(opnsense: &'a mut OPNsense, opnsense_shell: Arc<dyn OPNsenseShell>) -> Self {
-        Self {
-            opnsense,
-            opnsense_shell,
-        }
-    }
-
-    pub fn get_full_config(&self) -> &Option<NodeExporter> {
-        &self.opnsense.opnsense.node_exporter
-    }
-
-    fn with_node_exporter<F, R>(&mut self, f: F) -> Result<R, &'static str>
-    where
-        F: FnOnce(&mut NodeExporter) -> R,
-    {
-        match &mut self.opnsense.opnsense.node_exporter.as_mut() {
-            Some(node_exporter) => Ok(f(node_exporter)),
-            None => Err("node exporter is not yet installed"),
-        }
-    }
-
-    pub fn enable(&mut self, enabled: bool) -> Result<(), &'static str> {
-        self.with_node_exporter(|node_exporter| node_exporter.enabled = enabled as u8)
-            .map(|_| ())
-    }
-
-    pub async fn reload_restart(&self) -> Result<(), Error> {
-        self.opnsense_shell
-            .exec("configctl node_exporter stop")
-            .await?;
-        self.opnsense_shell
-            .exec("configctl template reload OPNsense/NodeExporter")
-            .await?;
-        self.opnsense_shell
-            .exec("configctl node_exporter configtest")
-            .await?;
-        self.opnsense_shell
-            .exec("configctl node_exporter start")
-            .await?;
-        Ok(())
-    }
-}