5
0
mirror of git://git.proxmox.com/git/pve-firewall.git synced 2025-01-07 21:17:59 +03:00

fix #4204: automatically update usages of group when it is renamed

When renaming a group, the usages didn't get updated automatically. To
get around problems with atomicity, the old rule is first cloned with the
new name, the usages are updated and only when updating has finished, the
old rule is deleted.

The subroutines that lock/update host configs had to be changed so that
it's possible to lock any config, not just the one of the current host.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
This commit is contained in:
Leo Nunner 2022-09-28 11:11:44 +02:00 committed by Wolfgang Bumiller
parent 0b8ac6616f
commit 6198a78f13
4 changed files with 90 additions and 22 deletions

View File

@ -30,6 +30,15 @@ my $get_security_group_list = sub {
return wantarray ? ($list, $digest) : $list;
};
my $rename_fw_rules = sub {
my ($old, $new, $rules) = @_;
for my $rule (@{$rules}) {
next if ($rule->{type} ne "group" || $rule->{action} ne $old);
$rule->{action} = $new;
}
};
__PACKAGE__->register_method({
name => 'list_security_groups',
path => '',
@ -91,35 +100,90 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
my $group = $param->{group};
my $rename = $param->{rename};
my $comment = $param->{comment};
PVE::Firewall::lock_clusterfw_conf(10, sub {
my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
if ($param->{rename}) {
if ($rename) {
my (undef, $digest) = &$get_security_group_list($cluster_conf);
PVE::Tools::assert_if_modified($digest, $param->{digest});
raise_param_exc({ group => "Security group '$param->{rename}' does not exist" })
if !$cluster_conf->{groups}->{$param->{rename}};
raise_param_exc({ group => "Security group '$rename' does not exist" })
if !$cluster_conf->{groups}->{$rename};
# prevent overwriting an existing group
raise_param_exc({ group => "Security group '$param->{group}' does already exist" })
if $cluster_conf->{groups}->{$param->{group}} &&
$param->{group} ne $param->{rename};
raise_param_exc({ group => "Security group '$group' does already exist" })
if $cluster_conf->{groups}->{$group} &&
$group ne $rename;
my $data = delete $cluster_conf->{groups}->{$param->{rename}};
$cluster_conf->{groups}->{$param->{group}} = $data;
if (my $comment = delete $cluster_conf->{group_comments}->{$param->{rename}}) {
$cluster_conf->{group_comments}->{$param->{group}} = $comment;
if ($rename eq $group) {
$cluster_conf->{group_comments}->{$rename} = $comment if defined($comment);
PVE::Firewall::save_clusterfw_conf($cluster_conf);
return;
}
$cluster_conf->{group_comments}->{$param->{group}} = $param->{comment} if defined($param->{comment});
# Create an exact copy of the old security group
$cluster_conf->{groups}->{$group} = $cluster_conf->{groups}->{$rename};
$cluster_conf->{group_comments}->{$group} = $cluster_conf->{group_comments}->{$rename};
# Update comment if provided
$cluster_conf->{group_comments}->{$group} = $comment if defined($comment);
# Write the copy to the cluster config, so that if something fails inbetween, the new firewall
# rules won't be broken when the new name is referenced
PVE::Firewall::save_clusterfw_conf($cluster_conf);
# Update all the host configs to the new copy
my $hosts = PVE::Cluster::get_nodelist();
foreach my $host (@$hosts) {
PVE::Firewall::lock_hostfw_conf($host, 10, sub {
my $host_conf_path = "/etc/pve/nodes/$host/host.fw";
my $host_conf = PVE::Firewall::load_hostfw_conf($cluster_conf, $host_conf_path);
if(defined($host_conf)) {
&$rename_fw_rules($rename,
$group,
$host_conf->{rules});
PVE::Firewall::save_hostfw_conf($host_conf, $host_conf_path);
}
});
}
# Update all the VM configs
my $vms = PVE::Cluster::get_vmlist();
foreach my $vm (keys %{$vms->{ids}}) {
PVE::Firewall::lock_vmfw_conf($vm, 10, sub {
my $vm_type = $vms->{ids}->{$vm}->{type} eq "lxc" ? "ct" : "vm";
my $vm_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $vm_type, $vm, "/etc/pve/firewall");
if (defined($vm_conf)) {
&$rename_fw_rules($rename,
$group,
$vm_conf->{rules});
PVE::Firewall::save_vmfw_conf($vm, $vm_conf);
}
});
}
# And also update the cluster itself
&$rename_fw_rules($rename,
$group,
$cluster_conf->{rules});
# Now that everything has been updated, the old rule can be deleted
delete $cluster_conf->{groups}->{$rename};
delete $cluster_conf->{group_comments}->{$rename};
} else {
foreach my $name (keys %{$cluster_conf->{groups}}) {
raise_param_exc({ group => "Security group '$name' already exists" })
if $name eq $param->{group};
if $name eq $group;
}
$cluster_conf->{groups}->{$param->{group}} = [];
$cluster_conf->{group_comments}->{$param->{group}} = $param->{comment} if defined($param->{comment});
$cluster_conf->{groups}->{$group} = [];
$cluster_conf->{group_comments}->{$group} = $comment if defined($comment);
}
PVE::Firewall::save_clusterfw_conf($cluster_conf);

View File

@ -118,7 +118,7 @@ __PACKAGE__->register_method({
code => sub {
my ($param) = @_;
PVE::Firewall::lock_hostfw_conf(10, sub {
PVE::Firewall::lock_hostfw_conf(undef, 10, sub {
my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
my $hostfw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf);

View File

@ -522,7 +522,7 @@ sub rule_env {
sub lock_config {
my ($class, $param, $code) = @_;
PVE::Firewall::lock_hostfw_conf(10, $code, $param);
PVE::Firewall::lock_hostfw_conf(undef, 10, $code, $param);
}
sub load_config {

View File

@ -3624,10 +3624,12 @@ sub save_clusterfw_conf {
}
}
sub lock_hostfw_conf {
my ($timeout, $code, @param) = @_;
sub lock_hostfw_conf : prototype($$$@) {
my ($node, $timeout, $code, @param) = @_;
my $res = PVE::Cluster::cfs_lock_firewall("host-$nodename", $timeout, $code, @param);
$node = $nodename if !defined($node);
my $res = PVE::Cluster::cfs_lock_firewall("host-$node", $timeout, $code, @param);
die $@ if $@;
return $res;
@ -3643,7 +3645,9 @@ sub load_hostfw_conf {
}
sub save_hostfw_conf {
my ($hostfw_conf) = @_;
my ($hostfw_conf, $filename) = @_;
$filename = $hostfw_conf_filename if !defined($filename);
my $raw = '';
@ -3658,9 +3662,9 @@ sub save_hostfw_conf {
}
if ($raw) {
PVE::Tools::file_set_contents($hostfw_conf_filename, $raw);
PVE::Tools::file_set_contents($filename, $raw);
} else {
unlink $hostfw_conf_filename;
unlink $filename;
}
}