fix(opnsense): valid HAProxy config + From<&str> codegen cleanup #273

Merged
stremblay merged 7 commits from fix/haproxy-issues into master 2026-04-22 17:01:54 +00:00
4 changed files with 3469 additions and 50 deletions

View File

@@ -53,7 +53,12 @@ impl LoadBalancer for OPNSenseFirewall {
async fn ensure_initialized(&self) -> Result<(), ExecutorError> { async fn ensure_initialized(&self) -> Result<(), ExecutorError> {
let lb = self.opnsense_config.load_balancer(); let lb = self.opnsense_config.load_balancer();
if lb.is_installed().await { let installed = lb.is_installed().await.map_err(|e| {
ExecutorError::UnexpectedError(format!(
"Failed to query HAProxy installation status on OPNsense: {e}"
))
})?;
if installed {
debug!("HAProxy is installed"); debug!("HAProxy is installed");
} else { } else {
self.opnsense_config self.opnsense_config
@@ -141,7 +146,7 @@ fn haproxy_service_to_harmony(svc: &HaproxyService) -> Option<LoadBalancerServic
let method: HttpMethod = hc.http_method.clone().unwrap_or_default().into(); let method: HttpMethod = hc.http_method.clone().unwrap_or_default().into();
let ssl = match hc.ssl.as_deref().unwrap_or("").to_uppercase().as_str() { let ssl = match hc.ssl.as_deref().unwrap_or("").to_uppercase().as_str() {
"SSL" => SSL::SSL, "SSL" => SSL::SSL,
"SSLNI" => SSL::SNI, "SSLSNI" => SSL::SNI,
"NOSSL" => SSL::Disabled, "NOSSL" => SSL::Disabled,
"" => SSL::Default, "" => SSL::Default,
other => { other => {
@@ -177,7 +182,7 @@ pub(crate) fn harmony_service_to_lb_types(
HealthCheck::HTTP(port, path, http_method, _status_code, ssl) => { HealthCheck::HTTP(port, path, http_method, _status_code, ssl) => {
let ssl_str = match ssl { let ssl_str = match ssl {
SSL::SSL => Some("ssl".to_string()), SSL::SSL => Some("ssl".to_string()),
SSL::SNI => Some("sslni".to_string()), SSL::SNI => Some("sslsni".to_string()),
SSL::Disabled => Some("nossl".to_string()), SSL::Disabled => Some("nossl".to_string()),
SSL::Default => Some(String::new()), SSL::Default => Some(String::new()),
SSL::Other(other) => Some(other.clone()), SSL::Other(other) => Some(other.clone()),

File diff suppressed because it is too large Load Diff

View File

@@ -924,6 +924,34 @@ impl CodeGenerator {
writeln!(self.output, "}}")?; writeln!(self.output, "}}")?;
writeln!(self.output)?; writeln!(self.output)?;
// Infallible conversions from user-facing strings. Matches wire values
// case-insensitively; unknown inputs are preserved via the `Other`
// variant so round-trip fidelity is kept and the mapping between
// string input and enum variant lives on the type, not the caller.
writeln!(self.output, "impl From<&str> for {} {{", enum_ir.name)?;
writeln!(self.output, " fn from(s: &str) -> Self {{")?;
writeln!(self.output, " match s.to_lowercase().as_str() {{")?;
for variant in &enum_ir.variants {
writeln!(
self.output,
" \"{}\" => Self::{},",
variant.wire_value.to_lowercase(),
variant.rust_name
)?;
}
writeln!(self.output, " _ => Self::Other(s.to_string()),")?;
writeln!(self.output, " }}")?;
writeln!(self.output, " }}")?;
writeln!(self.output, "}}")?;
writeln!(self.output)?;
writeln!(self.output, "impl From<String> for {} {{", enum_ir.name)?;
writeln!(self.output, " fn from(s: String) -> Self {{")?;
writeln!(self.output, " Self::from(s.as_str())")?;
writeln!(self.output, " }}")?;
writeln!(self.output, "}}")?;
writeln!(self.output)?;
Ok(()) Ok(())
} }

View File

@@ -2,9 +2,9 @@ use crate::Error;
use log::{debug, info}; use log::{debug, info};
use opnsense_api::generated::haproxy::{ use opnsense_api::generated::haproxy::{
BackendAlgorithm, BackendMode, BackendPersistenceCookiemode, FrontendConnectionBehaviour, BackendAlgorithm, BackendMode, BackendPersistenceCookiemode, FrontendConnectionBehaviour,
FrontendMode, HealthcheckType, OpNsenseHaProxyBackendsBackend, FrontendMode, HealthcheckHttpMethod, HealthcheckSsl, HealthcheckType,
OpNsenseHaProxyFrontendsFrontend, OpNsenseHaProxyHealthchecksHealthcheck, OpNsenseHaProxyBackendsBackend, OpNsenseHaProxyFrontendsFrontend,
OpNsenseHaProxyServersServer, ServerMode, ServerType, OpNsenseHaProxyHealthchecksHealthcheck, OpNsenseHaProxyServersServer, ServerMode, ServerType,
}; };
use opnsense_api::response::StatusResponse; use opnsense_api::response::StatusResponse;
use opnsense_api::OpnsenseClient; use opnsense_api::OpnsenseClient;
@@ -91,11 +91,23 @@ impl LoadBalancerConfig {
} }
/// Check if the HAProxy plugin is installed. /// Check if the HAProxy plugin is installed.
pub async fn is_installed(&self) -> bool { ///
self.client /// Returns `Ok(true)` if the settings endpoint responds successfully,
/// `Ok(false)` only when OPNsense replies HTTP 404 (controller not found —
/// the canonical signal for a missing plugin). Every other error — transport
/// failure, auth rejection, server 5xx, decode failure — is propagated so
/// the caller does not mistake an unreachable firewall for an uninstalled
/// plugin and trigger an install attempt.
pub async fn is_installed(&self) -> Result<bool, opnsense_api::Error> {
match self
.client
.get_typed::<serde_json::Value>("haproxy", "settings", "get") .get_typed::<serde_json::Value>("haproxy", "settings", "get")
.await .await
.is_ok() {
Ok(_) => Ok(true),
Err(opnsense_api::Error::Api { status, .. }) if status.as_u16() == 404 => Ok(false),
Err(e) => Err(e),
}
} }
/// Enable or disable HAProxy. /// Enable or disable HAProxy.
@@ -155,18 +167,13 @@ impl LoadBalancerConfig {
let hc_uuid = if let Some(hc) = &healthcheck { let hc_uuid = if let Some(hc) = &healthcheck {
let hc_struct = OpNsenseHaProxyHealthchecksHealthcheck { let hc_struct = OpNsenseHaProxyHealthchecksHealthcheck {
name: hc.name.clone(), name: hc.name.clone(),
r#type: Some(match hc.check_type.to_lowercase().as_str() { r#type: Some(HealthcheckType::from(hc.check_type.as_str())),
"tcp" => HealthcheckType::Tcp,
"http" => HealthcheckType::HttpDefault,
"agent" => HealthcheckType::Agent,
"mysql" => HealthcheckType::MySql,
"pgsql" | "postgresql" => HealthcheckType::PostgreSql,
"smtp" => HealthcheckType::Smtp,
"ssl" => HealthcheckType::Ssl,
other => HealthcheckType::Other(other.to_string()),
}),
interval: hc.interval.clone(), interval: hc.interval.clone(),
http_uri: hc.http_uri.clone(), http_uri: hc.http_uri.clone(),
http_method: hc.http_method.as_deref().map(HealthcheckHttpMethod::from),
http_version: None,
http_host: Some(String::new()),
ssl: hc.ssl.as_deref().map(HealthcheckSsl::from),
checkport: hc.checkport.as_deref().and_then(|p| p.parse().ok()), checkport: hc.checkport.as_deref().and_then(|p| p.parse().ok()),
..Default::default() ..Default::default()
}; };
@@ -199,17 +206,10 @@ impl LoadBalancerConfig {
address: Some(s.address.clone()), address: Some(s.address.clone()),
port: Some(s.port), port: Some(s.port),
enabled: s.enabled, enabled: s.enabled,
mode: Some(match s.mode.to_lowercase().as_str() { mode: Some(ServerMode::from(s.mode.as_str())),
"active" => ServerMode::ActiveDefault, r#type: Some(ServerType::from(s.server_type.as_str())),
"backup" => ServerMode::Backup, ssl: false,
"disabled" => ServerMode::Disabled, sslVerify: false,
other => ServerMode::Other(other.to_string()),
}),
r#type: Some(match s.server_type.to_lowercase().as_str() {
"static" => ServerType::Static,
"template" => ServerType::Template,
other => ServerType::Other(other.to_string()),
}),
..Default::default() ..Default::default()
}; };
#[derive(serde::Serialize)] #[derive(serde::Serialize)]
@@ -235,20 +235,8 @@ impl LoadBalancerConfig {
let be_struct = OpNsenseHaProxyBackendsBackend { let be_struct = OpNsenseHaProxyBackendsBackend {
name: backend.name.clone(), name: backend.name.clone(),
enabled: backend.enabled, enabled: backend.enabled,
mode: Some(match backend.mode.to_lowercase().as_str() { mode: Some(BackendMode::from(backend.mode.as_str())),
Review

MUCH better but still dumb. Why do we have to do mode from mode.as_str() ???? Two different types? There should be only one type, I'd rather pull the type from opnsense-api than have two types for no reason. If there is a major reason then implement a type1::from(type2) so we are type safe, just boilerplate, but safe boilerplate is better than passing strings around.

MUCH better but still dumb. Why do we have to do mode from mode.as_str() ???? Two different types? There should be only one type, I'd rather pull the type from opnsense-api than have two types for no reason. If there is a major reason then implement a type1::from(type2) so we are type safe, just boilerplate, but safe boilerplate is better than passing strings around.
"tcp" => BackendMode::TcpLayer4, algorithm: Some(BackendAlgorithm::from(backend.algorithm.as_str())),
"http" => BackendMode::HttpLayer7Default,
other => BackendMode::Other(other.to_string()),
}),
algorithm: Some(match backend.algorithm.to_lowercase().as_str() {
"roundrobin" => BackendAlgorithm::RoundRobin,
"source" => BackendAlgorithm::SourceIpHashDefault,
"leastconn" => BackendAlgorithm::LeastConnections,
"random" => BackendAlgorithm::RandomAlgorithm,
"static-rr" => BackendAlgorithm::StaticRoundRobin,
"uri" => BackendAlgorithm::UriHashOnlyHttpMode,
other => BackendAlgorithm::Other(other.to_string()),
}),
persistence_cookiemode: Some(BackendPersistenceCookiemode::PiggybackOnExistingCookie), persistence_cookiemode: Some(BackendPersistenceCookiemode::PiggybackOnExistingCookie),
healthCheckEnabled: backend.health_check_enabled, healthCheckEnabled: backend.health_check_enabled,
healthCheck: hc_uuid.clone(), healthCheck: hc_uuid.clone(),
@@ -330,12 +318,7 @@ impl LoadBalancerConfig {
name: frontend.name.clone(), name: frontend.name.clone(),
bind: Some(vec![frontend.bind.clone()]), bind: Some(vec![frontend.bind.clone()]),
enabled: frontend.enabled, enabled: frontend.enabled,
mode: Some(match frontend.mode.to_lowercase().as_str() { mode: Some(FrontendMode::from(frontend.mode.as_str())),
"tcp" => FrontendMode::Tcp,
"http" => FrontendMode::HttpHttpsSslOffloadingDefault,
"ssl" => FrontendMode::SslHttpsTcpMode,
other => FrontendMode::Other(other.to_string()),
}),
connectionBehaviour: Some(FrontendConnectionBehaviour::HttpKeepAliveDefault), connectionBehaviour: Some(FrontendConnectionBehaviour::HttpKeepAliveDefault),
defaultBackend: Some(backend_uuid), defaultBackend: Some(backend_uuid),
stickiness_expire: frontend stickiness_expire: frontend