fix(firewall): correct PVE API field names for rule listing and creation
Some checks failed
Test / frontend-tests (pull_request) Successful in 1m40s
Test / frontend-typecheck (pull_request) Successful in 1m53s
PR Review Automation / review (pull_request) Successful in 6m27s
Test / rust-fmt-check (pull_request) Failing after 12m7s
Test / rust-clippy (pull_request) Successful in 13m31s
Test / rust-tests (pull_request) Successful in 15m12s

PVE firewall rules use pos (not rule_num), proto (not protocol), and
enable as 0/1 integer (not enabled bool). Fix list_firewall_rules and
add_rule in firewall.rs to use correct field names. Normalize Tauri
command response to include rule/status fields that FirewallRuleList
expects. add_rule now sends required type: in field and only includes
optional fields when non-empty.
This commit is contained in:
Shaun Arman 2026-06-21 15:28:24 -05:00
parent 37f729923b
commit 671ee51626
2 changed files with 88 additions and 50 deletions

View File

@ -1423,10 +1423,23 @@ pub async fn list_firewall_rules(
.await .await
.map_err(|e| format!("Failed to list firewall rules: {}", e))?; .map_err(|e| format!("Failed to list firewall rules: {}", e))?;
// Normalize to match what FirewallRuleList component expects:
// rule (position number), action, protocol, source, destination, port, status
let json_rules: Vec<serde_json::Value> = rules let json_rules: Vec<serde_json::Value> = rules
.into_iter() .into_iter()
.map(|r| serde_json::to_value(r).map_err(|e| format!("Failed to serialize rule: {}", e))) .map(|r| {
.collect::<Result<Vec<_>, _>>()?; serde_json::json!({
"id": r.rule_num.to_string(),
"rule": r.rule_num,
"action": r.action,
"protocol": r.protocol,
"source": r.source,
"destination": r.destination,
"port": r.port,
"status": if r.enabled { "enabled" } else { "disabled" },
})
})
.collect();
Ok(json_rules) Ok(json_rules)
} }

View File

