[PATCH firewall/manager v2 0/9] allow updating references when renaming/deleting firewall objects

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

Renaming or deleting a firewall object (IPSet or alias) that is still
referenced currently leaves dangling references behind. The firewall
then fails to parse the affected rules, which become inactive without
any error visible from the UI.

This is especially dangerous on renames, where a user might reasonably
expect references to follow the rename, and could walk away believing
a set of rules is active while it has been effectively disabled.

This series introduces opt-in functionality allowing the related
firewall endpoints to keep references consistent.

- rename (POST /ipset/, PUT /aliases/{name}) gains an
  'update-references' option to rewrite all references to the new name.
- delete (DELETE /ipset/{name}, DELETE /aliases/{name}) gains a
  'delete-references' option to drop the referencing rules along with
  the object.

In the UI, 'update-references' is represented as a segmented button in
the edit panels allowing to set 'no', 'yes' and 'force' (see below).
'delete-references' is a check box in the remove confirm dialog.

The reference rewriting is centralized in a new helper that scans rules
(also in security groups) and IPSet members. When called for a cluster
object, it also updates references in downstream configs (guest, host,
vnet) across the cluster, since those may reference cluster-level
objects.

On a cluster rename, the new object is persisted before its references
are rewritten, and the old object is removed only afterwards. At no
point does an on-disk state contain a reference that is not defined,
which would otherwise make a concurrent firewall compilation drop the
affected rule. The trade-off is that an interrupted cluster rename
leaves both the old and new object defined, possibly with references
to both in downstream configs.

In order to have something standing between the admin and manually
fixing references across a whole cluster's worth of configs in case of
mid-rename failure, the rename can be finished by re-issuing it. The
'update-references' option is an enum, 'no' for opting out, 'yes' for
opting in, and 'force' for forcing the rename even if an object already
exists with the target name, which is the state a partial cluster rename
would leave the config in.

Only cluster objects fan out, host and vnet configs have no IPSet/alias
section of their own, and SDN-generated (sdn/) IPSets are regenerated
from the SDN config and never persisted, so neither is touched.

Future work:

- As noted by Stefan in the v1 review [0], IPSets are auto-generated for
  SDN objects. Those IPSets can also be referenced in user-managed
  configs and would therefore benefit from a similar feature for the
  SDN object edit/delete paths. It should be relatively straightforward
  to extend the shared helper (update_refs, commit 1 in pve-firewall)
  to also handle SDN references.

Changes since v1 (thanks @Stefan):

- Fix timing issue where on rename, the cluster fan-out would update
  downstream references to use the new name before persisting it in the
  cluster config. This would cause concurrent firewall compilations to
  fail to parse some rules and drop them. [2]

- Scan vnet configs for references as well [1]

- Generalize the cluster fan-out logic for renaming/deleting object
  references, and move all shared functionality into the Helpers
  module. [2]

- Make the 'update-references' option an enum to allow retrying a
  partial rename caused by a failure during cluster fan-out.

- Rename the filter_map helper to map_items. [1]

- Use POD for doc comments. [1]

[0] https://lore.proxmox.com/all/e7348b76-a215-4189-ade9-a7c5bbbd26cf@proxmox.com/
[1] https://lore.proxmox.com/all/c161fc57-1d8e-4340-9d2c-6621d1c39af8@proxmox.com/
[2] https://lore.proxmox.com/all/055351eb-f575-46c3-ac37-01925306c449@proxmox.com/


pve-firewall:

Arthur Bied-Charreton (6):
  api: helpers: add helper to update firewall object references
  api: ipset: add option to update references on edit
  api: ipset: add option to GC references on delete
  api: aliases: add option to update references on edit
  api: aliases: add option to GC references on delete
  firewall: tests: add tests for object reference update logic

 src/PVE/API2/Firewall/Aliases.pm |  51 +++++++-
 src/PVE/API2/Firewall/IPSet.pm   |  71 +++++++++--
 src/PVE/Firewall/Helpers.pm      | 195 +++++++++++++++++++++++++++++++
 test/Makefile                    |   1 +
 test/referenceupdatetests.pl     | 178 ++++++++++++++++++++++++++++
 5 files changed, 482 insertions(+), 14 deletions(-)
 create mode 100755 test/referenceupdatetests.pl


pve-manager:

Arthur Bied-Charreton (3):
  ui: firewall: add common widgets for deleting and updating references
  ui: firewall: ipset: add controls to update/delete references on edit
  ui: firewall: aliases: add controls to update/delete references on
    edit

 www/manager6/Makefile                     |   1 +
 www/manager6/grid/FirewallAliases.js      |  74 +++++++++++-----
 www/manager6/grid/FirewallObjectCommon.js | 100 ++++++++++++++++++++++
 www/manager6/panel/IPSet.js               |  31 ++++++-
 4 files changed, 181 insertions(+), 25 deletions(-)
 create mode 100644 www/manager6/grid/FirewallObjectCommon.js


Summary over all repositories:
  9 files changed, 663 insertions(+), 39 deletions(-)

-- 
Generated by murpp 0.12.0