From 0af8e7e6a8ae1c5304965afb38d149a91cff495b Mon Sep 17 00:00:00 2001 From: jeangab Date: Wed, 8 Jan 2025 16:30:56 -0500 Subject: [PATCH] fix(load-balancer): implement missing HAProxy reload and sanitize output handling Implement the `reload_restart` method in `LoadBalancerConfig` to ensure proper HAProxy configuration management. Additionally, enhance SSH command execution by sanitizing and logging outputs effectively. This ensures robust handling of HAProxy configurations and improves debugging capabilities through trace-level logs. --- .../src/domain/topology/load_balancer.rs | 1 + .../harmony/src/domain/topology/network.rs | 8 ++-- .../src/infra/opnsense/load_balancer.rs | 41 ++++++++++++------- .../harmony/src/modules/load_balancer.rs | 7 +++- .../opnsense-config-xml/src/data/opnsense.rs | 5 +++ .../opnsense-config/src/config/shell/ssh.rs | 6 ++- .../src/modules/load_balancer.rs | 11 ++++- 7 files changed, 56 insertions(+), 23 deletions(-) diff --git a/harmony-rs/harmony/src/domain/topology/load_balancer.rs b/harmony-rs/harmony/src/domain/topology/load_balancer.rs index 7c85529..4d4afda 100644 --- a/harmony-rs/harmony/src/domain/topology/load_balancer.rs +++ b/harmony-rs/harmony/src/domain/topology/load_balancer.rs @@ -15,6 +15,7 @@ pub trait LoadBalancer: Send + Sync { async fn list_services(&self) -> Vec; async fn ensure_initialized(&self) -> Result<(), ExecutorError>; async fn commit_config(&self) -> Result<(), ExecutorError>; + async fn reload_restart(&self) -> Result<(), ExecutorError>; async fn ensure_service_exists( &self, service: &LoadBalancerService, diff --git a/harmony-rs/harmony/src/domain/topology/network.rs b/harmony-rs/harmony/src/domain/topology/network.rs index 9c78fb0..eaedfee 100644 --- a/harmony-rs/harmony/src/domain/topology/network.rs +++ b/harmony-rs/harmony/src/domain/topology/network.rs @@ -120,8 +120,6 @@ pub enum Action { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct MacAddress(pub [u8; 6]); -// TODO create a small macro to provide a nice API to initiate a MacAddress -// MacAddress::from!("00:90:7f:df:2c:23"), impl MacAddress { #[cfg(test)] @@ -133,7 +131,7 @@ impl MacAddress { impl From<&MacAddress> for String { fn from(value: &MacAddress) -> Self { format!( - "{}:{}:{}:{}:{}:{}", + "{:02x}:{:02x}:{:02x}:{:02x}:{:02x}:{:02x}", value.0[0], value.0[1], value.0[2], value.0[3], value.0[4], value.0[5] ) } @@ -142,8 +140,8 @@ impl From<&MacAddress> for String { 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] + "MacAddress {}", + String::from(self) )) } } diff --git a/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs b/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs index 3480657..a314c1c 100644 --- a/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs +++ b/harmony-rs/harmony/src/infra/opnsense/load_balancer.rs @@ -3,9 +3,13 @@ use log::{debug, info, warn}; use opnsense_config_xml::{Frontend, HAProxy, HAProxyBackend, HAProxyHealthCheck, HAProxyServer}; use uuid::Uuid; -use crate::{executors::ExecutorError, topology::{ - BackendServer, HealthCheck, HttpMethod, HttpStatusCode, IpAddress, LoadBalancer, LoadBalancerService, LogicalHost -}}; +use crate::{ + executors::ExecutorError, + topology::{ + BackendServer, HealthCheck, HttpMethod, HttpStatusCode, IpAddress, LoadBalancer, + LoadBalancerService, LogicalHost, + }, +}; use super::OPNSenseFirewall; @@ -38,23 +42,33 @@ impl LoadBalancer for OPNSenseFirewall { } async fn commit_config(&self) -> Result<(), ExecutorError> { - OPNSenseFirewall::commit_config(self).await?; - todo!("Make sure load balancer is reloaded properly") + OPNSenseFirewall::commit_config(self).await + } + + async fn reload_restart(&self) -> Result<(), ExecutorError> { + self.opnsense_config + .write() + .await + .load_balancer() + .reload_restart() + .await + .map_err(|e| ExecutorError::UnexpectedError(e.to_string())) } async fn ensure_initialized(&self) -> Result<(), ExecutorError> { let mut config = self.opnsense_config.write().await; let load_balancer = config.load_balancer(); - if let Some(_) = load_balancer.get_full_config() { - debug!("HAProxy config available in opnsense config, assuming it is already installed"); - return Ok(()); + if let Some(config) = load_balancer.get_full_config() { + debug!("HAProxy config available in opnsense config, assuming it is already installed, {config:?}"); + } else { + config.install_package("os-haproxy").await.map_err(|e| { + ExecutorError::UnexpectedError(format!( + "Executor failed when trying to install os-haproxy package with error {e:?}" + )) + })?; + todo!() } - config.install_package("os-haproxy").await.map_err(|e| { - ExecutorError::UnexpectedError(format!( - "Executor failed when trying to install os-haproxy package with error {e:?}" - )) - })?; config.load_balancer().enable(true); Ok(()) } @@ -67,7 +81,6 @@ impl LoadBalancer for OPNSenseFirewall { } } - pub(crate) fn haproxy_xml_config_to_harmony_loadbalancer( haproxy: &Option, ) -> Vec { diff --git a/harmony-rs/harmony/src/modules/load_balancer.rs b/harmony-rs/harmony/src/modules/load_balancer.rs index 7e0f19d..0836930 100644 --- a/harmony-rs/harmony/src/modules/load_balancer.rs +++ b/harmony-rs/harmony/src/modules/load_balancer.rs @@ -64,7 +64,12 @@ impl Interpret for LoadBalancerInterpret { info!("Applying load balancer configuration"); topology.load_balancer.commit_config().await?; - todo!() + info!("Making a full reload and restart of haproxy"); + topology.load_balancer.reload_restart().await?; + Ok(Outcome::success(format!( + "Load balancer successfully configured {} services", + self.score.public_services.len() + self.score.private_services.len() + ))) } fn get_name(&self) -> InterpretName { InterpretName::LoadBalancer diff --git a/harmony-rs/opnsense-config-xml/src/data/opnsense.rs b/harmony-rs/opnsense-config-xml/src/data/opnsense.rs index e13bf13..36a85f4 100644 --- a/harmony-rs/opnsense-config-xml/src/data/opnsense.rs +++ b/harmony-rs/opnsense-config-xml/src/data/opnsense.rs @@ -110,6 +110,7 @@ pub struct Filters { #[yaserde(rename = "rule")] pub rules: Vec, pub bypassstaticroutes: Option, + pub scrub: Option, } #[derive(Default, PartialEq, Debug, YaSerialize, YaDeserialize)] @@ -338,6 +339,10 @@ pub struct Snmpd { pub struct Syslog { pub reverse: Option, pub preservelogs: Option, + pub nologdefaultblock: Option, + pub nologdefaultpass: Option, + pub nologbogons: Option, + pub nologprivatenets: Option, } #[derive(Default, PartialEq, Debug, YaSerialize, YaDeserialize)] diff --git a/harmony-rs/opnsense-config/src/config/shell/ssh.rs b/harmony-rs/opnsense-config/src/config/shell/ssh.rs index 159ff7c..dd9ed12 100644 --- a/harmony-rs/opnsense-config/src/config/shell/ssh.rs +++ b/harmony-rs/opnsense-config/src/config/shell/ssh.rs @@ -6,7 +6,7 @@ use std::{ use tokio_stream::StreamExt; use async_trait::async_trait; -use log::{debug, info}; +use log::{debug, info, trace}; use russh::{ client::{Config, Handler, Msg}, Channel, @@ -205,5 +205,7 @@ async fn wait_for_completion(channel: &mut Channel) -> Result { opnsense: &'a mut OPNsense, @@ -56,4 +56,13 @@ impl<'a> LoadBalancerConfig<'a> { pub fn add_servers(&mut self, mut servers: Vec) { self.with_haproxy(|haproxy| haproxy.servers.servers.append(&mut servers)); } + + pub async fn reload_restart(&self) -> Result<(), Error> { + self.opnsense_shell.exec("configctl haproxy stop").await?; + self.opnsense_shell.exec("configctl template reload OPNsense/HAProxy").await?; + self.opnsense_shell.exec("configctl template reload OPNsense/Syslog").await?; + self.opnsense_shell.exec("configctl haproxy configtest").await?; + self.opnsense_shell.exec("configctl haproxy start").await?; + Ok(()) + } }