@ -23,7 +23,8 @@ pub struct FirewallStatus {
pub rule_count: u32, pub rule_count: u32,
} }
/// List firewall rules /// List firewall rules — returns normalized Vec<FirewallRule> using correct PVE field names.
/// PVE uses: pos (position), proto (protocol), enable (0/1 integer), src (source), dest (destination).
pub async fn list_firewall_rules( pub async fn list_firewall_rules(
client: &crate::proxmox::client::ProxmoxClient, client: &crate::proxmox::client::ProxmoxClient,
node: &str, node: &str,
@ -35,23 +36,32 @@ pub async fn list_firewall_rules(
.await .await
.map_err(|e| format!("Failed to list firewall rules: {}", e))?; .map_err(|e| format!("Failed to list firewall rules: {}", e))?;
if let Some(rules) = response.as_array() { let rules = response.as_array().ok_or("Invalid response format")?;
let rule_list: Vec<FirewallRule> = rules let rule_list: Vec<FirewallRule> = rules
.iter() .iter()
.filter_map(|rule| { .filter_map(|rule| {
let rule_num = rule.get("rule_num")?.as_u64()? as u32; // PVE uses "pos" for the rule position number
let action = rule.get("action")?.as_str()?.to_string(); let rule_num = rule.get("pos").and_then(|v| v.as_u64()).unwrap_or(0) as u32;
let protocol = rule.get("protocol")?.as_str().unwrap_or("").to_string(); let action = rule.get("action").and_then(|v| v.as_str())?.to_string();
let source = rule.get("source")?.as_str().unwrap_or("").to_string(); // PVE uses "proto" not "protocol"
let destination = rule.get("dest")?.as_str().unwrap_or("").to_string(); let protocol = rule.get("proto").and_then(|v| v.as_str()).unwrap_or("").to_string();
// source and dest are optional fields
let source = rule.get("source").and_then(|v| v.as_str()).unwrap_or("").to_string();
let destination = rule.get("dest").and_then(|v| v.as_str()).unwrap_or("").to_string();
let port = rule let port = rule
.get("dport") .get("dport")
.or(rule.get("sport")) .or_else(|| rule.get("sport"))
.and_then(|p| p.as_str()) .and_then(|p| p.as_str())
.map(|s| s.to_string()); .map(|s| s.to_string());
// PVE uses "enable" as integer (1=enabled, 0=disabled), not "enabled" bool
let enabled = rule let enabled = rule
.get("enabled") .get("enable")
.and_then(|e| e.as_bool()) .and_then(|e| {
e.as_i64()
.map(|n| n != 0)
.or_else(|| e.as_bool())
})
.unwrap_or(true); .unwrap_or(true);
Some(FirewallRule { Some(FirewallRule {
@ -67,12 +77,9 @@ pub async fn list_firewall_rules(
.collect(); .collect();
Ok(rule_list) Ok(rule_list)
} else {
Err("Invalid response format".to_string())
}
} }
/// Add firewall rule /// Add firewall rule — uses correct PVE API field names (proto, enable, dest).
pub async fn add_rule( pub async fn add_rule(
client: &crate::proxmox::client::ProxmoxClient, client: &crate::proxmox::client::ProxmoxClient,
node: &str, node: &str,
@ -80,15 +87,29 @@ pub async fn add_rule(
ticket: &str, ticket: &str,
) -> Result<(), String> { ) -> Result<(), String> {
let path = format!("nodes/{}/firewall/rules", node); let path = format!("nodes/{}/firewall/rules", node);
let config = serde_json::json!({
let mut config = serde_json::json!({
"action": rule.action, "action": rule.action,
"protocol": rule.protocol, "type": "in",
"source": rule.source, "enable": if rule.enabled { 1 } else { 0 }
"dest": rule.destination,
"dport": rule.port,
"enabled": rule.enabled
}); });
// Only include optional fields when non-empty
if !rule.protocol.is_empty() {
config["proto"] = serde_json::Value::String(rule.protocol.clone());
}
if !rule.source.is_empty() {
config["source"] = serde_json::Value::String(rule.source.clone());
}
if !rule.destination.is_empty() {
config["dest"] = serde_json::Value::String(rule.destination.clone());
}
if let Some(ref port) = rule.port {
if !port.is_empty() {
config["dport"] = serde_json::Value::String(port.clone());
}
}
let _response: serde_json::Value = client let _response: serde_json::Value = client
.post(&path, &config, Some(ticket)) .post(&path, &config, Some(ticket))
.await .await
@ -200,19 +221,23 @@ pub async fn get_firewall_status(
.unwrap_or(&Vec::new()) .unwrap_or(&Vec::new())
.iter() .iter()
.filter_map(|rule| { .filter_map(|rule| {
let rule_num = rule.get("rule_num")?.as_u64()? as u32; let rule_num = rule.get("pos").and_then(|v| v.as_u64()).unwrap_or(0) as u32;
let action = rule.get("action")?.as_str()?.to_string(); let action = rule.get("action").and_then(|v| v.as_str())?.to_string();
let protocol = rule.get("protocol")?.as_str().unwrap_or("").to_string(); let protocol = rule.get("proto").and_then(|v| v.as_str()).unwrap_or("").to_string();
let source = rule.get("source")?.as_str().unwrap_or("").to_string(); let source = rule.get("source").and_then(|v| v.as_str()).unwrap_or("").to_string();
let destination = rule.get("dest")?.as_str().unwrap_or("").to_string(); let destination = rule.get("dest").and_then(|v| v.as_str()).unwrap_or("").to_string();
let port = rule let port = rule
.get("dport") .get("dport")
.or(rule.get("sport")) .or_else(|| rule.get("sport"))
.and_then(|p| p.as_str()) .and_then(|p| p.as_str())
.map(|s| s.to_string()); .map(|s| s.to_string());
let enabled = rule let enabled = rule
.get("enabled") .get("enable")
.and_then(|e| e.as_bool()) .and_then(|e| {
e.as_i64()
.map(|n| n != 0)
.or_else(|| e.as_bool())
})
.unwrap_or(true); .unwrap_or(true);
Some(FirewallRule { Some(FirewallRule {