public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH container/docs/firewall/manager/network/qemu-server v4 00/27] handle dangling references when firewall objects go away
Date: Thu, 22 Oct 2026 14:46:41 +0200	[thread overview]
Message-ID: <20261022124708.429069-1-a.bied-charreton@proxmox.com> (raw)

Renaming or deleting a firewall object (an IPSet, alias or security
group) that rules still reference leaves those references dangling.

The firewall fails to parse the affected rules and drops them from the
generated ruleset. For security groups, the reference reaches the
backend, which creates empty chains on iptables, and causes the ruleset
to fail to load on nftables.

This is especially bad in the rename case, where one might reasonably
expect the references to follow the new object name and walk away
thinking the firewall rules are active.

This series makes the affected operations offer to deal with the
references instead of leaving them behind.

pve-firewall gains a shared helper, update_refs(), whcih finds
references in rules, security groups and IPset members and applies one
of three actions: 'rename' rewrites the reference to point at the new
name, 'disable' disables the referencing rules, and 'drop' removes them.
An IPSet member has no disabled state, so 'disable' removes members as
well.

These options are added to the API as follows:

POST    .../firewall/ipset              dangling-references         keep|disable|drop|rename|force-rename
PUT     .../firewall/aliases/{name}     dangling-references         keep|disable|drop|rename|force-rename
POST    /cluster/firewall/groups        dangling-references         keep|disable|drop|rename|force-rename
DELETE  .../ipset/{name}                dangling-references         keep|disable|drop
DELETE  .../aliases/{name}              dangling-references         keep|disable|drop
DELETE  /cluster/firewall/groups/{name} dangling-references         keep|disable|drop
PUT     /cluster/sdn                    dangling-ipset-references   keep|disable|drop
DELETE  /nodes/{node}/qemu/{vmid}       dangling-ipset-references   keep|disable|drop
DELETE  /nodes/{node}/lxc/{vmid}        dangling-ipset-references   keep|disable|drop

For a cluster object, the helper also walks every downstream
config in the cluster (guest, host and vnet), locking and saving each
one. On a rename, the new object is persisted before its references are
rewritten, so a concurrent compilation does not observe a reference to
an object that does not exist yet. If a cluster-wide rewrite is
interrupted part way, passing 'force-rename' resumes it (without this,
the next attempt would fail due to the target name already existing in
the config).

While these options allow to trigger changes to the firewall
configurations from other endpoints requiring different permissions,
like guest destroy and SDN apply, they do not require any additional
permissions, see full explanation here [0].

The API keeps backwards-compatible defaults: 'keep' everywhere, with the
exception of POST /cluster/firewall/groups, which was already renaming
references by default and keeps doing so. The UI however defaults
towards renaming/dropping dangling references, as this is expected to
be the normal case.

Pre-built packages on sani under:

abied-charreton/firewall-object-references/v4/

Build dependencies:

    pve-container, qemu-server <- pve-firewall
    pve-manager <- pve-docs

Runtime dependencies:

    pve-network, pve-manager <- pve-firewall

Changes since [v3]
- Rename the option from 'update-references' (no|yes|force) to
  'dangling-references' (keep|disable|drop|rename|force-rename) so the
  parameter name stays the same everywhere.
- Add 'drop' and 'disable' actions to the rename paths
- Handle security groups as well (the rename path was missing vnet
  configs and the delete path did nothing, fixes #8176 [1]).
- Move all helpers related to firewall object references out of
  PVE::Firewall::Helpers into PVE::Firewall::References.
- update_refs() takes a callback for saving the cluster config so it
  can decide when the object has to be persisted instead of relying on
  the caller for correctness.
- Run every rename and delete as a task and log every change made to the
  configs to the task log for better observability.
- Drop 'force-rename' from the GUI and document it as an escape hatch
  for partially applied renames
- Trim the API option descriptions and move the reasoning into a
  pve-docs patch.
- Merge the two ExtJS widgets into one ComboGrid taking the list of
  actions from the caller and showing each descriptoin directly in
  a column instead of using a tooltip.
- Preselect 'rename' and 'drop' in the UI for edits and removals,
  respectively. API defaults keep doing the same as they were before.
- Move the pve-firewall tests to src/test
- Move selector to advanced section in ipset/alias edit/remove dialogs

[v3] https://lore.proxmox.com/pve-devel/20260925094230.844917-1-a.bied-charreton@proxmox.com/
[v2] https://lore.proxmox.com/pve-devel/20260818133413.450776-1-a.bied-charreton@proxmox.com/
[v1] https://lore.proxmox.com/pve-devel/20260407073658.90818-1-a.bied-charreton@proxmox.com/

[0] https://lore.proxmox.com/pve-devel/awaoshjzbr7adisjngsrcts4zs3hxgoxw2lgcoq66gtiap3al2@mpxrbmtyfcwj/
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=8176


pve-firewall:

Arthur Bied-Charreton (14):
  firewall: move object reference helpers to their own module
  firewall: references: add helpers to update object references
  firewall: references: log the references an object update touched
  firewall: references: add wrappers for the SDN-generated IPSets
  firewall: parser: do not log errors for disabled rules
  firewall: ipset: add option to handle dangling references on rename
  firewall: ipset: add option to handle dangling references on delete
  firewall: aliases: add option to handle dangling references on rename
  firewall: aliases: add option to handle dangling references on delete
  firewall: api: run reference updates as tasks
  firewall: tests: add tests for object reference updates
  firewall: references: handle security groups
  firewall: groups: add option to handle dangling references on rename
  fix #8176: firewall: groups: handle dangling references on delete

 src/PVE/API2/Firewall/Aliases.pm | 218 +++++++++---
 src/PVE/API2/Firewall/Cluster.pm |   6 +-
 src/PVE/API2/Firewall/Groups.pm  | 199 ++++++-----
 src/PVE/API2/Firewall/IPSet.pm   | 215 +++++++++---
 src/PVE/API2/Firewall/Rules.pm   |  53 ++-
 src/PVE/API2/Firewall/VM.pm      |   7 +-
 src/PVE/Firewall.pm              |   2 +-
 src/PVE/Firewall/Helpers.pm      |  38 ---
 src/PVE/Firewall/Makefile        |   2 +-
 src/PVE/Firewall/References.pm   | 570 +++++++++++++++++++++++++++++++
 src/test/Makefile                |   2 +-
 src/test/test_update_refs.pl     | 413 ++++++++++++++++++++++
 12 files changed, 1473 insertions(+), 252 deletions(-)
 create mode 100644 src/PVE/Firewall/References.pm
 create mode 100755 src/test/test_update_refs.pl


pve-network:

Arthur Bied-Charreton (1):
  apply: add option to handle dangling references on VNet deletion

 src/PVE/API2/Network/SDN.pm | 38 +++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)


