Re: [pdm-devel] [PATCH proxmox-datacenter-manager v2 2/4] api: firewall: add option, rules and status endpoints

[Hannes Laimer <h.laimer@proxmox.com>]

On 11/6/25 17:58, Michael Köppl wrote:
> 1 comment inline
> 
> On Wed Nov 5, 2025 at 5:35 PM CET, Hannes Laimer wrote:
>> +#[derive(Clone, Debug)]
>> +struct ClusterFirewallData {
>> +    status: Option<FirewallStatus>,
>> +    guests: Vec<ClusterResource>,
>> +}
>> +
>> +async fn fetch_cluster_firewall_data(
>> +    _context: (),
>> +    remote: Remote,
>> +    _node: String, // unused for cluster-level data
>> +) -> Result<ClusterFirewallData, Error> {
>> +    let pve = crate::connection::make_pve_client(&remote)?;
>> +
>> +    let guests = match pve.cluster_resources(Some(ClusterResourceKind::Vm)).await {
>> +        Ok(guests) => guests,
>> +        Err(_) => {
>> +            return Ok(ClusterFirewallData {
>> +                status: None,
>> +                guests: vec![],
>> +            });
>> +        }
>> +    };
>> +
>> +    let options_response = pve.cluster_firewall_options();
>> +    let rules_response = pve.list_cluster_firewall_rules();
>> +
>> +    let enabled = options_response.await.map(|opts| opts.enable != Some(0));
> 
> Shouldn't this be opts.enable == Some(1), as otherwise the firewall will
> also be shown as enabled for a newly-created cluster where the
> /etc/pve/firewall/cluster.fw file has not been created yet (i.e. the
> firewall is shown as off in the PVE web UI).
> 

good catch! should be something like
```
... |opts| opts.enable.map_or(false, |e| e != 0));
```
basically Some(0) and None, is disabled

>> +    let rules = rules_response.await.map(|rules| {
>> +        let all = rules.len();
>> +        let active = rules.iter().filter(|r| r.enable == Some(1)).count();
>> +        RuleStat { all, active }
>> +    });
>> +
>> +    let status = match (enabled, rules) {
>> +        (Ok(enabled), Ok(rules)) => Some(FirewallStatus { enabled, rules }),
>> +        _ => None,
>> +    };
>> +
>> +    Ok(ClusterFirewallData { status, guests })
>> +}
>> +
> 
> 
> 
> _______________________________________________
> pdm-devel mailing list
> pdm-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
> 
> 



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel