[PATCH container/docs/firewall/manager/network/qemu-server v5 00/27] handle dangling references when firewall objects go away

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

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/v5/

Build dependencies:

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

Runtime dependencies:

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

Changes since [v4]:
- Rebase onto current master

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

[v4] https://lore.proxmox.com/pve-devel/20261022124708.429069-1-a.bied-charreton@proxmox.com/
[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 | 224 +++++++++---
 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, 1476 insertions(+), 255 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, 2191 insertions(+), 321 deletions(-)

-- 
Generated by murpp 0.12.1