qemu-server:

Arthur Bied-Charreton (1):
  api: destroy_vm: add option to handle dangling IPSet references

 src/PVE/API2/Qemu.pm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)


pve-container:

Arthur Bied-Charreton (1):
  api: destroy_vm: add option to handle dangling IPSet references

 src/PVE/API2/LXC.pm | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)


pve-docs:

Arthur Bied-Charreton (2):
  firewall: remove trailing spaces
  firewall: document renaming and removing objects

 pve-firewall.adoc | 76 +++++++++++++++++++++++++++++++++++++++++++----
 pvesdn.adoc       |  9 ++++--
 2 files changed, 76 insertions(+), 9 deletions(-)


pve-manager:

Arthur Bied-Charreton (8):
  ui: firewall: add a common widget for handling dangling references
  ui: firewall: ipset: add controls for dangling references
  ui: firewall: aliases: add controls for dangling references
  ui: sdn: apply: add control for dangling IPSet references
  ui: guest destroy: use let for non-constant variable bindings
  ui: guest destroy: add control for dangling IPSet references
  ui: firewall: show the task for reference updates
  ui: firewall: groups: add controls for dangling references

 www/manager6/Makefile                     |   1 +
 www/manager6/Utils.js                     |   6 +
 www/manager6/dc/SecurityGroups.js         |  81 ++++++++++++-
 www/manager6/grid/FirewallAliases.js      | 125 ++++++++++++++++----
 www/manager6/grid/FirewallObjectCommon.js | 136 ++++++++++++++++++++++
 www/manager6/panel/IPSet.js               |  84 +++++++++++--
 www/manager6/sdn/StatusView.js            | 118 +++++++++++++++----
 www/manager6/window/SafeDestroyGuest.js   |  69 ++++++++++-
 8 files changed, 563 insertions(+), 57 deletions(-)
 create mode 100644 www/manager6/grid/FirewallObjectCommon.js


Summary over all repositories:
  25 files changed, 2188 insertions(+), 318 deletions(-)

-- 
Generated by murpp 0.12.1




             reply	other threads:[~2026-10-22 12:48 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 12:46 Arthur Bied-Charreton [this message]
2026-10-22 12:46 ` [PATCH pve-firewall v4 01/27] firewall: move object reference helpers to their own module Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 02/27] firewall: references: add helpers to update object references Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 03/27] firewall: references: log the references an object update touched Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 04/27] firewall: references: add wrappers for the SDN-generated IPSets Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 05/27] firewall: parser: do not log errors for disabled rules Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 06/27] firewall: ipset: add option to handle dangling references on rename Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 07/27] firewall: ipset: add option to handle dangling references on delete Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 08/27] firewall: aliases: add option to handle dangling references on rename Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 09/27] firewall: aliases: add option to handle dangling references on delete Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 10/27] firewall: api: run reference updates as tasks Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 11/27] firewall: tests: add tests for object reference updates Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 12/27] firewall: references: handle security groups Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 13/27] firewall: groups: add option to handle dangling references on rename Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-firewall v4 14/27] fix #8176: firewall: groups: handle dangling references on delete Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-network v4 15/27] apply: add option to handle dangling references on VNet deletion Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH qemu-server v4 16/27] api: destroy_vm: add option to handle dangling IPSet references Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-container v4 17/27] " Arthur Bied-Charreton
2026-10-22 12:46 ` [PATCH pve-docs v4 18/27] firewall: remove trailing spaces Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-docs v4 19/27] firewall: document renaming and removing objects Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 20/27] ui: firewall: add a common widget for handling dangling references Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 21/27] ui: firewall: ipset: add controls for " Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 22/27] ui: firewall: aliases: " Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 23/27] ui: sdn: apply: add control for dangling IPSet references Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 24/27] ui: guest destroy: use let for non-constant variable bindings Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 25/27] ui: guest destroy: add control for dangling IPSet references Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 26/27] ui: firewall: show the task for reference updates Arthur Bied-Charreton
2026-10-22 12:47 ` [PATCH pve-manager v4 27/27] ui: firewall: groups: add controls for dangling references Arthur Bied-Charreton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022124708.429069-1-a.bied-charreton@proxmox.com \
    --to=a.bied-charreton@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal