[pve-devel] applied: [PATCH manager] ui: pool: dynamic status update in members screen
applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] fix #5847: ui: gettextize several strings in ui
one comment inline On 11/11/24 12:35, Timothy Nicholson wrote: Several strings that should probably also be translated now use the gettext function to be translated. Signed-off-by: Timothy Nicholson --- [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=5847 www/manager6/Workspace.js | 2 +- www/manager6/ceph/OSD.js | 6 +++--- www/manager6/ceph/OSDDetails.js| 2 +- www/manager6/form/VMCPUFlagSelector.js | 24 www/manager6/qemu/ProcessorEdit.js | 2 +- www/manager6/window/Settings.js| 6 +++--- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js index 52c66108..56dde151 100644 --- a/www/manager6/Workspace.js +++ b/www/manager6/Workspace.js @@ -388,7 +388,7 @@ Ext.define('PVE.StdWorkspace', { }, }, { - text: 'TFA', + text: gettext('TFA'), is it a good idea to translate this? it's mostly a technical term (which we generally don't translate), but i'd be swayed if there is an actual example where a language want to have that translated... ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 4/4] fix #3893: ui: network: enable bridge_vids field
Signed-off-by: Aaron Lauterer Tested-By: Stefan Hanreich Reviewed-by: Shannon Sterz --- changes since v5: none v4: none v3: none v2: none www/manager6/node/Config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js index d27592ce..7bdfb6d9 100644 --- a/www/manager6/node/Config.js +++ b/www/manager6/node/Config.js @@ -194,6 +194,7 @@ Ext.define('PVE.node.Config', { showApplyBtn: true, groups: ['services'], nodename: nodename, + bridge_set_vids: true, onlineHelp: 'sysadmin_network_configuration', }, { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server v3 17/34] backup: move cleanup of fleecing images to cleanup method
On November 7, 2024 5:51 pm, Fiona Ebner wrote: > TPM drives are already detached there and it's better to group > these things together. > > Signed-off-by: Fiona Ebner > --- > > No changes in v3. > > PVE/VZDump/QemuServer.pm | 25 + > 1 file changed, 9 insertions(+), 16 deletions(-) > > diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm > index 012c9210..b2ced154 100644 > --- a/PVE/VZDump/QemuServer.pm > +++ b/PVE/VZDump/QemuServer.pm > @@ -690,7 +690,6 @@ sub archive_pbs { > > # get list early so we die on unkown drive types before doing anything > my $devlist = _get_task_devlist($task); > -my $use_fleecing; > > $self->enforce_vm_running_for_backup($vmid); > $self->{qmeventd_fh} = PVE::QemuServer::register_qmeventd_handle($vmid); > @@ -721,7 +720,7 @@ sub archive_pbs { > > my $is_template = > PVE::QemuConfig->is_template($self->{vmlist}->{$vmid}); > > - $use_fleecing = check_and_prepare_fleecing( > + $task->{'use-fleecing'} = check_and_prepare_fleecing( > $self, $vmid, $opts->{fleecing}, $task->{disks}, $is_template, > $qemu_support); > > my $fs_frozen = $self->qga_fs_freeze($task, $vmid); > @@ -735,7 +734,7 @@ sub archive_pbs { > devlist => $devlist, > 'config-file' => $conffile, > }; > - $params->{fleecing} = JSON::true if $use_fleecing; > + $params->{fleecing} = JSON::true if $task->{'use-fleecing'}; > > if (defined(my $ns = $scfg->{namespace})) { > $params->{'backup-ns'} = $ns; > @@ -784,11 +783,6 @@ sub archive_pbs { > } > $self->restore_vm_power_state($vmid); > > -if ($use_fleecing) { > - detach_fleecing_images($task->{disks}, $vmid); > - cleanup_fleecing_images($self, $task->{disks}); > -} > - > die $err if $err; > } > > @@ -891,7 +885,6 @@ sub archive_vma { > } > > my $devlist = _get_task_devlist($task); > -my $use_fleecing; > > $self->enforce_vm_running_for_backup($vmid); > $self->{qmeventd_fh} = PVE::QemuServer::register_qmeventd_handle($vmid); > @@ -911,7 +904,7 @@ sub archive_vma { > > $attach_tpmstate_drive->($self, $task, $vmid); > > - $use_fleecing = check_and_prepare_fleecing( > + $task->{'use-fleecing'} = check_and_prepare_fleecing( > $self, $vmid, $opts->{fleecing}, $task->{disks}, $is_template, > $qemu_support); > > my $outfh; > @@ -942,7 +935,7 @@ sub archive_vma { > devlist => $devlist > }; > $params->{'firewall-file'} = $firewall if -e $firewall; > - $params->{fleecing} = JSON::true if $use_fleecing; > + $params->{fleecing} = JSON::true if $task->{'use-fleecing'}; > add_backup_performance_options($params, $opts->{performance}, > $qemu_support); > > $qmpclient->queue_cmd($vmid, $backup_cb, 'backup', %$params); > @@ -984,11 +977,6 @@ sub archive_vma { > > $self->restore_vm_power_state($vmid); > > -if ($use_fleecing) { > - detach_fleecing_images($task->{disks}, $vmid); > - cleanup_fleecing_images($self, $task->{disks}); > -} > - > if ($err) { > if ($cpid) { > kill(9, $cpid); > @@ -1132,6 +1120,11 @@ sub cleanup { > > $detach_tpmstate_drive->($task, $vmid); > > +if ($task->{'use-fleecing'}) { > + detach_fleecing_images($task->{disks}, $vmid); > + cleanup_fleecing_images($self, $task->{disks}); > +} > + > if ($self->{qmeventd_fh}) { > close($self->{qmeventd_fh}); > } > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC qemu-server v3 20/34] backup: allow adding fleecing images also for EFI and TPM
Reviewed-by: Fabian Grünbichler but possibly needs a rebase in case the changes from patch #19 are adapted based on my feedback ;) On November 7, 2024 5:51 pm, Fiona Ebner wrote: > For the external backup API, it will be necessary to add a fleecing > image even for small disks like EFI and TPM, because there is no other > place the old data could be copied to when a new guest write comes in. > > Signed-off-by: Fiona Ebner > --- > > Changes in v3: > * adapt to context changes from previous patch > > PVE/VZDump/QemuServer.pm | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm > index 1ebafe6d..b6dcd6cc 100644 > --- a/PVE/VZDump/QemuServer.pm > +++ b/PVE/VZDump/QemuServer.pm > @@ -534,7 +534,7 @@ my sub cleanup_fleecing_images { > } > > my sub allocate_fleecing_images { > -my ($self, $disks, $vmid, $fleecing_storeid, $format) = @_; > +my ($self, $disks, $vmid, $fleecing_storeid, $format, $all_images) = @_; > > die "internal error - no fleecing storage specified\n" if > !$fleecing_storeid; > > @@ -545,7 +545,8 @@ my sub allocate_fleecing_images { > my $n = 0; # counter for fleecing image names > > for my $di ($disks->@*) { > - next if $di->{virtdev} =~ m/^(?:tpmstate|efidisk)\d$/; # too small > to be worth it > + # EFI/TPM are usually too small to be worth it, but it's required > for external providers > + next if !$all_images && $di->{virtdev} =~ > m/^(?:tpmstate|efidisk)\d$/; > if ($di->{type} eq 'block' || $di->{type} eq 'file') { > my $scfg = PVE::Storage::storage_config($self->{storecfg}, > $fleecing_storeid); > my $name = "vm-$vmid-fleece-$n"; > @@ -609,7 +610,7 @@ my sub attach_fleecing_images { > } > > my sub check_and_prepare_fleecing { > -my ($self, $task, $vmid, $fleecing_opts, $disks, $is_template, > $qemu_support) = @_; > +my ($self, $task, $vmid, $fleecing_opts, $disks, $is_template, > $qemu_support, $all_images) = @_; > > # Even if the VM was started specifically for fleecing, it's possible > that the VM is resumed and > # then starts doing IO. For VMs that are not resumed the fleecing images > will just stay empty, > @@ -632,7 +633,8 @@ my sub check_and_prepare_fleecing { > $self->{storecfg}, $fleecing_opts->{storage}); > my $format = scalar(grep { $_ eq 'qcow2' } $valid_formats->@*) ? > 'qcow2' : 'raw'; > > - allocate_fleecing_images($self, $disks, $vmid, > $fleecing_opts->{storage}, $format); > + allocate_fleecing_images( > + $self, $disks, $vmid, $fleecing_opts->{storage}, $format, > $all_images); > attach_fleecing_images($self, $disks, $vmid, $format); > } > > @@ -723,7 +725,7 @@ sub archive_pbs { > my $is_template = > PVE::QemuConfig->is_template($self->{vmlist}->{$vmid}); > > $task->{'use-fleecing'} = check_and_prepare_fleecing( > - $self, $task, $vmid, $opts->{fleecing}, $task->{disks}, > $is_template, $qemu_support); > + $self, $task, $vmid, $opts->{fleecing}, $task->{disks}, > $is_template, $qemu_support, 0); > > my $fs_frozen = $self->qga_fs_freeze($task, $vmid); > > @@ -907,7 +909,7 @@ sub archive_vma { > $attach_tpmstate_drive->($self, $task, $vmid); > > $task->{'use-fleecing'} = check_and_prepare_fleecing( > - $self, $task, $vmid, $opts->{fleecing}, $task->{disks}, > $is_template, $qemu_support); > + $self, $task, $vmid, $opts->{fleecing}, $task->{disks}, > $is_template, $qemu_support, 0); > > my $outfh; > if ($opts->{stdout}) { > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server v3 18/34] backup: cleanup: check if VM is running before issuing QMP commands
On November 7, 2024 5:51 pm, Fiona Ebner wrote: > When the VM is only started for backup, the VM will be stopped at that > point again. While the detach helpers do not warn about errors > currently, that might change in the future. This is also in > preparation for other cleanup QMP helpers that are more verbose about > failure. > > Signed-off-by: Fiona Ebner > --- > > No changes in v3. > > PVE/VZDump/QemuServer.pm | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm > index b2ced154..c46e607c 100644 > --- a/PVE/VZDump/QemuServer.pm > +++ b/PVE/VZDump/QemuServer.pm > @@ -1118,13 +1118,14 @@ sub snapshot { > sub cleanup { > my ($self, $task, $vmid) = @_; > > -$detach_tpmstate_drive->($task, $vmid); > - > -if ($task->{'use-fleecing'}) { > - detach_fleecing_images($task->{disks}, $vmid); > - cleanup_fleecing_images($self, $task->{disks}); > +# If VM was started only for backup, it is already stopped now. > +if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) { > + $detach_tpmstate_drive->($task, $vmid); > + detach_fleecing_images($task->{disks}, $vmid) if > $task->{'use-fleecing'}; > } > > +cleanup_fleecing_images($self, $task->{disks}) if > $task->{'use-fleecing'}; > + > if ($self->{qmeventd_fh}) { > close($self->{qmeventd_fh}); > } > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server v3 22/34] restore: die early when there is no size for a device
On November 7, 2024 5:51 pm, Fiona Ebner wrote: > Makes it a clean error for buggy (external) backup providers where the > size might not be set at all. > > Signed-off-by: Fiona Ebner > --- > > No changes in v3. > > PVE/QemuServer.pm | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 49b6ca17..30e51a8c 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -6813,6 +6813,7 @@ my $restore_allocate_devices = sub { > my $map = {}; > foreach my $virtdev (sort keys %$virtdev_hash) { > my $d = $virtdev_hash->{$virtdev}; > + die "got no size for '$virtdev'\n" if !defined($d->{size}); > my $alloc_size = int(($d->{size} + 1024 - 1)/1024); > my $storeid = $d->{storeid}; > my $scfg = PVE::Storage::storage_config($storecfg, $storeid); > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [RFC manager] triggers: add path-based trigger interest
On November 11, 2024 6:44 pm, Thomas Lamprecht wrote: > Am 05.11.24 um 12:43 schrieb Fabian Grünbichler: >> to avoid the need to mark every package shipping PVE-related perl code as >> activating the explicit trigger. the explicit trigger can still be used for >> packages that need to reload the API without shipping a perl module >> >> the explicit trigger activation can be dropped in PVE 9.0 in packages that >> ship >> perl code, without a need for versioned dependencies. > > Not for the HA usecase. the HA case could also switch over to this approach, if we want to reload HA for all PVE perl modules.. if we only want it for a subset, then yes, the/an explicit trigger is better :) >> >> Signed-off-by: Fabian Grünbichler >> --- >> Currently, the following packages ship perl modules in /usr/share/perl5/PVE, >> but don't activate the explicit trigger: >> >> libpve-apiclient-perl libpve-cluster-perl libpve-notify-perl pve-cluster >> libpve-u2f-server-perl dab > > At least for libpve-notify-perl it might be nice to get a trigger so that HA > get's restarted there too, most of the rest it would be rather superfluous for > HA. see above - the question is whether we want an explicit list of packages that trigger HA (and risk that it runs out of sync with the modules the HA actually uses), or whether we want to treat HA like the pve-manager services, and just let it reload on any perl module changes that *could* affect it.. if desired, I can send a similar patch for pve-ha-manager as well. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 3/4] fix #3893: api: network: add bridge_vids parameter
The API itself allows several list separators. The network configuration for bridge_vids expects a space separated list. We therefore convert it initially to a space separated list. Signed-off-by: Aaron Lauterer --- I opted for a comment before the step where we split and reassemble the list with spaces as separators as this step might be a bit obscure if one is not aware of the reason (interfaces syntax). Feel free to drop the comments if you think they are unnessecary changes since v5: * drop PVE::Tools::list_is_empty and check for empty lists directly v4: * use the list_is_empty function, therefore avoiding negative matches * recreate the list with spaces as separators v3: * changed "vlans" to "VLANs" in description v2: * added checks to handle empty lists PVE/API2/Network.pm | 23 ++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm index f39f04f5..b9db9b27 100644 --- a/PVE/API2/Network.pm +++ b/PVE/API2/Network.pm @@ -66,6 +66,11 @@ my $confdesc = { type => 'boolean', optional => 1, }, +bridge_vids => { + description => "Specify the allowed VLANs. For example: '2 4 100-200'. Only used if the bridge is VLAN aware.", + optional => 1, + type => 'string', format => 'pve-vlan-id-or-range-list', +}, bridge_ports => { description => "Specify the interfaces you want to add to your bridge.", optional => 1, @@ -469,6 +474,14 @@ __PACKAGE__->register_method({ if ! grep { $_ eq $iface } @ports; } + if ($param->{bridge_vids} && scalar(PVE::Tools::split_list($param->{bridge_vids}) == 0)) { + raise_param_exc({ bridge_vids => "VLAN list items are empty" }); + } + # make sure the list is space separated! other separators will cause problems in the + # network configuration + $param->{bridge_vids} = join(" ", PVE::Tools::split_list($param->{bridge_vids})) + if $param->{bridge_vids}; + $ifaces->{$iface} = $param; PVE::INotify::write_file('interfaces', $config); @@ -558,7 +571,15 @@ __PACKAGE__->register_method({ foreach my $k (keys %$param) { $ifaces->{$iface}->{$k} = $param->{$k}; } - + + if ($param->{bridge_vids} && scalar(PVE::Tools::split_list($param->{bridge_vids}) == 0)) { + raise_param_exc({ bridge_vids => "VLAN list items are empty" }); + } + # make sure the list is space separated! other separators will cause problems in the + # network configuration + $param->{bridge_vids} = join(" ", PVE::Tools::split_list($param->{bridge_vids})) + if $param->{bridge_vids}; + PVE::INotify::write_file('interfaces', $config); }; -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v6 2/4] Network: add explanation for bridge vids field
Make clear that it affects only out-/inbound traffic and can be used if the underlying physical NICs support only a limited number of VLANs when offloading is possible. Signed-off-by: Aaron Lauterer Reviewed-by: Shannon Sterz --- v5: shorten description and put into gettext v4: none v3-follow-up: * reordered inside the patch series * reworded explanation according to suggestion src/node/NetworkEdit.js | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js index 2529acc..c50dafd 100644 --- a/src/node/NetworkEdit.js +++ b/src/node/NetworkEdit.js @@ -112,6 +112,11 @@ Ext.define('Proxmox.node.NetworkEdit', { return true; }, }); + let vidsExplanation = Ext.create('Ext.form.field.Display', { + disabled: true, + userCls: 'pmx-hint', + value: gettext('Limits the number of VLANs forwarded by bridge ports, useful for NICs with restricted VLAN offloading support.'), + }); column2.push({ xtype: 'proxmoxcheckbox', fieldLabel: gettext('VLAN aware'), @@ -121,6 +126,7 @@ Ext.define('Proxmox.node.NetworkEdit', { change: function(f, newVal) { if (me.bridge_set_vids) { vids.setDisabled(!newVal); + vidsExplanation.setDisabled(!newVal); } }, }, @@ -136,6 +142,7 @@ Ext.define('Proxmox.node.NetworkEdit', { }); if (me.bridge_set_vids) { advancedColumn2.push(vids); + advancedColumn2.push(vidsExplanation); } } else if (me.iftype === 'OVSBridge') { column2.push({ -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v6 1/4] fix #3892: Network: add bridge vids field for bridge_vids
The new optional bridge_vids field allows to set that property via the GUI. Since the backend needs to support it, the field needs to be explicitly enabled. For now, Proxmox VE (PVE) is the use case. Signed-off-by: Aaron Lauterer Tested-By: Stefan Hanreich Reviewed-by: Shannon Sterz --- changes since v5: change label to "VLAN IDs" v4: none v3: * switched regex to one with non-capturing group * reworked valid VLAN check according to the suggestion v2: * added validation code following how it is implemented in the API src/node/NetworkEdit.js | 65 + src/node/NetworkView.js | 5 2 files changed, 70 insertions(+) diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js index 27c1baf..2529acc 100644 --- a/src/node/NetworkEdit.js +++ b/src/node/NetworkEdit.js @@ -2,6 +2,9 @@ Ext.define('Proxmox.node.NetworkEdit', { extend: 'Proxmox.window.Edit', alias: ['widget.proxmoxNodeNetworkEdit'], +// Enable to show the VLAN ID field +bridge_set_vids: false, + initComponent: function() { let me = this; @@ -57,11 +60,70 @@ Ext.define('Proxmox.node.NetworkEdit', { } if (me.iftype === 'bridge') { + let vids = Ext.create('Ext.form.field.Text', { + fieldLabel: gettext('VLAN IDs'), + name: 'bridge_vids', + emptyText: '2-4094', + disabled: true, + autoEl: { + tag: 'div', + 'data-qtip': gettext('Space-separated list of VLANs and ranges, for example: 2 4 100-200'), + }, + validator: function(value) { + if (!value) { // empty + return true; + } + + let vid_list = value.split(' '); + + let invalidVid = function(tag) { + if (!isNaN(tag) && (tag < 2 || tag > 4094)) { + return `not a valid VLAN ID '${tag}'`; + } + + return false; + }; + + for (const vid of vid_list) { + if (!vid) { + continue; + } + let res = vid.match(/^(\d+)(?:-(\d+))?$/); + if (!res) { + return `not a valid VLAN configuration '${vid}'`; + } + let start = Number(res[1]); + let end = Number(res[2]); + + res = invalidVid(start); + if (res) { + return res; + } + + res = invalidVid(end); + if (res) { + return res; + } + + if (start >= end) { + return `VID range must go from lower to higher tag: '${vid}'`; + } + } + return true; + }, + }); column2.push({ xtype: 'proxmoxcheckbox', fieldLabel: gettext('VLAN aware'), name: 'bridge_vlan_aware', deleteEmpty: !me.isCreate, + listeners: { + change: function(f, newVal) { + if (me.bridge_set_vids) { + vids.setDisabled(!newVal); + } + }, + }, }); column2.push({ xtype: 'textfield', @@ -72,6 +134,9 @@ Ext.define('Proxmox.node.NetworkEdit', { 'data-qtip': gettext('Space-separated list of interfaces, for example: enp0s0 enp1s0'), }, }); + if (me.bridge_set_vids) { + advancedColumn2.push(vids); + } } else if (me.iftype === 'OVSBridge') { column2.push({ xtype: 'textfield', diff --git a/src/node/NetworkView.js b/src/node/NetworkView.js index 1d67ac8..c08cbfa 100644 --- a/src/node/NetworkView.js +++ b/src/node/NetworkView.js @@ -33,6 +33,9 @@ Ext.define('Proxmox.node.NetworkView', { showApplyBtn: false, +// decides if VLAN IDs field for bridges is shown, depends on the product if needed +bridge_set_vids: false, + initComponent: function() { let me = this; @@ -100,6 +103,7 @@ Ext.define('Proxmox.node.NetworkView', { nodename: me.nodename, iface: rec.data.iface, iftype: rec.data.type, + bridge_set_vids: me.bridge_set_vids, listeners: { destroy: () => reload(), }, @@ -170,6 +174,7 @@ Ext.define('Proxmox.node.NetworkView', { nodename: me.nodename, iftype: iType, iface_default: findNextFreeInterface
Re: [pve-devel] [RFC qemu v3 06/34] PVE backup: add target ID in backup state
On November 7, 2024 5:51 pm, Fiona Ebner wrote: > In preparation for allowing multiple backup providers. Each backup > target can then have its own dirty bitmap and there can be additional > checks that the current backup state is actually associated to the > expected target. > > Signed-off-by: Fiona Ebner > --- > > No changes in v3. > > pve-backup.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/pve-backup.c b/pve-backup.c > index d931746453..e8031bb89c 100644 > --- a/pve-backup.c > +++ b/pve-backup.c > @@ -70,6 +70,7 @@ static struct PVEBackupState { > JobTxn *txn; > CoMutex backup_mutex; > CoMutex dump_callback_mutex; > +char *target_id; > } backup_state; > > static void pvebackup_init(void) > @@ -848,7 +849,7 @@ UuidInfo coroutine_fn *qmp_backup( > > if (backup_state.di_list) { > error_set(errp, ERROR_CLASS_GENERIC_ERROR, > - "previous backup not finished"); > + "previous backup by provider '%s' not finished", > backup_state.target_id); > qemu_co_mutex_unlock(&backup_state.backup_mutex); > return NULL; > } > @@ -1100,6 +1101,11 @@ UuidInfo coroutine_fn *qmp_backup( > backup_state.vmaw = vmaw; > backup_state.pbs = pbs; > > +if (backup_state.target_id) { > +g_free(backup_state.target_id); > +} > +backup_state.target_id = g_strdup("Proxmox"); if we take this opportunity to also support multiple PBS targets while we are at it, it might make sense to make this more of a "legacy" value? or not set it at all here to opt into the legacy behaviour? > + > backup_state.di_list = di_list; > > uuid_info = g_malloc0(sizeof(*uuid_info)); > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [RFC container v3 27/34] create: factor out tar restore command helper
Am 07.11.24 um 17:51 schrieb Fiona Ebner: > In preparation to re-use it for restore from backup providers. > > Signed-off-by: Fiona Ebner > --- > > New in v3. > > src/PVE/LXC/Create.pm | 42 +- > 1 file changed, 25 insertions(+), 17 deletions(-) > > applied this one with Fabian's T-b, seemed like a sensible clean up on its own to me, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager 0/3] Use Autoscaler instead of Autoscale on user-facing strings
Am 24.11.23 um 14:03 schrieb Maximiliano Sandoval: > The autoscaler is well-known Ceph concept. A translation might prefer to use > "Autoscaler" as-is in a translation, which in turn lends itself better to a > search online. > > The patch series is split into three commits since each one can be a bit more > controvertial than the last one. > > Maximiliano Sandoval (3): > ceph-pool: Add gettext to user-facing string > ceph-pool: Replace PG Autoscale with PG Autoscaler on user-facing > string > ceph-pool: Replace Autoscale with Autoscaler in user-facing string > > www/manager6/ceph/Pool.js | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > applied series but squashed them into a single patch, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 manager] ui: acme: switch cluster view items over to those from widget-toolkit
Am 30.04.24 um 13:17 schrieb Filip Schauer: > The pmxACMEAccountView & pmxACMEPluginView in proxmox-widget-toolkit > were copied from pve-manager in commits 5df894de and 658bfdff. This > makes pveACMEAccountView & pveACMEPluginView redundant, hence remove > them and use pmxACMEAccountView & pmxACMEPluginView instead. > > This leaves PVE.node.ACMEAccountView & pveACMEPluginEditor unused, so > remove them too. > > Signed-off-by: Filip Schauer > --- > Changes since v1: > * Also switch over account view > * Correct acmeUrl (/config/acme -> /cluster/acme) > * Reference introduction of pmxACMEAccountView & pmxACMEPluginView in > commit message > * Remove ACMEPluginEdit.js from the Makefile > > www/manager6/Makefile | 1 - > www/manager6/dc/ACMEClusterView.js | 204 +- > www/manager6/dc/ACMEPluginEdit.js | 223 - > www/manager6/node/ACME.js | 66 - > 4 files changed, 4 insertions(+), 490 deletions(-) > delete mode 100644 www/manager6/dc/ACMEPluginEdit.js > > Look OK in general but needs a rebase due to a typo fix in the deleted file and I get an exception when navigating to the Datacenter -> ACME panel, so maybe something else changed that needs adaption here. 20:45:51.345 Uncaught neither 'url' nor both, submitUrl and loadUrl specified proxmoxlib.js:12473:6 initComponent https://d8.work.tlmp.it:8006/proxmoxlib.js?ver=v4.3.0-t1731358651:12473 callParent ExtJS initComponent https://d8.work.tlmp.it:8006/proxmoxlib.js?ver=v4.3.0-t1731358651:14055 ExtJS 2 constructor https://d8.work.tlmp.it:8006/proxmoxlib.js?ver=v4.3.0-t1731358651:12284 ExtJS 17 activateCard https://d8.work.tlmp.it:8006/pve2/js/pvemanagerlib.js?ver=8.2.8:13600 selectionchange https://d8.work.tlmp.it:8006/pve2/js/pvemanagerlib.js?ver=8.2.8:13544 ExtJS 39 initComponent https://d8.work.tlmp.it:8006/pve2/js/pvemanagerlib.js?ver=8.2.8:60157 callParent ExtJS initComponent https://d8.work.tlmp.it:8006/pve2/js/pvemanagerlib.js?ver=8.2.8:60615 ExtJS 4 https://d8.work.tlmp.it:8006/#v1:0:18:4:10:=contentIso::30:8::=sdnmappings:41 ExtJS 15 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC container v3 27/34] create: factor out tar restore command helper
Reviewed-by: Fabian Grünbichler IMHO this would also be a candidate for applying now - but held off because of the RFC prefix ;) On November 7, 2024 5:51 pm, Fiona Ebner wrote: > In preparation to re-use it for restore from backup providers. > > Signed-off-by: Fiona Ebner > --- > > New in v3. > > src/PVE/LXC/Create.pm | 42 +- > 1 file changed, 25 insertions(+), 17 deletions(-) > > diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm > index 7c5bf0a..8c8cb9a 100644 > --- a/src/PVE/LXC/Create.pm > +++ b/src/PVE/LXC/Create.pm > @@ -59,12 +59,34 @@ sub restore_proxmox_backup_archive { > $scfg, $storeid, $cmd, $param, userns_cmd => $userns_cmd); > } > > -sub restore_tar_archive { > -my ($archive, $rootdir, $conf, $no_unpack_error, $bwlimit) = @_; > +my sub restore_tar_archive_command { > +my ($conf, $opts, $rootdir, $bwlimit) = @_; > > my ($id_map, $root_uid, $root_gid) = PVE::LXC::parse_id_maps($conf); > my $userns_cmd = PVE::LXC::userns_command($id_map); > > +my $cmd = [@$userns_cmd, 'tar', 'xpf', '-', $opts->@*, '--totals', > + @PVE::Storage::Plugin::COMMON_TAR_FLAGS, > + '-C', $rootdir]; > + > +# skip-old-files doesn't have anything to do with time (old/new), but is > +# simply -k (annoyingly also called --keep-old-files) without the 'treat > +# existing files as errors' part... iow. it's bsdtar's interpretation of > -k > +# *sigh*, gnu... > +push @$cmd, '--skip-old-files'; > +push @$cmd, '--anchored'; > +push @$cmd, '--exclude' , './dev/*'; > + > +if (defined($bwlimit)) { > + $cmd = [ ['cstream', '-t', $bwlimit*1024], $cmd ]; > +} > + > +return $cmd; > +} > + > +sub restore_tar_archive { > +my ($archive, $rootdir, $conf, $no_unpack_error, $bwlimit) = @_; > + > my $archive_fh; > my $tar_input = '<&STDIN'; > my @compression_opt; > @@ -92,21 +114,7 @@ sub restore_tar_archive { > $tar_input = '<&'.fileno($archive_fh); > } > > -my $cmd = [@$userns_cmd, 'tar', 'xpf', '-', @compression_opt, '--totals', > - @PVE::Storage::Plugin::COMMON_TAR_FLAGS, > - '-C', $rootdir]; > - > -# skip-old-files doesn't have anything to do with time (old/new), but is > -# simply -k (annoyingly also called --keep-old-files) without the 'treat > -# existing files as errors' part... iow. it's bsdtar's interpretation of > -k > -# *sigh*, gnu... > -push @$cmd, '--skip-old-files'; > -push @$cmd, '--anchored'; > -push @$cmd, '--exclude' , './dev/*'; > - > -if (defined($bwlimit)) { > - $cmd = [ ['cstream', '-t', $bwlimit*1024], $cmd ]; > -} > +my $cmd = restore_tar_archive_command($conf, [@compression_opt], > $rootdir, $bwlimit); > > if ($archive eq '-') { > print "extracting archive from STDIN\n"; > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager] api/ui: include the node ha status in resources call and show as icon
Am 31.05.24 um 10:07 schrieb Dominik Csapak: > we already have the information parsed, so it's cheap, and we already > have a mechanism in place that adds 'ha-' as a css class, so > let's reuse that. > > I chose a yellow wrench, as wrenches are associated with 'maintenance', > and because the state warrants more notice than 'online' but less than > 'offline'. > > Users mentioned in the forum that this would be nice: > https://forum.proxmox.com/threads/125768/ > > Signed-off-by: Dominik Csapak > --- > not sure about the color, since the yellow has relatively low contrast > in the light mode (in dark mode it's fine). It's the same yellow as for > 'io-errors' though. maybe blue as more of a "notice" color, this state is not problematic per se after all. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH widget-toolkit 0/5] improve webhook edit window
Am 12.11.24 um 15:41 schrieb Dominik Csapak: > by implementing Thomas suggestions from here: > https://lore.proxmox.com/pve-devel/f592fea7-e0a5-4858-af48-b0b2ed57b...@proxmox.com/ > > Dominik Csapak (5): > webhook edit: improve layout and component hierarchy > webhook edit: make items config not static > webhook edit: use type in add button text > webhook edit: add emptytext to key-value fields > webhook edit: display validity for added key/value fields > > src/panel/WebhookEditPanel.js | 185 +- > 1 file changed, 92 insertions(+), 93 deletions(-) > applied series, many thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer v2 3/4] assistant: pre-compile ignored block device patterns
Am 13.05.24 um 11:49 schrieb Christoph Heiss: > No functional changes. > > Signed-off-by: Christoph Heiss > --- > Changes v1 -> v2: > * no changes > > proxmox-auto-install-assistant/src/main.rs | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > applied this one with Aarons review tags, thanks! The rest would need to be rebased, sorry for the wait here. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version
Am 12.11.24 um 15:06 schrieb Christoph Heiss: > On Tue, Nov 12, 2024 at 02:44:48PM +0100, Thomas Lamprecht wrote: >> maybe still use $format-version or $hook-version or the like for the >> serialized key? I.e., to even better convey that this is, e.g., not some >> version from the installer or product. > > What do you think about > > "$hook": { "version": "1.0" }` > > maybe, thereby making it a bit more structured from the start? > Esp. when thinking about future expansions, if we ever want to add > additional metadata fields. > > As we're already on the topic of future changes. > > But no hard feelings, otherwise I'd just go with `$hook-version`. both sound fine to me, I first thought $hook-version is more greppable, but that is not really relevant here as the tool processing this needs to know json already anyway to do so in a sensible manner, so your proposal seems good to me too, feel free to chose whatever you prefer. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] partially-applied-series: [PATCH proxmox-ve-rs v3 16/24] tests: add ipam tests
Am 12.11.24 um 13:25 schrieb Stefan Hanreich: > Signed-off-by: Stefan Hanreich > --- > proxmox-ve-config/tests/sdn/main.rs | 45 +++ > proxmox-ve-config/tests/sdn/resources/ipam.db | 26 +++ > 2 files changed, 71 insertions(+) > create mode 100644 proxmox-ve-config/tests/sdn/resources/ipam.db > > applied the proxmox-ve-rs patrches on top of your stafff repo, thanks! I already mirrored it publicly [0], but as long as we do not yet use it anywhere (i.e., apply the rest of your patches) I'm fine with force-pushing, if really necesarry, or the like. [0]: https://git.proxmox.com/?p=proxmox-ve-rs.git;a=summary ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH] patches: upstream: python3.12 compat
--- Begin Message --- Prepare for debian trixie. Upstream has not yet made a new release since Dec 2023, backporting this commit to make ifupdown2 work on trixie. Also fixes an RC bug on debian (#1074250). Also fixed a typo in the commit msg. upstream: https://github.com/CumulusNetworks/ifupdown2/commit/fc0318378e878ffe639d1d1285936d1256dd67cf Signed-off-by: Jing Luo --- debian/patches/series | 1 + .../upstream/0001-python312-compat.patch | 27 +++ 2 files changed, 28 insertions(+) create mode 100644 debian/patches/upstream/0001-python312-compat.patch diff --git a/debian/patches/series b/debian/patches/series index 7ae3f0a..c14c181 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -14,3 +14,4 @@ upstream/0001-scheduler-import-traceback.patch upstream/0001-addons-vxlan-fix-compare-between-remote-ips-and-old_.patch upstream/0001-scheduler-avoid-waiting-for-stdout-eof-of-etc-networ.patch upstream/0001-vxlan-fix-vni-filter.patch +upstream/0001-python312-compat.patch diff --git a/debian/patches/upstream/0001-python312-compat.patch b/debian/patches/upstream/0001-python312-compat.patch new file mode 100644 index 000..c36482f --- /dev/null +++ b/debian/patches/upstream/0001-python312-compat.patch @@ -0,0 +1,27 @@ +From fc0318378e878ffe639d1d1285936d1256dd67cf Mon Sep 17 00:00:00 2001 +From: Jan Huijsmans +Date: Tue, 9 Jul 2024 09:03:25 +0200 +Subject: [PATCH] Bug #296: python 3.12 compatibility + +Since python 3.2, readfp needs to be replaced by read_file. +Python 3.12 dropped the readfp function. + +Patch provided as PR by me, as the original reporter failed to do +so since 4-4-2024 and my systems break due to this issue. +--- + ifupdown2/ifupdown/main.py | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ifupdown2/ifupdown/main.py b/ifupdown2/ifupdown/main.py +index 8e0f13f4..de6579ea 100644 +--- a/ifupdown2/ifupdown/main.py b/ifupdown2/ifupdown/main.py +@@ -138,7 +138,7 @@ def read_config(self): + configStr = '[ifupdown2]\n' + config + configFP = io.StringIO(configStr) + parser = configparser.RawConfigParser() +-parser.readfp(configFP) ++parser.read_file(configFP) + configmap_g = dict(parser.items('ifupdown2')) + + # Preprocess config map -- 2.47.0 --- End Message --- ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v3 19/34] backup: keep track of block-node size for fleecing
On 11.11.24 3:22 PM, Fabian Grünbichler wrote: > On November 7, 2024 5:51 pm, Fiona Ebner wrote: >> @@ -1042,6 +1044,31 @@ sub qga_fs_thaw { >> $self->logerr($@) if $@; >> } >> >> +# The size for fleecing images needs to be exactly the same size as QEMU >> sees. E.g. EFI disk can bex >> +# attached with a smaller size then the underyling image on the storage. >> +sub query_block_node_sizes { >> +my ($self, $vmid, $task) = @_; >> + >> +my $block_info = mon_cmd($vmid, "query-block"); >> +$block_info = { map { $_->{device} => $_ } $block_info->@* }; >> + >> +for my $diskinfo ($task->{disks}->@*) { > > only usage of $task > > so we don't actually need to add $task as parameter to the two existing > subs, but can just modify this here to take $task->{disks} directly? or > did I overlook something? > > if we do have to keep $task as parameter, it should come before $vmid in > the argument list, to be consistent with the rest.. > > other than that, consider this patch > Right, since $task->{disks} is itself a reference, this should work out fine :) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH novnc v2 1/3] upgrade noVNC and patches to 1.5.0
Signed-off-by: Dominik Csapak --- changes from v1: * rebased on master .../0001-add-PVE-specific-JS-code.patch | 4 ++-- ...002-add-custom-fbresize-event-on-rfb.patch | 10 +- ...nge-scaling-when-toggling-fullscreen.patch | 6 +++--- ...rectory-for-fetching-images-js-files.patch | 20 +-- .../0009-decrease-animation-time.patch| 2 +- .../0011-add-localCursor-setting-to-rfb.patch | 18 - ...ove-the-default-value-of-wsProtocols.patch | 4 ++-- novnc | 2 +- 8 files changed, 33 insertions(+), 33 deletions(-) diff --git a/debian/patches/0001-add-PVE-specific-JS-code.patch b/debian/patches/0001-add-PVE-specific-JS-code.patch index ca01d11..655a2cd 100644 --- a/debian/patches/0001-add-PVE-specific-JS-code.patch +++ b/debian/patches/0001-add-PVE-specific-JS-code.patch @@ -453,7 +453,7 @@ index 000..1a062ad +}, +}; diff --git a/app/ui.js b/app/ui.js -index c1f6776..c86f36c 100644 +index f27dfe2..f2f194c 100644 --- a/app/ui.js +++ b/app/ui.js @@ -17,6 +17,7 @@ import keysyms from "../core/input/keysymdef.js"; @@ -560,7 +560,7 @@ index c1f6776..c86f36c 100644 }, /* --^--- -@@ -1689,9 +1702,36 @@ const UI = { +@@ -1697,9 +1710,36 @@ const UI = { /* --^--- * /EXTRA KEYS * == diff --git a/debian/patches/0002-add-custom-fbresize-event-on-rfb.patch b/debian/patches/0002-add-custom-fbresize-event-on-rfb.patch index 1991bd6..286d665 100644 --- a/debian/patches/0002-add-custom-fbresize-event-on-rfb.patch +++ b/debian/patches/0002-add-custom-fbresize-event-on-rfb.patch @@ -13,10 +13,10 @@ Signed-off-by: Dominik Csapak 2 files changed, 21 insertions(+) diff --git a/app/ui.js b/app/ui.js -index c86f36c..5beef1e 100644 +index f2f194c..f32b67c 100644 --- a/app/ui.js +++ b/app/ui.js -@@ -1068,6 +1068,7 @@ const UI = { +@@ -1076,6 +1076,7 @@ const UI = { UI.rfb.addEventListener("clipboard", UI.clipboardReceive); UI.rfb.addEventListener("bell", UI.bell); UI.rfb.addEventListener("desktopname", UI.updateDesktopName); @@ -24,7 +24,7 @@ index c86f36c..5beef1e 100644 UI.rfb.clipViewport = UI.getSetting('view_clip'); UI.rfb.scaleViewport = UI.getSetting('resize') === 'scale'; UI.rfb.resizeSession = UI.getSetting('resize') === 'remote'; -@@ -1727,6 +1728,16 @@ const UI = { +@@ -1735,6 +1736,16 @@ const UI = { document.getElementById('pve_commands_button').classList.remove("noVNC_selected"); }, @@ -42,10 +42,10 @@ index c86f36c..5beef1e 100644 */PVE * == diff --git a/core/rfb.js b/core/rfb.js -index 6afd7c6..2f662ce 100644 +index f2deb0e..37ba099 100644 --- a/core/rfb.js +++ b/core/rfb.js -@@ -2881,6 +2881,16 @@ export default class RFB extends EventTargetMixin { +@@ -2873,6 +2873,16 @@ export default class RFB extends EventTargetMixin { this._updateClip(); this._updateScale(); diff --git a/debian/patches/0003-change-scaling-when-toggling-fullscreen.patch b/debian/patches/0003-change-scaling-when-toggling-fullscreen.patch index e80835a..5cf14e5 100644 --- a/debian/patches/0003-change-scaling-when-toggling-fullscreen.patch +++ b/debian/patches/0003-change-scaling-when-toggling-fullscreen.patch @@ -12,10 +12,10 @@ Signed-off-by: Dominik Csapak 1 file changed, 11 insertions(+) diff --git a/app/ui.js b/app/ui.js -index 5beef1e..1e64f20 100644 +index f32b67c..04a5a2a 100644 --- a/app/ui.js +++ b/app/ui.js -@@ -1286,6 +1286,13 @@ const UI = { +@@ -1294,6 +1294,13 @@ const UI = { } else if (document.msExitFullscreen) { document.msExitFullscreen(); } @@ -29,7 +29,7 @@ index 5beef1e..1e64f20 100644 } else { if (document.documentElement.requestFullscreen) { document.documentElement.requestFullscreen(); -@@ -1296,7 +1303,11 @@ const UI = { +@@ -1304,7 +1311,11 @@ const UI = { } else if (document.body.msRequestFullscreen) { document.body.msRequestFullscreen(); } diff --git a/debian/patches/0006-change-source-directory-for-fetching-images-js-files.patch b/debian/patches/0006-change-source-directory-for-fetching-images-js-files.patch index e4ede34..17586d7 100644 --- a/debian/patches/0006-change-source-directory-for-fetching-images-js-files.patch +++ b/debian/patches/0006-change-source-directory-for-fetching-images-js-files.patch @@ -13,7 +13,7 @@ Signed-off-by: Dominik Csapak 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/app/ui.js b/app/ui.js -index 1e64f20..1345cc5 100644 +index 04a5a2a..18bac4b 100644 --- a/app/ui.js +++ b/app/ui.js @@ -73,7 +73,7 @@ const UI = { @@ -25,15 +25,15 @@ index 1e64f20..1345cc5 100644 .then((response) => { if (!response.ok) { throw Error("" + response.status + " " + response.statusText); -@@ -1829,7
Re: [pve-devel] [RFC qemu-server v3 21/34] backup: implement backup for external providers
On November 12, 2024 3:35 pm, Fiona Ebner wrote: > On 12.11.24 1:27 PM, Fabian Grünbichler wrote: >> On November 7, 2024 5:51 pm, Fiona Ebner wrote: >>> + my $backup_access_info = eval { mon_cmd($vmid, "backup-access-setup", >>> $params->%*) }; >>> + my $qmperr = $@; >>> + >>> + $task->{cleanup}->{'backup-access-teardown'} = { 'target-id' => >>> $target_id, success => 0 }; >> >> should we differentiate here between setup success or failure? if not, >> should we move it directly before the setup call? >> > > No, there should be no differentiation. The teardown QMP command needs > to be called in any case. But how could it happen that we do reach > cleanup but haven't gotten through here after the setup call? The setup > call is in an eval and there is nothing that can die in between. I can > still move it if you feel that is cleaner. yeah, this is mostly about other stuff being added in-between later on. probably not too likely in this case, but I always prefer setting a cleanup-potentially-required flag *before* doing the thing that potentially requires cleanup, rather than after (even if the current code does everything right). it's the safer variant (even though it of course also has potential for stuff being added in-between, and then triggering cleanup without the cleanup-source actually having happened ;)) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH container v3 25/34] create: add missing include of PVE::Storage::Plugin
thanks! On November 7, 2024 5:51 pm, Fiona Ebner wrote: > used for the shared 'COMMON_TAR_FLAGS' variable. > > Signed-off-by: Fiona Ebner > --- > > New in v3. > > src/PVE/LXC/Create.pm | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm > index 117103c..7c5bf0a 100644 > --- a/src/PVE/LXC/Create.pm > +++ b/src/PVE/LXC/Create.pm > @@ -8,6 +8,7 @@ use Fcntl; > > use PVE::RPCEnvironment; > use PVE::Storage::PBSPlugin; > +use PVE::Storage::Plugin; > use PVE::Storage; > use PVE::DataCenterConfig; > use PVE::LXC; > -- > 2.39.5 > > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 01/24] debian: add files for packaging
Since we now have a standalone repository for Proxmox VE related crates, add the required files for packaging the crates contained in this repository. Signed-off-by: Stefan Hanreich --- .cargo/config.toml | 5 ++ .gitignore | 8 +++ Cargo.toml | 17 +++ Makefile | 69 ++ build.sh | 35 + bump.sh| 44 proxmox-ve-config/Cargo.toml | 19 +++ proxmox-ve-config/debian/changelog | 5 ++ proxmox-ve-config/debian/control | 46 + proxmox-ve-config/debian/copyright | 19 +++ proxmox-ve-config/debian/debcargo.toml | 4 ++ 11 files changed, 260 insertions(+), 11 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 .gitignore create mode 100644 Cargo.toml create mode 100644 Makefile create mode 100755 build.sh create mode 100755 bump.sh create mode 100644 proxmox-ve-config/debian/changelog create mode 100644 proxmox-ve-config/debian/control create mode 100644 proxmox-ve-config/debian/copyright create mode 100644 proxmox-ve-config/debian/debcargo.toml diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000..3b5b6e4 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,5 @@ +[source] +[source.debian-packages] +directory = "/usr/share/cargo/registry" +[source.crates-io] +replace-with = "debian-packages" diff --git a/.gitignore b/.gitignore new file mode 100644 index 000..d72b68b --- /dev/null +++ b/.gitignore @@ -0,0 +1,8 @@ +/target +/*/target +Cargo.lock +**/*.rs.bk +/*.buildinfo +/*.changes +/build +/*-deb diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 000..dc7f312 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,17 @@ +[workspace] +members = [ +"proxmox-ve-config", +] +exclude = [ +"build", +] +resolver = "2" + +[workspace.package] +authors = ["Proxmox Support Team "] +edition = "2021" +license = "AGPL-3" +homepage = "https://proxmox.com"; +exclude = [ "debian" ] +rust-version = "1.82" + diff --git a/Makefile b/Makefile new file mode 100644 index 000..0da9b74 --- /dev/null +++ b/Makefile @@ -0,0 +1,69 @@ +# Shortcut for common operations: + +CRATES != echo proxmox-*/Cargo.toml | sed -e 's|/Cargo.toml||g' + +# By default we just run checks: +.PHONY: all +all: check + +.PHONY: deb +deb: $(foreach c,$(CRATES), $c-deb) + echo $(foreach c,$(CRATES), $c-deb) + lintian build/*.deb + +.PHONY: dsc +dsc: $(foreach c,$(CRATES), $c-dsc) + echo $(foreach c,$(CRATES), $c-dsc) + lintian build/*.dsc + +.PHONY: autopkgtest +autopkgtest: $(foreach c,$(CRATES), $c-autopkgtest) + +.PHONY: dinstall +dinstall: + $(MAKE) clean + $(MAKE) deb + sudo -k dpkg -i build/librust-*.deb + +%-deb: + ./build.sh $* + touch $@ + +%-dsc: + BUILDCMD='dpkg-buildpackage -S -us -uc -d' ./build.sh $* + touch $@ + +%-autopkgtest: + autopkgtest build/$* build/*.deb -- null + touch $@ + +.PHONY: check +check: + cargo test + +# Prints a diff between the current code and the one rustfmt would produce +.PHONY: fmt +fmt: + cargo +nightly fmt -- --check + +# Doc without dependencies +.PHONY: doc +doc: + cargo doc --no-deps + +.PHONY: clean +clean: + cargo clean + rm -rf build/ + rm -f -- *-deb *-dsc *-autopkgtest *.build *.buildinfo *.changes + +.PHONY: update +update: + cargo update + +%-upload: %-deb + cd build; \ + dcmd --deb rust-$*_*.changes \ + | grep -v '.changes$$' \ + | tar -cf "$@.tar" -T-; \ + cat "$@.tar" | ssh -X repo...@repo.proxmox.com upload --product devel --dist bookworm diff --git a/build.sh b/build.sh new file mode 100755 index 000..39a8302 --- /dev/null +++ b/build.sh @@ -0,0 +1,35 @@ +#!/bin/sh + +set -eux + +export CARGO=/usr/bin/cargo +export RUSTC=/usr/bin/rustc + +CRATE=$1 +BUILDCMD=${BUILDCMD:-"dpkg-buildpackage -b -uc -us"} + +mkdir -p build +echo system >build/rust-toolchain +rm -rf "build/${CRATE}" + +CONTROL="$PWD/${CRATE}/debian/control" + +if [ -e "$CONTROL" ]; then +# check but only warn, debcargo fails anyway if crates are missing +dpkg-checkbuilddeps $PWD/${CRATE}/debian/control || true +# rm -f "$PWD/${CRATE}/debian/control" +fi + +debcargo package \ +--config "$PWD/${CRATE}/debian/debcargo.toml" \ +--changelog-ready \ +--no-overlay-write-back \ +--directory "$PWD/build/${CRATE}" \ +"${CRATE}" \ +"$(dpkg-parsechangelog -l "${CRATE}/debian/changelog" -SVersion | sed -e 's/-.*//')" + +cd "build/${CRATE}" +rm -f debian/source/format.debcargo.hint +${BUILDCMD} + +cp debian/control "$CONTROL" diff --git a/bump.sh b/bump.sh new file mode 100755 index 000..08ad119 --- /dev/null +++ b/bump.sh @@ -0,0 +1,44 @@ +#!/bin/bash + +package=$1 + +if [[ -z "$package" ]]; then +
[pve-devel] [PATCH proxmox-ve-rs v3 03/24] firewall: add ip range types
Currently we are using tuples to represent IP ranges which is suboptimal. Validation logic and invariant checking needs to happen at every site using the IP range rather than having a unified struct for enforcing those invariants. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 230 +- 1 file changed, 228 insertions(+), 2 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index e48ac1b..f7bde51 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -1,9 +1,9 @@ -use std::fmt; +use std::fmt::{self, Display}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::ops::Deref; use anyhow::{bail, format_err, Error}; -use serde_with::DeserializeFromStr; +use serde_with::{DeserializeFromStr, SerializeDisplay}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Family { @@ -239,6 +239,202 @@ impl> From for Ipv6Cidr { } } +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] +pub enum IpRangeError { +MismatchedFamilies, +StartGreaterThanLast, +InvalidFormat, +} + +impl std::error::Error for IpRangeError {} + +impl Display for IpRangeError { +fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +f.write_str(match self { +IpRangeError::MismatchedFamilies => "mismatched ip address families", +IpRangeError::StartGreaterThanLast => "start is greater than last", +IpRangeError::InvalidFormat => "invalid ip range format", +}) +} +} + +/// Represents a range of IPv4 or IPv6 addresses. +/// +/// For more information see [`AddressRange`] +#[derive(Clone, Copy, Debug, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)] +pub enum IpRange { +V4(AddressRange), +V6(AddressRange), +} + +impl IpRange { +/// Returns the family of the IpRange. +pub fn family(&self) -> Family { +match self { +IpRange::V4(_) => Family::V4, +IpRange::V6(_) => Family::V6, +} +} + +/// Creates a new [`IpRange`] from two [`IpAddr`]. +/// +/// # Errors +/// +/// This function will return an error if start and last IP address are not from the same family. +pub fn new(start: impl Into, last: impl Into) -> Result { +match (start.into(), last.into()) { +(IpAddr::V4(start), IpAddr::V4(last)) => Self::new_v4(start, last), +(IpAddr::V6(start), IpAddr::V6(last)) => Self::new_v6(start, last), +_ => Err(IpRangeError::MismatchedFamilies), +} +} + +/// construct a new Ipv4 Range +pub fn new_v4( +start: impl Into, +last: impl Into, +) -> Result { +Ok(IpRange::V4(AddressRange::new_v4(start, last)?)) +} + +pub fn new_v6( +start: impl Into, +last: impl Into, +) -> Result { +Ok(IpRange::V6(AddressRange::new_v6(start, last)?)) +} +} + +impl std::str::FromStr for IpRange { +type Err = IpRangeError; + +fn from_str(s: &str) -> Result { +if let Ok(range) = s.parse() { +return Ok(IpRange::V4(range)); +} + +if let Ok(range) = s.parse() { +return Ok(IpRange::V6(range)); +} + +Err(IpRangeError::InvalidFormat) +} +} + +impl fmt::Display for IpRange { +fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +match self { +IpRange::V4(range) => range.fmt(f), +IpRange::V6(range) => range.fmt(f), +} +} +} + +/// Represents a range of IP addresses from start to last. +/// +/// This type is for encapsulation purposes for the [`IpRange`] enum and should be instantiated via +/// that enum. +/// +/// # Invariants +/// +/// * start and last have the same IP address family +/// * start is less than or equal to last +/// +/// # Textual representation +/// +/// Two IP addresses separated by a hyphen, e.g.: `127.0.0.1-127.0.0.255` +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct AddressRange { +start: T, +last: T, +} + +impl AddressRange { +pub(crate) fn new_v4( +start: impl Into, +last: impl Into, +) -> Result, IpRangeError> { +let (start, last) = (start.into(), last.into()); + +if start > last { +return Err(IpRangeError::StartGreaterThanLast); +} + +Ok(Self { start, last }) +} +} + +impl AddressRange { +pub(crate) fn new_v6( +start: impl Into, +last: impl Into, +) -> Result, IpRangeError> { +let (start, last) = (start.into(), last.into()); + +if start > last { +return Err(IpRangeError::StartGreaterThanLast); +} + +Ok(Self { start, last }) +} +} + +impl AddressRange { +pub fn start(&self) -> &T { +&self.start +} + +pub fn last(&self) -> &T { +&self.last +} +} + +impl
[pve-devel] [PATCH proxmox-ve-rs v3 05/24] ipset: add range variant to addresses
A range can be used to store multiple IP addresses in an ipset that do not neatly fit into a single CIDR. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/types/ipset.rs | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index 6fbdca8..e8131b5 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -6,7 +6,7 @@ use anyhow::{bail, format_err, Error}; use serde_with::DeserializeFromStr; use crate::firewall::parse::match_non_whitespace; -use crate::firewall::types::address::Cidr; +use crate::firewall::types::address::{Cidr, IpRange}; use crate::firewall::types::alias::AliasName; use crate::guest::vm::NetworkConfig; @@ -93,6 +93,7 @@ impl Display for IpsetName { pub enum IpsetAddress { Alias(AliasName), Cidr(Cidr), +Range(IpRange), } impl FromStr for IpsetAddress { @@ -117,6 +118,12 @@ impl> From for IpsetAddress { } } +impl From for IpsetAddress { +fn from(range: IpRange) -> Self { +IpsetAddress::Range(range) +} +} + #[derive(Debug)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct IpsetEntry { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 07/24] ipset: address: add helper methods
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/types/address.rs | 10 ++ proxmox-ve-config/src/firewall/types/ipset.rs | 14 ++ 2 files changed, 24 insertions(+) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 95c58a7..938b8e1 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -11,6 +11,16 @@ pub enum Family { V6, } +impl Family { +pub fn is_ipv4(&self) -> bool { +*self == Self::V4 +} + +pub fn is_ipv6(&self) -> bool { +*self == Self::V6 +} +} + impl fmt::Display for Family { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index e8131b5..7511490 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -132,6 +132,20 @@ pub struct IpsetEntry { pub comment: Option, } +impl IpsetEntry { +pub fn new( +address: impl Into, +nomatch: bool, +comment: impl Into>, +) -> IpsetEntry { +IpsetEntry { +nomatch, +address: address.into(), +comment: comment.into(), +} +} +} + impl> From for IpsetEntry { fn from(value: T) -> Self { Self { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-firewall v3 10/18] api: add vnet endpoints
Signed-off-by: Stefan Hanreich --- src/PVE/API2/Firewall/Makefile | 1 + src/PVE/API2/Firewall/Rules.pm | 84 + src/PVE/API2/Firewall/Vnet.pm | 168 + src/PVE/Firewall.pm| 10 ++ 4 files changed, 263 insertions(+) create mode 100644 src/PVE/API2/Firewall/Vnet.pm diff --git a/src/PVE/API2/Firewall/Makefile b/src/PVE/API2/Firewall/Makefile index e916755..325c4d3 100644 --- a/src/PVE/API2/Firewall/Makefile +++ b/src/PVE/API2/Firewall/Makefile @@ -9,6 +9,7 @@ LIB_SOURCES=\ Cluster.pm \ Host.pm \ VM.pm \ + Vnet.pm \ Groups.pm all: diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm index f5cb002..e57b3de 100644 --- a/src/PVE/API2/Firewall/Rules.pm +++ b/src/PVE/API2/Firewall/Rules.pm @@ -18,6 +18,25 @@ my $api_properties = { }, }; +=head3 check_privileges_for_method($class, $method_name, $param) + +If the permission checks from the register_method() call are not sufficient, +this function can be overriden for performing additional permission checks +before API methods are executed. If the permission check fails, this function +should die with an appropriate error message. The name of the method calling +this function is provided by C<$method_name> and the parameters of the API +method are provided by C<$param> + +Default implementation is a no-op to preserve backwards compatibility with +existing subclasses, since this got added later on. It preserves existing +behavior without having to change every subclass. + +=cut + +sub check_privileges_for_method { +my ($class, $method_name, $param) = @_; +} + sub lock_config { my ($class, $param, $code) = @_; @@ -94,6 +113,8 @@ sub register_get_rules { code => sub { my ($param) = @_; + $class->check_privileges_for_method('get_rules', $param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules); @@ -192,6 +213,8 @@ sub register_get_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('get_rule', $param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules); @@ -232,6 +255,8 @@ sub register_create_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('create_rule', $param); + $class->lock_config($param, sub { my ($param) = @_; @@ -293,6 +318,8 @@ sub register_update_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('update_rule', $param); + $class->lock_config($param, sub { my ($param) = @_; @@ -359,6 +386,8 @@ sub register_delete_rule { code => sub { my ($param) = @_; + $class->check_privileges_for_method('delete_rule', $param); + $class->lock_config($param, sub { my ($param) = @_; @@ -634,4 +663,59 @@ sub save_rules { __PACKAGE__->register_handlers(); +package PVE::API2::Firewall::VnetRules; + +use strict; +use warnings; +use PVE::JSONSchema qw(get_standard_option); + +use base qw(PVE::API2::Firewall::RulesBase); + +__PACKAGE__->additional_parameters({ +vnet => get_standard_option('pve-sdn-vnet-id'), +}); + +sub check_privileges_for_method { +my ($class, $method_name, $param) = @_; + +if ($method_name eq 'get_rule' || $method_name eq 'get_rules') { + PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, ['SDN.Audit', 'SDN.Allocate']); +} elsif ($method_name =~ '(update|create|delete)_rule') { + PVE::API2::Firewall::Vnet::check_vnet_access($param->{vnet}, ['SDN.Allocate']); +} else { + die "unknown method: $method_name"; +} +} + +sub rule_env { +my ($class, $param) = @_; + +return 'vnet'; +} + +sub lock_config { +my ($class, $param, $code) = @_; + +PVE::Firewall::lock_vnetfw_conf($param->{vnet}, 10, $code, $param); +} + +sub load_config { +my ($class, $param) = @_; + +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); +my $fw_conf = PVE::Firewall::load_vnetfw_conf($cluster_conf, 'vnet', $param->{vnet}); +my $rules = $fw_conf->{rules}; + +return ($cluster_conf, $fw_conf, $rules); +} + +sub save_rules { +my ($class, $param, $fw_conf, $rules) = @_; + +$fw_conf->{rules} = $rules; +PVE::Firewall::save_vnetfw_conf($param->{vnet}, $fw_conf); +} + +__PACKAGE__->register_handlers(); + 1; diff --git a/src/PVE/API2/Firewall/Vnet.pm b/src/PVE/API2/Firewall/Vnet.pm new file mode 100644 index 000..cb49b67 --- /dev/null +++ b/src/PVE/API2/Firewall/Vnet.pm @@ -0,0 +1,168 @@
[pve-devel] [PATCH pve-firewall v3 11/18] firewall: move to arrow syntax for calling functions
Signed-off-by: Stefan Hanreich --- src/PVE/Firewall.pm | 80 ++--- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 4a13926..efd53fc 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -1727,18 +1727,18 @@ sub verify_rule { if (my $value = $rule->{$name}) { if ($value =~ m/^\+/) { if ($value =~ m@^\+(guest/|dc/|sdn/)?(${ipset_name_pattern})$@) { - &$add_error($name, "no such ipset '$2'") + $add_error->($name, "no such ipset '$2'") if !($cluster_conf->{ipset}->{$2} || ($fw_conf && $fw_conf->{ipset}->{$2}) || ($cluster_conf->{sdn} && $cluster_conf->{sdn}->{ipset}->{$2})); } else { - &$add_error($name, "invalid ipset name '$value'"); + $add_error->($name, "invalid ipset name '$value'"); } } elsif ($value =~ m@^(guest/|dc/)?(${ip_alias_pattern})$@){ my $scope = $1 // ""; my $alias = lc($2); - &$add_error($name, "no such alias '$value'") + $add_error->($name, "no such alias '$value'") if !($cluster_conf->{aliases}->{$alias} || ($fw_conf && $fw_conf->{aliases}->{$alias})); my $e; @@ -1757,8 +1757,8 @@ sub verify_rule { my $type = $rule->{type}; my $action = $rule->{action}; -&$add_error('type', "missing property") if !$type; -&$add_error('action', "missing property") if !$action; +$add_error->('type', "missing property") if !$type; +$add_error->('action', "missing property") if !$action; if ($type) { my $valid_types = $rule_env_direction_lookup->{$rule_env} @@ -1774,22 +1774,22 @@ sub verify_rule { $add_error->('action', "unknown action '$action'") if $action && ($action !~ m/^(ACCEPT|DROP|REJECT)$/); } elsif ($type eq 'group') { - &$add_error('type', "security groups not allowed") + $add_error->('type', "security groups not allowed") if !$allow_groups; - &$add_error('action', "invalid characters in security group name") + $add_error->('action', "invalid characters in security group name") if $action && ($action !~ m/^${security_group_name_pattern}$/); } else { - &$add_error('type', "unknown rule type '$type'"); + $add_error->('type', "unknown rule type '$type'"); } } if ($rule->{iface}) { - &$add_error('type', "parameter -i not allowed for this rule type") + $add_error->('type', "parameter -i not allowed for this rule type") if !$allow_iface; eval { PVE::JSONSchema::pve_verify_iface($rule->{iface}); }; - &$add_error('iface', $@) if $@; + $add_error->('iface', $@) if $@; if ($rule_env eq 'vm' || $rule_env eq 'ct') { - &$add_error('iface', "value does not match the regex pattern 'net\\d+'") + $add_error->('iface', "value does not match the regex pattern 'net\\d+'") if $rule->{iface} !~ m/^net(\d+)$/; } } @@ -1798,14 +1798,14 @@ sub verify_rule { if (my $preferred_name = $pve_fw_preferred_macro_names->{lc($rule->{macro})}) { $rule->{macro} = $preferred_name; } else { - &$add_error('macro', "unknown macro '$rule->{macro}'"); + $add_error->('macro', "unknown macro '$rule->{macro}'"); } } my $is_icmp = 0; if ($rule->{proto}) { eval { pve_fw_verify_protocol_spec($rule->{proto}); }; - &$add_error('proto', $@) if $@; + $add_error->('proto', $@) if $@; &$set_ip_version(4) if $rule->{proto} eq 'icmp'; &$set_ip_version(6) if $rule->{proto} eq 'icmpv6'; &$set_ip_version(6) if $rule->{proto} eq 'ipv6-icmp'; @@ -1814,34 +1814,34 @@ sub verify_rule { if ($rule->{dport}) { eval { parse_port_name_number_or_range($rule->{dport}, $is_icmp); }; - &$add_error('dport', $@) if $@; + $add_error->('dport', $@) if $@; my $proto = $rule->{proto}; - &$add_error('proto', "missing property - 'dport' requires this property") + $add_error->('proto', "missing property - 'dport' requires this property") if !$proto; - &$add_error('dport', "protocol '$proto' does not support ports") + $add_error->('dport', "protocol '$proto' does not support ports") if !$PROTOCOLS_WITH_PORTS->{$proto} && !$is_icmp; #special cases } if (my $icmp_type = $rule ->{'icmp-type'}) { my $proto = $rule->{proto}; - &$add_error('proto', "missing property - 'icmp-type' requires this property") + $add_error->('proto', "missing property - 'icmp-type' requires this property") if !$is_icmp; -
[pve-devel] [PATCH proxmox-ve-rs v3 01/18] firewall: add forward direction
This direction will be used for specifying rules on bridge-level firewalls as well as rules on the cluster / host level that are for forwarded network packets. Since with the introduction of this direction not every type of firewall configuration can contain all types of directions, we additionally add validation logic to the parser, so rules with an invalid direction get rejected by the parser. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/cluster.rs| 11 +++ proxmox-ve-config/src/firewall/common.rs | 11 +++ proxmox-ve-config/src/firewall/guest.rs | 13 + proxmox-ve-config/src/firewall/host.rs | 12 +++- proxmox-ve-config/src/firewall/mod.rs| 1 + proxmox-ve-config/src/firewall/types/rule.rs | 10 -- 6 files changed, 55 insertions(+), 3 deletions(-) diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs index 223124b..ce3dd53 100644 --- a/proxmox-ve-config/src/firewall/cluster.rs +++ b/proxmox-ve-config/src/firewall/cluster.rs @@ -25,12 +25,15 @@ pub const CLUSTER_EBTABLES_DEFAULT: bool = false; pub const CLUSTER_POLICY_IN_DEFAULT: Verdict = Verdict::Drop; /// default setting for [`Config::default_policy()`] pub const CLUSTER_POLICY_OUT_DEFAULT: Verdict = Verdict::Accept; +/// default setting for [`Config::default_policy()`] +pub const CLUSTER_POLICY_FORWARD_DEFAULT: Verdict = Verdict::Accept; impl Config { pub fn parse(input: R) -> Result { let parser_config = ParserConfig { guest_iface_names: false, ipset_scope: Some(IpsetScope::Datacenter), +allowed_directions: vec![Direction::In, Direction::Out, Direction::Forward], }; Ok(Self { @@ -86,6 +89,11 @@ impl Config { .options .policy_out .unwrap_or(CLUSTER_POLICY_OUT_DEFAULT), +Direction::Forward => self +.config +.options +.policy_forward +.unwrap_or(CLUSTER_POLICY_FORWARD_DEFAULT), } } @@ -121,6 +129,7 @@ pub struct Options { policy_in: Option, policy_out: Option, +policy_forward: Option, } #[cfg(test)] @@ -148,6 +157,7 @@ log_ratelimit: 1,rate=10/second,burst=20 ebtables: 0 policy_in: REJECT policy_out: REJECT +policy_forward: DROP [ALIASES] @@ -191,6 +201,7 @@ IN BGP(REJECT) -log crit -source 1.2.3.4 )), policy_in: Some(Verdict::Reject), policy_out: Some(Verdict::Reject), +policy_forward: Some(Verdict::Drop), } ); diff --git a/proxmox-ve-config/src/firewall/common.rs b/proxmox-ve-config/src/firewall/common.rs index a08f19c..3999168 100644 --- a/proxmox-ve-config/src/firewall/common.rs +++ b/proxmox-ve-config/src/firewall/common.rs @@ -6,6 +6,7 @@ use serde::de::IntoDeserializer; use crate::firewall::parse::{parse_named_section_tail, split_key_value, SomeString}; use crate::firewall::types::ipset::{IpsetName, IpsetScope}; +use crate::firewall::types::rule::{Direction, Kind}; use crate::firewall::types::{Alias, Group, Ipset, Rule}; #[derive(Debug, Default)] @@ -34,6 +35,7 @@ pub struct ParserConfig { /// Network interfaces must be of the form `netX`. pub guest_iface_names: bool, pub ipset_scope: Option, +pub allowed_directions: Vec, } impl Config @@ -150,6 +152,15 @@ where } } +if let Kind::Match(rule) = rule.kind() { +if !parser_cfg.allowed_directions.contains(&rule.dir) { +bail!( +"found not allowed direction in firewall config: {0}", +rule.dir +); +} +} + self.rules.push(rule); Ok(()) } diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs index c7e282f..1e70a67 100644 --- a/proxmox-ve-config/src/firewall/guest.rs +++ b/proxmox-ve-config/src/firewall/guest.rs @@ -31,6 +31,8 @@ pub const GUEST_IPFILTER_DEFAULT: bool = false; pub const GUEST_POLICY_IN_DEFAULT: Verdict = Verdict::Drop; /// default return value for [`Config::default_policy()`] pub const GUEST_POLICY_OUT_DEFAULT: Verdict = Verdict::Accept; +/// default return value for [`Config::default_policy()`] +pub const GUEST_POLICY_FORWARD_DEFAULT: Verdict = Verdict::Accept; #[derive(Debug, Default, Deserialize)] #[cfg_attr(test, derive(Eq, PartialEq))] @@ -61,6 +63,8 @@ pub struct Options { #[serde(rename = "policy_out")] policy_out: Option, + +policy_forward: Option, } #[derive(Debug)] @@ -84,6 +88,7 @@ impl Config { let parser_cfg = super::common::ParserConfig { guest_iface_names: true, ipset_scope: Some(IpsetScope::Guest), +allowed_directions: vec![Direction::In, Direction::Out], }; let config = super
[pve-devel] [PATCH pve-manager v3 14/18] firewall: make base_url dynamically configurable in options component
This adds the ability to dynamically configure and change the base_url for the firewall options. This is needed for the SDN firewall dialog, that updates the firewall components based on the selected vnet. This avoids having to reinstantiate the component every time the user selects a new vnet. Signed-off-by: Stefan Hanreich --- www/manager6/grid/FirewallOptions.js | 38 ++-- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js index fa482e0e4..d98cbe22c 100644 --- a/www/manager6/grid/FirewallOptions.js +++ b/www/manager6/grid/FirewallOptions.js @@ -9,10 +9,6 @@ Ext.define('PVE.FirewallOptions', { initComponent: function() { var me = this; - if (!me.base_url) { - throw "missing base_url configuration"; - } - if (!['dc', 'node', 'vm', 'vnet'].includes(me.fwtype)) { throw "unknown firewall option type"; } @@ -197,23 +193,49 @@ Ext.define('PVE.FirewallOptions', { }; Ext.apply(me, { - url: "/api2/json" + me.base_url, tbar: [edit_btn], - editorConfig: { - url: '/api2/extjs/' + me.base_url, - }, listeners: { itemdblclick: () => { if (canEdit) { me.run_editor(); } }, selectionchange: set_button_status, }, }); + if (me.base_url) { + me.applyUrl(me.base_url); + } else { + me.rstore = Ext.create('Proxmox.data.ObjectStore', { + interval: me.interval, + extraParams: me.extraParams, + rows: me.rows, + }); + } + me.callParent(); me.on('activate', me.rstore.startUpdate); me.on('destroy', me.rstore.stopUpdate); me.on('deactivate', me.rstore.stopUpdate); }, +applyUrl: function(url) { + let me = this; + + Ext.apply(me, { + url: "/api2/json" + url, + editorConfig: { + url: '/api2/extjs/' + url, + }, + }); +}, +setBaseUrl: function(url) { + let me = this; + + me.base_url = url; + + me.applyUrl(url); + + me.rstore.getProxy().setConfig('url', `/api2/extjs/${url}`); + me.rstore.reload(); +}, }); -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 04/24] firewall: address: use new iprange type for ip entries
Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 81 +++ proxmox-ve-config/src/firewall/types/rule.rs | 6 +- 2 files changed, 31 insertions(+), 56 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index f7bde51..d269054 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -439,57 +439,30 @@ impl fmt::Display for AddressRange { #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpEntry { Cidr(Cidr), -Range(IpAddr, IpAddr), +Range(IpRange), } impl std::str::FromStr for IpEntry { type Err = Error; fn from_str(s: &str) -> Result { -if s.is_empty() { -bail!("Empty IP specification!") +if let Ok(cidr) = s.parse() { +return Ok(IpEntry::Cidr(cidr)); } -let entries: Vec<&str> = s -.split('-') -.take(3) // so we can check whether there are too many -.collect(); - -match entries.as_slice() { -[cidr] => Ok(IpEntry::Cidr(cidr.parse()?)), -[beg, end] => { -if let Ok(beg) = beg.parse::() { -if let Ok(end) = end.parse::() { -if beg < end { -return Ok(IpEntry::Range(beg.into(), end.into())); -} - -bail!("start address is greater than end address!"); -} -} - -if let Ok(beg) = beg.parse::() { -if let Ok(end) = end.parse::() { -if beg < end { -return Ok(IpEntry::Range(beg.into(), end.into())); -} - -bail!("start address is greater than end address!"); -} -} - -bail!("start and end are not valid IP addresses of the same type!") -} -_ => bail!("Invalid amount of elements in IpEntry!"), +if let Ok(range) = s.parse() { +return Ok(IpEntry::Range(range)); } + +bail!("Invalid IP entry: {s}"); } } impl fmt::Display for IpEntry { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { -Self::Cidr(ip) => write!(f, "{ip}"), -Self::Range(beg, end) => write!(f, "{beg}-{end}"), +Self::Cidr(ip) => ip.fmt(f), +Self::Range(range) => range.fmt(f), } } } @@ -498,19 +471,7 @@ impl IpEntry { fn family(&self) -> Family { match self { Self::Cidr(cidr) => cidr.family(), -Self::Range(start, end) => { -if start.is_ipv4() && end.is_ipv4() { -return Family::V4; -} - -if start.is_ipv6() && end.is_ipv6() { -return Family::V6; -} - -// should never be reached due to constructors validating that -// start type == end type -unreachable!("invalid IP entry") -} +Self::Range(range) => range.family(), } } } @@ -521,6 +482,12 @@ impl From for IpEntry { } } +impl From for IpEntry { +fn from(value: IpRange) -> Self { +IpEntry::Range(value) +} +} + #[derive(Clone, Debug, DeserializeFromStr)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct IpList { @@ -708,7 +675,9 @@ mod tests { assert_eq!( entry, -IpEntry::Range([192, 168, 0, 1].into(), [192, 168, 99, 255].into()) +IpRange::new_v4([192, 168, 0, 1], [192, 168, 99, 255]) +.expect("valid IP range") +.into() ); entry = "fe80::1".parse().expect("valid IP entry"); @@ -733,10 +702,12 @@ mod tests { assert_eq!( entry, -IpEntry::Range( -[0xFD80, 0, 0, 0, 0, 0, 0, 1].into(), -[0xFD80, 0, 0, 0, 0, 0, 0, 0x].into(), +IpRange::new_v6( +[0xFD80, 0, 0, 0, 0, 0, 0, 1], +[0xFD80, 0, 0, 0, 0, 0, 0, 0x], ) +.expect("valid IP range") +.into() ); "192.168.100.0-192.168.99.255" @@ -764,7 +735,9 @@ mod tests { entries: vec![ IpEntry::Cidr(Cidr::new_v4([192, 168, 0, 1], 32).unwrap()), IpEntry::Cidr(Cidr::new_v4([192, 168, 100, 0], 24).unwrap()), -IpEntry::Range([172, 16, 0, 0].into(), [172, 32, 255, 255].into()), +IpRange::new_v4([172, 16, 0, 0], [172, 32, 255, 255]) +.unwrap() +.into(), ], family: Family::V4, } diff --git a/proxmox-ve-config/src/firewall/types/r
[pve-devel] [PATCH docs/firewall/manager/proxmox{-ve-rs, -firewall, -perl-rs} v3 00/24] autogenerate ipsets for sdn objects
This patch series adds support for autogenerating ipsets for SDN objects. It autogenerates ipsets for every VNet as follows: * ipset containing all IP ranges of the VNet * ipset containing all gateways of the VNet * ipset containing all IP ranges of the subnet - except gateways * ipset containing all dhcp ranges of the vnet Additionally it generates an IPSet for every guest that has one or more IPAM entries in the pve IPAM. Those can then be used in the cluster / host / guest firewalls. Firewall rules automatically update on changes of the SDN / IPAM configuration. This patch series works for the old firewall as well as the new firewall. The ipsets in nftables currently get generated as named ipsets in every table, this means that the `nft list ruleset` output can get quite crowded for large SDN configurations or large IPAM databases. Another option would be to only include them as anonymous IPsets in the rules, which would make the nft output far less crowded but this way would use more memory when making extensive use of the sdn ipsets, since everytime it is used in a rule we create an entirely new ipset. The base for proxmox-ve-rs (which is a filtered version of the proxmox-firewall repository can be found here:) staff/s.hanreich/proxmox-ve-rs.git master Dependencies: * proxmox-perl-rs and proxmox-firewall depend on proxmox-ve-rs * pve-firewall depends on proxmox-perl-rs * pve-manager depends on pve-firewall Changes from v2: * rename end in IpRange to last to avoid confusion - thanks @Wolfgang * bump Rust to 1.82 - thanks @Wolfgang * improvements to the code generating IPSets - thanks @Wolfgang * implement AsRef for SDN name types - thanks @Wolfgang * improve docstrings (proper capitalization and punctuation) - thanks @Wolfgang * included a patch that removes proxmox-ve-config from proxmox-firewall Changes from RFC: * added documentation * added separate SDN scope for IPSets * rustfmt fixes proxmox-ve-rs: Stefan Hanreich (16): debian: add files for packaging firewall: add sdn scope for ipsets firewall: add ip range types firewall: address: use new iprange type for ip entries ipset: add range variant to addresses iprange: add methods for converting an ip range to cidrs ipset: address: add helper methods firewall: guest: derive traits according to rust api guidelines common: add allowlist sdn: add name types sdn: add ipam module sdn: ipam: add method for generating ipsets sdn: add config module sdn: config: add method for generating ipsets tests: add sdn config tests tests: add ipam tests .cargo/config.toml|5 + .gitignore|8 + Cargo.toml| 17 + Makefile | 69 + build.sh | 35 + bump.sh | 44 + proxmox-ve-config/Cargo.toml | 19 +- proxmox-ve-config/debian/changelog|5 + proxmox-ve-config/debian/control | 46 + proxmox-ve-config/debian/copyright| 19 + proxmox-ve-config/debian/debcargo.toml|4 + proxmox-ve-config/src/common/mod.rs | 31 + .../src/firewall/types/address.rs | 1171 - proxmox-ve-config/src/firewall/types/alias.rs |4 +- proxmox-ve-config/src/firewall/types/ipset.rs | 32 +- proxmox-ve-config/src/firewall/types/rule.rs |6 +- proxmox-ve-config/src/guest/types.rs |7 +- proxmox-ve-config/src/guest/vm.rs | 11 +- proxmox-ve-config/src/lib.rs |2 + proxmox-ve-config/src/sdn/config.rs | 640 + proxmox-ve-config/src/sdn/ipam.rs | 368 ++ proxmox-ve-config/src/sdn/mod.rs | 251 proxmox-ve-config/tests/sdn/main.rs | 189 +++ proxmox-ve-config/tests/sdn/resources/ipam.db | 26 + .../tests/sdn/resources/running-config.json | 54 + 25 files changed, 2976 insertions(+), 87 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 .gitignore create mode 100644 Cargo.toml create mode 100644 Makefile create mode 100755 build.sh create mode 100755 bump.sh create mode 100644 proxmox-ve-config/debian/changelog create mode 100644 proxmox-ve-config/debian/control create mode 100644 proxmox-ve-config/debian/copyright create mode 100644 proxmox-ve-config/debian/debcargo.toml create mode 100644 proxmox-ve-config/src/common/mod.rs create mode 100644 proxmox-ve-config/src/sdn/config.rs create mode 100644 proxmox-ve-config/src/sdn/ipam.rs create mode 100644 proxmox-ve-config/src/sdn/mod.rs create mode 100644 proxmox-ve-config/tests/sdn/main.rs create mode 100644 proxmox-ve-config/tests/sdn/resources/ipam.db create mode 100644 proxmox-ve-config/tests/sdn/resources/running-config.json proxmox-firewall: Stefan Hanreich (3): add proxmox-ve-rs crate - move proxmox-ve-config ther
[pve-devel] [PATCH pve-manager v3 12/18] firewall: add forward direction to rule panel
Enables us to use the new forward direction as an option when creating or editing firewall rules. By introducing firewall_type we can switch between the available directions depending on which ruleset is being edited. Signed-off-by: Stefan Hanreich --- www/manager6/dc/Config.js | 1 + www/manager6/dc/SecurityGroups.js | 1 + www/manager6/grid/FirewallRules.js | 81 +- www/manager6/lxc/Config.js | 1 + www/manager6/node/Config.js| 1 + www/manager6/qemu/Config.js| 1 + 6 files changed, 73 insertions(+), 13 deletions(-) diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index ddbb58b12..720edefc6 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -241,6 +241,7 @@ Ext.define('PVE.dc.Config', { list_refs_url: '/cluster/firewall/refs', iconCls: 'fa fa-shield', itemId: 'firewall', + firewall_type: 'dc', }, { xtype: 'pveFirewallOptions', diff --git a/www/manager6/dc/SecurityGroups.js b/www/manager6/dc/SecurityGroups.js index 9e26b84c9..e7aa8081c 100644 --- a/www/manager6/dc/SecurityGroups.js +++ b/www/manager6/dc/SecurityGroups.js @@ -214,6 +214,7 @@ Ext.define('PVE.SecurityGroups', { list_refs_url: '/cluster/firewall/refs', tbar_prefix: '' + gettext('Rules') + ':', border: false, + firewall_type: 'group', }, { xtype: 'pveSecurityGroupList', diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js index 11881bf79..e2809f02b 100644 --- a/www/manager6/grid/FirewallRules.js +++ b/www/manager6/grid/FirewallRules.js @@ -147,6 +147,24 @@ let ICMPV6_TYPE_NAMES_STORE = Ext.create('Ext.data.Store', { ], }); +let DEFAULT_ALLOWED_DIRECTIONS = ['in', 'out']; + +let ALLOWED_DIRECTIONS = { +'dc': ['in', 'out', 'forward'], +'node': ['in', 'out', 'forward'], +'group': ['in', 'out', 'forward'], +'vm': ['in', 'out'], +'vnet': ['forward'], +}; + +let DEFAULT_ALLOWED_ACTIONS = ['ACCEPT', 'REJECT', 'DROP']; + +let ALLOWED_ACTIONS = { +'in': ['ACCEPT', 'REJECT', 'DROP'], +'out': ['ACCEPT', 'REJECT', 'DROP'], +'forward': ['ACCEPT', 'DROP'], +}; + Ext.define('PVE.FirewallRulePanel', { extend: 'Proxmox.panel.InputPanel', @@ -154,6 +172,9 @@ Ext.define('PVE.FirewallRulePanel', { list_refs_url: undefined, +firewall_type: undefined, +action_selector: undefined, + onGetValues: function(values) { var me = this; @@ -171,6 +192,23 @@ Ext.define('PVE.FirewallRulePanel', { return values; }, +setValidActions: function(type) { + let me = this; + + let allowed_actions = ALLOWED_ACTIONS[type] ?? DEFAULT_ALLOWED_ACTIONS; + me.action_selector.setComboItems(allowed_actions.map((action) => [action, action])); +}, + +onSetValues: function(values) { + let me = this; + + if (values.type) { + me.setValidActions(values.type); + } + + return values; +}, + initComponent: function() { var me = this; @@ -178,6 +216,17 @@ Ext.define('PVE.FirewallRulePanel', { throw "no list_refs_url specified"; } + let allowed_directions = ALLOWED_DIRECTIONS[me.firewall_type] ?? DEFAULT_ALLOWED_DIRECTIONS; + + me.action_selector = Ext.create('Proxmox.form.KVComboBox', { + xtype: 'proxmoxKVComboBox', + name: 'action', + value: 'ACCEPT', + comboItems: DEFAULT_ALLOWED_ACTIONS.map((action) => [action, action]), + fieldLabel: gettext('Action'), + allowBlank: false, + }); + me.column1 = [ { // hack: we use this field to mark the form 'dirty' when the @@ -190,19 +239,17 @@ Ext.define('PVE.FirewallRulePanel', { { xtype: 'proxmoxKVComboBox', name: 'type', - value: 'in', - comboItems: [['in', 'in'], ['out', 'out']], + value: allowed_directions[0], + comboItems: allowed_directions.map((dir) => [dir, dir]), fieldLabel: gettext('Direction'), allowBlank: false, + listeners: { + change: function(f, value) { + me.setValidActions(value); +}, +}, }, - { - xtype: 'proxmoxKVComboBox', - name: 'action', - value: 'ACCEPT', - comboItems: [['ACCEPT', 'ACCEPT'], ['DROP', 'DROP'], ['REJECT', 'REJECT']], - fieldLabel: gettext('Action'), - allowBlank: false, - }, + me.action_selector, ]; if (me.allow_iface) { @@ -387,6 +434,8 @@ Ext.define('PVE.FirewallRuleEdit', { allow_iface: false, +firewall_type: undefined, +
[pve-devel] [PATCH proxmox-ve-rs v3 02/24] firewall: add sdn scope for ipsets
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/types/ipset.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index c1af642..6fbdca8 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -14,6 +14,7 @@ use crate::guest::vm::NetworkConfig; pub enum IpsetScope { Datacenter, Guest, +Sdn, } impl FromStr for IpsetScope { @@ -23,6 +24,7 @@ impl FromStr for IpsetScope { Ok(match s { "+dc" => IpsetScope::Datacenter, "+guest" => IpsetScope::Guest, +"+sdn" => IpsetScope::Sdn, _ => bail!("invalid scope for ipset: {s}"), }) } @@ -33,6 +35,7 @@ impl Display for IpsetScope { let prefix = match self { Self::Datacenter => "dc", Self::Guest => "guest", +Self::Sdn => "sdn", }; f.write_str(prefix) -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC qemu-server v3 21/34] backup: implement backup for external providers
some nits/comments/questions below, but the general direction/structure already looks quite good I think! On November 7, 2024 5:51 pm, Fiona Ebner wrote: > The state of the VM's disk images at the time the backup is started is > preserved via a snapshot-access block node. Old data is moved to the > fleecing image when new guest writes come in. The snapshot-access > block node, as well as the associated bitmap in case of incremental > backup, will be made available to the external provider. They are > exported via NBD and for 'nbd' mechanism, the NBD socket path is > passed to the provider, while for 'block-device' mechanism, the NBD > export is made accessible as a regular block device first and the > bitmap information is made available via a $next_dirty_region->() > function. For 'block-device', the 'nbdinfo' binary is required. > > The provider can indicate that it wants to do an incremental backup by > returning the bitmap ID that was used for a previous backup and it > will then be told if the bitmap was newly created (either first backup > or old bitmap was invalid) or if the bitmap can be reused. > > The provider then reads the parts of the NBD or block device it needs, > either the full disk for full backup, or the dirty parts according to > the bitmap for incremental backup. The bitmap has to be respected, > reads to other parts of the image will return an error. After backing > up each part of the disk, it should be discarded in the export to > avoid unnecessary space usage in the fleecing image (requires the > storage underlying the fleecing image to support discard too). > > Signed-off-by: Fiona Ebner > --- > > Changes in v3: > * adapt to API changes, config files are now passed as raw > > PVE/VZDump/QemuServer.pm | 309 ++- > 1 file changed, 308 insertions(+), 1 deletion(-) > > diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm > index b6dcd6cc..d0218c9b 100644 > --- a/PVE/VZDump/QemuServer.pm > +++ b/PVE/VZDump/QemuServer.pm > @@ -20,7 +20,7 @@ use PVE::QMPClient; > use PVE::Storage::Plugin; > use PVE::Storage::PBSPlugin; > use PVE::Storage; > -use PVE::Tools; > +use PVE::Tools qw(run_command); > use PVE::VZDump; > use PVE::Format qw(render_duration render_bytes); > > @@ -277,6 +277,8 @@ sub archive { > > if ($self->{vzdump}->{opts}->{pbs}) { > $self->archive_pbs($task, $vmid); > +} elsif ($self->{vzdump}->{'backup-provider'}) { > + $self->archive_external($task, $vmid); > } else { > $self->archive_vma($task, $vmid, $filename, $comp); > } > @@ -1149,6 +1151,23 @@ sub cleanup { > > # If VM was started only for backup, it is already stopped now. > if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) { > + if ($task->{cleanup}->{'nbd-stop'}) { > + eval { PVE::QemuServer::QMPHelpers::nbd_stop($vmid); }; > + $self->logerr($@) if $@; > + } > + > + if (my $info = $task->{cleanup}->{'backup-access-teardown'}) { > + my $params = { > + 'target-id' => $info->{'target-id'}, > + timeout => 60, > + success => $info->{success} ? JSON::true : JSON::false, > + }; > + > + $self->loginfo("tearing down backup-access"); > + eval { mon_cmd($vmid, "backup-access-teardown", $params->%*) }; > + $self->logerr($@) if $@; > + } > + > $detach_tpmstate_drive->($task, $vmid); > detach_fleecing_images($task->{disks}, $vmid) if > $task->{'use-fleecing'}; > } > @@ -1160,4 +1179,292 @@ sub cleanup { > } > } > > +my sub block_device_backup_cleanup { > +my ($self, $paths, $cpids) = @_; > + > +for my $path ($paths->@*) { > + eval { run_command(["qemu-nbd", "-d", $path ]); }; > + $self->log('warn', "unable to disconnect NBD backup source '$path' - > $@") if $@; > +} > + > +my $waited; > +my $wait_limit = 5; > +for ($waited = 0; $waited < $wait_limit && scalar(keys $cpids->%*); > $waited++) { > + while ((my $cpid = waitpid(-1, POSIX::WNOHANG)) > 0) { > + delete($cpids->{$cpid}); > + } > + if ($waited == 0) { > + kill 15, $_ for keys $cpids->%*; > + } > + sleep 1; > +} > +if ($waited == $wait_limit && scalar(keys $cpids->%*)) { > + kill 9, $_ for keys $cpids->%*; > + sleep 1; > + while ((my $cpid = waitpid(-1, POSIX::WNOHANG)) > 0) { this could be a bit dangerous, since we have an explicit list of cpids we want to wait for, couldn't we just waitpid explicitly for them? just wary of potential side-effects on things like hookscripts or future features that also require forking ;) > + delete($cpids->{$cpid}); > + } > + $self->log('warn', "unable to collect nbdinfo child process '$_'") for > keys $cpids->%*; > +} > +} > + > +my sub block_device_backup_prepare { > +my ($self, $devicename, $size, $nbd_path, $bitmap_name, $count) = @_; nit: $device_name for consistency's sak
[pve-devel] [PATCH pve-docs v3 18/18] firewall: add documentation for forward direction
Additionally add information about the SDN VNet firewall, which has been introduced with this changes. Signed-off-by: Stefan Hanreich --- Makefile | 1 + gen-pve-firewall-vnet-opts.pl | 12 +++ pve-firewall-vnet-opts.adoc | 8 + pve-firewall.adoc | 65 +++ 4 files changed, 80 insertions(+), 6 deletions(-) create mode 100755 gen-pve-firewall-vnet-opts.pl create mode 100644 pve-firewall-vnet-opts.adoc diff --git a/Makefile b/Makefile index 801a2a3..f30d77a 100644 --- a/Makefile +++ b/Makefile @@ -62,6 +62,7 @@ GEN_SCRIPTS= \ gen-pve-firewall-macros-adoc.pl \ gen-pve-firewall-rules-opts.pl \ gen-pve-firewall-vm-opts.pl \ + gen-pve-firewall-vnet-opts.pl \ gen-output-format-opts.pl API_VIEWER_FILES= \ diff --git a/gen-pve-firewall-vnet-opts.pl b/gen-pve-firewall-vnet-opts.pl new file mode 100755 index 000..c9f4f13 --- /dev/null +++ b/gen-pve-firewall-vnet-opts.pl @@ -0,0 +1,12 @@ +#!/usr/bin/perl + +use lib '.'; +use strict; +use warnings; + +use PVE::Firewall; +use PVE::RESTHandler; + +my $prop = $PVE::Firewall::vnet_option_properties; + +print PVE::RESTHandler::dump_properties($prop); diff --git a/pve-firewall-vnet-opts.adoc b/pve-firewall-vnet-opts.adoc new file mode 100644 index 000..ed1e88f --- /dev/null +++ b/pve-firewall-vnet-opts.adoc @@ -0,0 +1,8 @@ +`enable`: `` ('default =' `0`):: + +Enable/disable firewall rules. + +`policy_forward`: `` :: + +Forward policy. + diff --git a/pve-firewall.adoc b/pve-firewall.adoc index b428703..d5c664f 100644 --- a/pve-firewall.adoc +++ b/pve-firewall.adoc @@ -48,18 +48,34 @@ there is no need to maintain a different set of rules for IPv6. Zones - -The Proxmox VE firewall groups the network into the following logical zones: +The Proxmox VE firewall groups the network into the following logical zones. +Depending on the zone, you can define firewall rules for incoming, outgoing or +forwarded traffic. Host:: -Traffic from/to a cluster node +Traffic going from/to a host or traffic that is forwarded by a host. + +You can define rules for this zone either at the datacenter level or at the node +level. Rules at node level take precedence over rules at datacenter level. VM:: -Traffic from/to a specific VM +Traffic going from/to a VM or CT. + +You cannot define rules for the forward direction, only for incoming / outgoing. + +VNet:: -For each zone, you can define firewall rules for incoming and/or -outgoing traffic. +Traffic passing through a SDN VNet, either from guest to guest or from host to +guest and vice-versa. Since this traffic is always forwarded traffic, it is only +possible to create rules with direction forward. + + +IMPORTANT: Creating rules for forwarded traffic or on a VNet-level is currently +only possible when using the new +xref:pve_firewall_nft[nftables-based proxmox-firewall]. Any forward rules will be +ignored by the stock `pve-firewall` and have no effect! Configuration Files @@ -202,10 +218,46 @@ can selectively enable the firewall for each interface. This is required in addition to the general firewall `enable` option. +[[pve_firewall_vnet_configuration]] +VNet Configuration +~~ +VNet related configuration is read from: + + /etc/pve/sdn/firewall/.fw + +This can be used for setting firewall configuration globally on a VNet level, +without having to set firewall rules for each VM inside the VNet separately. It +can only contain rules for the `FORWARD` direction, since there is no notion of +incoming or outgoing traffic. This affects all traffic travelling from one +bridge port to another, including the host interface. + +WARNING: This feature is currently only available for the new +xref:pve_firewall_nft[nftables-based proxmox-firewall] + +Since traffic passing the `FORWARD` chain is bi-directional, you need to create +rules for both directions if you want traffic to pass both ways. For instance if +HTTP traffic for a specific host should be allowed, you would need to create the +following rules: + + +FORWARD ACCEPT -dest 10.0.0.1 -dport 80 +FORWARD ACCEPT -source 10.0.0.1 -sport 80 + + +`[OPTIONS]`:: + +This is used to set VNet related firewall options. + +include::pve-firewall-vnet-opts.adoc[] + +`[RULES]`:: + +This section contains VNet specific firewall rules. + Firewall Rules -- -Firewall rules consists of a direction (`IN` or `OUT`) and an +Firewall rules consists of a direction (`IN`, `OUT` or `FORWARD`) and an action (`ACCEPT`, `DENY`, `REJECT`). You can also specify a macro name. Macros contain predefined sets of rules and options. Rules can be disabled by prefixing them with `|`. @@ -639,6 +691,7 @@ Ports used by {pve} * live migration (VM memory and local-disk data): 6-60050 (TCP) +[[pve_firewall_nft]] nf
[pve-devel] [PATCH proxmox-ve-rs v3 14/24] sdn: config: add method for generating ipsets
We generate the following ipsets for every vnet in the running sdn configuration: * {vnet}-all: contains all subnets of the vnet * {vnet}-no-gateway: contains all subnets of the vnet except for all gateways * {vnet}-gateway: contains all gateways in the vnet * {vnet}-dhcp: contains all dhcp ranges configured in the vnet All of them are in the new SDN scope, so the fully qualified name would look something like this: `+sdn/{vnet-all}`. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/sdn/config.rs | 70 + 1 file changed, 70 insertions(+) diff --git a/proxmox-ve-config/src/sdn/config.rs b/proxmox-ve-config/src/sdn/config.rs index b71084b..7ee1101 100644 --- a/proxmox-ve-config/src/sdn/config.rs +++ b/proxmox-ve-config/src/sdn/config.rs @@ -529,6 +529,76 @@ impl SdnConfig { self.zones() .flat_map(|zone| zone.vnets().map(move |vnet| (zone, vnet))) } + +/// Generates multiple [`Ipset`] for all SDN VNets. +/// +/// # Arguments +/// * `filter` - A [`Allowlist`] of VNet names for which IPsets should get returned +/// +/// It generates the following [`Ipset`] for all VNets in the config: +/// * all: Contains all CIDRs of all subnets in the VNet +/// * gateway: Contains all gateways of all subnets in the VNet (if any gateway exists) +/// * no-gateway: Matches all CIDRs of all subnets, except for the gateways (if any gateway +/// exists) +/// * dhcp: Contains all DHCP ranges of all subnets in the VNet (if any dhcp range exists) +pub fn ipsets<'a>( +&'a self, +filter: Option<&'a Allowlist>, +) -> impl Iterator + '_ { +self.zones +.values() +.flat_map(|zone| zone.vnets()) +.filter(move |vnet| { +filter +.map(|list| list.is_allowed(&vnet.name)) +.unwrap_or(true) +}) +.flat_map(|vnet| { +let mut ipset_all = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-all", vnet.name), +)); +ipset_all.comment = Some(format!("All subnets of VNet {}", vnet.name)); + +let mut ipset_gateway = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-gateway", vnet.name), +)); +ipset_gateway.comment = Some(format!("All gateways of VNet {}", vnet.name)); + +let mut ipset_all_wo_gateway = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-no-gateway", vnet.name), +)); +ipset_all_wo_gateway.comment = Some(format!( +"All subnets of VNet {}, excluding gateways", +vnet.name +)); + +let mut ipset_dhcp = Ipset::new(IpsetName::new( +IpsetScope::Sdn, +format!("{}-dhcp", vnet.name), +)); +ipset_dhcp.comment = Some(format!("DHCP ranges of VNet {}", vnet.name)); + +for subnet in vnet.subnets.values() { +ipset_all.push((*subnet.cidr()).into()); + +ipset_all_wo_gateway.push((*subnet.cidr()).into()); + +if let Some(gateway) = subnet.gateway { +let gateway_nomatch = IpsetEntry::new(gateway, true, None); +ipset_all_wo_gateway.push(gateway_nomatch); + +ipset_gateway.push(gateway.into()); +} + + ipset_dhcp.extend(subnet.dhcp_range.iter().cloned().map(IpsetEntry::from)); +} + +[ipset_all, ipset_gateway, ipset_all_wo_gateway, ipset_dhcp] +}) +} } impl TryFrom for SdnConfig { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 06/24] iprange: add methods for converting an ip range to cidrs
This is mainly used in proxmox-perl-rs, so the generated ipsets can be used in pve-firewall where only CIDRs are supported. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 818 ++ 1 file changed, 818 insertions(+) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index d269054..95c58a7 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -303,6 +303,17 @@ impl IpRange { ) -> Result { Ok(IpRange::V6(AddressRange::new_v6(start, last)?)) } + +/// Converts an IpRange into the minimal amount of CIDRs. +/// +/// see the concrete implementations of [`AddressRange`] or [`AddressRange`] +/// respectively +pub fn to_cidrs(&self) -> Vec { +match self { +IpRange::V4(range) => range.to_cidrs().into_iter().map(Cidr::from).collect(), +IpRange::V6(range) => range.to_cidrs().into_iter().map(Cidr::from).collect(), +} +} } impl std::str::FromStr for IpRange { @@ -362,6 +373,71 @@ impl AddressRange { Ok(Self { start, last }) } + +/// Returns the minimum amount of CIDRs that exactly represent the range +/// +/// The idea behind this algorithm is as follows: +/// +/// Start iterating with current = start of the IP range +/// +/// Find two netmasks +/// * The largest CIDR that the current IP can be the first of +/// * The largest CIDR that *only* contains IPs from current - last +/// +/// Add the smaller of the two CIDRs to our result and current to the first IP that is in +/// the range but not in the CIDR we just added. Proceed until we reached the last of the IP +/// range. +/// +pub fn to_cidrs(&self) -> Vec { +let mut cidrs = Vec::new(); + +let mut current = u32::from_be_bytes(self.start.octets()); +let last = u32::from_be_bytes(self.last.octets()); + +if current == last { +// valid Ipv4 since netmask is 32 +cidrs.push(Ipv4Cidr::new(current, 32).unwrap()); +return cidrs; +} + +// special case this, since this is the only possibility of overflow +// when calculating delta_min_mask - makes everything a lot easier +if current == u32::MIN && last == u32::MAX { +// valid Ipv4 since it is `0.0.0.0/0` +cidrs.push(Ipv4Cidr::new(current, 0).unwrap()); +return cidrs; +} + +while current <= last { +// netmask of largest CIDR that current IP can be the first of +// cast is safe, because trailing zeroes can at most be 32 +let current_max_mask = IPV4_LENGTH - (current.trailing_zeros() as u8); + +// netmask of largest CIDR that *only* contains IPs of the remaining range +// is at most 32 due to unwrap_or returning 32 and ilog2 being at most 31 +let delta_min_mask = ((last - current) + 1) // safe due to special case above +.checked_ilog2() // should never occur due to special case, but for good measure +.map(|mask| IPV4_LENGTH - mask as u8) +.unwrap_or(IPV4_LENGTH); + +// at most 32, due to current/delta being at most 32 +let netmask = u8::max(current_max_mask, delta_min_mask); + +// netmask is at most 32, therefore safe to unwrap +cidrs.push(Ipv4Cidr::new(current, netmask).unwrap()); + +let delta = 2u32.saturating_pow((IPV4_LENGTH - netmask).into()); + +if let Some(result) = current.checked_add(delta) { +current = result +} else { +// we reached the end of IP address space +break; +} +} + +cidrs +} } impl AddressRange { @@ -377,6 +453,61 @@ impl AddressRange { Ok(Self { start, last }) } + +/// Returns the minimum amount of CIDRs that exactly represent the [`AddressRange`]. +/// +/// This function works analogous to the IPv4 version, please refer to the respective +/// documentation of [`AddressRange`] +pub fn to_cidrs(&self) -> Vec { +let mut cidrs = Vec::new(); + +let mut current = u128::from_be_bytes(self.start.octets()); +let last = u128::from_be_bytes(self.last.octets()); + +if current == last { +// valid Ipv6 since netmask is 128 +cidrs.push(Ipv6Cidr::new(current, 128).unwrap()); +return cidrs; +} + +// special case this, since this is the only possibility of overflow +// when calculating delta_min_mask - makes everything a lot easier +if current == u128::MIN && last == u128::MAX { +// valid Ipv6 since it is `::/0` +cidrs.push(Ipv6Cidr::new(current, 0).unwrap()); +return cidrs; +
[pve-devel] [PATCH proxmox-ve-rs v3 10/24] sdn: add name types
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/lib.rs | 1 + proxmox-ve-config/src/sdn/mod.rs | 248 +++ 2 files changed, 249 insertions(+) create mode 100644 proxmox-ve-config/src/sdn/mod.rs diff --git a/proxmox-ve-config/src/lib.rs b/proxmox-ve-config/src/lib.rs index 1b6feae..d17136c 100644 --- a/proxmox-ve-config/src/lib.rs +++ b/proxmox-ve-config/src/lib.rs @@ -2,3 +2,4 @@ pub mod common; pub mod firewall; pub mod guest; pub mod host; +pub mod sdn; diff --git a/proxmox-ve-config/src/sdn/mod.rs b/proxmox-ve-config/src/sdn/mod.rs new file mode 100644 index 000..0752631 --- /dev/null +++ b/proxmox-ve-config/src/sdn/mod.rs @@ -0,0 +1,248 @@ +use std::{error::Error, fmt::Display, str::FromStr}; + +use serde_with::DeserializeFromStr; + +use crate::firewall::types::Cidr; + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum SdnNameError { +Empty, +TooLong, +InvalidSymbols, +InvalidSubnetCidr, +InvalidSubnetFormat, +} + +impl Error for SdnNameError {} + +impl Display for SdnNameError { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +f.write_str(match self { +SdnNameError::TooLong => "name too long", +SdnNameError::InvalidSymbols => "invalid symbols in name", +SdnNameError::InvalidSubnetCidr => "invalid cidr in name", +SdnNameError::InvalidSubnetFormat => "invalid format for subnet name", +SdnNameError::Empty => "name is empty", +}) +} +} + +fn validate_sdn_name(name: &str) -> Result<(), SdnNameError> { +if name.is_empty() { +return Err(SdnNameError::Empty); +} + +if name.len() > 8 { +return Err(SdnNameError::TooLong); +} + +// safe because of empty check +if !name.chars().next().unwrap().is_ascii_alphabetic() { +return Err(SdnNameError::InvalidSymbols); +} + +if !name.chars().all(|c| c.is_ascii_alphanumeric()) { +return Err(SdnNameError::InvalidSymbols); +} + +Ok(()) +} + +/// represents the name of an sdn zone +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, DeserializeFromStr)] +pub struct ZoneName(String); + +impl ZoneName { +/// construct a new zone name +/// +/// # Errors +/// +/// This function will return an error if the name is empty, too long (>8 characters), starts +/// with a non-alphabetic symbol or if there are non alphanumeric symbols contained in the name. +pub fn new(name: String) -> Result { +validate_sdn_name(&name)?; +Ok(ZoneName(name)) +} +} + +impl AsRef for ZoneName { +fn as_ref(&self) -> &str { +self.0.as_ref() +} +} + +impl FromStr for ZoneName { +type Err = SdnNameError; + +fn from_str(s: &str) -> Result { +Self::new(s.to_owned()) +} +} + +impl Display for ZoneName { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +self.0.fmt(f) +} +} + +/// represents the name of an sdn vnet +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, DeserializeFromStr)] +pub struct VnetName(String); + +impl VnetName { +/// construct a new vnet name +/// +/// # Errors +/// +/// This function will return an error if the name is empty, too long (>8 characters), starts +/// with a non-alphabetic symbol or if there are non alphanumeric symbols contained in the name. +pub fn new(name: String) -> Result { +validate_sdn_name(&name)?; +Ok(VnetName(name)) +} + +pub fn name(&self) -> &str { +&self.0 +} +} + +impl AsRef for VnetName { +fn as_ref(&self) -> &str { +self.0.as_ref() +} +} + +impl FromStr for VnetName { +type Err = SdnNameError; + +fn from_str(s: &str) -> Result { +Self::new(s.to_owned()) +} +} + +impl Display for VnetName { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +self.0.fmt(f) +} +} + +/// represents the name of an sdn subnet +/// +/// # Textual representation +/// A subnet name has the form `{zone_id}-{cidr_ip}-{cidr_mask}` +#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, DeserializeFromStr)] +pub struct SubnetName(ZoneName, Cidr); + +impl SubnetName { +pub fn new(zone: ZoneName, cidr: Cidr) -> Self { +SubnetName(zone, cidr) +} + +pub fn zone(&self) -> &ZoneName { +&self.0 +} + +pub fn cidr(&self) -> &Cidr { +&self.1 +} +} + +impl FromStr for SubnetName { +type Err = SdnNameError; + +fn from_str(s: &str) -> Result { +if let Some((name, cidr_part)) = s.split_once('-') { +if let Some((ip, netmask)) = cidr_part.split_once('-') { +let zone_name = ZoneName::from_str(name)?; + +let cidr: Cidr = format!("{ip}/{netmask}") +.parse() +.map_err(|_| SdnNameError::InvalidSubnetCidr)?; + +
[pve-devel] [PATCH pve-firewall v3 20/24] add support for loading sdn firewall configuration
This also includes support for parsing rules referencing IPSets in the new SDN scope and generating those IPSets in the firewall. Loading SDN configuration is optional, since loading it requires root privileges which we do not have in all call sites. Adding the flag allows us to selectively load the SDN configuration only where required and at the same time allows us to only elevate privileges in the API where absolutely needed. Signed-off-by: Stefan Hanreich --- src/PVE/Firewall.pm | 59 +++-- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 09544ba..6e02873 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -25,6 +25,7 @@ use PVE::Tools qw($IPV4RE $IPV6RE); use PVE::Tools qw(run_command lock_file dir_glob_foreach); use PVE::Firewall::Helpers; +use PVE::RS::Firewall::SDN; my $pvefw_conf_dir = "/etc/pve/firewall"; my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw"; @@ -1689,9 +1690,11 @@ sub verify_rule { if (my $value = $rule->{$name}) { if ($value =~ m/^\+/) { - if ($value =~ m@^\+(guest/|dc/)?(${ipset_name_pattern})$@) { + if ($value =~ m@^\+(guest/|dc/|sdn/)?(${ipset_name_pattern})$@) { &$add_error($name, "no such ipset '$2'") - if !($cluster_conf->{ipset}->{$2} || ($fw_conf && $fw_conf->{ipset}->{$2})); + if !($cluster_conf->{ipset}->{$2} + || ($fw_conf && $fw_conf->{ipset}->{$2}) + || ($cluster_conf->{sdn} && $cluster_conf->{sdn}->{ipset}->{$2})); } else { &$add_error($name, "invalid ipset name '$value'"); @@ -2108,13 +2111,15 @@ sub ipt_gen_src_or_dst_match { my $match; if ($adr =~ m/^\+/) { - if ($adr =~ m@^\+(guest/|dc/)?(${ipset_name_pattern})$@) { + if ($adr =~ m@^\+(guest/|dc/|sdn/)?(${ipset_name_pattern})$@) { my $scope = $1 // ""; my $name = $2; my $ipset_chain; - if ($scope ne 'dc/' && $fw_conf && $fw_conf->{ipset}->{$name}) { + if ((!$scope || $scope eq 'guest/') && $fw_conf && $fw_conf->{ipset}->{$name}) { $ipset_chain = compute_ipset_chain_name($fw_conf->{vmid}, $name, $ipversion); - } elsif ($scope ne 'guest/' && $cluster_conf && $cluster_conf->{ipset}->{$name}) { + } elsif ((!$scope || $scope eq 'dc/') && $cluster_conf && $cluster_conf->{ipset}->{$name}) { + $ipset_chain = compute_ipset_chain_name(0, $name, $ipversion); + } elsif ((!$scope || $scope eq 'sdn/') && $cluster_conf->{sdn} && $cluster_conf->{sdn}->{ipset}->{$name}) { $ipset_chain = compute_ipset_chain_name(0, $name, $ipversion); } else { die "no such ipset '$name'\n"; @@ -3644,7 +3649,12 @@ sub lock_clusterfw_conf { } sub load_clusterfw_conf { -my ($filename) = @_; +my ($filename, $options) = @_; + +my $sdn_conf = {}; +if ($options->{load_sdn_config}) { + $sdn_conf = load_sdn_conf(); +} $filename = $clusterfw_conf_filename if !defined($filename); my $empty_conf = { @@ -3655,6 +3665,7 @@ sub load_clusterfw_conf { group_comments => {}, ipset => {} , ipset_comments => {}, + sdn => $sdn_conf, }; my $cluster_conf = generic_fw_config_parser($filename, $empty_conf, $empty_conf, 'cluster'); @@ -3663,6 +3674,39 @@ sub load_clusterfw_conf { return $cluster_conf; } +sub load_sdn_conf { +my $rpcenv = PVE::RPCEnvironment::get(); +my $authuser = $rpcenv->get_user(); + +my $guests = PVE::Cluster::get_vmlist(); +my $allowed_vms = []; +foreach my $vmid (sort keys %{$guests->{ids}}) { + next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ], 1); + push @$allowed_vms, $vmid; +} + +my $vnets = PVE::Network::SDN::Vnets::config(1); +my $privs = [ 'SDN.Audit', 'SDN.Allocate' ]; +my $allowed_vnets = []; +foreach my $vnet (sort keys %{$vnets->{ids}}) { + my $zone = $vnets->{ids}->{$vnet}->{zone}; + next if !$rpcenv->check_any($authuser, "/sdn/zones/$zone/$vnet", $privs, 1); + push @$allowed_vnets, $vnet; +} + +my $sdn_config = { + ipset => {} , + ipset_comments => {}, +}; + +eval { + $sdn_config = PVE::RS::Firewall::SDN::config($allowed_vnets, $allowed_vms); +}; +warn $@ if $@; + +return $sdn_config; +} + sub save_clusterfw_conf { my ($cluster_conf) = @_; @@ -4043,6 +4087,7 @@ sub compile_ipsets { } generate_ipset_chains($ipset_ruleset, undef, $cluster_conf, undef, $cluster_conf->{ipset}); +generate_ipset_chains($ipset_ruleset, undef, $cluster_conf, undef, $cluster_conf->{sdn}->{ipset}); return $ipset_ruleset; } @@ -4731,7 +4776,7 @@ sub init { sub update { my $code = sub {
[pve-devel] [PATCH proxmox-firewall v3 19/24] ipsets: autogenerate ipsets for vnets and ipam
They act like virtual ipsets, similar to ipfilter-net, that can be used for defining firewall rules for sdn objects dynamically. The changes in proxmox-ve-config also introduced a dedicated struct for representing ip ranges, so we update the existing code, so that it uses that struct as well. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/firewall.rs | 22 +- proxmox-firewall/src/object.rs| 41 +- .../integration_tests__firewall.snap | 1288 + proxmox-nftables/src/expression.rs| 17 +- 4 files changed, 1354 insertions(+), 14 deletions(-) diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 941aa20..347f3af 100644 --- a/proxmox-firewall/src/firewall.rs +++ b/proxmox-firewall/src/firewall.rs @@ -197,6 +197,27 @@ impl Firewall { self.reset_firewall(&mut commands); let cluster_host_table = Self::cluster_table(); +let guest_table = Self::guest_table(); + +if let Some(sdn_config) = self.config.sdn() { +let ipsets = sdn_config +.ipsets(None) +.map(|ipset| (ipset.name().to_string(), ipset)) +.collect(); + +self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; +self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; +} + +if let Some(ipam_config) = self.config.ipam() { +let ipsets = ipam_config +.ipsets(None) +.map(|ipset| (ipset.name().to_string(), ipset)) +.collect(); + +self.create_ipsets(&mut commands, &ipsets, &cluster_host_table, None)?; +self.create_ipsets(&mut commands, &ipsets, &guest_table, None)?; +} if self.config.host().is_enabled() { log::info!("creating cluster / host configuration"); @@ -242,7 +263,6 @@ impl Firewall { commands.push(Delete::table(TableName::from(Self::cluster_table(; } -let guest_table = Self::guest_table(); let enabled_guests: BTreeMap<&Vmid, &GuestConfig> = self .config .guests() diff --git a/proxmox-firewall/src/object.rs b/proxmox-firewall/src/object.rs index 32c4ddb..cf7e773 100644 --- a/proxmox-firewall/src/object.rs +++ b/proxmox-firewall/src/object.rs @@ -72,20 +72,37 @@ impl ToNftObjects for Ipset { let mut nomatch_elements = Vec::new(); for element in self.iter() { -let cidr = match &element.address { -IpsetAddress::Cidr(cidr) => cidr, -IpsetAddress::Alias(alias) => env -.alias(alias) -.ok_or(format_err!("could not find alias {alias} in environment"))? -.address(), +let expression = match &element.address { +IpsetAddress::Range(range) => { +if family != range.family() { +continue; +} + +Expression::from(range) +} +IpsetAddress::Cidr(cidr) => { +if family != cidr.family() { +continue; +} + +Expression::from(Prefix::from(cidr)) +} +IpsetAddress::Alias(alias) => { +let cidr = env +.alias(alias) +.ok_or_else(|| { +format_err!("could not find alias {alias} in environment") +})? +.address(); + +if family != cidr.family() { +continue; +} + +Expression::from(Prefix::from(cidr)) +} }; -if family != cidr.family() { -continue; -} - -let expression = Expression::from(Prefix::from(cidr)); - if element.nomatch { nomatch_elements.push(expression); } else { diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap index 40d4405..e1b599c 100644 --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap @@ -202,6 +202,1294 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")" } } }, +{ + "add": { +"set": { + "family": "inet", + "table": "proxmox-firewall", + "name": "v4-sdn/public-all", + "type": "ipv4_addr", + "flags": [ +"interval" + ] +
[pve-devel] [PATCH proxmox-ve-rs v3 08/24] firewall: guest: derive traits according to rust api guidelines
Almost every type should implement them anyway, and many of them are required for those types to be used in BTreeMaps, which the nftables firewall uses for generating stable output. Additionally, we derive Serialize and Deserialize for a few types that occur in the sdn configuration. The following patches will use those for (de-)serialization. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 19 +++ proxmox-ve-config/src/firewall/types/alias.rs | 4 ++-- proxmox-ve-config/src/firewall/types/ipset.rs | 6 +++--- proxmox-ve-config/src/guest/types.rs | 7 --- proxmox-ve-config/src/guest/vm.rs | 7 --- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 938b8e1..57efb13 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -30,8 +30,9 @@ impl fmt::Display for Family { } } -#[derive(Clone, Copy, Debug)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive( +Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash, SerializeDisplay, DeserializeFromStr, +)] pub enum Cidr { Ipv4(Ipv4Cidr), Ipv6(Ipv6Cidr), @@ -101,8 +102,7 @@ impl From for Cidr { const IPV4_LENGTH: u8 = 32; -#[derive(Clone, Copy, Debug)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] pub struct Ipv4Cidr { addr: Ipv4Addr, mask: u8, @@ -176,8 +176,7 @@ impl fmt::Display for Ipv4Cidr { const IPV6_LENGTH: u8 = 128; -#[derive(Clone, Copy, Debug)] -#[cfg_attr(test, derive(Eq, PartialEq))] +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] pub struct Ipv6Cidr { addr: Ipv6Addr, mask: u8, @@ -271,7 +270,9 @@ impl Display for IpRangeError { /// Represents a range of IPv4 or IPv6 addresses. /// /// For more information see [`AddressRange`] -#[derive(Clone, Copy, Debug, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)] +#[derive( +Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] pub enum IpRange { V4(AddressRange), V6(AddressRange), @@ -364,7 +365,9 @@ impl fmt::Display for IpRange { /// # Textual representation /// /// Two IP addresses separated by a hyphen, e.g.: `127.0.0.1-127.0.0.255` -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive( +Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] pub struct AddressRange { start: T, last: T, diff --git a/proxmox-ve-config/src/firewall/types/alias.rs b/proxmox-ve-config/src/firewall/types/alias.rs index e6aa30d..5dfaa41 100644 --- a/proxmox-ve-config/src/firewall/types/alias.rs +++ b/proxmox-ve-config/src/firewall/types/alias.rs @@ -2,7 +2,7 @@ use std::fmt::Display; use std::str::FromStr; use anyhow::{bail, format_err, Error}; -use serde_with::DeserializeFromStr; +use serde_with::{DeserializeFromStr, SerializeDisplay}; use crate::firewall::parse::{match_name, match_non_whitespace}; use crate::firewall::types::address::Cidr; @@ -35,7 +35,7 @@ impl Display for AliasScope { } } -#[derive(Debug, Clone, DeserializeFromStr)] +#[derive(Debug, Clone, DeserializeFromStr, SerializeDisplay)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct AliasName { scope: AliasScope, diff --git a/proxmox-ve-config/src/firewall/types/ipset.rs b/proxmox-ve-config/src/firewall/types/ipset.rs index 7511490..fe5a930 100644 --- a/proxmox-ve-config/src/firewall/types/ipset.rs +++ b/proxmox-ve-config/src/firewall/types/ipset.rs @@ -88,7 +88,7 @@ impl Display for IpsetName { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub enum IpsetAddress { Alias(AliasName), @@ -124,7 +124,7 @@ impl From for IpsetAddress { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct IpsetEntry { pub nomatch: bool, @@ -208,7 +208,7 @@ impl Ipfilter<'_> { } } -#[derive(Debug)] +#[derive(Debug, Clone)] #[cfg_attr(test, derive(Eq, PartialEq))] pub struct Ipset { pub name: IpsetName, diff --git a/proxmox-ve-config/src/guest/types.rs b/proxmox-ve-config/src/guest/types.rs index 217c537..ed6a48c 100644 --- a/proxmox-ve-config/src/guest/types.rs +++ b/proxmox-ve-config/src/guest/types.rs @@ -2,8 +2,11 @@ use std::fmt; use std::str::FromStr; use anyhow::{format_err, Error}; +use serde_with::{DeserializeFromStr, SerializeDisplay}; -#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] +#[derive( +Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, SerializeDisplay, DeserializeFromStr, +)] pub struct Vmid(u32); impl Vmid { @@ -34,5 +37,3 @@ impl FromStr for Vmid { )) } } - -serde_plain::derive_deserialize_from_fromstr!(Vmid, "valid vmid"); dif
[pve-devel] [PATCH proxmox-ve-rs v3 09/24] common: add allowlist
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/common/mod.rs | 31 + proxmox-ve-config/src/lib.rs| 1 + 2 files changed, 32 insertions(+) create mode 100644 proxmox-ve-config/src/common/mod.rs diff --git a/proxmox-ve-config/src/common/mod.rs b/proxmox-ve-config/src/common/mod.rs new file mode 100644 index 000..ef09791 --- /dev/null +++ b/proxmox-ve-config/src/common/mod.rs @@ -0,0 +1,31 @@ +use core::hash::Hash; +use std::cmp::Eq; +use std::collections::HashSet; + +#[derive(Clone, Debug, Default)] +pub struct Allowlist(HashSet); + +impl FromIterator for Allowlist { +fn from_iter(iter: A) -> Self +where +A: IntoIterator, +{ +Allowlist(HashSet::from_iter(iter)) +} +} + +/// returns true if [`value`] is in the allowlist or if allowlist does not exist +impl Allowlist { +pub fn is_allowed(&self, value: &T) -> bool { +self.0.contains(value) +} +} + +impl Allowlist { +pub fn new(iter: I) -> Self +where +I: IntoIterator, +{ +Self::from_iter(iter) +} +} diff --git a/proxmox-ve-config/src/lib.rs b/proxmox-ve-config/src/lib.rs index 856b14f..1b6feae 100644 --- a/proxmox-ve-config/src/lib.rs +++ b/proxmox-ve-config/src/lib.rs @@ -1,3 +1,4 @@ +pub mod common; pub mod firewall; pub mod guest; pub mod host; -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 04/18] host: add struct representing bridge names
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/host/mod.rs | 1 + proxmox-ve-config/src/host/types.rs | 46 + 2 files changed, 47 insertions(+) create mode 100644 proxmox-ve-config/src/host/types.rs diff --git a/proxmox-ve-config/src/host/mod.rs b/proxmox-ve-config/src/host/mod.rs index b5614dd..b4ab6a6 100644 --- a/proxmox-ve-config/src/host/mod.rs +++ b/proxmox-ve-config/src/host/mod.rs @@ -1 +1,2 @@ +pub mod types; pub mod utils; diff --git a/proxmox-ve-config/src/host/types.rs b/proxmox-ve-config/src/host/types.rs new file mode 100644 index 000..7cbee01 --- /dev/null +++ b/proxmox-ve-config/src/host/types.rs @@ -0,0 +1,46 @@ +use std::{fmt::Display, str::FromStr}; + +use thiserror::Error; + +#[derive(Error, Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] +pub enum BridgeNameError { +#[error("name is too long")] +TooLong, +} + +#[derive(Error, Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] +pub struct BridgeName(String); + +impl BridgeName { +pub fn new(name: String) -> Result { +if name.len() > 15 { +return Err(BridgeNameError::TooLong); +} + +Ok(Self(name)) +} + +pub fn name(&self) -> &str { +&self.0 +} +} + +impl FromStr for BridgeName { +type Err = BridgeNameError; + +fn from_str(s: &str) -> Result { +Self::new(s.to_owned()) +} +} + +impl Display for BridgeName { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +f.write_str(&self.0) +} +} + +impl AsRef for BridgeName { +fn as_ref(&self) -> &str { +&self.0 +} +} -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 02/18] firewall: add bridge firewall config parser
We introduce a new type of firewall config file that can be used for defining rules on bridge-level, similar to the existing cluster/host/vm configuration files. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/bridge.rs | 64 1 file changed, 64 insertions(+) create mode 100644 proxmox-ve-config/src/firewall/bridge.rs diff --git a/proxmox-ve-config/src/firewall/bridge.rs b/proxmox-ve-config/src/firewall/bridge.rs new file mode 100644 index 000..4acb6fa --- /dev/null +++ b/proxmox-ve-config/src/firewall/bridge.rs @@ -0,0 +1,64 @@ +use std::io; + +use anyhow::Error; +use serde::Deserialize; + +use crate::firewall::parse::serde_option_bool; +use crate::firewall::types::log::LogLevel; +use crate::firewall::types::rule::{Direction, Verdict}; + +use super::common::ParserConfig; +use super::types::Rule; + +pub struct Config { +pub(crate) config: super::common::Config, +} + +/// default return value for [`Config::enabled()`] +pub const BRIDGE_ENABLED_DEFAULT: bool = false; +/// default return value for [`Config::policy_forward()`] +pub const BRIDGE_POLICY_FORWARD: Verdict = Verdict::Accept; + +impl Config { +pub fn parse(input: R) -> Result { +let parser_config = ParserConfig { +guest_iface_names: false, +ipset_scope: None, +allowed_directions: vec![Direction::Forward], +}; + +Ok(Self { +config: super::common::Config::parse(input, &parser_config)?, +}) +} + +pub fn enabled(&self) -> bool { +self.config.options.enable.unwrap_or(BRIDGE_ENABLED_DEFAULT) +} + +pub fn rules(&self) -> impl Iterator + '_ { +self.config.rules.iter() +} + +pub fn log_level_forward(&self) -> LogLevel { +self.config.options.log_level_forward.unwrap_or_default() +} + +pub fn policy_forward(&self) -> Verdict { +self.config +.options +.policy_forward +.unwrap_or(BRIDGE_POLICY_FORWARD) +} +} + +#[derive(Debug, Default, Deserialize)] +#[cfg_attr(test, derive(Eq, PartialEq))] +pub struct Options { +#[serde(default, with = "serde_option_bool")] +enable: Option, + +policy_forward: Option, + +log_level_forward: Option, +} -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs v3 22/24] add PVE::RS::Firewall::SDN module
Used for obtaining the IPSets that get autogenerated by the nftables firewall. The returned configuration has the same format as the pve-firewall uses internally, making it compatible with the existing pve-firewall code. Signed-off-by: Stefan Hanreich --- pve-rs/Cargo.toml | 1 + pve-rs/Makefile| 1 + pve-rs/src/firewall/mod.rs | 1 + pve-rs/src/firewall/sdn.rs | 130 + pve-rs/src/lib.rs | 1 + 5 files changed, 134 insertions(+) create mode 100644 pve-rs/src/firewall/mod.rs create mode 100644 pve-rs/src/firewall/sdn.rs diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml index 9470cb7..c0af2b3 100644 --- a/pve-rs/Cargo.toml +++ b/pve-rs/Cargo.toml @@ -45,3 +45,4 @@ proxmox-subscription = "0.5" proxmox-sys = "0.6" proxmox-tfa = { version = "5", features = ["api"] } proxmox-time = "2" +proxmox-ve-config = { version = "0.1.0" } diff --git a/pve-rs/Makefile b/pve-rs/Makefile index c6b4e08..d01da69 100644 --- a/pve-rs/Makefile +++ b/pve-rs/Makefile @@ -28,6 +28,7 @@ PERLMOD_GENPACKAGE := /usr/lib/perlmod/genpackage.pl \ PERLMOD_PACKAGES := \ PVE::RS::APT::Repositories \ + PVE::RS::Firewall::SDN \ PVE::RS::OpenId \ PVE::RS::ResourceScheduling::Static \ PVE::RS::TFA diff --git a/pve-rs/src/firewall/mod.rs b/pve-rs/src/firewall/mod.rs new file mode 100644 index 000..8bd18a8 --- /dev/null +++ b/pve-rs/src/firewall/mod.rs @@ -0,0 +1 @@ +pub mod sdn; diff --git a/pve-rs/src/firewall/sdn.rs b/pve-rs/src/firewall/sdn.rs new file mode 100644 index 000..5049f74 --- /dev/null +++ b/pve-rs/src/firewall/sdn.rs @@ -0,0 +1,130 @@ +#[perlmod::package(name = "PVE::RS::Firewall::SDN", lib = "pve_rs")] +mod export { +use std::collections::HashMap; +use std::{fs, io}; + +use anyhow::{bail, Context, Error}; +use serde::Serialize; + +use proxmox_ve_config::{ +common::Allowlist, +firewall::types::ipset::{IpsetAddress, IpsetEntry}, +firewall::types::Ipset, +guest::types::Vmid, +sdn::{ +config::{RunningConfig, SdnConfig}, +ipam::{Ipam, IpamJson}, +VnetName, +}, +}; + +#[derive(Clone, Debug, Default, Serialize)] +pub struct LegacyIpsetEntry { +nomatch: bool, +cidr: String, +comment: Option, +} + +impl LegacyIpsetEntry { +pub fn from_ipset_entry(entry: &IpsetEntry) -> Vec { +let mut entries = Vec::new(); + +match &entry.address { +IpsetAddress::Alias(name) => { +entries.push(Self { +nomatch: entry.nomatch, +cidr: name.to_string(), +comment: entry.comment.clone(), +}); +} +IpsetAddress::Cidr(cidr) => { +entries.push(Self { +nomatch: entry.nomatch, +cidr: cidr.to_string(), +comment: entry.comment.clone(), +}); +} +IpsetAddress::Range(range) => { +entries.extend(range.to_cidrs().into_iter().map(|cidr| Self { +nomatch: entry.nomatch, +cidr: cidr.to_string(), +comment: entry.comment.clone(), +})) +} +}; + +entries +} +} + +#[derive(Clone, Debug, Default, Serialize)] +pub struct SdnFirewallConfig { +ipset: HashMap>, +ipset_comments: HashMap, +} + +impl SdnFirewallConfig { +pub fn new() -> Self { +Default::default() +} + +pub fn extend_ipsets(&mut self, ipsets: impl IntoIterator) { +for ipset in ipsets { +let entries = ipset +.iter() +.flat_map(LegacyIpsetEntry::from_ipset_entry) +.collect(); + +self.ipset.insert(ipset.name().name().to_string(), entries); + +if let Some(comment) = &ipset.comment { +self.ipset_comments +.insert(ipset.name().name().to_string(), comment.to_string()); +} +} +} +} + +const SDN_RUNNING_CONFIG: &str = "/etc/pve/sdn/.running-config"; +const SDN_IPAM: &str = "/etc/pve/priv/ipam.db"; + +#[export] +pub fn config( +vnet_filter: Option>, +vm_filter: Option>, +) -> Result { +let mut refs = SdnFirewallConfig::new(); + +match fs::read_to_string(SDN_RUNNING_CONFIG) { +Ok(data) => { +let running_config: RunningConfig = serde_json::from_str(&data)?; +let sdn_config = SdnConfig::try_from(running_config) +.with_context(|| "Failed to parse SDN config".to_string())?; + +
[pve-devel] [PATCH proxmox-ve-rs v3 12/24] sdn: ipam: add method for generating ipsets
For every guest that has at least one entry in the IPAM we generate an ipset with the name `+sdn/guest-ipam-{vmid}`. The ipset contains all IPs from all zones for a guest with {vmid}. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 9 + proxmox-ve-config/src/sdn/ipam.rs | 40 ++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 3777dc3..9b73d3d 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -108,6 +108,15 @@ impl From for Cidr { } } +impl From for Cidr { +fn from(value: IpAddr) -> Self { +match value { +IpAddr::V4(addr) => Ipv4Cidr::from(addr).into(), +IpAddr::V6(addr) => Ipv6Cidr::from(addr).into(), +} +} +} + const IPV4_LENGTH: u8 = 32; #[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] diff --git a/proxmox-ve-config/src/sdn/ipam.rs b/proxmox-ve-config/src/sdn/ipam.rs index 95e3ba7..598b835 100644 --- a/proxmox-ve-config/src/sdn/ipam.rs +++ b/proxmox-ve-config/src/sdn/ipam.rs @@ -8,7 +8,11 @@ use std::{ use serde::Deserialize; use crate::{ -firewall::types::Cidr, +common::Allowlist, +firewall::types::{ +ipset::{IpsetEntry, IpsetScope}, +Cidr, Ipset, +}, guest::{types::Vmid, vm::MacAddress}, sdn::{SdnNameError, SubnetName, ZoneName}, }; @@ -309,6 +313,40 @@ impl Ipam { } } +impl Ipam { +/// Generates an [`Ipset`] for all guests with at least one entry in the IPAM. +/// +/// # Arguments +/// * `filter` - A [`Allowlist`] for which IPsets should get returned +/// +/// It contains all IPs in all VNets, that a guest has stored in IPAM. +/// Ipset name is of the form `guest-ipam-` +pub fn ipsets(&self, filter: Option<&Allowlist>) -> impl Iterator + '_ { +self.entries +.iter() +.flat_map(|(_, entries)| entries.iter()) +.filter_map(|entry| { +if let IpamData::Vm(data) = &entry.data() { +if filter.is_none_or(|list| list.is_allowed(&data.vmid)) { +return Some(data); +} +} + +None +}) +.fold(HashMapnew(), |mut acc, entry| { +acc.entry(entry.vmid) +.or_insert_with(|| { +Ipset::from_parts(IpsetScope::Sdn, format!("guest-ipam-{}", entry.vmid)) +}) +.push(IpsetEntry::from(entry.ip)); + +acc +}) +.into_values() +} +} + impl TryFrom for Ipam { type Error = IpamError; -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-network v3 17/18] firewall: add endpoints for vnet-level firewall
Signed-off-by: Stefan Hanreich --- src/PVE/API2/Network/SDN/Vnets.pm | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/PVE/API2/Network/SDN/Vnets.pm b/src/PVE/API2/Network/SDN/Vnets.pm index 05915f6..e48b048 100644 --- a/src/PVE/API2/Network/SDN/Vnets.pm +++ b/src/PVE/API2/Network/SDN/Vnets.pm @@ -14,6 +14,7 @@ use PVE::Network::SDN::VnetPlugin; use PVE::Network::SDN::Subnets; use PVE::API2::Network::SDN::Subnets; use PVE::API2::Network::SDN::Ips; +use PVE::API2::Firewall::Vnet; use Storable qw(dclone); use PVE::JSONSchema qw(get_standard_option); @@ -24,6 +25,11 @@ use PVE::RESTHandler; use base qw(PVE::RESTHandler); +__PACKAGE__->register_method ({ +subclass => "PVE::API2::Firewall::Vnet", +path => '{vnet}/firewall', +}); + __PACKAGE__->register_method ({ subclass => "PVE::API2::Network::SDN::Subnets", path => '{vnet}/subnets', -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall v3 05/18] nftables: derive additional traits for nftables types
Signed-off-by: Stefan Hanreich --- proxmox-nftables/src/types.rs | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxmox-nftables/src/types.rs b/proxmox-nftables/src/types.rs index 3101436..d8f3b62 100644 --- a/proxmox-nftables/src/types.rs +++ b/proxmox-nftables/src/types.rs @@ -16,10 +16,10 @@ use proxmox_ve_config::firewall::types::ipset::IpsetName; #[cfg(feature = "config-ext")] use proxmox_ve_config::guest::types::Vmid; -#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] pub struct Handle(i32); -#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum TableFamily { Ip, @@ -187,7 +187,7 @@ impl From for RateTimescale { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] pub struct TableName { family: TableFamily, name: String, @@ -210,7 +210,7 @@ impl TableName { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] pub struct TablePart { family: TableFamily, table: String, -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager v3 15/18] sdn: add firewall panel
Expose the ability to create vnet-level firewalls in the PVE UI Signed-off-by: Stefan Hanreich --- www/manager6/Makefile| 2 + www/manager6/dc/Config.js| 8 +++ www/manager6/sdn/FirewallPanel.js| 50 ++ www/manager6/sdn/FirewallVnetView.js | 77 4 files changed, 137 insertions(+) create mode 100644 www/manager6/sdn/FirewallPanel.js create mode 100644 www/manager6/sdn/FirewallVnetView.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index bcf44c39c..b2c44cd81 100644 --- a/www/manager6/Makefile +++ b/www/manager6/Makefile @@ -280,6 +280,8 @@ JSSRC= \ sdn/SubnetView.js \ sdn/ZoneContentView.js \ sdn/ZoneContentPanel.js \ + sdn/FirewallPanel.js\ + sdn/FirewallVnetView.js \ sdn/ZoneView.js \ sdn/IpamEdit.js \ sdn/OptionsPanel.js \ diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index 720edefc6..d44554954 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -221,6 +221,14 @@ Ext.define('PVE.dc.Config', { hidden: true, iconCls: 'fa fa-map-signs', itemId: 'sdnmappings', + }, + { + xtype: 'pveSDNFirewall', + groups: ['sdn'], + title: gettext('Firewall'), + hidden: true, + iconCls: 'fa fa-shield', + itemId: 'sdnfirewall', }); } diff --git a/www/manager6/sdn/FirewallPanel.js b/www/manager6/sdn/FirewallPanel.js new file mode 100644 index 0..0cdf31915 --- /dev/null +++ b/www/manager6/sdn/FirewallPanel.js @@ -0,0 +1,50 @@ + +Ext.define('PVE.sdn.FirewallPanel', { +extend: 'Ext.panel.Panel', +alias: 'widget.pveSDNFirewall', + +title: 'VNet', + +initComponent: function() { + let me = this; + + let tabPanel = Ext.create('Ext.TabPanel', { + fullscreen: true, + region: 'center', + border: false, + split: true, + disabled: true, + flex: 2, + items: [ + { + xtype: 'pveFirewallRules', + title: gettext('Rules'), + list_refs_url: '/cluster/firewall/refs', + firewall_type: 'vnet', + }, + { + xtype: 'pveFirewallOptions', + title: gettext('Options'), + fwtype: 'vnet', + }, + ], + }); + + let vnetPanel = Ext.createWidget('pveSDNFirewallVnetView', { + title: 'VNets', + region: 'west', + border: false, + split: true, + forceFit: true, + flex: 1, + tabPanel, + }); + + Ext.apply(me, { + layout: 'border', + items: [vnetPanel, tabPanel], + }); + + me.callParent(); +}, +}); diff --git a/www/manager6/sdn/FirewallVnetView.js b/www/manager6/sdn/FirewallVnetView.js new file mode 100644 index 0..861d4b5be --- /dev/null +++ b/www/manager6/sdn/FirewallVnetView.js @@ -0,0 +1,77 @@ +Ext.define('PVE.sdn.FirewallVnetView', { +extend: 'Ext.grid.GridPanel', +alias: 'widget.pveSDNFirewallVnetView', + +stateful: true, +stateId: 'grid-sdn-vnet-firewall', + +tabPanel: undefined, + +getRulesPanel: function() { + let me = this; + return me.tabPanel.items.getAt(0); +}, + +getOptionsPanel: function() { + let me = this; + return me.tabPanel.items.getAt(1); +}, + +initComponent: function() { + let me = this; + + let store = new Ext.data.Store({ + model: 'pve-sdn-vnet', + proxy: { +type: 'proxmox', + url: "/api2/json/cluster/sdn/vnets", + }, + sorters: { + property: ['zone', 'vnet'], + direction: 'ASC', + }, + }); + + let reload = () => store.load(); + + let sm = Ext.create('Ext.selection.RowModel', {}); + + Ext.apply(me, { + store: store, + reloadStore: reload, + selModel: sm, + viewConfig: { + trackOver: false, + }, + columns: [ + { + header: 'ID', + flex: 1, + dataIndex: 'vnet', + }, + { + header: gettext('Zone'), + flex: 1, + dataIndex: 'zone', + }, + { + header: gettext('Ali
[pve-devel] [PATCH proxmox-ve-rs v3 13/24] sdn: add config module
Similar to how the IPAM module works, we separate the internal representation from the concrete schema of the configuration file. We provide structs for parsing the running SDN configuration and a struct that is used internally for representing an SDN configuration, as well as a method for converting the running configuration to the internal representation. This is necessary because there are two possible sources for the SDN configuration: The running configuration as well as the SectionConfig that contains possible changes from the UI, that have not yet been applied. Simlarly to the IPAM, enforcing the invariants the way we currently do adds some runtime complexity when building the object, but we get the upside of never being able to construct an invalid struct. For the amount of entries the sdn config usually has, this should be fine. Should it turn out to be not performant enough we could always add a HashSet for looking up values and speeding up the validation. For now, I wanted to avoid the additional complexity. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/sdn/config.rs | 570 proxmox-ve-config/src/sdn/mod.rs| 1 + 2 files changed, 571 insertions(+) create mode 100644 proxmox-ve-config/src/sdn/config.rs diff --git a/proxmox-ve-config/src/sdn/config.rs b/proxmox-ve-config/src/sdn/config.rs new file mode 100644 index 000..b71084b --- /dev/null +++ b/proxmox-ve-config/src/sdn/config.rs @@ -0,0 +1,570 @@ +use std::{ +collections::{BTreeMap, HashMap}, +error::Error, +fmt::Display, +net::IpAddr, +str::FromStr, +}; + +use proxmox_schema::{property_string::PropertyString, ApiType, ObjectSchema, StringSchema}; + +use serde::Deserialize; +use serde_with::{DeserializeFromStr, SerializeDisplay}; + +use crate::{ +common::Allowlist, +firewall::types::{ +address::{IpRange, IpRangeError}, +ipset::{IpsetEntry, IpsetName, IpsetScope}, +Cidr, Ipset, +}, +sdn::{SdnNameError, SubnetName, VnetName, ZoneName}, +}; + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum SdnConfigError { +InvalidZoneType, +InvalidDhcpType, +ZoneNotFound, +VnetNotFound, +MismatchedCidrGateway, +MismatchedSubnetZone, +NameError(SdnNameError), +InvalidDhcpRange(IpRangeError), +DuplicateVnetName, +} + +impl Error for SdnConfigError { +fn source(&self) -> Option<&(dyn Error + 'static)> { +match self { +SdnConfigError::NameError(e) => Some(e), +SdnConfigError::InvalidDhcpRange(e) => Some(e), +_ => None, +} +} +} + +impl Display for SdnConfigError { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +match self { +SdnConfigError::NameError(err) => write!(f, "invalid name: {err}"), +SdnConfigError::InvalidDhcpRange(err) => write!(f, "invalid dhcp range: {err}"), +SdnConfigError::ZoneNotFound => write!(f, "zone not found"), +SdnConfigError::VnetNotFound => write!(f, "vnet not found"), +SdnConfigError::MismatchedCidrGateway => { +write!(f, "mismatched ip address family for gateway and CIDR") +} +SdnConfigError::InvalidZoneType => write!(f, "invalid zone type"), +SdnConfigError::InvalidDhcpType => write!(f, "invalid dhcp type"), +SdnConfigError::DuplicateVnetName => write!(f, "vnet name occurs in multiple zones"), +SdnConfigError::MismatchedSubnetZone => { +write!(f, "subnet zone does not match actual zone") +} +} +} +} + +#[derive( +Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] +pub enum ZoneType { +Simple, +Vlan, +Qinq, +Vxlan, +Evpn, +} + +impl FromStr for ZoneType { +type Err = SdnConfigError; + +fn from_str(s: &str) -> Result { +match s { +"simple" => Ok(ZoneType::Simple), +"vlan" => Ok(ZoneType::Vlan), +"qinq" => Ok(ZoneType::Qinq), +"vxlan" => Ok(ZoneType::Vxlan), +"evpn" => Ok(ZoneType::Evpn), +_ => Err(SdnConfigError::InvalidZoneType), +} +} +} + +impl Display for ZoneType { +fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +f.write_str(match self { +ZoneType::Simple => "simple", +ZoneType::Vlan => "vlan", +ZoneType::Qinq => "qinq", +ZoneType::Vxlan => "vxlan", +ZoneType::Evpn => "evpn", +}) +} +} + +#[derive( +Copy, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, SerializeDisplay, DeserializeFromStr, +)] +pub enum DhcpType { +Dnsmasq, +} + +impl FromStr for DhcpType { +type Err = SdnConfigError; + +fn from_str(s: &str) -> Result { +match s { +"dnsmasq" => Ok(DhcpType::Dnsmasq), +
[pve-devel] [PATCH pve-manager v3 23/24] firewall: add sdn scope to IPRefSelector
Signed-off-by: Stefan Hanreich --- www/manager6/form/IPRefSelector.js | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/www/manager6/form/IPRefSelector.js b/www/manager6/form/IPRefSelector.js index d41cde5f5..16078e428 100644 --- a/www/manager6/form/IPRefSelector.js +++ b/www/manager6/form/IPRefSelector.js @@ -67,6 +67,12 @@ Ext.define('PVE.form.IPRefSelector', { }); } + let scopes = { + 'dc': gettext("Datacenter"), + 'guest': gettext("Guest"), + 'sdn': gettext("SDN"), + }; + columns.push( { header: gettext('Name'), @@ -80,7 +86,7 @@ Ext.define('PVE.form.IPRefSelector', { hideable: false, width: 140, renderer: function(value) { - return value === 'dc' ? gettext("Datacenter") : gettext("Guest"); + return scopes[value] ?? "unknown scope"; }, }, { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 11/24] sdn: add ipam module
This module includes structs for representing the JSON schema from the PVE ipam. Those can be used to parse the current IPAM state. We also include a general Ipam struct, and provide a method for converting the PVE IPAM to the general struct. The idea behind this is that we have multiple IPAM plugins in PVE and will likely add support for importing them in the future. With the split, we can have our dedicated structs for representing the different data formats from the different IPAM plugins and then convert them into a common representation that can then be used internally, decoupling the concrete plugin from the code using the IPAM configuration. Enforcing the invariants the way we currently do adds a bit of runtime complexity when building the object, but we get the upside of never being able to construct an invalid struct. For the amount of entries the ipam usually has, this should be fine. Should it turn out to be not performant enough we could always add a HashSet for looking up values and speeding up the validation. For now, I wanted to avoid the additional complexity. Signed-off-by: Stefan Hanreich --- .../src/firewall/types/address.rs | 8 + proxmox-ve-config/src/guest/vm.rs | 4 + proxmox-ve-config/src/sdn/ipam.rs | 330 ++ proxmox-ve-config/src/sdn/mod.rs | 2 + 4 files changed, 344 insertions(+) create mode 100644 proxmox-ve-config/src/sdn/ipam.rs diff --git a/proxmox-ve-config/src/firewall/types/address.rs b/proxmox-ve-config/src/firewall/types/address.rs index 57efb13..3777dc3 100644 --- a/proxmox-ve-config/src/firewall/types/address.rs +++ b/proxmox-ve-config/src/firewall/types/address.rs @@ -61,6 +61,14 @@ impl Cidr { pub fn is_ipv6(&self) -> bool { matches!(self, Cidr::Ipv6(_)) } + +pub fn contains_address(&self, ip: &IpAddr) -> bool { +match (self, ip) { +(Cidr::Ipv4(cidr), IpAddr::V4(ip)) => cidr.contains_address(ip), +(Cidr::Ipv6(cidr), IpAddr::V6(ip)) => cidr.contains_address(ip), +_ => false, +} +} } impl fmt::Display for Cidr { diff --git a/proxmox-ve-config/src/guest/vm.rs b/proxmox-ve-config/src/guest/vm.rs index ed6c66a..3476b93 100644 --- a/proxmox-ve-config/src/guest/vm.rs +++ b/proxmox-ve-config/src/guest/vm.rs @@ -18,6 +18,10 @@ static LOCAL_PART: [u8; 8] = [0xFE, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; static EUI64_MIDDLE_PART: [u8; 2] = [0xFF, 0xFE]; impl MacAddress { +pub fn new(address: [u8; 6]) -> Self { +Self(address) +} + /// generates a link local IPv6-address according to RFC 4291 (Appendix A) pub fn eui64_link_local_address(&self) -> Ipv6Addr { let head = &self.0[..3]; diff --git a/proxmox-ve-config/src/sdn/ipam.rs b/proxmox-ve-config/src/sdn/ipam.rs new file mode 100644 index 000..95e3ba7 --- /dev/null +++ b/proxmox-ve-config/src/sdn/ipam.rs @@ -0,0 +1,330 @@ +use std::{ +collections::{BTreeMap, HashMap}, +error::Error, +fmt::Display, +net::IpAddr, +}; + +use serde::Deserialize; + +use crate::{ +firewall::types::Cidr, +guest::{types::Vmid, vm::MacAddress}, +sdn::{SdnNameError, SubnetName, ZoneName}, +}; + +/// Struct for deserializing a gateway entry in PVE IPAM. +/// +/// They are automatically generated by the PVE SDN module when creating a new subnet. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct IpamJsonDataGateway { +#[serde(rename = "gateway")] +_gateway: u8, +} + +/// Struct for deserializing a guest entry in PVE IPAM. +/// +/// They are automatically created when adding a guest to a VNet that has a Subnet with DHCP +/// configured. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct IpamJsonDataVm { +vmid: Vmid, +hostname: Option, +mac: MacAddress, +} + +/// Struct for deserializing a custom entry in PVE IPAM. +/// +/// Custom entries are created manually by the user via the Web UI / API. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct IpamJsonDataCustom { +mac: MacAddress, +} + +/// Enum representing the different kinds of entries that can be located in PVE IPAM. +/// +/// For more information about the members see the documentation of the respective structs in the +/// enum. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[serde(untagged)] +pub enum IpamJsonData { +Vm(IpamJsonDataVm), +Gateway(IpamJsonDataGateway), +Custom(IpamJsonDataCustom), +} + +/// Struct for deserializing IPs from the PVE IPAM. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Default)] +pub struct IpJson { +ips: BTreeMap, +} + +/// Struct for deserializing subnets from the PVE IPAM. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord, Default)] +pub struct SubnetJson { +subnets: BTreeMap, +} + +/// Struct fo
[pve-devel] [PATCH proxmox-ve-rs v3 03/18] config: firewall: add tests for interface and directions
Add tests for validating the directions in the guest firewall configuration. While I'm at it, I also added tests for validating interface names, since this functionality did not get tested before. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/firewall/guest.rs | 53 + 1 file changed, 53 insertions(+) diff --git a/proxmox-ve-config/src/firewall/guest.rs b/proxmox-ve-config/src/firewall/guest.rs index 1e70a67..23eaa4e 100644 --- a/proxmox-ve-config/src/firewall/guest.rs +++ b/proxmox-ve-config/src/firewall/guest.rs @@ -247,4 +247,57 @@ policy_forward: DROP } ); } + +#[test] +fn test_parse_valid_interface_prefix() { +const CONFIG: &str = r#" +[RULES] + +IN ACCEPT -p udp -dport 33 -sport 22 -log warning -i tapeth0 +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err(); +} + +#[test] +fn test_parse_invalid_interface_prefix() { +const CONFIG: &str = r#" +[RULES] + +IN ACCEPT -p udp -dport 33 -sport 22 -log warning -i eth0 +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err(); +} + +#[test] +fn test_parse_valid_directions() { +const CONFIG: &str = r#" +[RULES] + +IN ACCEPT -p udp -dport 33 -sport 22 -log warning +OUT ACCEPT -p udp -dport 33 -sport 22 -log warning +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap(); +} + +#[test] +fn test_parse_invalid_direction() { +const CONFIG: &str = r#" +[RULES] + +FORWARD ACCEPT -p udp -dport 33 -sport 22 -log warning +"#; + +let config = CONFIG.as_bytes(); +let network_config: Vec = Vec::new(); +Config::parse(&Vmid::new(100), "tap", config, network_config.as_slice()).unwrap_err(); +} } -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v3 00/18] add forward chain firewalling for hosts and vnets
## Introduction This patch series introduces a new direction for firewall rules: forward. Additionally this patch series introduces defining firewall rules on a vnet level. ## Use Cases For hosts: * hosts utilizing NAT can define firewall rules for NATed traffic * hosts utilizing EVPN zones can define rules for exit node traffic * hosts acting as gateway can firewall the traffic that passes through them For vnets: * can create firewall rules globally without having to attach/update security groups to every newly created VM This patch series is particularly useful when combined with my other current RFC 'autogenerate ipsets for sdn objects'. It enables users to quickly define rules like: on the host level: * only SNAT HTTP traffic from hosts in this vnet to a specific host * restricting traffic routed from hosts in one vnet to another vnet on the vnet level: * only allow DHCP/DNS traffic inside a bridge to the gateway Not only does this streamline creating firewall rules, it also enables users to create firewall rules that haven't been possible before and needed to rely on external firewall appliances. Since forwarded traffic goes *both* ways, you generally have to create two rules in case of bi-directional traffic. It might make sense to simplify this in the future by adding an additional option to the firewall config scheme that specifies that rules in the other direction should also get automatically generated. ## Usage For creating forward rules on the cluster/host level, you simply create a new rule with the new 'forward' direction. It uses the existing configuration files. For creating them on a vnet level, there are new firewall configuration files located under '/etc/pve/sdn/firewall/.fw'. It utilizes the same configuration format as the existing firewall configuration files. You can only define rules with direction 'forward' on a vnet-level. ## Dependencies depends on my other patch series 'autogenerate ipsets for sdn objects', further instruction can be found there. Furthermore: * proxmox-firewall depends on proxmox-ve-rs * pve-manager depends on pve-firewall * pve-network depends on pve-firewall Changes from v2 to v3: * do not allow REJECT rules in forward chains in UI and backend - thanks @Hannes * use arrow syntax for calling functions instead of &$ - thanks @Hannes * set width of new VNet firewall panel via flex, to avoid weird looking panel - thanks @Hannes * improve documentation - thanks @Hannes * show a warning in the frontend when creating forward rules - thanks @Thomas Changes from RFC to v2: * Fixed several bugs * SDN Firewall folder does not automatically created (thanks @Gabriel) * Firewall flushes the bridge table if no guest firewall is active, even though VNet-level rules exist * VNet-level firewall now matches on both input and output interface * Introduced log option for VNet firewall * Improved style of perl code (thanks @Thomas) * promox-firewall now verifies the directions of rules * added some additional tests to verify this behavior * added documentation proxmox-ve-rs: Stefan Hanreich (4): firewall: add forward direction firewall: add bridge firewall config parser config: firewall: add tests for interface and directions host: add struct representing bridge names proxmox-ve-config/src/firewall/bridge.rs | 64 +++ proxmox-ve-config/src/firewall/cluster.rs| 11 proxmox-ve-config/src/firewall/common.rs | 11 proxmox-ve-config/src/firewall/guest.rs | 66 proxmox-ve-config/src/firewall/host.rs | 12 +++- proxmox-ve-config/src/firewall/mod.rs| 1 + proxmox-ve-config/src/firewall/types/rule.rs | 10 ++- proxmox-ve-config/src/host/mod.rs| 1 + proxmox-ve-config/src/host/types.rs | 46 ++ 9 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 proxmox-ve-config/src/firewall/bridge.rs create mode 100644 proxmox-ve-config/src/host/types.rs proxmox-firewall: Stefan Hanreich (4): nftables: derive additional traits for nftables types sdn: add support for loading vnet-level firewall config sdn: create forward firewall rules use std::mem::take over drain() .../resources/proxmox-firewall.nft| 54 proxmox-firewall/src/config.rs| 88 - proxmox-firewall/src/firewall.rs | 122 +- proxmox-firewall/src/rule.rs | 7 +- proxmox-firewall/tests/integration_tests.rs | 12 ++ .../integration_tests__firewall.snap | 86 proxmox-nftables/src/expression.rs| 8 ++ proxmox-nftables/src/types.rs | 14 +- 8 files changed, 378 insertions(+), 13 deletions(-) pve-firewall: Stefan Hanreich (3): sdn: add vnet firewall configuration api: add vnet endpoints firewall: move to arrow syntax for calling functions src/PVE/API2/Firewall/Makefile | 1 + src/PVE/API2/Fire
[pve-devel] [PATCH proxmox-ve-rs v3 15/24] tests: add sdn config tests
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/tests/sdn/main.rs | 144 ++ .../tests/sdn/resources/running-config.json | 54 +++ 2 files changed, 198 insertions(+) create mode 100644 proxmox-ve-config/tests/sdn/main.rs create mode 100644 proxmox-ve-config/tests/sdn/resources/running-config.json diff --git a/proxmox-ve-config/tests/sdn/main.rs b/proxmox-ve-config/tests/sdn/main.rs new file mode 100644 index 000..2ac0cb3 --- /dev/null +++ b/proxmox-ve-config/tests/sdn/main.rs @@ -0,0 +1,144 @@ +use std::{ +net::{IpAddr, Ipv4Addr, Ipv6Addr}, +str::FromStr, +}; + +use proxmox_ve_config::{ +firewall::types::{address::IpRange, Cidr}, +sdn::{ +config::{ +RunningConfig, SdnConfig, SdnConfigError, SubnetConfig, VnetConfig, ZoneConfig, +ZoneType, +}, +SubnetName, VnetName, ZoneName, +}, +}; + +#[test] +fn parse_running_config() { +let running_config: RunningConfig = + serde_json::from_str(include_str!("resources/running-config.json")).unwrap(); + +let parsed_config = SdnConfig::try_from(running_config).unwrap(); + +let sdn_config = SdnConfig::from_zones([ZoneConfig::from_vnets( +ZoneName::from_str("zone0").unwrap(), +ZoneType::Simple, +[ +VnetConfig::from_subnets( +VnetName::from_str("vnet0").unwrap(), +[ +SubnetConfig::new( +SubnetName::from_str("zone0-fd80::-64").unwrap(), +Some(Ipv6Addr::new(0xFD80, 0, 0, 0, 0, 0, 0, 0x1).into()), +true, +[IpRange::new_v6( +[0xFD80, 0, 0, 0, 0, 0, 0, 0x1000], +[0xFD80, 0, 0, 0, 0, 0, 0, 0x], +) +.unwrap()], +) +.unwrap(), +SubnetConfig::new( +SubnetName::from_str("zone0-10.101.0.0-16").unwrap(), +Some(Ipv4Addr::new(10, 101, 1, 1).into()), +true, +[ +IpRange::new_v4([10, 101, 98, 100], [10, 101, 98, 200]).unwrap(), +IpRange::new_v4([10, 101, 99, 100], [10, 101, 99, 200]).unwrap(), +], +) +.unwrap(), +], +) +.unwrap(), +VnetConfig::from_subnets( +VnetName::from_str("vnet1").unwrap(), +[SubnetConfig::new( +SubnetName::from_str("zone0-10.102.0.0-16").unwrap(), +None, +false, +[], +) +.unwrap()], +) +.unwrap(), +], +) +.unwrap()]) +.unwrap(); + +assert_eq!(sdn_config, parsed_config); +} + +#[test] +fn sdn_config() { +let mut sdn_config = SdnConfig::new(); + +let zone0_name = ZoneName::new("zone0".to_string()).unwrap(); +let zone1_name = ZoneName::new("zone1".to_string()).unwrap(); + +let vnet0_name = VnetName::new("vnet0".to_string()).unwrap(); +let vnet1_name = VnetName::new("vnet1".to_string()).unwrap(); + +let zone0 = ZoneConfig::new(zone0_name.clone(), ZoneType::Qinq); +sdn_config.add_zone(zone0).unwrap(); + +let vnet0 = VnetConfig::new(vnet0_name.clone()); +assert_eq!( +sdn_config.add_vnet(&zone1_name, vnet0.clone()), +Err(SdnConfigError::ZoneNotFound) +); + +sdn_config.add_vnet(&zone0_name, vnet0.clone()).unwrap(); + +let subnet = SubnetConfig::new( +SubnetName::new(zone0_name.clone(), Cidr::new_v4([10, 0, 0, 0], 16).unwrap()), +IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)), +true, +[], +) +.unwrap(); + +assert_eq!( +sdn_config.add_subnet(&zone0_name, &vnet1_name, subnet.clone()), +Err(SdnConfigError::VnetNotFound), +); + +sdn_config +.add_subnet(&zone0_name, &vnet0_name, subnet) +.unwrap(); + +let zone1 = ZoneConfig::from_vnets( +zone1_name.clone(), +ZoneType::Evpn, +[VnetConfig::from_subnets( +vnet1_name.clone(), +[SubnetConfig::new( +SubnetName::new( +zone0_name.clone(), +Cidr::new_v4([192, 168, 0, 0], 24).unwrap(), +), +None, +false, +[], +) +.unwrap()], +) +.unwrap()], +) +.unwrap(); + +assert_eq!( +sdn_config.add_zones([zone1]), +Err(SdnConfigError::MismatchedSubnetZone), +); + +let zone1 = ZoneConfig::new(zone1_name.clone(), ZoneType::Evpn); +sdn_config.add_zone(zone1).unwrap(); + +assert_eq!( +sdn_config.add_vnet(&zone1_name, vnet0.clone(
[pve-devel] [PATCH proxmox-firewall v3 08/18] use std::mem::take over drain()
This is more efficient than draining and collecting the Vec. It also fixes the respective clippy lint. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/rule.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs index 3b947f0..b20a9c5 100644 --- a/proxmox-firewall/src/rule.rs +++ b/proxmox-firewall/src/rule.rs @@ -764,7 +764,7 @@ impl ToNftRules for FwMacro { fn to_nft_rules(&self, rules: &mut Vec, env: &NftRuleEnv) -> Result<(), Error> { log::trace!("applying macro: {self:?}"); -let initial_rules: Vec = rules.drain(..).collect(); +let initial_rules: Vec = std::mem::take(rules); for protocol in &self.code { let mut new_rules = initial_rules.to_vec(); -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-firewall v3 21/24] api: load sdn ipsets
Since the SDN configuration reads the IPAM config file, which resides in /etc/pve/priv we need to add the protected flag to several endpoints. Signed-off-by: Stefan Hanreich --- src/PVE/API2/Firewall/Cluster.pm | 8 ++-- src/PVE/API2/Firewall/Rules.pm | 12 +++- src/PVE/API2/Firewall/VM.pm | 3 ++- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm index 48ad90d..d5e8268 100644 --- a/src/PVE/API2/Firewall/Cluster.pm +++ b/src/PVE/API2/Firewall/Cluster.pm @@ -214,6 +214,7 @@ __PACKAGE__->register_method({ permissions => { check => ['perm', '/', [ 'Sys.Audit' ]], }, +protected => 1, parameters => { additionalProperties => 0, properties => { @@ -253,9 +254,12 @@ __PACKAGE__->register_method({ code => sub { my ($param) = @_; - my $conf = PVE::Firewall::load_clusterfw_conf(); + my $conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); + + my $cluster_refs = PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc"); + my $sdn_refs = PVE::Firewall::Helpers::collect_refs($conf->{sdn}, $param->{type}, "sdn"); - return PVE::Firewall::Helpers::collect_refs($conf, $param->{type}, "dc"); + return [@$sdn_refs, @$cluster_refs]; }}); 1; diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm index 9fcfb20..f5cb002 100644 --- a/src/PVE/API2/Firewall/Rules.pm +++ b/src/PVE/API2/Firewall/Rules.pm @@ -72,6 +72,7 @@ sub register_get_rules { path => '', method => 'GET', description => "List rules.", + protected => 1, permissions => PVE::Firewall::rules_audit_permissions($rule_env), parameters => { additionalProperties => 0, @@ -120,6 +121,7 @@ sub register_get_rule { path => '{pos}', method => 'GET', description => "Get single rule data.", + protected => 1, permissions => PVE::Firewall::rules_audit_permissions($rule_env), parameters => { additionalProperties => 0, @@ -412,7 +414,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $fw_conf = PVE::Firewall::load_clusterfw_conf(); +my $fw_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $rules = $fw_conf->{groups}->{$param->{group}}; die "no such security group '$param->{group}'\n" if !defined($rules); @@ -488,7 +490,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $fw_conf = PVE::Firewall::load_clusterfw_conf(); +my $fw_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $rules = $fw_conf->{rules}; return (undef, $fw_conf, $rules); @@ -528,7 +530,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf); my $rules = $fw_conf->{rules}; @@ -572,7 +574,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'vm', $param->{vmid}); my $rules = $fw_conf->{rules}; @@ -616,7 +618,7 @@ sub lock_config { sub load_config { my ($class, $param) = @_; -my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); +my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'ct', $param->{vmid}); my $rules = $fw_conf->{rules}; diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm index 4222103..4df725d 100644 --- a/src/PVE/API2/Firewall/VM.pm +++ b/src/PVE/API2/Firewall/VM.pm @@ -234,6 +234,7 @@ sub register_handlers { path => 'refs', method => 'GET', description => "Lists possible IPSet/Alias reference which are allowed in source/dest properties.", + protected => 1, permissions => { check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]], }, @@ -278,7 +279,7 @@ sub register_handlers { code => sub { my ($param) = @_; - my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(undef, { load_sdn_config => 1 }); my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $rule_env, $param->{vmid}); my $dc_refs = PVE::Firewall::Helpers::collect_refs($cluster_conf, $param->{type}, 'dc'); -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.prox
[pve-devel] [PATCH proxmox-firewall v3 06/18] sdn: add support for loading vnet-level firewall config
Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/config.rs | 88 - proxmox-firewall/tests/integration_tests.rs | 12 +++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs index c27aac6..ac60e15 100644 --- a/proxmox-firewall/src/config.rs +++ b/proxmox-firewall/src/config.rs @@ -1,10 +1,11 @@ use std::collections::BTreeMap; use std::default::Default; -use std::fs::File; +use std::fs::{self, DirEntry, File, ReadDir}; use std::io::{self, BufReader}; -use anyhow::{format_err, Context, Error}; +use anyhow::{bail, format_err, Context, Error}; +use proxmox_ve_config::firewall::bridge::Config as BridgeConfig; use proxmox_ve_config::firewall::cluster::Config as ClusterConfig; use proxmox_ve_config::firewall::guest::Config as GuestConfig; use proxmox_ve_config::firewall::host::Config as HostConfig; @@ -12,6 +13,7 @@ use proxmox_ve_config::firewall::types::alias::{Alias, AliasName, AliasScope}; use proxmox_ve_config::guest::types::Vmid; use proxmox_ve_config::guest::{GuestEntry, GuestMap}; +use proxmox_ve_config::host::types::BridgeName; use proxmox_nftables::command::{CommandOutput, Commands, List, ListOutput}; use proxmox_nftables::types::ListChain; @@ -33,6 +35,11 @@ pub trait FirewallConfigLoader { fn guest_firewall_config(&self, vmid: &Vmid) -> Result>, Error>; fn sdn_running_config(&self) -> Result>, Error>; fn ipam(&self) -> Result>, Error>; +fn bridge_list(&self) -> Result, Error>; +fn bridge_firewall_config( +&self, +bridge_name: &BridgeName, +) -> Result>, Error>; } #[derive(Default)] @@ -61,8 +68,31 @@ fn open_config_file(path: &str) -> Result, Error> { } } +fn open_config_folder(path: &str) -> Result, Error> { +match fs::read_dir(path) { +Ok(paths) => Ok(Some(paths)), +Err(err) if err.kind() == io::ErrorKind::NotFound => { +log::info!("SDN config folder {path} does not exist"); +Ok(None) +} +Err(err) => { +let context = format!("unable to open configuration folder at {BRIDGE_CONFIG_PATH}"); +Err(anyhow::Error::new(err).context(context)) +} +} +} + +fn fw_name(dir_entry: DirEntry) -> Option { +dir_entry +.file_name() +.to_str()? +.strip_suffix(".fw") +.map(str::to_string) +} + const CLUSTER_CONFIG_PATH: &str = "/etc/pve/firewall/cluster.fw"; const HOST_CONFIG_PATH: &str = "/etc/pve/local/host.fw"; +const BRIDGE_CONFIG_PATH: &str = "/etc/pve/sdn/firewall"; const SDN_RUNNING_CONFIG_PATH: &str = "/etc/pve/sdn/.running-config"; const SDN_IPAM_PATH: &str = "/etc/pve/priv/ipam.db"; @@ -154,6 +184,38 @@ impl FirewallConfigLoader for PveFirewallConfigLoader { Ok(None) } + +fn bridge_list(&self) -> Result, Error> { +let mut bridges = Vec::new(); + +if let Some(files) = open_config_folder(BRIDGE_CONFIG_PATH)? { +for file in files { +let bridge_name = fw_name(file?).map(BridgeName::new).transpose()?; + +if let Some(bridge_name) = bridge_name { +bridges.push(bridge_name); +} +} +} + +Ok(bridges) +} + +fn bridge_firewall_config( +&self, +bridge_name: &BridgeName, +) -> Result>, Error> { +log::info!("loading firewall config for bridge {bridge_name}"); + +let fd = open_config_file(&format!("/etc/pve/sdn/firewall/{bridge_name}.fw"))?; + +if let Some(file) = fd { +let buf_reader = Box::new(BufReader::new(file)) as Box; +return Ok(Some(buf_reader)); +} + +Ok(None) +} } pub trait NftConfigLoader { @@ -184,6 +246,7 @@ pub struct FirewallConfig { cluster_config: ClusterConfig, host_config: HostConfig, guest_config: BTreeMap, +bridge_config: BTreeMap, nft_config: BTreeMap, sdn_config: Option, ipam_config: Option, @@ -284,6 +347,22 @@ impl FirewallConfig { Ok(chains) } +pub fn parse_bridges( +firewall_loader: &dyn FirewallConfigLoader, +) -> Result, Error> { +let mut bridge_config = BTreeMap::new(); + +for bridge_name in firewall_loader.bridge_list()? { +if let Some(config) = firewall_loader.bridge_firewall_config(&bridge_name)? { +bridge_config.insert(bridge_name, BridgeConfig::parse(config)?); +} else { +bail!("Could not read config for {bridge_name}") +} +} + +Ok(bridge_config) +} + pub fn new( firewall_loader: &dyn FirewallConfigLoader, nft_loader: &dyn NftConfigLoader, @@ -292,6 +371,7 @@ impl FirewallConfig { cluster_config: Self::parse_cluster(firewall_loader)?, host_config: Self::parse_host(firewall_loader)?, guest_config
[pve-devel] [PATCH pve-manager v3 16/18] firewall: rules: show warning when creating forward rules
Since forward rules only take effect when the nftables firewall is enabled, show a warning to users that informs them of this. Signed-off-by: Stefan Hanreich --- www/manager6/grid/FirewallRules.js | 15 +++ 1 file changed, 15 insertions(+) diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js index e2809f02b..1f9832f0a 100644 --- a/www/manager6/grid/FirewallRules.js +++ b/www/manager6/grid/FirewallRules.js @@ -174,6 +174,7 @@ Ext.define('PVE.FirewallRulePanel', { firewall_type: undefined, action_selector: undefined, +forward_warning: undefined, onGetValues: function(values) { var me = this; @@ -199,11 +200,17 @@ Ext.define('PVE.FirewallRulePanel', { me.action_selector.setComboItems(allowed_actions.map((action) => [action, action])); }, +setForwardWarning: function(type) { + let me = this; + me.forward_warning.setHidden(type !== 'forward'); +}, + onSetValues: function(values) { let me = this; if (values.type) { me.setValidActions(values.type); + me.setForwardWarning(values.type); } return values; @@ -227,6 +234,12 @@ Ext.define('PVE.FirewallRulePanel', { allowBlank: false, }); + me.forward_warning = Ext.create('Proxmox.form.field.DisplayEdit', { + userCls: 'pmx-hint', + value: gettext('Forward rules only take effect when the nftables firewall is activated in the host options'), + hidden: true, + }); + me.column1 = [ { // hack: we use this field to mark the form 'dirty' when the @@ -246,6 +259,7 @@ Ext.define('PVE.FirewallRulePanel', { listeners: { change: function(f, value) { me.setValidActions(value); + me.setForwardWarning(value); }, }, }, @@ -420,6 +434,7 @@ Ext.define('PVE.FirewallRulePanel', { value: '', fieldLabel: gettext('Comment'), }, + me.forward_warning, ]; me.callParent(); -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-docs v3 24/24] sdn: add documentation for firewall integration
Signed-off-by: Stefan Hanreich --- pvesdn.adoc | 92 + 1 file changed, 92 insertions(+) diff --git a/pvesdn.adoc b/pvesdn.adoc index 39de80f..c187365 100644 --- a/pvesdn.adoc +++ b/pvesdn.adoc @@ -702,6 +702,98 @@ For more information please consult the documentation of xref:pvesdn_ipam_plugin_pveipam[the PVE IPAM plugin]. Changing DHCP leases is currently not supported for the other IPAM plugins. +Firewall Integration + + +SDN integrates with the Proxmox VE firewall by automatically generating IPSets +which can then be referenced in the source / destination fields of firewall +rules. This happens automatically for VNets and IPAM entries. + +VNets and Subnets +~ + +The firewall automatically generates the following IPSets in the SDN scope for +every VNet: + +`vnet-all`:: + Contains the CIDRs of all subnets in a VNet +`vnet-gateway`:: + Contains the IPs of the gateways of all subnets in a VNet +`vnet-no-gateway`:: + Contains the CIDRs of all subnets in a VNet, but excludes the gateways +`vnet-dhcp`:: + Contains all DHCP ranges configured in the subnets in a VNet + +When making changes to your configuration, the IPSets update automatically, so +you do not have to update your firewall rules when changing the configuration of +your Subnets. + +Simple Zone Example +^^^ + +Assuming the configuration below for a VNet and its contained subnets: + + +# /etc/pve/sdn/vnets.cfg + +vnet: vnet0 +zone simple + +# /etc/pve/sdn/subnets.cfg + +subnet: simple-192.0.2.0-24 +vnet vnet0 +dhcp-range start-address=192.0.2.100,end-address=192.0.2.199 +gateway 192.0.2.1 + +subnet: simple-2001:db8::-64 +vnet vnet0 +dhcp-range start-address=2001:db8::1000,end-address=2001:db8::1999 +gateway 2001:db8::1 + + +In this example we configured an IPv4 subnet in the VNet `vnet0`, with +'192.0.2.0/24' as its IP Range, '192.0.2.1' as the gateway and the DHCP range is +'192.0.2.100' - '192.0.2.199'. + +Additionally we configured an IPv6 subnet with '2001:db8::/64' as the IP range, +'2001:db8::1' as the gateway and a DHCP range of '2001:db8::1000' - +'2001:db8::1999'. + +The respective auto-generated IPsets for vnet0 would then contain the following +elements: + +`vnet0-all`:: +* '192.0.2.0/24' +* '2001:db8::/64' +`vnet0-gateway`:: +* '192.0.2.1' +* '2001:db8::1' +`vnet0-no-gateway`:: +* '192.0.2.0/24' +* '2001:db8::/64' +* '!192.0.2.1' +* '!2001:db8::1' +`vnet0-dhcp`:: +* '192.0.2.100 - 192.0.2.199' +* '2001:db8::1000 - 2001:db8::1999' + +IPAM + + +If you are using the built-in PVE IPAM, then the firewall automatically +generates an IPset for every guest that has entries in the IPAM. The respective +IPset for a guest with ID 100 would be `guest-ipam-100`. It contains all IP +addresses from all IPAM entries. So if guest 100 is member of multiple VNets, +then the IPset would contain the IPs from *all* VNets. + +When entries get added / updated / deleted, then the respective IPSets will be +updated accordingly. + +WARNING: When removing all entries for a guest and there are firewall rules +still referencing the auto-generated IPSet then the firewall will fail to update +the ruleset, since it references a non-existing IPSet. + [[pvesdn_setup_examples]] Examples -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs v3 16/24] tests: add ipam tests
Signed-off-by: Stefan Hanreich --- proxmox-ve-config/tests/sdn/main.rs | 45 +++ proxmox-ve-config/tests/sdn/resources/ipam.db | 26 +++ 2 files changed, 71 insertions(+) create mode 100644 proxmox-ve-config/tests/sdn/resources/ipam.db diff --git a/proxmox-ve-config/tests/sdn/main.rs b/proxmox-ve-config/tests/sdn/main.rs index 2ac0cb3..1815bec 100644 --- a/proxmox-ve-config/tests/sdn/main.rs +++ b/proxmox-ve-config/tests/sdn/main.rs @@ -5,11 +5,13 @@ use std::{ use proxmox_ve_config::{ firewall::types::{address::IpRange, Cidr}, +guest::vm::MacAddress, sdn::{ config::{ RunningConfig, SdnConfig, SdnConfigError, SubnetConfig, VnetConfig, ZoneConfig, ZoneType, }, +ipam::{Ipam, IpamDataVm, IpamEntry, IpamJson}, SubnetName, VnetName, ZoneName, }, }; @@ -142,3 +144,46 @@ fn sdn_config() { Err(SdnConfigError::DuplicateVnetName), ) } + +#[test] +fn parse_ipam() { +let ipam_json: IpamJson = serde_json::from_str(include_str!("resources/ipam.db")).unwrap(); +let ipam = Ipam::try_from(ipam_json).unwrap(); + +let zone_name = ZoneName::new("zone0".to_string()).unwrap(); + +assert_eq!( +Ipam::from_entries([ +IpamEntry::new( +SubnetName::new( +zone_name.clone(), +Cidr::new_v6([0xFD80, 0, 0, 0, 0, 0, 0, 0], 64).unwrap() +), +IpamDataVm::new( +Ipv6Addr::new(0xFD80, 0, 0, 0, 0, 0, 0, 0x1000), +1000, +MacAddress::new([0xBC, 0x24, 0x11, 0, 0, 0x01]), +"test0".to_string() +) +.into() +) +.unwrap(), +IpamEntry::new( +SubnetName::new( +zone_name.clone(), +Cidr::new_v4([10, 101, 0, 0], 16).unwrap() +), +IpamDataVm::new( +Ipv4Addr::new(10, 101, 99, 101), +1000, +MacAddress::new([0xBC, 0x24, 0x11, 0, 0, 0x01]), +"test0".to_string() +) +.into() +) +.unwrap(), +]) +.unwrap(), +ipam +) +} diff --git a/proxmox-ve-config/tests/sdn/resources/ipam.db b/proxmox-ve-config/tests/sdn/resources/ipam.db new file mode 100644 index 000..a3e6c87 --- /dev/null +++ b/proxmox-ve-config/tests/sdn/resources/ipam.db @@ -0,0 +1,26 @@ +{ + "zones": { +"zone0": { + "subnets": { +"fd80::/64": { + "ips": { +"fd80::1000": { + "vmid": "1000", + "mac": "BC:24:11:00:00:01", + "hostname": "test0" +} + } +}, +"10.101.0.0/16": { + "ips": { +"10.101.99.101": { + "mac": "BC:24:11:00:00:01", + "vmid": "1000", + "hostname": "test0" +} + } +} + } +} + } +} -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager v3 13/18] firewall: add vnet to firewall options component
Add the configuration options for vnet-level firewalls to the options component. Additionally add the new policy_forward configuration option to the datacenter-level firewall as well. Signed-off-by: Stefan Hanreich --- www/manager6/grid/FirewallOptions.js | 38 +++- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/www/manager6/grid/FirewallOptions.js b/www/manager6/grid/FirewallOptions.js index 6aacb47be..fa482e0e4 100644 --- a/www/manager6/grid/FirewallOptions.js +++ b/www/manager6/grid/FirewallOptions.js @@ -2,7 +2,7 @@ Ext.define('PVE.FirewallOptions', { extend: 'Proxmox.grid.ObjectGrid', alias: ['widget.pveFirewallOptions'], -fwtype: undefined, // 'dc', 'node' or 'vm' +fwtype: undefined, // 'dc', 'node', 'vm' or 'vnet' base_url: undefined, @@ -13,14 +13,14 @@ Ext.define('PVE.FirewallOptions', { throw "missing base_url configuration"; } - if (me.fwtype === 'dc' || me.fwtype === 'node' || me.fwtype === 'vm') { - if (me.fwtype === 'node') { - me.cwidth1 = 250; - } - } else { + if (!['dc', 'node', 'vm', 'vnet'].includes(me.fwtype)) { throw "unknown firewall option type"; } + if (me.fwtype === 'node') { + me.cwidth1 = 250; + } + let caps = Ext.state.Manager.get('GuiCap'); let canEdit = caps.vms['VM.Config.Network'] || caps.dc['Sys.Modify'] || caps.nodes['Sys.Modify']; @@ -81,6 +81,7 @@ Ext.define('PVE.FirewallOptions', { 'nf_conntrack_tcp_timeout_established', 7875, 250); add_log_row('log_level_in'); add_log_row('log_level_out'); + add_log_row('log_level_forward'); add_log_row('tcp_flags_log_level', 120); add_log_row('smurf_log_level'); add_boolean_row('nftables', gettext('nftables (tech preview)'), 0); @@ -114,6 +115,9 @@ Ext.define('PVE.FirewallOptions', { defaultValue: 'enable=1', }, }; + } else if (me.fwtype === 'vnet') { + add_boolean_row('enable', gettext('Firewall'), 0); + add_log_row('log_level_forward'); } if (me.fwtype === 'dc' || me.fwtype === 'vm') { @@ -150,6 +154,28 @@ Ext.define('PVE.FirewallOptions', { }; } + if (me.fwtype === 'vnet' || me.fwtype === 'dc') { + me.rows.policy_forward = { + header: gettext('Forward Policy'), + required: true, + defaultValue: 'ACCEPT', + editor: { + xtype: 'proxmoxWindowEdit', + subject: gettext('Forward Policy'), + items: { + xtype: 'pveFirewallPolicySelector', + name: 'policy_forward', + value: 'ACCEPT', + fieldLabel: gettext('Forward Policy'), + comboItems: [ + ['ACCEPT', 'ACCEPT'], + ['DROP', 'DROP'], + ], + }, + }, + }; + } + var edit_btn = new Ext.Button({ text: gettext('Edit'), disabled: true, -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-firewall v3 09/18] sdn: add vnet firewall configuration
Signed-off-by: Stefan Hanreich --- src/PVE/Firewall.pm | 127 ++-- src/PVE/Firewall/Helpers.pm | 12 2 files changed, 132 insertions(+), 7 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 6e02873..fc71d86 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -29,6 +29,7 @@ use PVE::RS::Firewall::SDN; my $pvefw_conf_dir = "/etc/pve/firewall"; my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw"; +my $vnetfw_conf_dir = "/etc/pve/sdn/firewall"; # dynamically include PVE::QemuServer and PVE::LXC # to avoid dependency problems @@ -1290,6 +1291,12 @@ our $cluster_option_properties = { optional => 1, enum => ['ACCEPT', 'REJECT', 'DROP'], }, +policy_forward => { + description => "Forward policy.", + type => 'string', + optional => 1, + enum => ['ACCEPT', 'DROP'], +}, log_ratelimit => { description => "Log ratelimiting settings", type => 'string', format => { @@ -1329,6 +1336,8 @@ our $host_option_properties = { description => "Log level for incoming traffic." }), log_level_out => get_standard_option('pve-fw-loglevel', { description => "Log level for outgoing traffic." }), +log_level_forward => get_standard_option('pve-fw-loglevel', { + description => "Log level for forwarded traffic." }), tcp_flags_log_level => get_standard_option('pve-fw-loglevel', { description => "Log level for illegal tcp flags filter." }), smurf_log_level => get_standard_option('pve-fw-loglevel', { @@ -1476,6 +1485,23 @@ our $vm_option_properties = { }; +our $vnet_option_properties = { +enable => { + description => "Enable/disable firewall rules.", + type => 'boolean', + default => 0, + optional => 1, +}, +policy_forward => { + description => "Forward policy.", + type => 'string', + optional => 1, + enum => ['ACCEPT', 'DROP'], +}, +log_level_forward => get_standard_option('pve-fw-loglevel', { + description => "Log level for forwarded traffic." }), +}; + my $addr_list_descr = "This can refer to a single IP address, an IP set ('+ipsetname') or an IP alias definition. You can also specify an address range like '20.34.101.207-201.3.9.99', or a list of IP addresses and networks (entries are separated by comma). Please do not mix IPv4 and IPv6 addresses inside such lists."; @@ -1493,7 +1519,7 @@ my $rule_properties = { description => "Rule type.", type => 'string', optional => 1, - enum => ['in', 'out', 'group'], + enum => ['in', 'out', 'forward', 'group'], }, action => { description => "Rule action ('ACCEPT', 'DROP', 'REJECT') or security group name.", @@ -1651,10 +1677,20 @@ my $rule_env_iface_lookup = { 'ct' => 1, 'vm' => 1, 'group' => 0, +'vnet' => 0, 'cluster' => 1, 'host' => 1, }; +my $rule_env_direction_lookup = { +'ct' => ['in', 'out', 'group'], +'vm' => ['in', 'out', 'group'], +'group' => ['in', 'out', 'forward'], +'cluster' => ['in', 'out', 'forward', 'group'], +'host' => ['in', 'out', 'forward', 'group'], +'vnet' => ['forward', 'group'], +}; + sub verify_rule { my ($rule, $cluster_conf, $fw_conf, $rule_env, $noerr) = @_; @@ -1725,8 +1761,17 @@ sub verify_rule { &$add_error('action', "missing property") if !$action; if ($type) { - if ($type eq 'in' || $type eq 'out') { - &$add_error('action', "unknown action '$action'") + my $valid_types = $rule_env_direction_lookup->{$rule_env} + or die "unknown rule_env '$rule_env'\n"; + + $add_error->('type', "invalid rule type '$type' for rule_env '$rule_env'") + if !(grep { $_ eq $type } @$valid_types); + + if ($type eq 'in' || $type eq 'out' || $type eq 'forward') { + $add_error->('action', 'cannot define REJECT rules on forward chain') + if $type eq 'forward' && $action eq 'REJECT'; + + $add_error->('action', "unknown action '$action'") if $action && ($action !~ m/^(ACCEPT|DROP|REJECT)$/); } elsif ($type eq 'group') { &$add_error('type', "security groups not allowed") @@ -2829,7 +2874,7 @@ sub parse_fw_rule { $rule->{type} = lc($1); $rule->{action} = $2; -if ($rule->{type} eq 'in' || $rule->{type} eq 'out') { +if ($rule->{type} eq 'in' || $rule->{type} eq 'out' || $rule->{type} eq 'forward') { if ($rule->{action} =~ m/^(\S+)\((ACCEPT|DROP|REJECT)\)$/) { $rule->{macro} = $1; $rule->{action} = $2; @@ -2943,7 +2988,7 @@ sub parse_hostfw_option { if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i) { $opt = lc($1); $value = int($2); -} elsif ($line =~ m/^(log_level_in|log_level_o
[pve-devel] [PATCH proxmox-firewall v3 07/18] sdn: create forward firewall rules
Signed-off-by: Stefan Hanreich --- .../resources/proxmox-firewall.nft| 54 proxmox-firewall/src/firewall.rs | 122 +- proxmox-firewall/src/rule.rs | 5 +- .../integration_tests__firewall.snap | 86 proxmox-nftables/src/expression.rs| 8 ++ proxmox-nftables/src/types.rs | 6 + 6 files changed, 275 insertions(+), 6 deletions(-) diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft index f42255c..af9454d 100644 --- a/proxmox-firewall/resources/proxmox-firewall.nft +++ b/proxmox-firewall/resources/proxmox-firewall.nft @@ -20,8 +20,12 @@ add chain inet proxmox-firewall allow-icmp add chain inet proxmox-firewall log-drop-smurfs add chain inet proxmox-firewall default-in add chain inet proxmox-firewall default-out +add chain inet proxmox-firewall before-bridge +add chain inet proxmox-firewall host-bridge-input {type filter hook input priority filter - 1; policy accept;} +add chain inet proxmox-firewall host-bridge-output {type filter hook output priority filter + 1; policy accept;} add chain inet proxmox-firewall input {type filter hook input priority filter; policy drop;} add chain inet proxmox-firewall output {type filter hook output priority filter; policy accept;} +add chain inet proxmox-firewall forward {type filter hook forward priority filter; policy accept;} add chain bridge proxmox-firewall-guests allow-dhcp-in add chain bridge proxmox-firewall-guests allow-dhcp-out @@ -39,6 +43,8 @@ add chain bridge proxmox-firewall-guests pre-vm-out add chain bridge proxmox-firewall-guests vm-out {type filter hook prerouting priority 0; policy accept;} add chain bridge proxmox-firewall-guests pre-vm-in add chain bridge proxmox-firewall-guests vm-in {type filter hook postrouting priority 0; policy accept;} +add chain bridge proxmox-firewall-guests before-bridge +add chain bridge proxmox-firewall-guests forward {type filter hook forward priority 0; policy accept;} flush chain inet proxmox-firewall do-reject flush chain inet proxmox-firewall accept-management @@ -55,8 +61,12 @@ flush chain inet proxmox-firewall allow-icmp flush chain inet proxmox-firewall log-drop-smurfs flush chain inet proxmox-firewall default-in flush chain inet proxmox-firewall default-out +flush chain inet proxmox-firewall before-bridge +flush chain inet proxmox-firewall host-bridge-input +flush chain inet proxmox-firewall host-bridge-output flush chain inet proxmox-firewall input flush chain inet proxmox-firewall output +flush chain inet proxmox-firewall forward flush chain bridge proxmox-firewall-guests allow-dhcp-in flush chain bridge proxmox-firewall-guests allow-dhcp-out @@ -74,6 +84,8 @@ flush chain bridge proxmox-firewall-guests pre-vm-out flush chain bridge proxmox-firewall-guests vm-out flush chain bridge proxmox-firewall-guests pre-vm-in flush chain bridge proxmox-firewall-guests vm-in +flush chain bridge proxmox-firewall-guests before-bridge +flush chain bridge proxmox-firewall-guests forward table inet proxmox-firewall { chain do-reject { @@ -223,6 +235,25 @@ table inet proxmox-firewall { chain option-in {} chain option-out {} +map bridge-map { +type ifname : verdict +} + +chain before-bridge { +meta protocol arp accept +meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop } +} + +chain host-bridge-input { +type filter hook input priority filter - 1; policy accept; +meta iifname vmap @bridge-map +} + +chain host-bridge-output { +type filter hook output priority filter + 1; policy accept; +meta oifname vmap @bridge-map +} + chain input { type filter hook input priority filter; policy accept; jump default-in @@ -240,12 +271,21 @@ table inet proxmox-firewall { jump cluster-out } +chain forward { +type filter hook forward priority filter; policy accept; +jump host-forward +jump cluster-forward +} + chain cluster-in {} chain cluster-out {} chain host-in {} chain host-out {} +chain cluster-forward {} +chain host-forward {} + chain ct-in {} } @@ -335,4 +375,18 @@ table bridge proxmox-firewall-guests { jump allow-icmp oifname vmap @vm-map-in } + +map bridge-map { +type ifname . ifname : verdict +} + +chain before-bridge { +meta protocol arp accept +meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop } +} + +chain forward { +type filter hook forward priority 0; policy accept; +meta ibrname . meta obrname vmap @bridge-map +} } diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs index 347f3af..bb54023 100644 --- a/proxmox-
[pve-devel] [PATCH proxmox-firewall v3 18/24] config: tests: add support for loading sdn and ipam config
Also add example SDN configuration files that get automatically loaded, which can be used for future tests. Signed-off-by: Stefan Hanreich --- proxmox-firewall/src/config.rs| 69 +++ .../tests/input/.running-config.json | 45 proxmox-firewall/tests/input/ipam.db | 32 + proxmox-firewall/tests/integration_tests.rs | 10 +++ proxmox-nftables/src/types.rs | 2 +- 5 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 proxmox-firewall/tests/input/.running-config.json create mode 100644 proxmox-firewall/tests/input/ipam.db diff --git a/proxmox-firewall/src/config.rs b/proxmox-firewall/src/config.rs index 5bd2512..c27aac6 100644 --- a/proxmox-firewall/src/config.rs +++ b/proxmox-firewall/src/config.rs @@ -16,6 +16,10 @@ use proxmox_ve_config::guest::{GuestEntry, GuestMap}; use proxmox_nftables::command::{CommandOutput, Commands, List, ListOutput}; use proxmox_nftables::types::ListChain; use proxmox_nftables::NftClient; +use proxmox_ve_config::sdn::{ +config::{RunningConfig, SdnConfig}, +ipam::{Ipam, IpamJson}, +}; pub trait FirewallConfigLoader { fn cluster(&self) -> Result>, Error>; @@ -27,6 +31,8 @@ pub trait FirewallConfigLoader { guest: &GuestEntry, ) -> Result>, Error>; fn guest_firewall_config(&self, vmid: &Vmid) -> Result>, Error>; +fn sdn_running_config(&self) -> Result>, Error>; +fn ipam(&self) -> Result>, Error>; } #[derive(Default)] @@ -58,6 +64,9 @@ fn open_config_file(path: &str) -> Result, Error> { const CLUSTER_CONFIG_PATH: &str = "/etc/pve/firewall/cluster.fw"; const HOST_CONFIG_PATH: &str = "/etc/pve/local/host.fw"; +const SDN_RUNNING_CONFIG_PATH: &str = "/etc/pve/sdn/.running-config"; +const SDN_IPAM_PATH: &str = "/etc/pve/priv/ipam.db"; + impl FirewallConfigLoader for PveFirewallConfigLoader { fn cluster(&self) -> Result>, Error> { log::info!("loading cluster config"); @@ -119,6 +128,32 @@ impl FirewallConfigLoader for PveFirewallConfigLoader { Ok(None) } + +fn sdn_running_config(&self) -> Result>, Error> { +log::info!("loading SDN running-config"); + +let fd = open_config_file(SDN_RUNNING_CONFIG_PATH)?; + +if let Some(file) = fd { +let buf_reader = Box::new(BufReader::new(file)) as Box; +return Ok(Some(buf_reader)); +} + +Ok(None) +} + +fn ipam(&self) -> Result>, Error> { +log::info!("loading IPAM config"); + +let fd = open_config_file(SDN_IPAM_PATH)?; + +if let Some(file) = fd { +let buf_reader = Box::new(BufReader::new(file)) as Box; +return Ok(Some(buf_reader)); +} + +Ok(None) +} } pub trait NftConfigLoader { @@ -150,6 +185,8 @@ pub struct FirewallConfig { host_config: HostConfig, guest_config: BTreeMap, nft_config: BTreeMap, +sdn_config: Option, +ipam_config: Option, } impl FirewallConfig { @@ -207,6 +244,28 @@ impl FirewallConfig { Ok(guests) } +pub fn parse_sdn( +firewall_loader: &dyn FirewallConfigLoader, +) -> Result, Error> { +Ok(match firewall_loader.sdn_running_config()? { +Some(data) => { +let running_config: RunningConfig = serde_json::from_reader(data)?; +Some(SdnConfig::try_from(running_config)?) +} +_ => None, +}) +} + +pub fn parse_ipam(firewall_loader: &dyn FirewallConfigLoader) -> Result, Error> { +Ok(match firewall_loader.ipam()? { +Some(data) => { +let raw_ipam: IpamJson = serde_json::from_reader(data)?; +Some(Ipam::try_from(raw_ipam)?) +} +_ => None, +}) +} + pub fn parse_nft( nft_loader: &dyn NftConfigLoader, ) -> Result, Error> { @@ -233,6 +292,8 @@ impl FirewallConfig { cluster_config: Self::parse_cluster(firewall_loader)?, host_config: Self::parse_host(firewall_loader)?, guest_config: Self::parse_guests(firewall_loader)?, +sdn_config: Self::parse_sdn(firewall_loader)?, +ipam_config: Self::parse_ipam(firewall_loader)?, nft_config: Self::parse_nft(nft_loader)?, }) } @@ -253,6 +314,14 @@ impl FirewallConfig { &self.nft_config } +pub fn sdn(&self) -> Option<&SdnConfig> { +self.sdn_config.as_ref() +} + +pub fn ipam(&self) -> Option<&Ipam> { +self.ipam_config.as_ref() +} + pub fn is_enabled(&self) -> bool { self.cluster().is_enabled() && self.host().nftables() } diff --git a/proxmox-firewall/tests/input/.running-config.json b/proxmox-firewall/tests/input/.running-config.json new file mode 100644 index 000..a4511f0 --- /dev/null +++ b/proxmox-firewall/tests/input/.running-config.json @@ -0,0 +1,45 @@ +{ + "subnets": { +
Re: [pve-devel] [PATCH docs/firewall/manager/network/proxmox{-ve-rs, -firewall} v2 00/17] add forward chain firewalling for hosts and vnets
superseded by: https://lore.proxmox.com/pve-devel/20241112122615.88854-1-s.hanre...@proxmox.com/T/#m646bd4b0be7652b2cc8afc411e6c96366ddb9a14 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs/firewall/manager/proxmox{-ve-rs, -firewall, -perl-rs} v2 00/25] autogenerate ipsets for sdn objects
superseded by: https://lore.proxmox.com/pve-devel/20241112122602.88598-1-s.hanre...@proxmox.com/T/#m3c6f184e088b362a92705fb4a5a6ddab11640e43 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v4] fix #5810: ui: show confirmation/warning dialog for sdn apply
Signed-off-by: Timothy Nicholson --- changes since v3 [0]: - implement conditional warning message that displays which node(s) have pending changes. I'd be thankful for any feedback, as this can probably be refined in some way. [0]: https://lore.proxmox.com/pve-devel/20241112120255.127300-1-t.nichol...@proxmox.com/ www/manager6/sdn/StatusView.js | 48 +++--- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/www/manager6/sdn/StatusView.js b/www/manager6/sdn/StatusView.js index 970aa919..009f5fee 100644 --- a/www/manager6/sdn/StatusView.js +++ b/www/manager6/sdn/StatusView.js @@ -40,16 +40,46 @@ Ext.define('PVE.sdn.StatusView', { tbar: [ { text: gettext('Apply'), - handler: function() { - Proxmox.Utils.API2Request({ - url: '/cluster/sdn/', - method: 'PUT', - waitMsgTarget: me, - failure: function(response, opts) { - Ext.Msg.alert(gettext('Error'), response.htmlStatus); - }, - }); + handler: async function() { + const applyChanges = () => { + Proxmox.Utils.API2Request({ + url: '/cluster/sdn', + method: 'PUT', + waitMsgTarget: me, + failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus), + }); + }; + + let nodesPending = []; + let { result: nodesResult } = await Proxmox.Async.api2({ url: '/nodes' }); + + await Promise.all(nodesResult.data.map(async(node) => { + let { result: networkResult } = await + Proxmox.Async.api2({ url: `/nodes/${node.node}/network` }); + if (networkResult.changes !== undefined) { + nodesPending.push(node.node); + } + })); + + if (nodesPending.length > 0) { + Ext.Msg.show({ + title: gettext('Confirm'), + msg: `${gettext('There are pending network changes on node(s)')} [${nodesPending}]. ` + +`${gettext('These will also be applied. Proceed?')}`, + icon: Ext.Msg.QUESTION, + buttons: Ext.Msg.YESNO, + callback: function(btn) { + if (btn === 'yes') { + applyChanges(); + } + }, + }); + } else { + applyChanges(); + } }, + + }, ], viewConfig: { -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 pve-manager 1/5] fix #5366: ui: ceph: services: parse and display build commit
one very small nit inline at the end On 2024-07-24 17:05, Max Carrara wrote: The build commit is displayed and taken into account when comparing monitor and manager versions in the client. Specifically, the shortened build commit is now displayed in parentheses next to the version for both monitors and managers like so: 18.2.2 (build: abcd1234) Should the build commit of the running service differ from the one that's installed on the host, the newer build commit will also be shown in parentheses: 18.2.2 (build: abcd1234 -> 5678fedc) The icon displayed for running a service with an outdated build is the same as for running an outdated version. The conditional display of icons related to cluster health remains the same otherwise. The Ceph summary view also displays the hash and will show a warning if a service is running with a build commit that doesn't match the one that's advertised by the host. This requires the introduction of two helper functions: 1. `PVE.Utils.parseCephBuildCommit`, which can be used to get the full hash "eccf199d..." in parentheses from a string like the following: ceph version 17.2.7 (eccf199d63457659c09677399928203b7903c888) quincy (stable) 2. `PVE.Utils.renderCephBuildCommit`, which is used to determine how to render a service's current build commit and which accompanying icon to choose. Signed-off-by: Max Carrara Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5366 --- Changes v2 --> v3: * add new `renderCephBuildCommit` helper function (thanks Thomas!) * add docstrings for helpers * use less ambiguous variable names (thanks Thomas!) * put 'build: ' in front of build commit when rendering (thanks Thomas!) * handle no rendered build commit being available in MGR / MON lists, returning the icon + version without the commit instead * make the modified logic in Services.js more readable * reword message about differing builds in the overview * reword commit title & message * add 'Fixes' trailer * remove outdated R-b and T-b trailers Changes v1 --> v2: * use camelCase instead of snake_case (thanks Lukas!) * use more descriptive variable names (thanks Lukas!) * use `let` instead of `const` for variables where applicable (thanks Lukas!) www/manager6/Utils.js| 107 +++ www/manager6/ceph/ServiceList.js | 33 +++--- www/manager6/ceph/Services.js| 19 +- 3 files changed, 148 insertions(+), 11 deletions(-) diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js index db86fa9a..1d42be34 100644 --- a/www/manager6/Utils.js +++ b/www/manager6/Utils.js @@ -128,6 +128,113 @@ Ext.define('PVE.Utils', { return undefined; }, +/** + * Parses a Ceph build commit from its version string. [...] getMaxVersions: function(store, records, success) { diff --git a/www/manager6/ceph/Services.js b/www/manager6/ceph/Services.js index dfafee43..ff6f80e9 100644 --- a/www/manager6/ceph/Services.js +++ b/www/manager6/ceph/Services.js @@ -155,6 +155,7 @@ Ext.define('PVE.ceph.Services', { title: metadata[type][id].name || name, host: host, version: PVE.Utils.parse_ceph_version(metadata[type][id]), + buildcommit: PVE.Utils.parseCephBuildCommit(metadata[type][id]), service: metadata[type][id].service, addr: metadata[type][id].addr || metadata[type][id].addrs || Proxmox.Utils.unknownText, }; @@ -181,7 +182,15 @@ Ext.define('PVE.ceph.Services', { } if (result.version) { - result.statuses.push(gettext('Version') + ": " + result.version); + let installedBuildCommit = metadata.node[host]?.buildcommit ?? ''; + let runningBuildCommit = result.buildcommit ?? ''; + + let statusLine = gettext('Version') + `: ${result.version}`; + if (runningBuildCommit) { + statusLine += ` (build: ${runningBuildCommit.substring(0, 9)})`; + } + + result.statuses.push(statusLine); if (PVE.Utils.compare_ceph_versions(result.version, maxversion) !== 0) { let host_version = metadata.node[host]?.version?.parts || metadata.version?.[host] || ""; @@ -202,6 +211,14 @@ Ext.define('PVE.ceph.Services', { gettext('Other cluster members use a newer version of this service, please upgrade and restart'), ); } + } else if (installedBuildCommit !== "" && runningBuildCommit !== installedBuildCommit) { + if (result.health > healthstates.HEALTH_OLD) { + result.health = healthstates.HEALTH_OLD; + } + result.messages.push( + PVE
[pve-devel] [PATCH installer] post-hook: add `$version` field describing document schema version
This adds a metadata-field `$version` to the post-hook json, indicating which schema version (and thus structure) this document uses. The field is of format ".", following the semantic versioning meaning for both the major and minor number. A patch version is left out here, as it doesn't make much sense in this context. This was suggested by Thomas when originally introducing the post-hook functionality in [0]. [0] https://lore.proxmox.com/pve-devel/00019f8d-e4f2-420e-892a-b89e8b886...@proxmox.com/ Suggested-by: Thomas Lamprecht Signed-off-by: Christoph Heiss --- proxmox-post-hook/src/main.rs | 14 ++ 1 file changed, 14 insertions(+) diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs index 8558da2..fb51ba9 100644 --- a/proxmox-post-hook/src/main.rs +++ b/proxmox-post-hook/src/main.rs @@ -133,6 +133,17 @@ struct CpuInfo { #[derive(Serialize)] #[serde(rename_all = "kebab-case")] struct PostHookInfo { +/// major.minor version describing the schema version of this document, in a semanticy-version +/// way. +/// +/// major: Incremented for incompatible/breaking API changes, e.g. removing an existing +/// field. +/// minor: Incremented when adding functionality in a backwards-compatible matter, e.g. +/// adding a new field. +// This field is prefixed by `$` on purpose, to indicate that it is document metadata and not +// part of the actual content itself. (E.g. JSON Schema uses a similar naming scheme) +#[serde(rename = "$version")] +schema_version: String, /// major.minor version of Debian as installed, retrieved from /etc/debian_version debian_version: String, /// PVE/PMG/PBS version as reported by `pveversion`, `pmgversion` or @@ -166,6 +177,8 @@ struct PostHookInfo { const SIZE_GIB: usize = 1024 * 1024 * 1024; impl PostHookInfo { +const SCHEMA_VERSION: &str = "1.0"; + /// Gathers all needed information about the newly installed system for sending /// it to a specified server. /// @@ -215,6 +228,7 @@ impl PostHookInfo { }; Ok(Self { +schema_version: Self::SCHEMA_VERSION.to_owned(), debian_version: read_file("/etc/debian_version")?, product: Self::gather_product_info(&setup_info, &run_cmd)?, iso: setup_info.iso_info.clone(), -- 2.47.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v2] post-hook: add `$hook` field describing document schema version
This adds a metadata-field `$hook` containing a single key `version` (for now) to the post-hook json, indicating which schema version (and thus structure) this document uses. In the resulting JSON, this will look like this: { "$hook": { "version": "1.0" }, "debian-version": .., .. } The field follows the format "." and applies semantic versioning meaning for both the major and minor number. A patch version is left out here, as it doesn't make much sense in this context. This was suggested by Thomas when originally introducing the post-hook functionality in [0]. [0] https://lore.proxmox.com/pve-devel/00019f8d-e4f2-420e-892a-b89e8b886...@proxmox.com/ Suggested-by: Thomas Lamprecht Signed-off-by: Christoph Heiss --- v1: https://lore.proxmox.com/pve-devel/20241112131815.670475-2-c.he...@proxmox.com/ Changes v1 -> v2: * introduced `$hook` top-level field, nesting `version` under it proxmox-post-hook/src/main.rs | 31 +++ 1 file changed, 31 insertions(+) diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs index 8558da2..e2ae0ca 100644 --- a/proxmox-post-hook/src/main.rs +++ b/proxmox-post-hook/src/main.rs @@ -129,10 +129,40 @@ struct CpuInfo { sockets: usize, } +/// Metadata of the hook, such as schema version of the document. +#[derive(Serialize)] +#[serde(rename_all = "kebab-case")] +struct PostHookInfoMeta { +/// major.minor version describing the schema version of this document, in a semanticy-version +/// way. +/// +/// major: Incremented for incompatible/breaking API changes, e.g. removing an existing +/// field. +/// minor: Incremented when adding functionality in a backwards-compatible matter, e.g. +/// adding a new field. +version: String, +} + +impl PostHookInfoMeta { +const SCHEMA_VERSION: &str = "1.0"; +} + +impl Default for PostHookInfoMeta { +fn default() -> Self { +Self { +version: Self::SCHEMA_VERSION.to_owned(), +} +} +} + /// All data sent as request payload with the post-hook POST request. #[derive(Serialize)] #[serde(rename_all = "kebab-case")] struct PostHookInfo { +// This field is prefixed by `$` on purpose, to indicate that it is document metadata and not +// part of the actual content itself. (E.g. JSON Schema uses a similar naming scheme) +#[serde(rename = "$hook")] +hook_meta: PostHookInfoMeta, /// major.minor version of Debian as installed, retrieved from /etc/debian_version debian_version: String, /// PVE/PMG/PBS version as reported by `pveversion`, `pmgversion` or @@ -215,6 +245,7 @@ impl PostHookInfo { }; Ok(Self { +hook_meta: PostHookInfoMeta::default(), debian_version: read_file("/etc/debian_version")?, product: Self::gather_product_info(&setup_info, &run_cmd)?, iso: setup_info.iso_info.clone(), -- 2.47.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH novnc v2 2/3] add BUILDDIR to clean make target
Am 12.11.24 um 08:51 schrieb Dominik Csapak: > so it gets cleaned up too > > Signed-off-by: Dominik Csapak > --- > new in v2 > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 431dff9..d1159e0 100644 > --- a/Makefile > +++ b/Makefile > @@ -57,7 +57,7 @@ distclean: clean > > .PHONY: clean > clean: > - rm -rf $(PACKAGE)-[0-9]*/ $() *.deb *.dsc $(PACKAGE)*.tar.[gx]z > *.changes *.dsc *.buildinfo *.build > + rm -rf $(BUILDDIR) $(PACKAGE)-[0-9]*/ $() *.deb *.dsc > $(PACKAGE)*.tar.[gx]z *.changes *.dsc *.buildinfo *.build > > .PHONY: dinstall > dinstall: deb I replaced this with changing the build-dir to be derived from package name and upstream version, like most modern packages of ours (and using dget on debian source packages) do. The clean target already assumed this to be the case. diff --git a/Makefile b/Makefile index 431dff9..1df5937 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ include /usr/share/dpkg/default.mk PACKAGE=novnc-pve SRCDIR=novnc -BUILDDIR=$(SRCDIR).tmp +BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM) ORIG_SRC_TAR=$(PACKAGE)_$(DEB_VERSION_UPSTREAM).orig.tar.gz GITVERSION:=$(shell git rev-parse HEAD) Also adapted the build-dir target to be atomic in another, unrelated, follow-up. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 3/3] ui: resource tree: show nodes/storages in pool/tag view by default
On 11/12/24 14:38, Thomas Lamprecht wrote: Am 12.11.24 um 12:45 schrieb Dominik Csapak: and make it configurable in the tree browser settings. this makes now use of the new sorting order and the more efficient 'getFilterFn' method of the viewSelector works OK besides some eslint warnings (see below), but what I noticed is that we never listed storages under their assigned pools, which is IMO a bit odd and might be something to fix separately. yeah looking at this, i noticed we don't return the pools for storages at all currently in the resources api call. Also we can have a storage in multiple pools (in contrast to guests) so that is a bit of a problem here. I could return the pools as comma separated list (that would satisfy the return schema, but isn't pretty) and use the same mechanism for duplicating storages as for the tag view. should i go this route or should i omit this for the pool view for now and simply add the toggle for the tag view? Btw. having this default one for pool view might not be welcomed by all, as some used this view to hide the nodes and that stuff for those people that are "afraid" of them (managers and such), but no hard feelings here. Grouping the resource without tag or pool under a collapsible node, say "Resources without Tag/Pool" for avoiding spending time with naming, might be a trade-off here. Btw. having an expand-all and collapse-all inline button between view selection and settings might be nice too for bigger setups, and probably not that much work. I.e., somewhat similar to how the API viewer endpoint tree has those [0], just not as header tools. [0]: https://pve.proxmox.com/pve-docs/api-viewer/index.html Signed-off-by: Dominik Csapak --- not sure if we should make each option seperate, so either separate options for tag/pool view or separate options for nodes/storages (sdn maybe too?) or both (that would be 4 new options) If I'd do a multi-select combobox for each Pool and Tag view, but we might wait that out to see if there is actual user demand. www/manager6/UIOptions.js | 1 + www/manager6/form/ViewSelector.js | 16 ++-- www/manager6/tree/ResourceTree.js | 2 +- www/manager6/window/TreeSettingsEdit.js | 13 + 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/www/manager6/UIOptions.js b/www/manager6/UIOptions.js index 057c8f03..97b689b0 100644 --- a/www/manager6/UIOptions.js +++ b/www/manager6/UIOptions.js @@ -93,6 +93,7 @@ Ext.define('PVE.UIOptions', { 'sort-field': 'vmid', 'group-templates': true, 'group-guest-types': true, + 'more-types': true, }; return browserValues?.[key] ?? defaults[key]; diff --git a/www/manager6/form/ViewSelector.js b/www/manager6/form/ViewSelector.js index f5de5c8e..8c656fc8 100644 --- a/www/manager6/form/ViewSelector.js +++ b/www/manager6/form/ViewSelector.js @@ -30,12 +30,24 @@ Ext.define('PVE.form.ViewSelector', { text: gettext('Pool View'), groups: ['pool'], // Pool View only lists VMs and Containers - getFilterFn: () => ({ data }) => data.type === 'qemu' || data.type === 'lxc' || data.type === 'pool', + getFilterFn: function() { + let types = ['qemu', 'lxc', 'pool']; + if (PVE.UIOptions.getTreeSortingValue('more-types')) { + types.push('node', 'storage'); + } + return ({data}) => types.indexOf(data.type) !== -1; above causes eslint to output warnings due to missing spaces for `{ data }` + }, }, tags: { text: gettext('Tag View'), groups: ['tag'], - getFilterFn: () => ({ data }) => data.type === 'qemu' || data.type === 'lxc', + getFilterFn: function() { + let types = ['qemu', 'lxc']; + if (PVE.UIOptions.getTreeSortingValue('more-types')) { + types.push('node', 'storage'); + } + return ({data}) => types.indexOf(data.type) !== -1; same eslint warning as above sorry, i have to stop testing gui patches with just 'make install'... ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH novnc v2 1/3] upgrade noVNC and patches to 1.5.0
Am 12.11.24 um 08:51 schrieb Dominik Csapak: > Signed-off-by: Dominik Csapak > --- > changes from v1: > * rebased on master > > .../0001-add-PVE-specific-JS-code.patch | 4 ++-- > ...002-add-custom-fbresize-event-on-rfb.patch | 10 +- > ...nge-scaling-when-toggling-fullscreen.patch | 6 +++--- > ...rectory-for-fetching-images-js-files.patch | 20 +-- > .../0009-decrease-animation-time.patch| 2 +- > .../0011-add-localCursor-setting-to-rfb.patch | 18 - > ...ove-the-default-value-of-wsProtocols.patch | 4 ++-- > novnc | 2 +- > 8 files changed, 33 insertions(+), 33 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH novnc v2 3/3] bump version to 1.5.0-1
Am 12.11.24 um 08:51 schrieb Dominik Csapak: > Signed-off-by: Dominik Csapak > --- > changes from v1: > * rebased on master > > debian/changelog | 8 > 1 file changed, 8 insertions(+) > > applied, thanks! But I dropped the note w.r.t. to the buildsys fix, for packages that are not development ones such things are not really interesting as the package content did not change at all due to this. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] fix #5847: ui: gettextize several strings in ui
> Dominik Csapak hat am 12.11.2024 10:16 CET geschrieben: > > > one comment inline > > On 11/11/24 12:35, Timothy Nicholson wrote: > > Several strings that should probably also be translated now use > > the gettext function to be translated. > > > > Signed-off-by: Timothy Nicholson > > --- > > [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=5847 > > > > www/manager6/Workspace.js | 2 +- > > www/manager6/ceph/OSD.js | 6 +++--- > > www/manager6/ceph/OSDDetails.js| 2 +- > > www/manager6/form/VMCPUFlagSelector.js | 24 > > www/manager6/qemu/ProcessorEdit.js | 2 +- > > www/manager6/window/Settings.js| 6 +++--- > > 6 files changed, 21 insertions(+), 21 deletions(-) > > > > diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js > > index 52c66108..56dde151 100644 > > --- a/www/manager6/Workspace.js > > +++ b/www/manager6/Workspace.js > > @@ -388,7 +388,7 @@ Ext.define('PVE.StdWorkspace', { > > }, > > }, > > { > > - text: 'TFA', > > + text: gettext('TFA'), > > is it a good idea to translate this? it's mostly a technical term (which we > generally don't > translate), but i'd be swayed if there is an actual example where a language > want > to have that translated... Some languages already translate TFA as 2FA (e.g. German) and TFA is already translated in other parts of the UI. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v2] fix #5847: ui: gettextize several strings in ui
Signed-off-by: Timothy Nicholson --- I missed one occurence of the String 'TFA' not using gettext, so here is the updated patch. www/manager6/Workspace.js | 2 +- www/manager6/ceph/OSD.js | 6 +++--- www/manager6/ceph/OSDDetails.js| 2 +- www/manager6/dc/UserView.js| 2 +- www/manager6/form/VMCPUFlagSelector.js | 24 www/manager6/qemu/ProcessorEdit.js | 2 +- www/manager6/window/Settings.js| 6 +++--- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js index 52c66108..56dde151 100644 --- a/www/manager6/Workspace.js +++ b/www/manager6/Workspace.js @@ -388,7 +388,7 @@ Ext.define('PVE.StdWorkspace', { }, }, { - text: 'TFA', + text: gettext('TFA'), itemId: 'tfaitem', iconCls: 'fa fa-fw fa-lock', handler: function(btn, event, rec) { diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js index d2caafa4..5a32769c 100644 --- a/www/manager6/ceph/OSD.js +++ b/www/manager6/ceph/OSD.js @@ -268,11 +268,11 @@ Ext.define('PVE.CephSetFlags', { dataIndex: 'value', }, { - text: 'Name', + text: gettext('Name'), dataIndex: 'name', }, { - text: 'Description', + text: gettext('Description'), flex: 1, dataIndex: 'description', }, @@ -716,7 +716,7 @@ Ext.define('PVE.node.CephOsdTree', { columns: [ { xtype: 'treecolumn', - text: 'Name', + text: gettext('Name'), dataIndex: 'name', width: 150, }, diff --git a/www/manager6/ceph/OSDDetails.js b/www/manager6/ceph/OSDDetails.js index 3b1c1d9c..5c14aa05 100644 --- a/www/manager6/ceph/OSDDetails.js +++ b/www/manager6/ceph/OSDDetails.js @@ -220,7 +220,7 @@ Ext.define('PVE.CephOsdDetails', { renderer: Proxmox.Utils.render_size, }, { - text: 'Discard', + text: gettext('Discard'), dataIndex: 'support_discard', hidden: true, }, diff --git a/www/manager6/dc/UserView.js b/www/manager6/dc/UserView.js index 12c3e854..d3b88701 100644 --- a/www/manager6/dc/UserView.js +++ b/www/manager6/dc/UserView.js @@ -189,7 +189,7 @@ Ext.define('PVE.dc.UserView', { dataIndex: 'firstname', }, { - header: 'TFA', + header: gettext('TFA'), width: 120, sortable: true, renderer: function(v, metaData, record) { diff --git a/www/manager6/form/VMCPUFlagSelector.js b/www/manager6/form/VMCPUFlagSelector.js index ace3c531..48589d3f 100644 --- a/www/manager6/form/VMCPUFlagSelector.js +++ b/www/manager6/form/VMCPUFlagSelector.js @@ -21,18 +21,18 @@ Ext.define('PVE.form.VMCPUFlagSelector', { fields: ['flag', { name: 'state', defaultValue: '=' }, 'desc'], data: [ // FIXME: let qemu-server host this and autogenerate or get from API call?? - { flag: 'md-clear', desc: 'Required to let the guest OS know if MDS is mitigated correctly' }, - { flag: 'pcid', desc: 'Meltdown fix cost reduction on Westmere, Sandy-, and IvyBridge Intel CPUs' }, - { flag: 'spec-ctrl', desc: 'Allows improved Spectre mitigation with Intel CPUs' }, - { flag: 'ssbd', desc: 'Protection for "Speculative Store Bypass" for Intel models' }, - { flag: 'ibpb', desc: 'Allows improved Spectre mitigation with AMD CPUs' }, - { flag: 'virt-ssbd', desc: 'Basis for "Speculative Store Bypass" protection for AMD models' }, - { flag: 'amd-ssbd', desc: 'Improves Spectre mitigation performance with AMD CPUs, best used with "virt-ssbd"' }, - { flag: 'amd-no-ssb', desc: 'Notifies guest OS that host is not vulnerable for Spectre on AMD CPUs' }, - { flag: 'pdpe1gb', desc: 'Allow guest OS to use 1GB size pages, if host HW supports it' }, - { flag: 'hv-tlbflush', desc: 'Improve performance in overcommitted Windows guests. May lead to guest bluescreens on old CPUs.' }, - { flag: 'hv-evmcs
Re: [pve-devel] [PATCH widget-toolkit v5 4/7] fix #3892: Network: add bridge vids field for bridge_vids
On 2024-11-11 21:55, Thomas Lamprecht wrote: Am 02.10.24 um 15:11 schrieb Aaron Lauterer: The new optional bridge_vids field allows to set that property via the GUI. Since the backend needs to support it, the field needs to be explicitly enabled. For now, Proxmox VE (PVE) is the use case. Signed-off-by: Aaron Lauterer Tested-By: Stefan Hanreich Reviewed-by: Shannon Sterz --- changes since v4: none v3: * switched regex to one with non-capturing group * reworked valid VLAN check according to the suggestion v2: * added validation code following how it is implemented in the API src/node/NetworkEdit.js | 65 + src/node/NetworkView.js | 5 2 files changed, 70 insertions(+) diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js index 27c1baf..bfd0268 100644 --- a/src/node/NetworkEdit.js +++ b/src/node/NetworkEdit.js @@ -2,6 +2,9 @@ Ext.define('Proxmox.node.NetworkEdit', { extend: 'Proxmox.window.Edit', alias: ['widget.proxmoxNodeNetworkEdit'], +// Enable to show the VLAN ID field +bridge_set_vids: false, + initComponent: function() { let me = this; @@ -57,11 +60,70 @@ Ext.define('Proxmox.node.NetworkEdit', { } if (me.iftype === 'bridge') { + let vids = Ext.create('Ext.form.field.Text', { + fieldLabel: gettext('Bridge VIDS'), I know ifupdown2 names it VIDS, but is that really a good name here? AFAICT it's not used outside of ifupdown2/cumuls, or do you got any references? Maybe "Bridge VLAN IDs" would be a bit more telling? I tested it, and that is long enough to cause a line break in the label. I think I would opt for "VLAN IDs". It is only present on linux bridges and with the info box below as well it should be clear what it is for, without causing layout issues. Will send a small v6 with the other suggestions too. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common, widget-toolkit, manager v5 0/7] fix #3893: make bridge vids configurable
sent a v6 https://lore.proxmox.com/pve-devel/20241112092554.106723-1-a.laute...@proxmox.com/T/#t On 2024-10-02 15:11, Aaron Lauterer wrote: Since this version reworks a few things, especially in the logic, I dropped the r-b and t-b tags in some patches. The following patch has been dropped as this is handled in the API call itself now. [PATCH common v4 3/7] inotify: interfaces: make sure bridge_vids use space as separator There is now a new one (2/7) changing the check in the iNotify.pm file slightly. this version reworks a few parts since v4: incorporated @fionas suggestions, besides some style nits: * rename check_list_empty to list_is_empty and adapt the return values * renamed $check_vid to $valid_vid with return values for each case * switch list separators to spaces in the API call, instead of when writing the file * rework small logical error in the check if the end VLAN ID is larger than the start and $noerr is set. v3: incorporated @shannons recommendations, in detail: * improve regex with non-capturing group * reworked check if valid VLAN config in UI field check * small code style and spelling things * reworded UI explanation text for bridge vids v2: * renamed format in JSONSchema to a more generic `pve-vlan-id-or-range` * explicitly use spaces when writing interfaces file. This is one possible approach to deal with the fact, that the generic `-list` format will accept quite a few delimiters and we need spaces. * code style improvements such as naming the regex results. * add parameter verification to the web ui With the changes to the JSONSchema we can then work on using it too for the guest trunk option. This hasn't been started yet though. common: Aaron Lauterer (3): tools: add check_list_empty function inotify: interfaces: check if bridge_vids is truthy instead of defined fix #3893: network: add vlan id and range parameter definitions src/PVE/INotify.pm| 2 +- src/PVE/JSONSchema.pm | 39 +++ src/PVE/Tools.pm | 8 3 files changed, 48 insertions(+), 1 deletion(-) widget-toolkit: Aaron Lauterer (2): fix #3892: Network: add bridge vids field for bridge_vids Network: add explanation for bridge vids field src/node/NetworkEdit.js | 73 + src/node/NetworkView.js | 5 +++ 2 files changed, 78 insertions(+) manager: Aaron Lauterer (2): fix #3893: api: network: add bridge_vids parameter fix #3893: ui: network: enable bridge_vids field PVE/API2/Network.pm | 23 ++- www/manager6/node/Config.js | 1 + 2 files changed, 23 insertions(+), 1 deletion(-) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] partially-applied: [RFC qemu/common/storage/qemu-server/container/manager v3 00/34] backup provider API
Am 07.11.24 um 17:51 schrieb Fiona Ebner: > Fiona Ebner (9): > block/reqlist: allow adding overlapping requests > PVE backup: fixup error handling for fleecing > PVE backup: factor out setting up snapshot access for fleecing > PVE backup: save device name in device info structure > PVE backup: include device name in error when setting up snapshot > access fails Applied above QEMU patches already. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-network v2 3/4] vnets : add ports isolation
From: Alexandre Derumier via pve-devel Add support for bridge ports isolation https://github.com/torvalds/linux/commit/7d850abd5f4edb1b1ca4b4141a4453305736f564 This allow to drop traffic between all ports having isolation enabled on the local bridge, but allow traffic with non isolated ports. Here,we isolate traffic between vms but allow traffic coming from outside. Main usage is for layer3 routed or natted setup, but some users have requested it for layer2/bridge network with proxy arp. So we can enable it at vnet level. Signed-off-by: Alexandre Derumier [ SH: improve option naming and description slightly ] Signed-off-by: Stefan Hanreich --- src/PVE/Network/SDN/VnetPlugin.pm | 5 + src/PVE/Network/SDN/Zones/Plugin.pm | 1 + 2 files changed, 6 insertions(+) diff --git a/src/PVE/Network/SDN/VnetPlugin.pm b/src/PVE/Network/SDN/VnetPlugin.pm index 062904c..f44380a 100644 --- a/src/PVE/Network/SDN/VnetPlugin.pm +++ b/src/PVE/Network/SDN/VnetPlugin.pm @@ -72,6 +72,10 @@ sub properties { maxLength => 256, optional => 1, }, + 'isolate-ports' => { + type => 'boolean', + description => "If true, sets the isolated property for all members of this VNet", + } }; } @@ -81,6 +85,7 @@ sub options { tag => { optional => 1}, alias => { optional => 1 }, vlanaware => { optional => 1 }, + 'isolate-ports' => { optional => 1 }, }; } diff --git a/src/PVE/Network/SDN/Zones/Plugin.pm b/src/PVE/Network/SDN/Zones/Plugin.pm index 26cc0da..a860168 100644 --- a/src/PVE/Network/SDN/Zones/Plugin.pm +++ b/src/PVE/Network/SDN/Zones/Plugin.pm @@ -236,6 +236,7 @@ sub tap_plug { my $opts = {}; $opts->{learning} = 0 if $plugin_config->{'bridge-disable-mac-learning'}; +$opts->{isolation} = 1 if $vnet->{'isolate-ports'}; PVE::Network::tap_plug($iface, $vnetid, $tag, $firewall, $trunks, $rate, $opts); } -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer v2] post-hook: add `$hook` field describing document schema version
Am 12.11.24 um 15:53 schrieb Christoph Heiss: > This adds a metadata-field `$hook` containing a single key `version` > (for now) to the post-hook json, indicating which schema version (and > thus structure) this document uses. > > In the resulting JSON, this will look like this: > { > "$hook": { > "version": "1.0" > }, > "debian-version": .., > .. > } > > The field follows the format "." and applies semantic > versioning meaning for both the major and minor number. A patch version > is left out here, as it doesn't make much sense in this context. > > This was suggested by Thomas when originally introducing the post-hook > functionality in [0]. > > [0] > https://lore.proxmox.com/pve-devel/00019f8d-e4f2-420e-892a-b89e8b886...@proxmox.com/ > > Suggested-by: Thomas Lamprecht > Signed-off-by: Christoph Heiss > --- > v1: > https://lore.proxmox.com/pve-devel/20241112131815.670475-2-c.he...@proxmox.com/ > > Changes v1 -> v2: > * introduced `$hook` top-level field, nesting `version` under it > > proxmox-post-hook/src/main.rs | 31 +++ > 1 file changed, 31 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-manager v2 2/4] sdn: vnet: add isolate-ports option
From: Alexandre Derumier via pve-devel We add this as advanced option in the UI and also move vlan-aware to advanced section. Signed-off-by: Alexandre Derumier [ SH: improve naming and commit msg slightly ] Signed-off-by: Stefan Hanreich --- www/manager6/sdn/VnetEdit.js | 12 1 file changed, 12 insertions(+) diff --git a/www/manager6/sdn/VnetEdit.js b/www/manager6/sdn/VnetEdit.js index 9fb6cd6c7..e5870893e 100644 --- a/www/manager6/sdn/VnetEdit.js +++ b/www/manager6/sdn/VnetEdit.js @@ -71,6 +71,18 @@ Ext.define('PVE.sdn.VnetInputPanel', { deleteEmpty: "{!isCreate}", }, }, +], +advancedItems: [ + { + xtype: 'proxmoxcheckbox', + name: 'isolate-ports', + uncheckedValue: null, + checked: false, + fieldLabel: gettext('Isolate Ports'), + cbind: { + deleteEmpty: "{!isCreate}", + }, + }, { xtype: 'proxmoxcheckbox', itemId: 'sdnVnetVlanAwareField', -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit v5 4/7] fix #3892: Network: add bridge vids field for bridge_vids
Am 12.11.24 um 10:03 schrieb Aaron Lauterer: > On 2024-11-11 21:55, Thomas Lamprecht wrote: >> Am 02.10.24 um 15:11 schrieb Aaron Lauterer: >>> if (me.iftype === 'bridge') { >>> + let vids = Ext.create('Ext.form.field.Text', { >>> + fieldLabel: gettext('Bridge VIDS'), >> >> I know ifupdown2 names it VIDS, but is that really a good name here? >> AFAICT it's not used outside of ifupdown2/cumuls, or do you got any >> references? >> >> Maybe "Bridge VLAN IDs" would be a bit more telling? > > I tested it, and that is long enough to cause a line break in the label. While not breaking lines is naturally nice as long as there is a concise and understandable text, but going for such niche abbreviations is IMO worse compared to breaking layout slightly. And FWIW, we could also make the labels longer. > I think I would opt for "VLAN IDs". It is only present on linux bridges > and with the info box below as well it should be clear what it is for, > without causing layout issues. I'd still rather have a docs patch compared to the info box, maybe use a tooltip there instead for a less intrusive hint? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH installer] buildsys: add static version of proxmox-auto-install-assistant
On Mon, Nov 11, 2024 at 11:31:05PM +0100, Thomas Lamprecht wrote: > Am 16.08.24 um 18:19 schrieb Christoph Heiss: > > This adds a separate debian package to the build, containing a > > statically-built version of `proxmox-auto-install-assistant`, as > > was suggested in #4788 [0] (for proxmox-backup-client), separately by > > Thomas also. > > [..] > > --- > > Marked RFC to see if the packaging is overall sane -- I haven't done a > > lot of (Debian) packaging yet, so please don't hesitate with > > suggestions. > > That looks alright to me, only open question is IMO if we want to have the > compat symlink or avoid that and allow installing both in parallel (mostly > useful for testing, but slightly worse UX for users, no hard feelings here) I would side with the better UX side of things. While yes, makes having both installed at the same impossible, I don't think we should necessarily degrade ease-of-testing over better UX. Also, since it is a static binary after all, building and/or grabbing it from e.g. some share for testing would be easy enough TBH. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC common v3 10/34] env: add module with helpers to run a Perl subroutine in a user namespace
On 11.11.24 7:33 PM, Thomas Lamprecht wrote: > Am 07.11.24 um 17:51 schrieb Fiona Ebner: >> +package PVE::Env; > > can this module and it's name be more specific to doing stuff with/in > namespaces? > > e.g. PVE::Namespaces or PVE::Sys::Namespaces (there might be other stuff that > might > fit well in a future libproxmox-sys-perl and Proxmox::Sys::* respectively, so > maybe that module path would be better?) > > I'd also make all sub's private if not really intended to be used outside > this module. > Will do! > If the more general fork/wait-child helpers are needed elsewhere, or deemed > to be useful, then they could go in their own module, like e.g. > PVE::Sys::Process > Since you already pointed out a potential user, will go for this too. >> +sub forked(&%) { > > FWIW, there's some "forked" method in test/lock_file.pl that this might > replace too, > if it stay public. > I'll check if it can be adapted easily. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-docs v2 4/4] sdn: add documentation for isolated ports option
Signed-off-by: Stefan Hanreich --- pvesdn.adoc | 5 + 1 file changed, 5 insertions(+) diff --git a/pvesdn.adoc b/pvesdn.adoc index 39de80f..b1f2578 100644 --- a/pvesdn.adoc +++ b/pvesdn.adoc @@ -383,6 +383,11 @@ Tag:: The unique VLAN or VXLAN ID VLAN Aware:: Enables vlan-aware option on the interface, enabling configuration in the guest. +Isolate Ports:: Sets the isolated flag for all members of this port, except for +the bridge port. This means that every port can only send traffic to the bridge +port. In order for this setting to take effect, you need to restart the VMs +that have interfaces on the VNet. + [[pvesdn_config_subnet]] Subnets -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 pve-manager 0/5] Fix #5366: Ceph Build Commit in UI
Tested the series in my test cluster. Had one node with a newer build recently installed but no services restarted just yet. The build short IDs were visible where I would expect it and the Ceph status, MON, OSD and MDS panels showed nicely that the services should be restarted to run the new build. One thing I noticed is that if one node has a newer build installed, the OSDs on that node show, as expected, that they need to be restarted to run the latest build. But the other nodes don't show any hint that there is a newer build used in the cluster, and the remaining nodes should update. I don't see an easy way how we could achieve that consistently with the current approach where we don't know the apt package versions used in the cluster. Only found one tiny nit in patch 1/5 in the phrasing of one message. Consider this series: Tested-By: Aaron Lauterer Reviewed-By: Aaron Lauterer On 2024-07-24 17:05, Max Carrara wrote: Ceph Build Commit in UI - Version 3 === Notable Changes Since v2 * Rebase on master branch as v2 was partially applied (thanks!) * Factor duplicate build commit rendering code into separate helper as suggested [1] (thanks Thomas!) * Make variable names more clear / less ambiguous (thanks Thomas!) * Increase default width of version column in OSD tree view ever so slightlymore * Reword commit titles and messages to reflect the changes made For a detailed list of changes please see the comments in the individual patches. NOTE: Any T-b and R-b trailers on patches that received changes are considered outdated and are thus removed. Older Versions -- v1: https://lists.proxmox.com/pipermail/pve-devel/2024-April/063772.html v2: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064349.html References -- [1]: https://lists.proxmox.com/pipermail/pve-devel/2024-July/064789.html Summary of Changes -- Max Carrara (5): fix #5366: ui: ceph: services: parse and display build commit fix #5366: api: ceph: add host build commit to Ceph OSD index data fix #5366: ui: ceph: osd: rework version field rendering ui: ceph: osd: increase width of version column fix #5366: api: ceph: change version format in OSD metadata endpoint PVE/API2/Ceph/OSD.pm | 9 ++- www/manager6/Utils.js| 107 +++ www/manager6/ceph/OSD.js | 54 www/manager6/ceph/ServiceList.js | 33 +++--- www/manager6/ceph/Services.js| 19 +- 5 files changed, 198 insertions(+), 24 deletions(-) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC common v3 10/34] env: add module with helpers to run a Perl subroutine in a user namespace
On November 7, 2024 5:51 pm, Fiona Ebner wrote: > The first use case is running the container backup subroutine for > external providers inside a user namespace. That allows them to see > the filesystem to back-up from the containers perspective and also > improves security because of isolation. > > Copied and adapted the relevant parts from the pve-buildpkg > repository. > > Originally-by: Wolfgang Bumiller > [FE: add $idmap parameter, drop $aux_groups parameter] > Signed-off-by: Fiona Ebner > --- > > New in v3. > > src/Makefile | 1 + > src/PVE/Env.pm | 136 + > 2 files changed, 137 insertions(+) > create mode 100644 src/PVE/Env.pm > > diff --git a/src/Makefile b/src/Makefile > index 2d8bdc4..dba26e3 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -15,6 +15,7 @@ LIB_SOURCES = \ > Certificate.pm \ > CpuSet.pm \ > Daemon.pm \ > + Env.pm \ > Exception.pm \ > Format.pm \ > INotify.pm \ > diff --git a/src/PVE/Env.pm b/src/PVE/Env.pm > new file mode 100644 > index 000..e11bec0 > --- /dev/null > +++ b/src/PVE/Env.pm > @@ -0,0 +1,136 @@ > +package PVE::Env; I agree with Thomas that this name might be a bit too generic ;) I also wonder - since this seems to be only used in pve-container, and it really mostly makes sense in that context, wouldn't it be better off there? or do we expect other areas where we need userns handling? (granted, some of the comments below would require other changes to pve-common anyway ;)) > + > +use strict; > +use warnings; > + > +use Fcntl qw(O_WRONLY); > +use POSIX qw(EINTR); > +use Socket; > + > +require qw(syscall.ph); PVE::Syscall already does this, and has the following: BEGIN { die "syscall.ph can only be required once!\n" if $INC{'syscall.ph'}; require("syscall.ph"); don't those two clash? I think those syscall related parts should probably move there? > + > +use constant {CLONE_NEWNS => 0x0002, > + CLONE_NEWUSER => 0x1000}; > + > +sub unshare($) { > +my ($flags) = @_; > +return 0 == syscall(272, $flags); > +} this is PVE::Tools::unshare, maybe the latter should move here? > + > +sub __set_id_map($$$) { > +my ($pid, $what, $value) = @_; > +sysopen(my $fd, "/proc/$pid/${what}_map", O_WRONLY) > + or die "failed to open child process' ${what}_map\n"; > +my $rc = syswrite($fd, $value); > +if (!$rc || $rc != length($value)) { > + die "failed to set sub$what: $!\n"; > +} > +close($fd); > +} > + > +sub set_id_map($$) { > +my ($pid, $id_map) = @_; > + > +my $gid_map = ''; > +my $uid_map = ''; > + > +for my $map ($id_map->@*) { > + my ($type, $ct, $host, $length) = $map->@*; > + > + $gid_map .= "$ct $host $length\n" if $type eq 'g'; > + $uid_map .= "$ct $host $length\n" if $type eq 'u'; > +} > + > +__set_id_map($pid, 'gid', $gid_map) if $gid_map; > +__set_id_map($pid, 'uid', $uid_map) if $uid_map; > +} do we gain a lot here from not just using newuidmap/newgidmap? > + > +sub wait_for_child($;$) { > +my ($pid, $noerr) = @_; > +my $interrupts = 0; > +while (waitpid($pid, 0) != $pid) { > + if ($! == EINTR) { > + warn "interrupted...\n"; > + kill(($interrupts > 3 ? 9 : 15), $pid); > + $interrupts++; > + } > +} > +my $status = POSIX::WEXITSTATUS($?); > +return $status if $noerr; > + > +if ($? == -1) { > + die "failed to execute\n"; > +} elsif (POSIX::WIFSIGNALED($?)) { > + my $sig = POSIX::WTERMSIG($?); > + die "got signal $sig\n"; > +} elsif ($status != 0) { > + warn "exit code $status\n"; > +} > +return $status; > +} > + > +sub forked(&%) { this seems very similar to the already existing PVE::Tools::run_fork / run_fork_with_timeout helpers.. any reason we can't extend those with `afterfork` support and use them? > +my ($code, %opts) = @_; > + > +pipe(my $except_r, my $except_w) or die "pipe: $!\n"; > + > +my $pid = fork(); > +die "fork failed: $!\n" if !defined($pid); > + > +if ($pid == 0) { > + close($except_r); > + eval { $code->() }; > + if ($@) { > + print {$except_w} $@; > + $except_w->flush(); > + POSIX::_exit(1); > + } > + POSIX::_exit(0); > +} > +close($except_w); > + > +my $err; > +if (my $afterfork = $opts{afterfork}) { > + eval { $afterfork->($pid); }; > + if ($err = $@) { > + kill(15, $pid); > + $opts{noerr} = 1; > + } > +} > +if (!$err) { > + $err = do { local $/ = undef; <$except_r> }; > +} > +my $rv = wait_for_child($pid, $opts{noerr}); > +die $err if $err; > +die "an unknown error occurred\n" if $rv != 0; > +return $rv; > +} > + > +sub run_in_userns(&;$) { > +my ($code, $id_map) = @_; > +socketpair(my $sp, my $sc, AF_UNIX, SOCK_STREAM, PF_UNSPEC) > + or die "socketpair: $!\n"; > +forked(sub { > +
Re: [pve-devel] applied: [RFC manager] triggers: add path-based trigger interest
Am 12.11.24 um 10:11 schrieb Fabian Grünbichler: > the HA case could also switch over to this approach, if we want to > reload HA for all PVE perl modules.. if we only want it for a subset, > then yes, the/an explicit trigger is better :) [...] > see above - the question is whether we want an explicit list of packages > that trigger HA (and risk that it runs out of sync with the modules the > HA actually uses), or whether we want to treat HA like the pve-manager > services, and just let it reload on any perl module changes that *could* > affect it.. > > if desired, I can send a similar patch for pve-ha-manager as well. Yes, that's an option, but I was wording my reply from yesterday so vague by choice (or well, being to lazy to decide so late) to not go in a explicit direction, as while it would be nice to have this covered in a general fashion, more frequently restarting HA is also something that can increase problem frequency. But, tbh., it should not be that much and it has to work anyway, so can be fine by me. Still needs the trigger from pmxcfs I guess? As IMO we should not depend that it will always ship in the same package as some perl code that falls under your generic trigger. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH installer] buildsys: add static version of proxmox-auto-install-assistant
Am 12.11.24 um 11:07 schrieb Christoph Heiss: > On Mon, Nov 11, 2024 at 11:31:05PM +0100, Thomas Lamprecht wrote: >> Am 16.08.24 um 18:19 schrieb Christoph Heiss: >>> This adds a separate debian package to the build, containing a >>> statically-built version of `proxmox-auto-install-assistant`, as >>> was suggested in #4788 [0] (for proxmox-backup-client), separately by >>> Thomas also. >>> [..] >>> --- >>> Marked RFC to see if the packaging is overall sane -- I haven't done a >>> lot of (Debian) packaging yet, so please don't hesitate with >>> suggestions. >> >> That looks alright to me, only open question is IMO if we want to have the >> compat symlink or avoid that and allow installing both in parallel (mostly >> useful for testing, but slightly worse UX for users, no hard feelings here) > > I would side with the better UX side of things. While yes, makes having > both installed at the same impossible, I don't think we should > necessarily degrade ease-of-testing over better UX. FWIW, some Debian packages use a separate package for that, e.g. sequoia (rust GPG thing) has a "sequoia-chameleon-gnupg" package providing the compatible binary under "/usr/bin/gpg-sq" and a separate package "gpg-from-sq" that provides the symlink. If you want to have the static thing do both by default you could slightly adapt that design and do: - proxmox-auto-install-assistant-static-impl -> the executable and docs and a Suggests entry for proxmox-auto-install-assistant-static - proxmox-auto-install-assistant-static -> the compat symlink and hard dependency to proxmox-auto-install-assistant-static-impl. > Also, since it is a static binary after all, building and/or grabbing it > from e.g. some share for testing would be easy enough TBH. Testing does not always imply knowing devs or experienced admins, can also be users on an older ubuntu or what not. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [RFC manager] triggers: add path-based trigger interest
> Thomas Lamprecht hat am 12.11.2024 10:52 CET > geschrieben: > > > Am 12.11.24 um 10:11 schrieb Fabian Grünbichler: > > the HA case could also switch over to this approach, if we want to > > reload HA for all PVE perl modules.. if we only want it for a subset, > > then yes, the/an explicit trigger is better :) > > [...] > > > see above - the question is whether we want an explicit list of packages > > that trigger HA (and risk that it runs out of sync with the modules the > > HA actually uses), or whether we want to treat HA like the pve-manager > > services, and just let it reload on any perl module changes that *could* > > affect it.. > > > > if desired, I can send a similar patch for pve-ha-manager as well. > > Yes, that's an option, but I was wording my reply from yesterday so vague > by choice (or well, being to lazy to decide so late) to not go in a > explicit direction, as while it would be nice to have this covered in a > general fashion, more frequently restarting HA is also something that > can increase problem frequency. But, tbh., it should not be that much and > it has to work anyway, so can be fine by me. > > Still needs the trigger from pmxcfs I guess? As IMO we should not depend > that it will always ship in the same package as some perl code that falls > under your generic trigger. do we need a trigger for pmxcfs itself? if it gets updated, it restarts itself anyway, and nothing else directly depends on it? the perl bindings to talk to pmxcfs (PVE::IPCC and PVE::Cluster) are covered by the generic path-based trigger for /usr/share/perl5/PVE , so anything using that to talk to it should just express interest on that path.. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit v5 4/7] fix #3892: Network: add bridge vids field for bridge_vids
On 2024-11-12 10:55, Thomas Lamprecht wrote: Am 12.11.24 um 10:03 schrieb Aaron Lauterer: On 2024-11-11 21:55, Thomas Lamprecht wrote: Am 02.10.24 um 15:11 schrieb Aaron Lauterer: if (me.iftype === 'bridge') { + let vids = Ext.create('Ext.form.field.Text', { + fieldLabel: gettext('Bridge VIDS'), I know ifupdown2 names it VIDS, but is that really a good name here? AFAICT it's not used outside of ifupdown2/cumuls, or do you got any references? Maybe "Bridge VLAN IDs" would be a bit more telling? I tested it, and that is long enough to cause a line break in the label. While not breaking lines is naturally nice as long as there is a concise and understandable text, but going for such niche abbreviations is IMO worse compared to breaking layout slightly. And FWIW, we could also make the labels longer. Okay, I can send a follow up with the full ¨Bridge VLAN IDs" label, and if it is okay, I would increase the label size. The label and input will not align with the ones above in the same column, though. Not sure if that is a dealbreaker if that input is a bit shorter. I think I would opt for "VLAN IDs". It is only present on linux bridges and with the info box below as well it should be clear what it is for, without causing layout issues. I'd still rather have a docs patch compared to the info box, maybe use a tooltip there instead for a less intrusive hint? The tooltip already explains the format. I tried to add the offloading info. What about the following? "Space-separated list of VLANs and ranges offloaded to the hardware. Useful for NICs with restricted VLAN offloading support. For example: '2 4 100-200'" It is quite a bit, but If we go for a docs patch I am not sure where to put it in our current network docs. We don't seem to have a section listing all the netwokr options and explaining them. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v3] fix #5810: ui: show confirmation/warning dialog for sdn apply
Signed-off-by: Timothy Nicholson --- changes since v2 [0]: - changed icon - modified confirmation message A conditional confirmation message would definitely be nicer, I can send a patch for that later on today as v4. [0]: https://lore.proxmox.com/pve-devel/20241104122457.95494-1-t.nichol...@proxmox.com/ www/manager6/sdn/StatusView.js | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/www/manager6/sdn/StatusView.js b/www/manager6/sdn/StatusView.js index 970aa919..ab500897 100644 --- a/www/manager6/sdn/StatusView.js +++ b/www/manager6/sdn/StatusView.js @@ -41,14 +41,22 @@ Ext.define('PVE.sdn.StatusView', { { text: gettext('Apply'), handler: function() { - Proxmox.Utils.API2Request({ - url: '/cluster/sdn/', - method: 'PUT', - waitMsgTarget: me, - failure: function(response, opts) { - Ext.Msg.alert(gettext('Error'), response.htmlStatus); - }, - }); + 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), + }); + } + } + }) }, }, ], -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v2] fix #5847: ui: gettextize several strings in ui
Am 12.11.24 um 10:38 schrieb Timothy Nicholson: > Signed-off-by: Timothy Nicholson > --- > > I missed one occurence of the String 'TFA' not using gettext, so > here is the updated patch. well that's the only one I'm not 100% certain about, how would such an abbreviation even get translated? Albeit it might make sense to change that to the seemingly more frequent "MFA" (Mutli-factor authentication) variant in a separate patch; I write seemingly, because I only checked a handful of Wikipedia languages for this and that was the one abbreviation that I saw in everyone. https://en.wikipedia.org/wiki/Multi-factor_authentication ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel