* [PATCH pve-firewall v3 01/16] helpers: add helpers to update firewall object references
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 02/16] parser: do not log errors for disabled rules Arthur Bied-Charreton
` (15 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Renaming or deleting a firewall object (ipset or alias) that is still
referenced by rules, security groups or ipset members leaves dangling
references. The firewall fails to parse the affected rules and drops
them, so an edit can effectively disable a whole set of rules.
Add update_refs(), which finds such references and applies one of three
actions to them: 'rename' points them at the new name, 'disable' turns
off the referencing rules, and 'drop' removes them. An ipset member has
no disabled state, so 'disable' removes members as well. Matching is
case-insensitive and rewritten references are normalized to lowercase.
When operating on the cluster config it also covers every downstream
config (guest, host and vnet) across the cluster.
On top of that, add three wrappers for the SDN-generated IPSets, which
no caller can delete but which disappear once the configuration stops
generating them: update_sdn_ipset_refs() for a list of such IPSets,
update_vnet_ipset_refs() for the four IPSets of a VNet, and
update_guest_ipam_ipset_refs() for a guest's IPAM IPSet.
Downstream configs are locked and saved individually as they are
visited, so on a cluster rename the caller must persist the config with
the new object already present before calling update_refs (keep both the
old and new object until all references are migrated). Otherwise a
concurrent firewall compilation could encounter a reference to an object
that does not exist yet and drop the rule.
Object references are not guaranteed to be scoped (dc/, guest/, sdn/)
if the rules have been added by manually editing the configs. This is
not an issue for cluster, host and vnet configs, as in those cases the
reference can only point to an object defined in the cluster config.
Guest configs can however define their own objects. Unscoped references
in guest rules are therefore resolved by first checking for a definition
in the relevant guest config, and only then in the cluster config, to
prevent overwriting the wrong reference.
SDN IPSets can additionally be shadowed by IPSets defined in the cluster
config. This is handled by also checking for same-named IPSets in the
cluster config for SDN IPSets.
The ipset and alias endpoints in the following commits build on this, as
do PUT /cluster/sdn in pve-network and the guest destroy endpoints in
qemu-server and pve-container.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/Firewall/Helpers.pm | 308 ++++++++++++++++++++++++++++++++++++
1 file changed, 308 insertions(+)
diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm
index fa3646c..2331e27 100644
--- a/src/PVE/Firewall/Helpers.pm
+++ b/src/PVE/Firewall/Helpers.pm
@@ -9,6 +9,7 @@ use File::Basename qw(fileparse);
use IO::Zlib;
use PVE::Cluster;
use PVE::Network;
+use PVE::Network::SDN::Vnets;
use PVE::Tools qw(file_get_contents file_set_contents);
use base 'Exporter';
@@ -18,8 +19,12 @@ our @EXPORT_OK = qw(
clone_vmfw_conf
collect_refs
flush_fw_ct_entries_by_mark
+ update_refs
+ get_object_spec
);
+require PVE::Firewall;
+
my $pvefw_conf_dir = "/etc/pve/firewall";
sub lock_vmfw_conf {
@@ -234,4 +239,307 @@ sub flush_fw_ct_entries_by_mark {
);
}
+=head3 map_items($items, $action, $matches)
+
+Apply C<$action> to each item C<$item> in C<$items> for which C<$matches->($item)> is true. Remove
+C<$item> from C<$items> if C<$action->($item)> returns C<undef>.
+
+Return the updated items arrayref and a boolean indicating whether any item was matched.
+
+=cut
+
+sub map_items {
+ my ($items, $action, $matches) = @_;
+ my @result;
+ my $modified = 0;
+ for my $item (($items // [])->@*) {
+ if ($matches->($item)) {
+ $modified = 1;
+ my $new = $action->($item);
+ push @result, $new if defined $new;
+ } else {
+ push @result, $item;
+ }
+ }
+ return (\@result, $modified);
+}
+
+=head3 foreach_conf_in_env($conf, $rule_env, $rewrite)
+
+Apply C<$rewrite> to the main firewall configs and, if C<$rule_env> is 'cluster', to all guest, host
+and vnet firewall configs across the cluster. Configs where C<$rewrite> returns true are saved. The
+caller is responsible for locking and saving the cluster config (C<$conf>).
+
+=cut
+
+sub foreach_conf_in_env {
+ my ($conf, $rule_env, $rewrite) = @_;
+
+ $rewrite->($conf, $rule_env, 0);
+
+ return if $rule_env ne 'cluster' && $rule_env ne 'sdn';
+
+ my $vmlist = PVE::Cluster::get_vmlist();
+ my $vmids = ($vmlist // {})->{ids} // {};
+ for my $vmid (keys $vmids->%*) {
+ PVE::Firewall::lock_vmfw_conf(
+ $vmid,
+ 10,
+ sub {
+ my $env = $vmlist->{ids}->{$vmid}->{type} eq 'lxc' ? 'ct' : 'vm';
+ my $guest_conf = PVE::Firewall::load_vmfw_conf($conf, $env, $vmid);
+ if ($rewrite->($guest_conf, $rule_env, 1)) {
+ PVE::Firewall::save_vmfw_conf($vmid, $guest_conf);
+ }
+ },
+ );
+ }
+
+ for my $node (PVE::Cluster::get_nodelist()->@*) {
+ my $host_conf_path = "/etc/pve/nodes/$node/host.fw";
+ PVE::Firewall::lock_hostfw_conf(
+ $node,
+ 10,
+ sub {
+ my $host_conf = PVE::Firewall::load_hostfw_conf($conf, $host_conf_path);
+ if ($rewrite->($host_conf, $rule_env, 0)) {
+ PVE::Firewall::save_hostfw_conf($host_conf, $host_conf_path);
+ }
+ },
+ );
+ }
+
+ my $vnets = (PVE::Network::SDN::Vnets::config(1) // {})->{ids} // {};
+ for my $vnet (keys $vnets->%*) {
+ PVE::Firewall::lock_vnetfw_conf(
+ $vnet,
+ 10,
+ sub {
+ my $vnet_conf = PVE::Firewall::load_vnetfw_conf($conf, 'vnet', $vnet);
+ if ($rewrite->($vnet_conf, $rule_env, 0)) {
+ PVE::Firewall::save_vnetfw_conf($vnet, $vnet_conf);
+ }
+ },
+ );
+ }
+}
+
+my $object_ref_specs = {
+ ipset => { prefix => '+', self => 'ipset' },
+ aliases => { prefix => '', self => 'aliases' },
+};
+
+=head3 get_object_spec($kind)
+
+Get the spec hash for C<$kind>. Refer to the C<update_refs> POD for details.
+
+=cut
+
+sub get_object_spec {
+ my ($kind) = @_;
+ return $object_ref_specs->{$kind};
+}
+
+=head3 rewrite_refs_in_conf($conf, $spec, $old, $new, $env, $is_guest, $cluster_conf, $action)
+
+Apply C<$action> to all references to C<$old> across C<$conf>, renaming them to C<$new>, disabling
+the referencing rules, or dropping them.
+
+Only exposed for testing, see POD for C<update_refs> for details.
+
+=cut
+
+sub rewrite_refs_in_conf {
+ my ($conf, $spec, $old, $new, $env, $is_guest, $cluster_conf, $action) = @_;
+
+ my $ref_fields = ['source', 'dest', 'cidr'];
+ my $prefix = $spec->{prefix};
+
+ my $repl = {};
+ for my $name ($old->@*) {
+ my $shadowed = $is_guest && $conf->{ $spec->{self} }->{$name};
+
+ my $scopes = [];
+ if ($env eq 'cluster') {
+ push $scopes->@*, 'dc/';
+ push $scopes->@*, '' if !$shadowed;
+ } elsif ($env eq 'sdn') {
+ push $scopes->@*, 'sdn/';
+ push $scopes->@*, '' if !$shadowed && !$cluster_conf->{ $spec->{self} }->{$name};
+ } else {
+ push $scopes->@*, '';
+ push $scopes->@*, 'guest/';
+ }
+
+ $repl->{"$prefix$_$name"} = defined($new) ? "$prefix$_$new" : undef for $scopes->@*;
+ }
+
+ my $matches = sub {
+ my ($obj) = @_;
+ grep { exists($repl->{ lc($obj->{$_} // '') }) } $ref_fields->@*;
+ };
+
+ my $rename = sub {
+ my ($obj) = @_;
+
+ for my $f ($ref_fields->@*) {
+ my $r = lc($obj->{$f} // '');
+ $obj->{$f} = $repl->{$r} if exists($repl->{$r});
+ }
+
+ return $obj;
+ };
+
+ my $rewrite_rule = sub {
+ my ($obj) = @_;
+
+ return $rename->($obj) if $action eq 'rename';
+ return undef if $action eq 'drop';
+
+ $obj->{enable} = 0;
+ return $obj;
+ };
+
+ my $rewrite_member = sub {
+ my ($obj) = @_;
+ # an ipset member cannot be disabled
+ return $action eq 'rename' ? $rename->($obj) : undef;
+ };
+
+ my $modified = 0;
+ my ($rules, $ch) = map_items($conf->{rules}, $rewrite_rule, $matches);
+ $conf->{rules} = $rules;
+ $modified ||= $ch;
+
+ for my $section ([groups => $rewrite_rule], [ipset => $rewrite_member]) {
+ my ($name, $rewrite) = $section->@*;
+ my $map = $conf->{$name} // {};
+ for my $key (keys $map->%*) {
+ ($map->{$key}, my $c) = map_items($map->{$key}, $rewrite, $matches);
+ $modified ||= $c;
+ }
+ }
+
+ return $modified;
+}
+
+=head3 update_refs($conf, $spec, $old, $new, $rule_env, $action)
+
+Rename, disable or drop all references to a firewall object across the environment.
+
+References are matched in rules, security groups and IPSet members. Matching is case-insensitive
+and renames are written back lowercased.
+
+C<$conf> is the firewall configuration the object is defined in.
+
+C<$spec> describes the object kind:
+
+ { prefix => '+' | '', self => 'ipset' | 'aliases' }
+
+C<prefix> is the prefix a reference carries; C<self> is the section a downstream config may use to
+shadow a same-named cluster object.
+
+C<$old> is the name of the object whose references are to be edited. The disable and drop paths
+also accept an array reference, so that references to several objects can be handled in a single
+pass over the configs.
+
+C<$new> is the new name of the object, and is only used when renaming.
+
+C<$rule_env> describes the environment the object comes from, and may be C<cluster> for cluster
+objects, C<sdn> for SDN objects, or C<vm>/C<ct> for guest-defined objects.
+
+C<$action> is what to do with the references, and may be C<rename> to point them at C<$new>,
+C<disable> to disable the referencing rules, or C<drop> to remove them. IPSet members have no
+disabled state, so C<disable> removes them as well. It defaults to C<rename> if C<$new> is
+defined, and to C<disable> otherwise.
+
+The caller is responsible for locking and saving C<$conf>.
+
+If C<$conf> is the cluster config, i.e. if C<$rule_env> is C<cluster> or C<sdn>, guest, host and
+vnet configs will be sequentially locked, updated and saved. Therefore, if this function is called
+for one of those environments, a I<renaming> caller must first persist C<$conf> with the new
+(renamed) object present, so references rewritten in those downstream configs do not point at a
+not-yet-saved object during concurrent compilations.
+
+=cut
+
+sub update_refs {
+ my ($conf, $spec, $old, $new, $rule_env, $action) = @_;
+
+ $action //= defined($new) ? 'rename' : 'disable';
+
+ die "invalid action '$action'\n" if $action !~ m/^(rename|disable|drop)$/;
+
+ my $lc_old = [map { lc($_) } (ref($old) eq 'ARRAY' ? $old->@* : $old)];
+ my $lc_new = $action eq 'rename' ? lc($new) : undef;
+
+ die "cannot rename more than one object at once\n"
+ if $action eq 'rename' && scalar($lc_old->@*) != 1;
+
+ my $code = sub {
+ my ($fw_conf, $env, $is_guest) = @_;
+ return rewrite_refs_in_conf(
+ $fw_conf, $spec, $lc_old, $lc_new, $env, $is_guest, $conf, $action,
+ );
+ };
+
+ return foreach_conf_in_env($conf, $rule_env, $code);
+}
+
+=head3 update_sdn_ipset_refs($ipsets, $action)
+
+Apply C<$action> to all references to the SDN-generated IPSets named in C<$ipsets>, which are not
+objects a caller could delete, but exist for as long as the SDN configuration generates them.
+
+=cut
+
+sub update_sdn_ipset_refs {
+ my ($ipsets, $action) = @_;
+
+ PVE::Firewall::lock_clusterfw_conf(
+ 10,
+ sub {
+ my $conf = PVE::Firewall::load_clusterfw_conf();
+
+ update_refs($conf, get_object_spec('ipset'), $ipsets, undef, 'sdn', $action);
+
+ PVE::Firewall::save_clusterfw_conf($conf);
+ },
+ );
+}
+
+=head3 update_vnet_ipset_refs($vnets, $action)
+
+Apply C<$action> to all references to the IPSets generated for the VNets named in C<$vnets>, for
+callers about to remove those VNets from the running SDN configuration.
+
+=cut
+
+sub update_vnet_ipset_refs {
+ my ($vnets, $action) = @_;
+
+ my $ipsets = [
+ map {
+ my $vnet = $_;
+ map { "$vnet-$_" } qw(all gateway no-gateway dhcp)
+ } $vnets->@*
+ ];
+
+ return update_sdn_ipset_refs($ipsets, $action);
+}
+
+=head3 update_guest_ipam_ipset_refs($vmid, $action)
+
+Apply C<$action> to all references to the IPAM IPSet of the guest C<$vmid>, for callers about to
+destroy that guest. Note that this IPSet also disappears whenever a guest releases its last IPAM
+entry, which happens on ordinary network changes too, so this is only appropriate when the guest
+is going away for good.
+
+=cut
+
+sub update_guest_ipam_ipset_refs {
+ my ($vmid, $action) = @_;
+
+ return update_sdn_ipset_refs("guest-ipam-$vmid", $action);
+}
1;
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-firewall v3 02/16] parser: do not log errors for disabled rules
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 01/16] helpers: add helpers to update firewall object references Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 03/16] api: ipset: add option to update references on edit Arthur Bied-Charreton
` (14 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
The rule parser always logged errors when encountering them, even if the
offending rule was disabled.
As a preparatory step for implementing automatic disabling of rules
with dangling references to IPSets or Aliases, avoid logging errors for
disabled rules.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/Firewall.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index da6976e..a01738c 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -3384,7 +3384,7 @@ sub parse_fw_rule {
die "unable to parse rule parameters: $line\n" if length($line);
$rule = verify_rule($rule, $cluster_conf, $fw_conf, $rule_env, 1);
- if ($rule->{errors}) {
+ if ($rule->{enable} && $rule->{errors}) {
# The verbose flag really means we're running from the CLI and want
# output on the console - in the other case we really want such errors
# to go into the syslog instead.
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-firewall v3 03/16] api: ipset: add option to update references on edit
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 01/16] helpers: add helpers to update firewall object references Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 02/16] parser: do not log errors for disabled rules Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 04/16] api: ipset: add option to handle dangling references on delete Arthur Bied-Charreton
` (13 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Renaming an ipset still referenced by rules leaves dangling references.
The firewall then fails to parse those rules during compilation and
drops them. The errors, while logged to the journal, are not visible
from the GUI - a rename can therefore effectively disable a whole set
of rules.
Add an 'update-references' option to the rename path to rewrite them to
the new name. For cluster ipsets, this also covers references in
downstream configs (host, guest and vnet).
The new ipset is persisted before its references are rewritten, so a
concurrent firewall compilation never observes a dangling reference. If
the cluster-wide rewrite is interrupted, it can be retried by passing
'update-references=force'.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Firewall/IPSet.pm | 60 +++++++++++++++++++++++++++-------
1 file changed, 48 insertions(+), 12 deletions(-)
diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 82b9aaf..e734b8c 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -534,6 +534,7 @@ package PVE::API2::Firewall::BaseIPSetList;
use strict;
use warnings;
+use PVE::Firewall::Helpers qw(update_refs get_object_spec);
use PVE::JSONSchema qw(get_standard_option);
use PVE::Exception qw(raise_param_exc);
use PVE::Firewall;
@@ -661,6 +662,16 @@ sub register_create {
},
);
+ $properties->{'update-references'} = {
+ type => 'string',
+ enum => ['no', 'yes', 'force'],
+ optional => 1,
+ description =>
+ "Update all references to the IPSet when renaming it. Use 'force' to also "
+ . "overwrite an existing target IPSet, e.g. to resume an interrupted rename.",
+ default => 'no',
+ };
+
$class->register_method({
name => 'create_ipset',
path => '',
@@ -681,6 +692,8 @@ sub register_create {
sub {
my ($param) = @_;
+ my $update_references = $param->{'update-references'} // 'no';
+
my ($cluster_conf, $fw_conf) = $class->load_config($param);
if ($param->{rename}) {
@@ -690,19 +703,42 @@ sub register_create {
raise_param_exc({ name => "IPSet '$param->{rename}' does not exist" })
if !$fw_conf->{ipset}->{ $param->{rename} };
- # prevent overwriting existing ipset
- raise_param_exc({ name => "IPSet '$param->{name}' does already exist" })
- if $fw_conf->{ipset}->{ $param->{name} }
- && $param->{name} ne $param->{rename};
-
- my $data = delete $fw_conf->{ipset}->{ $param->{rename} };
- $fw_conf->{ipset}->{ $param->{name} } = $data;
- if (
- my $comment =
- delete $fw_conf->{ipset_comments}->{ $param->{rename} }
- ) {
- $fw_conf->{ipset_comments}->{ $param->{name} } = $comment;
+ if ($param->{name} ne $param->{rename}) {
+ # prevent overwriting existing ipset
+ raise_param_exc({
+ name => "IPSet '$param->{name}' does already exist" })
+ if $fw_conf->{ipset}->{ $param->{name} }
+ && $update_references ne 'force';
+
+ $fw_conf->{ipset}->{ $param->{name} } =
+ $fw_conf->{ipset}->{ $param->{rename} };
+
+ if ($update_references ne 'no') {
+ my $env = $class->rule_env();
+ my $spec = get_object_spec('ipset');
+ my $old = $param->{rename};
+ my $new = $param->{name};
+
+ # persist the new ipset before rewriting references so a concurrent
+ # compilation never sees a reference to a not-yet-saved ipset.
+ $class->save_config($param, $fw_conf) if $env eq 'cluster';
+
+ eval { update_refs($fw_conf, $spec, $old, $new, $env) };
+ die "rename interrupted, references may be partially updated; "
+ . "retry with 'force' to finish: $@"
+ if $@;
+ }
+
+ delete $fw_conf->{ipset}->{ $param->{rename} };
+
+ if (
+ my $comment =
+ delete $fw_conf->{ipset_comments}->{ $param->{rename} }
+ ) {
+ $fw_conf->{ipset_comments}->{ $param->{name} } = $comment;
+ }
}
+
$fw_conf->{ipset_comments}->{ $param->{name} } = $param->{comment}
if defined($param->{comment});
} else {
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-firewall v3 04/16] api: ipset: add option to handle dangling references on delete
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (2 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-firewall v3 03/16] api: ipset: add option to update references on edit Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 05/16] api: aliases: add option to update references on edit Arthur Bied-Charreton
` (12 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Deleting an ipset referenced by rules or security groups leaves dangling
references, which the firewall fails to parse and drops.
Add a 'dangling-references' option to the delete endpoint, to either
'disable' the referencing rules or 'drop' them along with the ipset. The
default, 'keep', leaves them as they are. For cluster ipsets, this also
covers all downstream configs (host, guest and vnet).
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Firewall/IPSet.pm | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index e734b8c..32e3630 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -2,6 +2,7 @@ package PVE::API2::Firewall::IPSetBase;
use strict;
use warnings;
+use PVE::Firewall::Helpers qw(update_refs get_object_spec);
use PVE::Exception qw(raise raise_param_exc);
use PVE::JSONSchema qw(get_standard_option);
@@ -140,6 +141,15 @@ sub register_delete_ipset {
optional => 1,
description => 'Delete all members of the IPSet, if there are any.',
};
+ $properties->{'dangling-references'} = {
+ type => 'string',
+ enum => ['keep', 'disable', 'drop'],
+ optional => 1,
+ description =>
+ "Handle references that the deletion would leave dangling. Use 'disable' to disable "
+ . "the referencing rules, or 'drop' to remove them entirely.",
+ default => 'keep',
+ };
$class->register_method({
name => 'delete_ipset',
@@ -166,6 +176,19 @@ sub register_delete_ipset {
die "IPSet '$param->{name}' is not empty\n"
if scalar(@$ipset) && !$param->{force};
+ my $action = $param->{'dangling-references'} // 'keep';
+ if ($action ne 'keep') {
+ my $spec = get_object_spec('ipset');
+ update_refs(
+ $fw_conf,
+ $spec,
+ $param->{name},
+ undef,
+ $class->rule_env(),
+ $action,
+ );
+ }
+
$class->save_ipset($param, $fw_conf, undef);
},
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-firewall v3 05/16] api: aliases: add option to update references on edit
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (3 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-firewall v3 04/16] api: ipset: add option to handle dangling references on delete Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-firewall v3 06/16] api: aliases: add option to handle dangling references on delete Arthur Bied-Charreton
` (11 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Renaming an alias still referenced by rules or ipset members leaves
dangling references. The firewall then fails to parse those rules
during compilation and drops them. The errors, while logged to the
journal, are not visible from the GUI - a rename can therefore
effectively disable a whole set of rules.
Add an 'update-references' option to the rename path to rewrite them to
the new name. For cluster aliases, this also covers references in
downstream configs (host, guest and vnet).
The new alias is persisted before its references are rewritten, so a
concurrent firewall compilation never observes a dangling reference. If
the cluster-wide rewrite is interrupted, it can be retried by passing
'update-references=force'.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Firewall/Aliases.pm | 39 +++++++++++++++++++++++++++++---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm
index 4f6960d..b5bad48 100644
--- a/src/PVE/API2/Firewall/Aliases.pm
+++ b/src/PVE/API2/Firewall/Aliases.pm
@@ -2,6 +2,8 @@ package PVE::API2::Firewall::AliasesBase;
use strict;
use warnings;
+
+use PVE::Firewall::Helpers qw(update_refs get_object_spec);
use PVE::Exception qw(raise raise_param_exc);
use PVE::JSONSchema qw(get_standard_option);
@@ -219,6 +221,16 @@ sub register_update_alias {
$properties->{comment} = $api_properties->{comment};
$properties->{digest} = get_standard_option('pve-config-digest');
+ $properties->{'update-references'} = {
+ type => 'string',
+ enum => ['no', 'yes', 'force'],
+ optional => 1,
+ description =>
+ "Update all references to the alias when renaming it. Use 'force' to also "
+ . "overwrite an existing target alias, e.g. to resume an interrupted rename.",
+ default => 'no',
+ };
+
$class->register_method({
name => 'update_alias',
path => '{name}',
@@ -239,6 +251,8 @@ sub register_update_alias {
sub {
my ($param) = @_;
+ my $update_references = $param->{'update-references'} // 'no';
+
my ($fw_conf, $aliases) = $class->load_config($param);
my $list = &$aliases_to_list($aliases);
@@ -261,9 +275,28 @@ sub register_update_alias {
if ($rename && ($name ne $rename)) {
raise_param_exc({ name => "alias '$param->{rename}' already exists" })
- if defined($aliases->{$rename});
- $aliases->{$name}->{name} = $param->{rename};
- $aliases->{$rename} = $aliases->{$name};
+ if defined($aliases->{$rename})
+ && $update_references ne 'force';
+
+ $aliases->{$rename} =
+ { $aliases->{$name}->%*, name => $param->{rename} };
+
+ if ($update_references ne 'no') {
+ my $env = $class->rule_env();
+ my $spec = get_object_spec('aliases');
+ my $new_name = $param->{rename};
+
+ # persist the new alias before rewriting references so a concurrent
+ # compilation never sees a reference to a not-yet-saved alias.
+ $class->save_aliases($param, $fw_conf, $aliases)
+ if $env eq 'cluster';
+
+ eval { update_refs($fw_conf, $spec, $name, $new_name, $env) };
+ die "rename interrupted, references may be partially updated; "
+ . "retry with 'force' to finish: $@"
+ if $@;
+ }
+
delete $aliases->{$name};
}
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-firewall v3 06/16] api: aliases: add option to handle dangling references on delete
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (4 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-firewall v3 05/16] api: aliases: add option to update references on edit Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` SPAM: [PATCH pve-firewall v3 07/16] firewall: tests: add tests for object reference update logic Arthur Bied-Charreton
` (10 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Deleting an alias referenced by rules, security groups or ipset members
leaves dangling references, which the firewall fails to parse and drops.
Add a 'dangling-references' option to the delete endpoint, to either
'disable' the referencing rules or 'drop' them along with the alias. The
default, 'keep', leaves them as they are. Ipset members have no disabled
state, so they are removed in both cases. For cluster aliases, this also
covers all downstream configs (host, guest and vnet).
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Firewall/Aliases.pm | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm
index b5bad48..8b9caaf 100644
--- a/src/PVE/API2/Firewall/Aliases.pm
+++ b/src/PVE/API2/Firewall/Aliases.pm
@@ -316,6 +316,16 @@ sub register_delete_alias {
$properties->{name} = $api_properties->{name};
$properties->{digest} = get_standard_option('pve-config-digest');
+ $properties->{'dangling-references'} = {
+ type => 'string',
+ enum => ['keep', 'disable', 'drop'],
+ optional => 1,
+ description =>
+ "Handle references that the deletion would leave dangling. Use 'disable' to disable "
+ . "the referencing rules, or 'drop' to remove them entirely. IPSet members "
+ . "referencing the alias are always removed, as they cannot be disabled.",
+ default => 'keep',
+ };
$class->register_method({
name => 'remove_alias',
@@ -344,6 +354,20 @@ sub register_delete_alias {
PVE::Tools::assert_if_modified($digest, $param->{digest});
my $name = lc($param->{name});
+
+ my $action = $param->{'dangling-references'} // 'keep';
+ if ($action ne 'keep') {
+ my $spec = get_object_spec('aliases');
+ update_refs(
+ $fw_conf,
+ $spec,
+ $param->{name},
+ undef,
+ $class->rule_env(),
+ $action,
+ );
+ }
+
delete $aliases->{$name};
$class->save_aliases($param, $fw_conf, $aliases);
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* SPAM: [PATCH pve-firewall v3 07/16] firewall: tests: add tests for object reference update logic
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (5 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-firewall v3 06/16] api: aliases: add option to handle dangling references on delete Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` SPAM: [PATCH pve-network v3 08/16] apply: add option to handle dangling references on VNet deletion Arthur Bied-Charreton
` (9 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
The reference updating logic has quite a few edge cases, especially
regarding the fact that cluster objects may be shadowed by guest
objects.
Add a few tests to consolidate the intended functionality. Note that
these tests do not cover the actual iteration over the different
configs in the cluster, rather they are focused on the core reference
rewriting logic.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
test/Makefile | 1 +
test/referenceupdatetests.pl | 296 +++++++++++++++++++++++++++++++++++
2 files changed, 297 insertions(+)
create mode 100755 test/referenceupdatetests.pl
diff --git a/test/Makefile b/test/Makefile
index fea9c21..15d2ae3 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -4,6 +4,7 @@ all:
.PHONY: check
check:
./fwtester.pl
+ ./referenceupdatetests.pl
.PHONY: install
install: check
diff --git a/test/referenceupdatetests.pl b/test/referenceupdatetests.pl
new file mode 100755
index 0000000..3912aab
--- /dev/null
+++ b/test/referenceupdatetests.pl
@@ -0,0 +1,296 @@
+#!/usr/bin/perl
+
+# tests for the alias/ipset reference updating logic
+
+use lib '../src';
+
+use v5.36;
+
+use Test::More;
+
+use PVE::Firewall::Helpers;
+
+my $ipset_spec = PVE::Firewall::Helpers::get_object_spec('ipset');
+my $alias_spec = PVE::Firewall::Helpers::get_object_spec('aliases');
+
+sub rewrite($conf, $spec, $old, $new, $env, $is_guest, $action = undef, $cluster_conf = {}) {
+ $action //= defined($new) ? 'rename' : 'disable';
+ $old = [$old] if ref($old) ne 'ARRAY';
+ return PVE::Firewall::Helpers::rewrite_refs_in_conf(
+ $conf, $spec, $old, $new, $env, $is_guest, $cluster_conf, $action,
+ );
+}
+
+subtest 'ipset: cluster rename rewrites dc/ and bare refs' => sub {
+ my $conf = {
+ rules => [
+ { source => '+dc/foo' },
+ { dest => '+foo' },
+ { source => '+bar' },
+ { source => '+dc/bar' },
+ ],
+ groups => {
+ grp => [{ source => '+dc/foo' }, { dest => '+foo' }],
+ },
+ ipset => {
+ set => [{ cidr => '10.0.0.0/8' }],
+ },
+ };
+
+ my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0);
+
+ ok($modified, 'reports modified');
+ is($conf->{rules}->[0]->{source}, '+dc/baz', 'dc/ ref rewritten');
+ is($conf->{rules}->[1]->{dest}, '+baz', 'bare ref rewritten');
+ is($conf->{rules}->[2]->{source}, '+bar', 'unrelated bare ref kept');
+ is($conf->{rules}->[3]->{source}, '+dc/bar', 'unrelated dc/ ref kept');
+ is($conf->{groups}->{grp}->[0]->{source}, '+dc/baz', 'group dc/ ref rewritten');
+ is($conf->{groups}->{grp}->[1]->{dest}, '+baz', 'group bare ref rewritten');
+ is($conf->{ipset}->{set}->[0]->{cidr}, '10.0.0.0/8', 'ipset member not touched');
+};
+
+subtest 'ipset: cluster delete disables matching rules' => sub {
+ my $conf = {
+ rules => [
+ { source => '+dc/foo', enable => 1 },
+ { source => '+foo', enable => 1 },
+ { source => '+other', enable => 1 },
+ ],
+ groups =>
+ { grp => [{ dest => '+dc/foo', enable => 1 }, { dest => '+keep', enable => 1 }] },
+ };
+
+ my $modified = rewrite($conf, $ipset_spec, 'foo', undef, 'cluster', 0);
+
+ ok($modified, 'reports modified');
+ is(scalar($conf->{rules}->@*), 3, 'no rule dropped');
+ is($conf->{rules}->[0]->{enable}, 0, 'dc/ ref rule disabled');
+ is($conf->{rules}->[1]->{enable}, 0, 'bare ref rule disabled');
+ is($conf->{rules}->[2]->{enable}, 1, 'unrelated rule untouched');
+ is($conf->{groups}->{grp}->[0]->{enable}, 0, 'group rule disabled');
+ is($conf->{groups}->{grp}->[1]->{enable}, 1, 'unrelated group rule untouched');
+};
+
+subtest 'ipset: cluster drop removes matching rules' => sub {
+ my $conf = {
+ rules => [
+ { source => '+dc/foo', enable => 1 },
+ { source => '+foo', enable => 1 },
+ { source => '+other', enable => 1 },
+ ],
+ groups =>
+ { grp => [{ dest => '+dc/foo', enable => 1 }, { dest => '+keep', enable => 1 }] },
+ };
+
+ my $modified = rewrite($conf, $ipset_spec, 'foo', undef, 'cluster', 0, 'drop');
+
+ ok($modified, 'reports modified');
+ is(scalar($conf->{rules}->@*), 1, 'matching rules dropped');
+ is($conf->{rules}->[0]->{source}, '+other', 'unrelated rule kept');
+ is($conf->{rules}->[0]->{enable}, 1, 'kept rule not disabled');
+ is(scalar($conf->{groups}->{grp}->@*), 1, 'matching group rule dropped');
+ is($conf->{groups}->{grp}->[0]->{dest}, '+keep', 'unrelated group rule kept');
+};
+
+subtest 'ipset: guest shadows cluster object' => sub {
+ my $conf = {
+ rules => [
+ { source => '+foo' }, { dest => '+dc/foo' },
+ ],
+ ipset => { foo => [] },
+ };
+
+ my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 1);
+
+ ok($modified, 'reports modified');
+ is($conf->{rules}->[0]->{source}, '+foo', 'shadowed bare ref not touched');
+ is($conf->{rules}->[1]->{dest}, '+dc/baz', 'explicit cluster ref rewritten');
+};
+
+subtest 'ipset: guest without own object' => sub {
+ my $conf = {
+ rules => [{ source => '+foo' }, { dest => '+dc/foo' }],
+ ipset => {},
+ };
+
+ rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 1);
+
+ is($conf->{rules}->[0]->{source}, '+baz', 'implicit cluster ref rewritten');
+ is($conf->{rules}->[1]->{dest}, '+dc/baz', 'explicit cluster ref rewritten');
+};
+
+subtest 'ipset: guest-level rename' => sub {
+ my $conf = {
+ rules => [
+ { source => '+foo' }, { dest => '+guest/foo' }, { source => '+dc/foo' },
+ ],
+ };
+
+ rewrite($conf, $ipset_spec, 'foo', 'baz', 'vm', 0);
+
+ is($conf->{rules}->[0]->{source}, '+baz', 'implicit guest ref rewritten');
+ is($conf->{rules}->[1]->{dest}, '+guest/baz', 'guest/ ref rewritten');
+ is($conf->{rules}->[2]->{source}, '+dc/foo', 'dc/ ref not touched in guest env');
+};
+
+subtest 'ipset: sdn scope, shadowing and multiple objects' => sub {
+ my $conf = {
+ rules => [
+ { source => '+sdn/vnet0-all', enable => 1 },
+ { dest => '+vnet0-all', enable => 1 },
+ { source => '+sdn/vnet0-dhcp', enable => 1 },
+ { dest => '+vnet0-dhcp', enable => 1 },
+ { source => '+dc/vnet0-all', enable => 1 },
+ ],
+ # shadows the SDN-generated IPSet of the same name
+ ipset => { 'vnet0-dhcp' => [] },
+ };
+
+ my $vnet_ipsets = ['vnet0-all', 'vnet0-dhcp'];
+ my $modified = rewrite($conf, $ipset_spec, $vnet_ipsets, undef, 'sdn', 0, 'disable', $conf);
+
+ ok($modified, 'reports modified');
+ is($conf->{rules}->[0]->{enable}, 0, 'sdn/ ref disabled');
+ is($conf->{rules}->[1]->{enable}, 0, 'bare ref disabled');
+ is($conf->{rules}->[2]->{enable}, 0, 'sdn/ ref disabled despite cluster IPSet');
+ is($conf->{rules}->[3]->{enable}, 1, 'bare ref kept, resolves to the cluster IPSet');
+ is($conf->{rules}->[4]->{enable}, 1, 'dc/ ref not touched in sdn env');
+
+ my $guest_conf = {
+ rules => [
+ { source => '+sdn/vnet0-all', enable => 1 },
+ { dest => '+vnet0-all', enable => 1 },
+ ],
+ ipset => { 'vnet0-all' => [] },
+ };
+
+ rewrite($guest_conf, $ipset_spec, $vnet_ipsets, undef, 'sdn', 1, 'disable', {});
+
+ is($guest_conf->{rules}->[0]->{enable}, 0, 'sdn/ ref disabled in guest conf');
+ is($guest_conf->{rules}->[1]->{enable}, 1, 'bare ref kept, resolves to the guest IPSet');
+};
+
+subtest 'ipset: not confused with alias' => sub {
+ my $conf = { rules => [{ source => 'foo' }, { dest => '+foo' }] };
+
+ my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0);
+
+ ok($modified, 'reports modified');
+ is($conf->{rules}->[0]->{source}, 'foo', 'bare alias ref not touched');
+ is($conf->{rules}->[1]->{dest}, '+baz', 'ipset ref rewritten');
+};
+
+subtest 'alias: cluster rename rewrites rules and ipset members' => sub {
+ my $conf = {
+ rules => [{ source => 'al' }, { dest => 'dc/al' }, { source => 'other' }],
+ groups => { grp => [{ source => 'dc/al' }] },
+ ipset => {
+ set => [{ cidr => 'al' }, { cidr => 'dc/al' }, { cidr => '10.0.0.1' }],
+ },
+ };
+
+ my $modified = rewrite($conf, $alias_spec, 'al', 'new', 'cluster', 0);
+
+ ok($modified, 'reports modified');
+ is($conf->{rules}->[0]->{source}, 'new', 'bare alias in rule rewritten');
+ is($conf->{rules}->[1]->{dest}, 'dc/new', 'dc/ alias in rule rewritten');
+ is($conf->{rules}->[2]->{source}, 'other', 'unrelated rule kept');
+ is($conf->{groups}->{grp}->[0]->{source}, 'dc/new', 'alias in group rewritten');
+ is($conf->{ipset}->{set}->[0]->{cidr}, 'new', 'bare alias in ipset member rewritten');
+ is($conf->{ipset}->{set}->[1]->{cidr}, 'dc/new', 'dc/ alias in ipset member rewritten');
+ is($conf->{ipset}->{set}->[2]->{cidr}, '10.0.0.1', 'literal cidr member kept');
+};
+
+subtest 'alias: cluster delete disables rules, drops ipset members' => sub {
+ my $conf = {
+ rules => [
+ { source => 'al', dest => '10.0.0.1', action => 'ACCEPT', enable => 1 },
+ { source => 'keep', enable => 1 },
+ ],
+ groups =>
+ { grp => [{ source => 'dc/al', enable => 1 }, { source => 'keep', enable => 1 }] },
+ ipset => {
+ set =>
+ [{ cidr => 'al' }, { cidr => 'dc/al', nomatch => 1 }, { cidr => '10.0.0.1' }],
+ },
+ };
+
+ my $modified = rewrite($conf, $alias_spec, 'al', undef, 'cluster', 0);
+
+ ok($modified, 'reports modified');
+
+ is(scalar($conf->{rules}->@*), 2, 'no rule dropped');
+ is($conf->{rules}->[0]->{enable}, 0, 'matching rule disabled');
+ is($conf->{rules}->[0]->{source}, 'al', 'disabled rule keeps its reference');
+ is($conf->{rules}->[0]->{dest}, '10.0.0.1', 'disabled rule keeps its other properties');
+ is($conf->{rules}->[1]->{enable}, 1, 'unrelated rule untouched');
+
+ is(scalar($conf->{groups}->{grp}->@*), 2, 'no group rule dropped');
+ is($conf->{groups}->{grp}->[0]->{enable}, 0, 'matching group rule disabled');
+ is($conf->{groups}->{grp}->[1]->{enable}, 1, 'unrelated group rule untouched');
+
+ is(scalar($conf->{ipset}->{set}->@*), 1, 'matching members dropped, not disabled');
+ is($conf->{ipset}->{set}->[0]->{cidr}, '10.0.0.1', 'literal member kept');
+ ok(
+ !(grep { exists($_->{enable}) } $conf->{ipset}->{set}->@*),
+ 'no ipset member carries an enable flag',
+ );
+};
+
+subtest 'alias: cluster drop removes rules and ipset members' => sub {
+ my $conf = {
+ rules => [{ source => 'al', enable => 1 }, { source => 'keep', enable => 1 }],
+ ipset => { set => [{ cidr => 'al' }, { cidr => 'dc/al' }, { cidr => '10.0.0.1' }] },
+ };
+
+ rewrite($conf, $alias_spec, 'al', undef, 'cluster', 0, 'drop');
+
+ is(scalar($conf->{rules}->@*), 1, 'matching rule dropped');
+ is($conf->{rules}->[0]->{source}, 'keep', 'unrelated rule kept');
+ is($conf->{rules}->[0]->{enable}, 1, 'kept rule not disabled');
+ is(scalar($conf->{ipset}->{set}->@*), 1, 'matching members dropped');
+ is($conf->{ipset}->{set}->[0]->{cidr}, '10.0.0.1', 'literal member kept');
+};
+
+subtest 'alias: not confused with ipset' => sub {
+ my $conf = { rules => [{ source => '+al' }, { dest => 'al' }] };
+
+ rewrite($conf, $alias_spec, 'al', 'new', 'cluster', 0);
+
+ is($conf->{rules}->[0]->{source}, '+al', 'ipset ref (+) not touched');
+ is($conf->{rules}->[1]->{dest}, 'new', 'alias ref rewritten');
+};
+
+subtest 'case-insensitive match, written back lowercase' => sub {
+ my $conf = { rules => [{ source => '+DC/FOO' }, { dest => '+Foo' }] };
+
+ rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0);
+
+ is($conf->{rules}->[0]->{source}, '+dc/baz', 'dc/ ref matched and lowercased');
+ is($conf->{rules}->[1]->{dest}, '+baz', 'bare ref matched and lowercased');
+};
+
+subtest 'no match reports not modified' => sub {
+ my $conf = { rules => [{ source => '+other' }, { dest => 'somealias' }] };
+
+ my $modified = rewrite($conf, $ipset_spec, 'foo', 'baz', 'cluster', 0);
+
+ ok(!$modified, 'nothing matched -> not modified');
+ is($conf->{rules}->[0]->{source}, '+other', 'unrelated refs not touched');
+};
+
+subtest 'update_refs: rejects invalid argument combinations' => sub {
+ my $conf = { rules => [] };
+
+ eval {
+ PVE::Firewall::Helpers::update_refs($conf, $ipset_spec, 'foo', 'baz', 'vm', 'bogus');
+ };
+ like($@, qr/invalid action/, 'unknown action rejected');
+
+ eval {
+ PVE::Firewall::Helpers::update_refs($conf, $ipset_spec, ['foo', 'bar'], 'baz', 'vm');
+ };
+ like($@, qr/cannot rename more than one object/, 'renaming several objects rejected');
+};
+
+done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* SPAM: [PATCH pve-network v3 08/16] apply: add option to handle dangling references on VNet deletion
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (6 preceding siblings ...)
2026-09-25 9:42 ` SPAM: [PATCH pve-firewall v3 07/16] firewall: tests: add tests for object reference update logic Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH qemu-server v3 09/16] api: destroy_vm: add option to handle dangling IPSet references Arthur Bied-Charreton
` (8 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
For each VNet, 4 IPSets are auto-generated (<vnet>-all, -gateway,
-no-gateway and -dhcp) [0]. Therefore, deleting a VNet can lead to
rules referencing non-existing IPSets, which the firewall fails to
parse and ignores.
Add a 'dangling-ipset-references' option for PUT /cluster/sdn allowing
to 'drop' rules referencing now-deleted IPSets, 'disable' them, or
'keep' them as they are.
[0] https://pve.proxmox.com/wiki/Software-Defined_Network#pvesdn_firewall_integration
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Network/SDN.pm | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index e3c8d9d..e304299 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -8,6 +8,8 @@ use JSON qw(from_json);
use PVE::Cluster qw(cfs_lock_file cfs_read_file cfs_write_file);
use PVE::Exception qw(raise_param_exc);
+use PVE::Firewall;
+use PVE::Firewall::Helpers;
use PVE::JSONSchema qw(get_standard_option);
use PVE::RESTHandler;
use PVE::RPCEnvironment;
@@ -302,6 +304,18 @@ __PACKAGE__->register_method({
description =>
'When lock-token has been provided and configuration successfully committed, release the lock automatically afterwards',
},
+ 'dangling-ipset-references' => {
+ type => 'string',
+ enum => ['keep', 'disable', 'drop'],
+ optional => 1,
+ default => 'keep',
+ description =>
+ 'If the applied SDN changes delete one or more VNets, handle the firewall '
+ . 'rules referencing their auto-generated IPSets. Use \'disable\' to disable '
+ . 'those rules, or \'drop\' to remove them entirely. Kept as they are, they '
+ . 'reference an IPSet that no longer exists and are dropped from the '
+ . 'generated ruleset.',
+ },
},
},
returns => {
@@ -315,6 +329,7 @@ __PACKAGE__->register_method({
my $lock_token = extract_param($param, 'lock-token');
my $release_lock = extract_param($param, 'release-lock');
+ my $dangling_refs = extract_param($param, 'dangling-ipset-references') // 'keep';
my $previous_config_has_frr;
my $new_config_has_frr;
@@ -322,6 +337,23 @@ __PACKAGE__->register_method({
PVE::Network::SDN::lock_sdn_config(
sub {
$previous_config_has_frr = PVE::Network::SDN::running_config_has_frr();
+
+ if ($dangling_refs ne 'keep') {
+ my $running = PVE::Network::SDN::running_config() // {};
+ my $vnets = PVE::Network::SDN::Vnets::config() // {};
+ my $removed =
+ [grep { !$vnets->{ids}->{$_} } keys $running->{vnets}->{ids}->%*];
+
+ if ($removed->@*) {
+ eval {
+ PVE::Firewall::Helpers::update_vnet_ipset_refs(
+ $removed, $dangling_refs,
+ );
+ };
+ warn "could not update references to VNet IPSets: $@" if $@;
+ }
+ }
+
PVE::Network::SDN::commit_config();
$new_config_has_frr = PVE::Network::SDN::running_config_has_frr();
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH qemu-server v3 09/16] api: destroy_vm: add option to handle dangling IPSet references
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (7 preceding siblings ...)
2026-09-25 9:42 ` SPAM: [PATCH pve-network v3 08/16] apply: add option to handle dangling references on VNet deletion Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` SPAM: [PATCH pve-container v3 10/16] " Arthur Bied-Charreton
` (7 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
For each guest with IPAM entries, a 'guest-ipam-<vmid>' IPSet is
auto-generated [0]. Therefore, deleting a guest can lead to firewall
rules referencing non-existing IPSets, which the firewall fails to
parse and ignores.
Add a 'dangling-ipset-references' option for DELETE
/nodes/{node}/qemu/{vmid} allowing to 'drop' rules referencing
now-deleted IPSets, to 'disable' them, or to 'keep' them as they are.
[0] https://pve.proxmox.com/wiki/Software-Defined_Network#pvesdn_firewall_integration
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Qemu.pm | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 922c6599..75219ad0 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -2818,6 +2818,16 @@ __PACKAGE__->register_method({
optional => 1,
default => 0,
},
+ 'dangling-ipset-references' => {
+ type => 'string',
+ enum => ['keep', 'disable', 'drop'],
+ optional => 1,
+ default => 'keep',
+ description => 'Handle firewall rules referencing the IPSet auto-generated for '
+ . 'this guest (\'guest-ipam-<vmid>\'). Use \'disable\' to disable them, or '
+ . '\'drop\' to remove them entirely. Kept as they are, they reference an IPSet '
+ . 'that no longer exists and are dropped from the generated ruleset.',
+ },
},
},
returns => {
@@ -2895,6 +2905,16 @@ __PACKAGE__->register_method({
}
}
+ my $action = $param->{'dangling-ipset-references'} // 'keep';
+ if ($action ne 'keep') {
+ eval {
+ PVE::Firewall::Helpers::update_guest_ipam_ipset_refs(
+ $vmid, $action,
+ );
+ };
+ warn "could not $action dangling IPSet references: $@" if $@;
+ }
+
# only now remove the zombie config, else we can have reuse race
PVE::QemuConfig->destroy_config($vmid);
},
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* SPAM: [PATCH pve-container v3 10/16] api: destroy_vm: add option to handle dangling IPSet references
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (8 preceding siblings ...)
2026-09-25 9:42 ` [PATCH qemu-server v3 09/16] api: destroy_vm: add option to handle dangling IPSet references Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` SPAM: [PATCH pve-manager v3 11/16] ui: firewall: add common widgets for deleting and updating references Arthur Bied-Charreton
` (6 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
For each guest with IPAM entries, a 'guest-ipam-<vmid>' IPSet is
auto-generated [0]. Therefore, deleting a guest can lead to firewall
rules referencing non-existing IPSets, which the firewall fails to
parse and ignores.
Add a 'dangling-ipset-references' option for DELETE
/nodes/{node}/lxc/{vmid} allowing to 'drop' rules referencing
now-deleted IPSets, to 'disable' them, or to 'keep' them as they are.
[0] https://pve.proxmox.com/wiki/Software-Defined_Network#pvesdn_firewall_integration
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/LXC.pm | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 5f94d5a..ac0bbce 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -845,6 +845,16 @@ __PACKAGE__->register_method({
. " enabled storages which are not referenced in the config.",
optional => 1,
},
+ 'dangling-ipset-references' => {
+ type => 'string',
+ enum => ['keep', 'disable', 'drop'],
+ optional => 1,
+ default => 'keep',
+ description => 'Handle firewall rules referencing the IPSet auto-generated for '
+ . 'this guest (\'guest-ipam-<vmid>\'). Use \'disable\' to disable them, or '
+ . '\'drop\' to remove them entirely. Kept as they are, they reference an IPSet '
+ . 'that no longer exists and are dropped from the generated ruleset.',
+ },
},
},
returns => {
@@ -922,6 +932,12 @@ __PACKAGE__->register_method({
}
}
+ my $action = $param->{'dangling-ipset-references'} // 'keep';
+ if ($action ne 'keep') {
+ eval { PVE::Firewall::Helpers::update_guest_ipam_ipset_refs($vmid, $action); };
+ warn "could not $action dangling IPSet references: $@" if $@;
+ }
+
# only now remove the zombie config, else we can have reuse race
PVE::LXC::Config->destroy_config($vmid);
};
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* SPAM: [PATCH pve-manager v3 11/16] ui: firewall: add common widgets for deleting and updating references
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (9 preceding siblings ...)
2026-09-25 9:42 ` SPAM: [PATCH pve-container v3 10/16] " Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-manager v3 12/16] ui: firewall: ipset: add controls to update/delete references on edit Arthur Bied-Charreton
` (5 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
The ipset and alias grids need the same two UI pieces to let users
update/delete referencing rules when removing or renaming an object. Add
them as reusable components.
FirewallObjectRemove extends ConfirmRemoveDialog with a
keep/disable/drop selector that sets 'dangling-references',
FirewallUpdateReferences is a no/yes/force selector that submits
'update-references'.
Callers will be added in subsequent commits.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/grid/FirewallObjectCommon.js | 143 ++++++++++++++++++++++
2 files changed, 144 insertions(+)
create mode 100644 www/manager6/grid/FirewallObjectCommon.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index d2ea786b..e211ad40 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -98,6 +98,7 @@ JSSRC= \
form/TagFieldSet.js \
form/IsoSelector.js \
grid/BackupView.js \
+ grid/FirewallObjectCommon.js \
grid/FirewallAliases.js \
grid/FirewallOptions.js \
grid/FirewallRules.js \
diff --git a/www/manager6/grid/FirewallObjectCommon.js b/www/manager6/grid/FirewallObjectCommon.js
new file mode 100644
index 00000000..75c012b5
--- /dev/null
+++ b/www/manager6/grid/FirewallObjectCommon.js
@@ -0,0 +1,143 @@
+Ext.define('PVE.window.FirewallObjectRemove', {
+ extend: 'Proxmox.window.ConfirmRemoveDialog',
+ alias: 'widget.pveFirewallObjectRemove',
+
+ width: 500,
+
+ keepTip: undefined,
+ disableTip: undefined,
+ dropTip: undefined,
+
+ initComponent: function () {
+ let me = this;
+
+ me.additionalItems = [
+ {
+ xtype: 'pveFirewallDanglingReferences',
+ keepTip: me.keepTip,
+ disableTip: me.disableTip,
+ dropTip: me.dropTip,
+ },
+ ];
+
+ me.callParent();
+ },
+
+ getParams: function () {
+ let me = this;
+
+ // do not write into the shared config default, it outlives this dialog
+ me.params = Ext.applyIf(
+ { 'dangling-references': me.down('pveFirewallDanglingReferences').getValue() },
+ me.params,
+ );
+
+ return me.callParent();
+ },
+});
+
+Ext.define('PVE.form.FirewallUpdateReferences', {
+ extend: 'Ext.form.FieldContainer',
+ alias: 'widget.pveFirewallUpdateReferences',
+
+ fieldLabel: gettext('Update references'),
+ labelWidth: 150,
+ layout: { type: 'hbox', pack: 'end' },
+
+ // submitted parameter
+ name: 'update-references',
+ value: 'no',
+
+ noTip: undefined,
+ yesTip: undefined,
+ forceTip: undefined,
+
+ initComponent: function () {
+ let me = this;
+
+ for (const tip of ['noTip', 'yesTip', 'forceTip']) {
+ if (me[tip] === undefined) {
+ throw new Error(`${tip} is undefined, cannot initialize component`);
+ }
+ }
+
+ me.items = [
+ {
+ xtype: 'segmentedbutton',
+ allowMultiple: false,
+ value: me.value,
+ items: [
+ { text: gettext('No'), value: 'no', tooltip: me.noTip },
+ { text: gettext('Yes'), value: 'yes', tooltip: me.yesTip },
+ { text: gettext('Force'), value: 'force', tooltip: me.forceTip },
+ ],
+ listeners: {
+ change: (btn, val) => me.down('hiddenfield').setValue(val),
+ },
+ },
+ {
+ xtype: 'hiddenfield',
+ name: me.name,
+ value: me.value,
+ isDirty: () => false,
+ },
+ ];
+
+ me.callParent();
+ },
+});
+
+Ext.define('PVE.form.FirewallDanglingReferences', {
+ extend: 'Ext.form.FieldContainer',
+ alias: 'widget.pveFirewallDanglingReferences',
+
+ fieldLabel: gettext('Referencing rules'),
+ labelWidth: 200,
+ layout: { type: 'hbox', pack: 'end' },
+
+ // submitted parameter
+ name: 'dangling-references',
+ value: 'disable',
+
+ keepTip: undefined,
+ disableTip: undefined,
+ dropTip: undefined,
+
+ getValue: function () {
+ return this.down('hiddenfield').getValue();
+ },
+
+ initComponent: function () {
+ let me = this;
+
+ for (const tip of ['keepTip', 'disableTip', 'dropTip']) {
+ if (me[tip] === undefined) {
+ throw new Error(`${tip} is undefined, cannot initialize component`);
+ }
+ }
+
+ me.items = [
+ {
+ xtype: 'segmentedbutton',
+ allowMultiple: false,
+ value: me.value,
+ items: [
+ { text: gettext('Keep'), value: 'keep', tooltip: me.keepTip },
+ { text: gettext('Disable'), value: 'disable', tooltip: me.disableTip },
+ { text: gettext('Delete'), value: 'drop', tooltip: me.dropTip },
+ ],
+ listeners: {
+ change: (btn, val) => me.down('hiddenfield').setValue(val),
+ },
+ },
+ {
+ xtype: 'hiddenfield',
+ name: me.name,
+ value: me.value,
+ isDirty: () => false,
+ },
+ ];
+
+ me.callParent();
+ },
+});
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-manager v3 12/16] ui: firewall: ipset: add controls to update/delete references on edit
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (10 preceding siblings ...)
2026-09-25 9:42 ` SPAM: [PATCH pve-manager v3 11/16] ui: firewall: add common widgets for deleting and updating references Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-manager v3 13/16] ui: firewall: aliases: " Arthur Bied-Charreton
` (4 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Add the shared 'update-references' selector to the update dialog, so a
rename can (force-)update all references to the ipset, and replace the
plain remove button with a confirmation dialog to handle dangling
references following the ipset deletion.
StdRemoveButton cannot pass the extra 'dangling-references' parameter,
so the remove button is switched to a plain button that opens the
shared FirewallObjectRemove dialog.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/panel/IPSet.js | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/www/manager6/panel/IPSet.js b/www/manager6/panel/IPSet.js
index 9e0203a0..7dc3668f 100644
--- a/www/manager6/panel/IPSet.js
+++ b/www/manager6/panel/IPSet.js
@@ -91,6 +91,16 @@ Ext.define('PVE.IPSetList', {
value: rec.data.comment,
fieldLabel: gettext('Comment'),
},
+ {
+ xtype: 'pveFirewallUpdateReferences',
+ noTip: gettext(
+ 'Do not update referencing rules when renaming this IPSet. Note that this will render any reference to this IPSet invalid.',
+ ),
+ yesTip: gettext('Update referencing rules to point to the new IPSet name.'),
+ forceTip: gettext(
+ 'Update referencing rules to point to the new IPSet name, overwriting any pre-existing IPSet with the target name. Can be used in case a failed rename left the cluster configuration in a partially-updated state.',
+ ),
+ },
],
});
win.show();
@@ -134,11 +144,30 @@ Ext.define('PVE.IPSetList', {
},
});
- me.removeBtn = Ext.create('Proxmox.button.StdRemoveButton', {
+ me.removeBtn = Ext.create('Proxmox.button.Button', {
+ text: gettext('Remove'),
+ disabled: true,
+ dangerous: true,
enableFn: (rec) => canEdit,
selModel: sm,
- baseurl: me.base_url + '/',
- callback: reload,
+ handler: function (btn, event, rec) {
+ Ext.create('PVE.window.FirewallObjectRemove', {
+ item: { id: rec.data.name },
+ url: me.base_url + '/' + rec.data.name,
+ text: Ext.String.format(
+ gettext("Are you sure you want to remove IPSet '{0}'?"),
+ rec.data.name,
+ ),
+ keepTip: gettext(
+ 'Keep referencing rules untouched. Some firewall configurations may end up with dangling references, which are dropped from the generated ruleset.',
+ ),
+ disableTip: gettext(
+ 'Disable all firewall rules referencing this IPSet, keeping them in the configuration.',
+ ),
+ dropTip: gettext('Delete all firewall rules referencing this IPSet.'),
+ apiCallDone: reload,
+ }).show();
+ },
});
Ext.apply(me, {
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-manager v3 13/16] ui: firewall: aliases: add controls to update/delete references on edit
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (11 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-manager v3 12/16] ui: firewall: ipset: add controls to update/delete references on edit Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-manager v3 14/16] ui: sdn: apply: add control for dangling IPSet references Arthur Bied-Charreton
` (3 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Add the shared 'update-references' selector to the update dialog, so a
rename can (force-)update all references to the alias, and replace the
plain remove button with a confirmation dialog to handle dangling
references following the alias deletion.
StdRemoveButton cannot pass the extra 'dangling-references' parameter,
so the remove button is switched to a plain button that opens the
shared FirewallObjectRemove dialog.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/grid/FirewallAliases.js | 80 ++++++++++++++++++++--------
1 file changed, 58 insertions(+), 22 deletions(-)
diff --git a/www/manager6/grid/FirewallAliases.js b/www/manager6/grid/FirewallAliases.js
index 06801d33..d4dacc7d 100644
--- a/www/manager6/grid/FirewallAliases.js
+++ b/www/manager6/grid/FirewallAliases.js
@@ -20,27 +20,43 @@ Ext.define('PVE.FirewallAliasEdit', {
me.method = 'PUT';
}
+ let items = [
+ {
+ xtype: 'textfield',
+ name: me.isCreate ? 'name' : 'rename',
+ fieldLabel: gettext('Name'),
+ allowBlank: false,
+ },
+ {
+ xtype: 'textfield',
+ name: 'cidr',
+ fieldLabel: gettext('IP/CIDR'),
+ allowBlank: false,
+ },
+ {
+ xtype: 'textfield',
+ name: 'comment',
+ fieldLabel: gettext('Comment'),
+ },
+ ];
+ if (!me.isCreate) {
+ items.push({
+ xtype: 'pveFirewallUpdateReferences',
+ noTip: gettext(
+ 'Do not update referencing rules and IPSets when renaming this alias. Note that this will render any reference to this alias invalid.',
+ ),
+ yesTip: gettext(
+ 'Update referencing rules and IPSets to point to the new alias name.',
+ ),
+ forceTip: gettext(
+ 'Update referencing rules and IPSets to point to the new alias name, overwriting any pre-existing alias with the target name. Can be used in case a failed rename left the cluster configuration in a partially-updated state.',
+ ),
+ });
+ }
+
let ipanel = Ext.create('Proxmox.panel.InputPanel', {
isCreate: me.isCreate,
- items: [
- {
- xtype: 'textfield',
- name: me.isCreate ? 'name' : 'rename',
- fieldLabel: gettext('Name'),
- allowBlank: false,
- },
- {
- xtype: 'textfield',
- name: 'cidr',
- fieldLabel: gettext('IP/CIDR'),
- allowBlank: false,
- },
- {
- xtype: 'textfield',
- name: 'comment',
- fieldLabel: gettext('Comment'),
- },
- ],
+ items,
});
Ext.apply(me, {
@@ -158,15 +174,35 @@ Ext.define('PVE.FirewallAliases', {
},
});
- me.removeBtn = Ext.create('Proxmox.button.StdRemoveButton', {
+ me.removeBtn = Ext.create('Proxmox.button.Button', {
+ text: gettext('Remove'),
disabled: true,
+ dangerous: true,
selModel: sm,
enableFn: (rec) =>
!!caps.vms['VM.Config.Network'] ||
!!caps.dc['Sys.Modify'] ||
!!caps.nodes['Sys.Modify'],
- baseurl: me.base_url + '/',
- callback: reload,
+ handler: function (btn, event, rec) {
+ Ext.create('PVE.window.FirewallObjectRemove', {
+ item: { id: rec.data.name },
+ url: me.base_url + '/' + rec.data.name,
+ text: Ext.String.format(
+ gettext("Are you sure you want to remove alias '{0}'"),
+ rec.data.name,
+ ),
+ keepTip: gettext(
+ 'Keep referencing rules untouched. Some firewall configurations may end up with dangling references, which are dropped from the generated ruleset.',
+ ),
+ disableTip: gettext(
+ 'Disable all firewall rules referencing this alias. IPSet members referencing it are removed, as they cannot be disabled.',
+ ),
+ dropTip: gettext(
+ 'Delete all firewall rules and IPSet members referencing this alias.',
+ ),
+ apiCallDone: reload,
+ }).show();
+ },
});
Ext.apply(me, {
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-manager v3 14/16] ui: sdn: apply: add control for dangling IPSet references
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (12 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-manager v3 13/16] ui: firewall: aliases: " Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-manager v3 15/16] ui: guest destroy: use let for non-constant variable bindings Arthur Bied-Charreton
` (2 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Applying an SDN configuration that removes a VNet also removes the
four IPSets auto-generated for it [0], so rules referencing those no
longer resolve and are dropped from the generated ruleset.
Replace the plain confirmation message box with a window carrying the
shared selector, so those rules can be disabled or deleted in the same
step. The Apply button first queries the pending VNets, and the window
only shows the selector if the apply actually removes one, since there
is nothing to decide otherwise.
[0] https://pve.proxmox.com/wiki/Software-Defined_Network#pvesdn_firewall_integration
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/sdn/StatusView.js | 105 ++++++++++++++++++++++++++-------
1 file changed, 85 insertions(+), 20 deletions(-)
diff --git a/www/manager6/sdn/StatusView.js b/www/manager6/sdn/StatusView.js
index fada5041..79ca815c 100644
--- a/www/manager6/sdn/StatusView.js
+++ b/www/manager6/sdn/StatusView.js
@@ -1,3 +1,74 @@
+Ext.define('PVE.sdn.ApplyWindow', {
+ extend: 'Proxmox.window.Edit',
+ alias: 'widget.pveSdnApplyWindow',
+
+ title: gettext('Apply SDN Configuration'),
+
+ url: '/cluster/sdn',
+ method: 'PUT',
+ isCreate: true,
+ submitText: gettext('Apply'),
+ width: 600,
+
+ // IDs of the VNets that this apply removes, empty if it removes none
+ removedVnets: [],
+
+ initComponent: function () {
+ let me = this;
+
+ me.items = [
+ {
+ xtype: 'displayfield',
+ value: gettext(
+ 'Applying pending SDN changes will also apply any pending local node network changes.',
+ ),
+ },
+ ];
+
+ // the auto-generated IPSets of a removed VNet go away with it, so only then is there
+ // anything to decide about the rules referencing them
+ if (me.removedVnets.length !== 0) {
+ let removedHint = Ext.htmlEncode(
+ Ext.String.format(
+ gettext('This removes the following VNet(s): {0}.'),
+ me.removedVnets.join(', '),
+ ),
+ );
+ let helpLink = Ext.htmlEncode(
+ Proxmox.Utils.get_help_link('pvesdn_firewall_integration'),
+ );
+ let helpLabel = Ext.htmlEncode(gettext('their auto-generated IPSets'));
+ let refsHint = Ext.String.format(
+ gettext('Firewall rules referencing {0} will no longer resolve.'),
+ `<a target="_blank" href="${helpLink}">${helpLabel}</a>`,
+ );
+
+ me.items.push(
+ {
+ xtype: 'displayfield',
+ userCls: 'pmx-hint',
+ value: `${removedHint}<br>${refsHint}`,
+ },
+ {
+ xtype: 'pveFirewallDanglingReferences',
+ name: 'dangling-ipset-references',
+ value: 'keep',
+ labelWidth: 200,
+ keepTip: gettext(
+ 'Keep referencing rules. They are dropped from the generated ruleset and shown as invalid until the VNet is recreated.',
+ ),
+ disableTip: gettext(
+ 'Disable referencing rules, keeping them in the firewall configuration.',
+ ),
+ dropTip: gettext('Delete referencing rules.'),
+ },
+ );
+ }
+
+ me.callParent();
+ },
+});
+
Ext.define(
'PVE.sdn.StatusView',
{
@@ -45,26 +116,20 @@ Ext.define(
{
text: gettext('Apply'),
handler: function () {
- Ext.Msg.show({
- title: gettext('Confirm'),
- icon: Ext.Msg.QUESTION,
- msg: gettext(
- 'Applying pending SDN changes will also apply any pending local node network changes. Proceed?',
- ),
- buttons: Ext.Msg.YESNO,
- callback: function (btn) {
- if (btn === 'yes') {
- Proxmox.Utils.API2Request({
- url: '/cluster/sdn/',
- method: 'PUT',
- waitMsgTarget: me,
- failure: (response) =>
- Ext.Msg.alert(
- gettext('Error'),
- response.htmlStatus,
- ),
- });
- }
+ Proxmox.Utils.API2Request({
+ url: '/cluster/sdn/vnets',
+ method: 'GET',
+ params: { pending: 1 },
+ waitMsgTarget: me,
+ failure: (response) =>
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+ success: function (response) {
+ Ext.create('PVE.sdn.ApplyWindow', {
+ removedVnets: response.result.data
+ .filter((vnet) => vnet.state === 'deleted')
+ .map((vnet) => vnet.vnet),
+ autoShow: true,
+ });
},
});
},
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-manager v3 15/16] ui: guest destroy: use let for non-constant variable bindings
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (13 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-manager v3 14/16] ui: sdn: apply: add control for dangling IPSet references Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 9:42 ` [PATCH pve-manager v3 16/16] ui: guest destroy: add control for dangling IPSet references Arthur Bied-Charreton
2026-09-25 11:05 ` SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
See [JS Style Guide].
[JS Style Guide] https://pve.proxmox.com/wiki/Javascript_Style_Guide#Variables
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/window/SafeDestroyGuest.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/www/manager6/window/SafeDestroyGuest.js b/www/manager6/window/SafeDestroyGuest.js
index ee0c649c..45b277e5 100644
--- a/www/manager6/window/SafeDestroyGuest.js
+++ b/www/manager6/window/SafeDestroyGuest.js
@@ -37,10 +37,10 @@ Ext.define('PVE.window.SafeDestroyGuest', {
getParams: function () {
let me = this;
- const purgeCheckbox = me.lookupReference('purgeCheckbox');
+ let purgeCheckbox = me.lookupReference('purgeCheckbox');
me.params.purge = purgeCheckbox.checked ? 1 : 0;
- const destroyUnreferencedCheckbox = me.lookupReference('destroyUnreferencedCheckbox');
+ let destroyUnreferencedCheckbox = me.lookupReference('destroyUnreferencedCheckbox');
me.params['destroy-unreferenced-disks'] = destroyUnreferencedCheckbox.checked ? 1 : 0;
return me.callParent();
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH pve-manager v3 16/16] ui: guest destroy: add control for dangling IPSet references
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (14 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-manager v3 15/16] ui: guest destroy: use let for non-constant variable bindings Arthur Bied-Charreton
@ 2026-09-25 9:42 ` Arthur Bied-Charreton
2026-09-25 11:05 ` SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 9:42 UTC (permalink / raw)
To: pve-devel
Destroying a guest releases its IPAM entries, so the auto-generated
'guest-ipam-<vmid>' IPSet disappears with it. Rules referencing that
IPSet do not resolve anymore and are silently dropped from the
generated ruleset.
Add the shared selector to the destroy dialog, so those rules can be
disabled or deleted along with the guest, next to a hint linking to
the SDN firewall integration documentation. 'keep' remains the
default, matching the API.
The 'note' config only renders a single hint, so it is replaced by
two display fields among the additional items.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
www/manager6/window/SafeDestroyGuest.js | 44 ++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/www/manager6/window/SafeDestroyGuest.js b/www/manager6/window/SafeDestroyGuest.js
index 45b277e5..45099342 100644
--- a/www/manager6/window/SafeDestroyGuest.js
+++ b/www/manager6/window/SafeDestroyGuest.js
@@ -32,7 +32,46 @@ Ext.define('PVE.window.SafeDestroyGuest', {
},
],
- note: gettext('Referenced disks will always be destroyed.'),
+ initComponent: function () {
+ let me = this;
+
+ let helpLink = Ext.htmlEncode(Proxmox.Utils.get_help_link('pvesdn_firewall_integration'));
+ let helpLabel = Ext.htmlEncode(gettext('this guest\'s auto-generated IPSet'));
+ let refsHint = Ext.String.format(
+ gettext('Firewall rules referencing {0} will no longer resolve.'),
+ `<a target="_blank" href="${helpLink}">${helpLabel}</a>`,
+ );
+
+ me.additionalItems = [
+ // build fresh array per instance, pushing would append every time the dialog is opened
+ ...me.additionalItems,
+ {
+ xtype: 'displayfield',
+ userCls: 'pmx-hint',
+ value: gettext('Referenced disks will always be destroyed.'),
+ },
+ {
+ xtype: 'displayfield',
+ userCls: 'pmx-hint',
+ value: refsHint,
+ },
+ {
+ xtype: 'pveFirewallDanglingReferences',
+ reference: 'firewallDanglingReferencesButton',
+ value: 'keep',
+ labelWidth: 200,
+ keepTip: gettext(
+ 'Keep referencing rules. They are dropped from the generated ruleset and shown as invalid until a guest with the same VMID is recreated.',
+ ),
+ disableTip: gettext(
+ 'Disable referencing rules, keeping them in the firewall configuration.',
+ ),
+ dropTip: gettext('Delete referencing rules.'),
+ },
+ ];
+
+ me.callParent();
+ },
getParams: function () {
let me = this;
@@ -43,6 +82,9 @@ Ext.define('PVE.window.SafeDestroyGuest', {
let destroyUnreferencedCheckbox = me.lookupReference('destroyUnreferencedCheckbox');
me.params['destroy-unreferenced-disks'] = destroyUnreferencedCheckbox.checked ? 1 : 0;
+ let danglingRefs = me.lookupReference('firewallDanglingReferencesButton');
+ me.params['dangling-ipset-references'] = danglingRefs.getValue();
+
return me.callParent();
},
});
--
2.47.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away
2026-09-25 9:42 SPAM: [PATCH container/firewall/manager/network/qemu-server v3 00/16] handle dangling references when firewall objects go away Arthur Bied-Charreton
` (15 preceding siblings ...)
2026-09-25 9:42 ` [PATCH pve-manager v3 16/16] ui: guest destroy: add control for dangling IPSet references Arthur Bied-Charreton
@ 2026-09-25 11:05 ` Arthur Bied-Charreton
16 siblings, 0 replies; 18+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-25 11:05 UTC (permalink / raw)
To: pve-devel
On Fri, Sep 25, 2026 at 11:42:14AM +0200, Arthur Bied-Charreton wrote:
> Renaming or deleting a firewall object (an IPSet or an alias) that rules
> still reference leaves those references dangling. The firewall fails to
> parse the affected rules and drops them from the generated ruleset, so
> an edit in one place can disable a whole set of rules somewhere. This
> is especially bad in the rename case, where one might reasonably expect
> the references to follow the new object name.
>
> This series makes the affected operations offer to deal with the
> references instead of leaving them behind.
>
> [...]
forgot to add: there are pre-built packages on sani under
packages/abied-charreton/firewall-object-references/v3
^ permalink raw reply [flat|nested] 18+ messages in thread